Drafter hardening: reverse lookup by location + the write-path trigger #79

Merged

bvandeusen merged 8 commits from dev into main 2026-07-28 10:12:48 -04:00

Conversation 0 Commits 8 Files Changed 36

+2440 -76

bvandeusen commented 2026-07-28 10:12:41 -04:00

Owner

Copy Link

Copy Source

Milestone #232 (Drafter — recall hardening and discovery), 6 of 10 steps. 8 commits, CI green on head e0328f2 (run 2997).

Together these make recorded reuse findable at the moment it matters: an agent about to write a helper is now shown the prior art instead of discovering it a turn too late.

What lands

cca40af fc9c811	#2081 — notes.data JSONB + GIN (migration 0070), the indexed mirror of snippet fields
8977bed	#2084 — auto-inject menu lines name the record kind ([snippet], [issue], …)
9fa474b	#2159 — get_note / get_task over MCP were owner-only; a shared record the menu had just offered read as "not found"
0d396de	#2087 — merge provenance: the survivor records what it absorbed
dd1b5e5 083944f	#2083 — reverse lookup: list_snippets(repo=…, path=…, symbol=…)
e0328f2	#2082 — write-path trigger: PreToolUse hook on Write/Edit offering prior art

Things worth knowing before touching this code

• The write-path hook must never gain a permissionDecision. It returns additionalContext only, so it informs a write and cannot block one. Plain stdout from a PreToolUse hook doesn't reach the model, and deny/ask would turn a recall aid into a gate on the operator's work. A test enforces this.
• The location filter is one predicate in two dialects (services/knowledge.py): jsonpath data @? for the SQL arms, location_matches for the semantic arm. They must change together — an integration test pins them to identical results over seven filter shapes.
• notes.data is now backfilled at startup, so every consumer may assume the mirror is present. No body-regex fallback arm is needed anywhere.
• A JSONB column has two empty states. data=None persists JSON null, not SQL NULL (SQLAlchemy's none_as_null=False). The backfill covers both; this cost a CI cycle.

Deployment note

plugin.json goes 0.1.16 → 0.1.18, and the write-path trigger needs the new GET /api/plugin/prior-art endpoint — clients on the old image get a 404 and fail silent by design.

Also note the trigger is live but quiet until snippets exist: an empty result is indistinguishable from "no prior art here", so verify with a snippet known to sit at the path being edited.

🤖 Generated with Claude Code

https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs

Milestone #232 (*Drafter — recall hardening and discovery*), 6 of 10 steps. 8 commits, CI green on head `e0328f2` (run 2997). Together these make recorded reuse **findable at the moment it matters**: an agent about to write a helper is now shown the prior art instead of discovering it a turn too late. ## What lands | | | |---|---| | `cca40af` `fc9c811` | **#2081** — `notes.data` JSONB + GIN (migration 0070), the indexed mirror of snippet fields | | `8977bed` | **#2084** — auto-inject menu lines name the record kind (`[snippet]`, `[issue]`, …) | | `9fa474b` | **#2159** — `get_note` / `get_task` over MCP were owner-only; a shared record the menu had just offered read as "not found" | | `0d396de` | **#2087** — merge provenance: the survivor records what it absorbed | | `dd1b5e5` `083944f` | **#2083** — reverse lookup: `list_snippets(repo=…, path=…, symbol=…)` | | `e0328f2` | **#2082** — write-path trigger: PreToolUse hook on Write/Edit offering prior art | ## Things worth knowing before touching this code - **The write-path hook must never gain a `permissionDecision`.** It returns `additionalContext` only, so it informs a write and cannot block one. Plain stdout from a PreToolUse hook doesn't reach the model, and deny/ask would turn a recall aid into a gate on the operator's work. A test enforces this. - **The location filter is one predicate in two dialects** (`services/knowledge.py`): jsonpath `data @?` for the SQL arms, `location_matches` for the semantic arm. They must change together — an integration test pins them to identical results over seven filter shapes. - **`notes.data` is now backfilled at startup**, so every consumer may assume the mirror is present. No body-regex fallback arm is needed anywhere. - **A JSONB column has two empty states.** `data=None` persists JSON `null`, not SQL NULL (SQLAlchemy's `none_as_null=False`). The backfill covers both; this cost a CI cycle. ## Deployment note `plugin.json` goes 0.1.16 → 0.1.18, and the write-path trigger needs the new `GET /api/plugin/prior-art` endpoint — clients on the old image get a 404 and fail silent by design. Also note the trigger is **live but quiet until snippets exist**: an empty result is indistinguishable from "no prior art here", so verify with a snippet known to sit at the path being edited. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01UaYUaouG9jjhATyuxCKrQs

bvandeusen added 8 commits 2026-07-28 10:12:42 -04:00

feat(snippets): add notes.data JSONB — the indexed mirror of snippet fields ... cca40affe4

Milestone #232 step 1 (task #2081). Takes the enabler first rather than the
write-path trigger: reverse lookup, drift checks and the duplicate finder all
need to QUERY structured fields, and building them on body-regex first means
writing them twice.

#227 deferred this bag "unless body-convention ergonomics prove insufficient" —
answering "which snippets live in this file?" by scanning every snippet and
regexing its body is that condition being met.

Migration 0070 adds `notes.data` (nullable JSONB) + a GIN index. The body is
UNCHANGED and still what gets embedded and read by humans; `data` mirrors the
same facts in a shape Postgres can index. Code is deliberately not copied into
it — the body holds it, and duplicating a blob into the column we index around
would be waste.

- compose_data() builds the mirror, omitting empties so the column stays sparse
- snippet_fields() prefers `data`, falling back to parsing the body. Rows written
  before 0070 have no `data` and are never backfilled, so a hand-edited body
  stays authoritative for them with no conversion deadline
- create / update / merge all write body and mirror from the same merged field
  set, so the two can't drift; merge in particular has to grow the mirror with
  the survivor's location set or a merged snippet would be unfindable at the very
  call sites the merge just recorded

Named `data`, not `metadata`, because that collides with SQLAlchemy's declarative
Base.metadata — which is why the pre-0069 model had to map an awkward
`entity_metadata` attribute. Not a revival of the column 0069 dropped: different
name, different purpose, nothing reads the old shape.

Two test fakes needed an explicit `data = None`: snippet_fields prefers `data`
when truthy and an auto-MagicMock attribute is truthy, so every parsed field
would have come back a MagicMock. Checked every fake reaching snippet code this
time rather than waiting for CI (note 2109).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RLwAaV4DQEmVyn496HnEvt

test(snippets): fix the round-trip test's shared kwargs spread ... fc9c8119a2

Run 2923: 1 failed, 408 passed. My test bug, not a code one — I spread one
`fields` dict into both compose_body and compose_data, but compose_body takes no
`name` (the name lives in the title). Each serializer now gets its own argument
list.

The migration itself was fine: the integration lane ran 0001→0070 on
pgvector/pg17 green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RLwAaV4DQEmVyn496HnEvt

feat(inject): label the record kind on each auto-inject menu line ... 8977bed28d

Every injected hit rendered identically, so a recorded snippet was
indistinguishable from a stray dev-log in the one place prior art most
needs to stand out. Each line now carries its kind — [snippet],
[process], [task], [issue], [note] — and the header says "records"
rather than "notes", which it can no longer claim.

Task-ness wins over note_type in the marker: "there's an open issue
about this" is the more useful thing to know at a glance.

Still title-first: the marker is metadata already on the ORM object,
so no extra query and no bodies (#2084).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RLwAaV4DQEmVyn496HnEvt

fix(mcp): make get_note / get_task share-aware ... 9fa474b3c4

The injected menu tells the agent to open any hit with get_note(id), and
that menu can list a collaborator's record reached through a shared
project. The fetch was still owner-only, so those lines answered
"not found" — for a record the same user opens fine in the browser.
The agent path was strictly narrower than the web path for the same id.

Same boundary miss as #2093: the list side was widened for sharing, the
fetch side wasn't. Both tools now resolve through get_note_for_user,
apply the trash filter themselves (permission resolution says nothing
about liveness), and attach describe_provenance so a shared record
arrives marked as someone else's rather than passing as the caller's.

routes/tasks.py's parent-title lookup had the same narrowness: a shared
subtask rendered as an orphan when its parent was equally shared.

Four fetch tests across three files were patching notes_svc.get_note and
had to be retargeted — note 2109's third sub-case, caught by grepping
tests/ for the old name before pushing rather than by CI (#2159).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RLwAaV4DQEmVyn496HnEvt

feat(snippets): record merge provenance on the survivor ... 0d396de215

Merge kept the target's fields, unioned locations and tags, and trashed
the sources — recording nothing about what it absorbed. If a variant
handled an edge case the survivor doesn't, that difference left the
visible record entirely; recovering it meant knowing to go digging in
the trash.

The survivor now carries `merged_from`: a "**Merged from:** #2, #3" line
in the body for humans, and the same list in the `data` mirror for
queries, written from one value like every other field (#2087).

It accumulates rather than replaces — a target merged twice keeps both
histories — and skipped sources (cross-owner, per #231) are excluded, so
the record never claims to contain something it never absorbed.

Ordinary edits carry it forward. update_snippet recomposes body and
mirror from scratch, so an omission there would silently erase the
history on the next unrelated edit; that path is pinned by its own test,
including the pre-0070 case where the body line is the only copy.

Surfaced in the snippet detail view as a "Merged from" row — the view
renders parsed fields, not the raw body, so the body line alone would
have been invisible to the operator (rule #27).

Un-merge, the other half of #2087, stays open: restoring a source from
trash still doesn't strip its locations off the survivor, and what
partial un-merge should mean is a design question, not a coding one.
`merged_from` is the record that makes it tractable.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RLwAaV4DQEmVyn496HnEvt

feat(snippets): reverse lookup — find snippets by repo/path/symbol ... dd1b5e5ddb

"What canonical helpers already live in this file?" was unanswerable:
location lived only in the body markdown. It is now a jsonpath containment
query over the `notes.data` mirror added by migration 0070.

- One predicate in two dialects in services/knowledge.py: SQL (`data @?`,
  applied in the browse arm and the keyword arm before count/pagination, so
  totals stay honest) and Python (`location_matches`, for the semantic arm
  which post-filters candidates it already holds). Both must change together.
- Parts are ANDed within a SINGLE locations entry — repo A in one entry and
  path B in another is not "recorded at A/B". `path` also matches as a
  directory prefix, via jsonpath `starts with` rather than `@>`, which the
  same GIN index serves.
- `repo`/`path`/`symbol` reach the service, the REST list and the MCP tool
  under one name with one default (rule #33); the MCP docstring teaches the
  place form, and so does the reusing-code skill (plugin.json bumped).
- UI: a Location disclosure beside the snippet search, with its own empty
  state — "nothing kept there, so what you're about to write is new."

Settles #2083's open question (pre-0070 NULL `data`) by backfilling after
all: `backfill_snippet_data` runs at startup, deriving the mirror from the
body with the same parser the read path trusts. 0070's caution was about
mangling a hand-edited body; this never touches the body. The alternative
was a permanent second body-regex arm, or a query that silently answers
"nothing here" for an old snippet and gets the helper written twice.

Refs #2083, milestone #232.

fix(snippets): backfill must also catch JSON null, not just SQL NULL ... 083944f0fd

Integration lane caught it (run 2984): the backfill reported 0 rows to fill
and left `data` unset. `IS NULL` was the whole predicate, but a JSONB column
has two empty states. SQLAlchemy's JSON types default to
`none_as_null=False`, so assigning Python `None` persists the JSON encoding
of null — `IS NULL` walks straight past it.

Migration 0070 left genuine SQL NULLs, so the product path was right; the
test was constructing the wrong shape with `data=None`. Fixed both ways,
because both states mean "no usable mirror":
- predicate is now `data IS NULL OR jsonb_typeof(data) = 'null'`;
- the legacy-row test OMITS `data` (a real SQL NULL, 0070's actual shape),
  and a second test covers the JSON-null shape and asserts the premise with
  `jsonb_typeof` rather than assuming it.

Refs #2083.

feat(plugin): write-path trigger — offer prior art before code is rewritten ... e0328f2b1c

The milestone headline. Auto-inject fires on the operator's prompt; the
moment reuse is actually lost is later, when the agent decides mid-task to
write a helper. A PreToolUse hook on Write|Edit now fires there.

Channel: `additionalContext` with NO permissionDecision, so the note reaches
Claude beside the tool result and the write is never blocked — a recall aid
must not be able to stop the operator's work. Plain stdout would have been
invisible to the model, and deny/ask would have made a nudge into a gate.

Two arms, different in kind:
- BY PLACE — a snippet recorded at this path (or its directory) is prior art
  by definition, not resemblance, so it is neither scored nor thresholded.
  This is what #2083's reverse lookup was built to answer.
- BY MEANING — semantic search restricted to snippets (new `note_type` filter
  on semantic_search_notes) over the code about to be written.
Place ranks first; the top-k cap spans both arms.

Gates carried over from milestone 93 verbatim: threshold, margin, session
dedup, titles-never-bodies. Own `source='write_path'` in retrieval_logs so
precision is tunable separately — the docstring records that the place arm
is unlogged and hands that to #2085.

Its own on/off in Settings but the SAME threshold/top-k: one "how loud may
Scribe be" knob is easier to reason about than two that drift, and splitting
them later is then a data-backed change rather than a guess.

Details worth keeping: the hook sends a REPO-RELATIVE path because that is
how locations are recorded; the git remote resolves to a project and is never
used as the location `repo` filter (different namespaces, would silently
match nothing); the endpoint stays a GET because a read-scoped API key cannot
POST and every other hook depends on that.

plugin.json 0.1.17 -> 0.1.18. Refs #2082, milestone #232.

bvandeusen merged commit ca94c332ee into main 2026-07-28 10:12:48 -04:00

bvandeusen referenced this issue from a commit 2026-07-28 10:15:47 -04:00

fix(deps): cap mcp below 2.0 — it removed mcp.server.fastmcp

bvandeusen referenced this pull request 2026-07-28 10:18:17 -04:00

fix(deps): cap mcp below 2.0 — un-reds main #80

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

bvandeusen

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: bvandeusen/FabledScribe#79