From f6ac7b36e6b998b5304878b15e032796ab3a0d01 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Sat, 1 Aug 2026 01:50:44 -0500 Subject: [PATCH] fix(mcp): blast radius follows caller chains before claiming no test coverage (#1475) (#1494) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "no covering tests found" flag only inspected a symbol's direct callers, so helpers exercised transitively by tests (logDebug runs 1,471x under npm test) were reported untested — wrong for ~40% of flagged symbols per the issue's measurement. The check now BFSes up the caller graph (3 hops, 64-lookup budget per entry) and reports indirect coverage as "tested via callers: ". When nothing is found it claims only what was measured — "no tests found within 3 caller hops", or the weaker "no test calls this directly" if the budget ran out — and drops the warning glyph. Co-authored-by: Claude Fable 5 --- CHANGELOG.md | 1 + __tests__/explore-blast-radius.test.ts | 46 +++++++++++++++++++++++-- src/mcp/tools.ts | 47 +++++++++++++++++++++++++- 3 files changed, 91 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b144480..8dcd9b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - On Windows, the Claude Code prompt hook written by `codegraph install` failed with "command not found" when hooks run through Git Bash, which needs the `.cmd` extension to find the launcher. The installer now writes the platform-correct command, and re-running `codegraph install` (or `codegraph upgrade`) repairs an existing install in place. (#1466) - Python classes used as values — `return SomeSerializer` from a factory method, `handler = SomeClass` aliases, registry dicts and lists, and classes passed as arguments — now produce reference edges in the graph. Previously these idioms were invisible, so on Django and Django REST Framework projects, asking for a serializer's callers or the impact of editing it missed the views that actually use it. Re-index after upgrading to pick up the new edges. (#1478) - When a file changed on disk after its last index sync, `codegraph_node` and `codegraph_explore` could return a different symbol's code under the requested name — current file bytes cut at outdated line positions — while presenting it as verbatim, trustworthy source. This hit hardest on projects queried through `projectPath` (for example, sub-projects of a monorepo), which have no live file watcher to flag pending edits. Both tools now verify each file against the index before showing sliced code: an out-of-date file is either shown whole with its full current source, or its code is withheld with a clear "changed on disk" notice — never served as a wrong slice. A fresh re-index restores normal output automatically. Thanks @inth3shadows for the thorough report and verification passes. (#1474) +- The blast-radius section of `codegraph_explore` flagged "no covering tests found" whenever no test called a symbol directly — falsely branding helpers that tests exercise through their callers as untested (about 40% of flagged symbols in a measured sample). The check now follows caller chains up to 3 hops and reports indirect coverage as "tested via callers"; when nothing is found it states exactly what was checked instead of an unconditional warning. Thanks @inth3shadows for measuring the false-positive rate. (#1475) ## [1.5.0] - 2026-07-21 diff --git a/__tests__/explore-blast-radius.test.ts b/__tests__/explore-blast-radius.test.ts index e85b073..50ad362 100644 --- a/__tests__/explore-blast-radius.test.ts +++ b/__tests__/explore-blast-radius.test.ts @@ -40,6 +40,28 @@ describe('codegraph_explore — blast radius', () => { path.join(src, 'leaf.ts'), `export function lonelyLeaf() { return 42; }\n`, ); + // `deepHelper` is only called by production code (`midCaller`), but the + // test file exercises it transitively — 2 caller hops up (#1475). + fs.writeFileSync( + path.join(src, 'util.ts'), + `export function deepHelper() { return 1; }\n`, + ); + fs.writeFileSync( + path.join(src, 'mid.ts'), + `import { deepHelper } from './util';\n` + + `export function midCaller() { return deepHelper(); }\n`, + ); + fs.writeFileSync( + path.join(src, 'mid.test.ts'), + `import { midCaller } from './mid';\n` + + `export function checkMid() { return midCaller(); }\n`, + ); + // `untestedHelper` has a caller but no test anywhere up its caller chain. + fs.writeFileSync( + path.join(src, 'untested.ts'), + `export function untestedHelper() { return 3; }\n` + + `export function untestedCaller() { return untestedHelper(); }\n`, + ); cg = CodeGraph.initSync(testDir, { config: { include: ['**/*.ts'], exclude: [] } }); await cg.indexAll(); @@ -60,8 +82,28 @@ describe('codegraph_explore — blast radius', () => { expect(text).toMatch(/caller/); // a caller count is reported // It names WHERE (the caller file) — not the caller's source body. expect(text).toContain('feature.ts'); - // Test coverage is surfaced (either the covering test file, or the warning). - expect(text).toMatch(/tests:.*feature\.test\.ts|no covering tests/); + // The direct covering test file is surfaced. + expect(text).toMatch(/tests:.*feature\.test\.ts/); + }); + + it('surfaces tests that cover a symbol transitively through its callers (#1475)', async () => { + const res = await handler.execute('codegraph_explore', { query: 'deepHelper' }); + const text = res.content[0].text; + + // deepHelper's only direct caller is production code, but mid.test.ts sits + // one more hop up — that must NOT read as "no tests". + expect(text).toMatch(/`deepHelper`[^\n]*tested via callers:[^\n]*mid\.test\.ts/); + const line = text.split('\n').find((l: string) => l.startsWith('- `deepHelper`')); + expect(line).not.toMatch(/no tests found|no covering tests/); + }); + + it('states only what was measured when no test exists up the caller chain', async () => { + const res = await handler.execute('codegraph_explore', { query: 'untestedHelper' }); + const text = res.content[0].text; + + // Bounded claim, no warning glyph — the tool verified nothing beyond 3 hops. + expect(text).toMatch(/`untestedHelper`[^\n]*no tests found within 3 caller hops/); + expect(text).not.toContain('⚠️ no covering tests found'); }); it('omits symbols that have no dependents from the blast radius', async () => { diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index 3995ab3..08308ac 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -2473,7 +2473,7 @@ export class ToolHandler { const where = nonTest.length > 0 ? ` in ${shown}${more}` : ''; const tests = testFiles.length > 0 ? `; tests: ${testFiles.slice(0, FILE_CAP).map((f) => `\`${f}\``).join(', ')}${testFiles.length > FILE_CAP ? ` +${testFiles.length - FILE_CAP}` : ''}` - : '; ⚠️ no covering tests found'; + : this.indirectTestNote(cg, uniq, rel); entries.push( `- \`${root.name}\` (${rel(root.filePath)}:${root.startLine}) — ${uniq.length} caller${uniq.length === 1 ? '' : 's'}${where}${tests}`, @@ -2489,6 +2489,51 @@ export class ToolHandler { ].join('\n'); } + /** + * Test-coverage note for a blast-radius entry whose DIRECT callers include no + * test file. A helper called only by production code can still be exercised + * by tests further up the caller chain (#1475: 40% of directly-unflagged + * symbols had a test within 2-3 hops), so walk up to 2 more hops before + * claiming anything — and even then claim only what was measured. + */ + private indirectTestNote(cg: CodeGraph, directCallers: Node[], rel: (p: string) => string): string { + const MAX_HOPS = 3; // direct callers are hop 1 + const BUDGET = 64; // getCallers lookups per entry — bounds god-fan-in symbols + const FILE_CAP = 2; + let budget = BUDGET; + const visited = new Set(directCallers.map((n) => n.id)); + let frontier = directCallers; + for (let hop = 2; hop <= MAX_HOPS && frontier.length > 0 && budget > 0; hop++) { + const next: Node[] = []; + const found = new Set(); + for (const node of frontier) { + if (budget-- <= 0) break; + let callers: Array<{ node: Node }> = []; + try { callers = cg.getCallers(node.id) as Array<{ node: Node }>; } catch { continue; } + for (const c of callers) { + const n = c?.node; + if (!n || visited.has(n.id)) continue; + visited.add(n.id); + const f = rel(n.filePath); + if (isTestFile(f)) found.add(f); + else next.push(n); + } + } + if (found.size > 0) { + const files = [...found]; + const shown = files.slice(0, FILE_CAP).map((f) => `\`${f}\``).join(', '); + const more = files.length > FILE_CAP ? ` +${files.length - FILE_CAP}` : ''; + return `; tested via callers: ${shown}${more}`; + } + frontier = next; + } + // Budget exhaustion means hops 2-3 weren't fully searched — fall back to + // the weaker claim that IS established by the direct-caller check. + return budget > 0 + ? `; no tests found within ${MAX_HOPS} caller hops` + : '; no test calls this directly'; + } + /** * Graph-connectivity relevance via Random-Walk-with-Restart (personalized * PageRank) from the query's matched SEED nodes over the call/reference graph.