Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 52 additions & 7 deletions nextcloud_mcp_server/api/visualization.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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
)
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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",
)
Expand Down
170 changes: 170 additions & 0 deletions tests/unit/api/test_vector_viz_granularity_api.py
Original file line number Diff line number Diff line change
@@ -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"
Loading