fix(mcp): bound plan context reads and derive lane freshness once - #2297
Conversation
Plan-mode context attributed test coverage through a whole-generation test-annotation census: it paged every symbol and fanned out over every annotation edge before looking at the anchors' callers. On this repository's ~200k-symbol index that took the read past its 10 s project route deadline (`project route error (timed_out)`). Coverage now comes from the anchors' two-hop caller files alone: named test files directly, inline-annotated files through the per-file catalog index. The unscoped census had no other production caller and is removed. Context reads now carry the verified graph read's cost receipt. A search or context response could say `state=fresh rebuild_in_flight=false` and still mark lanes stale: the query gate that served the lanes and the scheduler reading behind the verdict were two freshness authorities. Lane states are now restated under the scheduler reading, so a generation the scheduler proves current reads complete in every lane, and a newer sealed generation keeps them stale.
|
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b46bd7e76c
ℹ️ 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".
| CodeIndexLaneStatusV1::Stale { generation } if generation == served_generation => { | ||
| *lane = CodeIndexLaneStatusV1::Complete; |
There was a problem hiding this comment.
Preserve lane-local stale outcomes
Do not convert every matching Stale lane to Complete: CodeIndexSearchCoverageV1::from_fallback_lane_coverage uses this variant both for a whole-generation fallback and when an authenticated retriever independently returns PublicRetrieverStatus::Stale. In the latter case, a scheduler report that the worktree generation is current does not make that lane's evidence current; this rewrite can turn an empty/stale exact, lexical, or graph lane into complete and make the overall response report fresh. The conversion needs provenance that the status came specifically from served_stale, or it must preserve lane-local stale statuses.
AGENTS.md reference: AGENTS.md:L9-L12
Useful? React with 👍 / 👎.
Two findings from running the released 1.0.0-beta.55 against a ~200k-symbol index. They share the context read path (
compute_contextinhandlers/graph/search.rs), so they land together.(a) Plan-mode context timed out
Rebased onto #2296, which moved
compute_contextintohandlers/graph/context.rs; the change lands there.Cause.
verified_plan_contextcomputed test coverage withtest_annotated_logical_files(None, …): a whole-generation census that paged every symbol (symbols_page) and fanned out over every annotation edge (edges_among), and only then filtered to the anchors' callers. On this repository that alone exceeded the 10 s project route deadline.Change. Coverage comes from the anchors' two-hop caller files: named test files directly, inline-annotated files through the scoped
test_annotated_logical_files(&paths, …)(per-file catalog index, the same pathaffected/diff_context/pr_contextalready use). The unscoped census had no other production caller, so theOptionarm is removed.tracedecay_contextnow reports the verified graph read'sRequestCostReceiptV1, so its reads are observable (tracedecay_cost:trailer).Journey (this repo as corpus, isolated profile, one 12 GB-capped daemon). Same query,
tracedecay tool context --args '{"task":"run_update_command","mode":"<m>","lexical_anchors":["run_update_command"],"format":"json"}':Before, released beta.55 binary (release profile):
After, this branch (perf profile + hotpath, slower than release):
Hotpath (after, 3 plan calls):
mcp.graph.plan_contextavg 2.11 ms,usecases.graph.verified.test_annotated_filesavg 1.44 ms,mcp.graph.context.totalavg 953 ms (search-dominated, same as explore). The release build has no hotpath lane; the before number is the wall time above.Test.
plan_context_reads_only_the_anchor_neighborhood(mcp_suite): a fixture with 40 unrelated files carrying inline#[test]s; plan mode must reporttest_files == ["src/lib.rs"]and a cost receipt withadjacency_rows < 40.assertion left == right failed: one cost trailer: [...] left: 0 right: 1(no receipt; the census path is unmetered).(c) Search freshness contradicted itself
Cause. The executor marks lanes
stalewhenever the query gate served a generation it could not prove current itself, while the verdict comes from the scheduler's worktree reading. The two disagreed in one envelope.Change.
lanes_under_scheduler_freshnessrestates the executor's lane coverage under the scheduler reading: when the scheduler proves the served generation current (fresh, no rebuild, latest == served), a lane stale against that generation is complete (a partial lane drops its stale generation).search_freshnessand the new function share one predicate,scheduler_proves_current. A newer sealed generation keeps the lanes stale.Test.
search_lanes_answer_to_the_freshness_the_verdict_reports(tracedecay lib)."summary": "state=fresh rebuild_in_flight=false served_generation=generation.mcp-verified-graph-fixture.1 latest_generation=generation.mcp-verified-graph-fixture.1 stale_lanes=exact,graph","state": "possibly_stale".{"freshness":{"state":"fresh"}}withexact/graphcomplete; with a newer latest generation,stale_lanes == ["exact","graph"].Verification
cargo test -p tracedecay-graph-query -p tracedecay-mcp --lib: graph-query 23 passed, mcp 390 passed, 0 failedcargo test -p tracedecay --features test-transport,test-helpers --lib -- graph_search_dispatch search_graph_independence: 12 passed, 0 failedcargo test -p tracedecay --features test-transport,test-helpers --test mcp_suite -- typed_evidence_trailers context_behavior context_related search_behavior: 15 passed, 2 failed. The two failures aretyped_callees_carry_their_read_cost_on_the_envelope_and_the_trailerandtyped_callers_carry_their_read_cost, red on master (test(mcp): read-cost pins red after single-row code edges (#2277) #2291), not touched here.cargo clippy -p tracedecay-graph-query -p tracedecay-mcp -p tracedecay --all-targets -D warnings, with and withouttest-transport,test-helpers: clean.cargo fmt --all -- --check: clean.pnpm run contracts:check(dashboard):contracts up to date.