Skip to content
This repository was archived by the owner on Jul 3, 2026. It is now read-only.

feat: un-gate synthesizer from correction regex; feed tool evidence to classifier - #12

Closed
v2nic wants to merge 21 commits into
mainfrom
recall-8-tool-evidence
Closed

feat: un-gate synthesizer from correction regex; feed tool evidence to classifier#12
v2nic wants to merge 21 commits into
mainfrom
recall-8-tool-evidence

Conversation

@v2nic

@v2nic v2nic commented Jun 12, 2026

Copy link
Copy Markdown
Owner

Implements GitHub issue #8.

Summary

Two coupled changes to improve recall and mechanism-grounding:

Change 1 — Un-gate the digest

  • buildDigest() now includes a truncated verbatim user-text snippet (text=) for every pair, not just those where the correction regex matched
  • The regex remains a ranking/enrichment signal (note=) but no longer gates what the synthesizer can see
  • Per-line truncation at 200 chars keeps the digest within char budgets

Change 2 — Feed tool evidence to the classifier

  • PairToolCall extended with argumentsPreview (bash command string or key=value summary for other tools, truncated at 300 chars)
  • PairToolResult extended with errorHead (first 300 chars of error text, null for non-errors)
  • ClassifyInput/buildClassifyPrompt extended with ToolCallEvidence[] and ToolResultEvidence[]
  • CLASSIFY_PROMPT updated with guidance to prefer friction_type: tool_misuse when tool evidence reveals the mechanism

Version bumps

  • turn-pair-llm: 1.0 → 1.1
  • session-overview: 1.0 → 1.1

DESIGN.md

  • Added note that tool arguments and error payloads are first-class evidence
  • Documented that the correction regex is a ranking signal only, never a visibility gate

Tests (7 new)

  • Digest: unmatched correction still contributes text; plain pairs get text snippets; long text is truncated
  • Prompt: classify prompt contains failing command for tool-error turn; shows tool calls, errors, arguments
  • Build: tool call arguments, error heads, key=value summaries, null errorHead for non-errors, long command truncation

All 203 unit+component tests pass; integration test passes (20/20); TypeScript compiles cleanly.

v2nic added 21 commits June 10, 2026 22:28
…vider

Implements the analyzer framework on a clean foundation:

- Append-only, typed-edge analysis graph (no parent_id; edges are the
  single source of truth). New 'revises' edge kind links version
  alternatives of the same logical unit.
- 100% incremental via scan(): every planned unit is classified as
  missing|stale|current by input_hash + source-set lookup. shallow runs
  process missing units only; deep runs re-analyse stale units into new
  versions linked by revises edges, so alternatives coexist at the same
  level with timestamps and navigable lineage.
- No crash-recovery bookkeeping: idempotency is structural (finished
  nodes are 'current'; unfinished units remain missing/stale).
- LLM wired to Pi's AI provider system (modelRegistry.find +
  getApiKeyAndHeaders + @earendil-works/pi-ai complete()); never Ollama
  or a local server. Deterministic mock caller for tests.
- Three analyzers: turn-pair-core (deterministic friction), turn-pair-llm
  (cheap LLM enrichment of high-signal pairs), session-overview (digest +
  map-reduce, emits proposals). Dependency-visibility enforcement and
  edge validation in the framework.
- Clean v2 schema (proposals + framework tables), proposal materialiser
  with SHA-256 dedup, env-overridable config.

Tests: 130 unit/component + 19 integration, all green; ~98% line
coverage. TypeBox for all new data shapes.
Remove docs/analyzer-design-c.md (the pre-implementation design draft,
now partly obsolete: it specified progress cursors and crash recovery,
which the implementation deliberately replaced with scan-based
incrementality) and add a root-level DESIGN.md.

DESIGN.md is an orientation guide for engineers and AI agents new to the
repo. It describes the system's purpose and the rationale for the
current architecture, and fixes a ubiquitous language for the core
concepts (session, pair, analysis node, edge kinds, recipe/input hash,
scan, unit status, run modes, lineage, model tiers, proposal lifecycle).
It contains no code pointers by design.
A tier (cheap/mid/expensive) is shorthand that resolves to a concrete
provider/model; fold that resolved model into input_hash via a new
modelBundleHash. Changing which model a tier maps to now marks affected
nodes stale (deep run revises into a new version; shallow leaves them,
so swaps never force surprise recomputation). Deterministic analyzers
declare no model and are unaffected.
buildTurnPairs now starts a new turn at any host turn-start entry:
user and bashExecution messages, plus branch_summary/custom_message
entries. Compaction summaries remain boundaries, not turn starts.
--model now pins every tier to one concrete model for the invocation
(applyModelOverride), instead of being recorded but ignored. The same
effective tiers feed both the framework and the LLM caller, and analyzers
resolve their tier from ctx.modelTiers before calling the model, so the
model used and the model folded into node identity are one source of
truth and cannot diverge. A pinned run produces its own nodes; rerunning
under the normal mapping marks them stale. Drop the vestigial
defaultModel seam.
Rework the ubiquitous language so a run's reach is a composable set of
revise reasons rather than a shallow/deep depth, and split staleness by
why a node is out of date:

- Analyzer version is a major.minor pair the author owns; only the
  version is graded (major/minor). The shipped logic, default prompt,
  and default tier are all represented by the version.
- Everything the user sets (thresholds, prompt override, tier->model
  mapping, model pin, and so the resolved model) is config, and config
  is never graded -- a different prompt or model is just different.
- Identity is analyzer + version + config + source set.
- A run always fills missing work; --revise widens its reach with
  major/minor/config in any combination. Reasons only select which
  stale units to touch; a selected unit is always recomputed to the
  current recipe in full (no intermediate state, no half-updated node).
Replace the shallow/deep run mode with a composable --revise reason set
and grade staleness by cause, per DESIGN.md.

- AnalyzerVersion is now { major, minor } (was an opaque versionId
  string); versionIdOf/parseVersionId give the canonical "M.m" form
  stored on runs and nodes.
- Identity drops the standalone shipped-prompt and model-bundle hashes
  and folds the user's config + resolved model into one config
  fingerprint: input_hash = analyzer + version + config_fingerprint +
  source set. The shipped prompt is represented by the version; the
  prompt bundle hash is kept only as run provenance.
- scan() grades each stale unit's reasons: a version move is major or
  minor (gradeVersionMove), a differing config fingerprint is config.
  run({ revise }) selects missing + stale units whose reasons intersect
  the requested set (minor implies major); selected units recompute to
  the current recipe.
- /prospect-analyze takes --revise major|minor|config|all (replacing
  --deep); any reason re-scans all sessions, a plain fill only the
  unanalysed ones.
- analysis_nodes gains config_fingerprint; run mode is recorded as a
  reach label (fill or revise:...).
- New version.ts (version + reason helpers) with unit tests; new
  revise-reasons component test proves major/minor/config selection;
  model-identity test reframed (a model swap is the config reason).
The README still described the pre-framework design: a stub analyzer
that ran an LLM over "unprocessed sessions", space-form command names
(/prospect analyze), the wrong database name (sessions.db), proposal
statuses new/accepted/rejected, a non-existent analyze tool action, and
an install path pointing at the wrong owner.

Rewrite it to match the implementation and DESIGN.md's language:

- Dash-form commands (/prospect-sync, /prospect-analyze, ...) with the
  real flags, including --revise major|minor|config|all and the
  --model per-run pin.
- The append-only, incrementally-recomputed analysis graph and its
  deterministic-first layering (turn-pair-core -> turn-pair-llm ->
  session-overview); turn as the unit of analysis.
- Correct DB name (prospector.db), proposal statuses
  (open/applied/rejected/duplicate), and the prospect tool's actual
  actions (no analyze action).
- Configuration documents the fields that actually take effect
  (dbPath, modelTiers) plus the test-only env overrides; install uses
  git:github.com/v2nic/pi-prospector.
- Points at DESIGN.md as the canonical model.
…ings

Three correctness fixes surfaced by code review, each making the
implementation match its intended design:

- session-overview dropped all turn-pair-llm enrichment. The digest
  merged classifications via anchorUserId(), which always returned null
  (the promised "merge by order" fallback was never written), so
  sentiment/friction never reached the digest or the proposals it feeds.
  turn-pair-llm now records the anchor user_message_id in its node
  content (set from the planned unit, not the model), and the digest
  merges by that id. Split the pure model result (ClassifyResult) from
  the stored shape (TurnPairLLMProperties).

- turn-pair-llm ignored its maxPairsPerSession cost guard: plan() emitted
  a unit per high-signal pair, so a large session could fire unbounded
  LLM calls. plan() now receives the resolved config (new
  AnalyzerPlanContext.config) and enriches at most maxPairsPerSession
  pairs, highest friction first (ties broken by pair order for
  deterministic, idempotent selection).

- role spellings disagreed with the host platform. Pi emits the entry
  types branch_summary and custom_message (snake_case); turn-pair-core's
  turn boundaries already matched, but the MessageRole type, the
  session-overview digest, and a parser test used camelCase
  branchSummary, so branch summaries were silently omitted from digests.
  Standardised on Pi's snake_case across the type, the digest, and the
  test fixture.

Tests: digest merges enrichment by id and includes branch_summary text;
an e2e run with >cap high-signal pairs produces exactly maxPairsPerSession
classifications. 161 unit/component + 19 integration green; coverage ~98.5%.
…ling

Running the analyzers against real sessions through the Pi CLI surfaced two
rough edges in the production LLM path:

- A reasoning model (e.g. gemini-2.5-flash) spends its output-token budget on
  thinking and truncates the JSON answer. The truncated body then failed deep
  in the parser with a cryptic "Unterminated JSON object". toLLMResponse now
  detects stopReason "length" and throws an actionable error naming the output
  limit and pointing at maxTokens / a non-reasoning tier.

- A throttled provider (HTTP 429) failed a whole run on the first call. The
  caller now passes maxRetries so pi-ai rides out transient rate limits.

Also document running a single command non-interactively straight from the Pi
CLI (pi -e src/index.ts -ne --no-session -p "/prospect-..."), including pointing
the env overrides at a small private session subset for local iteration.

Tests: pi-llm asserts the truncation error path. 162 unit/component + 19
integration green.
Running prospector from the command line previously required Pi's generic
print mode (pi -e src/index.ts -p "/prospect-..."). The extension now registers
its own --prospect flag so a bare invocation runs one command and exits, with
no -p:

  pi -e ./src/index.ts --prospect sync
  pi -e ./src/index.ts --prospect stats
  pi -e ./src/index.ts --prospect "analyze --limit 3 --model openrouter/anthropic/claude-3.5-haiku"
  pi -e ./src/index.ts --prospect proposals

A session_start hook reads the flag, dispatches to the matching command, and
calls ctx.shutdown() (guarded to run once). When --prospect is absent the
extension stays fully interactive. The flag value is "<command> [args]";
commands are sync | analyze | stats | proposals | accept | reject.

To wire this up, each command's handler is extracted into an exported function
(prospectSync/Stats/Proposals/Accept/Reject/Analyze) reused by both the slash
command and the flag dispatcher, and pi-stubs gains registerFlag/getFlag/on plus
ctx.shutdown.

Tests: headless dispatcher unit tests (spec splitting, dispatch, empty/unknown,
error propagation); commands.test mock updated for the new API. 169
unit/component + 19 integration green.
An error node's identity is now recipe + message + timestamp (plus the
node id as a uniqueness nonce) instead of the recipe alone, so a failed
attempt never occupies the successful result's identity. classify() sees
only successful nodes at a recipe (findLatestNodeBySourceSet skips errors),
so a unit whose only history is failures stays `missing` and is recomputed
on the next scan. A session is retired from the unanalysed queue only when
it completes with no errors, so a plain `fill` self-heals prior failures
with no special retry mode. Error nodes stay append-only (never deleted).

Verified on real local sessions: a bogus-model run records 9 error nodes
and leaves all sessions queued; a subsequent plain fill recomputes exactly
the 9 missing LLM nodes, retains the 9 error nodes, and a third fill scans
nothing. 170 unit/component + 19 integration green; framework.ts and
analysis-queries.ts at 100% line coverage.
The proposals listing (slash command and `--prospect proposals`) now ranks
recommendations by confidence (desc, nulls last, newest-first tiebreak) and
shows the confidence percentage inline. A new `--full`/`-v` flag adds the
proposal's detail, evidence, and source provenance (session · analyzer ·
proposal id) so analyzer output can be evaluated without dropping to SQL.
Status filtering (open|applied|rejected|duplicate) is unchanged and composes
with the flag in any order.

Adds unit tests for parseProposalsArgs and rankProposals (176 unit/component
+ 19 integration green).
The proposals listing now groups recommendations under a per-session header
(short id · cwd · count), keeping confidence ranking both across sessions
(the session with the strongest single recommendation leads) and within each
group. Adds getSessionLabels query and a sessionLabel helper (cwd with $HOME
collapsed to ~, falling back to project, then short id). Duplicate/overlapping
recommendations are intentionally retained — grouping makes them easy for a
downstream AI consumer to read per session. 178 unit/component + 19
integration green.
Make analysis identities global, reproducible, and hash-verifiable instead of
DB-local. Two changes to the identity model:

- Rename the recipe identity `input_hash` -> `input_key` (analyzer + version +
  config + source set; inputs only, never the LLM output). The config dimension
  now hashes the config's *content* hash, not its DB-local uuid row id, so
  input_key no longer depends on incidental ids.
- Add `output_key = H(input_key | canonical(content))`, the content-addressed id
  of a node's result. Consumers (turn-pair-llm, session-overview) reference their
  upstream sources by output_key instead of node uuid, so a consumer's input_key
  transitively commits to upstream outputs. The graph is now a Merkle DAG:
  identical inputs+outputs reproduce identical keys on any machine, after any
  wipe.

Proposals: rename `dedup_key` -> `input_key`, derived from the source node's
output_key + ordinal (never the model's title/path/severity). Re-materialising a
node is idempotent; distinct sources keep their proposals (duplicates retained,
grouped per session at display).

Add `prospect verify`: recompute every node's output_key from stored content and
confirm it matches (tamper/corruption detection).

Tests: cross-DB reproducibility (identical input_key/output_key over the same
fixture in two independent DBs), verify clean+tamper+unparseable, output_key
unit tests, and all renamed-field updates. 183 unit/component + 20 integration
green; input-hash.ts/proposal-materializer.ts 100%, verify.ts ~99%, ~98% overall.
Validated on real sessions: 155 nodes, 16 proposals, 0 errors, verify clean.

Implements #5.
Reviewing a proposal previously required re-opening the raw session
transcript to check the LLM's free-text evidence string. `prospect show`
walks the provenance graph instead — proposal → source summary node
--consumes--> the turn nodes --anchors--> their messages — and prints the
verbatim anchored turns: the user text, the assistant text, and crucially
each tool call WITH its arguments, plus any tool-result errors. Because the
overview consumes every turn, output is focused to the high-signal turns
(high friction or an LLM classification) and capped, with a remainder note.

This makes human review trustable without a transcript hunt, and surfaces
mechanism-level reality the text-only classifier misses: e.g. for the
gh-pr-review session it shows the user shouting "PUSHED TO v2nic/..." next
to the agent's actual `git push origin …` (correct) and
`gh pr create --repo v2nic/…`/`--head` calls — exposing that the friction
was PR-creation targeting, not the push.

Also: proposal listings now print the full proposal id with a ready
`prospect show <id>` hint (uuidv7 ids share long prefixes, so short
prefixes are ambiguous); `show` resolves by exact id or unambiguous prefix.

Adds getSessionMessageRows; registers prospect-show slash command + headless
`show` action. Tests: unit (toolCallPreview, renderAnchoredTurns incl. args/
errors/cap) + component (full pipeline → resolve → provenance walk → output).
191 unit/component + 20 integration green; show.ts ~98% lines.
README: add `/prospect-show <id>` (provenance walk to verbatim anchored
turns incl. tool-call arguments) and `/prospect-verify` (the latter was
undocumented since it shipped); note the proposals listing now prints the
full id + a `prospect show <id>` hint and accepts `--full`; add show/verify
to the headless command list.

DESIGN: remove the stale "Dedup key" glossary bullet — proposal identity is
the content-addressed input_key (source output_key + ordinal) and duplicates
are intentionally retained, so the old dedup-by-title concept no longer
exists; add the `prospect show` evidence-walk to the Materialisation concept;
fix one "recommendations" → "proposals" term slip.

README intro: "deduplicated" → "ranked" proposals, matching the design
(overlaps are retained and ranked, not collapsed).
…o classifier

Issue #8: Two coupled changes to improve recall and mechanism-grounding.

Change 1 — Un-gate the digest:
- buildDigest() now includes a truncated verbatim user-text snippet
  (text=) for EVERY pair, not just those where the correction regex matched.
- The regex remains a ranking/enrichment signal (note=) but no longer
  gates what the synthesizer can see.
- Per-line truncation at 200 chars keeps the digest within
  mapReduceOverChars/segmentChars budgets.

Change 2 — Feed tool evidence to the classifier:
- PairToolCall extended with argumentsPreview (bash command or
  key=value summary for other tools, truncated at 300 chars).
- PairToolResult extended with errorHead (first 300 chars of error
  text from toolResult content_text, null for non-errors).
- ClassifyInput/buildClassifyPrompt extended with ToolCallEvidence[]
  and ToolResultEvidence[] — includes a TOOL CALLS section showing
  tool names, truncated arguments, and FAILED markers with error text.
- CLASSIFY_PROMPT updated with guidance to prefer friction_type
  tool_misuse when tool evidence reveals the mechanism.

Both analyzers bumped minor version: turn-pair-llm 1.0→1.1,
session-overview 1.0→1.1.

DESIGN.md updated with note that tool arguments and error payloads
are first-class evidence, and the correction regex is a ranking
signal only, never a visibility gate.

Tests added:
- Digest: unmatched correction still contributes text; plain pairs
  get text snippets; long text is truncated.
- Prompt: classify prompt contains failing command for tool-error
  turn; shows tool calls, errors, and arguments.
- Build: tool call arguments, error heads, key=value summaries,
  null errorHead for non-errors, long command truncation.
@v2nic v2nic closed this Jun 12, 2026
@v2nic
v2nic deleted the recall-8-tool-evidence branch June 12, 2026 03:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant