mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-18 13:53:01 +08:00
A query span like `scripts/deploy` was neither pinned nor reported. `isClearlyPathShaped` demands a dot-extension on the last segment, so a dotless slashed span that resolved to no indexed file fell through every arm: it was not consumed, never entered `unresolvedPathSpans`, and its fragments (`scripts`, `deploy`) went on to feed FTS. The agent got a pile of unrelated source with no hint that the file it named was never read. Both caveat sinks — the empty-subgraph note and the summary line — were already in place; they were simply never handed the span. Shape alone cannot decide this: `and/or`, `input/output` and `client/server` have exactly the same shape, and loosening the shape test to "has a slash" strips that prose out of every query and mints a false "No indexed file uniquely matches `and/or`" caveat (the existing `leaves slash-bearing non-paths alone` test pins this). The file's existence on disk is the discriminator instead — and since query-paths.ts is deliberately pure (no DB, no fs, safe in the query-pool workers), the check is injected by the caller as an `existsOnDisk` predicate. The call site in tools.ts owns the project root and runs it through `validatePathWithinRoot`, so a `../` span cannot probe outside the project, and only a regular file counts — a directory span keeps flowing to the normal matching pipeline. Retrieval effect (AGENTS.md "do not regress"), measured as a deterministic A/B on the built dist over an indexed repro, baseline = upstream/main: - query that is only the dotless path — before: 998 chars of unrelated source, no caveat; after: `No relevant code found ... (no indexed file uniquely matches `scripts/deploy`)`. - same path inside a real symbol query — output byte-identical except the added caveat sentence (+51 chars); the same 4 symbols across 2 files. - query with no path span — byte-identical between arms. Claude-Session: https://claude.ai/code/session_01XDR1wm73oH8J8cRWnKyv9m Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -169,6 +169,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
|
||||
- The prompt hook no longer injects unrelated projects when run from your home directory or a broader directory containing a stray workspace manifest. (#1454)
|
||||
|
||||
- `codegraph_explore` now says so when a query names an extension-less file the index doesn't hold. A path like `scripts/deploy` has no extension on its last segment, so it failed the shape test that decides a span is a path beyond doubt — the name was left in the query, shredded into `scripts` and `deploy`, and the answer came back as a pile of unrelated source with no hint that the file you named was never consulted. Such a span is now checked against the project directory: if it is a real file, the answer carries the same `No indexed file uniquely matches ...` note a misspelled `src/foo.ts` already got. Slashed prose — `and/or`, `input/output`, `gen_server:call/2` — has no file behind it and is still left in the query untouched. (#1830)
|
||||
|
||||
- Indexing now succeeds when Node.js's SQLite lacks FTS5, with search falling back to name and fuzzy matching; thanks @aniruddhaadak80. (#1532)
|
||||
|
||||
- `codegraph_explore` now makes clear that suggested call counts are advisory, so agents keep exploring when an answer is incomplete; thanks @rongbc. (#1504, #1570)
|
||||
|
||||
@@ -236,3 +236,84 @@ describe('extractQueryPaths — extension-less kebab basenames', () => {
|
||||
expect(out.strippedQuery).toBe('background-image-table then');
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Dotless slashed spans (#1830). `scripts/deploy` names a real file that the
|
||||
* index does not hold (no recognized extension), but the shape test demands a
|
||||
* dot-extension on the last segment — so the span was neither pinned NOR
|
||||
* reported, and its fragments (`scripts`, `deploy`) went on to feed FTS. The
|
||||
* agent got a pile of unrelated source with no hint that the file it named was
|
||||
* never consulted. Shape alone cannot decide this (`and/or` is the same shape),
|
||||
* so the caller injects an `existsOnDisk` predicate and the file's existence
|
||||
* decides. These tests also pin that the predicate is genuinely CONSULTED —
|
||||
* a predicate that is never called would make the whole arm vacuous.
|
||||
*/
|
||||
describe('extractQueryPaths — dotless slashed spans, decided on disk', () => {
|
||||
/** Records every span the predicate is asked about. */
|
||||
const probe = (onDisk: readonly string[]) => {
|
||||
const asked: string[] = [];
|
||||
return {
|
||||
asked,
|
||||
existsOnDisk: (rel: string) => { asked.push(rel); return onDisk.includes(rel); },
|
||||
};
|
||||
};
|
||||
|
||||
it('reports a dotless path that exists on disk but is not indexed', () => {
|
||||
const p = probe(['scripts/deploy']);
|
||||
const out = extractQueryPaths(
|
||||
'why does scripts/deploy fail on release', INDEX, { existsOnDisk: p.existsOnDisk },
|
||||
);
|
||||
expect(out.pinnedFiles).toEqual([]);
|
||||
expect(out.unresolvedPathSpans).toEqual(['scripts/deploy']);
|
||||
expect(out.strippedQuery).toBe('why does fail on release');
|
||||
// Vacuity guard: the verdict came from the predicate, not from some other arm.
|
||||
expect(p.asked).toContain('scripts/deploy');
|
||||
});
|
||||
|
||||
it('leaves the same span alone when no predicate is injected', () => {
|
||||
const q = 'why does scripts/deploy fail on release';
|
||||
const out = extractQueryPaths(q, INDEX);
|
||||
expect(out.unresolvedPathSpans).toEqual([]);
|
||||
expect(out.strippedQuery).toBe(q);
|
||||
});
|
||||
|
||||
it('leaves `and/or` prose alone even though a predicate is injected', () => {
|
||||
const p = probe(['scripts/deploy']);
|
||||
const q = 'does gen_server:call/2 block and/or timeout';
|
||||
const out = extractQueryPaths(q, INDEX, { existsOnDisk: p.existsOnDisk });
|
||||
expect(out.pinnedFiles).toEqual([]);
|
||||
expect(out.unresolvedPathSpans).toEqual([]);
|
||||
expect(out.strippedQuery).toBe(q);
|
||||
// Consulted and refused — not skipped by shape.
|
||||
expect(p.asked).toContain('and/or');
|
||||
});
|
||||
|
||||
it('leaves a slashed word pair that is not a file on disk alone', () => {
|
||||
const p = probe(['scripts/deploy']);
|
||||
const q = 'trace the input/output buffering path';
|
||||
const out = extractQueryPaths(q, INDEX, { existsOnDisk: p.existsOnDisk });
|
||||
expect(out.unresolvedPathSpans).toEqual([]);
|
||||
expect(out.strippedQuery).toBe(q);
|
||||
expect(p.asked).toContain('input/output');
|
||||
});
|
||||
|
||||
it('still reports a dotted span that matches nothing and is not on disk', () => {
|
||||
const p = probe([]);
|
||||
const out = extractQueryPaths(
|
||||
'crash in src/routes/gone/missing-page.svelte on load', INDEX,
|
||||
{ existsOnDisk: p.existsOnDisk },
|
||||
);
|
||||
expect(out.unresolvedPathSpans).toEqual(['src/routes/gone/missing-page.svelte']);
|
||||
expect(out.strippedQuery).toBe('crash in on load');
|
||||
});
|
||||
|
||||
it('pins an indexed dotless path by resolution, never asking about it', () => {
|
||||
const p = probe(['scripts/pre-commit']);
|
||||
const out = extractQueryPaths(
|
||||
'what does scripts/pre-commit run', INDEX, { existsOnDisk: p.existsOnDisk },
|
||||
);
|
||||
expect(out.pinnedFiles).toEqual(['scripts/pre-commit']);
|
||||
expect(out.unresolvedPathSpans).toEqual([]);
|
||||
expect(p.asked).not.toContain('scripts/pre-commit');
|
||||
});
|
||||
});
|
||||
|
||||
+20
-1
@@ -157,6 +157,25 @@ export function normalizeQuerySpelling(query: string): string {
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Does this query-named span point at a real FILE inside the project?
|
||||
*
|
||||
* The `existsOnDisk` predicate `extractQueryPaths` takes (that module is pure —
|
||||
* no DB, no fs — so the fs access lives here, where the project root is known).
|
||||
* Only a REGULAR FILE counts: a directory span (`src/search`) is not a file
|
||||
* reference and must keep flowing to the normal matching pipeline. Containment
|
||||
* is enforced by `validatePathWithinRoot`, so a `../` span in a query cannot
|
||||
* probe outside the project, and every fs error answers `false`.
|
||||
*/
|
||||
function pathIsProjectFile(projectRoot: string, relPath: string): boolean {
|
||||
try {
|
||||
const abs = validatePathWithinRoot(projectRoot, relPath);
|
||||
return abs !== null && statSync(abs).isFile();
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Calculate the recommended number of codegraph_explore calls based on project size.
|
||||
* Larger codebases need more exploration calls to cover their surface area,
|
||||
@@ -3318,7 +3337,7 @@ export class ToolHandler {
|
||||
const extraction = extractQueryPaths(
|
||||
rawQuery,
|
||||
cg.getFiles().map((f) => f.path),
|
||||
{ maxPins: maxFiles },
|
||||
{ maxPins: maxFiles, existsOnDisk: (rel) => pathIsProjectFile(projectRoot, rel) },
|
||||
);
|
||||
if (extraction.pinnedFiles.length > 0 || extraction.unresolvedPathSpans.length > 0) {
|
||||
pinnedFiles = extraction.pinnedFiles;
|
||||
|
||||
@@ -24,7 +24,11 @@
|
||||
* spans removed.
|
||||
* Callers treat pinned files as first-class: guaranteed admission, top rank,
|
||||
* funded first. Pure string work — no DB, no fs — so it is trivially testable
|
||||
* and safe inside the query-pool workers.
|
||||
* and safe inside the query-pool workers. The one question string shape cannot
|
||||
* answer (is this dotless slashed span, `scripts/deploy`, a real file that is
|
||||
* merely unindexed, or is it prose like `and/or`?) is delegated to an OPTIONAL
|
||||
* `existsOnDisk` predicate the caller injects — the fs access stays at the call
|
||||
* site, which owns the project root, and this module stays pure.
|
||||
*/
|
||||
|
||||
export interface QueryPathExtraction {
|
||||
@@ -189,7 +193,18 @@ function resolveSpan(
|
||||
export function extractQueryPaths(
|
||||
query: string,
|
||||
indexedPaths: readonly string[],
|
||||
opts: { maxPins?: number; maxMatchesPerSpan?: number } = {},
|
||||
opts: {
|
||||
maxPins?: number;
|
||||
maxMatchesPerSpan?: number;
|
||||
/**
|
||||
* Does this repo-relative span name a real FILE in the project? Optional,
|
||||
* injected by the caller (see the module docstring): it is the only way to
|
||||
* tell a dotless path the index simply doesn't hold (`scripts/deploy`)
|
||||
* from slashed prose (`and/or`), and it must stay out of this module.
|
||||
* Must not throw — the caller absorbs fs errors and returns false.
|
||||
*/
|
||||
existsOnDisk?: (relPath: string) => boolean;
|
||||
} = {},
|
||||
): QueryPathExtraction {
|
||||
const maxPins = Math.max(1, opts.maxPins ?? 8);
|
||||
const maxMatchesPerSpan = Math.max(1, opts.maxMatchesPerSpan ?? 3);
|
||||
@@ -235,10 +250,19 @@ export function extractQueryPaths(
|
||||
pinnedSeen.add(m);
|
||||
pinned.push(m);
|
||||
}
|
||||
} else if (ambiguous || isClearlyPathShaped(normalized)) {
|
||||
} else if (
|
||||
ambiguous
|
||||
|| isClearlyPathShaped(normalized)
|
||||
|| (normalized.includes('/') && opts.existsOnDisk?.(normalized) === true)
|
||||
) {
|
||||
// A real path that didn't resolve to a usable set. Keeping it in the
|
||||
// query is strictly worse — its fragments are what minted the junk
|
||||
// matches this module exists to stop — so strip it and say so.
|
||||
// The third arm covers the DOTLESS slashed span (`scripts/deploy`,
|
||||
// `bin/build`): shape alone cannot tell it from `and/or` or
|
||||
// `input/output`, so the file's existence on disk decides. Loosening the
|
||||
// SHAPE test instead would strip that prose out of every query and mint a
|
||||
// false "no indexed file matches `and/or`" caveat.
|
||||
consumed.add(i);
|
||||
if (unresolved.length < 4) unresolved.push(normalized);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user