refactor(search): address PR #750 round 2 review feedback
Implements fire-and-forget eviction (ADR-019 §"Lazy eviction"): the search response no longer waits on Qdrant deletes, instead spawning evict() on a long-lived lifespan-owned task group. Falls back to inline eviction in modes without vector sync and in unit tests. Also: harden _verify_news_items against non-numeric ids (fail open instead of crashing the verifier); document the get_file_info None-on-404 contract; add INDEXED_DOC_TYPES single source of truth in vector/scanner.py referenced by the CI-guard test; write a Verify-on-Read Latency Budget section in docs/configuration.md covering the unbounded news.get_items fetch. Closes the two remaining ADR-019 implementation checklist items. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
7784ec02d7
commit
21e5608a39
@@ -225,5 +225,5 @@ In `server/semantic.py::nc_semantic_search_answer`, replace the per-type `if res
|
||||
- [ ] Update existing docstrings in `search/semantic.py:52` and `search/bm25_hybrid.py:75` to point at the new helper.
|
||||
- [ ] Unit tests: each verifier handles 200/403/404/transient distinctly; dedup collapses chunks; eviction is scheduled on `False`.
|
||||
- [ ] Integration test: index a note, delete via API (no webhook), confirm the next semantic search does not return it.
|
||||
- [ ] CI guard: enumerate indexed doc_types in `vector/scanner.py` and assert each has a registered verifier.
|
||||
- [ ] Document the latency budget and rate-limit posture in `docs/configuration.md`.
|
||||
- [x] CI guard: enumerate indexed doc_types in `vector/scanner.py` and assert each has a registered verifier. (`INDEXED_DOC_TYPES` in `vector/scanner.py`; `tests/unit/search/test_verification.py::test_supported_doc_types_covers_indexed_types`.)
|
||||
- [x] Document the latency budget and rate-limit posture in `docs/configuration.md`. (See "Verify-on-Read Latency Budget" section.)
|
||||
|
||||
@@ -474,6 +474,45 @@ DOCUMENT_CHUNK_OVERLAP=100
|
||||
|
||||
**Important**: Changing chunk size requires re-embedding all documents. The collection naming strategy (see "Qdrant Collection Naming" above) helps manage this by creating separate collections for different configurations.
|
||||
|
||||
### Verify-on-Read Latency Budget
|
||||
|
||||
Every semantic search request runs an access-control verification pass over its
|
||||
results before returning them, to filter out documents the user can no longer
|
||||
access (deleted, unshared, permissions changed). See
|
||||
[ADR-019](ADR-019-verify-on-read-for-semantic-search.md) for the full design.
|
||||
|
||||
This adds Nextcloud round-trips to the search path that operators should be
|
||||
aware of:
|
||||
|
||||
- **Per-search cost**: one Nextcloud round-trip per *unique* `(doc_id, doc_type)`
|
||||
in the result set. Chunking means a 10-result page typically references 3-5
|
||||
unique documents, so verification adds 3-5 round-trips. With the default
|
||||
20-way concurrency this is one parallel batch — usually under 100 ms on a
|
||||
healthy connection.
|
||||
- **Concurrency**: all verifications fan out under a shared semaphore
|
||||
(`DEFAULT_VERIFICATION_CONCURRENCY = 20` in `search/verification.py`). The
|
||||
limit is not currently exposed as an env var; if production workloads
|
||||
saturate Nextcloud, consider opening an issue to make it tunable.
|
||||
- **News API caveat**: the News app has no per-item endpoint, so the news
|
||||
verifier issues a single `news.get_items(batch_size=-1, get_read=True)` call
|
||||
per search that contains any news result, then intersects locally. The
|
||||
payload is **unbounded** — for users with very large feed backlogs this can
|
||||
dominate verification latency. Disabling News in the indexer or running with
|
||||
a smaller backlog mitigates this; per-item paginated verification is tracked
|
||||
as a future improvement.
|
||||
- **Eviction**: when verification finds a definitive miss (404 / 403), the
|
||||
corresponding Qdrant points are deleted in the background on a lifespan-owned
|
||||
task group — fire-and-forget, does **not** block the search response.
|
||||
Eviction failures are logged but never propagated; the next query will
|
||||
re-verify and re-attempt (self-healing).
|
||||
- **Failure modes**: transient errors (5xx, network) keep results visible
|
||||
(fail open) so a flaky link does not silently shrink result pages; only
|
||||
*definitive* 404 / 403 drops them.
|
||||
|
||||
If verification ever needs to be disabled (debugging, benchmarking), the
|
||||
`evict_on_missing=False` flag on `verify_search_results()` skips eviction
|
||||
without changing what is returned to the caller.
|
||||
|
||||
### Environment Variables Reference
|
||||
|
||||
| Variable | Required | Default | Description |
|
||||
|
||||
Reference in New Issue
Block a user