diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b57b2000..754ae6b1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### 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) - A CodeGraph process that gets force-killed — by the stuck-process watchdog, a crash, or the OS — no longer leaves the database's write-ahead log behind to grow without bound. Previously each killed session stacked more data onto the same log file and nothing ever shrank it, which on machines where sessions were killed regularly could quietly eat tens of gigabytes of disk. The log is now capped, and any oversized leftover is reclaimed automatically the next time the project is opened. Thanks @tiendungdev for the exceptional Windows report that pinned this down. (#1431) - The background server's watchdog no longer kills a healthy server that is just waiting on a slow disk: like indexing already does, it now checks whether the database files are still making progress before concluding the process is stuck. Fewer spurious kills also means fewer leftover write-ahead logs. (#1431) - `codegraph status` now shows the write-ahead log's size next to the database size and warns when killed sessions have left it oversized, and every line in the background server's log now carries a timestamp so kills and restarts can be placed in time. (#1431) 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 c6e259374..1405018a3 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; @@ -1864,8 +1884,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; } @@ -1873,9 +1895,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); @@ -1884,16 +1904,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) { @@ -1943,8 +1953,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; } @@ -1952,9 +1964,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); @@ -1963,15 +1973,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) { @@ -2021,8 +2022,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; } @@ -2032,9 +2035,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 }); @@ -2048,15 +2049,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 08308aca9..e43340420 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -1651,7 +1651,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); @@ -1724,7 +1724,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); @@ -1794,7 +1794,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); @@ -4731,6 +4731,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 @@ -4753,41 +4757,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 }; } /**