From 6613a08e0090ae2a6c77842918c8515d0e3bb41d Mon Sep 17 00:00:00 2001 From: Max Hsu Date: Fri, 11 Sep 2026 08:55:05 +0800 Subject: [PATCH] fix(explore): report extension-less paths the index doesn't hold (#1830) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01XDR1wm73oH8J8cRWnKyv9m --- CHANGELOG.md | 2 + __tests__/query-paths.test.ts | 81 +++++++++++++++++++++++++++++++++++ src/mcp/tools.ts | 21 ++++++++- src/search/query-paths.ts | 30 +++++++++++-- 4 files changed, 130 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9b68b091..4f8d47cc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -153,6 +153,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) diff --git a/__tests__/query-paths.test.ts b/__tests__/query-paths.test.ts index f1e3c0e2e..06ea29a5e 100644 --- a/__tests__/query-paths.test.ts +++ b/__tests__/query-paths.test.ts @@ -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'); + }); +}); diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index 42fdd2288..de5fef397 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -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; diff --git a/src/search/query-paths.ts b/src/search/query-paths.ts index c91272ddb..9855c8b7d 100644 --- a/src/search/query-paths.ts +++ b/src/search/query-paths.ts @@ -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); }