Skip to content

Hook-driven claim extraction for guard mode (precise mode, opt-in)#115

Open
justinstimatze wants to merge 3 commits into
fiorastudio:masterfrom
justinstimatze:feat/hook-driven-extraction
Open

Hook-driven claim extraction for guard mode (precise mode, opt-in)#115
justinstimatze wants to merge 3 commits into
fiorastudio:masterfrom
justinstimatze:feat/hook-driven-extraction

Conversation

@justinstimatze
Copy link
Copy Markdown
Contributor

This tries to fix the lossy claim extraction symptom we talked about. Guard mode today relies on the host model calling buddy_observe with claims/edges, which it stops doing past ~100k tokens of context. The graph goes silent mid-session — exactly the report.

Approach mirrors slimemold's hook architecture: move extraction out of band into the Stop hook so it doesn't depend on the host model's attention budget. It's opt-in via BUDDY_EXTRACTION_KEY (or ANTHROPIC_API_KEY). Without a key, guard mode keeps working exactly as before — no regression for existing users.

What's here

  • Stop hook extractor reads the transcript, calls Anthropic with a v7 prompt (1h prompt cache), feeds the result into the existing runGuardPipeline.
  • UserPromptSubmit drains pending findings as a [buddy observation] block before each prompt.
  • Per-host-session cursor + cross-batch context so consecutive Stop hooks don't re-extract the same window. Was a real duplication bug in the first cut.
  • Persistent extraction stats (reasoning_extraction_stats table) so buddy_doctor actually sees hook-side activity — the in-memory telemetry.ts counters reset every Stop hook process.
  • WAL journaling, atomic JSON1 increments in recordFailure, cross-process graph-cache invalidation via PRAGMA data_version.
  • Backoff after sustained failure with graceful fallback to accepting model claims, so the graph doesn't go silent during outages.
  • Mute respected (hook skips, delivery skips). buddy_forget all clears the new tables.
  • BUDDY_DEBUG=1 logs per-claim drop reasons in writeClaims plus a shape snapshot at the buddy_observe boundary. Should help debug the 0 claims via MCP path you mentioned — running with the env set will print which field is making writeClaims silently drop.

Compatibility

Strictly additive. MCP protocol unchanged, runGuardPipeline signature unchanged, the existing model-driven path is preserved (and still the only path when no key is set). New tables are CREATE TABLE IF NOT EXISTS, new column is an idempotent ALTER. No manual migration.

@anthropic-ai/sdk added as a runtime dep, loaded only when an extraction key is present at hook fire time.

Tests

~90 new (extractor reader, shape conversion, key/model resolver, persistent state, multi-call incremental flow, end-to-end with stubbed SDK, cross-connection cache invalidation, doctor regressions, mute respect). 826/829 in the full suite pass — the 3 remaining are pre-existing penguin animation / oldBuddy stats randomness on master, not touched by this branch.

I didn't run a real-API smoke test — no key on hand. The SDK call shape matches the typed interface, but if you want to gate the merge on a manual run, say the word and I'll add a tiny smoke script.

A few things I left for your call

  • Default model is claude-haiku-4-5. Override via BUDDY_EXTRACTION_MODEL or extraction.model in ~/.buddy/config.json. I didn't touch the install script's onboard messaging — felt like that's your voice to write.
  • BUDDY_INSTRUCTIONS still injects the extraction-prompt guidance into CLAUDE.md at install time. With precise mode active that guidance is wasted prompt budget every turn (model tries to comply, my buddy_observe gate discards). Could be made conditional, but felt out of scope.
  • History squashed because most of the commits were self-review revisions. Happy to break this into separate PRs if size is awkward to review — natural cuts at extractor module / hooks wiring / robustness round / coexistence with model path.

Honest disclosure: most of this was written with Claude. I reviewed and iterated heavily but a senior engineer's eyes on it would not be wasted.

@justinstimatze
Copy link
Copy Markdown
Contributor Author

Smoke-tested against the live API after all — borrowed an Anthropic key for a single call.

[smoke] response in 3729ms
[smoke] shaped claims: 7 edges: 7
  c0 vibes       user      "We should switch to postgres for the auth service."
  c1 llm_output  assistant "Switching to postgres for the auth service means we'll need session..."
  c2 llm_output  assistant "We need to implement rotating tokens with a refresh flow."
  c3 deduction   user      "The need for session storage and rotating tokens with a refresh flo..."
  c4 definition  assistant "We're using express 5 already."
  c5 vibes       assistant "Express 5's middleware story is straightforward."
  c6 deduction   assistant "Because we're using express 5, implementing the postgres auth migra..."
edges: c1→c0 depends_on, c2→c0 depends_on, c3→c1 supports, c3→c2 supports, c6→c4 depends_on, c6→c5 depends_on, c6→c0 supports

Confirms: SDK 0.92.0 accepts the request shape (cache_control 1h TTL + tool schema + tool_choice), the model returns the v7 format, basis classification looks sensible, intra-batch edges resolve correctly, latency ~3.7s for a 5-turn transcript fits the Stop-hook budget.

@terpjwu1
Copy link
Copy Markdown
Collaborator

terpjwu1 commented May 2, 2026

@justinstimatze feedback from codex and claude code

Both reviewers agree: Approve with minor suggestions

Shared findings (flagged independently by both)

`
Both reviewers agree: Approve with minor suggestions

Shared findings (flagged independently by both)

┌───────────────────┬──────────┬──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Issue │ Severity │ Details │
├───────────────────┼──────────┼──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ │ │ countTranscriptTurns re-reads the entire transcript right after readRecentTranscript already parsed it. Wastes I/O │
│ Double file read │ Medium │ and opens a small race window where a turn appended between the two reads gets cursor-skipped without extraction. │
│ │ │ Fix: return turn count from readRecentTranscript as a side-effect. │
├───────────────────┼──────────┼──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Cursor bumped │ Low │ Deliberate tradeoff, well-documented. But a pipeline crash silently loses a batch with no log unless BUDDY_DEBUG=1. │
│ before pipeline │ │ Consider warn-level logging on pipeline throw. │
├───────────────────┼──────────┼──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ describeKind │ Nit │ Function definition splits the import block in server/index.ts. │
│ placement │ │ │
└───────────────────┴──────────┴──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘

Additional from Codex

┌───────────────────┬──────────┬──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Issue │ Severity │ Details │
├───────────────────┼──────────┼──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ UTF-8 split on │ Low │ 2MB buffer offset could land mid-character. skipFirstLine mostly mitigates, but if the split is at a newline │
│ tail-seek │ │ boundary, a replacement char leaks into the next line's JSON parse (silently skipped). Worth documenting. │
├───────────────────┼──────────┼──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ .env parser edge │ Low │ Inline comments after values (sk-foo # comment) would be included. Fine in practice since API keys don't contain # │
│ case │ │ or spaces. │
├───────────────────┼──────────┼──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Path-based cursor │ Low │ path:/absolute/path fallback is machine-specific. Moving project dirs resets the cursor (benign — one extra API │
│ key │ │ call). │
└───────────────────┴──────────┴──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘

Additional from my review

┌───────────────────────────────┬──────────┬──────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Issue │ Severity │ Details │
├───────────────────────────────┼──────────┼──────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ readRecentTranscript with │ Low │ Forward-scans from byte 0 on every incremental call. No tail-seek for the incremental path. Fine if the │
│ sinceTurn > 0 │ │ hook fires frequently (small delta), but on a 200MB transcript at turn 5000/5050 it reads everything. │
├───────────────────────────────┼──────────┼──────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Prompt cache TTL format │ Low │ cache_control: { type: 'ephemeral', ttl: '1h' } — verify against current SDK schema. │
└───────────────────────────────┴──────────┴──────────────────────────────────────────────────────────────────────────────────────────────────────────┘

Both reviewers praised

  • Graceful degradation / preciseModeActiveForObserve symmetry with shouldBackoff
  • WAL + PRAGMA data_version cross-process invalidation
  • Atomic JSON1 increments in recordFailure
  • Key redaction thoroughness
  • Mute respect with non-advancing cursor
  • Test quality (especially the incremental extraction integration test)
  • buddy_forget all clearing extraction tables`

@terpjwu1
Copy link
Copy Markdown
Collaborator

terpjwu1 commented May 2, 2026

Bottom line

One actionable item before merge: combine readRecentTranscript and countTranscriptTurns into a single pass to eliminate the double-read and the race
window. Everything else is improvement-tier, not blocker-tier.

justinstimatze and others added 2 commits May 3, 2026 19:20
Guard mode currently relies on the host model voluntarily calling
buddy_observe with claims/edges. As context grows past ~100k tokens
the model drops these calls and the graph goes silent.

This adds an opt-in precise mode: the Stop hook reads the transcript
JSONL directly (with a 2MB tail seek for long sessions), calls Anthropic
with a v7 extraction prompt + 1h prompt cache, and feeds the structured
output into the existing runGuardPipeline. UserPromptSubmit drains
pending findings into the next prompt as a [buddy observation] block.
Independent of the host model's attention budget; works through long
sessions.

Opt-in via BUDDY_EXTRACTION_KEY (or ANTHROPIC_API_KEY). Without a key,
guard mode falls back to today's behavior — no regression.

Highlights
- Per-host-session cursor + cross-batch existing-claims context so
  consecutive Stop hooks don't re-extract the same window.
- Persistent extraction telemetry (reasoning_extraction_stats) visible
  across the MCP server / Stop hook process boundary; in-memory
  counters reset every Stop hook process so the doctor needs DB-backed
  state to see hook-side activity.
- WAL journaling, atomic JSON1 increments in recordFailure, cross-
  process graph-cache invalidation via PRAGMA data_version (catches
  the case where the Stop hook's writes don't bump the MCP server's
  in-process generation counter).
- Backoff after sustained failure with graceful fallback to accepting
  model claims so the graph doesn't go silent during outages.
- Mute respected: hook skips extraction, UserPromptSubmit skips
  delivery while mood='muted'.
- buddy_forget all clears the new tables so the cursor doesn't point
  past an empty graph.
- Doctor + buddy_reasoning_status surface mode, stats, backoff state.
- BUDDY_DEBUG=1 logs per-claim drop reasons in writeClaims and a
  shape snapshot at the buddy_observe boundary.

Schema
- New tables: reasoning_extraction_state (per-host-session cursor),
  reasoning_extraction_stats (per-companion telemetry).
- New column on reasoning_observe_seq: last_delivered_finding_id.
- All idempotent — CREATE TABLE IF NOT EXISTS and try/wrapped ALTER.

Dep
- @anthropic-ai/sdk added (loaded only when an extraction key is
  present at hook fire time; not loaded otherwise).

Tests
- ~90 new across the extractor, persistent state, multi-call
  incremental flow (regression test for the duplication bug), end-
  to-end with stubbed SDK, cross-connection cache invalidation,
  doctor regressions, mute respect, BUDDY_DEBUG output.
- 826/829 in the full suite pass; the 3 remaining are pre-existing
  penguin animation / oldBuddy stats randomness on master.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collapses readRecentTranscript and countTranscriptTurns into one parse
that returns { chunk, totalTurns }. Closes the race window where the
transcript could grow between the two reads, advancing the cursor past
turns the LLM never saw — and drops the redundant read.

The tail-seek micro-optimization (when sinceTurn==0) is gone with it;
once a cursor exists every fire was already doing a full scan, so the
savings only applied to the first fire and complicated the cursor
advance for marginal benefit. Full reads on multi-MB transcripts parse
in well under 50ms.

Per maintainer feedback on PR fiorastudio#115.
@justinstimatze justinstimatze force-pushed the feat/hook-driven-extraction branch from ebd427c to c268f24 Compare May 4, 2026 02:26
@justinstimatze
Copy link
Copy Markdown
Contributor Author

Done in c268f24readRecentTranscript now returns { chunk, totalTurns } from a single parse, the call site uses that count for the cursor bump, and countTranscriptTurns is gone.

Side effect: the 2MB tail-seek on the first fire dropped out with it. Once a cursor exists every subsequent fire was already doing a full scan, so the optimization only applied to fire #1 and made the cursor advance fiddly to keep race-free. Full reads on multi-MB transcripts parse in well under 50ms either way.

Tests updated (29 passing across the affected suites; full repo at 827/830, same 3 pre-existing failures as before).

- stop-handler: pipeline-throw stderr is no longer BUDDY_DEBUG-gated.
  Backoff/API-failure logs stay gated (expected, recurring, captured in
  stats), but a pipeline throw is rare and silently loses a batch since
  the cursor was already bumped — worth a visible warn line.
- server/index.ts: move describeKind below the import block instead of
  splitting it.

Per reviewer comments on PR fiorastudio#115.
@justinstimatze
Copy link
Copy Markdown
Contributor Author

Worked through the consolidated review:

Already addressed (c268f24)

  • Double file read + the race window — readRecentTranscript is single-pass now, returns { chunk, totalTurns }.
  • UTF-8 split on tail-seek — moot now that tail-seek is gone.

Just landed (03d9b7c)

  • Cursor-before-pipeline: pipeline-throw stderr is no longer BUDDY_DEBUG-gated. Backoff and API-failure lines stay gated (recurring, already captured in stats), but a pipeline throw is rare and silently loses a batch since the cursor was already bumped. Worth a visible warn line.
  • describeKind now sits below the import block instead of splitting it.

Verified, no change needed

  • Prompt cache TTL '1h' — SDK type union is '5m' | '1h' (sdk@0.92.0 messages.d.ts:162). Live smoke test against haiku-4-5 yesterday accepted it.

Acknowledged but not changing

  • .env inline-comment edge case — flagged Low and "fine in practice" by the reviewer; API keys don't contain # or spaces.
  • Path-based cursor key on machine moves — flagged Low and "benign — one extra API call" by the reviewer.
  • readRecentTranscript with sinceTurn>0 does a full forward scan on a 200MB transcript — same tradeoff just accepted in c268f24 to close the race. Could re-introduce a tail-seek that uses byte offsets cached in reasoning_extraction_state for the incremental path, but I'd want a real session in the multi-tens-of-MB range showing measurable hook latency before adding that complexity. Filing as a future optimization rather than a blocker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants