Skip to content

Tree-format tool output: prefix-grouped rows, exactly-once pagination, real change impact, cycle detection#1154

Merged
DeusData merged 6 commits into
mainfrom
feat/toon-round2
Jul 18, 2026
Merged

Tree-format tool output: prefix-grouped rows, exactly-once pagination, real change impact, cycle detection#1154
DeusData merged 6 commits into
mainfrom
feat/toon-round2

Conversation

@DeusData

Copy link
Copy Markdown
Owner

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_connected is now a real connected column in both encodings instead of forcing a verbose JSON shape.

Exactly-once pagination

trace_path pages 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 exact callees_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 depth but 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 complete impacted_modules rollup, 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 or all call. On this repo it surfaces 26 real cycles in ~5KB, under a second.

Correctness fixes found along the way

  • bfs_collect_edges built 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.
  • Multi-seed traces recorded wrong hop distances (risk labels inherited them) and nondeterministic row order; BFS output is now canonically (hop,id)-ordered with min-hop dedup.
  • Invalid direction values silently returned empty traces; unknown aspects, unknown search fields, and core-column fields requests all get teaching errors or hints now.
  • get_code_snippet on 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)

Call this branch v0.9.0
search_graph, 192 hits 2,929 B 246,246 B
trace_path both, 82 rows 1,690 B 3,855 B
get_architecture 3,038 B 51,381 B
detect_changes, 7-file diff 13,029 B 76,962 B

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.

DeusData added 6 commits July 17, 2026 22:32
…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
DeusData merged commit 94c8e96 into main Jul 18, 2026
21 checks passed
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.

1 participant