From 8f2e556def640e73ce58d163ceb9bcb3849d7824 Mon Sep 17 00:00:00 2001 From: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com> Date: Thu, 1 Oct 2026 08:02:28 -0400 Subject: [PATCH] fix(search): apply hard filters before candidate limits Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com> --- CHANGELOG.md | 1 + __tests__/search-query-parser.test.ts | 114 ++++++++++++++++++++- src/db/queries.ts | 137 ++++++++++++++++---------- 3 files changed, 200 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2eaa9f0bee..f3f6ffa959 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In Objective-C, `[super init]` and other messages to `super` now reach the superclass's method, never the sending class's own override. This also works where the index lost the `@implementation`'s range. On SDWebImage, `NSObject` subclasses' `[super init]` used to land on `SDDiskCache`'s `init`, and `[self class]` in tests on a proxy class's `class`. On AFNetworking, a serializer's `[super encodeWithCoder:]` now reaches its superclass. Re-index after upgrading. - In Solidity, a call with no receiver now reaches a function of the contract around it or of one it inherits. On OpenZeppelin, `_msgSender()` in contracts built on `Context` used to land on `ERC2771Context`'s override 83 times. In Erlang, a call to another module's function now follows the file's `-import(Mod, [f/N])`. On cowboy, `-import(req_SUITE, [do_get/3])` calls used to land on another suite's `do_get/3`. A bare call with no such import no longer links to some other module. Re-index after upgrading. - Calls on a library's own types and modules no longer land on a project method that happens to share the name. In Rust, `Vec::new()`, `String::from(…)` and module paths like `task::spawn(…)` stay the standard library's. A later link of a multi-line Rust or Go chain now counts as a chained call, named after the chain's first line. In Go, `w.Header().Get(…)` is net/http's. In R, a plain call like `range(x)` is a function, not an object's method. In JavaScript, `response.text()` / `.json()` is fetch's. On tokio, `task::spawn(…)` used to land on `TaskTracker::spawn` 300 times. On ggplot2, `range(…)` used to land on a coordinate system's `range` method. Re-index after upgrading. +- Searches with `path:` or `name:` filters now find matching symbols before unrelated results consume the limit, including typo searches and queries containing only filters. - In C and C++, a qualified name like `fmt::format` now reaches a declaration inside a namespace the library opens with a macro, like fmt's `FMT_BEGIN_NAMESPACE`, pybind11's `PYBIND11_NAMESPACE_BEGIN(…)` or rapidjson's and spdlog's `…_NAMESPACE_BEGIN`. It also works through a namespace alias like `namespace py = pybind11;`. On fmt, 1,728 `fmt::format(…)` calls used to link to nothing, and so did about 1,400 `py::…` calls on pybind11. - A method guessed from a receiver's name no longer lands on a request handler the web framework dispatches to, such as a Django or DRF view's `post` / `get` or a controller's `update` / `destroy`. It also no longer lands on a test double the calling file never mentions. On django-allauth, tests' `client.post(…)` used to land on a `ClientRegistrationView` 425 times. On mealie, `response.json()` used to land on a test's `_FakeHTTPResponse` 424 times. In Python, `site = Site.objects.create(…)` now makes `site` a `Site`, so `site.save()` reaches the model's own `save`. Re-index after upgrading. - Kotlin infix calls, like `Users.id eq id1` or koin's `single { … } bind MyApi::class`, are now recorded as calls to the infix function. Before, they had no callers at all. On Exposed, `eq` now shows over a thousand callers. Bit operations on numbers (`x and 0xff`, `h shr 8`) are left to Kotlin's own operators, even where the project defines a same-named extension. Re-index after upgrading. diff --git a/__tests__/search-query-parser.test.ts b/__tests__/search-query-parser.test.ts index 8a7767dac3..acba91ae27 100644 --- a/__tests__/search-query-parser.test.ts +++ b/__tests__/search-query-parser.test.ts @@ -4,7 +4,12 @@ * `name:` filtering and the fuzzy typo fallback. */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import CodeGraph from '../src'; +import { ToolHandler } from '../src/mcp/tools'; import { parseQuery, boundedEditDistance } from '../src/search/query-parser'; describe('parseQuery', () => { @@ -140,3 +145,110 @@ describe('boundedEditDistance', () => { expect(boundedEditDistance('aaaaa', 'bbbbb', 2)).toBe(3); }); }); + +describe('search hard filters precede candidate limits', () => { + let root: string; + let cg: CodeGraph; + const names = (query: string, limit = 1) => cg.searchNodes(query, { limit, kinds: ['function'] }).map(r => r.node.name); + + beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-search-filter-')); + const write = (file: string, source: string) => { + fs.mkdirSync(path.dirname(path.join(root, file)), { recursive: true }); + fs.writeFileSync(path.join(root, file), source); + }; + const filler = Array.from({ length: 120 }, (_, i) => [ + `export function authenticate${i}() { return ${i}; }`, + `export function aaaNeedle${i}() { return ${i}; }`, + ].join('\n')); + // Distinct names with the same folded spelling occupy the fuzzy name cap. + const variants = Array.from({ length: 64 }, (_, mask) => 'G' + [...'etUser'].map((c, i) => + mask & (1 << i) ? c.toUpperCase() : c.toLowerCase(), + ).join('')); + write('aaa/noise.ts', filler.join('\n') + '\nexport function authenticate() {}\n' + + variants.map(n => `export function ${n}() {}`).join('\n')); + write('product/handler.ts', [ + 'export function authenticateUserHandler() {}', + 'export function zzNeedleHandler() {}', + 'export function zzzRareHandler() {}', + 'export function getUser() {}', + ].join('\n')); + write('Ünicode/handler.ts', 'export function Öffnen() {}'); + write('percent%/unit_/literal.ts', 'export function percent_name() {}'); + write('percentA/unitA/decoy.ts', 'export function percentAname() {}'); + cg = await CodeGraph.init(root, { index: true }); + }); + + afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); + }); + + it.each([1, 2, 10])('finds a selective FTS path/name hit with limit %i', limit => { + expect(cg.getBackend()).toBe('node-sqlite'); + expect(names('authenticate path:product/', limit)).toEqual(['authenticateUserHandler']); + expect(names('authenticate name:UserHandler', limit)).toEqual(['authenticateUserHandler']); + }); + + it('finds filter-only hits beyond the alphabetically first candidates', () => { + expect(names('kind:function name:zzzRareHandler')).toEqual(['zzzRareHandler']); + expect(names('kind:function path:product/', 10)).toHaveLength(4); + }); + + it('filters LIKE fallback before its limit', () => { + expect(names('needle path:product/')).toEqual(['zzNeedleHandler']); + expect(names('needle name:zzNeedleHandler')).toEqual(['zzNeedleHandler']); + }); + + it('filters fuzzy definitions before per-name and follow-up caps', () => { + expect(names('getUsar path:product/')).toEqual(['getUser']); + expect(names('getUsar name:getUser')).toHaveLength(1); + }); + + it('preserves Unicode case-insensitive literal substring matching', () => { + expect(names('kind:function path:ünicode/ name:öffnen')).toEqual(['Öffnen']); + }); + + it('treats percent signs and underscores literally', () => { + expect(names('kind:function path:percent%/ name:percent_')).toEqual(['percent_name']); + expect(names('kind:function path:unit_/', 10)).toEqual(['percent_name']); + expect(names('kind:function name:percent_', 10)).toEqual(['percent_name']); + }); + + it('ORs repeated filters within each family and ANDs the two families', () => { + expect(names('kind:function path:product/ path:ünicode/ name:zzzRareHandler name:öffnen', 10).sort()) + .toEqual(['zzzRareHandler', 'Öffnen'].sort()); + expect(names('kind:function path:product/ name:öffnen', 10)).toEqual([]); + }); + + it('keeps exact matches first in unfiltered search and avoids supplement leakage', () => { + expect(names('authenticate')).toEqual(['authenticate']); + expect(names('authenticate path:product/')).toEqual(['authenticateUserHandler']); + expect(names('authenticate path:absent/', 100)).toEqual([]); + expect(names('authenticate name:NoSuchHandler', 100)).toEqual([]); + }); + + it('skips filtered candidates, rather than unrelated rows, for offset', () => { + const all = cg.searchNodes('kind:function path:product/', { limit: 10 }); + const page = cg.searchNodes('kind:function path:product/', { limit: 1, offset: 1 }); + expect(page[0]!.node.id).toBe(all[1]!.node.id); + }); + + it('returns a real MCP match and success-shaped guidance only for an absent filter', async () => { + const prior = process.env.CODEGRAPH_MCP_TOOLS; + process.env.CODEGRAPH_MCP_TOOLS = 'search'; + const handler = new ToolHandler(cg); + try { + const result = await handler.execute('codegraph_search', { query: 'authenticate path:product/', limit: 1 }); + expect(result.isError).not.toBe(true); + expect(JSON.stringify(result.content)).toContain('authenticateUserHandler'); + const absent = await handler.execute('codegraph_search', { query: 'authenticate path:absent/', limit: 1 }); + expect(absent.isError).not.toBe(true); + expect(JSON.stringify(absent.content)).toContain('No results found'); + } finally { + await handler.closeAll(); + if (prior === undefined) delete process.env.CODEGRAPH_MCP_TOOLS; + else process.env.CODEGRAPH_MCP_TOOLS = prior; + } + }); +}); diff --git a/src/db/queries.ts b/src/db/queries.ts index 443b851372..663c97e39a 100644 --- a/src/db/queries.ts +++ b/src/db/queries.ts @@ -231,6 +231,12 @@ function rowToFileRecord(row: FileRow): FileRecord { }; } +interface SearchFieldFilters { + /** Lowercased literal substrings, OR within each family and AND across families. */ + pathFilters?: string[]; + nameFilters?: string[]; +} + /** * Query builder for the knowledge graph database */ @@ -1466,8 +1472,9 @@ export class QueryBuilder { parsed.languages.length > 0 ? Array.from(new Set([...(options.languages ?? []), ...parsed.languages])) : options.languages; - const pathFilters = parsed.pathFilters; - const nameFilters = parsed.nameFilters; + const pathFilters = parsed.pathFilters.map(p => p.toLowerCase()); + const nameFilters = parsed.nameFilters.map(n => n.toLowerCase()); + const fieldFilters = { pathFilters, nameFilters }; // The text portion drives FTS/LIKE; if all the user typed was // filters (`kind:function`), we still need *some* candidate set, // so synthesise an empty-text path that returns everything matching @@ -1478,16 +1485,14 @@ export class QueryBuilder { // First try FTS5 with prefix matching (skip if FTS5 not available, #1532) let results = text - ? (this._fts5Available !== false ? this.searchNodesFTS(text, { kinds, languages, limit, offset }) : []) - // Over-fetch by 5× when running filter-only (no text). The - // post-scoring path: + name: filters can be very selective, so - // a smaller multiplier risks returning fewer than `limit` - // results despite the DB having plenty of matches. - : this.searchAllByFilters({ kinds, languages, limit: limit * 5 }); + ? (this._fts5Available !== false ? this.searchNodesFTS(text, { kinds, languages, limit, offset, ...fieldFilters }) : []) + // Keep the existing ranking pool, counting only rows that satisfy the + // explicit filters so unrelated symbols cannot consume that pool. + : this.searchAllByFilters({ kinds, languages, limit: limit * 5, offset, ...fieldFilters }); // If no FTS results, try LIKE-based substring search if (results.length === 0 && text.length >= 2) { - results = this.searchNodesLike(text, { kinds, languages, limit, offset }); + results = this.searchNodesLike(text, { kinds, languages, limit, offset, ...fieldFilters }); } // Final fuzzy fallback: scan all known names and keep those within @@ -1495,7 +1500,7 @@ export class QueryBuilder { // returned nothing AND there's a text portion long enough to be // worth fuzzing (1-char queries would match too much). if (results.length === 0 && text.length >= 3) { - results = this.searchNodesFuzzy(text, { kinds, languages, limit }); + results = this.searchNodesFuzzy(text, { kinds, languages, limit, ...fieldFilters }); } // Supplement: ensure exact name matches are always candidates. @@ -1532,8 +1537,7 @@ export class QueryBuilder { sql += ` AND language IN (${languages.map(() => '?').join(',')})`; params.push(...languages); } - sql += ' LIMIT 20'; - const rows = this.db.prepare(sql).all(...params) as NodeRow[]; + const rows = this.readSearchRows(sql, params, { limit: 20, ...fieldFilters }); for (const row of rows) { if (!existingIds.has(row.id)) { results.push({ node: rowToNode(row), score: maxFtsScore }); @@ -1571,40 +1575,56 @@ export class QueryBuilder { } } - // Apply path: + name: filters AFTER scoring. Scoring already uses - // path/name as a soft signal; the explicit filters here are a hard - // gate. Done last so the FTS limit fetched plenty of candidates to - // narrow from. - if (pathFilters.length > 0) { - const lowered = pathFilters.map((p) => p.toLowerCase()); - results = results.filter((r) => { - const fp = r.node.filePath.toLowerCase(); - return lowered.some((p) => fp.includes(p)); - }); + return results; + } + + /** + * Apply hard field filters before a candidate consumes the search budget. + * Iteration keeps the selected pool bounded even for a very selective + * filter. JavaScript's case folding and literal substring semantics are + * preserved: SQLite's lower() only folds ASCII, and LIKE treats %/_ as + * wildcards. The unfiltered hot path still uses SQL LIMIT/OFFSET directly. + */ + private readSearchRows( + sql: string, + params: (string | number)[], + options: SearchOptions & SearchFieldFilters, + ): T[] { + const { limit = 100, offset = 0, pathFilters = [], nameFilters = [] } = options; + if (pathFilters.length === 0 && nameFilters.length === 0) { + return this.db.prepare(`${sql} LIMIT ? OFFSET ?`).all(...params, limit, offset) as T[]; } - if (nameFilters.length > 0) { - const lowered = nameFilters.map((n) => n.toLowerCase()); - results = results.filter((r) => { - const nm = r.node.name.toLowerCase(); - return lowered.some((n) => nm.includes(n)); - }); + if (limit === 0) return []; + + const rows: T[] = []; + let skipped = 0; + for (const row of this.db.prepare(sql).iterate(...params) as IterableIterator) { + if (!this.matchesSearchFieldFilters(row, options)) continue; + if (skipped++ < offset) continue; + rows.push(row); + if (limit > 0 && rows.length >= limit) break; } + return rows; + } - return results; + private matchesSearchFieldFilters(row: Pick, filters: SearchFieldFilters): boolean { + const { pathFilters = [], nameFilters = [] } = filters; + return (pathFilters.length === 0 || pathFilters.some(p => row.file_path.toLowerCase().includes(p))) + && (nameFilters.length === 0 || nameFilters.some(n => row.name.toLowerCase().includes(n))); } /** * Match-everything path used when the user supplied only field * filters (`kind:function lang:typescript`) with no text. Returns - * candidates ordered by name; the caller's filter pass narrows to - * what was asked for. + * candidates ordered by name, applying field filters before the budget. */ private searchAllByFilters(options: { kinds?: NodeKind[]; languages?: Language[]; limit: number; - }): SearchResult[] { - const { kinds, languages, limit } = options; + offset?: number; + } & SearchFieldFilters): SearchResult[] { + const { kinds, languages } = options; let sql = 'SELECT * FROM nodes WHERE 1=1'; const params: (string | number)[] = []; if (kinds && kinds.length > 0) { @@ -1615,9 +1635,8 @@ export class QueryBuilder { sql += ` AND language IN (${languages.map(() => '?').join(',')})`; params.push(...languages); } - sql += ' ORDER BY name LIMIT ?'; - params.push(limit); - const rows = this.db.prepare(sql).all(...params) as NodeRow[]; + sql += ' ORDER BY name'; + const rows = this.readSearchRows(sql, params, options); return rows.map((row) => ({ node: rowToNode(row), score: 1 })); } @@ -1631,7 +1650,7 @@ export class QueryBuilder { */ private searchNodesFuzzy( text: string, - options: { kinds?: NodeKind[]; languages?: Language[]; limit: number } + options: { kinds?: NodeKind[]; languages?: Language[]; limit: number } & SearchFieldFilters ): SearchResult[] { const { kinds, languages, limit } = options; const lowered = text.toLowerCase(); @@ -1641,7 +1660,27 @@ export class QueryBuilder { // by getAllNodeNames(); even on a 200k-node project the distinct // name set is typically O(10k) because most names repeat. The // candidate-cap below bounds memory regardless. - const allNames = this.getAllNodeNames(); + let allNames = this.getAllNodeNames(); + if ((options.pathFilters?.length ?? 0) > 0 || (options.nameFilters?.length ?? 0) > 0) { + // Establish scope in one streaming read BEFORE the per-name query cap. + // Otherwise many similar names in unrelated files can consume that cap + // even though none of their definitions satisfies the hard filters. + let sql = 'SELECT DISTINCT name, file_path FROM nodes WHERE 1=1'; + const params: (string | number)[] = []; + if (kinds && kinds.length > 0) { + sql += ` AND kind IN (${kinds.map(() => '?').join(',')})`; + params.push(...kinds); + } + if (languages && languages.length > 0) { + sql += ` AND language IN (${languages.map(() => '?').join(',')})`; + params.push(...languages); + } + const eligibleNames = new Set(); + for (const row of this.db.prepare(sql).iterate(...params) as IterableIterator>) { + if (this.matchesSearchFieldFilters(row, options)) eligibleNames.add(row.name); + } + allNames = allNames.filter(name => eligibleNames.has(name)); + } const candidates: Array<{ name: string; dist: number }> = []; for (const name of allNames) { const dist = boundedEditDistance(name.toLowerCase(), lowered, maxDist); @@ -1671,8 +1710,7 @@ export class QueryBuilder { sql += ` AND language IN (${languages.map(() => '?').join(',')})`; params.push(...languages); } - sql += ' LIMIT 5'; - const rows = this.db.prepare(sql).all(...params) as NodeRow[]; + const rows = this.readSearchRows(sql, params, { ...options, limit: 5 }); for (const row of rows) { if (seen.has(row.id)) continue; seen.add(row.id); @@ -1733,8 +1771,8 @@ export class QueryBuilder { /** * FTS5 search with prefix matching */ - private searchNodesFTS(query: string, options: SearchOptions): SearchResult[] { - const { kinds, languages, limit = 100, offset = 0 } = options; + private searchNodesFTS(query: string, options: SearchOptions & SearchFieldFilters): SearchResult[] { + const { kinds, languages, limit = 100 } = options; // Add prefix wildcard for better matching (e.g., "auth" matches "AuthService", "authenticate") // Escape special FTS5 characters and add prefix wildcard. @@ -1783,11 +1821,10 @@ export class QueryBuilder { params.push(...languages); } - sql += ' ORDER BY score LIMIT ? OFFSET ?'; - params.push(ftsLimit, offset); + sql += ' ORDER BY score'; try { - const rows = this.db.prepare(sql).all(...params) as (NodeRow & { score: number })[]; + const rows = this.readSearchRows(sql, params, { ...options, limit: ftsLimit }); return rows.map((row) => ({ node: rowToNode(row), score: Math.abs(row.score), // bm25 returns negative scores @@ -1802,8 +1839,8 @@ export class QueryBuilder { * LIKE-based substring search for cases where FTS doesn't match * Useful for camelCase matching (e.g., "signIn" finds "signInWithGoogle") */ - private searchNodesLike(query: string, options: SearchOptions): SearchResult[] { - const { kinds, languages, limit = 100, offset = 0 } = options; + private searchNodesLike(query: string, options: SearchOptions & SearchFieldFilters): SearchResult[] { + const { kinds, languages } = options; let sql = ` SELECT nodes.*, @@ -1847,10 +1884,8 @@ export class QueryBuilder { params.push(...languages); } - sql += ' ORDER BY score DESC, length(name) ASC LIMIT ? OFFSET ?'; - params.push(limit, offset); - - const rows = this.db.prepare(sql).all(...params) as (NodeRow & { score: number })[]; + sql += ' ORDER BY score DESC, length(name) ASC'; + const rows = this.readSearchRows(sql, params, options); return rows.map((row) => ({ node: rowToNode(row),