From 29500bea7e15c1ce300f3fb33e4c78598eb8cd9a Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Fri, 29 May 2026 17:49:21 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20PR=20#813=20review=20=E2=80=94=20cap=20u?= =?UTF-8?q?nified=5Fsearch=20multi-type=20pool;=20document=20deck=20self-o?= =?UTF-8?q?nly?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🟡 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) --- nextcloud_mcp_server/api/visualization.py | 8 ++++++++ nextcloud_mcp_server/search/context.py | 8 +++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/nextcloud_mcp_server/api/visualization.py b/nextcloud_mcp_server/api/visualization.py index 6bca433c..736d9858 100644 --- a/nextcloud_mcp_server/api/visualization.py +++ b/nextcloud_mcp_server/api/visualization.py @@ -247,7 +247,15 @@ async def unified_search(request: Request) -> JSONResponse: 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 = results[: search_limit * 2] else: results = await search_algo.search( query=query, diff --git a/nextcloud_mcp_server/search/context.py b/nextcloud_mcp_server/search/context.py index 2bec1105..42eb8813 100644 --- a/nextcloud_mcp_server/search/context.py +++ b/nextcloud_mcp_server/search/context.py @@ -190,7 +190,13 @@ async def _get_deck_metadata_from_qdrant( qdrant_client = await get_qdrant_client() 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( collection_name=settings.get_collection_name(), scroll_filter=Filter(