feat(capture): correction-capture — auto-propose a draft claim when the user pushes back - #679
Conversation
the adapter captures tool outcomes passively, but the single highest-signal
event in a session — the user correcting the agent ("no, we deploy from
main not release") — evaporated unless someone remembered to propose a
claim afterwards. that is exactly the knowledge worth keeping, and the
knowledge most reliably lost.
detection is a regex on the turn boundary: a pushback opener that also
asserts something. no llm call, so it costs nothing per turn and stays
deterministic. deliberately conservative — a false negative loses one
correction, a false positive spends a reviewer's attention, and the second
is what makes an ambient feature get switched off. the opener itself is
stripped, so "no, we deploy from main" is kept as "we deploy from main":
the disagreement is context, the assertion is the knowledge.
the whole design constraint is that this proposes and never writes. the
module routes exclusively through propose_quoted_claim, does not import
approve, and a test asserts there is no import of it. the pending queue is
the draft state; a human still drains it.
the claim carries a receipt rather than a paraphrase: the user's message
is registered as a `message` source and the corrective sentence is quoted
verbatim out of it, so verify_receipt confirms it by string comparison.
three guards bound the heuristic:
- a per-session cap (capture.correction.max_per_session, default 3),
counted from the pending queue rather than in memory so it holds across
a process restart — the unattended case it exists for.
- dedup against approved claims and pending corrections. lexical, with the
vouchdev#147 embedding hits folded in on top: that path needs the [embeddings]
extra, and dedup that silently stops working on a base install is
precisely how an unattended capture floods a queue.
- secret masking before anything durable is written. a correction is
free-form text typed in a hurry, which is where a pasted credential
shows up.
wired into the existing UserPromptSubmit hook via maybe_capture, which
swallows its own failures — the hook contract is that a broken kb drops
the correction rather than breaking the turn. registered as
kb.capture_correction across the four sites, and added to the admission
gate's auto-capture actors so the same deterministic floor applies to it
as to every other passive firehose.
closes vouchdev#430
the diff-coverage gate wants 100% of changed python. what was uncovered was the cli and mcp bodies (registered, never called) and the guards — which are the part of an ambient heuristic worth pinning, since they are what keeps it from becoming a reviewer's problem. added: the cli and mcp surfaces filing a real proposal; overlap with no shared signal; every load_config fallback and the three numeric-typo coercions; pushback that strips to nothing, strips below min_chars, or carries no assertion; dedup against an already-approved claim as well as a pending correction from another session. the embedding half of dedup is exercised both ways on purpose — stubbed in one test so the fold-in runs, and with the import forced to fail in another. ci installs `[dev,web]`, so without the second test the base install's lexical-only path is the one nobody checks, and dedup silently not working is exactly how an unattended capture floods a queue.
…ture goals (vouchdev#427) and the agent registry (vouchdev#607) landed on test while this branch was open. every conflict was additive; nothing from either side is dropped. the one that mattered: kb_capture_correction and kb_propose_goal collided under a single shared @mcp.tool(), so taking the merge as produced would have dropped kb_propose_goal off the mcp surface and failed the four-site parity check. each now carries its own decorator. the jsonl handlers, the cli command blocks and the unreleased changelog stanza all keep both sides.
|
the propose-never-write invariant is the right one and asserting it structurally — no import of the conservative-detection argument is correct and worth having written down: a false negative loses one correction, a false positive spends reviewer attention, and the second is what gets an ambient feature switched off. declining on a bare counting the per-session cap from the pending queue rather than in memory is the detail i would have flagged if it were missing — the unattended case is exactly the one where the process restarts, and an in-memory counter resets to zero right when the cap matters most. two things worth a maintainer's attention rather than mine. first, merge order against #690. that PR removes the second, the portfolio question. #690 argues that per-turn hook work is overhead worth deleting; this adds regex work on every |
`METHOD_SCOPES` was exhaustive over `capabilities.METHODS` when this branch was written. four methods have merged to `test` since — kb.capture_correction (vouchdev#679), and kb.list_goals / kb.propose_goal / kb.set_goal_status (vouchdev#676) — leaving the table stale by four and all three matrix jobs red on `test_every_method_is_classified`. that failure is the guard working. by this branch's own rule an unclassified method is denied to a scoped caller, so merging as-is would have silently locked every scoped credential out of goal writes and correction capture. the right failure direction, but still a regression, and invisible from the diff. the classifications: - kb.list_goals -> kb:read. a listing that cannot change durable state, beside kb.list_sessions. - kb.propose_goal -> kb:propose. files a PENDING goal a human approves. - kb.capture_correction -> kb:propose. routes exclusively through propose_quoted_claim and has no import of approve. - kb.set_goal_status -> kb:approve, not kb:propose. this is the one that departs from the suggestion on the closing comment, and deliberately: it is a lifecycle op living in lifecycle.py beside supersede/archive/confirm, it mutates an already approved goal in place, and server.py documents it as "the only write path for goal status" — status moves never go through a second proposal. filing it under kb:propose would let a propose-only credential change durable state with no review, which is the exact boundary this module exists to hold. no other change: the scope machinery, the two safety rules and the tests are as reviewed. Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
the adapter captures tool outcomes passively (
PostToolUse), but the single highest-signal event in a session — the user correcting the agent ("no, we deploy frommainnotrelease") — evaporated unless someone remembered to propose a claim afterwards. this turns a detected correction into a proposal, automatically, never an auto-write.The invariant
correction capture proposes, never writes. the module routes exclusively through
proposals.propose_quoted_claim, has no import ofapprove, andtest_the_module_has_no_path_to_approveasserts that. the pending queue is the "draft" state a human drains.it also carries a receipt rather than a paraphrase: the user's message is registered as a
messagesource and the corrective sentence is quoted verbatim out of it, soverify_receiptconfirms the citation by string comparison. what reaches the reviewer is checkable, not trusted.Detection
a regex on the turn boundary — a pushback opener that also asserts something. no LLM call, so it costs nothing per turn and stays deterministic. deliberately conservative: a false negative loses one correction, a false positive spends a reviewer's attention, and the second is what makes an ambient feature get switched off.
"there is no config file"(mid-sentence "no") and a bare"no."(disagreement without content) both correctly decline.the opener is stripped —
"no, we deploy from main"is kept as"we deploy from main". the disagreement is context; the assertion is the knowledge.Guards
capture.correction.max_per_session(default 3), counted from the pending queue rather than in memory so it holds across a process restart, which is the unattended case it exists for.find_similar_on_proposehits folded in on top: that path needs the[embeddings]extra, and dedup that silently stops working on a base install is precisely how an unattended capture floods a queue.admission.AUTO_CAPTURE_ACTORS, so the existing deterministic admission floor applies to this firehose like every other one.capture.correction.enabled(default true) gates the behaviour. declines return{"captured": false, "reason": ...}so a caller can see why nothing was filed instead of inferring it from silence.Wiring
driven from the existing
UserPromptSubmithook (it already sees the prompt) viamaybe_capture, which swallows its own failures — the hook contract is that a broken KB drops the correction rather than breaking the turn. also exposed askb.capture_correctionacross the four registration sites for adapters that would rather call it explicitly.Test Plan
tests/test_capture_correction.py(30 tests): the propose-only invariant, the no-import-of-approve invariant, receipt verification, six positive and six negative detection cases, the cap (including that it is re-read from the queue by a freshKBStore), per-session isolation of the cap, dedup positive and negative, config on/off including a quoted"true", secret masking,maybe_capturenever raising, and the end-to-end hook pathCloses #430