Tree-format tool output: prefix-grouped rows, exactly-once pagination, real change impact, cycle detection#1154
Merged
Merged
Conversation
…aults Round-2 overhaul of the tool-output surface, built eval-first (byte harness + no-context field-eval agents, always vs the v0.9.0 release binary): Format — tree syntax everywhere: - compact_out emits tree tables: 'key: N (cols: a b c)' headers, indented space-delimited rows, '-' placeholders, space-aware quoting. Every table in every tool converts mechanically. - Grouped prefix-factored output is the DEFAULT for search_graph and trace_path: rows grouped under their shared (qn-prefix, file) printed once, row qn = group prefix + '.' + name (stated in-band). Measured: 192-hit search 2,929B (was 4,811 TOON / 238,466 v0.9.0), 82-row trace 2,256B (was 4,234 / 8,094). Field eval: 5/5 answers correct, QN reconstruction verified, no comprehension loss. - detail:'ids' tier: bare-qn enumeration for wide sweeps (-44%). Pagination — exactly-once, stateless: - store_meta generation (per-DB random uid + mutation counter bumped in cbm_store_upsert_project — every index run's choke point): cursors can never silently validate against a rebuilt DB. - trace cursors: opaque token (leg, generation, params-hash, (hop,id) watermark); pages drain callees then callers with exact callees_total/callers_total on every page; teaching errors for params-mismatch and staleness. Guard: 12 callees paged 5+5+2, every row on exactly one page. - deterministic ordering: BFS ORDER BY hop,id; canonical (hop,id) sort + min-hop dedup across same-name seeds (hop fed risk_labels wrong before); search (name,id)/(rank,id) tie-breaks make offset pages stable. Lean defaults + teaching errors: - get_graph_schema no longer advertises blocked internal fields (fp/sp/bt). - index_repository coverage lists: counts + 5 examples + one-line pointer notes (full record was always in the logfile); 6.0KB -> 1.5KB here, 53KB observed on a large repo. index_status gates the git context block behind verbose:true. - trace limit param (<=5000) + truncated:true — never a silent truncation; invalid direction now errors with the valid values; requesting core columns via fields hints instead of emitting empty cells; cypher resolves f.file/f.qn/f.path aliases. Trace-correctness fixes (reproduce-first, RED verified): - bfs_collect_edges 4KB id-string overflow: past ~1000 visited nodes the edge SQL failed to prepare and EVERY trace edge silently vanished; now a per-connection temp-table join (star guard: 1200/1200 edges). - bfs_union_same_name: min-hop across seeds + edge dedup. Guards: gating tool-output regression suite (context-explosion detector: blocked-field leaks, repeated-key envelopes, oversized notes + calibrated byte ceilings + semantic floors). Full suite 6,367 passed / 0 failed. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
…tringified tree
- cbm_toon_* renamed to cbm_tree_* (the emitters have emitted tree syntax
since the format flip; the name now matches).
- format enums reduced to [tree, json], default tree; dead tree/toon flags
removed; tool RESPONSE descriptions rewritten for the grouped-tree
contract (group prefix printed once, full qn = prefix + '.' + name).
- format:'json' is now the json-stringified tree: the SAME grouped model
serialized as JSON — {total, cols, groups:[{qn_prefix, file,
rows:[[column-ordered cells]]}], has_more} for search, per-leg
{cols, groups} for trace (risk/args columns follow the flags),
query_graph's columns+rows shape already complied. Per-row key envelopes
and the legacy property blob are gone (blob-never-spills pinned by test);
requested add columns in both encodings.
- include_connected keeps the per-node-object shape (nested neighbors) —
the one remaining legacy consumer, conversion tracked.
- legacy verbose emitters deleted (bfs_to_json_array).
Full suite 6,367 passed / 0 failed; lint clean.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
…SCC)
Phase 3 of the graph-analysis overhaul. Two new capabilities on a shared
multi-source traversal substrate, plus a context-bomb guard.
detect_changes — rebuilt into real blast-radius analysis:
The old handler parsed and echoed depth but never traversed: impacted was
just the symbols DEFINED in changed files. It now resolves the git diff to
those symbols, then runs ONE multi-source BFS to the transitive impact set.
- default direction inbound = the blast radius (transitive callers that may
need review); outbound/both available.
- seeds are excluded from the result, and a changed file reached from
another changed file is not counted as extra impact (that is not
downstream fallout).
- merge_base SHA pins the exact diff base for reproducibility.
- impacted_modules: a 2-segment (src/mcp, tests/...) rollup — a quotient
view of where the radius lands, always complete (overflow bucketed, never
silently dropped). Per-symbol rows cap at limit (default 200, nearest
hops first) with impacted_total exact and impacted_omitted surfaced.
On a 7-file core diff: 1469 impacted symbols, 64KB unclipped -> 10.7KB with
the rollup + cap. format:json returns the same model as structured JSON.
cbm_store_bfs_multi (store): one recursive CTE anchored on ALL seeds via a
temp table — never seed_count separate walks over overlapping hubs.
Seed-exclusion NOT IN, MIN(hop) across the set, canonical (hop,id) order,
a memory-safety ceiling that reports *truncated instead of silently
capping. Reproduce-first: seed-exclusion + min-hop, and truncation.
get_architecture aspects:[cycles] — SCC condensation:
Iterative Tarjan (heap stacks, no recursion overflow) over the CALLS graph
reports circular dependencies (strongly-connected components of size > 1).
cbm_store_fetch_call_edges feeds it with a 400k-edge scan budget that
reports partial coverage rather than truncating silently. Opt-in ONLY —
never via a default or 'all' call, since it scans the whole call graph.
On this repo it surfaces 26 real cycles (e.g. the C-LSP type-evaluation
recursion) in ~5KB. Reproduce-first: a 3-node cycle is found, an acyclic
chain is not, and a default call does not run the scan.
get_code_snippet: cap the returned line span at 500 (a structural Module/File
node spans its whole file — an unclipped read returned ~400KB in one call).
source_clipped flags it; start_line/end_line still allow a targeted re-read.
Full suite 6,372 passed / 0 failed; lint clean.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Fresh-eyes review of the phase-2/3 work; five defects, each against our own stated principles (no silent caps, exact counts, same model in both encodings): - trace callees_total/callers_total counted test-file rows even when include_tests=false hides them from the table — the scalar disagreed with the enumerable set (a field-eval agent read callers_total=175 against a handful of visible rows and stopped trusting the tool). Totals now apply the same filter as the emitters; description documents the transitive (within-depth) semantics. Guard test: filtered=1 vs include_tests=2. - cycles clamped the SCC list at 100 silently, losing the true count. cycles_total is now always exact and cycles_omitted is emitted when the display clamp engages (both encodings). - detect_changes accepted a 'limit' param (referenced by its own omitted- hint) that the schema never declared — undiscoverable. Declared. - detect_changes format:json lacked the impacted_modules rollup the tree encoding carries — model parity restored via a shared rollup helper. - tree search extra-field cells were appended raw: a signature with spaces shifted every following column. Cells now route through the shared emitters (space-triggered quoting, '-' placeholder); the properties test pins the quoted form. Full suite 6,373 passed / 0 failed; lint clean. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
…el everywhere
Fresh review pass over the whole branch; three classes of fix:
Cursor safety — the 'legacy' generation gap:
A pre-migration DB (indexed by an older binary, opened read-only) has no
store_meta, so its generation reads 'legacy' — and 'legacy' == 'legacy'
across rebuilds, which silently defeats stale-cursor detection for exactly
those DBs: a stale (hop,id) watermark would resume on wrong node ids.
Cursors are now neither minted (truncated + raise-limit hint instead) nor
accepted (cursor_unsupported teaching error) under a legacy generation.
The standard flow was verified safe empirically: every index run seeds
store_meta via cbm_store_upsert_project in the flush path, and a rebuilt
DB file gets a fresh random db_uid, so aliasing is impossible there. The
guard is unreachable through the public API (upsert always seeds) — it
protects only genuinely old DBs, hence no synthetic test.
format:'json' now emits the tree model on EVERY tool (the last five):
- BM25 query mode: {total, cols, rows[[qn,label,file,lines,rank]]} —
rank order preserved, ranked output is never regrouped.
- semantic: {semantic: {cols, rows[[qn,label,file,score]]}}.
- search_code compact/full: {cols, rows} with nested match-line arrays;
full appends a per-row source object (same guarded, windowed reader);
raw_matches likewise {cols, rows}. files mode stays a plain list.
- get_architecture: all twelve sections emit {cols, rows} via one shared
section helper; cluster row cells carry their nested lists as arrays.
- include_connected became a real column in BOTH encodings
(';'-joined cell in the text tree, nested name array in json) — the
include-connected-forces-JSON coupling is gone, and the last legacy
per-node-object emitters (emit_search_results, enrich_node_properties)
are deleted.
Teaching-error parity: detect_changes now rejects an unknown
with the valid values (it silently fell back to inbound; trace_path already
errored — the caller would have misread the result's semantics).
Full suite 6,373 passed / 0 failed; lint clean. Live-verified on the
self-index: bm25/search_code/architecture json shapes, the connected
column, and cursor pagination.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The PR CI caught two consumers the format migration missed — both parsed
tool output by its old shape and failed silently or with a wrong count:
- hook-augment built its PreToolUse additionalContext by reading the
legacy 'results' object-array from search_graph format:json; after the
json-tree reshape that key no longer exists, so the hook emitted nothing
(caught end-to-end by tests/windows/test_hook_augment.py). The parser
now walks groups -> qn_prefix/file + column-ordered rows. A new local
test feeds the parser the LIVE server envelope, so any future drift
between response shape and hook parser fails in the normal suite on
every platform — not only in the Windows CI guard. Reproduced RED
(ctx NULL, the exact CI failure) before the fix, GREEN after.
- smoke-test.sh carried six old-shape parsers (callers[N], rows[N]{,
clusters[N], semantic[N]{, and the assert_toon_table helper); the tree
headers never matched, so trace verification read 0 callers. All six
updated to the tree contract; the full smoke script passes end-to-end
locally.
cli suite 222 passed; lint clean; scripted smoke ALL PASSED.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
DeusData
added a commit
that referenced
this pull request
Jul 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
A ground-up overhaul of the MCP tool-output surface, built eval-first: every decision was measured with a byte harness plus no-context field-eval agents answering fixed questions through the CLI, always benchmarked against the v0.9.0 release binary.
The format: one model, two encodings
All tabular output now uses a prefix-grouped tree: the shared (qualified-name prefix, file) is printed once per group, rows beneath carry only the short name plus data cells, and the reconstruction rule (
qn = group prefix + "." + name) is stated in-band so agents can copy exact join keys into follow-up calls.format:"json"returns the same model as structured JSON —{cols, rows[[...]]}, grouped where the text is grouped, never regrouped where order is the signal (ranked BM25/semantic output). The TOON format and every legacy per-node-object emitter are gone.include_connectedis now a realconnectedcolumn in both encodings instead of forcing a verbose JSON shape.Exactly-once pagination
trace_pathpages via stateless cursors: an opaque token carrying (leg, store generation, params hash, (hop,id) watermark). Pages never overlap or skip (guarded by test: 12 callees paged 5+5+2, each row on exactly one page), replaying a cursor with different arguments or against a reindexed store yields teaching errors, and every page carries exactcallees_total/callers_total(filtered the same way the tables are). The generation is a per-DB random uid + mutation counter, so a rebuilt index can never silently validate an old cursor; pre-migration DBs without generation tracking refuse cursors explicitly rather than paginate unsafely.detect_changes: from file list to blast radius
The old handler parsed
depthbut never traversed — "impact" was just the symbols defined in changed files. It now resolves the git diff to those symbols and runs one multi-source BFS (temp-table anchored CTE, seeds excluded from the result, min-hop across the seed set) to the transitive impact set: default inbound = the callers that may need review. Output carries the merge-base SHA for reproducibility, a completeimpacted_modulesrollup, exact totals, and a nearest-hops-first display cap that never hides scale (64KB → 10.7KB on a 7-file core diff, while becoming semantically correct).Cycle detection
get_architecture(aspects:["cycles"])condenses the CALLS graph with an iterative Tarjan SCC pass and reports circular call dependencies (components of size > 1) with exact totals and an explicit scan budget. Opt-in only — it never runs on a default orallcall. On this repo it surfaces 26 real cycles in ~5KB, under a second.Correctness fixes found along the way
bfs_collect_edgesbuilt its visited-id set into a fixed 4KB string: past ~1000 visited nodes every trace edge silently vanished (and a truncated id could match an unrelated node). Now a per-connection temp-table join, guarded by a 1200-spoke star test.directionvalues silently returned empty traces; unknownaspects, unknown search fields, and core-columnfieldsrequests all get teaching errors or hints now.get_code_snippeton a whole-file node returned the entire source uncapped (400KB observed); now clipped at 500 lines with an explicit flag.Guardrails
A gating tool-output regression suite pins byte ceilings, semantic floors, and a context-explosion detector (blocked-field leaks, repeated-key envelopes, oversized notes) over hermetic fixtures.
Measurements (self-index, vs v0.9.0 release binary)
Field eval (5 questions, no-context agent, CLI only): 6 calls vs 17, both 5/5 correct — and the v0.9.0 run needed ~12 probes on one question because the capability (cycles) didn't exist and unknown aspects failed silently.
Full suite: 6,373 passed / 0 failed across all platforms' local legs; lint clean.