fix(core): restrict the vector filter pass to its own candidates - #1443
Merged
Conversation
A filtered vector or hybrid query cannot evaluate a structured filter against embeddings, so it asked an FTS-mode pass which candidates the filter admits. That pass requested a page of the filter's *whole* match set, capped at VECTOR_FILTER_SCAN_LIMIT (50,000), and every candidate outside the page was then intersected away as disallowed. The loss was not arbitrary. A filter-only pass carries search_text=None, so there is no relevance signal in the ordering: Postgres fell through to its search_index.id ASC tiebreak and kept the earliest-indexed rows, and SQLite had no tiebreak at all. Newer content was what disappeared, identically on every rerun — a result-completeness bug, not a derived-state race that a later write or index pass repairs. Push the candidate keys into the filter query instead. The pass now asks which of *these* rows the filter admits, so its answer is bounded by the question and the cap is no longer needed on that path (VECTOR_FILTER_SCAN_LIMIT still bounds the adapter rescan it was written for). Both backends build the restriction from one shared helper, following file_path_prefix_condition: a restriction that admitted different rows per dialect would hand semantic search a different candidate set depending on the database underneath. The key list is split at the existing VECTOR_HYDRATION_BATCH_SIZE bound, since both engines cap bind parameters. Keys are grouped by row type so each branch is one type-scoped IN list — one bind per key rather than two, and a shape both planners drive from the index. Scoped to the whole intersection, not one filter: all ten filters share the single call site, and fixing the newest alone would have left the more reachable ones broken while making the path look safe. Measured on a 50,001-row project (both backends, best of three): old capped page 522 ms (pg) / 592 ms (sqlite), 50,000 rows hydrated 100 candidates 27 ms (pg) / 30 ms (sqlite), one round trip 250 candidates 33 ms (pg) / 32 ms (sqlite), one round trip 1000 candidates 112 ms (pg) / 130 ms (sqlite), four round trips Postgres plans the restricted form as an Index Scan on search_index_pkey (0.8 ms) where the old page was a Seq Scan plus an external merge sort spilling 1.6 MB to disk (70 ms). A default query's candidate pool is ~100 rows, so the common case keeps its single round trip; only deep pages add any. Closes #1431. Refs #1438. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp Signed-off-by: phernandez <paul@basicmachines.co>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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.
Closes #1431. Part of #1438.
The defect
A filtered vector or hybrid query cannot evaluate a structured filter against embeddings, so it asked an FTS-mode pass which of its candidates the filter admits. That pass requested a page of the filter's whole match set, capped at
VECTOR_FILTER_SCAN_LIMIT(50,000), and every candidate outside the page was intersected away as disallowed.The loss is not arbitrary. A filter-only pass carries
search_text=None, so there is no relevance signal in the ordering: PostgreSQL falls through to itssearch_index.id ASCtiebreak and keeps the earliest-indexed rows; SQLite has no tiebreak at all. Newer content is what disappears, identically on every rerun — result-completeness, not a derived-state race that a later write or index pass repairs.Reproduction
A project with 50,001 rows under one
file_path_prefix, one genuinely matching row indexed last, and a vector hit on that row. Verified on both backends before touching any source:Identical on SQLite and PostgreSQL (
BASIC_MEMORY_TEST_POSTGRES=1).The fix
Push the candidate keys into the filter query. The pass now asks which of these rows the filter admits, so its answer is bounded by the question and no cap is needed on that path.
VECTOR_FILTER_SCAN_LIMITstays where it belongs — bounding the adapter rescan it was written for.candidate_keyson the abstractsearch(), theSearchRepositoryprotocol, and both backends.candidate_key_restriction_condition, followingfile_path_prefix_conditionandmetadata_filter_content_type_condition. A restriction that admitted different rows per dialect would hand semantic search a different candidate set depending on which database is underneath.INlist: one bind per key rather than two, and a shape both planners drive from the index.VECTOR_HYDRATION_BATCH_SIZE(250) bound, since both engines cap bind parameters — the same bound the file already uses for manifest hydration, not a second one. (reindex fails on Postgres projects above ~32k entities: unchunked find_by_ids #1442 is the live instance of that cap on another path.)Scoped to the whole intersection rather than one filter: all ten filters share the single call site, and fixing only the newest would have left the more reachable ones broken while making the path look safe.
The two rejected alternatives from the issue are still rejected — exhausting the pages removes the safety valve, and raising the constant moves the cliff.
Performance
Measured on a 50,001-row project, best of three, both backends:
PostgreSQL plans the restricted form as an
Index Scan using search_index_pkeywithid = ANY(...)— 0.8 ms in-database — where the old page was aSeq Scanplus an external merge sort spilling 1.6 MB to disk (70 ms).Round trips: a default query's candidate pool is ~100 rows (
semantic_vector_k=100), so the common case keeps its single round trip. Batching only adds round trips on deep pages, and four cheap indexed lookups still beat one unbounded scan by ~4.6x.Tests
tests/repository/test_vector_filter_candidate_restriction.py— the two database tests seed a genuinely over-window project (50,001 rows) rather than mocking the limit, and assert the matching row is returned:Fail-before / pass-after confirmed by reverting the intersection hunk: the two database tests fail on both backends, and pass with it restored.
Verification
uv run ruff check src tests test-int— cleanuv run ruff format --check .— cleanuv run ty check src tests test-int— cleantests/repository tests/api tests/serviceson SQLite — 1828 passed, 40 skippedtests/repository tests/api tests/serviceson PostgreSQL — 1808 passed, 60 skipped🤖 Generated with Claude Code
https://claude.ai/code/session_014pmKq6bqCi6Zp6BTHuZjrp