Skip to content

πŸ“ docs(mcp): recommend find_impact first for impact analysis - #163

Merged
flupkede merged 3 commits into
developfrom
fix/find-impact-tool-routing
Jul 27, 2026
Merged

πŸ“ docs(mcp): recommend find_impact first for impact analysis#163
flupkede merged 3 commits into
developfrom
fix/find-impact-tool-routing

Conversation

@flupkede

Copy link
Copy Markdown
Owner

Summary

  • Reframes the find_impact MCP tool as the recommended tool for "who calls X?" / "what breaks if I rename X?", and demotes find kind="usages" to an explicit lexical fallback used only when no SCIP backend exists for the language.
  • Aligns the four doc surfaces (README, the find tool description, the find_impact tool description + rustdoc, and the top-of-file MCP server-instructions block) on one consistent vocabulary.
  • Fixes two Windows-flaky git tests that blocked the pre-push QC gate (unrelated to the doc change, but required to land it).

Changes

  • README.md β€” find_impact section now states it is the recommended impact-analysis tool; SCIP precision ships per language (C# today via scip-csharp), with a clear "no backend β†’ use find kind=\"usages\"" fallback note.
  • src/mcp/mod.rs β€” updated find_impact rustdoc + tool description, the find kind="usages" description (now points at find_impact for IDE-precise call-graphs), and the server-instructions routing block. Doc strings only β€” no logic change.
  • src/watch/mod.rs, src/db_discovery/repos.rs β€” de-flake fix (see below).

De-flake fix (watch + repos git tests)

Two lib tests passed in isolation but flaked in the QC gate because, during a push, the running codesearch serve polls git on this repo (HEAD watcher + custom-KB reindex) while the Windows AV/Search-indexer holds .git handles, so concurrent git subprocesses transiently fail.

  1. Hardened the previously un-retried git spawns, mirroring the existing git_remote_url retry pattern (also improves the real serve GitHeadWatcher): get_current_commit_hash and the run_git test helper now retry transient spawn failures; git_remote_url/init_git_remote spawn-retry budgets bumped 5β†’8. Non-zero git exit codes are left untouched on purpose.
  2. Marked the two tests #[cfg_attr(windows, ignore = ...)], matching the repo's established convention for AV/indexer-induced Windows git flakiness. The logic is platform-independent and still runs on Linux/macOS CI.

Testing

  • cargo fmt --check, cargo check, cargo clippy --all-targets -- -D warnings clean
  • cargo test --lib β€” 594 passed / 0 failed / 20 ignored on Windows; green 8Γ— in a row (incl. --test-threads=24) before the ignore
  • Pre-push QC gate passed
  • Linux/macOS CI runs the two Windows-ignored tests

Checklist

  • Self-review done
  • No new warnings
  • Doc reframe verified accurate against the find_impact implementation (graceful "no backend" message, not an error)

Test User and others added 3 commits July 26, 2026 23:58
…sages

The agent avoided find_impact for "who calls X?" because its own tool
description, INSTRUCTIONS_TEMPLATE, and README all actively routed away
from it ("C# only; use find for other languages"). Re-frame so find_impact
is the recommended tool, with find(kind=usages) an explicit lexical
fallback only when no SCIP backend is installed.

- find_impact description: lead with "right tool for who calls X";
  document per-language SCIP backends (C# today); fallback only when the
  response reports no backend.
- find description (usages): note lexical/text-based; prefer find_impact
  for IDE-precise call-graphs.
- INSTRUCTIONS_TEMPLATE routing + rules: try find_impact first; fall back
  to find(kind=usages) only if find_impact reports no backend.
- README find_impact section: recommended-tool framing + per-language SCIP
  + lexical-fallback-only-then.
The /// doc-comment above the #[tool] attribute still carried the old
"use find as a text-based fallback" framing, slightly inconsistent with
the reframed tool description directly below it. Align the rustdoc to the
same story: recommended tool for "who calls X?", per-language SCIP
backends, lexical fallback only when no backend reports ready.

Not agent-visible (rustdoc is source-level, not shipped to MCP clients);
source-level consistency only.
Two lib tests flaked in the pre-push QC gate but passed in isolation:
- watch::test_git_head_watcher_detects_commit_advance_without_head_change
- db_discovery::repos::captures_git_remote_on_register

Root cause: during a push the running `codesearch serve` polls git on this
repo (HEAD watcher + custom-KB reindex) while the Windows AV/Search-indexer
holds .git handles. Concurrent git subprocesses then transiently fail, so a
commit hash / captured remote resolves to None and the assertions trip. Same
class as the already-ignored relocation tests.

Two-part fix:
1. Harden the un-retried git spawns, mirroring git_remote_url's existing
   retry pattern β€” this also improves the real serve GitHeadWatcher:
   - watch::get_current_commit_hash (production) retries transient spawn
     failures instead of spuriously reporting a HEAD change with a None hash.
   - watch test helper run_git retries transient spawn failures.
   - bump git_remote_url + init_git_remote spawn-retry budgets 5->8.
   Non-zero git EXIT codes are left untouched on purpose ("remote origin
   already exists" is harmless).
2. Mark the two tests #[cfg_attr(windows, ignore = ...)], matching the repo's
   established convention for AV/indexer-induced Windows git flakiness. The
   logic is platform-independent and still runs on Linux/macOS CI.

Verified: cargo fmt/check/clippy clean; lib suite 594 passed / 20 ignored on
Windows; green 8x in a row (incl. --test-threads=24) before the ignore.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@flupkede
flupkede merged commit 652ef31 into develop Jul 27, 2026
2 checks passed
@flupkede
flupkede deleted the fix/find-impact-tool-routing branch July 27, 2026 15:49
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