Repository navigation
fix(search): apply hard filters before candidate limits - #2254
Closed
rudycelekli wants to merge 3 commits into
Closed
rudycelekli wants to merge 3 commits into
rudycelekli wants to merge 3 commits into
Conversation
Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
Author
|
Updated this PR against current main Tested signed head |
Resolve conflict while preserving the focused fix and upstream history. Native focused regressions pass; full-suite failures remain disclosed.
colbymchenry
added a commit
that referenced
this pull request
Oct 29, 2026
QueryBuilder.searchNodes applied `path:` and `name:` only to its final list, after every candidate source had cut its rows to the limit: full-text search to max(limit*5, 100) rows, the substring fallback to `limit`, the typo fallback to 50 names, a filter-only search to the first limit*5 names in name order, and the exact-name supplement to 20 rows. Whenever enough symbols outside the filters ranked first, nothing the filters asked for was left, so `codegraph query "authenticate path:product/"` found nothing at limits 1, 2 and 10, and found the match at 200. That broke the query parser's contract: filters narrow the set and the search ranks within it. Every source now applies the filters before its own limit and offset. Its statement is streamed and JavaScript decides each row, so matching keeps toLowerCase's Unicode folding and literal substrings (SQLite's lower() and LIKE fold ASCII only, and LIKE reads % and _ as wildcards). A page and its offset count only the rows that pass, and the read stops once the page is full. To spare JavaScript the rows that cannot pass, each statement also carries a SQL pre-filter that keeps every row the filters could pass: instr(lower(...)) for a printable ASCII value, which agrees exactly with JavaScript on printable ASCII text, while text with any other character passes through to JavaScript. The typo fallback looks names up until it has its results instead of giving up after 50 names, and skips names that `name:` rules out. A filter that no symbol's name, or no symbol's file, passes returns nothing at once, checked against the memoized name list and a memoized list of file paths. The old final filter stays as a safety net, now before the cut. A search without these filters runs the same SQL as before, except that a filter-only search now honours its offset (the same rows at offset 0, which every caller in the codebase passes). Validation: - Kit repro through `codegraph query --json`: on main all 18 filtered queries (6 queries at limits 1, 2 and 10) returned nothing; with the fix each returns its match. - 9 new tests in search-field-filters.test.ts; 7 fail on main's source. Mutations caught: each of the five candidate sources left unfiltered, the offset ignored, the non-ASCII pass-through removed or kept only for non-ASCII values, and the JavaScript check removed. - Latency A/B, main vs fix in one process on one index, 5 interleaved rounds of the median of 5 calls: vscode src/vs/base + src/vs/editor (56,406 nodes) and a 10x replica of it (564,060 nodes). The 45 unfiltered queries (explore's kinds at limit 16, plain limit 10) return identical ids and scores: 89.7 vs 88.4 ms and 435.7 vs 441.1 ms in total. On the replica the filtered queries now find their matches in 0.2-26 ms; the slowest are a substring fallback that finds nothing (156 ms, what an unfiltered miss already costs) and `kind:method path:...` (116 ms). A filter nothing passes answers in 0.2 ms instead of reading every row. Reimplements PR #2254 by @rudycelekli. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
colbymchenry
added a commit
that referenced
this pull request
Oct 29, 2026
QueryBuilder.searchNodes applied `path:` and `name:` only to its final list, after every candidate source had cut its rows to the limit: full-text search to max(limit*5, 100) rows, the substring fallback to `limit`, the typo fallback to 50 names, a filter-only search to the first limit*5 names in name order, and the exact-name supplement to 20 rows. Whenever enough symbols outside the filters ranked first, nothing the filters asked for was left, so `codegraph query "authenticate path:product/"` found nothing at limits 1, 2 and 10, and found the match at 200. That broke the query parser's contract: filters narrow the set and the search ranks within it. Every source now applies the filters before its own limit and offset. Its statement is streamed and JavaScript decides each row, so matching keeps toLowerCase's Unicode folding and literal substrings (SQLite's lower() and LIKE fold ASCII only, and LIKE reads % and _ as wildcards). A page and its offset count only the rows that pass, and the read stops once the page is full. To spare JavaScript the rows that cannot pass, each statement also carries a SQL pre-filter that keeps every row the filters could pass: instr(lower(...)) for a printable ASCII value, which agrees exactly with JavaScript on printable ASCII text, while text with any other character passes through to JavaScript. The typo fallback looks names up until it has its results instead of giving up after 50 names, and skips names that `name:` rules out. A filter that no symbol's name, or no symbol's file, passes returns nothing at once, checked against the memoized name list and a memoized list of file paths. The old final filter stays as a safety net, now before the cut. A search without these filters runs the same SQL as before, except that a filter-only search now honours its offset (the same rows at offset 0, which every caller in the codebase passes). Validation: - Kit repro through `codegraph query --json`: on main all 18 filtered queries (6 queries at limits 1, 2 and 10) returned nothing; with the fix each returns its match. - 9 new tests in search-field-filters.test.ts; 7 fail on main's source. Mutations caught: each of the five candidate sources left unfiltered, the offset ignored, the non-ASCII pass-through removed or kept only for non-ASCII values, and the JavaScript check removed. - Latency A/B, main vs fix in one process on one index, 5 interleaved rounds of the median of 5 calls: vscode src/vs/base + src/vs/editor (56,406 nodes) and a 10x replica of it (564,060 nodes). The 45 unfiltered queries (explore's kinds at limit 16, plain limit 10) return identical ids and scores: 89.7 vs 88.4 ms and 435.7 vs 441.1 ms in total. On the replica the filtered queries now find their matches in 0.2-26 ms; the slowest are a substring fallback that finds nothing (156 ms, what an unfiltered miss already costs) and `kind:method path:...` (116 ms). A filter nothing passes answers in 0.2 ms instead of reading every row. Reimplements PR #2254 by @rudycelekli. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
colbymchenry
added a commit
that referenced
this pull request
Oct 29, 2026
…3018) QueryBuilder.searchNodes applied `path:` and `name:` only to its final list, after every candidate source had cut its rows to the limit: full-text search to max(limit*5, 100) rows, the substring fallback to `limit`, the typo fallback to 50 names, a filter-only search to the first limit*5 names in name order, and the exact-name supplement to 20 rows. Whenever enough symbols outside the filters ranked first, nothing the filters asked for was left, so `codegraph query "authenticate path:product/"` found nothing at limits 1, 2 and 10, and found the match at 200. That broke the query parser's contract: filters narrow the set and the search ranks within it. Every source now applies the filters before its own limit and offset. Its statement is streamed and JavaScript decides each row, so matching keeps toLowerCase's Unicode folding and literal substrings (SQLite's lower() and LIKE fold ASCII only, and LIKE reads % and _ as wildcards). A page and its offset count only the rows that pass, and the read stops once the page is full. To spare JavaScript the rows that cannot pass, each statement also carries a SQL pre-filter that keeps every row the filters could pass: instr(lower(...)) for a printable ASCII value, which agrees exactly with JavaScript on printable ASCII text, while text with any other character passes through to JavaScript. The typo fallback looks names up until it has its results instead of giving up after 50 names, and skips names that `name:` rules out. A filter that no symbol's name, or no symbol's file, passes returns nothing at once, checked against the memoized name list and a memoized list of file paths. The old final filter stays as a safety net, now before the cut. A search without these filters runs the same SQL as before, except that a filter-only search now honours its offset (the same rows at offset 0, which every caller in the codebase passes). Validation: - Kit repro through `codegraph query --json`: on main all 18 filtered queries (6 queries at limits 1, 2 and 10) returned nothing; with the fix each returns its match. - 9 new tests in search-field-filters.test.ts; 7 fail on main's source. Mutations caught: each of the five candidate sources left unfiltered, the offset ignored, the non-ASCII pass-through removed or kept only for non-ASCII values, and the JavaScript check removed. - Latency A/B, main vs fix in one process on one index, 5 interleaved rounds of the median of 5 calls: vscode src/vs/base + src/vs/editor (56,406 nodes) and a 10x replica of it (564,060 nodes). The 45 unfiltered queries (explore's kinds at limit 16, plain limit 10) return identical ids and scores: 89.7 vs 88.4 ms and 435.7 vs 441.1 ms in total. On the replica the filtered queries now find their matches in 0.2-26 ms; the slowest are a substring fallback that finds nothing (156 ms, what an unfiltered miss already costs) and `kind:method path:...` (116 ms). A filter nothing passes answers in 0.2 ms instead of reading every row. Reimplements PR #2254 by @rudycelekli. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Owner
|
Thanks, @rudycelekli, for finding that |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
path:andname:filters while collecting FTS, substring, filter-only and exact-name candidates, before unrelated rows consume their budgets.%/_, repeated-filter semantics and the unfiltered SQL limit path.Reproduction
On main
34ede4de(package 1.6.1), index 120authenticate*functions outsideproduct/and oneauthenticateUserHandlerinside it.searchNodes('authenticate path:product/', {limit: 1})returns nothing; a large limit finds the symbol. The MCP search consequently reports “No results found” for an indexed match. Filter-only queries have the same problem when unrelated names fill the initial alphabetical pool.A typo query also loses an eligible definition when more than 50 similar names outside its path fill the fuzzy follow-up cap. This patch applies the hard filters before both row and fuzzy-name budgets. The compiled public API and MCP now return the actual match at limits 1, 2 and 10; genuinely absent filters remain empty.
Validation
npm run buildpassed.search-query-parser.test.ts,context-ranking.test.tsandfts5-fallback.test.ts: 55 tests passed on Node 22.19.0 with native SQLite.The literal substring and repeated-filter contract from #131 is retained. No active duplicate was found in the open PR inventory.
Current-main maintenance (2026-10-03)
Updated this PR against current main
6560052a6f856855d3f71eee838fd66ccfa4285d(release 1.6.2). The only conflict was the release-note promotion: the pending fix stays under Unreleased, and the entire published 1.6.2 changelog section is byte-identical to upstream. Production and test files retain Git's automatic merge result, with no manual changes.Tested signed head
aafccdb1112718f8c82d9a5c88389ce1c0732cd6on Node 22.19.0 with native SQLite and the repository's normal Vitest/global build setup: 55 passed across search parser, context ranking and FTS fallback suites. The completenpm run buildalso passed, including the viewer and all 29 copied grammars. Validation remains scoped to those relevant suites; I have not claimed the full test suite or broader language/framework efficacy. No release/version/tag changes.October 21 conflict reconciliation and validation
Normal merge of
f66d064279c02f483c33602abec2e5ce9e13522a, preserving the focused fix and upstream history. Exact head:fe69b1d3eb0ab6bfccb75a9f329e8523ba34a5b3. The resulting tree also integrates cleanly with current main81fd007850ca8af8b88819d2d33ede4f59aa7361; native validation below is specifically on the frozen merge tree, not that newer integration.All 12 added regression cases passed on both Linux and Windows. Build, library build and all eight exact-tree identity guards passed across the four repairs. Full suites are not green:
Native full-suite run. The Linux dedup failure also reproduces on exact main: 25 passed, 1 failed. A matched Windows baseline 93-case diagnostic reproduced 12 failure pairs with identical test bytes (78 passed, 12 failed, 3 skipped). Other Windows timeouts/daemon teardown failures remain unclassified; the Windows dedup failure was not directly baselined. These are failed tests, not waived or green checks.
Windows proof used a hosted x64 Windows VM, not the maintainer ARM64 VM. Linux proof used an Ubuntu 24.04 hosted VM, not the Docker recipe. New upstream CI and maintainer review remain pending; no merge-readiness claim. AI assistance was used for reconciliation, investigation and validation.