diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ca5b9ff1..761b6b1b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Fixes + +- `codegraph callers`, `codegraph callees` and `codegraph impact` now understand qualified symbol names. Asking for something like `Accounts.Format.group` used to match nothing, so the command quietly fell back to whichever symbol full-text search happened to rank first — or reported "not found" for a symbol that plainly exists. Qualified names now select exactly the definition you named, including in languages whose module names themselves contain dots. +- Those same commands now tell you when a plain name matches several different definitions. Previously the results for every same-named symbol — often in different languages, since collisions cluster on short names like `group`, `num` or `parse` — were merged into a single list with nothing saying they came from different symbols. The list still aggregates, but it now names what it aggregated and shows you the qualified spelling that narrows it. ## [1.6.0] - 2026-08-26 diff --git a/__tests__/symbol-lookup.test.ts b/__tests__/symbol-lookup.test.ts index c81aaabd4..e78b12ec2 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 19038df1b..b532e0be4 100644 --- a/src/bin/codegraph.ts +++ b/src/bin/codegraph.ts @@ -53,6 +53,7 @@ import { relaunchWithWasmRuntimeFlagsIfNeeded } from '../extraction/wasm-runtime import { installCommandSupervision } from './command-supervision'; import { EXTRACTION_VERSION } from '../extraction/extraction-version'; import { getTelemetry, TELEMETRY_DOCS, recordIndexEvent } from '../telemetry'; +import { lookupSymbolNodes, describeSymbolNode } from '../graph/symbol-lookup'; // Decided once, before `--color`/`--no-color` are stripped from argv below // (#1281). Piped/redirected stdout, NO_COLOR, or --no-color -> plain output. @@ -356,6 +357,56 @@ function warn(message: string): void { console.log(chalk.yellow(getGlyphs().warn) + ' ' + message); } +/** + * Disclose that a name resolved to several DISTINCT definitions, whose results + * were merged into the list just printed. + * + * `callers` / `callees` / `impact` aggregate across every definition a name + * matches. That is the useful default — an interface method and its overrides + * are usually all wanted — but presenting the union under one heading, with + * nothing saying the name was ambiguous, is how a query for a common + * identifier ends up reporting callers that belong to an entirely unrelated + * symbol (often in another language, since collisions cluster on short generic + * names like `group`, `num`, `parse`). Naming the targets keeps the aggregate + * useful and makes the widening visible, and tells the user the qualified + * spelling that would narrow it. + */ +function printAmbiguityNote( + symbol: string, + targets: Array<{ qualifiedName: string; kind: string; language: string; filePath: string; startLine: number }>, + ambiguous: boolean +): void { + if (!ambiguous || targets.length < 2) return; + const languages = new Set(targets.map((t) => t.language)); + console.log( + chalk.yellow(getGlyphs().warn) + + ` "${symbol}" names ${targets.length} definitions` + + (languages.size > 1 ? ` across ${languages.size} languages` : '') + + ' — the results above are the union of all of them:' + ); + for (const t of targets.slice(0, 10)) { + console.log(chalk.dim(` ${describeSymbolNode(t as never)}`)); + } + if (targets.length > 10) console.log(chalk.dim(` … +${targets.length - 10} more`)); + console.log(chalk.dim(` Narrow it with a qualified name, e.g. "${narrowingExample(targets[0]!)}".`)); +} + +/** + * A qualified spelling that would select exactly this definition. Languages + * that carry the container in `qualifiedName` (Elixir, Java, C++, class-scoped + * methods) can offer it directly; the ones that encode their module in the + * FILE PATH instead (Python, Rust) have a bare qualifiedName, so suggesting it + * would just echo the ambiguous name back. For those, `.` is the + * spelling that resolves — it is what the file-path stage of `matchesSymbol` + * matches on. + */ +function narrowingExample(target: { qualifiedName: string; name?: string; filePath: string }): string { + const qualified = target.qualifiedName.replace(/::/g, '.'); + if (qualified.includes('.')) return qualified; + const basename = target.filePath.split('/').pop()?.replace(/\.[^.]+$/, ''); + return basename ? `${basename}.${qualified}` : qualified; +} + type IndexResult = { success: boolean; filesIndexed: number; @@ -1957,8 +2008,8 @@ program const cg = await CodeGraph.open(projectPath); const limit = parseInt(options.limit || '20', 10); - const matches = cg.searchNodes(symbol, { limit: 50 }); - if (matches.length === 0) { + const { nodes: targets, ambiguous } = lookupSymbolNodes(cg, symbol); + if (targets.length === 0) { info(`Symbol "${symbol}" not found`); cg.destroy(); return; @@ -1967,20 +2018,8 @@ 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 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)) { + for (const target of targets) { + for (const c of cg.getCallers(target.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 }); @@ -1991,7 +2030,17 @@ program const limited = allCallers.slice(0, limit); if (options.json) { - console.log(JSON.stringify({ symbol, callers: limited }, null, 2)); + console.log(JSON.stringify({ + symbol, + // Which definitions the name resolved to. An aggregate over several + // distinct symbols has to say so — see graph/symbol-lookup. + targets: targets.map((t) => ({ + qualifiedName: t.qualifiedName, kind: t.kind, language: t.language, + filePath: t.filePath, startLine: t.startLine, + })), + ambiguous, + callers: limited, + }, null, 2)); } else if (limited.length === 0) { info(`No callers found for "${symbol}"`); } else { @@ -2005,6 +2054,7 @@ program console.log(chalk.dim(` ${node.filePath}${loc}`)); console.log(); } + printAmbiguityNote(symbol, targets, ambiguous); } cg.destroy(); @@ -2036,8 +2086,8 @@ program const cg = await CodeGraph.open(projectPath); const limit = parseInt(options.limit || '20', 10); - const matches = cg.searchNodes(symbol, { limit: 50 }); - if (matches.length === 0) { + const { nodes: targets, ambiguous } = lookupSymbolNodes(cg, symbol); + if (targets.length === 0) { info(`Symbol "${symbol}" not found`); cg.destroy(); return; @@ -2046,19 +2096,8 @@ 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 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)) { + for (const target of targets) { + for (const c of cg.getCallees(target.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 }); @@ -2069,7 +2108,15 @@ program const limited = allCallees.slice(0, limit); if (options.json) { - console.log(JSON.stringify({ symbol, callees: limited }, null, 2)); + console.log(JSON.stringify({ + symbol, + targets: targets.map((t) => ({ + qualifiedName: t.qualifiedName, kind: t.kind, language: t.language, + filePath: t.filePath, startLine: t.startLine, + })), + ambiguous, + callees: limited, + }, null, 2)); } else if (limited.length === 0) { info(`No callees found for "${symbol}"`); } else { @@ -2083,6 +2130,7 @@ program console.log(chalk.dim(` ${node.filePath}${loc}`)); console.log(); } + printAmbiguityNote(symbol, targets, ambiguous); } cg.destroy(); @@ -2114,22 +2162,20 @@ program const cg = await CodeGraph.open(projectPath); const depth = Math.min(Math.max(parseInt(options.depth || '2', 10), 1), 10); - const matches = cg.searchNodes(symbol, { limit: 50 }); - if (matches.length === 0) { + const { nodes: targets, ambiguous } = lookupSymbolNodes(cg, symbol); + if (targets.length === 0) { info(`Symbol "${symbol}" not found`); cg.destroy(); return; } - // Merge impact subgraphs across all exact-matching symbols + // Merge impact subgraphs across every definition the name resolved to. 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 target of targets) { + const impact = cg.getImpactRadius(target.id, depth); for (const [id, n] of impact.nodes) { mergedNodes.set(id, { name: n.name, kind: n.kind, filePath: n.filePath, startLine: n.startLine }); } @@ -2142,19 +2188,15 @@ 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, depth, + targets: targets.map((t) => ({ + qualifiedName: t.qualifiedName, kind: t.kind, language: t.language, + filePath: t.filePath, startLine: t.startLine, + })), + ambiguous, nodeCount: mergedNodes.size, edgeCount, affected: Array.from(mergedNodes.values()), @@ -2180,6 +2222,7 @@ program } console.log(); } + printAmbiguityNote(symbol, targets, ambiguous); } cg.destroy(); diff --git a/src/graph/symbol-lookup.ts b/src/graph/symbol-lookup.ts new file mode 100644 index 000000000..afeefed38 --- /dev/null +++ b/src/graph/symbol-lookup.ts @@ -0,0 +1,178 @@ +/** + * 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`). */ +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; +} + +/** + * 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: ranked.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/tools.ts b/src/mcp/tools.ts index 5c23f675d..535490067 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 { lastQualifierPart, matchesSymbol as matchesSymbolShared } from '../graph/symbol-lookup'; import { extractQueryPaths, queryMightContainPaths } from '../search/query-paths'; import { existsSync, @@ -104,13 +105,9 @@ const MAX_INPUT_LENGTH = 10_000; const MAX_PATH_LENGTH = 4_096; /** - * 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`. + * (Rust path roots — `crate`/`super`/`self` — moved to + * ../graph/symbol-lookup along with `matchesSymbol`.) */ -const RUST_PATH_PREFIXES = new Set(['crate', 'super', 'self']); /** * Node kinds that contain other symbols. For these, `codegraph_node` with @@ -123,15 +120,10 @@ const CONTAINER_NODE_KINDS = new Set([ ]); /** - * 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). + * Symbol-name resolution lives in ../graph/symbol-lookup so the CLI verbs and + * these tools share one path. `lastQualifierPart` and `matchesSymbol` are + * re-exported through this module's own call sites unchanged. */ -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; -} /** * Normalize Erlang-native symbol spellings in an explore query into the shapes @@ -6728,53 +6720,7 @@ export class ToolHandler { * Python — `stage_apply::run` matches a `run` in `stage_apply.rs`) */ private 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) - ); + return matchesSymbolShared(node, symbol); } /**