refactor(mcp): answer search through the owner - #2296
Conversation
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
tracedecay_search is an ApplicationSurfaceOperation the project's graph-tool owner answers, on MCP and `tracedecay tool` alike. Its arguments decode against a typed request in the contracts crate, so an unknown key or a mistyped limit is refused instead of being silently ignored or defaulted. Its result is a typed catalog result that serializes to the JSON the tool already emitted: the freshness verdict, per-lane coverage (a partial lane keeps its null generation and reason), ranked rows with display and route evidence, the lexical route and anchor receipts with admitted/dropped counts, the scope report, graph evidence, the external import hint, and parked indexing detail on an unavailable search. Every surface renders it through one renderer; the stale-graph trailer rides the owner's served generation. The 2,345-line graph search module is split by tool: search keeps the request decode, the owner computation, and the renderer; context, find_exact_symbol, the clone family, and rename_preview move to their own modules. The root graph dispatch group, its portable dispatch table, and the Graph binding group are deleted, and the search schema bounds move to the contracts crate beside the request they describe.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a015bfbdc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| compute_search( | ||
| ctx, | ||
| open(read("code_symbol_search")?), | ||
| args, |
There was a problem hiding this comment.
When a caller requests format: "json" and a ranked result has display metadata but a non-code-symbol: anchor (for example, a lexical chunk), this new owner path receives only the canonical request map because separate_application_tool_request removes format before invocation. Consequently compute_search sees render::wants_json(&args) as false and never calls node_id_for, whereas the previous in-client handler enriched these JSON rows. The response therefore silently loses node_id, preventing clients from following the result directly with tracedecay_source_body; carry the requested format to the owner or compute node IDs independently of presentation.
AGENTS.md reference: AGENTS.md:L213-L214
Useful? React with 👍 / 👎.
4a015bf to
86272c6
Compare
1 participant
tracedecay_searchis now anApplicationSurfaceOperationthat the project's graph-tool owner answers, on MCP andtracedecay toolalike. It follows #2284 (retrieve) and #2273.crates/tracedecay-contracts/src/retrieval/search_surface.rs. The result is either a complete page or a typed unavailable state, and it serializes to the JSON this tool already emitted. It carries:freshnessverdict, which also opens the markdown body;SearchCoverageV1), where a partial lane keeps itsnullgeneration and reason exactly as before;display,node_idand per-row route matches;lexical_routesreceipt, and thelexical_anchorsreceipt withmatched/admitted/droppedfrom fix(context): return admitted anchor sites and count what was returned #2090;scope_prefixreport, the unavailableverified_graph_evidence, and theexternal_import_hint;parkedproblem detail from feat(contracts): carry typed problem detail beside the message #2259, which names the failure message.code_graph_freshnesstrailer now comes from the owner's served generation, andtouched_filesfrom the typed completion. Search does not emit atracedecay_costreceipt today, so the completion's cost slot stays empty and the output stays identical.limit("5",5.0) silently fell back to 10. Both are now refused with the field named. A missingqueryis now the typed refusal every owner-served tool gives.crates/tracedecay-mcp/src/handlers/graph/search.rsgoes from 2,345 lines to 762 (491 production, the rest tests). It now holds three pieces: the typed request decode (compute_search→lexical_routing::routing_from_request), the owner computation (compute_search), and the one renderer every surface uses (render_search). The already owner-served reads move to their own modules, unchanged except for imports:context.rs,exact_symbol.rs,clones.rs(similar/redundancy) andrename_preview.rs. The kernel-receipt → wire anchor conversion is now one function (lexical_routing::anchor_outcome), shared by search and context. The in-client search path is deleted: the rootdispatch_graph_tools, the portablegraph::dispatch_tooltable,McpToolDispatchGroup::Graphand its binding group row.SEARCH_MAX_LEXICAL_*) move to the contracts crate, next to the request they bound.graph_report_spec).Fail-before / pass-after (production MCP)
mcp_handler_test::search_behavior_test::search_returns_the_named_symbol_and_refuses_arguments_outside_its_typed_requestnow asserts typed refusals for three requests: a missingquery, an unknownsemantic_mode, andlimit: "5". On unchangedorigin/master(0c85014):With this change, it passes.
On the built binaries, the same corpus shows master silently accepting what this branch refuses (
tracedecay serve):Identical output (80 responses)
The check runs the same 80 calls through
tracedecay serve, first with a binary built from this branch's base (origin/mastercdc1a2a), then with this branch. Both run against one isolated profile and one 768-file corpus (768 files × 128 one-line functions). The 80 calls are 20 symbols × {search, context} × {markdown, json}. Each run waits until the verified graph answers context before it starts. The full text blocks are compared, including thefreshnessline, thecode_graph_freshnesstrailer when present, and thetracedecay_metricsfooter. A truncated response is compared through its stored handle content.The 20 search JSON pages differ only in
next_cursor.expiry,next_cursor.signature, and thequery_fallback_digestthat covers the cursor. These fields are minted per call. Two runs of the master binary differ in exactly the same fields: master-vs-master is 60/80 raw and 80/80 normalized. Branch-vs-master is 60/80 raw and 80/80 normalized, with the same normalized digest1cabaaa2d7fa7419on all three runs. The first comparison exposed one real drift before this was fixed: a partial lexical lane (candidate_sources…) lost itsnullgeneration.SearchCoverageV1now keeps it.Existing tests changed
schema_test(missingquery) andsearch_behavior_test, as above.graph_search_dispatch_tests(9) andsearch_graph_independence_tests(4), plus thedispatch_testsstale-trailer probe. They now run throughdispatch_on_graph_authority, the owner computation plus the shared renderer.cancel_candidate_journeymounts the in-process daemon invocation service beside the server (mcp_server_with_project_retained_owner_for_test), because the search it cancels is now an owner invocation.lexical_routingunit tests decode a typed request and assert typed rows. The route and anchor JSON they pin is unchanged.search_schema_testsread the generated catalog schema and resolve its$defs.catalog_discovery's "legacy tools stay discoverable" probe usestracedecay_dashboard, and search joins the catalog-bound tools filtered like context.Runtime journey (debug
tracedecayfrom this branch, isolated HOME/profile, one daemon undersystemd-run --user --scope -p MemoryMax=6G -p MemorySwapMax=1G)Checks (local)
On 4a015bf (rebased on cdc1a2a):
cargo test -p tracedecay --features test-transport,test-helpers --test mcp_suite(full): 576 passed, 5 failed. The 5 are the read-cost pins that are red on master since perf(graph-db): store each code edge as one relation row #2277/perf(graph-db): stream the sealed store build one column at a time #2282, filed as test(mcp): read-cost pins red after single-row code edges (#2277) #2291 with the master reproduction (relation_page_cost×2,typed_evidence_trailers::typed_callees_carry_their_read_cost_on_the_envelope_and_the_trailer,typed_callers_carry_their_read_cost,test_map_reads_each_test_once_across_the_symbols_it_covers). This covers every search, retrieve, schema, protocol, dependency-hint and graph-query module.cargo test -p tracedecay --features test-transport,test-helpers --lib -- mcp::: 182 passed.mcp::server::routing::tests::many_slow_initialize_roots_share_one_discovery_budgettimed out once under host load at 3.007 s against its 3 s bound. It passed when rerun alone. The test is unrelated to this change.tracedecay-mcplib 390,tracedecay-querylib 264, contracts 420 +contracts_suite277, mcp-catalog 29, daemon-protocol 65, tool-catalog 5 + 22,search_quality_suite71.cargo test -p tracedecay-cli --bin tracedecay -- tool_command: 57 passed.cargo clippy -p tracedecay -p tracedecay-mcp -p tracedecay-contracts -p tracedecay-daemon-protocol -p tracedecay-api -p tracedecay-mcp-catalog -p tracedecay-tool-catalog -p tracedecay-cli --all-targets -- -D warnings, with and without--features tracedecay/test-transport,tracedecay/test-helpers: clean.cargo fmt --all -- --check: clean.pnpm run contracts:generate, thencontracts:check: up to date (SDK regenerated).After the final rebase onto 450a55f (#2294, #2295; neither touches this path), focused reruns:
mcp_suite -- search retrieve schema_test protocol_test dependency_hint graph_query_test: 127 passed, 5 failed. Four of the failures are the test(mcp): read-cost pins red after single-row code edges (#2277) #2291 read-cost pins. The fifth,retrieve_truncation_test::diff_context_large_response_uses_retrievable_truncation_handle, hit a graph that was still warming under host load (load average 105). Rerun alone, it passes.--lib -- mcp::: 179 passed, 3 failed. The three failures were host-load fixture timeouts:connection::cancellable_queue_tests×2 ("production-composition code index did not publish … after 20000 ms") andhost_admission_tests::owned_project_replay_worker_continues_past_one_bounded_batch. Rerun alone: 7 passed.tracedecay-mcplib 390 passed.contracts:check: up to date.