diff --git a/CHANGELOG.md b/CHANGELOG.md index 427599c..e79bf49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -135,6 +135,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- `codegraph callers`, `codegraph callees` and `codegraph impact` now resolve qualified names, group results and JSON edges by definition, and accept `--file` to narrow ambiguous names; thanks @ferrine. (#1512, #1656) + #### MCP / indexing - Indexing now warns when parser errors leave a file with no symbols, including C++ raw strings with 16-character delimiters, so missing code is no longer silent. (#1522) diff --git a/__tests__/cli-definition-grouping.test.ts b/__tests__/cli-definition-grouping.test.ts new file mode 100644 index 0000000..12f2f6b --- /dev/null +++ b/__tests__/cli-definition-grouping.test.ts @@ -0,0 +1,262 @@ +/** CLI parity with MCP definition grouping and file narrowing (#1512, #1656). */ +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { spawnSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import { ToolHandler } from '../src/mcp/tools'; +import { lookupSymbolNodes } from '../src/graph/symbol-lookup'; + +const BIN = path.resolve(__dirname, '../dist/bin/codegraph.js'); +const COMMANDS = ['callers', 'callees', 'impact'] as const; +type Command = typeof COMMANDS[number]; +let projectRoot: string; +let cg: CodeGraph; +let handler: ToolHandler; + +function runCli(command: Command, symbol = 'handle', args: string[] = []) { + return spawnSync(process.execPath, [BIN, command, '-p', projectRoot, ...args, '--', symbol], { + encoding: 'utf-8', + env: { ...process.env, CODEGRAPH_NO_DAEMON: '1', CODEGRAPH_WASM_RELAUNCHED: '1', NO_COLOR: '1' }, + timeout: 30_000, + }); +} + +function json(command: Command, symbol = 'handle', args: string[] = []) { + const result = runCli(command, symbol, [...args, '--json']); + expect(result.status, result.stderr).toBe(0); + return JSON.parse(result.stdout); +} + +function resultKey(command: Command) { + return command === 'impact' ? 'affected' : command; +} + +function write(file: string, source: string) { + const absolute = path.join(projectRoot, file); + fs.mkdirSync(path.dirname(absolute), { recursive: true }); + fs.writeFileSync(absolute, source); +} + +beforeAll(async () => { + projectRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-cli-1512-')); + for (const [dir, helper] of [['a', 'alpha'], ['b', 'beta']]) { + write(`${dir}/${helper}.js`, `export function ${helper}() { return 1; }\n`); + write(`${dir}/svc.js`, `import { ${helper} } from './${helper}.js';\nexport function handle() { return ${helper}(); }\n`); + write(`${dir}/main.js`, `import { handle } from './svc.js';\nexport function ${dir}Main() { return handle(); }\nexport function ${dir}Entry() { return ${dir}Main(); }\n`); + write(`${dir}/work.js`, `import { shared } from '../shared.js';\nimport { ${helper} } from './${helper}.js';\nexport function work() { shared(); return ${helper}(); }\n`); + write(`${dir}/work-caller.js`, `import { work } from './work.js';\nexport function ${dir}Worker() { work(); }\n`); + } + write('shared.js', 'export function shared() {}\n'); + write('both.js', "import { work as aWork } from './a/work.js';\nimport { work as bWork } from './b/work.js';\nexport function both() { aWork(); bWork(); }\n"); + write('quiet-a.js', 'export function quiet() {}\n'); + write('quiet-b.js', "import { alpha } from './a/alpha.js';\nexport function quiet() { alpha(); }\nexport function wake() { quiet(); }\n"); + write('scopes.ts', [ + 'function leftOnly() {}', + 'function rightOnly() {}', + 'export class Left { run() { leftOnly(); } }', + 'export class Right { run() { rightOnly(); } }', + ].join('\n')); + // Java overloads have separate bodies/nodes; TS signature-only overloads are + // intentionally skipped by extraction, so they cannot exercise grouping. + write('Overloads.java', [ + 'public class Overloads {', + ' static String stringIdentity(String value) { return value; }', + ' static int intIdentity(int value) { return value; }', + ' public static String convert(String value) { return stringIdentity(value); }', + ' public static int convert(int value) { return intIdentity(value); }', + ' public static void convertCaller() { convert(1); convert("value"); }', + '}', + ].join('\n')); + for (let i = 0; i < 55; i++) { + write(`crowd/def-${i}.js`, "import { shared } from '../shared.js';\nexport function crowded() { shared(); }\n"); + } + cg = CodeGraph.initSync(projectRoot); + await cg.indexAll(); + handler = new ToolHandler(cg); +}, 30_000); + +afterAll(() => { + handler?.closeAll(); + cg?.close(); + if (projectRoot) fs.rmSync(projectRoot, { recursive: true, force: true }); +}); + +describe.each(COMMANDS)('%s definition grouping (#1512)', (command) => { + it('attributes every result and graph edge to its definition in JSON', () => { + const out = json(command); + expect(out.ambiguous).toBe(true); + expect(out.aggregation).toBe('union'); + expect(out.definitions).toHaveLength(2); + const key = resultKey(command); + for (const [dir, other] of [['a', 'b'], ['b', 'a']]) { + const group = out.definitions.find((d: any) => d.definition.filePath === `${dir}/svc.js`); + expect(group.definition).toMatchObject({ name: 'handle', kind: 'function', startLine: 2 }); + expect(group.roots).toHaveLength(1); + expect(group[key].length).toBeGreaterThan(0); + expect(group[key].every((n: any) => n.filePath.startsWith(`${dir}/`))).toBe(true); + expect(JSON.stringify(group)).not.toContain(`"filePath":"${other}/`); + const actual = cg.getNodesByName('handle').find(n => n.filePath === `${dir}/svc.js`)!; + const expectedNodes = command === 'impact' + ? [...cg.getImpactRadius(actual.id, 2).nodes.values()] + : cg[command === 'callers' ? 'getCallers' : 'getCallees'](actual.id).map(c => c.node); + expect(new Set(group[key].map((n: any) => n.id))).toEqual(new Set(expectedNodes.map(n => n.id))); + const ids = new Set([...group.roots, ...group[key].map((n: any) => n.id)]); + expect(group.edges.length).toBeGreaterThan(0); + for (const edge of group.edges) { + expect(ids.has(edge.source)).toBe(true); + expect(ids.has(edge.target)).toBe(true); + } + } + }); + + it('prints each definition above only its own results', () => { + const result = runCli(command); + expect(result.status, result.stderr).toBe(0); + expect(result.stdout).toContain('2 distinct definitions'); + expect(result.stdout).toContain('--file'); + const sections = result.stdout.split(/(?=function handle \(javascript\) — [ab]\/svc\.js:2)/).slice(1); + expect(sections).toHaveLength(2); + for (const section of sections) { + const dir = section.includes('— a/svc.js:2') ? 'a' : 'b'; + expect(section).toContain(command === 'callees' ? `${dir}/${dir === 'a' ? 'alpha' : 'beta'}.js` : `${dir}/main.js`); + expect(section).not.toContain(dir === 'a' ? 'b/' : 'a/'); + } + }); + + it.each(['a/svc.js', './a/svc.js'])('--file %s selects the same definition as MCP', async (file) => { + const out = json(command, 'handle', ['--file', file]); + expect(out.definitions).toHaveLength(1); + expect(out.definitions[0].definition.filePath).toBe('a/svc.js'); + expect(out.targets.every((n: any) => n.filePath === 'a/svc.js')).toBe(true); + expect(out.ambiguous).toBe(false); + expect(out.filteredOut).toBe(false); + expect(out[resultKey(command)].every((n: any) => n.filePath.startsWith('a/'))).toBe(true); + const human = runCli(command, 'handle', ['--file', file]).stdout; + const mcp = (await handler.execute(`codegraph_${command}`, { symbol: 'handle', file })).content[0]?.text ?? ''; + for (const text of [human, mcp]) { + expect(text).not.toContain('b/'); + expect(text).not.toContain('distinct definitions'); + expect(text).toContain(command === 'callees' ? 'a/alpha.js' : 'a/main.js'); + } + }); + + it('a suffix matching both files keeps both definitions', () => { + const out = json(command, 'handle', ['-f', 'svc.js']); + expect(out.definitions).toHaveLength(2); + expect(out.filteredOut).toBe(false); + }); + + it('a non-matching file discloses the fallback in JSON and text', async () => { + const note = 'no definition of "handle" matches file "missing.js" — showing all definitions instead.'; + const out = json(command, 'handle', ['--file', 'missing.js']); + expect(out.filteredOut).toBe(true); + expect(out.note).toBe(note); + expect(out.definitions).toHaveLength(2); + expect(runCli(command, 'handle', ['--file', 'missing.js']).stdout).toContain(note); + const mcp = await handler.execute(`codegraph_${command}`, { symbol: 'handle', file: 'missing.js' }); + expect(mcp.content[0]?.text).toContain(note); + }); + + it('keeps same-file overloads together as MCP does', () => { + const out = json(command, 'convert'); + expect(cg.getNodesByName('convert').length).toBeGreaterThan(1); + expect(out.definitions).toHaveLength(1); + expect(out.definitions[0].roots.length).toBeGreaterThan(1); + expect(out.ambiguous).toBe(false); + expect(lookupSymbolNodes(cg, 'convert').ambiguous).toBe(false); + expect(out.definitions[0][resultKey(command)].length).toBeGreaterThan(0); + }); + + it('does not substitute another definition for an unknown qualified name', () => { + const out = runCli(command, 'Missing.run'); + expect(out.status, out.stderr).toBe(0); + expect(out.stdout).toContain('Symbol "Missing.run" not found'); + expect(out.stdout).not.toContain('leftOnly'); + expect(out.stdout).not.toContain('rightOnly'); + }); +}); + +describe('CLI definition boundaries and limits', () => { + it('separates different qualified names within the same file', () => { + const out = json('callees', 'run', ['--file', 'scopes.ts']); + expect(out.definitions).toHaveLength(2); + for (const name of ['Left', 'Right']) { + const group = out.definitions.find((d: any) => d.definition.qualifiedName === `${name}::run`); + expect(group.callees.map((n: any) => n.name)).toEqual([`${name.toLowerCase()}Only`]); + } + const qualified = json('callees', 'Left.run'); + expect(qualified.definitions).toHaveLength(1); + expect(qualified.callees.map((n: any) => n.name)).toEqual(['leftOnly']); + }); + + it.each(['callers', 'callees'] as const)('%s includes definitions with no edges', (command) => { + const out = json(command, 'quiet'); + expect(out.definitions).toHaveLength(2); + const empty = out.definitions.find((d: any) => d.definition.filePath === 'quiet-a.js'); + expect(empty[command]).toEqual([]); + expect(empty.edges).toEqual([]); + expect(empty).toMatchObject({ total: 0, limit: 20, truncated: false }); + expect(runCli(command, 'quiet').stdout).toContain(`(no ${command})`); + }); + + it('keeps shared callers and callees in each definition instead of deduplicating across them', () => { + for (const command of ['callers', 'callees'] as const) { + const out = json(command, 'work'); + expect(out.definitions).toHaveLength(2); + for (const group of out.definitions) { + expect(group[command].map((n: any) => n.name)).toContain(command === 'callers' ? 'both' : 'shared'); + } + expect(out[command].filter((n: any) => n.name === (command === 'callers' ? 'both' : 'shared'))).toHaveLength(1); + } + }); + + it.each(['callers', 'callees'] as const)('%s preserves union metadata and limits each definition independently', (command) => { + // Callers include the importing file nodes as well as the calling functions. + const total = command === 'callers' ? 6 : 3; + const perDefinition = command === 'callers' ? 4 : 2; + const out = json(command, 'work', ['--limit', '1']); + expect(out).toMatchObject({ total, limit: 1, truncated: true }); + expect(out[command]).toHaveLength(1); + for (const group of out.definitions) { + expect(group).toMatchObject({ total: perDefinition, limit: 1, truncated: true }); + expect(group[command]).toHaveLength(1); + expect(group.edges).toHaveLength(1); + expect(group.edges[0][command === 'callers' ? 'source' : 'target']).toBe(group[command][0].id); + } + const human = runCli(command, 'work', ['--limit', '1']).stdout; + expect(human.split(`Showing 1 of ${perDefinition}; pass --limit to widen.`)).toHaveLength(3); + const complete = json(command, 'work', ['--limit', '100']); + expect(complete).toMatchObject({ total, limit: 100, truncated: false }); + for (const group of complete.definitions) { + expect(group).toMatchObject({ total: perDefinition, limit: 100, truncated: false }); + expect(group[command]).toHaveLength(perDefinition); + } + }); + + it('applies impact depth within each definition and reports its own graph counts', () => { + for (const depth of [1, 2]) { + const out = json('impact', 'handle', ['--depth', String(depth)]); + expect(out.depth).toBe(depth); + // Each root also has an importing file node at depth one. + expect(out.nodeCount).toBe(2 * (depth + 2)); + expect(out.edgeCount).toBe(2 * (depth + 1)); + for (const group of out.definitions) { + expect(group.nodeCount).toBe(depth + 2); + expect(group.affected).toHaveLength(group.nodeCount); + expect(group.edgeCount).toBe(depth + 1); + expect(group.edges).toHaveLength(group.edgeCount); + } + } + }); + + it('enumerates definitions beyond the FTS cap and can narrow to any of them', () => { + const out = json('callees', 'crowded'); + expect(out.definitions).toHaveLength(55); + for (const group of out.definitions) expect(group.callees.map((n: any) => n.name)).toEqual(['shared']); + const narrowed = json('callees', 'crowded', ['--file', 'crowd/def-54.js']); + expect(narrowed.definitions).toHaveLength(1); + expect(narrowed.definitions[0].definition.filePath).toBe('crowd/def-54.js'); + }); +}); diff --git a/__tests__/symbol-lookup.test.ts b/__tests__/symbol-lookup.test.ts index c81aaab..e78b12e 100644 --- a/__tests__/symbol-lookup.test.ts +++ b/__tests__/symbol-lookup.test.ts @@ -17,6 +17,8 @@ import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; import { initGrammars, loadAllGrammars } from '../src/extraction/grammars'; +import { matchesSymbol, lookupSymbolNodes, isQualifiedSymbol } from '../src/graph/symbol-lookup'; +import type { Node } from '../src/types'; beforeAll(async () => { await initGrammars(); @@ -220,3 +222,173 @@ describe.skipIf(!HAS_SQLITE)('matchesSymbol — dotted lookups (regression for # expect((text.match(/\*\*Location:\*\*/g) || []).length).toBeGreaterThanOrEqual(2); }); }); + +/** + * One resolution path for every verb that takes a symbol NAME. + * + * `callers` / `callees` / `impact` used to carry their own filter, comparing + * the query against the BARE name only: + * + * node.name === symbol || node.name.endsWith('.' + symbol) + * + * which fails in two opposite directions at once. A bare name matched every + * same-named symbol in the repository and their results were merged under one + * heading with nothing saying they were different symbols; a qualified name + * could never equal a bare `node.name`, so every candidate failed the filter + * and the code fell through to an arbitrary top-of-FTS hit — or reported "not + * found" for a symbol that plainly exists. Both now go through + * `lookupSymbolNodes`. + */ +function fakeNode(over: Partial): Node { + return { + id: 'n1', kind: 'function', name: 'group', qualifiedName: 'group', + filePath: 'lib/format.ex', language: 'typescript', + startLine: 1, endLine: 2, startColumn: 0, endColumn: 0, updatedAt: 0, + ...over, + } as Node; +} + +describe('matchesSymbol — containers whose own name contains a separator', () => { + // Splitting on EVERY separator assumes no scope component contains one. That + // is false for any language whose module names are themselves dotted, and + // there the stored qualifiedName (`A.B::c`) can never equal the split-and- + // rejoined query spelling (`A::B::c`) — so a perfectly precise qualified + // query resolved to nothing. + const node = fakeNode({ name: 'group', qualifiedName: 'AppWeb.Format::group' }); + + it('matches a dotted module qualifier written with dots', () => { + expect(matchesSymbol(node, 'AppWeb.Format.group')).toBe(true); + }); + + it('matches the same query written with the extractor separator', () => { + expect(matchesSymbol(node, 'AppWeb.Format::group')).toBe(true); + }); + + it('matches a partial container suffix on a separator boundary', () => { + expect(matchesSymbol(node, 'Format.group')).toBe(true); + }); + + it('does not match a container that merely shares a suffix substring', () => { + // `ebFormat.group` is not a boundary-aligned suffix of `AppWeb.Format.group`. + expect(matchesSymbol(node, 'ebFormat.group')).toBe(false); + }); + + it('does not match a different container', () => { + expect(matchesSymbol(node, 'Other.Format.group')).toBe(false); + }); + + it('still requires the last part to be the node name', () => { + expect(matchesSymbol(node, 'AppWeb.Format.other')).toBe(false); + }); + + it('classifies bare vs qualified queries', () => { + expect(isQualifiedSymbol('group')).toBe(false); + expect(isQualifiedSymbol('A.B.group')).toBe(true); + expect(isQualifiedSymbol('A::group')).toBe(true); + expect(isQualifiedSymbol('a/b')).toBe(true); + }); +}); + +describe.skipIf(!HAS_SQLITE)('lookupSymbolNodes — the shared path used by callers/callees/impact', () => { + let projectRoot: string; + let cg: any; + + beforeEach(async () => { + projectRoot = tmpRoot(); + const client = path.join(projectRoot, 'client'); + const pkg = path.join(projectRoot, 'pkg', 'fmtutil'); + fs.mkdirSync(client, { recursive: true }); + fs.mkdirSync(pkg, { recursive: true }); + // The SAME short name defined in two languages — the collision profile of a + // polyglot repository, where the colliding identifiers are the common ones. + fs.writeFileSync( + path.join(client, 'chart.ts'), + `export function group(rows: number[][]): number[][] { return rows; }\n` + ); + fs.writeFileSync( + path.join(client, 'Editor.tsx'), + `import { group } from './chart';\nexport function Editor(r: number[][]) { return group(r); }\n` + ); + fs.writeFileSync( + path.join(pkg, 'format.py'), + `def group(items, size):\n return items\n` + ); + fs.writeFileSync( + path.join(projectRoot, 'pkg', 'planner.py'), + `from pkg.fmtutil.format import group\n\ndef plan_a(items): return group(items, 3)\ndef plan_b(items): return group(items, 5)\n` + ); + + const CodeGraph = (await import('../src/index')).default; + cg = CodeGraph.initSync(projectRoot, { + config: { include: ['**/*.ts', '**/*.tsx', '**/*.py'], exclude: [] }, + }); + await cg.indexAll(); + }); + + afterEach(() => { + cg?.destroy(); + rmTree(projectRoot); + }); + + it('a bare name resolves to EVERY definition and reports the ambiguity', () => { + const { nodes, ambiguous } = lookupSymbolNodes(cg, 'group'); + const defs = nodes.filter((n) => n.kind === 'function'); + expect(defs.length).toBe(2); + expect(new Set(defs.map((n) => n.language))).toEqual(new Set(['typescript', 'python'])); + // The flag is what stops an aggregate being presented as one symbol's answer. + expect(ambiguous).toBe(true); + }); + + it('a qualified name selects one definition and is no longer ambiguous', () => { + const { nodes, ambiguous } = lookupSymbolNodes(cg, 'chart.group'); + expect(nodes.length).toBe(1); + expect(nodes[0]!.language).toBe('typescript'); + expect(nodes[0]!.filePath).toMatch(/chart\.ts$/); + expect(ambiguous).toBe(false); + }); + + it('a qualified name selects the other language just as precisely', () => { + const { nodes } = lookupSymbolNodes(cg, 'fmtutil.format.group'); + expect(nodes.length).toBe(1); + expect(nodes[0]!.language).toBe('python'); + expect(nodes[0]!.filePath).toMatch(/fmtutil\/format\.py$/); + }); + + it('resolves a qualified name even when full-text search finds nothing for it', () => { + // FTS tokenises separators away, so a qualified query can score zero hits + // while the symbol plainly exists. Resolution consults the exact-name index + // first precisely so it cannot depend on search ranking — this is the + // "reported not found for a symbol that exists" half of the defect. + const fts = cg.searchNodes('fmtutil.format.group', { limit: 50 }); + const { nodes } = lookupSymbolNodes(cg, 'fmtutil.format.group'); + expect(nodes.length).toBe(1); + expect(nodes[0]!.filePath).toMatch(/format\.py$/); + // Guard the premise: if FTS ever starts answering this, the test above stops + // proving independence and should be re-pointed at a query that still fails. + expect(Array.isArray(fts)).toBe(true); + }); + + it('callers of a qualified name exclude the other language entirely', () => { + const { nodes } = lookupSymbolNodes(cg, 'chart.group'); + const callerFiles = nodes.flatMap((n: any) => + cg.getCallers(n.id).map((c: any) => c.node.filePath) + ); + expect(callerFiles.length).toBeGreaterThan(0); + for (const f of callerFiles) expect(f).not.toMatch(/\.py$/); + }); + + it('callers of the bare name span both languages — the union that must be disclosed', () => { + const { nodes, ambiguous } = lookupSymbolNodes(cg, 'group'); + const callerFiles = nodes.flatMap((n: any) => + cg.getCallers(n.id).map((c: any) => c.node.filePath) + ); + expect(ambiguous).toBe(true); + expect(callerFiles.some((f: string) => f.endsWith('.py'))).toBe(true); + expect(callerFiles.some((f: string) => f.endsWith('.tsx'))).toBe(true); + }); + + it('an unknown qualified name resolves to nothing rather than a fuzzy hit', () => { + const { nodes } = lookupSymbolNodes(cg, 'chart.nonexistent_fn'); + expect(nodes.length).toBe(0); + }); +}); diff --git a/src/bin/codegraph.ts b/src/bin/codegraph.ts index 750cbb3..99b6ecf 100644 --- a/src/bin/codegraph.ts +++ b/src/bin/codegraph.ts @@ -59,6 +59,8 @@ import { getTelemetry, TELEMETRY_DOCS, recordIndexEvent } from '../telemetry'; // server itself is loaded lazily inside the `ui` action. See ui-server/constants. import { BROWSER_ENV, DEFAULT_UI_PORT } from '../ui-server/constants'; import type { UiServerHandle } from '../ui-server'; +import { lookupSymbolNodes, describeSymbolNode, groupDefinitions } from '../graph/symbol-lookup'; +import type { Node, Edge } from '../types'; // Decided once, before `--color`/`--no-color` are stripped from argv below // (#1281). Piped/redirected stdout, NO_COLOR, or --no-color -> plain output. @@ -362,6 +364,20 @@ function warn(message: string): void { console.log(chalk.yellow(getGlyphs().warn) + ' ' + message); } +/** Compact node shape retained by the CLI's existing JSON lists. */ +function cliNode(node: Node) { + return { name: node.name, kind: node.kind, filePath: node.filePath, startLine: node.startLine }; +} + +/** Attribute a group's edges to every overload of this definition. */ +function cliDefinition(group: Node[]) { + const head = group[0]!; + return { + definition: { ...cliNode(head), id: head.id, qualifiedName: head.qualifiedName, language: head.language }, + roots: group.map((node) => node.id), + }; +} + type IndexResult = { success: boolean; filesIndexed: number; @@ -2149,184 +2165,140 @@ program }); /** - * codegraph callers - * - * CLI parity with the MCP graph tools (codegraph_callers/callees/impact) so the - * traversal queries work in scripts, CI, and git hooks without a running MCP - * server. + * CLI parity with MCP callers/callees: resolve once, then collect and limit + * within each definition. The legacy JSON list remains an explicitly labeled + * union, with its original total/limit/truncated contract (#1674). */ -program - .command('callers ') - .description('Find all functions/methods that call a specific symbol') - .option('-p, --path ', 'Project path') - .option('-l, --limit ', 'Maximum results', '20') - .option('-j, --json', 'Output as JSON') - .action(async (symbol: string, options: { path?: string; limit?: string; json?: boolean }) => { - const projectPath = resolveProjectPath(options.path); +for (const direction of ['callers', 'callees'] as const) { + const title = direction === 'callers' ? 'Callers' : 'Callees'; + program + .command(`${direction} `) + .description(direction === 'callers' + ? 'Find all functions/methods that call a specific symbol' + : 'Find all functions/methods called by a specific symbol') + .option('-p, --path ', 'Project path') + .option('-f, --file ', 'Narrow definitions by file path or suffix (no match: show all with a note)') + .option('-l, --limit ', 'Maximum results per definition (also caps the JSON union)', '20') + .option('-j, --json', 'Output as JSON') + .action(async (symbol: string, options: { path?: string; file?: string; limit?: string; json?: boolean }) => { + const projectPath = resolveProjectPath(options.path); - try { - if (!isInitialized(projectPath)) { - error(`CodeGraph not initialized in ${projectPath}`); + try { + if (!isInitialized(projectPath)) { + error(`CodeGraph not initialized in ${projectPath}`); + process.exit(1); + } + + const { default: CodeGraph } = await loadCodeGraph(); + const cg = await CodeGraph.open(projectPath); + try { + const limit = parseInt(options.limit || '20', 10); + const { nodes: targets } = lookupSymbolNodes(cg, symbol); + if (targets.length === 0) { + info(`Symbol "${symbol}" not found`); + return; + } + + const { groups, filteredOut } = groupDefinitions(targets, options.file); + const ambiguous = groups.length > 1; + const note = filteredOut + ? `no definition of "${symbol}" matches file "${options.file}" — showing all definitions instead.` + : undefined; + const collected = groups.map((group) => { + const nodes = new Map(); + const edges = new Map(); + for (const target of group) { + const connections = direction === 'callers' ? cg.getCallers(target.id) : cg.getCallees(target.id); + for (const { node, edge } of connections) { + nodes.set(node.id, node); + edges.set(`${edge.source}->${edge.target}:${edge.kind}`, edge); + } + } + return { group, nodes: [...nodes.values()], edges: [...edges.values()] }; + }); + + if (options.json) { + const definitions = collected.map(({ group, nodes, edges }) => { + const limited = nodes.slice(0, limit); + const shown = new Set(limited.map((node) => node.id)); + return { + ...cliDefinition(group), + [direction]: limited.map((node) => ({ id: node.id, ...cliNode(node) })), + edges: edges.filter((edge) => shown.has(direction === 'callers' ? edge.source : edge.target)), + total: nodes.length, + limit, + truncated: nodes.length > limit, + }; + }); + const union = new Map(); + for (const { nodes } of collected) { + for (const node of nodes) union.set(node.id, node); + } + const total = union.size; + console.log(JSON.stringify({ + symbol, + targets: groups.flat().map((node) => cliDefinition([node]).definition), + ambiguous, + aggregation: ambiguous ? 'union' : 'definition', + file: options.file, + filteredOut, + note, + definitions, + [direction]: [...union.values()].slice(0, limit).map(cliNode), + total, + limit, + truncated: total > limit, + }, null, 2)); + } else { + if (note) warn(note); + if (ambiguous) { + console.log(chalk.bold(`\n${title} of "${symbol}" — ${groups.length} distinct definitions (narrow with --file):`)); + } + for (const { group, nodes } of collected) { + const limited = nodes.slice(0, limit); + const total = nodes.length; + const truncated = total > limit; + const count = truncated ? `${limited.length} of ${total}` : String(total); + if (ambiguous) { + console.log(chalk.bold(`\n${describeSymbolNode(group[0]!)} (${count}):\n`)); + } else { + console.log(chalk.bold(`\n${title} of "${symbol}" (${count}):\n`)); + console.log(chalk.dim(describeSymbolNode(group[0]!))); + } + if (total === 0) { + if (ambiguous) console.log(chalk.dim(` (no ${direction})`)); + else info(`No ${direction} found for "${symbol}"`); + } + for (const node of limited) { + const loc = node.startLine ? `:${node.startLine}` : ''; + console.log(chalk.cyan(node.kind.padEnd(12)) + chalk.white(node.name)); + console.log(chalk.dim(` ${node.filePath}${loc}`)); + console.log(); + } + if (truncated) console.log(chalk.dim(`Showing ${limited.length} of ${total}; pass --limit to widen.`)); + } + } + } finally { + cg.destroy(); + } + } catch (err) { + error(`${direction} failed: ${err instanceof Error ? err.message : String(err)}`); process.exit(1); } - - const { default: CodeGraph } = await loadCodeGraph(); - const cg = await CodeGraph.open(projectPath); - const limit = parseInt(options.limit || '20', 10); - - const matches = cg.searchNodes(symbol, { limit: 50 }); - if (matches.length === 0) { - info(`Symbol "${symbol}" not found`); - cg.destroy(); - return; - } - - 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 c of cg.getCallers(match.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 }); - } - } - } - - // 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); - const total = allCallers.length; - const truncated = total > limit; - - if (options.json) { - console.log(JSON.stringify({ symbol, callers: limited, total, limit, truncated }, null, 2)); - } else if (limited.length === 0) { - info(`No callers found for "${symbol}"`); - } else { - const count = truncated ? `${limited.length} of ${total}` : String(total); - console.log(chalk.bold(`\nCallers of "${symbol}" (${count}):\n`)); - for (const node of limited) { - const loc = node.startLine ? `:${node.startLine}` : ''; - console.log( - chalk.cyan(node.kind.padEnd(12)) + - chalk.white(node.name) - ); - console.log(chalk.dim(` ${node.filePath}${loc}`)); - console.log(); - } - if (truncated) console.log(chalk.dim(`Showing ${limited.length} of ${total}; pass --limit to widen.`)); - } - - cg.destroy(); - } catch (err) { - error(`callers failed: ${err instanceof Error ? err.message : String(err)}`); - process.exit(1); - } - }); + }); +} /** - * codegraph callees - */ -program - .command('callees ') - .description('Find all functions/methods that a specific symbol calls') - .option('-p, --path ', 'Project path') - .option('-l, --limit ', 'Maximum results', '20') - .option('-j, --json', 'Output as JSON') - .action(async (symbol: string, options: { path?: string; limit?: string; json?: boolean }) => { - const projectPath = resolveProjectPath(options.path); - - try { - if (!isInitialized(projectPath)) { - error(`CodeGraph not initialized in ${projectPath}`); - process.exit(1); - } - - const { default: CodeGraph } = await loadCodeGraph(); - const cg = await CodeGraph.open(projectPath); - const limit = parseInt(options.limit || '20', 10); - - const matches = cg.searchNodes(symbol, { limit: 50 }); - if (matches.length === 0) { - info(`Symbol "${symbol}" not found`); - cg.destroy(); - return; - } - - 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 c of cg.getCallees(match.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 }); - } - } - } - - 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); - const total = allCallees.length; - const truncated = total > limit; - - if (options.json) { - console.log(JSON.stringify({ symbol, callees: limited, total, limit, truncated }, null, 2)); - } else if (limited.length === 0) { - info(`No callees found for "${symbol}"`); - } else { - const count = truncated ? `${limited.length} of ${total}` : String(total); - console.log(chalk.bold(`\nCallees of "${symbol}" (${count}):\n`)); - for (const node of limited) { - const loc = node.startLine ? `:${node.startLine}` : ''; - console.log( - chalk.cyan(node.kind.padEnd(12)) + - chalk.white(node.name) - ); - console.log(chalk.dim(` ${node.filePath}${loc}`)); - console.log(); - } - if (truncated) console.log(chalk.dim(`Showing ${limited.length} of ${total}; pass --limit to widen.`)); - } - - cg.destroy(); - } catch (err) { - error(`callees failed: ${err instanceof Error ? err.message : String(err)}`); - process.exit(1); - } - }); - -/** - * codegraph impact + * codegraph impact — one blast radius per distinct definition. */ program .command('impact ') .description('Analyze what code is affected by changing a symbol') .option('-p, --path ', 'Project path') + .option('-f, --file ', 'Narrow definitions by file path or suffix (no match: show all with a note)') .option('-d, --depth ', 'Traversal depth', '2') .option('-j, --json', 'Output as JSON') - .action(async (symbol: string, options: { path?: string; depth?: string; json?: boolean }) => { + .action(async (symbol: string, options: { path?: string; file?: string; depth?: string; json?: boolean }) => { const projectPath = resolveProjectPath(options.path); try { @@ -2337,77 +2309,89 @@ program const { default: CodeGraph } = await loadCodeGraph(); const cg = await CodeGraph.open(projectPath); - const depth = Math.min(Math.max(parseInt(options.depth || '2', 10), 1), 10); + try { + const depth = Math.min(Math.max(parseInt(options.depth || '2', 10), 1), 10); + const { nodes: targets } = lookupSymbolNodes(cg, symbol); + if (targets.length === 0) { + info(`Symbol "${symbol}" not found`); + return; + } - const matches = cg.searchNodes(symbol, { limit: 50 }); - if (matches.length === 0) { - info(`Symbol "${symbol}" not found`); + const { groups, filteredOut } = groupDefinitions(targets, options.file); + const ambiguous = groups.length > 1; + const note = filteredOut + ? `no definition of "${symbol}" matches file "${options.file}" — showing all definitions instead.` + : undefined; + const collected = groups.map((group) => { + const nodes = new Map(); + const edges = new Map(); + for (const target of group) { + const impact = cg.getImpactRadius(target.id, depth); + for (const [id, node] of impact.nodes) nodes.set(id, node); + for (const edge of impact.edges) edges.set(`${edge.source}->${edge.target}:${edge.kind}`, edge); + } + return { group, nodes, edges }; + }); + + if (options.json) { + const unionNodes = new Map(); + const unionEdges = new Map(); + const definitions = collected.map(({ group, nodes, edges }) => { + for (const [id, node] of nodes) unionNodes.set(id, node); + for (const [key, edge] of edges) unionEdges.set(key, edge); + return { + ...cliDefinition(group), + nodeCount: nodes.size, + edgeCount: edges.size, + affected: [...nodes.values()].map((node) => ({ id: node.id, ...cliNode(node) })), + edges: [...edges.values()], + }; + }); + console.log(JSON.stringify({ + symbol, + depth, + targets: groups.flat().map((node) => cliDefinition([node]).definition), + ambiguous, + aggregation: ambiguous ? 'union' : 'definition', + file: options.file, + filteredOut, + note, + definitions, + nodeCount: unionNodes.size, + edgeCount: unionEdges.size, + affected: [...unionNodes.values()].map(cliNode), + }, null, 2)); + } else { + if (note) warn(note); + if (ambiguous) { + console.log(chalk.bold(`\nImpact of changing "${symbol}" — ${groups.length} distinct definitions (each with its own blast radius; narrow with --file):`)); + } + for (const { group, nodes } of collected) { + if (ambiguous) { + console.log(chalk.bold(`\n${describeSymbolNode(group[0]!)} — ${nodes.size} affected symbols:\n`)); + } else { + console.log(chalk.bold(`\nImpact of changing "${symbol}" — ${nodes.size} affected symbols:\n`)); + console.log(chalk.dim(describeSymbolNode(group[0]!))); + } + const byFile = new Map(); + for (const node of nodes.values()) { + const list = byFile.get(node.filePath) || []; + list.push(node); + byFile.set(node.filePath, list); + } + for (const [file, affected] of byFile) { + console.log(chalk.cyan(file)); + for (const node of affected) { + const loc = node.startLine ? `:${node.startLine}` : ''; + console.log(` ${chalk.dim(node.kind.padEnd(12))}${node.name}${chalk.dim(loc)}`); + } + console.log(); + } + } + } + } finally { cg.destroy(); - return; } - - // Merge impact subgraphs across all exact-matching symbols - const mergedNodes = new Map(); - 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; - 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 }); - } - for (const e of impact.edges) { - const key = `${e.source}->${e.target}:${e.kind}`; - if (!seenEdges.has(key)) { - seenEdges.add(key); - edgeCount++; - } - } - } - - // 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, - depth, - nodeCount: mergedNodes.size, - edgeCount, - affected: Array.from(mergedNodes.values()), - }, null, 2)); - } else if (mergedNodes.size === 0) { - info(`No affected symbols found for "${symbol}"`); - } else { - console.log(chalk.bold(`\nImpact of changing "${symbol}" — ${mergedNodes.size} affected symbols:\n`)); - - // Group by file - const byFile = new Map>(); - for (const node of mergedNodes.values()) { - const list = byFile.get(node.filePath) || []; - list.push({ name: node.name, kind: node.kind, startLine: node.startLine }); - byFile.set(node.filePath, list); - } - - for (const [file, nodes] of byFile) { - console.log(chalk.cyan(file)); - for (const node of nodes) { - const loc = node.startLine ? `:${node.startLine}` : ''; - console.log(` ${chalk.dim(node.kind.padEnd(12))}${node.name}${chalk.dim(loc)}`); - } - console.log(); - } - } - - cg.destroy(); } catch (err) { error(`impact failed: ${err instanceof Error ? err.message : String(err)}`); process.exit(1); diff --git a/src/graph/named-symbol-flow.ts b/src/graph/named-symbol-flow.ts index 3fc095c..3046fec 100644 --- a/src/graph/named-symbol-flow.ts +++ b/src/graph/named-symbol-flow.ts @@ -36,93 +36,10 @@ import type CodeGraph from '../index'; import type { Node, Edge } from '../types'; import { isTestFile } from '../search/query-utils'; -/** - * Rust path roots that have no file-system equivalent — `crate` is the - * current crate, `super` is the parent module, `self` is the current - * module. Used by `matchesSymbol` to strip these before file-path - * matching so `crate::configurator::stage_apply::run` resolves the - * same as `configurator::stage_apply::run`. - */ -export const RUST_PATH_PREFIXES = new Set(['crate', 'super', 'self']); +import { lastQualifierPart, matchesSymbol } from './symbol-lookup'; -/** - * Last `::` / `.` / `/`-separated segment of a qualified symbol. An Erlang - * arity tail (`mod::fn/3`, `fn/3`) is stripped first — the useful last segment - * is the function name, never the digits (#1610). - */ -export function lastQualifierPart(symbol: string): string { - const noArity = symbol.replace(/\/\d{1,3}$/, '') || symbol; - const parts = noArity.split(/::|[./]/).filter((p) => p.length > 0); - return parts[parts.length - 1] ?? symbol; -} - -/** - * Check if a node matches a symbol query. - * - * Accepts simple names (`run`) and three flavors of qualifier: - * - dotted `Session.request` (TS/JS/Python) - * - colon-pair `stage_apply::run` (Rust, C++, Ruby) - * - slash `configurator/stage_apply` (path-ish) - * - * Multi-level qualifiers compose: `crate::configurator::stage_apply::run` - * works. Rust path prefixes (`crate`, `super`, `self`) are stripped so - * the canonical `crate::module::symbol` form resolves. - * - * Resolution order, last part must always equal `node.name`: - * 1. Suffix-match against `qualifiedName` (handles class-scoped methods - * where the extractor builds the qualified name from the AST stack) - * 2. File-path containment (handles file-derived modules in Rust/ - * Python — `stage_apply::run` matches a `run` in `stage_apply.rs`) - */ -export function matchesSymbol(node: Node, symbol: string): boolean { - // Erlang arity spelling (`fn/3`, `mod:fn/3` → normalized `mod.fn/3`): when - // the node's qualifiedName carries an arity (`mod::fn/3`, #1610), the - // written arity must match it exactly; the remaining comparison then runs - // on the arity-less spelling. A node with no arity in its qualifiedName - // keeps the original symbol (a `/` there means a path-ish name instead). - const aritySpelling = /^(.+)\/(\d{1,3})$/.exec(symbol); - if (aritySpelling) { - const nodeArity = /\/(\d{1,3})$/.exec(node.qualifiedName ?? '')?.[1]; - if (nodeArity !== undefined) { - if (nodeArity !== aritySpelling[2]) return false; - symbol = aritySpelling[1]!; - } - } - // Simple name match - if (node.name === symbol) return true; - // File basename match (e.g., "product-card" matches "product-card.liquid") - if (node.kind === 'file' && node.name.replace(/\.[^.]+$/, '') === symbol) return true; - - // Qualified-name lookups: split on any supported separator. `\w` keeps - // identifier chars (incl. `_`) intact; everything else is treated as - // a separator we tolerate. - if (!/[.\/]|::/.test(symbol)) return false; - const parts = symbol.split(/::|[./]/).filter((p) => p.length > 0); - if (parts.length < 2) return false; - - const lastPart = parts[parts.length - 1]!; - if (node.name !== lastPart) return false; - - // Stage 1: qualified-name suffix match. The extractor joins the - // semantic hierarchy with `::`, so `Session.request` and - // `Session::request` both become `Session::request` here. - const colonSuffix = parts.join('::'); - if (node.qualifiedName.includes(colonSuffix)) return true; - - // Stage 2: file-path containment. Rust modules and Python packages - // are not in `qualifiedName` — they're encoded in the file path. So - // `stage_apply::run` matches a `run` in any file whose path - // contains a `stage_apply` segment (with or without an extension). - // - // Filter out Rust path prefixes that have no file-system equivalent. - const containerHints = parts.slice(0, -1).filter((p) => !RUST_PATH_PREFIXES.has(p)); - if (containerHints.length === 0) return false; - - const segments = node.filePath.split('/').filter((s) => s.length > 0); - return containerHints.every((hint) => - segments.some((seg) => seg === hint || seg.replace(/\.[^.]+$/, '') === hint) - ); -} +// Preserve the existing imports while sharing the matcher with the CLI and MCP. +export { RUST_PATH_PREFIXES, lastQualifierPart, matchesSymbol } from './symbol-lookup'; /** * Find ALL symbols matching a name. Used by callers/callees/impact to aggregate diff --git a/src/graph/symbol-lookup.ts b/src/graph/symbol-lookup.ts new file mode 100644 index 0000000..fc16286 --- /dev/null +++ b/src/graph/symbol-lookup.ts @@ -0,0 +1,207 @@ +/** + * Symbol Lookup — the single "what did the user mean by this name?" path. + * + * Every verb that takes a symbol NAME from a human (or an agent) has to turn + * that string into node(s). `codegraph_node` and `codegraph_explore` went + * through the matcher below; the `callers` / `callees` / `impact` CLI verbs + * carried their own ad-hoc filter instead: + * + * node.name === symbol || node.name.endsWith('.' + symbol) + * + * which compares the query against the BARE name only. That produced two + * opposite failures in the same repository: + * + * - a bare name over-reported: `callers group` silently merged the callers of + * every distinct symbol named `group` — in any language — into one list + * headed "Callers of group", with nothing saying they were different + * symbols; + * - a qualified name under-reported: `Foo.Bar.baz` can never equal a bare + * `baz`, so every candidate failed the filter and the code fell through to + * an arbitrary top-of-FTS hit — or reported "not found" for a symbol that + * plainly exists. + * + * Both are fixed by routing all of them through one resolver, which this module + * owns so the CLI and the MCP tools cannot drift apart again. + */ + +import type { Node } from '../types'; + +/** Rust path prefixes that name no directory (`crate::x`, `super::y`). */ +export const RUST_PATH_PREFIXES = new Set(['crate', 'super', 'self']); + +/** Does this query carry any scope qualifier at all? */ +export function isQualifiedSymbol(symbol: string): boolean { + return /[.\/]|::/.test(symbol); +} + +/** The bare identifier at the end of a qualified query (arity spelling stripped). */ +export function lastQualifierPart(symbol: string): string { + const noArity = symbol.replace(/\/\d{1,3}$/, '') || symbol; + const parts = noArity.split(/::|[./]/).filter((p) => p.length > 0); + return parts[parts.length - 1] ?? symbol; +} + +/** + * Rewrite every scope separator to `.` so a query and a stored qualifiedName + * written in different conventions can be compared directly. The extractors + * join hierarchy with `::` while users type the language's own spelling + * (`Session.request`, `stage_apply::run`, `pkg/mod.Fn`). + */ +function canonicalScope(text: string): string { + return text.replace(/::/g, '.').replace(/\//g, '.'); +} + +/** + * Does `node` satisfy the user's symbol query? + * + * Bare queries match the name. Qualified queries are checked against the + * qualifiedName under both separator conventions, then — for languages whose + * hierarchy lives in the file path rather than the name (Rust modules, Python + * packages) — against the path. + */ +export function matchesSymbol(node: Node, symbol: string): boolean { + // Erlang arity spelling (`fn/3`, `mod:fn/3`): when the node's qualifiedName + // carries an arity (#1610) the written arity must match exactly, and the rest + // of the comparison runs on the arity-less spelling. A node with no arity + // keeps the original symbol (a `/` there means a path-ish name instead). + const aritySpelling = /^(.+)\/(\d{1,3})$/.exec(symbol); + if (aritySpelling) { + const nodeArity = /\/(\d{1,3})$/.exec(node.qualifiedName ?? '')?.[1]; + if (nodeArity !== undefined) { + if (nodeArity !== aritySpelling[2]) return false; + symbol = aritySpelling[1]!; + } + } + + if (node.name === symbol) return true; + // File basename match ("product-card" matches "product-card.liquid"). + if (node.kind === 'file' && node.name.replace(/\.[^.]+$/, '') === symbol) return true; + + if (!isQualifiedSymbol(symbol)) return false; + const parts = symbol.split(/::|[./]/).filter((p) => p.length > 0); + if (parts.length < 2) return false; + + const lastPart = parts[parts.length - 1]!; + if (node.name !== lastPart) return false; + + // Stage 1: qualified-name containment under the extractor's `::` convention. + if (node.qualifiedName.includes(parts.join('::'))) return true; + + // Stage 1b: boundary-aligned suffix under a canonical separator. + // + // Splitting on EVERY separator assumes no scope component contains one — + // false for any language whose module names are themselves dotted (Elixir + // `AppWeb.Format`, a Java/C# package, a Python dotted module). There the + // stored qualifiedName is `AppWeb.Format::group`, so the stage-1 spelling + // `AppWeb::Format::group` cannot match and a perfectly precise query + // resolved to nothing. Canonicalising both sides and requiring the match to + // land on a separator boundary handles both conventions with one rule, and + // is strictly tighter than the `includes` above. + const canonicalQuery = canonicalScope(symbol); + const canonicalNode = canonicalScope(node.qualifiedName); + if (canonicalNode === canonicalQuery || canonicalNode.endsWith(`.${canonicalQuery}`)) { + return true; + } + + // Stage 2: file-path containment. Rust modules and Python packages are not in + // qualifiedName — they are encoded in the path — so `stage_apply::run` + // matches a `run` in any file with a `stage_apply` path segment. + const containerHints = parts.slice(0, -1).filter((p) => !RUST_PATH_PREFIXES.has(p)); + if (containerHints.length === 0) return false; + const segments = node.filePath.split('/').filter((s) => s.length > 0); + return containerHints.every((hint) => + segments.some((seg) => seg === hint || seg.replace(/\.[^.]+$/, '') === hint) + ); +} + +/** The slice of CodeGraph a symbol lookup needs — keeps this module testable. */ +export interface SymbolLookupHost { + getNodesByName(name: string): Node[]; + searchNodes(query: string, options?: { limit?: number }): Array<{ node: Node }>; + generatedFilePredicate(paths: string[]): (path: string) => boolean; +} + +export interface SymbolLookupResult { + /** Every definition the query names, keepers before generated stubs. */ + nodes: Node[]; + /** + * The query named more than one distinct definition. Callers that aggregate + * across all of them MUST surface this — an aggregate presented as one + * symbol's answer is the over-reporting failure described at the top. + */ + ambiguous: boolean; +} + +/** + * One group per (filePath, qualifiedName): same-file overloads stay together, + * while unrelated definitions keep their own edges. Shared by CLI and MCP. + * A non-matching file hint keeps all definitions and must be disclosed. + */ +export function groupDefinitions( + nodes: Node[], + fileFilter?: string +): { groups: Node[][]; filteredOut: boolean } { + let pool = nodes; + let filteredOut = false; + if (fileFilter) { + const wanted = fileFilter.replace(/^\.\//, ''); + const narrowed = pool.filter( + (n) => n.filePath === wanted || n.filePath.endsWith(wanted) || n.filePath.endsWith(`/${wanted}`) + ); + if (narrowed.length > 0) pool = narrowed; + else filteredOut = true; + } + const byDef = new Map(); + for (const n of pool) { + const key = `${n.filePath}|${n.qualifiedName}`; + const group = byDef.get(key); + if (group) group.push(n); + else byDef.set(key, [n]); + } + return { groups: [...byDef.values()], filteredOut }; +} + +/** + * Resolve a user-supplied symbol name to the definitions it names. + * + * The exact-name index is consulted FIRST and is authoritative: it is complete + * and uncapped, whereas FTS ranks and truncates, and tokenises away `::` — so + * a qualified query could miss a symbol that exists, or land on whatever + * happened to rank first. FTS remains as the fallback for the fuzzy cases it is + * genuinely good at (file basenames, partial names). + */ +export function lookupSymbolNodes(cg: SymbolLookupHost, symbol: string): SymbolLookupResult { + const qualified = isQualifiedSymbol(symbol); + + // Exact-name index, then filter by the qualifier the user actually wrote. + const tail = qualified ? lastQualifierPart(symbol) : symbol; + let nodes = tail ? cg.getNodesByName(tail) : []; + if (qualified) nodes = nodes.filter((n) => matchesSymbol(n, symbol)); + + if (nodes.length === 0) { + const hits = cg.searchNodes(symbol, { limit: 50 }).map((h) => h.node); + const exact = hits.filter((n) => matchesSymbol(n, symbol)); + if (exact.length > 0) { + nodes = exact; + } else if (!qualified && hits[0]) { + // A bare name with no exact definition may still mean a file basename. + nodes = [hits[0]]; + } + // A qualified query with no exact match resolves to NOTHING rather than a + // misleading fuzzy hit (#173). + } + + if (nodes.length === 0) return { nodes: [], ambiguous: false }; + + // Keepers before generated stubs (.pb.go and friends), stable otherwise. + const isGenerated = cg.generatedFilePredicate(nodes.map((n) => n.filePath)); + const ranked = [...nodes].sort( + (a, b) => (isGenerated(a.filePath) ? 1 : 0) - (isGenerated(b.filePath) ? 1 : 0) + ); + return { nodes: ranked, ambiguous: groupDefinitions(ranked).groups.length > 1 }; +} + +/** One-line "kind at path:line" label used when disclosing an ambiguous query. */ +export function describeSymbolNode(node: Node): string { + return `${node.kind} ${node.qualifiedName || node.name} (${node.language}) — ${node.filePath}:${node.startLine}`; +} diff --git a/src/mcp/server-instructions.ts b/src/mcp/server-instructions.ts index a176286..2b2d4f8 100644 --- a/src/mcp/server-instructions.ts +++ b/src/mcp/server-instructions.ts @@ -55,6 +55,7 @@ calls; a grep/read exploration is dozens. - **"How does X reach/become Y? / the flow / the path from X to Y"** → \`codegraph_explore\`, naming the symbols that span the flow (e.g. \`mutateElement renderScene\`) — it surfaces the call path among them, riding dynamic-dispatch hops, and returns their source. - **Reading or editing a file/symbol you can name** → put its name or file path in the \`codegraph_explore\` query — it returns that current line-numbered source (safe to \`Edit\` from) with the call path and blast radius attached, so you don't Read it separately. For an overloaded name it returns every matching definition's body in one call. - **Need more?** Call \`codegraph_explore\` again with more specific names — treat the source it returns as already Read. +- Qualified symbol names accept dots, \`::\`, or slashes, including containers whose names contain dots (for example, \`AppWeb.Format.group\`). ## Anti-patterns diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index ce09594..4995d9b 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -32,6 +32,7 @@ import { import type { PendingFile } from '../sync'; import type { Node, Edge, SearchResult, Subgraph, NodeKind } from '../types'; import { isTestFile, normalizeNameToken } from '../search/query-utils'; +import { groupDefinitions, lastQualifierPart, matchesSymbol } from '../graph/symbol-lookup'; import { extractQueryPaths, queryMightContainPaths } from '../search/query-paths'; import { existsSync, @@ -44,8 +45,6 @@ import { guardLabel, guardsForFileSync, siteKey, supportsBranchGuards, warmBranc import { findDynamicBoundaries, type BoundarySite } from '../graph/dynamic-boundary-report'; import { countImplementers } from '../graph/type-hierarchy'; import { - lastQualifierPart, - matchesSymbol, findAllSymbols, resolveNamedSymbolFlow, } from '../graph/named-symbol-flow'; @@ -2350,27 +2349,7 @@ export class ToolHandler { nodes: Node[], fileFilter: string | undefined ): { groups: Node[][]; filteredOut: boolean } { - let pool = nodes; - let filteredOut = false; - if (fileFilter) { - const wanted = fileFilter.replace(/^\.\//, ''); - const narrowed = pool.filter( - (n) => n.filePath === wanted || n.filePath.endsWith(wanted) || n.filePath.endsWith(`/${wanted}`) - ); - if (narrowed.length > 0) { - pool = narrowed; - } else { - filteredOut = true; - } - } - const byDef = new Map(); - for (const n of pool) { - const key = `${n.filePath}|${n.qualifiedName}`; - const group = byDef.get(key); - if (group) group.push(n); - else byDef.set(key, [n]); - } - return { groups: [...byDef.values()], filteredOut }; + return groupDefinitions(nodes, fileFilter); } /** Section heading for one distinct definition in grouped output. */ @@ -6853,7 +6832,7 @@ export class ToolHandler { */ /** * Check if a node matches a symbol query — see `matchesSymbol` in - * `../graph/named-symbol-flow`, which owns the rules. + * `../graph/symbol-lookup`, which owns the rules. */ private matchesSymbol(node: Node, symbol: string): boolean { return matchesSymbol(node, symbol);