gather: deterministic context-pack tool (aft_gather) - #152
Conversation
There was a problem hiding this comment.
4 issues found across 7 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
f8e7fa9 to
cf09b9b
Compare
|
Follow-up on the two maintainability notes from the Greptile summary (they weren't separate review threads, so noting here) — both addressed in
Matched/rendered text is byte-identical; 23/23 gather tests green. |
1d068c9 to
d2e15e6
Compare
5364eac to
f42c5ec
Compare
f42c5ec to
66e05e2
Compare
66e05e2 to
d20f276
Compare
|
Rebased onto current The rename covers the advertised tool name and the agent-facing error strings only. The wire command stays Rebase conflicts were confined to Verified: Still one commit. Two notes on the full plugin suite, both pre-existing on |
b06cd3b to
a4b6aae
Compare
|
Opened #192 to ask the design question separately from the diff: whether you want one-call context packs in aft at all, and in what shape. Answer there and I'll rework this to match — or close it if the answer is no. Current state here: green on |
a4b6aae to
500f1a9
Compare
|
Rebased onto v0.49.3 ( CI: 13 green, one red — Scenario 1's
Same job, same assertions, on the two nearest green runs:
So the same code path passes on the parent commit and on the previous revision of this branch. The delta between them is a 90s wall clock on a cold Windows runner, not the diff — nothing in this change is reachable from bridge spawn or plugin boot. I can't re-run it — Separately: #192 asks whether you want this capability at all, which is the more useful question than another rebase. |
500f1a9 to
9bb5af3
Compare
|
Rebased onto
The four new commits didn't touch any file this branch touches, so the rebase was clean and only the governed manifests needed regenerating for the new base. |
a486e48 to
c72bcfd
Compare
5eb5870 to
6b50f10
Compare
6755cb9 to
4707e57
Compare
|
Green on Since the last update this branch picked up The flag was inert on one path. The coverage gap that allowed it is closed. Restored, it passes with 20 neighbors including the test callers. Red → green → red, all three outcomes observed. A Windows-only test bug. Two red rounds along the way were runner flakes, both cleared on the next sha with no code change touching them — Full gate locally: Still the open question from #192, and it is yours rather than mine: whether one-call context packs belong in aft at all. There is now a second person asking for them in that thread with an independently-arrived-at use case. |
4707e57 to
9e147b9
Compare
9e147b9 to
631a4d8
Compare
|
Greptile's P1 was correct and I've fixed it in
The schema advertises Fixed by reading either spelling and writing the internal one: fn gather_file_path(map_in: &Map<String, Value>) -> Option<&str> {
["path", "filePath"]
.into_iter()
.find_map(|key| map_in.get(key).and_then(Value::as_str))
.filter(|value| !value.is_empty())
}On the tests, since the masking is the more interesting part of this report. Both existing tests passed
Red control on both new tests: removing Verification: 4222 Rust tests, 1 pre-existing sandbox failure that also fails on clean Also rebased onto |
Normalize path separators before comparison so absolute and relative spellings of the same file dedupe on Windows. Path::is_absolute() is false for rooted-no-drive paths there, and strip_prefix emits backslashes, so the same symbol previously rendered twice in one pack. Backslash is a legal filename character on Unix, so a file literally named 'a\b.rs' now normalizes to 'a/b.rs' and renders a not-found stub. Accepted: matches normalize_path_for_compare, fails gracefully.
631a4d8 to
28930e0
Compare
|
Closing with #192 — see the rationale there. Thanks for building the exploration out; the neighbor-expansion thinking fed into the read-range hints that did ship. |
gather: deterministic context-pack tool (aft_gather)
What
One new tool —
aft_gather— assembles a bounded "context pack" (ranked, deduped, budgeted verbatim code evidence) in a single call. It replaces the multi-turnsearch → outline → zoom → callgraphread chain an agent otherwise runs to build context around a question or a symbol.Two modes (mutually exclusive):
question: "how does X work?"— seeds fromhandle_semantic_search(same pipeline asaft_search, all lanes/fallbacks)symbol+filePath— seeds from the callgraph (impactdepth-1 callers +call_treedepth-1 callees)Seeds expand one hop through the callgraph, dedupe by canonicalized (file, symbol) with seeds winning, and render via
render_symbol_within_budgetuntil a hard line budget (default 400, cap 800) is spent. Everything past the cut appears as one-line stubs under## Beyond budget (zoom to expand)— nothing is silently dropped.Why
Agents burn serial turns assembling context: search, then outline the hits, then zoom the symbols, then chase callers. Each turn round-trips through the model. A pack returns evidence (verbatim bodies with file:line headers), not conclusions — the agent reasons over it directly, ready to attach to a subagent dispatch.
Measured on a real config repo (3 questions, one call each vs. the manual chain):
used=283/400. Manual baseline: 5-6 tool calls.budget=200.Independently reproduced on the aft codebase itself:
question: "how does bash output compression dispatch pick a compressor"→seeds=15, used=226/400, one pack assembling the full dispatch chain (compress→ gate →compress_with_registry_exit_code20-compressor array →Compressortrait → install path → subc mirror) that otherwise takes a 4–5-call search→zoom chain.Honest degradation
The pack never lies about its own quality:
file:line (no containing symbol)stubs and flags the header withdegraded=semantic-index-building (partial results — retry when index ready)— detected via the response'ssemantic_statusfield, cleared as soon as one real seed resolves. No blocking or retry inside the tool.{file, line_text, line}, no symbol name) resolve to their containing symbol by line containment — definitions, call sites, and comment hits all upgrade to the enclosing symbol. Hits with no containing symbol stay visible as stubs.(N unresolved external calls omitted)) instead of drowning the stub list; unresolved seeds and callers are never suppressed.Implementation
crates/aft/src/commands/gather.rs— Rust-side composition: callshandle_semantic_search/impact_result/call_tree_result/render_symbol_within_budgetdirectly (shared&AppContext, no bridge round-trips, no parallel reimplementation of search).main.rsdispatch arm,subc_translate.rsmapping, TS factorypackages/opencode-plugin/src/tools/gather.ts+ registration (same tier asaft_callgraph— depends on the callgraph store).protocol.rsResponsedoc-comment):success:false+codefor un-performable calls (e.g.invalid_requeston a bad mode combo),success:truewith a visible degraded/stub pack for partial results — never a bare empty success.Tests
22 unit tests in
gather.rs, including red-checked regressions (each confirmed to fail against pre-fix code): mid-codepoint truncation panic, duplicate-symbol line-anchored resolution, abs/rel path dedupe, containing-symbol resolution via a realTreeSitterProvider, callee-only stub suppression driven through the productionbuild_packpath, and degradation-flag presence/absence/mixed cases.Limitations (deliberate scope)
aft_callgraph.Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds a deterministic context-pack builder:
gatherin Rust andaft_gather_contextin@opencode. It replaces the multi-step search→outline→zoom→callgraph chain with one call that returns ranked, deduped, verbatim code within a fixed line budget.question(semantic seeds) orsymbol+path(impact callers + call-tree callees); 1-hop expansion; dedupe by (file, symbol) with seeds winning.degraded=semantic-index-buildingandneighbors=skipped(callgraph-unavailable).includeTeststo include test-file neighbors (default off).pathmaps to internalfilePath(translator accepts both). Schema inventory updated (22 bare tools)./and renders a clear not-found stub.includeTests. No new dependencies.Written for commit 28930e0. Summary will update on new commits.
Greptile Summary
This PR introduces
aft_gather— a single-call "context pack" builder that replaces multi-turnsearch → outline → zoom → callgraphchains. It seeds from either a natural-language question (viahandle_semantic_search) or asymbol+filePathpair (viaimpact+call_tree), expands one callgraph hop, deduplicates by canonicalized(file, symbol), and renders verbatim symbol bodies within a hard line budget, listing overflow as visible stubs.gather.rs(1665 lines): core pack assembly with budget accounting, degradation flags, path normalization, grep-fallback symbol resolution, and 22 unit tests including red-path regressions. Acollect_callgraph_neighborspath passes normalized-relative seed paths toimpact_result/call_tree_resultwhile symbol mode usesctx.validate_path-resolved absolute paths for the same functions — a mismatch that could silently produce empty neighbor sets in question mode.subc_translate.rs:translate_gather+gather_file_pathhelper correctly accepts both"path"(schema-advertised) and"filePath"(internal) spellings, resolving the previously-flagged schema-field bug. Schema-exhaustiveness test added.gather.ts+ wiring: TypeScript adapter exposespath, renames tofilePathbefore bridge dispatch; registered asALL_ONLYon the OpenCode surface alongsideaft_callgraph.Confidence Score: 4/5
gather_file_path) and theinclude_teststhread-through are correct. The one open question is whethercollect_callgraph_neighborssilently produces empty neighbor sets in question mode: it callsimpact_resultandcall_tree_resultwith normalized-relativeseed.filepaths while every other call site in the codebase uses absolute validated paths, and errors are swallowed withif let Ok. If the callgraph store is keyed on absolute paths, question-mode neighbor expansion returns nothing with no visible indicator. The integration test covers only symbol mode, so this path is untested end-to-end.collect_callgraph_neighborslines 421–447 and the absence of a question-mode callgraph expansion integration test.Important Files Changed
collect_callgraph_neighborspasses normalized relativeseed.filepaths toimpact_result/call_tree_resultwhile symbol mode uses absolute validated paths for the same functions; errors are silently swallowed, making question-mode neighbor expansion potentially invisible. Also contains unreachable third dedup loop. Otherwise the budget accounting, degradation flags, path normalization, and suppression logic are well-designed with thorough unit tests.translate_gatherwith agather_file_pathhelper that correctly accepts both"path"(schema-advertised) and"filePath"(internal/OpenCode) spellings. The previously-reported schema-field mismatch bug is resolved. New tests cover both spellings,includeTestsforwarding, and schema-key exhaustiveness.pathexternally, renames it tofilePathbefore dispatching to the Rust bridge. Mode validation mirrorstranslate_gather. Transport-invariance test covers the key rename and confirms no internal transport names leak into args.includeTestsflag in symbol mode (hiding vs. showing test-file callers) with a real filesystem fixture. No integration test exercises question mode's callgraph neighbor expansion path, which is the path most likely to be affected by the relative-path inconsistency.gatherin the agent-core tool set,HeavyInitlane (same tier ascallgraph/semantic_search), and the manifest tool list. Manifest test updated. Changes are consistent with existing patterns.aft_gather_contexttoALL_ONLY_TOOLSand spreadsgatherTools(ctx)unconditionally, consistent with how other ALL_ONLY tools likeaft_refactorare registered (surface filtering is handled bynormalizeToolMap).Sequence Diagram
sequenceDiagram participant Agent participant gather.ts participant translate_gather participant handle_gather participant handle_semantic_search participant collect_callgraph_neighbors participant build_pack Agent->>gather.ts: "aft_gather_context({question|symbol+path})" gather.ts->>gather.ts: rename path→filePath, validate mode gather.ts->>translate_gather: gather_file_path() accepts "path" or "filePath" translate_gather->>handle_gather: "{command:"gather", filePath:...}" alt question mode handle_gather->>handle_semantic_search: "top_k=15 seeds" handle_semantic_search-->>handle_gather: results + semantic_status handle_gather->>collect_callgraph_neighbors: seeds (normalized-relative file paths) Note over collect_callgraph_neighbors: impact_result(seed_path) where seed_path is relative<br/>symbol mode uses ctx.validate_path (absolute) — inconsistency collect_callgraph_neighbors-->>handle_gather: neighbors (or silently empty) else symbol mode handle_gather->>handle_gather: ctx.validate_path → absolute file_path handle_gather->>handle_gather: "impact_result(&file_path) — absolute ✓" end handle_gather->>build_pack: seeds + neighbors + budget build_pack->>build_pack: dedup, render within budget, stub overflow build_pack-->>Agent: gather pack text with file:line headersReviews (29): Last reviewed commit: "gather: deterministic context-pack tool ..." | Re-trigger Greptile