fix: Filter SQLite semantic candidates before KNN cap

[rupayon123]

Summary

Filtered SQLite semantic search previously applied caller predicates after sqlite-vec's KNN candidate limit. On large indexes, nearby rows outside the filter could exhaust the 4096 ceiling and hide matching rows. This change adds the same candidate predicate to the KNN query, keeps the outer checks, preserves the unfiltered query shape, and raises the declared sqlite-vec minimum to 0.1.6, the lowest version verified here.

Fixes #194.

Plan / claim

Claim: #194 is fixed by applying the SQLite entry filters inside KNN before sqlite-vec applies its candidate budget. The regression test injects a small ceiling and verifies matches beyond that budget. Postgres is unchanged.

The maintainer's sqlite-vec 0.1.6 spike showed rowid IN (SELECT ...) works in the KNN WHERE. This branch also exercised it through SQLiteBackend with the extension at 0.1.6 and confirmed matching entry and vec_entry rowids.

Testing

• Focused semantic-filter, KNN-budget/escalation, and changelog tests: 42 passed, 25 skipped (backend/platform coverage unavailable), 134 deselected.
• Repository pre-push scripts/test-affected --run -n 4 gate: passed.
• ruff check, ruff format --check, git diff --check, and commit hooks: passed.
• A synthetic in-memory 6,000-row sqlite-vec microbenchmark (31 runs/query) measured median 4.145 ms for unfiltered KNN and 7.676 ms for a candidate subquery matching all rows. This is a small synthetic benchmark, not a production workload; the additional absolute time was about 3.5 ms in this setup.

Notes for the reviewer

The subquery is used only when caller filters are supplied; the unfiltered hot path remains unchanged. It avoids a schema migration or embedding rebuild. The existing escalation remains necessary for max_distance culling and is now tested separately. I found no cap-specific warning in the semantic service to remove; the SQLite backend documentation now describes the remaining cap and distance-culling limits.

[markramm]

Thanks for this. It's a careful fix, and it holds. A review checked it in a throwaway tree with the patch applied, with the KNN ceiling set to 8 over 120 entries in two KBs:

• Every filter is applied inside KNN. Ten filter combinations (KB, type, one or two tags, date from/to, status plus state, a three-way combination, include_archived) each return the full 5 matches with none leaking; on dev, 9 of 10 under-return.
• The subquery reuses the outer check's clause and parameters, so the two can't disagree. User values are all ? parameters.
• The unfiltered SQL is unchanged.
• Read scope still holds: the readable-KB check (_restrict, search_service.py ~289) still runs afterwards, so nothing outside the caller's KBs can come back.
• Your regression test fails on dev for the right reason (assert 0 == 2).

Three small things before merge:

1. The sqlite-vec floor (pyproject.toml ~74). The rowid IN (subquery) KNN query filters before the cap on every release from 0.1.0 to 0.1.5 (and 0.1.9), so 0.1.6 isn't the lowest version that works. The bump forces upgrades nobody needs. Please revert to the old floor, or say what 0.1.6 is needed for.
2. Two edited tests pass on dev. CI's verify-red line reads 1 red · 0 import-only · 2 unexpected pass. They are test_search_semantic_fills_limit_despite_selective_filter (only its docstring changed) and test_unfiltered_distance_culling_escalates (dev's escalation already satisfies it). Both are useful regression guards, so mark them @pytest.mark.control(reason="..."), which is what that marker is for.
3. Filter coverage. Filtered semantic search can return fewer results than limit above 4096 embedded rows (sqlite-vec) #194's acceptance asks for one case per filter at the small ceiling, plus zero matches and hybrid mode. The PR covers entry_type only. A parametrised version of your test across the filters would close it.

One note, not for this PR: the readable-KB scope is still applied after KNN, so a scoped search without kb_name can still come back short at the ceiling. We'll track that on #194.

Automated review by an agent for the maintainer; comment only.

[rupayon123]

Updated on c3698257cab19d701fe3c1bca517111582e3479d: I restored the sqlite-vec minimum to >=0.1.0, marked the two already-passing-on-dev tests as controls, and added small-ceiling cases for the requested filters, zero matches, and hybrid mode. The repository pre-push hook passed, including its configured full test suite after running the documented extension setup. The new exact-head Actions run is 37115176914; the tests and experimental jobs are still running, while path-filtered jobs are skipped. I will recheck that head when the selected jobs finish. I left the separate readable-KB/no-kb_name recall limitation out of this PR as requested.

[markramm]

Automated review. I'm an AI agent that triages pull requests for this project's maintainer. A person reads these and makes the final call. If I've got something wrong, say so here.

Thank you for working through the review points so quickly. I read the new head (c369825); its checks pass (gate, test 3.12, verify-red, experimental).

Matches the issue

• Filtered semantic search can return fewer results than limit above 4096 embedded rows (sqlite-vec) #194's groom asks for filtering inside the KNN, with each filter, zero matches and hybrid mode covered at a small injected ceiling. The new tests are parametrised over entry_type, tags, state, fips, status, date_from and date_to, plus a zero-match test and a hybrid test (tests/test_search_filters_across_modes.py).
• pyproject.toml is not in the changed files, so the sqlite-vec floor stays at >=0.1.0. The two tests that already passed on dev carry @pytest.mark.control.

Differs from the groom

• The groom's regimes list kb_name and include_archived among the filters; neither is in the parametrised cases. (kb_name is not checked by the diff either way; I did not run anything.)
• The new comment in sqlite_backend.py says "sqlite-vec 0.1.6+ accepts a rowid subquery", while the declared floor is 0.1.0.

Overlaps another open pull request

• test(storage): cover backlink relation contract on Postgres #653 (draft) also adds tests to tests/backends/test_backend_conformance.py. I did not compare the lines.

Housekeeping

• The changelog fragment is present.

This is a first pass for the maintainer, who makes the review decision.

---

Generated by Claude Code

[rupayon123]

Thanks for catching the remaining coverage gap. I restored the declared sqlite-vec>=0.1.0 floor and removed the version-specific comment; I’m relying on your compatibility test for the earlier releases rather than claiming I independently exercised every version.

On the current head, I added small-cap regressions where nearer candidates from another knowledge base and archived entries would otherwise fill the KNN budget before an eligible result is considered. The existing include_archived=True tests still check that archived entries are returned in keyword, semantic, and hybrid modes. The focused cap cases pass (3 passed), the affected test file passed (77 passed before narrowing the KB case to semantic/hybrid), and the repository pre-push gate passed.

The exact PR head is b9ddd6c21171dc37195c730d6d57fdf0c0366887. Its Actions run is still executing the Python 3.12 and experimental jobs; the path-filtered jobs are skipped. I’ll recheck that head when those jobs finish.

[rupayon123]

Thanks for the second pass. I checked the two special filters against the current branch rather than folding them into the generic value-filter matrix: test_kb_name_filter_is_applied_before_knn_cap covers the KB scope in semantic and hybrid mode, and test_archived_entries_do_not_consume_the_knn_cap proves archived distractors cannot exhaust the cap before an active match. The existing archived-entry tests also cover include_archived=True in keyword, semantic, and hybrid modes. I reran those cases on b9ddd6c21171dc37195c730d6d57fdf0c0366887: 6 passed.

I also confirmed the version-specific wording you flagged is gone; the backend comment now only documents the sqlite-vec 0.1.9 k ceiling. The exact-head CI run 37205212096 passed gate, verify-red, experimental, test (3.12), and changes; coverage, e2e, KB, smoke, frontend, and experimental-issues were skipped by their path filters. PR #653 is still a draft and changes tests/backends/test_backend_conformance.py; this PR's filter regressions are in tests/test_search_filters_across_modes.py, so the changed files do not overlap.

If you intended a separate small-cap assertion for include_archived=True itself, please point me to the expected ordering/recall behavior when the cap is smaller than the eligible result set. The current test pins both the default exclusion at the cap and the explicit include behavior across all search modes.

[rupayon123]

Thanks for the follow-up. I added an explicit include_archived=True small-cap regression for semantic and hybrid search: it creates more archived matches than the injected cap and asserts that the returned page remains full and includes archived results. The nine focused cap/archive cases pass on Python 3.13, and the repository pre-push gate passed its configured core and affected tests. The exact new head is d4f18db98777eed5596d9dadb77c5be22708608c; Actions 37228004933 has changes passing while test (3.12) and experimental are queued.\n\nI also rechecked the current branch: the declared sqlite-vec>=0.1.0 floor is unchanged, and I cannot find the 0.1.6+ wording in the current backend diff or file. That may be stale review text; if you still see it, could you point me to the current path/line? I’ll address it if there is another occurrence.

[markramm]

Automated review. I'm an AI agent that triages pull requests for this project's maintainer. A person reads these and makes the final call. If I've got something wrong, say so here.

Thank you for the quick, thorough rounds on #194. I read the current head (d4f18db); its checks pass (gate, test 3.12, verify-red, experimental).

Matches the issue

• The KNN query gets AND rowid IN (SELECT e.rowid FROM entry e WHERE 1=1{where}) only when a selective filter is present; the unfiltered query is unchanged. This is option A in the 2026-10-02 groom.
• Tests inject a small cap, with kb_name, archived-entry, zero-match and hybrid cases (tests/test_search_filters_across_modes.py, tests/backends/test_backend_conformance.py), and pyproject.toml is untouched.

Differs from the groom

• The groom's acceptance says "The sqlite-vec pin names the lowest version the test passes on." The pin stays at >=0.1.0; the PR body still says it raises the minimum to 0.1.6, which is no longer what the diff does. A later review comment asked for the old floor, so this may be intended.

Overlaps another open pull request

• test(storage): cover backlink relation contract on Postgres #653 (draft) also edits tests/backends/test_backend_conformance.py; I did not compare the lines.

Tests

• The new cap tests go through SQLiteBackend and SearchService, not the CLI, REST or MCP.

Housekeeping

• The changelog fragment is present.

This is a first pass for the maintainer, who makes the review decision.

---

Generated by Claude Code