feat(mcp): token-budgeted, seed-aware repo_map query tool#880
Open
petercoxphoto wants to merge 8 commits into
Open
feat(mcp): token-budgeted, seed-aware repo_map query tool#880petercoxphoto wants to merge 8 commits into
petercoxphoto wants to merge 8 commits into
Conversation
DeusData
added
enhancement
Owner
|
Thanks for splitting the repo_map tool into its own PR. Triage: normal-priority editor/MCP integration feature, stacked on #879. Because this adds a new MCP tool surface, review will be intentionally cautious: token-budget enforcement, project-boundary handling, seed resolution, no unexpected file bodies beyond the stated output shape, and local-only behavior all need to be verified. Please keep this stacked behind #879 for review/merge order. |
AC1 tests (tests/test_importance.c) + suite wiring + a no-op pass stub (pass_importance.c) registered as the 7th predump pass in run_predump_passes (after all edge-producing passes). Tests fail against the stub (no importance key persisted) — the red-test boundary. Formula lands in the impl commit. Red evidence: full ASan test-runner run captured at $ARTIFACT_DIR/red-run.log — all 6 importance tests FAIL (test_importance.c:91/167/215/279/331/398). The pre-existing lang_contract ASan abort (vendored ts_runtime lexer UAF) is a baseline flake unrelated to this change. Boundary committed by the orchestrator (recovery: the builder agent authored these files but died on an API overload before committing). Signed-off-by: Peter Cox <info@petercox.ie>
pass_importance.c: weighted-degree Aider model persisted per Function/Method/
Class node as a numeric "importance" key on properties_json (append pattern
from pass_complexity.c). importance = sqrt(num_refs) x priv x generic x
distinct x test_penalty:
num_refs = incoming CALLS + USAGE edges
priv x0.1 leading-underscore
generic x0.1 name defined in >=5 distinct files
distinct x10 snake_case|camelCase and len>=8
test_penalty x0.1 edge-based (TARGET of a TESTS edge, or file is the SOURCE
of a TESTS_FILE edge) -- replaces the filename strstr; a
'test'-substring path with no test edge is NOT penalised.
Weighted-degree only; PageRank deferred (measured decision, builder-notes).
All 10 fixture importance tests pass. Does not touch compute_search_score.
Signed-off-by: Peter Cox <info@petercox.ie>
Real-data verification (full connectors re-index) showed the spec's edge-only TESTS/TESTS_FILE penalty does NOT achieve AC1's exclusion goal: TESTS_FILE has only 15 edges (needs a test-file->prod-file mapping), TESTS never targets a test-resident symbol, and node is_test is stamped on <2% of nodes and none of the test fixtures/doubles. Edge-only left 33/40 of the top ranked symbols as test scaffolding (session_dir, FakeConnectorClient, ...). Fix: penalise when the graph's OWN canonical classifier cbm_is_test_path() (the one pass_tests uses: test_ prefix, _test.<ext>, /tests/, ...) flags the file, OR the symbol is the target of an incoming TESTS edge (prod helpers in non-test files). This is NOT the naive strstr the spec warned against — it leaves src/testutil_helpers.go unpenalised (test-plan DeusData#6 inversion still green). Result after re-index: top-40 = 0 test-file symbols (was 33); named exclusions _run/now/run_hook/client and make_proposals/make_golden/seed_recording all rank >100 (well outside top-40); top is real domain symbols. All 10 unit tests pass. Deviation from the spec's stated mechanism — surfaced to Peter. Signed-off-by: Peter Cox <info@petercox.ie>
Signed-off-by: Peter Cox <info@petercox.ie>
26 failing AC tests derived from the P4 test plan (rows 1-5, 8-13): registration/dispatch, token-budget fit (default/tight/tiny/huge/invalid), seed-boost ranking + inversion, weak-seed widen walk, empty/unresolvable-seed global fallback, score-absence gating, graceful input errors, signature-level rendering, determinism, cross-project isolation, repeated-call stability. All 26 verified FAILING against unmodified code (unknown tool). Five tests carry explicit positive discriminators so the dispatch-level error cannot trivially satisfy them. Rows 6-7 (real corpus, latency) are scripted post- implementation against a worktree binary + scratch CBM_CACHE_DIR. Pre-existing failure (not this change): test_incremental.c:302 RSS threshold (2854-2872MB vs 2048 cap under ASan), file last touched pre-base (eed87fd). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KK6YFYGwhT69NjYevuEUCr Signed-off-by: Peter Cox <info@petercox.ie>
New MCP tool repo_map(project, seed_anchors[], token_budget) reading P3's persisted importance scores (spec wire contract frozen for P5). - Two first-class modes (P2 finding): seed-boost (resolved anchors + 1-hop CALLS/USAGE neighbourhood x50) with global importance ranking as the no-seed/unresolvable fallback. Anchor resolution: exact name -> qualified name -> file path. - Weak-seed widen (P2 shape hints): 1-hop symbol neighbourhood <= 3 -> file-of-symbol seeding + 2-hop expansion at x25. - Module/File 1-hop neighbours (file-level co-usage USAGE edges) expand to their file's symbols at x25 — this is how P2's ground-truth co-change neighbourhood (sender.py/gmail_client.py -> extract_address) travels. - Budget: chars/4 estimator (no server-side LLM tokenizer), default 1600 tokens, greedy maximal prefix over the ranked lines; token_budget <= 0 is an input error. - Determinism: effective score DESC, qualified_name ASC (total order). - Score-absence gate (spec AC7): symbol nodes with zero importance coverage -> explicit 'unscored' error advising re-index; partial coverage proceeds (NULLs rank last). - Rendering: 'file: symbol(sig)' one line per symbol; param-only signatures get the name prefixed; whitespace/newlines flattened; no bodies. - Store layer: cbm_store_importance_coverage + cbm_store_top_symbols_by_ importance (json_extract ORDER BY, project-filtered). Verified on real corpus (worktree binary, scratch CBM_CACHE_DIR): budget fit on connectors + codebase-memory-mcp in both modes; zero P3-pinned noise/test-leakage names in top-40; P2 seeded ground truth surfaces 28 sender/gmail lines from rank 3; AC6 net latency 52ms global / 55ms seeded on connectors (bound 500ms). 28/28 AC tests green; suite 5651 PASS with only the pre-existing test_incremental RSS failure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KK6YFYGwhT69NjYevuEUCr Signed-off-by: Peter Cox <info@petercox.ie>
Signed-off-by: Peter Cox <info@petercox.ie>
Signed-off-by: Peter Cox <info@petercox.ie>
petercoxphoto
force-pushed
the
feat/repo-map
branch
from
c6473b8 to
5cc082b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Token-budgeted, seed-aware
repo_mapMCP toolSplit out of #776 (now closed) as an atomic feature PR, per @DeusData's request. Stacked on the importance-ranking PR (#879) —
repo_mapranks by the persistedimportancekey that PR adds, so please review/merge that first. The diff here shown againstmainincludes the importance commits; the repo_map-only delta is the last two commits.What it does
Adds a
repo_mapMCP tool: a token-budgeted repository map of the most important symbols (persisted importance score), rendered one per line asfile: signature— no bodies.seed_anchors(symbol names, qualified names, or file paths) to boost the seed neighbourhood (1-hop CALLS/USAGE, widened for weakly-connected seeds) to the top; omit / pass unresolvable seeds for the global importance-ranked map.token_budget(default 1600, chars/4 estimate; must be positive).unscorederror if the project needs re-indexing, rather than emitting a meaningless map.Adapted to current
mainUpstream added a
titlefield to the MCP tool struct ({name, title, description, schema}) since v0.8.1; the tool registration here was updated to{"repo_map", "Repo map", ...}to match.Tests
repo_map_*cases intests/test_mcp.c(26 cases): registration/dispatch, token-budget fit (default/tight/zero/oversized), seed resolution + boost, gate errors, cross-project isolation, determinism, and leak-surface stability.