Telemetry coverage, and the rule arms stop filtering to one tier #143

Merged

bvandeusen merged 4 commits from dev into main 2026-09-08 11:11:40 -04:00

Conversation 0 Commits 4 Files Changed 3

+339 -5

bvandeusen commented 2026-09-08 11:11:34 -04:00

Owner

Copy Link

Copy Source

Five commits, two tasks and one issue. Both halves are about a readout or a retrieval that looked correct while being wrong.

#3702 — the rule arms stop filtering the corpus to one tier

plugin_context.py's write-path and pre-tool arms searched with tier="conditional", so an always_on rule could never be retrieved at the moment it applied. The reasoning behind that filter was that a resident rule is already in context — but PRESENT IN CONTEXT and SALIENT AT THE MOMENT are different properties, and a rule preloaded thirty turns ago competing with everything since is not the same as one surfaced against the file being edited. Both arms now search the whole corpus.

The 0.72 threshold's rationale needed updating with it: its stated premise was that the eligible corpus is tiny because it was conditional-only, which is no longer the case. semantic_search_rules's docstring stopped documenting a decision it no longer makes.

#3712 — a window that predates a counter must say so

Every counter block now carries complete_from and covers_window, so a window opening before a counter started reports that rather than a plausible fraction. This is the defect that cost milestone #379 five steps aimed at a loss that did not exist.

Two design calls, both places where the cheaper implementation rebuilds the original bug:

• The grain is the source, not the table. retrieval_logs goes back months; a table-level answer lets an old surface vouch for an arm that started last week.
• A section takes its latest contributor, not its earliest. A total is complete only once every contributor was being written, so the aggregate is a max. A min is the same error in miniature.

covers_window is null rather than false when nothing was recorded — the convention #3497 set for suppression, one level up.

Also corrects a stale claim in the tool docstring: it taught readers that write_path_rule had "never once declined to fire" (#3311), which #3497 falsified.

#3720 — a surface that stopped recording is not one that never ran

Found while fixing #3712's own fixture. sources was built only from the windowed query, so a source with rows in the table but none in the window was absent — indistinguishable from a source that never existed. Every such source now gets a zero bucket.

Zero is a real measurement here: the all-time query proves the source was recording, and it made no calls across a window it fully covers. But only the calls are observed — the score fields stay null, because rendering p50 as 0.0 for a source nobody sampled would assert a distribution that was never measured.

Notes

• 950c93c fixes a self-inflicted one: the two new helpers were written between retrieval_summary's docstring and its body, and since blank lines don't close a block, the whole function body became unreachable code inside a sync helper.
• Both features are server-side and inert until this is deployed. #3702's "after" read needs a day or two of sessions once it is live.

CI green on 7a2aff7 (run 6010), all six lanes.

🤖 Generated with Claude Code

https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ

Five commits, two tasks and one issue. Both halves are about a readout or a retrieval that looked correct while being wrong. ## #3702 — the rule arms stop filtering the corpus to one tier `plugin_context.py`'s write-path and pre-tool arms searched with `tier="conditional"`, so an `always_on` rule could never be retrieved at the moment it applied. The reasoning behind that filter was that a resident rule is already in context — but PRESENT IN CONTEXT and SALIENT AT THE MOMENT are different properties, and a rule preloaded thirty turns ago competing with everything since is not the same as one surfaced against the file being edited. Both arms now search the whole corpus. The 0.72 threshold's rationale needed updating with it: its stated premise was that the eligible corpus is tiny *because* it was conditional-only, which is no longer the case. `semantic_search_rules`'s docstring stopped documenting a decision it no longer makes. ## #3712 — a window that predates a counter must say so Every counter block now carries `complete_from` and `covers_window`, so a window opening before a counter started reports that rather than a plausible fraction. This is the defect that cost milestone #379 five steps aimed at a loss that did not exist. Two design calls, both places where the cheaper implementation rebuilds the original bug: - **The grain is the source, not the table.** `retrieval_logs` goes back months; a table-level answer lets an old surface vouch for an arm that started last week. - **A section takes its latest contributor, not its earliest.** A total is complete only once every contributor was being written, so the aggregate is a `max`. A `min` is the same error in miniature. `covers_window` is null rather than false when nothing was recorded — the convention #3497 set for `suppression`, one level up. Also corrects a stale claim in the tool docstring: it taught readers that `write_path_rule` had "never once declined to fire" (#3311), which #3497 falsified. ## #3720 — a surface that stopped recording is not one that never ran Found while fixing #3712's own fixture. `sources` was built only from the windowed query, so a source with rows in the table but none in the window was absent — indistinguishable from a source that never existed. Every such source now gets a zero bucket. Zero is a real measurement here: the all-time query proves the source was recording, and it made no calls across a window it fully covers. But only the *calls* are observed — the score fields stay null, because rendering p50 as 0.0 for a source nobody sampled would assert a distribution that was never measured. ## Notes - `950c93c` fixes a self-inflicted one: the two new helpers were written between `retrieval_summary`'s docstring and its body, and since blank lines don't close a block, the whole function body became unreachable code inside a sync helper. - Both features are server-side and inert until this is deployed. #3702's "after" read needs a day or two of sessions once it is live. CI green on `7a2aff7` (run 6010), all six lanes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ

bvandeusen added 4 commits 2026-09-08 11:11:34 -04:00

feat(telemetry): every counter says when it started being recorded (#3712) ... 21a5831479

A window that opens before a counter existed reports that counter as though
it had been measured throughout. The reader cannot tell "zero because
nothing happened" from "zero because nobody was counting yet", and — worse
— cannot tell a partial count from a complete one. That middle case yields
a plausible FRACTION rather than an obvious zero, which is what makes it
dangerous.

It is not hypothetical. A 7-day window opened while the ranked rule
surfacing recorders were four days old produced an apparent 64% write loss,
which survived a code review, four ruled-out alternative causes and a
five-step milestone before an identity check falsified it in one read.

Every counter block now carries `complete_from` and `covers_window`.

THE GRAIN IS THE SOURCE. retrieval_logs accumulates for months, so a
per-table earliest row says months for every source it holds — including an
arm added days ago whose counter means something else entirely. The old
source would vouch for the young one, which is the exact reading this
prevents.

A SECTION TAKES ITS LATEST CONTRIBUTOR, NOT ITS EARLIEST. A figure summing
several sources is complete only once every one of them was being written,
so "*" is a max. Using min would reproduce the original error in miniature.

`covers_window` is null, never false, when nothing was ever recorded: "no
measurement" is not "partial measurement" — the null convention #3497
established for `suppression`, one level up.

Also corrects a stale claim in the tool docstring: it still taught readers
that write_path_rule "has never once declined to fire" (#3311). That was
the arm writing its retrieval_logs row only on calls that found something;
#3497 fixed it, and the arm declines the large majority of its calls.

_complete_from takes the caller's session rather than opening its own,
departing from the services canon (#2860) because it runs inside an
existing block; to be recorded against the ledger once it ingests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ

fix(telemetry): the coverage helpers were defined inside the function they serve (#3712) ... 950c93c5d4

`_complete_from` and `_coverage` landed between `retrieval_summary`'s
docstring and its body. Python does not object to that the way it looks
like it should: blank lines do not close a block, so the whole remaining
body — indented four spaces, sitting after `_coverage`'s `return` —
became unreachable code INSIDE `_coverage`, and `retrieval_summary`
became a function that is nothing but a docstring.

The error surfaced three ways at once, none of which named the cause:
fourteen F821s for `days` and `user_id` (real: those are
`retrieval_summary`'s parameters, and the body no longer lived there), a
SyntaxError on `async with` (real: `_coverage` is sync), and every test
module that imports this file failing to collect.

Moved both helpers above `retrieval_summary`, beside `_bucket` and
`_round`, where the file's other helpers already are. No behaviour
change — this is the code that was meant to be there.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ

test(telemetry): the old surface needs a row in the window to have a bucket at all (#3712) ... 0808e8259a

The per-source grain test gave `auto_inject` a single row 90 days back and
`pre_tool_rule` one 2 days back, then asserted on both buckets. Only the
young arm got a bucket: `out["sources"]` is built from the WINDOWED query,
so a source with no rows inside the window is absent entirely, and the
assertion died on KeyError before it could test anything.

`complete_from` and the bucket come from different queries — all-time for
the first, windowed for the second — and the fixture only satisfied one of
them. Gave `auto_inject` a second row inside the window, which is also the
shape being described: an old surface that is STILL recording. The 90-day
row still sets its `complete_from`.

Still discriminating: auto_inject reads True and pre_tool_rule False, and a
per-table `_complete_from` would make both 90 days and fail the second
assertion — the regression this test is for.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ

fix(telemetry): a surface that stopped recording is not one that never ran (#3720) ... 7a2aff7bc1

`out["sources"]` was built only from the windowed aggregate, so a source
with rows in `retrieval_logs` but none inside the window got no bucket at
all. Absent is exactly how a source that never existed renders, so a
surface that WAS recording and went silent became unreadable — #2663 one
level up, the failure that looks like the correct answer.

Two queries at different scopes, and only one shaped the output.
`_complete_from` reads all-time and knows every source the table has ever
held; the windowed loop dropped whatever it did not return.

Every such source now gets a zero bucket. Zero is a real measurement here
rather than a manufactured one: the all-time query proves the source was
recording, and it made no calls across a window it fully covers. No
`covers_window` special case is needed either — a source whose first row
fell after `since` would have that row IN the window and already hold a
bucket, so anything reaching this branch began before it.

The counts are 0 and everything else is null. A sampled distribution is
not the same claim as a call count, and rendering p50 as 0.0 for a source
nobody sampled would assert a measurement — #3311's mistake, in the
readout built to prevent it.

Found while fixing #3712's fixture, which failed with KeyError for this
exact reason.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011cPyzNnegXHr5iRMzzy5KJ

bvandeusen merged commit be08edcd7e into main 2026-09-08 11:11:40 -04:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate-bot bvandeusen

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: bvandeusen/FabledScribe#143