From 3ecf7479a140aa51c00a7a593c441fc01df2b0ad Mon Sep 17 00:00:00 2001 From: zhangtianshuo Date: Thu, 30 Jul 2026 16:35:30 +0800 Subject: [PATCH] fix: stop callers/callees/impact from silently substituting fuzzy hits (#1473) Exact-name only for CLI and MCP; missing names get not-found + did-you-mean instead of another symbol's results. Co-authored-by: Cursor --- CHANGELOG.md | 3 + __tests__/no-silent-fuzzy-symbol.test.ts | 217 +++++++++++++++++++++++ src/bin/codegraph.ts | 78 ++++---- src/mcp/tools.ts | 65 ++++--- 4 files changed, 297 insertions(+), 66 deletions(-) create mode 100644 __tests__/no-silent-fuzzy-symbol.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index d2ae567d0..35686a2a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Fixes + +- `codegraph callers` / `callees` / `impact` (CLI and MCP) no longer silently answer for a different symbol when the name you typed does not exist, or when an exact match has no callers — they report not found (with a did-you-mean hint) instead of labelling a fuzzy hit under your query. (#1473, #1455) ## [1.5.0] - 2026-07-21 diff --git a/__tests__/no-silent-fuzzy-symbol.test.ts b/__tests__/no-silent-fuzzy-symbol.test.ts new file mode 100644 index 000000000..80cc77592 --- /dev/null +++ b/__tests__/no-silent-fuzzy-symbol.test.ts @@ -0,0 +1,217 @@ +/** + * #1473 — callers/callees/impact must not silently answer for a different + * symbol when the requested name has no exact match (or has an exact match + * with zero callers). Fuzzy FTS hits may appear only as did-you-mean hints. + */ + +import { describe, it, expect, beforeAll, beforeEach, afterEach } from 'vitest'; +import { execFileSync } from 'child_process'; +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; +import { initGrammars, loadAllGrammars } from '../src/extraction/grammars'; + +const BIN = path.resolve(__dirname, '../dist/bin/codegraph.js'); + +beforeAll(async () => { + await initGrammars(); + await loadAllGrammars(); +}); + +function hasSqliteBindings(): boolean { + try { + const { DatabaseSync } = require('node:sqlite'); + const db = new DatabaseSync(':memory:'); + db.close(); + return true; + } catch { + return false; + } +} +const HAS_SQLITE = hasSqliteBindings(); + +function tmpRoot(prefix: string): string { + return fs.mkdtempSync(path.join(os.tmpdir(), prefix)); +} + +function rmTree(dir: string): void { + if (fs.existsSync(dir)) fs.rmSync(dir, { recursive: true, force: true }); +} + +function runCli(args: string[], cwd: string): { stdout: string; status: number } { + try { + const stdout = execFileSync(process.execPath, [BIN, ...args], { + cwd, + encoding: 'utf-8', + env: { + ...process.env, + CODEGRAPH_NO_DAEMON: '1', + CODEGRAPH_TELEMETRY: '0', + }, + stdio: ['ignore', 'pipe', 'pipe'], + }); + return { stdout, status: 0 }; + } catch (err: unknown) { + const e = err as { stdout?: string; status?: number }; + return { stdout: e.stdout ?? '', status: typeof e.status === 'number' ? e.status : 1 }; + } +} + +describe.skipIf(!HAS_SQLITE)('no silent fuzzy substitution (#1473) — MCP', () => { + let projectRoot: string; + let cg: any; + let handler: any; + + beforeEach(async () => { + projectRoot = tmpRoot('codegraph-1473-mcp-'); + const src = path.join(projectRoot, 'src', 'a', 'b', 'c'); + fs.mkdirSync(src, { recursive: true }); + fs.writeFileSync( + path.join(src, 'D.java'), + `package a.b.c;\n\npublic class D {\n public void e() { System.out.println("e"); }\n public void ef() { System.out.println("ef"); }\n}\n` + ); + fs.writeFileSync( + path.join(src, 'Caller.java'), + `package a.b.c;\n\npublic class Caller {\n public void callsEfOnly() { D d = new D(); d.ef(); }\n public void alsoCallsEf() { D d = new D(); d.ef(); }\n}\n` + ); + // Case-differing pair: exact Fetch has 0 callers; lowercase fetch has callers. + fs.writeFileSync( + path.join(projectRoot, 'src', 'Fetch.cs'), + `public class Torture {\n public void Fetch() {}\n}\n` + ); + fs.writeFileSync( + path.join(projectRoot, 'src', 'fetch.py'), + `def fetch():\n return 1\n\ndef load():\n return fetch()\n` + ); + + const CodeGraph = (await import('../src/index')).default; + const { ToolHandler } = await import('../src/mcp/tools'); + cg = CodeGraph.initSync(projectRoot); + await cg.indexAll(); + handler = new ToolHandler(cg); + }); + + afterEach(() => { + handler?.closeAll(); + cg?.destroy(); + rmTree(projectRoot); + }); + + async function text(tool: string, args: Record): Promise { + const res = await handler.execute(tool, args); + return res.content?.[0]?.text ?? ''; + } + + it('callers: missing name is not found (with did-you-mean), not a fuzzy hit labelled as the typed name', async () => { + const out = await text('codegraph_callers', { symbol: 'Calls' }); + expect(out).toMatch(/Symbol "Calls" not found/); + expect(out).toMatch(/Did you mean:/); + expect(out).not.toMatch(/Callees of Calls|Callers of Calls/); + expect(out).not.toMatch(/\bef\b/); + }); + + it('callees: missing name does not return another method\'s callees', async () => { + const out = await text('codegraph_callees', { symbol: 'Calls' }); + expect(out).toMatch(/Symbol "Calls" not found/); + expect(out).not.toContain('Callees of Calls'); + }); + + it('impact: missing prefix does not substitute a longer name', async () => { + const out = await text('codegraph_impact', { symbol: 'callsEf' }); + expect(out).toMatch(/Symbol "callsEf" not found/); + expect(out).toMatch(/Did you mean:.*callsEfOnly/); + // Suggestion only — must not claim impact results for the mistyped name. + expect(out).not.toMatch(/Impact:|"callsEf" affects|affected/); + }); + + it('callers: exact name with zero callers stays empty (no case-sibling substitution)', async () => { + const out = await text('codegraph_callers', { symbol: 'Fetch' }); + expect(out).toMatch(/No callers found for "Fetch"/); + expect(out).not.toContain('load'); + }); + + it('callers: real exact name still resolves', async () => { + const out = await text('codegraph_callers', { symbol: 'ef' }); + expect(out).toContain('Callers of ef'); + expect(out).toContain('callsEfOnly'); + expect(out).toContain('alsoCallsEf'); + }); + + it('findAllSymbols returns no nodes for a fuzzy-only hit', async () => { + const findAllSymbols = (handler as any).findAllSymbols.bind(handler); + const all = findAllSymbols(cg, 'Calls'); + expect(all.nodes).toEqual([]); + expect(all.note).toMatch(/Did you mean:/); + }); +}); + +describe.skipIf(!HAS_SQLITE || !fs.existsSync(BIN))('no silent fuzzy substitution (#1473) — CLI', () => { + let projectRoot: string; + + beforeEach(async () => { + projectRoot = tmpRoot('codegraph-1473-cli-'); + const src = path.join(projectRoot, 'src', 'a', 'b', 'c'); + fs.mkdirSync(src, { recursive: true }); + fs.writeFileSync( + path.join(src, 'D.java'), + `package a.b.c;\n\npublic class D {\n public void e() { System.out.println("e"); }\n public void ef() { System.out.println("ef"); }\n}\n` + ); + fs.writeFileSync( + path.join(src, 'Caller.java'), + `package a.b.c;\n\npublic class Caller {\n public void callsEfOnly() { D d = new D(); d.ef(); }\n public void alsoCallsEf() { D d = new D(); d.ef(); }\n}\n` + ); + fs.writeFileSync( + path.join(projectRoot, 'src', 'Fetch.cs'), + `public class Torture {\n public void Fetch() {}\n}\n` + ); + fs.writeFileSync( + path.join(projectRoot, 'src', 'fetch.py'), + `def fetch():\n return 1\n\ndef load():\n return fetch()\n` + ); + + const CodeGraph = (await import('../src/index')).default; + const cg = CodeGraph.initSync(projectRoot); + await cg.indexAll(); + cg.close(); + }); + + afterEach(() => { + rmTree(projectRoot); + }); + + it('callers: fuzzy-only name → not found with did-you-mean (JSON stays empty)', () => { + const { stdout } = runCli(['callers', 'Calls', '--json'], projectRoot); + // JSON path is only taken on a successful resolve; not-found prints info text. + expect(stdout).toMatch(/Symbol "Calls" not found/); + expect(stdout).toMatch(/did you mean/i); + expect(stdout).not.toMatch(/"name":\s*"ef"/); + }); + + it('callees: fuzzy-only name → not found', () => { + const { stdout } = runCli(['callees', 'Calls', '--json'], projectRoot); + expect(stdout).toMatch(/Symbol "Calls" not found/); + expect(stdout).not.toMatch(/"name":\s*"ef"/); + }); + + it('impact: prefix of a real name → not found', () => { + const { stdout } = runCli(['impact', 'callsEf', '--json'], projectRoot); + expect(stdout).toMatch(/Symbol "callsEf" not found/); + expect(stdout).toMatch(/did you mean:.*callsEfOnly/i); + // Not a successful JSON impact payload for the fuzzy hit. + expect(stdout).not.toMatch(/"affected"\s*:/); + expect(stdout).not.toMatch(/"symbol":\s*"callsEf"/); + }); + + it('callers: exact Fetch with zero callers → empty list, not fetch\'s callers', () => { + const { stdout } = runCli(['callers', 'Fetch', '--json'], projectRoot); + expect(stdout).toContain('"symbol": "Fetch"'); + expect(stdout).toMatch(/"callers":\s*\[\s*\]/); + expect(stdout).not.toContain('load'); + }); + + it('callers: exact ef still lists real callers', () => { + const { stdout } = runCli(['callers', 'ef', '--json'], projectRoot); + expect(stdout).toContain('callsEfOnly'); + expect(stdout).toContain('alsoCallsEf'); + }); +}); diff --git a/src/bin/codegraph.ts b/src/bin/codegraph.ts index eefb5d907..2ef3bb20d 100644 --- a/src/bin/codegraph.ts +++ b/src/bin/codegraph.ts @@ -356,6 +356,26 @@ function warn(message: string): void { console.log(chalk.yellow(getGlyphs().warn) + ' ' + message); } +/** + * Exact-name match for CLI callers/callees/impact (#1473). + * A single fuzzy FTS hit must NOT count as exact — that was the silent + * substitution path when `matches.length === 1`. + */ +function isCliExactSymbolMatch(nodeName: string, symbol: string): boolean { + return ( + nodeName === symbol || + nodeName.endsWith(`.${symbol}`) || + nodeName.endsWith(`::${symbol}`) + ); +} + +/** "not found" (+ optional did-you-mean) when no exact symbol matches. */ +function formatSymbolNotFound(symbol: string, fuzzyNames: string[]): string { + const suggestions = [...new Set(fuzzyNames.filter((n) => n !== symbol))].slice(0, 3); + if (suggestions.length === 0) return `Symbol "${symbol}" not found`; + return `Symbol "${symbol}" not found — did you mean: ${suggestions.join(', ')}?`; +} + type IndexResult = { success: boolean; filesIndexed: number; @@ -1850,8 +1870,10 @@ program const limit = parseInt(options.limit || '20', 10); const matches = cg.searchNodes(symbol, { limit: 50 }); - if (matches.length === 0) { - info(`Symbol "${symbol}" not found`); + const exact = matches.filter((m) => isCliExactSymbolMatch(m.node.name, symbol)); + if (exact.length === 0) { + // #1473: never answer for a fuzzy hit under the typed name + info(formatSymbolNotFound(symbol, matches.map((m) => m.node.name))); cg.destroy(); return; } @@ -1859,9 +1881,7 @@ program const seen = new Set(); const allCallers: Array<{ name: string; kind: string; filePath: string; startLine?: number }> = []; - for (const match of matches) { - const exactMatch = match.node.name === symbol || match.node.name.endsWith(`.${symbol}`) || match.node.name.endsWith(`::${symbol}`); - if (!exactMatch && matches.length > 1) continue; + for (const match of exact) { for (const c of cg.getCallers(match.node.id)) { if (!seen.has(c.node.id)) { seen.add(c.node.id); @@ -1870,16 +1890,6 @@ program } } - // Fallback: if exact filter removed everything, use the top match - if (allCallers.length === 0 && matches[0]) { - for (const c of cg.getCallers(matches[0].node.id)) { - if (!seen.has(c.node.id)) { - seen.add(c.node.id); - allCallers.push({ name: c.node.name, kind: c.node.kind, filePath: c.node.filePath, startLine: c.node.startLine }); - } - } - } - const limited = allCallers.slice(0, limit); if (options.json) { @@ -1929,8 +1939,10 @@ program const limit = parseInt(options.limit || '20', 10); const matches = cg.searchNodes(symbol, { limit: 50 }); - if (matches.length === 0) { - info(`Symbol "${symbol}" not found`); + const exact = matches.filter((m) => isCliExactSymbolMatch(m.node.name, symbol)); + if (exact.length === 0) { + // #1473: never answer for a fuzzy hit under the typed name + info(formatSymbolNotFound(symbol, matches.map((m) => m.node.name))); cg.destroy(); return; } @@ -1938,9 +1950,7 @@ program const seen = new Set(); const allCallees: Array<{ name: string; kind: string; filePath: string; startLine?: number }> = []; - for (const match of matches) { - const exactMatch = match.node.name === symbol || match.node.name.endsWith(`.${symbol}`) || match.node.name.endsWith(`::${symbol}`); - if (!exactMatch && matches.length > 1) continue; + for (const match of exact) { for (const c of cg.getCallees(match.node.id)) { if (!seen.has(c.node.id)) { seen.add(c.node.id); @@ -1949,15 +1959,6 @@ program } } - if (allCallees.length === 0 && matches[0]) { - for (const c of cg.getCallees(matches[0].node.id)) { - if (!seen.has(c.node.id)) { - seen.add(c.node.id); - allCallees.push({ name: c.node.name, kind: c.node.kind, filePath: c.node.filePath, startLine: c.node.startLine }); - } - } - } - const limited = allCallees.slice(0, limit); if (options.json) { @@ -2007,8 +2008,10 @@ program const depth = Math.min(Math.max(parseInt(options.depth || '2', 10), 1), 10); const matches = cg.searchNodes(symbol, { limit: 50 }); - if (matches.length === 0) { - info(`Symbol "${symbol}" not found`); + const exact = matches.filter((m) => isCliExactSymbolMatch(m.node.name, symbol)); + if (exact.length === 0) { + // #1473: never answer for a fuzzy hit under the typed name + info(formatSymbolNotFound(symbol, matches.map((m) => m.node.name))); cg.destroy(); return; } @@ -2018,9 +2021,7 @@ program const seenEdges = new Set(); let edgeCount = 0; - for (const match of matches) { - const exactMatch = match.node.name === symbol || match.node.name.endsWith(`.${symbol}`) || match.node.name.endsWith(`::${symbol}`); - if (!exactMatch && matches.length > 1) continue; + for (const match of exact) { const impact = cg.getImpactRadius(match.node.id, depth); for (const [id, n] of impact.nodes) { mergedNodes.set(id, { name: n.name, kind: n.kind, filePath: n.filePath, startLine: n.startLine }); @@ -2034,15 +2035,6 @@ program } } - // Fallback to top match if exact filter removed everything - if (mergedNodes.size === 0 && matches[0]) { - const impact = cg.getImpactRadius(matches[0].node.id, depth); - for (const [id, n] of impact.nodes) { - mergedNodes.set(id, { name: n.name, kind: n.kind, filePath: n.filePath, startLine: n.startLine }); - } - edgeCount = impact.edges.length; - } - if (options.json) { console.log(JSON.stringify({ symbol, diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index b31c64fc7..22612a9bd 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -1579,7 +1579,7 @@ export class ToolHandler { const allMatches = this.findAllSymbols(cg, symbol); if (allMatches.nodes.length === 0) { - return this.textResult(`Symbol "${symbol}" not found in the codebase`); + return this.textResult(`Symbol "${symbol}" not found in the codebase${allMatches.note}`); } const { groups, filteredOut } = this.groupDefinitions(allMatches.nodes, fileFilter); @@ -1652,7 +1652,7 @@ export class ToolHandler { const allMatches = this.findAllSymbols(cg, symbol); if (allMatches.nodes.length === 0) { - return this.textResult(`Symbol "${symbol}" not found in the codebase`); + return this.textResult(`Symbol "${symbol}" not found in the codebase${allMatches.note}`); } const { groups, filteredOut } = this.groupDefinitions(allMatches.nodes, fileFilter); @@ -1722,7 +1722,7 @@ export class ToolHandler { const allMatches = this.findAllSymbols(cg, symbol); if (allMatches.nodes.length === 0) { - return this.textResult(`Symbol "${symbol}" not found in the codebase`); + return this.textResult(`Symbol "${symbol}" not found in the codebase${allMatches.note}`); } const { groups, filteredOut } = this.groupDefinitions(allMatches.nodes, fileFilter); @@ -4494,6 +4494,10 @@ export class ToolHandler { /** * Find ALL symbols matching a name. Used by callers/callees/impact to aggregate * results across all matching symbols (e.g., multiple classes with an `execute` method). + * + * Exact matches only (#1473): a missing / mistyped name must NOT silently + * resolve to the top fuzzy FTS hit under the caller's typed label. Closest + * hits may appear in `note` as a did-you-mean hint when `nodes` is empty. */ private findAllSymbols(cg: CodeGraph, symbol: string): { nodes: Node[]; note: string } { // Nix option paths: the declaration is stored as `options.` and @@ -4516,41 +4520,56 @@ export class ToolHandler { return { nodes, note: '' }; } } - let results = cg.searchNodes(symbol, { limit: 50 }); - // Mirror the fallback in `findSymbol` for qualified queries — FTS - // strips colons, so a module-qualified lookup needs a second pass - // by the bare last part. - if (results.length === 0 && /[.\/]|::/.test(symbol)) { - const tail = lastQualifierPart(symbol); - if (tail && tail !== symbol) results = cg.searchNodes(tail, { limit: 50 }); - } + const isQualified = /[.\/]|::/.test(symbol); + let exactNodes: Node[]; - if (results.length === 0) { - return { nodes: [], note: '' }; + if (!isQualified) { + // Direct index — every exact-name overload, case-sensitive. Avoids FTS + // ranking a differently-cased sibling above the real node (#1473 Fetch). + exactNodes = cg.getNodesByName(symbol); + } else { + let results = cg.searchNodes(symbol, { limit: 50 }); + // Mirror findSymbolMatches — FTS strips colons, so re-search by bare tail. + if (results.length === 0) { + const tail = lastQualifierPart(symbol); + if (tail && tail !== symbol) results = cg.searchNodes(tail, { limit: 50 }); + } + exactNodes = results + .filter((r) => this.matchesSymbol(r.node, symbol)) + .map((r) => r.node); } - const exactMatches = results.filter(r => this.matchesSymbol(r.node, symbol)); + if (exactNodes.length === 0) { + const fuzzy = cg.searchNodes(symbol, { limit: 5 }); + const suggestions = [ + ...new Set(fuzzy.map((r) => r.node.name).filter((n) => n !== symbol)), + ].slice(0, 3); + const note = + suggestions.length > 0 + ? `\n\n> **Note:** no symbol named "${symbol}". Did you mean: ${suggestions.join(', ')}?` + : ''; + return { nodes: [], note }; + } - if (exactMatches.length <= 1) { - const node = exactMatches[0]?.node ?? results[0]!.node; - return { nodes: [node], note: '' }; + if (exactNodes.length === 1) { + return { nodes: exactNodes, note: '' }; } // Same generated-file down-rank as findSymbol — keeps callers/callees // /impact aggregation aligned (a query against "Send" returns the // hand-written implementations before the protobuf scaffold). - const ranked = [...exactMatches].sort((a, b) => { - const aGen = isGeneratedFile(a.node.filePath) ? 1 : 0; - const bGen = isGeneratedFile(b.node.filePath) ? 1 : 0; + const ranked = [...exactNodes].sort((a, b) => { + const aGen = isGeneratedFile(a.filePath) ? 1 : 0; + const bGen = isGeneratedFile(b.filePath) ? 1 : 0; return aGen - bGen; }); - const locations = ranked.map(r => - `${r.node.kind} at ${r.node.filePath}:${r.node.startLine}` + const locations = ranked.map( + (n) => `${n.kind} at ${n.filePath}:${n.startLine}` ); const note = `\n\n> **Note:** Aggregated results across ${ranked.length} symbols named "${symbol}": ${locations.join(', ')}`; - return { nodes: ranked.map(r => r.node), note }; + return { nodes: ranked, note }; } /**