fix: PR #813 review — cap unified_search multi-type pool; document deck self-only
🟡 Performance: unified_search's _execute sorted but did not cap the merged multi-doc_type pool, so N doc_types each fetched at search_limit sent N*search_limit candidates into verify-on-read (one Nextcloud round-trip each). Cap to search_limit*2 after the sort, matching vector_search, nc_semantic_search and the viz_routes pattern — bounding verification cost to O(2*search_limit) regardless of how many doc_types are requested. 🟡 Consistency: _get_deck_metadata_from_qdrant is the one internal Qdrant lookup that uses a raw user_id filter instead of build_ownership_filter. This is not a bug — deck cards are a documented cross-user gap (the Deck API is per-user, so cross-user context can't be fetched with the caller's credentials) — but the inconsistency was unexplained. Added a comment documenting the deliberate self-only scope. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
063bd06ba6
commit
29500bea7e
@@ -247,7 +247,15 @@ async def unified_search(request: Request) -> JSONResponse:
|
|||||||
accessible_owners=owners,
|
accessible_owners=owners,
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
# Sort, then cap to a fixed over-fetch budget before the result
|
||||||
|
# reaches verify-on-read. Without this, N doc_types each fetched
|
||||||
|
# at search_limit would send N*search_limit candidates into
|
||||||
|
# verification — one Nextcloud round-trip each — scaling the cost
|
||||||
|
# with len(doc_types). 2x leaves headroom for verify-on-read
|
||||||
|
# drops before pagination, matching the nc_semantic_search and
|
||||||
|
# viz_routes pattern.
|
||||||
results.sort(key=lambda r: r.score, reverse=True)
|
results.sort(key=lambda r: r.score, reverse=True)
|
||||||
|
results = results[: search_limit * 2]
|
||||||
else:
|
else:
|
||||||
results = await search_algo.search(
|
results = await search_algo.search(
|
||||||
query=query,
|
query=query,
|
||||||
|
|||||||
@@ -190,7 +190,13 @@ async def _get_deck_metadata_from_qdrant(
|
|||||||
qdrant_client = await get_qdrant_client()
|
qdrant_client = await get_qdrant_client()
|
||||||
settings = get_settings()
|
settings = get_settings()
|
||||||
|
|
||||||
# Query for any chunk of this card (we just need metadata)
|
# Query for any chunk of this card (we just need metadata).
|
||||||
|
# Intentionally self-only (raw user_id, not build_ownership_filter):
|
||||||
|
# deck cards are a documented cross-user gap — the Deck API is per-user,
|
||||||
|
# so cross-user deck context can't be fetched with the caller's
|
||||||
|
# credentials anyway (see the doc_type=="file"-only gate in
|
||||||
|
# get_chunk_with_context). Every other internal Qdrant lookup here is
|
||||||
|
# ACL-aware; this one is the deliberate exception.
|
||||||
scroll_result = await qdrant_client.scroll(
|
scroll_result = await qdrant_client.scroll(
|
||||||
collection_name=settings.get_collection_name(),
|
collection_name=settings.get_collection_name(),
|
||||||
scroll_filter=Filter(
|
scroll_filter=Filter(
|
||||||
|
|||||||
Reference in New Issue
Block a user