From 49927d4faf216434231be743b66f5d4e81f774aa Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Wed, 19 Aug 2026 22:45:05 +0200 Subject: [PATCH] feat(api): accept `granularity` on /api/v1/vector-viz/search MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This endpoint is the only one the Astrolabe app's search page calls, and it did not accept `granularity` at all — it passed no value to the search algorithm and hardcoded `chunk` into its metrics. So `granularity="document"` (one row per document rather than per passage) was unreachable from the UI, even though `/api/v1/search` and `nc_semantic_search` both expose it. That mattered beyond a missing feature: ADR-034's relevance curves were fitted at document granularity, so the app page could not request the retrieval shape its own relevance numbers were calibrated on. Brings the endpoint into line with its sibling on all four points: the value is read, an unknown value is rejected rather than silently downgraded to chunk, the document+semantic combination is refused with the same 422 payload, and the value reaches both the single-search and doc_types branches of the algorithm call. Search metrics now report the granularity actually used. Found by auditing which search settings the Astrolabe UI can reach versus what the server supports, after a retrieval benchmark turned out to be sweeping parameters no deployment could request. Deck #1070. Co-Authored-By: Claude Opus 5 (1M context) --- nextcloud_mcp_server/api/visualization.py | 59 +++++- .../api/test_vector_viz_granularity_api.py | 170 ++++++++++++++++++ 2 files changed, 222 insertions(+), 7 deletions(-) create mode 100644 tests/unit/api/test_vector_viz_granularity_api.py diff --git a/nextcloud_mcp_server/api/visualization.py b/nextcloud_mcp_server/api/visualization.py index ac0671d47..93c173831 100644 --- a/nextcloud_mcp_server/api/visualization.py +++ b/nextcloud_mcp_server/api/visualization.py @@ -891,6 +891,27 @@ async def vector_search(request: Request) -> JSONResponse: ) except ValueError as e: return JSONResponse({"error": str(e)}, status_code=400) + # Result granularity. Absent here until now, which made + # granularity="document" — one row per document rather than per chunk — + # unreachable from the Astrolabe search page, the only surface that + # calls this endpoint. That is the shape "which files mention X" needs, + # and the shape ADR-034's relevance curves were FITTED at, so the app + # page could not request the retrieval shape its own relevance numbers + # were calibrated on. Validated exactly as /api/v1/search does: an + # unrecognized value is rejected rather than silently downgraded to + # chunk, so a caller that asked for document granularity is never + # quietly served something else. + granularity = body.get("granularity", GRANULARITY_CHUNK) + if granularity not in VALID_GRANULARITIES: + return JSONResponse( + { + "error": ( + f"Invalid granularity {granularity!r}. " + f"Must be one of {sorted(VALID_GRANULARITIES)}" + ) + }, + status_code=400, + ) include_pca = body.get("include_pca", True) doc_types = body.get("doc_types") # Optional list of document types # Optional cross-encoder rerank, same flag and same gating as @@ -957,6 +978,21 @@ async def vector_search(request: Request) -> JSONResponse: except UnsupportedSearchType as e: return _unsupported_search_type_response(e) + # Grouped retrieval needs a sparse leg to group on, so document + # granularity is unsupported for dense-only search. Same 422 shape and + # same position (after the algorithm resolves) as /api/v1/search, so a + # client sees one error contract across both endpoints. + if granularity == GRANULARITY_DOCUMENT and algorithm == "semantic": + return JSONResponse( + { + "error": "granularity_unsupported_for_algorithm", + "granularity": granularity, + "algorithm": algorithm, + "supported_algorithms": ["bm25", "hybrid"], + }, + status_code=422, + ) + # Capability gate after the query and algorithm checks, matching # /api/v1/search so an unsupported algorithm still wins over an # unconfigured reranker on both endpoints. @@ -966,15 +1002,22 @@ async def vector_search(request: Request) -> JSONResponse: rerank_outcome = RERANK_SKIPPED # Reranking can only reorder what retrieval supplied, so it needs a - # deeper candidate pool than the caller's limit. This endpoint always - # searches chunks (grouped=False) and has no offset, so the floor is - # simply `limit` — which is also what it retrieves when rerank is off, - # leaving that path byte-identical to before. NB with several + # deeper candidate pool than the caller's limit. This endpoint has no + # offset, so the floor is simply `limit` — which is also what it + # retrieves when rerank is off, leaving that path byte-identical to + # before. `grouped` now tracks the requested granularity: the grouped + # prefetch is bounded by MAX_DOCUMENT_PREFETCH, so asking for more + # groups than it can fill makes Qdrant widen its grouping search and + # reorder the head before the reranker sees it. NB with several # `doc_types` this depth is fetched PER TYPE before the merge, so # retrieval cost scales with len(doc_types) — the same shape as # unified_search's own doc_types loop. retrieval_limit = ( - effective_pool_size(settings, floor=limit, grouped=False) + effective_pool_size( + settings, + floor=limit, + grouped=granularity == GRANULARITY_DOCUMENT, + ) if rerank else limit ) @@ -999,6 +1042,7 @@ async def _execute(scope: AccessibleScope | None) -> list: doc_type=doc_type, accessible_owners=owners, shared_root_ids=roots, + granularity=granularity, modified_after=modified_after, modified_before=modified_before, path_prefixes=path_prefixes, @@ -1015,6 +1059,7 @@ async def _execute(scope: AccessibleScope | None) -> list: limit=retrieval_limit, accessible_owners=owners, shared_root_ids=roots, + granularity=granularity, modified_after=modified_after, modified_before=modified_before, path_prefixes=path_prefixes, @@ -1143,7 +1188,7 @@ async def _execute(scope: AccessibleScope | None) -> list: record_search_request( surface="http_viz", algorithm=_search_algorithm_label(algorithm, fusion), - granularity=GRANULARITY_CHUNK, + granularity=granularity, reranked=_reranked_label(rerank, rerank_outcome), status="success", results_returned=len(formatted_results), @@ -1171,7 +1216,7 @@ async def _execute(scope: AccessibleScope | None) -> list: record_search_request( surface="http_viz", algorithm=_search_algorithm_label(algorithm, fusion), - granularity=GRANULARITY_CHUNK, + granularity=granularity, reranked="false", status="error", ) diff --git a/tests/unit/api/test_vector_viz_granularity_api.py b/tests/unit/api/test_vector_viz_granularity_api.py new file mode 100644 index 000000000..46b536323 --- /dev/null +++ b/tests/unit/api/test_vector_viz_granularity_api.py @@ -0,0 +1,170 @@ +"""`granularity` on POST /api/v1/vector-viz/search. + +This endpoint is the ONLY one the Astrolabe app's search page calls, and until +now it did not accept `granularity` at all — it passed no value to the search +algorithm and hardcoded `chunk` into its metrics. So `granularity="document"` +(one row per document rather than per passage) was unreachable from the UI, even +though `/api/v1/search` and `nc_semantic_search` both exposed it. + +That mattered beyond a missing feature: ADR-034's relevance curves were FITTED +at document granularity, so the app page could not request the retrieval shape +its own relevance numbers were calibrated on. + +Sibling of test_vector_viz_rerank_api.py; same handler, same scaffolding. The +contract asserted here is that this endpoint now agrees with /api/v1/search on +all four points: the value is read, an unknown value is rejected rather than +silently downgraded, the document+semantic combination is refused with the same +422 payload, and the value actually reaches the algorithm. +""" + +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from starlette.applications import Starlette +from starlette.routing import Route +from starlette.testclient import TestClient + +from nextcloud_mcp_server.api.visualization import vector_search +from nextcloud_mcp_server.search.algorithms import SearchResult +from nextcloud_mcp_server.vector.oauth_sync import NotProvisionedError + +pytestmark = pytest.mark.unit + + +def _settings(): + settings = MagicMock() + settings.vector_sync_enabled = True + settings.search_rerank_enabled = False + settings.embedding_gateway_url = "" + settings.search_rerank_model = "vendor/model" + settings.search_rerank_pool_size = 200 + settings.search_rerank_timeout_seconds = 30.0 + settings.search_rerank_max_concurrency = 1 + settings.usage_metering_enabled = False + return settings + + +def _app() -> Starlette: + app = Starlette( + routes=[Route("/api/v1/vector-viz/search", vector_search, methods=["POST"])] + ) + app.state.oauth_context = {"config": {"nextcloud_host": "https://nc.example"}} + return app + + +def _rows(n): + return [ + SearchResult( + id=str(i), + doc_type="file", + title=f"d{i}", + excerpt=f"t{i}", + score=1.0 - (i / (n or 1)), + ) + for i in range(n) + ] + + +def _post(body, *, rows=2): + """POST with the algorithm stubbed; returns (response, algo).""" + algo = MagicMock() + algo.search = AsyncMock(return_value=_rows(rows)) + algo.query_token_count = 0 + algo.query_embedding = None + + body = {"include_pca": False, **body} + + with ( + patch( + "nextcloud_mcp_server.api.visualization.get_settings", + return_value=_settings(), + ), + patch( + "nextcloud_mcp_server.api.visualization.validate_token_and_get_user", + new=AsyncMock(return_value=("alice", {})), + ), + patch( + "nextcloud_mcp_server.api.visualization.BM25HybridSearchAlgorithm", + return_value=algo, + ), + patch( + "nextcloud_mcp_server.api.visualization.SemanticSearchAlgorithm", + return_value=algo, + ), + # Unprovisioned ⇒ the real _execute closure runs, so its kwargs are + # observable on the algo spy. + patch( + "nextcloud_mcp_server.api.visualization.get_user_client_basic_auth", + new=AsyncMock(side_effect=NotProvisionedError("not provisioned")), + ), + ): + client = TestClient(_app()) + return client.post("/api/v1/vector-viz/search", json=body), algo + + +def test_default_granularity_is_chunk(): + """Omitting the field must behave exactly as before it was accepted — + every existing Astrolabe release sends no value.""" + resp, algo = _post({"query": "anything"}) + + assert resp.status_code == 200 + assert algo.search.await_args.kwargs["granularity"] == "chunk" + + +def test_document_granularity_reaches_the_algorithm(): + """The point of the change: the value is not merely accepted, it is passed + down. Accepting it and ignoring it would be worse than rejecting it.""" + resp, algo = _post({"query": "anything", "granularity": "document"}) + + assert resp.status_code == 200 + assert algo.search.await_args.kwargs["granularity"] == "document" + + +def test_document_granularity_reaches_the_algorithm_on_the_doc_types_branch(): + """`doc_types` takes a separate loop through the same closure. A parameter + threaded on one branch and not the other is invisible to anyone who happens + to filter by type — which the Astrolabe UI does on every search.""" + resp, algo = _post( + {"query": "anything", "granularity": "document", "doc_types": ["file"]} + ) + + assert resp.status_code == 200 + assert algo.search.await_args.kwargs["granularity"] == "document" + + +def test_unknown_granularity_is_rejected_with_400(): + """Rejected, not silently downgraded to chunk: a caller that asked for one + row per document and quietly received passages cannot tell that apart from + a corpus that genuinely has one chunk per document.""" + resp, algo = _post({"query": "anything", "granularity": "paragraph"}) + + assert resp.status_code == 400 + assert "granularity" in resp.json()["error"] + algo.search.assert_not_awaited() + + +def test_document_granularity_with_semantic_is_refused_with_422(): + """Grouped retrieval needs a sparse leg to group on, so document + granularity is unsupported for dense-only search. Same error contract as + /api/v1/search so a client handles one shape across both endpoints.""" + resp, _ = _post( + {"query": "anything", "granularity": "document", "algorithm": "semantic"} + ) + + assert resp.status_code == 422 + body = resp.json() + assert body["error"] == "granularity_unsupported_for_algorithm" + assert body["granularity"] == "document" + assert body["algorithm"] == "semantic" + assert body["supported_algorithms"] == ["bm25", "hybrid"] + + +def test_chunk_granularity_with_semantic_is_allowed(): + """The 422 is specific to the document+dense combination — dense-only + passage search is the endpoint's oldest behaviour and must keep working.""" + resp, algo = _post( + {"query": "anything", "granularity": "chunk", "algorithm": "semantic"} + ) + + assert resp.status_code == 200 + assert algo.search.await_args.kwargs["granularity"] == "chunk"