mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-09-19 07:34:57 +08:00
fix(cli): route callers/callees/impact through one symbol resolver
`callers`, `callees` and `impact` each carried their own symbol filter,
comparing the user's query against the BARE node name only:
node.name === symbol || node.name.endsWith('.' + symbol)
That fails in two opposite directions in the same repository.
A bare name OVER-reports. Every same-named definition passes the filter
and their results are unioned under one heading — "Callers of group" can
list callers that belong to an entirely different `group`, with nothing
saying the name was ambiguous. Collisions cluster on short generic names
(`group`, `num`, `parse`), so this bites hardest exactly where the verbs
would otherwise be most useful.
A qualified name UNDER-reports. `Foo.Bar.baz` can never equal a bare
`baz`, so every candidate fails the filter, and the guarded fallback
takes whichever node full-text search ranked first — or reports "not
found" for a symbol that plainly exists. It only ever appeared to work
when FTS happened to return exactly one hit.
All three now resolve through graph/symbol-lookup, which the MCP tools
share, so a verb cannot drift from the matcher again:
- the exact-name index is consulted first and is authoritative. It is
complete and uncapped, whereas FTS ranks, truncates, and tokenises
`::` away — so resolution no longer depends on search ranking. FTS
stays as the fallback for the fuzzy cases it is good at.
- `matchesSymbol` gains a boundary-aligned suffix match under a
canonical separator. Splitting on every separator assumes no scope
component contains one, which is false for any language whose module
names are themselves dotted: the stored `A.B::c` can never equal the
split-and-rejoined `A::B::c`, so a precise query resolved to nothing.
- an ambiguous bare name still aggregates (an interface method and its
overrides are usually all wanted) but the union is now disclosed, with
the matched definitions named and a qualified spelling that narrows
it. `--json` gains a `targets` array and an `ambiguous` flag.
`matchesSymbol` moves out of mcp/tools.ts unchanged apart from the new
stage; the tool path delegates to it, so its existing coverage applies.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013Bsi9EH64kMisnik4E1gf7
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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>): 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);
|
||||
});
|
||||
});
|
||||
|
||||
+92
-49
@@ -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, `<file>.<name>` 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<string>();
|
||||
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<string>();
|
||||
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<string, { name: string; kind: string; filePath: string; startLine?: number }>();
|
||||
const seenEdges = new Set<string>();
|
||||
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();
|
||||
|
||||
@@ -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}`;
|
||||
}
|
||||
+7
-61
@@ -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<NodeKind>([
|
||||
]);
|
||||
|
||||
/**
|
||||
* 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);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user