mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-17 05:07:37 +08:00
fix(explore): pin only the same-named file that defines the symbols a query names (#2071)
A bare basename several directories share (`editorOptions.ts`, `worker.rs`, `compiler.py`) pinned every match, and the matches split the pinned reservation: on vscode `editorOptions.ts clampedInt cursorStyleToString cursorStyleFromString` gave the file holding all three 8,626 chars, against 15,181 without the path, and spent the rest on a workbench helper defining none of them. Path extraction now takes an injected `symbolFiles` lookup (the module stays DB-free, like `existsOnDisk`). When a span matches several files, the matches defining a precise symbol token the query names (camelCase, PascalCase, snake_case, qualified — the seeder's NL-stopword shape test) are pinned and the rest set aside; the summary line names them. When no match defines one, or every match does, the span resolves exactly as before. The same test resolves a basename shared past the ambiguity budget (django's `models.py`) when it narrows into budget, and a line anchor binds once the span narrows to one file. Replay of 196 queries over 8 indexed repos, main vs this branch: every unambiguous pin is byte-identical (38 unique basenames, 13 dir/basename, 8 file:line, 38 earlier-sweep path queries), as are 26 path-free queries, 13 where every match defines a named symbol and 8 naming no symbol. All 52 changed responses are shared basenames — named FULL 207 -> 231 (11 up, 0 down), named lines 6,045 -> 7,577, none over 25K. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Colby Mchenry
@@ -167,6 +167,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
- When you name several functions in one file and unrelated code sits between them, `codegraph_explore` now returns the functions you named before the code around them, instead of spending the file's room on that code and leaving one of the functions you asked for with no source at all.
|
||||
- `codegraph_explore` now returns the body of a method you name together with its class, such as `SQLCompiler.as_sql`, even in a file full of same-named overrides. It used to shrink the method you asked for to its signature line while smaller methods around it came back in full. A method too long for the answer now comes back as its opening lines plus the places it calls the other symbols you named, with the exact follow-up query that returns the rest.
|
||||
- `codegraph_explore` now honours line numbers in a query: `compiler.py:776` returns the method containing that line, and `compiler.py lines 900-1003` (or `:900-1003`, `#L900-L1003`, `L900-L1003`) returns exactly those lines. It used to pin the file and ignore the numbers.
|
||||
- When a file name in a `codegraph_explore` query matches files in more than one folder, only the ones defining a function or class the query also names are pinned, and the summary line lists the files left out. Every match used to be pinned, so the file you meant got half the room it gets when you leave its name out; a name shared by many files, like `models.py`, now resolves the same way instead of being reported as ambiguous.
|
||||
|
||||
#### MCP / indexing
|
||||
|
||||
|
||||
@@ -0,0 +1,290 @@
|
||||
/**
|
||||
* A bare basename two directories share pins only the file defining what the
|
||||
* query names.
|
||||
*
|
||||
* vscode has two `editorOptions.ts`: the editor's option registry (which holds
|
||||
* `clampedInt`, `cursorStyleToString`, `cursorStyleFromString`) and a small
|
||||
* workbench helper holding none of them. `editorOptions.ts clampedInt
|
||||
* cursorStyleToString cursorStyleFromString` pinned BOTH, and the two pins split
|
||||
* the pinned reservation — 8,626 chars each, against 15,181 for the named file
|
||||
* when the same query left the path out. So writing the file's name made the
|
||||
* answer worse: the named file got half the room, and the unrelated one spent
|
||||
* the rest.
|
||||
*
|
||||
* Path extraction now asks which of a span's matches define a symbol the query
|
||||
* names, and pins those; the rest are set aside and named in the summary line.
|
||||
* When no match defines one, or every match does, the symbols pick nothing out
|
||||
* and every match pins as before.
|
||||
*
|
||||
* The fixture is that shape, generated so the sizes that decide it are visible
|
||||
* here: the registry is ~60 option classes with the three named functions in
|
||||
* the middle, the helper is a handful of unrelated functions.
|
||||
*/
|
||||
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
import * as os from 'os';
|
||||
import CodeGraph from '../src/index';
|
||||
import { ToolHandler } from '../src/mcp/tools';
|
||||
import type { ExploreDiagnosticReport } from '../src/mcp/explore-diagnostics';
|
||||
|
||||
const REGISTRY = 'src/editor/common/config/editorOptions.ts';
|
||||
const HELPER = 'src/workbench/common/editor/editorOptions.ts';
|
||||
const NAMED = ['clampedInt', 'cursorStyleToString', 'cursorStyleFromString'];
|
||||
|
||||
function registrySource(): string {
|
||||
const L: string[] = [
|
||||
'export interface IConfigurationPropertySchema {',
|
||||
' type?: string;',
|
||||
' default?: unknown;',
|
||||
' minimum?: number;',
|
||||
' maximum?: number;',
|
||||
'}',
|
||||
'',
|
||||
'export abstract class BaseEditorOption<K extends number, V> {',
|
||||
' constructor(public readonly id: K, public readonly name: string, public readonly defaultValue: V) {}',
|
||||
' public abstract validate(input: unknown): V;',
|
||||
'}',
|
||||
];
|
||||
const optionClass = (i: number) => {
|
||||
L.push(
|
||||
'',
|
||||
`export class EditorOption${i} extends BaseEditorOption<number, string> {`,
|
||||
` private readonly allowed${i} = ['alpha${i}', 'beta${i}', 'gamma${i}'];`,
|
||||
'',
|
||||
` constructor() {`,
|
||||
` super(${i}, 'option${i}', 'alpha${i}');`,
|
||||
' }',
|
||||
'',
|
||||
' public validate(input: unknown): string {',
|
||||
" if (typeof input !== 'string') {",
|
||||
' return this.defaultValue;',
|
||||
' }',
|
||||
` return this.allowed${i}.includes(input) ? input : this.defaultValue;`,
|
||||
' }',
|
||||
'}',
|
||||
);
|
||||
};
|
||||
for (let i = 0; i < 30; i++) optionClass(i);
|
||||
L.push(
|
||||
'',
|
||||
'export function clampedInt<T>(value: unknown, defaultValue: T, minimum: number, maximum: number): number | T {',
|
||||
" if (typeof value === 'undefined') {",
|
||||
' return defaultValue;',
|
||||
' }',
|
||||
' let r = parseInt(String(value), 10);',
|
||||
' if (isNaN(r)) {',
|
||||
' return defaultValue;',
|
||||
' }',
|
||||
' r = Math.max(minimum, r);',
|
||||
' r = Math.min(maximum, r);',
|
||||
' return r | 0;',
|
||||
'}',
|
||||
'',
|
||||
'export class EditorIntOption<K extends number> extends BaseEditorOption<K, number> {',
|
||||
' public static clampedInt<T>(value: unknown, defaultValue: T, minimum: number, maximum: number): number | T {',
|
||||
' return clampedInt(value, defaultValue, minimum, maximum);',
|
||||
' }',
|
||||
'',
|
||||
' constructor(id: K, name: string, defaultValue: number, public readonly minimum: number, public readonly maximum: number, schema?: IConfigurationPropertySchema) {',
|
||||
" if (typeof schema !== 'undefined') {",
|
||||
" schema.type = 'integer';",
|
||||
' schema.default = defaultValue;',
|
||||
' schema.minimum = minimum;',
|
||||
' schema.maximum = maximum;',
|
||||
' }',
|
||||
' super(id, name, defaultValue);',
|
||||
' }',
|
||||
'',
|
||||
' public validate(input: unknown): number {',
|
||||
' return EditorIntOption.clampedInt(input, this.defaultValue, this.minimum, this.maximum);',
|
||||
' }',
|
||||
'}',
|
||||
'',
|
||||
'export const enum TextEditorCursorStyle {',
|
||||
' Line = 1,',
|
||||
' Block = 2,',
|
||||
' Underline = 3,',
|
||||
' LineThin = 4,',
|
||||
' BlockOutline = 5,',
|
||||
' UnderlineThin = 6,',
|
||||
'}',
|
||||
'',
|
||||
'export function cursorStyleToString(cursorStyle: TextEditorCursorStyle): string {',
|
||||
' switch (cursorStyle) {',
|
||||
" case TextEditorCursorStyle.Line: return 'line';",
|
||||
" case TextEditorCursorStyle.Block: return 'block';",
|
||||
" case TextEditorCursorStyle.Underline: return 'underline';",
|
||||
" case TextEditorCursorStyle.LineThin: return 'line-thin';",
|
||||
" case TextEditorCursorStyle.BlockOutline: return 'block-outline';",
|
||||
" case TextEditorCursorStyle.UnderlineThin: return 'underline-thin';",
|
||||
' }',
|
||||
'}',
|
||||
'',
|
||||
'export function cursorStyleFromString(cursorStyle: string): TextEditorCursorStyle {',
|
||||
' switch (cursorStyle) {',
|
||||
" case 'line': return TextEditorCursorStyle.Line;",
|
||||
" case 'block': return TextEditorCursorStyle.Block;",
|
||||
" case 'underline': return TextEditorCursorStyle.Underline;",
|
||||
" case 'line-thin': return TextEditorCursorStyle.LineThin;",
|
||||
" case 'block-outline': return TextEditorCursorStyle.BlockOutline;",
|
||||
" case 'underline-thin': return TextEditorCursorStyle.UnderlineThin;",
|
||||
' }',
|
||||
' return TextEditorCursorStyle.Line;',
|
||||
'}',
|
||||
);
|
||||
for (let i = 30; i < 60; i++) optionClass(i);
|
||||
return L.join('\n') + '\n';
|
||||
}
|
||||
|
||||
function helperSource(): string {
|
||||
const L: string[] = [
|
||||
'export interface ITextEditorViewState { scrollTop: number; cursor: number }',
|
||||
'',
|
||||
'export function applyTextEditorOptions(options: { selection?: number; viewState?: ITextEditorViewState }, editor: { reveal(line: number): void; restore(s: ITextEditorViewState): void }): boolean {',
|
||||
' if (options.viewState) {',
|
||||
' editor.restore(massageEditorViewState(options.viewState));',
|
||||
' return true;',
|
||||
' }',
|
||||
' if (typeof options.selection === "number") {',
|
||||
' editor.reveal(options.selection);',
|
||||
' return true;',
|
||||
' }',
|
||||
' return false;',
|
||||
'}',
|
||||
'',
|
||||
'function massageEditorViewState(state: ITextEditorViewState): ITextEditorViewState {',
|
||||
' return { scrollTop: Math.max(0, state.scrollTop), cursor: Math.max(0, state.cursor) };',
|
||||
'}',
|
||||
];
|
||||
for (let i = 0; i < 12; i++) {
|
||||
L.push(
|
||||
'',
|
||||
`export function restoreEditorGroup${i}(state: ITextEditorViewState): number {`,
|
||||
` const offset = state.scrollTop * ${i + 2} + state.cursor;`,
|
||||
` return offset > ${100 * (i + 1)} ? offset - ${i} : offset + ${i};`,
|
||||
'}',
|
||||
);
|
||||
}
|
||||
return L.join('\n') + '\n';
|
||||
}
|
||||
|
||||
let dir: string;
|
||||
let cg: CodeGraph;
|
||||
|
||||
/** One explore call plus the CG-4 diagnostic for it. */
|
||||
async function explore(query: string): Promise<{ text: string; report: ExploreDiagnosticReport }> {
|
||||
const sidecar = path.join(fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-same-basename-diag-')), 'r.jsonl');
|
||||
const previous = process.env.CODEGRAPH_EXPLORE_DEBUG;
|
||||
process.env.CODEGRAPH_EXPLORE_DEBUG = sidecar;
|
||||
let text = '';
|
||||
try {
|
||||
const res = await new ToolHandler(cg).execute('codegraph_explore', { query });
|
||||
text = res.content?.[0]?.text ?? '';
|
||||
} finally {
|
||||
if (previous === undefined) delete process.env.CODEGRAPH_EXPLORE_DEBUG;
|
||||
else process.env.CODEGRAPH_EXPLORE_DEBUG = previous;
|
||||
}
|
||||
const lines = fs.readFileSync(sidecar, 'utf-8').trim().split('\n').filter(Boolean);
|
||||
fs.rmSync(path.dirname(sidecar), { recursive: true, force: true });
|
||||
return { text, report: JSON.parse(lines[lines.length - 1]!) as ExploreDiagnosticReport };
|
||||
}
|
||||
|
||||
const fileRecord = (report: ExploreDiagnosticReport, file: string) =>
|
||||
report.files.find((f) => f.path === file);
|
||||
|
||||
/** The response renders a source section for `file`. */
|
||||
const hasSection = (response: string, file: string): boolean =>
|
||||
response.includes('**`' + file + '`');
|
||||
|
||||
/** Every `<n>\t<text>` line number the response sent for `file`. */
|
||||
function renderedLines(response: string, file: string): Set<number> {
|
||||
const out = new Set<number>();
|
||||
let current: string | null = null;
|
||||
let inFence = false;
|
||||
for (const line of response.split('\n')) {
|
||||
const header = !inFence ? line.match(/^\*\*`([^`]+)`\*\*/) : null;
|
||||
if (header) { current = header[1]!; continue; }
|
||||
if (line.startsWith('```')) { inFence = !inFence; continue; }
|
||||
const m = inFence && current === file ? line.match(/^(\d+)\t/) : null;
|
||||
if (m) out.add(Number(m[1]));
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/** How many of the registry's named definitions the response sent whole. */
|
||||
function completeNamedBodies(response: string): number {
|
||||
const sent = renderedLines(response, REGISTRY);
|
||||
return cg.getNodesInFile(REGISTRY)
|
||||
.filter((n) => NAMED.includes(n.name) && n.kind !== 'import')
|
||||
.filter((n) => {
|
||||
for (let l = n.startLine; l <= n.endLine; l++) if (!sent.has(l)) return false;
|
||||
return true;
|
||||
}).length;
|
||||
}
|
||||
|
||||
beforeAll(async () => {
|
||||
dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-same-basename-'));
|
||||
for (const [file, source] of [[REGISTRY, registrySource()], [HELPER, helperSource()]] as const) {
|
||||
fs.mkdirSync(path.join(dir, path.dirname(file)), { recursive: true });
|
||||
fs.writeFileSync(path.join(dir, file), source);
|
||||
}
|
||||
cg = CodeGraph.initSync(dir);
|
||||
await cg.indexAll();
|
||||
}, 180_000);
|
||||
|
||||
afterAll(() => {
|
||||
cg?.destroy();
|
||||
if (dir && fs.existsSync(dir)) fs.rmSync(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
describe('fixture shape — if this rots, the gates below mean nothing', () => {
|
||||
it('the named symbols live in the registry and nowhere in the helper', () => {
|
||||
const registry = cg.getNodesInFile(REGISTRY).map((n) => n.name);
|
||||
const helper = cg.getNodesInFile(HELPER).map((n) => n.name);
|
||||
for (const name of NAMED) {
|
||||
expect(registry).toContain(name);
|
||||
expect(helper).not.toContain(name);
|
||||
}
|
||||
// Two `clampedInt`s — the function and the static method — so four bodies.
|
||||
expect(cg.getNodesInFile(REGISTRY).filter((n) => NAMED.includes(n.name))).toHaveLength(4);
|
||||
});
|
||||
});
|
||||
|
||||
describe('a basename two directories share', () => {
|
||||
it('pins only the file defining the named symbols, and names the one it set aside', async () => {
|
||||
const { text, report } = await explore(`editorOptions.ts ${NAMED.join(' ')}`);
|
||||
expect(fileRecord(report, REGISTRY)?.pinned).toBe(true);
|
||||
expect(fileRecord(report, HELPER)?.pinned ?? false).toBe(false);
|
||||
expect(hasSection(text, HELPER)).toBe(false);
|
||||
expect(text).toContain('1 file pinned from the query.');
|
||||
expect(text).toContain(`Not pinned: \`${HELPER}\`, which defines none of the named symbols.`);
|
||||
});
|
||||
|
||||
it('gives the named file the room it gets when the query leaves the path out', async () => {
|
||||
const pinned = await explore(`editorOptions.ts ${NAMED.join(' ')}`);
|
||||
const unpinned = await explore(NAMED.join(' '));
|
||||
const pinnedAllowance = fileRecord(pinned.report, REGISTRY)?.allowance ?? 0;
|
||||
const unpinnedAllowance = fileRecord(unpinned.report, REGISTRY)?.allowance ?? 0;
|
||||
expect(unpinnedAllowance).toBeGreaterThan(0);
|
||||
// Pre-fix the two pins split the reservation: half the room, or less.
|
||||
expect(pinnedAllowance).toBeGreaterThanOrEqual(unpinnedAllowance);
|
||||
expect(completeNamedBodies(pinned.text)).toBeGreaterThanOrEqual(completeNamedBodies(unpinned.text));
|
||||
expect(completeNamedBodies(pinned.text)).toBe(4);
|
||||
});
|
||||
|
||||
it('pins both when the query names no symbol to choose between them', async () => {
|
||||
const { text, report } = await explore('editorOptions.ts');
|
||||
expect(fileRecord(report, REGISTRY)?.pinned).toBe(true);
|
||||
expect(fileRecord(report, HELPER)?.pinned).toBe(true);
|
||||
expect(text).toContain('2 files pinned from the query.');
|
||||
expect(text).not.toContain('Not pinned:');
|
||||
});
|
||||
|
||||
it('a path with its directory pins that file alone, with no set-aside note', async () => {
|
||||
const { text, report } = await explore(`${HELPER} ${NAMED.join(' ')}`);
|
||||
expect(fileRecord(report, HELPER)?.pinned).toBe(true);
|
||||
expect(fileRecord(report, REGISTRY)?.pinned ?? false).toBe(false);
|
||||
expect(text).not.toContain('Not pinned:');
|
||||
});
|
||||
});
|
||||
@@ -151,7 +151,7 @@ describe('extractQueryPaths — resolution and stripping', () => {
|
||||
it('passes through untouched when nothing resolves', () => {
|
||||
const q = 'plain prose question about scrolling';
|
||||
const out = extractQueryPaths(q, INDEX);
|
||||
expect(out).toEqual({ strippedQuery: q, pinnedFiles: [], unresolvedPathSpans: [], lineAnchors: [] });
|
||||
expect(out).toEqual({ strippedQuery: q, pinnedFiles: [], unresolvedPathSpans: [], lineAnchors: [], setAsideMatches: [] });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -191,13 +191,13 @@ describe('extractQueryPaths — extension-less kebab basenames', () => {
|
||||
it('leaves kebab prose that names no indexed file untouched — and unreported', () => {
|
||||
const q = 'how does cross-call dedup make explore non-blocking';
|
||||
const out = extractQueryPaths(q, INDEX);
|
||||
expect(out).toEqual({ strippedQuery: q, pinnedFiles: [], unresolvedPathSpans: [], lineAnchors: [] });
|
||||
expect(out).toEqual({ strippedQuery: q, pinnedFiles: [], unresolvedPathSpans: [], lineAnchors: [], setAsideMatches: [] });
|
||||
});
|
||||
|
||||
it('leaves a stem shared by too many files alone — one hot name must not pin half the repo', () => {
|
||||
const q = 'refactor the user-profile rendering';
|
||||
const out = extractQueryPaths(q, INDEX);
|
||||
expect(out).toEqual({ strippedQuery: q, pinnedFiles: [], unresolvedPathSpans: [], lineAnchors: [] });
|
||||
expect(out).toEqual({ strippedQuery: q, pinnedFiles: [], unresolvedPathSpans: [], lineAnchors: [], setAsideMatches: [] });
|
||||
});
|
||||
|
||||
it('pins all files sharing a stem when within the ambiguity budget', () => {
|
||||
@@ -371,3 +371,134 @@ describe('extractQueryPaths — line anchors', () => {
|
||||
expect(twoFiles.lineAnchors).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* A span that matches several files, narrowed by the symbols the query names.
|
||||
*
|
||||
* vscode has two `editorOptions.ts`; `editorOptions.ts clampedInt
|
||||
* cursorStyleToString` pinned both, and the pins split the reservation, so the
|
||||
* file holding the named functions got half the room it got WITHOUT the path.
|
||||
* The caller injects `symbolFiles` (the index lookup; this module stays
|
||||
* DB-free) and the matches defining a named symbol are the ones pinned.
|
||||
*/
|
||||
describe('extractQueryPaths — same-named files, narrowed by named symbols', () => {
|
||||
const REGISTRY = 'src/editor/config/editorOptions.ts';
|
||||
const HELPER = 'src/workbench/editor/editorOptions.ts';
|
||||
const SHARED = [...INDEX, REGISTRY, HELPER];
|
||||
/** Records every symbol the lookup is asked about. */
|
||||
const lookup = (defs: Record<string, string[]>) => {
|
||||
const asked: string[] = [];
|
||||
return {
|
||||
asked,
|
||||
symbolFiles: (symbol: string) => { asked.push(symbol); return defs[symbol] ?? []; },
|
||||
};
|
||||
};
|
||||
|
||||
it('pins only the match defining a named symbol and reports the one set aside', () => {
|
||||
const l = lookup({ clampedInt: [REGISTRY], cursorStyleToString: [REGISTRY] });
|
||||
const out = extractQueryPaths(
|
||||
'editorOptions.ts clampedInt cursorStyleToString', SHARED, { symbolFiles: l.symbolFiles },
|
||||
);
|
||||
expect(out.pinnedFiles).toEqual([REGISTRY]);
|
||||
expect(out.setAsideMatches).toEqual([{ span: 'editorOptions.ts', files: [HELPER] }]);
|
||||
expect(out.strippedQuery).toBe('clampedInt cursorStyleToString');
|
||||
// The span itself is a file name, not a symbol to look up.
|
||||
expect(l.asked).toEqual(['clampedInt', 'cursorStyleToString']);
|
||||
});
|
||||
|
||||
it('narrows a shared kebab stem the same way', () => {
|
||||
const l = lookup({ GenericModalFooter: ['src/y/generic-modal.tsx'] });
|
||||
const out = extractQueryPaths('generic-modal GenericModalFooter', SHARED, { symbolFiles: l.symbolFiles });
|
||||
expect(out.pinnedFiles).toEqual(['src/y/generic-modal.tsx']);
|
||||
expect(out.setAsideMatches).toEqual([{ span: 'generic-modal', files: ['src/x/generic-modal.tsx'] }]);
|
||||
});
|
||||
|
||||
it('pins every match when none defines a named symbol, or every one does', () => {
|
||||
const none = lookup({ clampedInt: ['src/lib/chat-manager.ts'] });
|
||||
const noneOut = extractQueryPaths('editorOptions.ts clampedInt', SHARED, { symbolFiles: none.symbolFiles });
|
||||
expect(noneOut.pinnedFiles).toEqual([REGISTRY, HELPER]);
|
||||
expect(noneOut.setAsideMatches).toEqual([]);
|
||||
// Consulted and found nothing — not skipped.
|
||||
expect(none.asked).toEqual(['clampedInt']);
|
||||
|
||||
const both = lookup({ EditorOptions: [HELPER, REGISTRY] });
|
||||
const bothOut = extractQueryPaths('editorOptions.ts EditorOptions', SHARED, { symbolFiles: both.symbolFiles });
|
||||
expect(bothOut.pinnedFiles).toEqual([REGISTRY, HELPER]);
|
||||
expect(bothOut.setAsideMatches).toEqual([]);
|
||||
});
|
||||
|
||||
it('does not let a bare English word pick a file', () => {
|
||||
// `options` and `close` are words as much as names: a file defining one
|
||||
// says nothing about which same-named file the agent meant.
|
||||
const l = lookup({ options: [HELPER], close: ['src/x/generic-modal.tsx'] });
|
||||
expect(extractQueryPaths('editorOptions.ts options', SHARED, { symbolFiles: l.symbolFiles }).pinnedFiles)
|
||||
.toEqual([REGISTRY, HELPER]);
|
||||
expect(extractQueryPaths('generic-modal close behavior', SHARED, { symbolFiles: l.symbolFiles }).pinnedFiles)
|
||||
.toEqual(['src/x/generic-modal.tsx', 'src/y/generic-modal.tsx']);
|
||||
expect(l.asked).toEqual([]);
|
||||
});
|
||||
|
||||
it('looks a qualified token up as written, and reads `name()` as the name', () => {
|
||||
const l = lookup({ 'EditorIntOption.clampedInt': [REGISTRY], cursorStyleFromString: [REGISTRY] });
|
||||
const out = extractQueryPaths(
|
||||
'editorOptions.ts EditorIntOption.clampedInt `cursorStyleFromString()`', SHARED,
|
||||
{ symbolFiles: l.symbolFiles },
|
||||
);
|
||||
expect(out.pinnedFiles).toEqual([REGISTRY]);
|
||||
expect(l.asked).toEqual(['EditorIntOption.clampedInt', 'cursorStyleFromString']);
|
||||
});
|
||||
|
||||
it('resolves a basename shared past the ambiguity budget when one match defines the named symbol', () => {
|
||||
const l = lookup({ UserProfileCard: ['src/c/user-profile.tsx'] });
|
||||
const out = extractQueryPaths('user-profile UserProfileCard', SHARED, { symbolFiles: l.symbolFiles });
|
||||
expect(out.pinnedFiles).toEqual(['src/c/user-profile.tsx']);
|
||||
expect(out.setAsideMatches).toEqual([{
|
||||
span: 'user-profile',
|
||||
files: ['src/a/user-profile.tsx', 'src/b/user-profile.tsx', 'src/d/user-profile.tsx'],
|
||||
}]);
|
||||
|
||||
const dotted = extractQueryPaths('+page.svelte ChatScroller', SHARED, {
|
||||
symbolFiles: lookup({ ChatScroller: ['src/routes/(protected)/chat-window/+page.svelte'] }).symbolFiles,
|
||||
});
|
||||
expect(dotted.pinnedFiles).toEqual(['src/routes/(protected)/chat-window/+page.svelte']);
|
||||
expect(dotted.unresolvedPathSpans).toEqual([]);
|
||||
});
|
||||
|
||||
it('keeps an over-budget span ambiguous when the symbols do not narrow it into budget', () => {
|
||||
const l = lookup({ PageHeader: [
|
||||
'src/routes/m/projects/[id]/runs/[runId]/+page.svelte',
|
||||
'src/routes/m/projects/[id]/chat/[scope]/+page.svelte',
|
||||
'src/routes/m/projects/[id]/+page.svelte',
|
||||
'src/routes/(protected)/chat-window/+page.svelte',
|
||||
] });
|
||||
const out = extractQueryPaths('why do all +page.svelte PageHeader files flash', SHARED, { symbolFiles: l.symbolFiles });
|
||||
expect(out.pinnedFiles).toEqual([]);
|
||||
expect(out.unresolvedPathSpans).toEqual(['+page.svelte']);
|
||||
expect(out.setAsideMatches).toEqual([]);
|
||||
|
||||
const kebab = extractQueryPaths('refactor the user-profile rendering', SHARED, { symbolFiles: l.symbolFiles });
|
||||
expect(kebab.pinnedFiles).toEqual([]);
|
||||
expect(kebab.strippedQuery).toBe('refactor the user-profile rendering');
|
||||
});
|
||||
|
||||
it('binds a line anchor once the span narrows to one file', () => {
|
||||
const l = lookup({ clampedInt: [REGISTRY] });
|
||||
const out = extractQueryPaths('editorOptions.ts:1291 clampedInt', SHARED, { symbolFiles: l.symbolFiles });
|
||||
expect(out.pinnedFiles).toEqual([REGISTRY]);
|
||||
expect(out.lineAnchors).toEqual([{ file: REGISTRY, start: 1291, end: 1291 }]);
|
||||
});
|
||||
|
||||
it('does not report a file the query also named by its full path', () => {
|
||||
const l = lookup({ clampedInt: [REGISTRY] });
|
||||
const out = extractQueryPaths(`editorOptions.ts clampedInt ${HELPER}`, SHARED, { symbolFiles: l.symbolFiles });
|
||||
expect(out.pinnedFiles).toEqual([REGISTRY, HELPER]);
|
||||
expect(out.setAsideMatches).toEqual([]);
|
||||
});
|
||||
|
||||
it('never looks anything up for a span that matches one file', () => {
|
||||
const l = lookup({ ChatManager: ['src/lib/chat-manager.ts'] });
|
||||
const out = extractQueryPaths('chat-manager.ts ChatManager', SHARED, { symbolFiles: l.symbolFiles });
|
||||
expect(out.pinnedFiles).toEqual(['src/lib/chat-manager.ts']);
|
||||
expect(l.asked).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
+48
-4
@@ -33,8 +33,13 @@ 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, type QueryLineAnchor } from '../search/query-paths';
|
||||
import { groupDefinitions, isQualifiedSymbol, lastQualifierPart, matchesSymbol } from '../graph/symbol-lookup';
|
||||
import {
|
||||
extractQueryPaths,
|
||||
queryMightContainPaths,
|
||||
type QueryLineAnchor,
|
||||
type QuerySetAsideMatch,
|
||||
} from '../search/query-paths';
|
||||
import {
|
||||
existsSync,
|
||||
readFileSync,
|
||||
@@ -196,6 +201,29 @@ function pathIsProjectFile(projectRoot: string, relPath: string): boolean {
|
||||
}
|
||||
}
|
||||
|
||||
/** Kinds that name a thing without defining it in the file that holds them. */
|
||||
const NOT_A_DEFINITION = new Set(['file', 'import', 'export', 'parameter']);
|
||||
|
||||
/**
|
||||
* Which indexed files define a symbol spelled like this query token?
|
||||
*
|
||||
* The `symbolFiles` lookup `extractQueryPaths` takes, split out for the same
|
||||
* reason as `pathIsProjectFile`: that module stays DB-free. Exact names only —
|
||||
* and the shared matcher for a qualified token (`SQLCompiler.as_sql`) — so it
|
||||
* agrees with what explore's named-symbol seeding resolves. Lookup errors
|
||||
* answer "none", which leaves the span's pins as they were.
|
||||
*/
|
||||
function filesDefiningSymbol(cg: CodeGraph, symbol: string): string[] {
|
||||
try {
|
||||
const nodes = isQualifiedSymbol(symbol)
|
||||
? cg.getNodesByName(lastQualifierPart(symbol)).filter((n) => matchesSymbol(n, symbol))
|
||||
: cg.getNodesByName(symbol);
|
||||
return nodes.filter((n) => !NOT_A_DEFINITION.has(n.kind)).map((n) => n.filePath);
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Calculate the recommended number of codegraph_explore calls based on project size.
|
||||
* Larger codebases need more exploration calls to cover their surface area,
|
||||
@@ -3709,22 +3737,36 @@ export class ToolHandler {
|
||||
let pinnedFiles: string[] = [];
|
||||
let unresolvedPathSpans: string[] = [];
|
||||
let lineAnchors: QueryLineAnchor[] = [];
|
||||
let setAsideMatches: QuerySetAsideMatch[] = [];
|
||||
let matchQuery = query;
|
||||
if (queryMightContainPaths(rawQuery)) {
|
||||
try {
|
||||
const extraction = extractQueryPaths(
|
||||
rawQuery,
|
||||
cg.getFiles().map((f) => f.path),
|
||||
{ maxPins: maxFiles, existsOnDisk: (rel) => pathIsProjectFile(projectRoot, rel) },
|
||||
{
|
||||
maxPins: maxFiles,
|
||||
existsOnDisk: (rel) => pathIsProjectFile(projectRoot, rel),
|
||||
symbolFiles: (symbol) => filesDefiningSymbol(cg, symbol),
|
||||
},
|
||||
);
|
||||
if (extraction.pinnedFiles.length > 0 || extraction.unresolvedPathSpans.length > 0) {
|
||||
pinnedFiles = extraction.pinnedFiles;
|
||||
unresolvedPathSpans = extraction.unresolvedPathSpans;
|
||||
lineAnchors = extraction.lineAnchors;
|
||||
setAsideMatches = extraction.setAsideMatches;
|
||||
matchQuery = normalizeQuerySpelling(extraction.strippedQuery);
|
||||
}
|
||||
} catch { /* path pinning must never fail an explore call */ }
|
||||
}
|
||||
// A same-named file the span did not pin is named in the summary line, so
|
||||
// an agent that did mean it sees where it went instead of a silent drop.
|
||||
const setAsideNote = setAsideMatches.map((s) => {
|
||||
const which = s.files.length <= 2
|
||||
? s.files.map((f) => `\`${f}\``).join(', ')
|
||||
: `${s.files.length} other \`${s.span}\` files`;
|
||||
return ` Not pinned: ${which}, which define${s.files.length === 1 ? 's' : ''} none of the named symbols.`;
|
||||
}).join('');
|
||||
const pinnedSet = new Set(pinnedFiles);
|
||||
const pinnedOrder = new Map(pinnedFiles.map((p, i) => [p, i]));
|
||||
|
||||
@@ -4907,7 +4949,8 @@ export class ToolHandler {
|
||||
+ (pinnedFiles.length > 0 ? ` ${plural(pinnedFiles.length, 'file')} pinned from the query.`.length : 0)
|
||||
+ (unresolvedPathSpans.length > 0
|
||||
? ` No indexed file uniquely matches ${unresolvedPathSpans.map((sp) => `\`${sp}\``).join(', ')}.`.length
|
||||
: 0);
|
||||
: 0)
|
||||
+ setAsideNote.length;
|
||||
const epilogueFloor = EPILOGUE_LOST_NOTE.length + 2 + cliffPointerFloor + summaryReserve;
|
||||
// Absolute stop for the render loop. Reservations already fit the envelope, so
|
||||
// this only catches their bounded overshoot (the whole-file grace, an oversize
|
||||
@@ -7071,6 +7114,7 @@ export class ToolHandler {
|
||||
if (unresolvedPathSpans.length > 0) {
|
||||
summaryLine += ` No indexed file uniquely matches ${unresolvedPathSpans.map((s) => `\`${s}\``).join(', ')}.`;
|
||||
}
|
||||
summaryLine += setAsideNote;
|
||||
finalText = finalText.replace(SUMMARY_SENTINEL, summaryLine);
|
||||
|
||||
// Emit the allocation diagnostic from the FINAL text, so per-file bytes and
|
||||
|
||||
+122
-5
@@ -54,6 +54,18 @@ export interface QueryPathExtraction {
|
||||
* across two candidate files.
|
||||
*/
|
||||
lineAnchors: QueryLineAnchor[];
|
||||
/**
|
||||
* Files a span matched but did NOT pin: the span named several files, some
|
||||
* of which define a symbol the query also names, and these define none.
|
||||
* Surfaced so a set-aside file is visible, not silently dropped.
|
||||
*/
|
||||
setAsideMatches: QuerySetAsideMatch[];
|
||||
}
|
||||
|
||||
export interface QuerySetAsideMatch {
|
||||
/** The span as the query wrote it, normalized (`editorOptions.ts`). */
|
||||
span: string;
|
||||
files: string[];
|
||||
}
|
||||
|
||||
export interface QueryLineAnchor {
|
||||
@@ -100,6 +112,43 @@ const KEBAB_BASENAME = /^[A-Za-z0-9]+(?:-[A-Za-z0-9]+)+$/;
|
||||
/** A basename's last dot-extension, same shape DOTTED_BASENAME accepts. */
|
||||
const LAST_EXTENSION = /\.[A-Za-z][A-Za-z0-9]{0,7}$/;
|
||||
|
||||
/**
|
||||
* A query token that names a symbol, in the shape explore's named-symbol
|
||||
* seeder reads: `clampedInt`, `SQLCompiler.as_sql`, `Engine::ServeHTTP`.
|
||||
*/
|
||||
const SYMBOL_TOKEN = /^[A-Za-z_$][\w$]*(?:(?:::|\.)[\w$]+)*$/;
|
||||
/** Symbol tokens consulted per query — the seeder's cap. */
|
||||
const MAX_SYMBOL_TOKENS = 16;
|
||||
|
||||
/**
|
||||
* The seeder's NL-stopword test: camelCase, PascalCase, snake_case, `$` and
|
||||
* qualified tokens are unmistakably code. A bare lowercase word (`options`,
|
||||
* `render`) is also English, and a file defining one proves nothing about
|
||||
* which same-named file the agent meant.
|
||||
*/
|
||||
function isPreciseSymbolToken(t: string): boolean {
|
||||
return /[._$]|::/.test(t) || /[a-z][A-Z]/.test(t) || /^[A-Z]/.test(t);
|
||||
}
|
||||
|
||||
/**
|
||||
* The precise symbol tokens a query names, excluding the files it names
|
||||
* (`editorOptions.ts` is identifier-shaped too). Split on the same brackets
|
||||
* the seeder splits on, so `clampedInt()` still counts.
|
||||
*/
|
||||
function querySymbolTokens(tokens: readonly string[], indexedBasenames: ReadonlySet<string>): string[] {
|
||||
const out = new Set<string>();
|
||||
for (const raw of tokens) {
|
||||
for (const part of raw.split(/[,()[\]{}]+/)) {
|
||||
const t = part.replace(/^['"`<]+/, '').replace(/['"`>.,;:!?]+$/, '');
|
||||
if (t.length < 3 || !SYMBOL_TOKEN.test(t) || !isPreciseSymbolToken(t)) continue;
|
||||
if (indexedBasenames.has(t.toLowerCase())) continue;
|
||||
out.add(t);
|
||||
if (out.size >= MAX_SYMBOL_TOKENS) return [...out];
|
||||
}
|
||||
}
|
||||
return [...out];
|
||||
}
|
||||
|
||||
/**
|
||||
* Lowercased basename stems of the hyphen-named indexed files, stem → paths.
|
||||
* A stem drops only the LAST extension (`a-b.module.scss` → `a-b.module`), so
|
||||
@@ -256,6 +305,16 @@ export function extractQueryPaths(
|
||||
* Must not throw — the caller absorbs fs errors and returns false.
|
||||
*/
|
||||
existsOnDisk?: (relPath: string) => boolean;
|
||||
/**
|
||||
* Which indexed files define a symbol spelled like this query token
|
||||
* (`clampedInt`, `SQLCompiler.as_sql`)? Optional, injected by the caller
|
||||
* for the same reason as `existsOnDisk`: the answer lives in the index,
|
||||
* and this module stays DB-free. Consulted only when a span matches
|
||||
* several files (a basename two directories share): the matches that
|
||||
* define a symbol the query also names are pinned, the rest set aside.
|
||||
* Must not throw — the caller absorbs lookup errors and returns nothing.
|
||||
*/
|
||||
symbolFiles?: (symbol: string) => Iterable<string>;
|
||||
} = {},
|
||||
): QueryPathExtraction {
|
||||
const maxPins = Math.max(1, opts.maxPins ?? 8);
|
||||
@@ -266,6 +325,7 @@ export function extractQueryPaths(
|
||||
pinnedFiles: [],
|
||||
unresolvedPathSpans: [],
|
||||
lineAnchors: [],
|
||||
setAsideMatches: [],
|
||||
};
|
||||
if (!query.trim() || indexedPaths.length === 0) return passthrough;
|
||||
|
||||
@@ -280,10 +340,60 @@ export function extractQueryPaths(
|
||||
const pinnedSeen = new Set<string>();
|
||||
const unresolved: string[] = [];
|
||||
const anchors: QueryLineAnchor[] = [];
|
||||
const setAside: QuerySetAsideMatch[] = [];
|
||||
/** Token index → the ONE file it pinned, for binding prose line ranges. */
|
||||
const singleFileAt = new Map<number, string>();
|
||||
let candidatesExamined = 0;
|
||||
|
||||
// A bare basename two directories share (`editorOptions.ts` is both vscode's
|
||||
// editor option registry and a workbench helper) used to pin EVERY match,
|
||||
// and the matches split the pinned reservation — so a query that also named
|
||||
// three functions in one of them got half the room for them it would have
|
||||
// got without the path. When the query names symbols, the file it means is
|
||||
// the one defining them: keep the matches that define at least one, set the
|
||||
// rest aside. When no match defines one, or every match does, the symbols
|
||||
// pick nothing out and the span resolves exactly as before.
|
||||
//
|
||||
// The same test lets a basename shared by more files than the ambiguity
|
||||
// budget (django's 40 `models.py`) resolve after all — to the one defining
|
||||
// the named class — so a span is collected past the budget only while
|
||||
// narrowing is possible, and still reports as ambiguous if it doesn't
|
||||
// narrow into the budget.
|
||||
let symbolTokens: string[] | null = null;
|
||||
const definersOf = new Map<string, ReadonlySet<string>>();
|
||||
const narrowBySymbols = (matches: readonly string[]): string[] | null => {
|
||||
if (!opts.symbolFiles || matches.length < 2) return null;
|
||||
if (symbolTokens === null) {
|
||||
const basenames = new Set<string>();
|
||||
for (const p of indexedPaths) {
|
||||
basenames.add(p.slice(Math.max(p.lastIndexOf('/'), p.lastIndexOf('\\')) + 1).toLowerCase());
|
||||
}
|
||||
symbolTokens = querySymbolTokens(tokens, basenames);
|
||||
}
|
||||
const candidates = new Set(matches);
|
||||
const definers = new Set<string>();
|
||||
for (const t of symbolTokens) {
|
||||
let files = definersOf.get(t);
|
||||
if (!files) {
|
||||
files = new Set(opts.symbolFiles(t));
|
||||
definersOf.set(t, files);
|
||||
}
|
||||
for (const f of files) if (candidates.has(f)) definers.add(f);
|
||||
}
|
||||
if (definers.size === 0 || definers.size === candidates.size) return null;
|
||||
return matches.filter((m) => definers.has(m));
|
||||
};
|
||||
/** Narrow a span's matches, recording what was set aside; null = over budget. */
|
||||
const pinnableMatches = (span: string, matches: string[]): string[] | null => {
|
||||
const narrowed = matches.length > 1 ? narrowBySymbols(matches) : null;
|
||||
if (narrowed && narrowed.length <= maxMatchesPerSpan) {
|
||||
setAside.push({ span, files: matches.filter((m) => !narrowed.includes(m)) });
|
||||
return narrowed;
|
||||
}
|
||||
return matches.length > maxMatchesPerSpan ? null : matches;
|
||||
};
|
||||
const collectLimit = opts.symbolFiles ? Number.POSITIVE_INFINITY : maxMatchesPerSpan;
|
||||
|
||||
for (let i = 0; i < tokens.length; i++) {
|
||||
if (pinned.length >= maxPins) break;
|
||||
if (candidatesExamined >= MAX_CANDIDATE_SPANS) break;
|
||||
@@ -296,9 +406,10 @@ export function extractQueryPaths(
|
||||
if (!normalized) continue;
|
||||
candidatesExamined++;
|
||||
|
||||
const { matches, ambiguous } = resolveSpan(
|
||||
normalized.toLowerCase(), lowerToOriginal, maxMatchesPerSpan,
|
||||
);
|
||||
const resolved = resolveSpan(normalized.toLowerCase(), lowerToOriginal, collectLimit);
|
||||
const pinnable = resolved.matches.length > 0 ? pinnableMatches(normalized, resolved.matches) : null;
|
||||
const matches = pinnable ?? [];
|
||||
const ambiguous = resolved.ambiguous || (resolved.matches.length > 0 && pinnable === null);
|
||||
if (matches.length > 0) {
|
||||
consumed.add(i);
|
||||
for (const m of matches) {
|
||||
@@ -352,8 +463,9 @@ export function extractQueryPaths(
|
||||
const { path: stripped, lines } = stripWrapping(tokens[i]!);
|
||||
if (stripped.length < 4 || !KEBAB_BASENAME.test(stripped)) continue;
|
||||
basenameStems ??= buildBasenameStems(indexedPaths);
|
||||
const matches = basenameStems.get(stripped.toLowerCase());
|
||||
if (!matches || matches.length > maxMatchesPerSpan) continue;
|
||||
const stemMatches = basenameStems.get(stripped.toLowerCase());
|
||||
const matches = stemMatches ? pinnableMatches(stripped, stemMatches) : null;
|
||||
if (!matches) continue;
|
||||
consumed.add(i);
|
||||
for (const m of matches) {
|
||||
if (pinnedSeen.has(m) || pinned.length >= maxPins) continue;
|
||||
@@ -419,5 +531,10 @@ export function extractQueryPaths(
|
||||
seenAnchor.add(key);
|
||||
return true;
|
||||
}),
|
||||
// A file another span pinned outright (the agent also wrote its full path)
|
||||
// was not set aside after all.
|
||||
setAsideMatches: setAside
|
||||
.map((s) => ({ span: s.span, files: s.files.filter((f) => !pinnedSeen.has(f)) }))
|
||||
.filter((s) => s.files.length > 0),
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user