Three fixes where the capability already existed and only the door was missing (#2782, #2975, #2278) #130

Merged

bvandeusen merged 3 commits from dev into main 2026-08-24 07:56:44 -04:00

Conversation 0 Commits 3 Files Changed 18

+708 -38

bvandeusen commented 2026-08-24 07:56:38 -04:00

Owner

Copy Link

Copy Source

Three unrelated pieces of open work, which turned out to share a shape: in every one the thing needed was already built one layer down, and what was missing was the way to reach it.

An annotated record is not a diverged one (#2782)

Pull-time freshness confirmed a cached body by containment — the cached code must appear inside the fetched file. Right for a verbatim record, permanently wrong for a deliberately annotated one, whose commentary is exactly what makes containment fail. #2508 read diverged although its declarations match the source exactly, and always would. That was quietly poisoning decision #2707's scoreboard, which watches body_freshness for diverged.

verification.commit_sha turned out to be purpose-built for the fix — its own docstring (#2688) says it "makes 'the REPO moved on since the check' computable, once the forge integration can compare it against the current head." This is that comparison. A standing verdict can now vouch when containment fails, on four conditions: it says ok, it has not expired against the record's current body, it was not push-invalidated, and the file just fetched is at exactly the commit it was stamped at. That last one keeps it honest — it vouches for one known commit, never for whatever the source became since.

Also worth recording: the cached-SHA short-circuit masks this while the file hasn't moved, so an annotated record reads fine until the source changes once. That intermittency is why it looked like a one-off.

The retrieval tuning loop had no read half (#2975)

retrieval_logs was write-only — rows accrued and the only select() over them in the tree lived in a test. So #1038's gate ("build the reranker once telemetry shows precision is the bottleneck") was unsatisfiable by construction, which is why the one real tuning decision on record was reached by hand-probing the live instance with eight payloads.

retrieval_summary + a retrieval_telemetry MCP tool now return per-surface score distribution and corpus-side pull-through. Read as two aggregates side by side rather than joined: NoteUsageEvent's docstring says the tables are complements and that result_ids can't be indexed at the per-note grain.

The code corrected the implementation twice. pulled is split into agent/human because NoteUsageEvent's comment says that prefix is load-bearing and names #1038 while saying so. And distinct-note counts get their own queries — count(distinct note_id) per group cannot be summed across groups.

Tested against real Postgres deliberately: percentile_cont ... WITHIN GROUP only proves it parses against a database, and #2663 is the standing proof that a mocked lane passes over a query the server rejects. Same reason read_failed is distinct from an empty window.

The last two parallel-family gaps (#2278) — closes milestone #253

Both halves began with enumeration, and counting changed the answer twice.

The 19 list_* tools are genuinely heterogeneous, so the guard this task imagined would have invented a convention the API lacks. The real contract is narrower: a limit without an offset is a truncation with no continuation. Two tools had it, and both were capped over a service that already accepted an offset — list_snippets never exposed it, list_processes hardcoded offset=0. Guarded, and verified non-vacuous against the pre-fix tree.

The verb pairs produced no finding, which is itself the deliverable: preview/apply and dry_run/commit exist nowhere in the 102 tools. create_system lacking force is a deliberate non-instance — its gate is an exact-name match, and forcing it would split one area's records across two piles.

The hook config preamble was in all five hooks, not three, and four lines rather than two — the extra two guard against an unexpanded ${...} being sent as a garbage Bearer token. Now scribe_config, which sets globals rather than echoing (a token should not pass through subshell output) and returns a status so session_context can still serve its static floor unconfigured.

A checker gap fell out of it: check_plugin.py ran shellcheck without -x, so shared helpers were invisible — every variable they set read as unassigned and every bug inside them went unlinted at the call site. Fixed.

Verification

CI green on every commit — runs 4343, 4346, 4388. All five hooks additionally probed live against the instance beyond the twelve fail-open scenarios. Plugin 0.1.46 (#2209).

Three unrelated pieces of open work, which turned out to share a shape: in every one the thing needed was already built one layer down, and what was missing was the way to reach it. ## An annotated record is not a diverged one (#2782) Pull-time freshness confirmed a cached body by containment — the cached code must appear inside the fetched file. Right for a verbatim record, permanently wrong for a deliberately annotated one, whose commentary is exactly what makes containment fail. #2508 read `diverged` although its declarations match the source exactly, and always would. That was quietly poisoning decision #2707's scoreboard, which watches `body_freshness` for `diverged`. `verification.commit_sha` turned out to be purpose-built for the fix — its own docstring (#2688) says it "makes 'the REPO moved on since the check' computable, once the forge integration can compare it against the current head." This is that comparison. A standing verdict can now vouch when containment fails, on four conditions: it says `ok`, it has not expired against the record's current body, it was not push-invalidated, and the file just fetched is at **exactly** the commit it was stamped at. That last one keeps it honest — it vouches for one known commit, never for whatever the source became since. Also worth recording: the cached-SHA short-circuit masks this while the file hasn't moved, so an annotated record reads fine until the source changes *once*. That intermittency is why it looked like a one-off. ## The retrieval tuning loop had no read half (#2975) `retrieval_logs` was write-only — rows accrued and the only `select()` over them in the tree lived in a test. So #1038's gate ("build the reranker once telemetry shows precision is the bottleneck") was unsatisfiable by construction, which is why the one real tuning decision on record was reached by hand-probing the live instance with eight payloads. `retrieval_summary` + a `retrieval_telemetry` MCP tool now return per-surface score distribution and corpus-side pull-through. Read as two aggregates side by side rather than joined: `NoteUsageEvent`'s docstring says the tables are complements and that `result_ids` can't be indexed at the per-note grain. The code corrected the implementation twice. `pulled` is split into agent/human because `NoteUsageEvent`'s comment says that prefix is load-bearing and names #1038 while saying so. And distinct-note counts get their own queries — `count(distinct note_id)` per group cannot be summed across groups. Tested against real Postgres deliberately: `percentile_cont ... WITHIN GROUP` only proves it parses against a database, and #2663 is the standing proof that a mocked lane passes over a query the server rejects. Same reason `read_failed` is distinct from an empty window. ## The last two parallel-family gaps (#2278) — closes milestone #253 Both halves began with enumeration, and counting changed the answer twice. The 19 `list_*` tools are genuinely heterogeneous, so the guard this task imagined would have invented a convention the API lacks. The real contract is narrower: a `limit` without an `offset` is a truncation with no continuation. Two tools had it, and both were capped over a service that already accepted an offset — `list_snippets` never exposed it, `list_processes` hardcoded `offset=0`. Guarded, and verified non-vacuous against the pre-fix tree. The verb pairs produced no finding, which is itself the deliverable: `preview`/`apply` and `dry_run`/`commit` exist nowhere in the 102 tools. `create_system` lacking `force` is a deliberate non-instance — its gate is an exact-name match, and forcing it would split one area's records across two piles. The hook config preamble was in all **five** hooks, not three, and four lines rather than two — the extra two guard against an unexpanded `${...}` being sent as a garbage Bearer token. Now `scribe_config`, which sets globals rather than echoing (a token should not pass through subshell output) and returns a status so `session_context` can still serve its static floor unconfigured. A checker gap fell out of it: `check_plugin.py` ran shellcheck without `-x`, so shared helpers were invisible — every variable they set read as unassigned and every bug inside them went unlinted at the call site. Fixed. ## Verification CI green on every commit — runs 4343, 4346, 4388. All five hooks additionally probed live against the instance beyond the twelve fail-open scenarios. Plugin 0.1.46 (#2209).

bvandeusen added 3 commits 2026-08-24 07:56:38 -04:00

fix(snippets): an annotated record is not a diverged one — a standing verdict vouches at its commit (#2782) ... 446d6da0d7

Pull-time freshness confirms a cached body by containment: normalised cached
code must appear inside the fetched file. That is right for a record kept
verbatim and permanently wrong for a deliberately annotated one. A record
whose job is to say WHY the shape is what it is carries commentary the source
does not, so containment fails on every pull, forever — #2508 was reading
`diverged` although its declarations match the source exactly, and always
would. Annotation is a sanctioned record style, so this was two deliberate
designs colliding, and it was quietly poisoning the one honest drift signal:
decision #2707's scoreboard watches body_freshness for `diverged` and was
accruing false positives it could never age out.

The escape hatch is the verdict itself. verify_snippet is exactly where a
human or agent already judged this body a faithful rendering of that source,
and `verification.commit_sha` records the repo commit they judged it at — a
field whose own docstring (#2688) anticipated this: "makes 'the REPO moved on
since the check' computable, once the forge integration can compare it against
the current head." This is that comparison. When containment fails, a standing
verdict can still vouch, on four conditions and no fewer:

  - the verdict says `ok`;
  - it has not EXPIRED — verification_view recomputes code_sha against the
    record's current body, so editing the record retires the verdict;
  - it was not INVALIDATED by a push touching the location (#2691);
  - the file just fetched is at the very commit the verdict was stamped at.

That last one is what keeps it honest: the hatch vouches for a body against
ONE known commit, never against whatever the source became since. The moment
the file moves, containment resumes as the authority and the record reads
`diverged` until someone re-verifies — correct, because at that point nobody
has looked. The first three are checked by reusing verification_view rather
than restating its rule, so "expired" keeps meaning one thing.

Nothing is rewritten and no new freshness value is minted; `verification`
already travels in the same payload, so a reader can see the basis rather
than take "current" on faith. Verdicts predating commit stamping carry no
commit to compare and therefore do not vouch — they fall through to
containment rather than passing on age alone.

Tests pin the fix and, more usefully, every condition that switches it back
off: moved commit, expired verdict, push-invalidated verdict, non-ok verdict,
no verdict, and a pre-#2688 verdict with no commit_sha. Plus a regression
that a verbatim record still takes the containment path untouched.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

feat(telemetry): a read surface over retrieval_logs — the tuning loop had no read half (#2975) ... 64bfa5725f

`retrieval_logs` was write-only. `record_retrieval` inserted rows and nothing
in the tree ever selected from them: the only `select()` over RetrievalLog
lived in a test. So #1038's gate — "build the reranker once telemetry shows
precision is the bottleneck" — was unsatisfiable by construction, and the one
real tuning decision on record (the 0.68 write-path threshold, #2223) had to
be reached by hand-probing the live instance with eight payloads. This adds
the half that was missing.

`retrieval_summary(user_id, days=30)` returns two aggregates side by side,
each read from the table built for it — NOT a join. NoteUsageEvent's docstring
is explicit that the two are complements ("RetrievalLog tunes the threshold,
this tunes the corpus") and that RetrievalLog's JSONB `result_ids` cannot be
indexed at the per-note grain, so correlating through it would be both slower
and less honest than reading each source directly. That corrects the approach
sketched on the task.

  - `sources`, per surface: calls, zero_result_calls, cleared_threshold (how
    often the best hit beat the threshold in force for THAT call), the
    top_score spread as p10/p50/p90/min/max, avg_result_count, p90 duration.
    Zero-result calls are counted apart from low-scoring ones — they are a
    different failure and averaging them together would hide both.
  - `usage`, from note_usage_events: ranked surfacings, ambient surfacings,
    and pulls split into `pulled_by_agent` / `pulled_by_human`.

That split is not decoration. NoteUsageEvent's own comment says the mcp_/rest_
prefix is load-bearing and names #1038 while saying so: "is this dead weight?"
is answered by any pull, "was that injected line useful?" only by an agent
pull. `pull_through` exists to answer the second, so it counts agent pulls
over ranked surfacings; both halves ship so the first stays answerable.

Two things the code made me get right rather than guess:

  - Distinct-note counts get their own queries. `count(distinct note_id)` per
    (event, source) group cannot be summed across groups — a note surfaced by
    two sources is one distinct note and would be counted twice. A wrong
    number labelled "distinct" is worse than no number.
  - No CASE in the GROUP BY. #2663 is the bug where a second case() rendered
    its own expanding bind names, Postgres rejected the query, a broad except
    swallowed it, and every counter read zero in production while mocked tests
    passed. Grouping on raw `source` and classifying in Python cannot fail
    that way. For the same reason the readout distinguishes `read_failed` from
    an empty window, and its tests are integration against real Postgres —
    percentile_cont ... WITHIN GROUP only proves it parses against a database.

Exposed as the `retrieval_telemetry` MCP tool, added to `_READ_ONLY_TOOLS`:
it mutates nothing, but its name carries no read prefix, so the completeness
test cannot derive it and it would otherwise have failed closed for read-only
keys in silence — the same reason `enter_project` is spelled out there. Docs
updated to name both exceptions rather than leave the rule looking derivable.

Scoped to the caller's own telemetry: a retrieval log records what one user's
agent asked for, query text included, and is not a shared record kind — the
owner filter is the whole access rule, not a shortcut past access.py.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

refactor(plugin+tests): the last two parallel-family gaps — pageable list tools, one config preamble (#2278) ... a8f35e465e

DRY pass 3's remainder. Both halves start from enumeration, because the task's
candidate list was hypotheses and the process requires counting before
proposing — and counting changed the answer twice.

## The list_* family: a limit with no offset

Enumerated all 19 `list_*` MCP tools first. They are genuinely heterogeneous —
8 take `project_id`, 6 take `limit`, six take no arguments at all — so a
common-parameter guard would invent a convention the API does not have, which
is the over-DRY trap (§5). One contract IS real: a `limit` without an `offset`
is a truncation with no continuation. The caller is told there are 250 results,
handed 50, and given no way to ask for the rest.

Two tools had it, and both were capped over a service that already accepted an
offset: `snippets_svc.list_snippets(offset=0)` was simply not exposed, and
`list_processes` passed a hardcoded `offset=0` into `query_knowledge`. The
capability existed one layer down in both; only the door was missing — the
missing-sibling shape exactly. Both now expose it.

`tests/test_mcp_list_family.py` guards it, with `list_tags` exempted for a
stated reason (a ranked top-N over a bounded vocabulary has no "rest" to page
into). Candidates derived, decision explicit, same design as test_mcp_auth —
plus the reverse checks: a stale exemption, and an offset with no limit, which
would page through an unbounded result set. Verified non-vacuous by running the
sweep against the pre-fix tree, where it fails naming both tools.

## The verb pairs: no finding, which is the finding

`preview`/`apply` and `dry_run`/`commit` do not exist anywhere in the 102
tools — those were guesses about a shape Scribe never adopted. `count_*` does
not exist either. Of the create/delete stems only `project_rule` lacks a
`delete_X`, and deliberately: a project rule IS a rule, `delete_rule` removes
it, and the docstring says so. `force` sits on 6 of 7 duplicate-gated creates;
the exception is `create_system`, whose gate is an exact normalized-NAME match
rather than a semantic near-match — forcing it would split one area's records
across two piles, which its own message explains. No guard added: it would
need a seven-entry exemption list to defend against a hypothetical. Recorded
on the leave-alone list instead, which the process asks for by name.

## The hook config preamble

Not 3 of 6 hooks as recorded — all FIVE carried their own copy, and of four
lines rather than two. The extra two are a guard treating an unexpanded
`${...}` placeholder as unset, so it is never sent as a garbage Bearer token:
precisely the correctness detail a sixth hook would omit with nothing failing
loudly. Now `scribe_config` in scribe_defs.sh, which also declares the two
names it owns. It sets globals rather than echoing, so a token never passes
through a subshell's output where xtrace or a log could catch it, and returns
a status so a caller can bail (`|| exit 0`) or continue degraded — the
session-context hook still owes its static floor when Scribe is unconfigured.

`check_plugin.py` now runs shellcheck with `-x`. Without it the shared helpers
were invisible: every variable they set read as unassigned and every bug inside
them went unlinted at the call site, which is the opposite of what sharing them
was for. All twelve fail-open scenarios still pass, and all five hooks were
probed live against the instance — prior_art and after_write both still name
canon, autoinject returns context, session_context serves 11k chars of rules,
sync_processes stays silent. Plugin 0.1.46 (#2209).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

bvandeusen merged commit 6b1f5e8031 into main 2026-08-24 07:56:44 -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#130