Give change.detect deadline-aware phase budgets and return partial results, not tool errors

Read internal/mcp/tools_analysis.go and internal/analysis/diffmap.go first, then trace the shared helpers in internal/mcp/analysis_diff.go and the deadline rules in internal/mcp/tool_deadline.go. Implement phase budgets while preserving fast parent cancellation and partial results, then run the named detect budget, lowering, enrichment, and cancellation tests. Done means partial work returns before the reserved deadline and the stated load and regression targets pass.

Why

Under load, change.detect (detect_changes) can still run past its MCP tool deadline (60 s by default). When that happens the agent gets an abandoned call or a raw context deadline exceeded error, not an answer. A report on macOS arm64 saw 59 s abandonments on a 3-file compare. Each phase has a fixed budget that ignores the request deadline. Added together they come to about 36 s before memberships and serialization even start:

• freshness wait: up to 3 s
• git diff plus symbol lowering: one shared 30 s cap
• impact walk: 3 s
• community/process memberships: limited only by the request context

With store contention the total can go past the deadline. Worse, finished work gets thrown away. If the deadline fires after enrichment has completed, the impact result is dropped and a tool error comes back. Clients then retry right away, and each retry adds another full computation on the daemon.

Target for this release: 0 abandonments in 200 detect calls under contention, and detect p95 ≤ 15 s.

Current state

Checked on main (015b3410). None of this work has been merged.

• internal/mcp/edit_serialization.go: defaultMutationReindexWait = 3 * time.Second is the freshness wait that handleDetectChanges goes through (awaitMutationFreshnessForRepos).
• internal/analysis/diffmap.go, MapGitDiffContext: puts both phases under one context.WithTimeout(ctx, 30*time.Second) (line ~99) and passes that same ctx to joinHunksToSymbolsContext (line ~113). A slow lowering therefore uses up the git budget. It comes back as a bare ctx.Err() from the per-hunk and per-node checks (~223/234/250), not as "git diff failed".
• internal/mcp/tools_analysis.go, handleDetectChanges: any MapGitDiffContext error becomes mcp.NewToolResultError (~384-387), and so does any analyzeDiffImpact error (~436-439). For a non-ready graph the handler sets reader = nil and returns file changes with complete: false and risk: "UNKNOWN". That path already works and must keep working.
• internal/mcp/analysis_diff.go: diffImpactContext caps the walk at a fixed 3 s. analyzeDiffImpact returns an error when analysisMembershipsForNodes sees a cancel (~95-97), and it ends with return result, ctx.Err() (~120), which throws away a finished result.
• The same diff and membership helpers are used by review.diff_context (internal/mcp/tools_enhancements.go, handleDiffContext, ~3054/3070) and pr_context (internal/mcp/tools_pr_review_context.go, ~191/235).
• The tool deadline comes from internal/mcp/tool_deadline.go: DefaultToolCallTimeout is 60 s, the override env is GORTEX_MCP_TOOL_TIMEOUT, and transportDeadlineMargin is 1 s. AbandonedToolCalls() is a counter shared by the whole process.
• There is no detectBudget helper, no lowering_truncated field and no enrichment field yet.
• bench/daemon-latency has no detect_changes lanes, no dirty-tree fixture and no writer option yet. They are added by the detect validation and harness work, which is tracked separately.

This work builds on the structured detect envelope (complete / truncated / lower_bound on every response, plus per-phase timings_ms). That envelope is tracked separately and must land first.

What to do

• Split MapGitDiffContext so git keeps its own capped context and lowering gets a separate budget. A git timeout or failure stays a tool error, because a failed git diff is not an empty diff.
• Add a detectBudget helper in a new file, internal/mcp/detect_budget.go:
  • It derives a soft deadline from ctx.Deadline() minus a reserve for serialization (start at 2 s).
  • Each phase gets min(fixed cap, share of remaining time). Starting shares: git ≤ 40 %, lowering ≤ 25 %, impact ≤ 25 %, memberships get the rest.
  • Tune the shares from the per-phase timings_ms that the envelope work exposes, and record the final values in the PR.
• When a phase hits its soft budget, return what it has plus a reason:
  • lowering: changed_symbols is a prefix, lowering_truncated: true is set, and the files not yet joined are listed.
  • impact: truncated: true, which AnalyzeImpactContext already produces.
  • memberships hit their budget or see a cancel: return a result with enrichment: "unavailable", not an error.
  • analyzeDiffImpact returns the result with enrichment: "complete" | "partial" | "unavailable" instead of ctx.Err().
• Any partial result sets complete: false. A partial changed_symbols is never paired with risk: "NONE".
• When the caller (parent context) cancels, as opposed to the soft budget running out, keep returning ctx.Err() promptly.
• Keep the reader = nil path for a non-ready graph ahead of all budget logic.
• Use the same helper in review.diff_context and pr_context.
• All new response keys are additive. Existing keys keep their shape.

Out of scope

• Count-only detect output (detail: counts) and lowering hunks per file instead of per hunk.
• Single-flight coalescing of identical concurrent detect calls. That is a separate issue and it depends on this one.
• Handling of failed or pending receipts: refusals, retry limits, reconcile.
• Changes to the global tool deadline or to the dispatch queue.
• Adding the detect_changes lanes to the latency benchmark (covered by the detect validation and harness work).

Done when

• Reference load: a 4-worktree workspace with about 130k nodes per worktree (±20 %; per-worktree node and edge counts recorded on this issue), a dirty tree of 10 modified files / about 40 hunks / 1 deleted file, a background analyze health loop and 2 concurrent edit_file writers. On this load, detect_changes with full detail has 0 abandonments in 200 calls.
• On the same load, detect p95 ≤ 15 s on Apple M-series 16 GB and on a GitHub ubuntu-24.04 4 vCPU runner, and p95 ≤ 25 s on GitHub windows-2022.
• TestDetectPhaseBudgetTable has a slow-reader case for each phase (freshness, git, lowering, impact, memberships). Every case returns before deadline − reserve with complete: false and the right reason.
• TestDetectSlowLoweringReturnsPrefix gets a prefix with lowering_truncated: true, never a raw context deadline exceeded.
• TestDetectDeadlineAfterEnrichmentReturnsResult gets the result back, not an error.
• TestDetectParentCancelExitsFast: the handler exits within 100 ms of a parent cancel.
• Over 50 forced-abandon runs on the reference load (ubuntu-24.04 4 vCPU), the handler goroutine exits ≤ 1 s after the deadline at p99, and never later than 10 s.
• A test proves a git failure is still a tool error and never shows up as "no changes".
• No test or fixture produces a partial changed_symbols together with risk: "NONE".
• review.diff_context and pr_context use the same helper, with one test for each covering a budget hit.
• No regression: on the gortex repo (~31k nodes, single worktree, no contention), full detect returns complete: true with no budget reason in 20 of 20 runs. Existing detect golden outputs are unchanged apart from the new additive keys (lowering_truncated, enrichment).
• go test -race ./internal/mcp/... ./internal/analysis/... passes.

How to verify

• go test -race -run 'TestDetect(PhaseBudget|SlowLowering|DeadlineAfterEnrichment|ParentCancel)' ./internal/mcp/. The cancellation tests read AbandonedToolCalls(), so they must not use t.Parallel().
• Run bench/daemon-latency with the detect_changes lanes, the dirty-tree fixture and 2 writers against the reference workspace on each host. Report p50/p95/p99, the abandonment count and the per-phase timings_ms, and paste the numbers here.
• To force the deadline, lower GORTEX_MCP_TOOL_TIMEOUT (read by toolCallTimeoutFromEnv). Compare the goroutine count before and after the forced-abandon runs.

Dependencies

Blocked by (must land first):

• #1066 — Detect phase budgets build on the truthful detect envelope.
  Merge after (same code region, keep this landing order):
• #1068 — Edit-region landing order puts the remaining change-tool partials before detect phase budgets.
  Blocks:
• #1070 (needs this)

Related issues

• Addresses: #691 (close those when the targets below pass and the reporters confirm)
• Related: #692
• PRs: #857

Planning

• Milestone: v0.64.15
• Priority: latency (first priority)
• Estimate: ~4 engineer-days