Drafter hardening (milestone #232) — plugin hook fix, usage signal, drift check, duplicate finder, un-merge #82

Merged
bvandeusen merged 6 commits from dev into main 2026-07-28 20:10:42 -04:00
Owner

Completes milestone #232. Six commits, CI green on each.

The blocker, fixed first

Every plugin hook was inert, silently (#2198). Three independent defects, each fatal alone:

  1. Wrong env var case. Claude Code exports userConfig to hooks as CLAUDE_PLUGIN_OPTION_<KEY> uppercased; all four hooks read the lowercase spelling, so URL and token were always empty. This disabled the SessionStart dynamic tier, process sync, prompt auto-inject, and the write-path trigger at once.
  2. jq -rR '@uri' encodes line by line. A multi-line payload came back as several encoded lines joined by raw newlines — invalid URL, curl fails, hook exits 0 in silence. Auto-inject only ever worked for single-line prompts; the write-path trigger sends code, so it could never have worked at all.
  3. cut -c1-1200 caps each line, not the payload — so the prior-art size budget wasn't one.

Verified live against the running instance after the fix: dynamic rules + project context load, auto-inject surfaces a snippet on a multi-line prompt, and the write-path trigger returns its place-arm hit with session dedup suppressing the repeat. That closes the outstanding live verification for #2082, which was blocked rather than broken.

The SessionStart warning now also covers "neither URL nor token arrived" — previously the single install state that produced no signal at all, which is exactly the state defect 1 created.

The record can now age honestly

Three complementary signals — the three ways a snippet corpus decays:

  • #2085 unused — new note_usage_events (migration 0071) records surfaced vs pulled. Closes the gap #2082 left open: the write-path place arm carries no score and so has no home in retrieval_logs, meaning the arm firing on the strongest possible claim was the one nobody could measure. Both arms now emit events under distinct sources.
  • #2086 wrong — agent-side drift check (verify_snippet). The server has no checkout and deliberately never gets one. A verdict is stamped with a hash of the code it was checked against, so it expires by itself on edit — no invalidation logic to maintain or forget. verification=attention catches failing and expired verdicts in one index-served predicate.
  • #2088 redundant — near-duplicate finder over the existing embeddings, one indexed pgvector self-join, grouped into merge sets by connected components. Threshold is a setting (0.82), deliberately looser than the 0.90 write gate: the gate blocks a create and must be unforgiving, this only proposes a merge under review.

Plus #2165 un-merge — the exact inverse of merge. Per-source attribution is now recorded at merge time (each entry keeps only what that source added), which makes partial un-merge exact and makes it impossible to strip a call site the survivor legitimately owns.

Two bugs found in passing, fixed here

  • The merge modal filtered its selection against the current page while doMerge derived source ids from that list — a corpus-wide suggested group would have rendered incomplete and silently merged only the visible subset. Latent before this work; a corpus-wide selection exposed it.
  • trash had no per-entity restore, only batch. Added restore_entity, the missing inverse of delete().

Known state after merge

  • The corpus is one snippet. Every counter starts at zero and measures forward — none of this retroactively judges what's recorded, and the duplicate finder has nothing to pair yet.
  • #2204 is open and systemic: plugin/** isn't in CI's paths: filter, and the plugin installs straight from this repo — so plugin changes are untested and live on push. That is how the three hook defects reached a real install. The Python/UI work here is covered; the hooks still have no CI.
  • The plugin manifest is at 0.1.18; an install needs /plugin update + /reload-plugins to pick up the hook fixes.

🤖 Generated with Claude Code

https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs

Completes milestone #232. Six commits, CI green on each. ## The blocker, fixed first **Every plugin hook was inert, silently** (#2198). Three independent defects, each fatal alone: 1. **Wrong env var case.** Claude Code exports userConfig to hooks as `CLAUDE_PLUGIN_OPTION_<KEY>` **uppercased**; all four hooks read the lowercase spelling, so URL and token were always empty. This disabled the SessionStart dynamic tier, process sync, prompt auto-inject, and the write-path trigger at once. 2. **`jq -rR '@uri'` encodes line by line.** A multi-line payload came back as several encoded lines joined by raw newlines — invalid URL, curl fails, hook exits 0 in silence. Auto-inject only ever worked for single-line prompts; the write-path trigger sends *code*, so it could never have worked at all. 3. **`cut -c1-1200` caps each line, not the payload** — so the prior-art size budget wasn't one. Verified live against the running instance after the fix: dynamic rules + project context load, auto-inject surfaces a snippet on a multi-line prompt, and the write-path trigger returns its place-arm hit with session dedup suppressing the repeat. That closes the outstanding live verification for **#2082**, which was blocked rather than broken. The SessionStart warning now also covers "neither URL nor token arrived" — previously the single install state that produced no signal at all, which is exactly the state defect 1 created. ## The record can now age honestly Three complementary signals — the three ways a snippet corpus decays: - **#2085 unused** — new `note_usage_events` (migration 0071) records *surfaced* vs *pulled*. Closes the gap #2082 left open: the write-path place arm carries no score and so has no home in `retrieval_logs`, meaning the arm firing on the strongest possible claim was the one nobody could measure. Both arms now emit events under distinct sources. - **#2086 wrong** — agent-side drift check (`verify_snippet`). The server has no checkout and deliberately never gets one. A verdict is stamped with a hash of the code it was checked against, so it expires by itself on edit — no invalidation logic to maintain or forget. `verification=attention` catches failing *and* expired verdicts in one index-served predicate. - **#2088 redundant** — near-duplicate finder over the existing embeddings, one indexed pgvector self-join, grouped into merge sets by connected components. Threshold is a setting (0.82), deliberately looser than the 0.90 write gate: the gate blocks a create and must be unforgiving, this only proposes a merge under review. Plus **#2165 un-merge** — the exact inverse of merge. Per-source attribution is now recorded at merge time (each entry keeps only what that source *added*), which makes partial un-merge exact and makes it impossible to strip a call site the survivor legitimately owns. ## Two bugs found in passing, fixed here - The merge modal filtered its selection against the **current page** while `doMerge` derived source ids from that list — a corpus-wide suggested group would have rendered incomplete and silently merged only the visible subset. Latent before this work; a corpus-wide selection exposed it. - `trash` had no per-entity restore, only batch. Added `restore_entity`, the missing inverse of `delete()`. ## Known state after merge - **The corpus is one snippet.** Every counter starts at zero and measures forward — none of this retroactively judges what's recorded, and the duplicate finder has nothing to pair yet. - **#2204 is open and systemic:** `plugin/**` isn't in CI's `paths:` filter, and the plugin installs straight from this repo — so plugin changes are untested *and* live on push. That is how the three hook defects reached a real install. The Python/UI work here is covered; the hooks still have no CI. - The plugin manifest is at 0.1.18; an install needs `/plugin` update + `/reload-plugins` to pick up the hook fixes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs
bvandeusen added 6 commits 2026-07-28 20:10:32 -04:00
Three defects, each of which independently made a hook a no-op, and all
three failing silently — which is why the whole dynamic side of the plugin
looked "shipped" while doing nothing.

1. Wrong env var case. Claude Code exports userConfig to hooks as
   CLAUDE_PLUGIN_OPTION_<KEY> with the key UPPERCASED. All four hooks read
   CLAUDE_PLUGIN_OPTION_api_endpoint / _api_token, so both values were
   always empty. That killed the SessionStart dynamic tier, process sync,
   prompt auto-inject, and the write-path prior-art trigger at once.

2. jq -rR is line-oriented. `@uri` under -R encodes input LINE BY LINE, so
   a multi-line payload came back as several encoded lines joined by raw
   newlines — an invalid URL, curl fails, hook exits 0 in silence. Now
   -sRr. This one hid behind (1): auto-inject only ever worked for
   single-line prompts, and prior-art (which posts code, always
   multi-line) could never have worked at all.

3. cut -c1-1200 caps each LINE, not the payload, so the prior-art code
   budget wasn't a budget. Now head -c 1200.

Also widens the SessionStart warning: "neither URL nor token arrived" used
to be treated as a benign unconfigured install and stayed quiet. That is
exactly the state defect (1) produced, so the one install state that most
needed a signal was the only one that emitted none. It now says so, and
names the two other features it silently disables.

Verified against the live instance: dynamic rules + project context load,
auto-inject surfaces #2192 on a multi-line prompt, and the write-path
trigger returns the [here] place-arm hit on a multi-line edit with session
dedup suppressing the repeat.

Refs #2198, #2082

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs
feat(snippets): usage signal — was a surfaced record ever actually pulled?
CI & Build / Python lint (push) Successful in 2s
CI & Build / integration (push) Successful in 31s
CI & Build / TypeScript typecheck (push) Successful in 34s
CI & Build / Python tests (push) Successful in 56s
CI & Build / Build & push image (push) Successful in 44s
2b85443dd1
retrieval_logs answers "what did the ranker return, at what scores" — the
right substrate for tuning a threshold. It cannot answer the question the
snippet corpus actually needs: did anyone open this? A snippet nobody
opens is not neutral. It takes a slot in every future auto-inject menu and
crowds out something useful.

Adds note_usage_events (migration 0071): one row per note per event,
either 'surfaced' (we put its title in front of an agent) or 'pulled'
(someone opened it in full), tagged with which surface produced it.

Closes the gap #2082 recorded against this work. The write-path PLACE arm
carries no score, so it has no home in retrieval_logs — folding it in
would corrupt the score distribution that table exists to capture. The
result was that the arm firing on the STRONGEST claim ("there is already a
canonical helper in this exact file") was the one arm nobody could
measure. Both arms now emit usage events under distinct sources, so their
pull-through rates are finally comparable.

Deliberate departures from the task as written:

- Not in-session correlation. The original framing was "correlate
  result_ids against a later get_note in the same session." There is no
  session identity server-side — the MCP endpoint is stateless and the
  hooks send no session id — and adding one would mean threading an
  opaque client-supplied token through every read path. Two independent
  counters answer the question without it: surfaced 40×, pulled 0 is dead
  weight regardless of how those events distribute across sessions.

- Pulls record at the ENTRY POINTS (MCP tools, REST detail route), not in
  snippets_svc.get_snippet, which update and merge also reach. Counting
  those would inflate precisely the number meant to say "someone chose to
  look at this."

- get_note records for every note kind, not just snippets. The auto-inject
  menu surfaces tasks and processes too; scoping this to snippets would
  pin those at zero pulls forever and make them read as dead weight next
  to snippets that merely had a counter.

Surfaced in the Snippets list as an "N/M used" badge, warning-toned once a
record has been offered 3+ times and never opened, with the tooltip saying
what to do about it (usually: its "when to reach for it" doesn't say
when). No badge at all below one surfacing — "0/0" reads as a verdict when
it's an absence of evidence. Also returned from MCP list_snippets so the
agent can see dead weight without opening the UI.

Telemetry keeps the retrieval_telemetry contract throughout: writes are
fire-and-forget, reads degrade to zeroes, and no path can raise into the
surface it observes.

Refs #2085

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs
feat(snippets): drift check — verify a snippet still matches its source
CI & Build / Python lint (push) Successful in 3s
CI & Build / integration (push) Successful in 19s
CI & Build / TypeScript typecheck (push) Failing after 20s
CI & Build / Python tests (push) Failing after 22s
CI & Build / Build & push image (push) Has been skipped
35f3f09d12
A recorded snippet points at a repo · path · symbol that WILL rot: files
move, symbols get renamed, implementations diverge from the copy stored
here. Nothing detected any of it, so a record degraded silently from
"canonical reference" to "confidently wrong" — worse than no record, since
it is surfaced with the same authority either way.

WHERE THE CHECK RUNS. Agent-side, which the task flagged as the design
question to settle first. Scribe has no checkout of the operator's repos
and must not acquire one: giving the server repo access would make every
install a credential problem and break instance-agnosticism (rule #115).
The agent already has the working tree, so it does the comparing; the
server remembers the verdict, makes it queryable, and knows when it has
expired. New MCP tool verify_snippet teaches the four-step procedure and
records the result; a REST endpoint mirrors it so the UI can clear a
marker after a manual fix.

WHY THE VERDICT CARRIES A CODE HASH. A verdict describes the code it was
checked against. Invalidating it on edit means deciding which edits count
— a when_to_use tweak shouldn't void a code check, a rewrite must — which
is fiddly and easy to get subtly wrong, and easy for a new write path to
forget entirely. Stamping the verdict with a hash sidesteps all of it: one
whose code_sha no longer matches is self-evidently expired, computed at
read time, no invalidation branch to maintain.

That makes "expired" the interesting filter case. It is not `drifted`
(nothing was found wrong) and not `unverified` (a check did happen), yet
it plainly needs looking at — so `verification=attention` covers both. To
keep that one index-served predicate rather than a post-filter that would
make the pagination total a lie, data now also mirrors the CURRENT code's
fingerprint as data.code_sha, and a jsonpath compares the two fields
within the row. The filter is implemented in both dialects, SQL and
Python, for the same reason the location filter is: the semantic arm's
candidates arrive already fetched.

A merge deliberately carries no verdict forward — the survivor's code is a
union of several sources, so no prior check describes it, and unverified
is the honest answer.

UI: a danger-toned drift badge on each card (an actively misleading record
outranks a merely unused one), and a "Needs attention" filter. Its empty
state says plainly that never-verified snippets don't appear there —
otherwise `attention` would mean "everything" on day one and be useless as
a worklist.

Refs #2086

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs
fix(snippets): satisfy the parity guards the drift check tripped
CI & Build / Python lint (push) Successful in 3s
CI & Build / integration (push) Successful in 16s
CI & Build / TypeScript typecheck (push) Successful in 34s
CI & Build / Python tests (push) Successful in 52s
CI & Build / Build & push image (push) Successful in 45s
84c5c0dc81
Two CI failures, both the guards working as intended rather than defects
in them:

- The MCP tool-name manifest and the REST route/service parity lists are
  explicit, so a new capability has to be added to both surfaces or the
  test fails. verify_snippet / verify_snippet_route / record_verification
  added, plus an assertion that the `verification` filter reaches both
  callers — a verdict an agent records must be visible to the human
  looking at the same corpus, or the two surfaces disagree about what's
  rotten (rule #33).

- vue-tsc rejected two object-literal lookups keyed by the status union:
  `status` includes "ok" and "unverified", and an object literal has to
  enumerate every member even just to say "nothing to show for these".
  Typed as Record<string, string>, which is what the ?? "" fallback
  already assumed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs
feat(snippets): near-duplicate finder — surface the sets worth merging
CI & Build / Python lint (push) Successful in 3s
CI & Build / TypeScript typecheck (push) Successful in 14s
CI & Build / integration (push) Successful in 36s
CI & Build / Python tests (push) Successful in 55s
CI & Build / Build & push image (push) Successful in 44s
6db791965f
#231's premise was unifying reusable things already scattered as one-offs.
The create gate PREVENTS a new duplicate and merge_snippets CURES one you
point it at, but nothing FOUND the duplicates already in the record —
someone had to notice them by hand, which is the exact failure the Drafter
exists to remove.

One indexed self-join over note_embeddings, not an N² Python scan:
pgvector's cosine distance is the same operator semantic search uses, so a
similarity floor is a distance ceiling and the work stays in Postgres.
`left.note_id < right.note_id` yields each unordered pair once and drops
the self-pair that would otherwise dominate the ranking.

Pairs are collapsed into merge SETS by connected components. Transitive on
purpose: A~B plus B~C puts all three together even when A and C don't
directly clear the bar, which is what merge actually does (it folds every
source into one survivor). The cost is that a chain of mild resemblances
can rope in a member that isn't really alike — so the UI presents a set as
a proposal, shows the members, and never merges without a confirm.

Two scope decisions worth naming:

- OWN snippets only. merge_snippets requires one owner across the set, so
  surfacing someone else's would propose a merge that cannot be performed.
  The report is bounded by what the operator can act on, not what they can
  see.

- Threshold defaults to 0.82, LOOSER than the write gate's 0.90, and is a
  setting rather than a constant (rule #25). The gate blocks a create and
  has to be unforgiving of noise; this only suggests a merge under review,
  so it must reach further or it would never surface the pairs the gate
  already let through — which are precisely the ones that accumulated.

Fixes a real bug in the merge flow while wiring the UI: selectedList
filtered the selection against the CURRENT PAGE, and doMerge derives its
source ids from that list. A corpus-wide suggested group with off-page
members would have rendered incomplete and silently merged only the
visible subset. A group under review is now the authority for that list.

Refs #2088

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs
feat(snippets): un-merge — reverse one source out of a merged survivor
CI & Build / Python lint (push) Successful in 3s
CI & Build / integration (push) Successful in 24s
CI & Build / TypeScript typecheck (push) Successful in 36s
CI & Build / Python tests (push) Successful in 54s
CI & Build / Build & push image (push) Successful in 40s
fe63f3985b
Closes the last of milestone #232. The task said to settle the design
before coding; here is what was settled and why.

THE HAZARD. Restoring a merged-in source from the trash brought the record
back but never stripped its locations off the survivor, so both claimed
the same call sites and the reverse lookup read the duplicate claims as
real. Subtracting blindly is not a fix: a location can arrive from a
source AND genuinely be the survivor's own, and _normalize_locations dedups
them into one, so blind subtraction would strip a call site the survivor
owns. Same problem defeated partial un-merge — `merged_from` recorded ids,
not which locations came from which source.

THE ANSWER. Record per-source attribution AT MERGE TIME, where it is known
exactly: each entry keeps only what that source ADDED, computed
incrementally as sources fold in. Anything the survivor already had, or an
earlier source already brought, is attributed to nobody. Both open
questions fall out of that one change — partial un-merge is exact, and a
survivor-owned location can never be stripped, because it was never
attributed in the first place.

The shape moved from [id] to [{id, locations, tags}]. Free to do: the
corpus holds one snippet and zero merges, so there is no legacy data (rule
#22). A bare int still normalizes to {"id": n} — not legacy tolerance, but
because snippet_fields falls back to PARSING THE BODY when a row has no
`data`, and the body's provenance line can only carry ids. Such an entry
shows history and refuses un-merge with a reason rather than guessing.

WHICH SURFACE. Neither option in the task, quite. Making trash-restore
notice the merge would teach the generic trash path snippet semantics for
one record type. Instead un-merge OWNS the restore: one operation, one
authorization check, trash stays ignorant. Restoring by hand is still
allowed and still leaves both records claiming the same places — so
un-merge treats an already-alive source as the normal case and goes
straight to the subtraction that repairs it. That is the state that
motivated the feature, not an error.

Adds trash.restore_entity(user_id, type, id) — the missing inverse of
delete(), which returns a batch id callers don't keep. Restores the whole
batch, since the batch is the entity plus its cascade.

Refs #2165

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs
bvandeusen merged commit 3284ac67dd into main 2026-07-28 20:10:42 -04:00
Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: bvandeusen/FabledScribe#82