feat(api): reject explicit unsupported search algorithm with 422 (ADR-030)

[cbcoutinho]

Summary

GET /api/v1/status already advertises supported_search_types (ADR-030). This makes the enforcement side strict: /api/v1/search and /api/v1/vector-viz/search now reject an explicit unsupported algorithm with HTTP 422 instead of silently coercing it (e.g. semantic → bm25 while SEARCH_MODE=keyword).

Motivation: a keyword-only tenant was still returning results for semantic/hybrid queries because the server coerced the algorithm — lexical results dressed up as a semantic answer. Now the client (astrolabe) fails loud and self-corrects from the advertised set.

HTTP 422
{ "error": "unsupported_search_type", "requested": "semantic",
  "supported_search_types": ["bm25"] }

Changes

• api/management.py: new select_search_algorithm (explicit → raise UnsupportedSearchType; absent → graceful default) wrapping the retained lenient resolve_search_algorithm; new UnsupportedSearchType exception.
• api/visualization.py: both search endpoints translate the exception to 422. Absent algorithm still defaults gracefully across modes.
• Tests: TestSelectSearchAlgorithm unit tests; contract-first pact gains the keyword-mode 422 interaction (provider state the server advertises keyword-only search support, already registered).
• docs/ADR-030: documents the strict rejection.

Contract alignment

Paired with the astrolabe consumer PR (gates the algorithm client-side from /api/v1/status and surfaces this 422 as the backstop; adds the matching pact-php consumer interaction). Deck card #512.

Test plan

• pytest tests/unit/test_management_status_endpoint.py (26 passed)
• pytest -m contract tests/contract/test_mcp_status_search_types_consumer.py (3 passed)
• ruff + ty-check (pre-commit) clean
• Provider verification job replays astrolabe's published consumer pact (keyword-mode 422)

---

This PR was generated with the help of AI, and reviewed by a Human

[claude]

Claude finished @cbcoutinho's task in 1m 59s —— View job

---

Claude finished reviewing this PR

Todo list

• Gather context (diff, CLAUDE.md, changed files)
• Review api/management.py changes (select_search_algorithm, UnsupportedSearchType)
• Review api/visualization.py changes (422 wiring, shared response builder)
• Review test coverage (unit, contract, endpoint-wiring tests)
• Review ADR-030 doc updates
• Post final review feedback

Summary

This is a clean, well-scoped change: it turns the previously-silent resolve_search_algorithm coercion into a strict gate (select_search_algorithm) for explicit algorithm requests, while keeping the lenient path for the implicit default. The two review rounds already visible in the PR comments (endpoint-wiring test, shared 422 builder, reject-path log, null caveat doc, hybrid parametrization) addressed the obvious gaps, and the diff reflects those fixes. I confirmed select_search_algorithm/resolve_search_algorithm have no other call sites (nextcloud_mcp_server/api/visualization.py's two search endpoints only), so the stricter behavior is fully contained.

Findings

🟢 Nit — UnsupportedSearchType.__init__ type hint is narrower than reality (nextcloud_mcp_server/api/management.py:78)
requested: str assumes body.get("algorithm") is always a string, but it's raw JSON input — a client could send "algorithm": 123 or ["semantic"]. Neither crashes (%r/JSONResponse both handle non-str fine), so this is cosmetic, not a bug. Pre-existing looseness (the old body.get("algorithm", "hybrid") had the same lack of narrowing), just newly visible now that the value flows into a typed exception field. Not blocking.

🟢 Nit — algorithm gate runs after the empty-query short-circuit, in both endpoints (nextcloud_mcp_server/api/visualization.py:265-276 and :509-533)
unified_search returns {"results": [], "total_found": 0} for query="" before reaching select_search_algorithm, so {"query": "", "algorithm": "semantic"} against a keyword-only server gets a 200, not a 422. Same ordering pre-dates this PR (query check always came before algorithm resolution), so it's not a regression — just worth confirming astrolabe doesn't rely on the 422 firing independent of query content.

What's solid

• Single source of truth preserved: SUPPORTED_SEARCH_ALGORITHMS / supported_search_types() still back both the /api/v1/status advertisement and the new gate — the advertised and enforced sets can't drift.
• Shared 422 builder (_unsupported_search_type_response) correctly deduplicates the payload shape across both endpoints (round-1 finding, now fixed).
• Endpoint-wiring test (tests/unit/test_search_endpoint_algorithm_gate.py) drives the real Starlette handlers via TestClient rather than only unit-testing the pure helper — good, this is exactly the kind of coverage that would have caught a wiring mistake the pure-function tests couldn't.
• null vs. absent caveat is now explicitly documented in the docstring (management.py:104-106) — a real, non-obvious body.get trait worth calling out.
• Contract test correctly pins the keyword-mode 422 against the existing the server advertises keyword-only search support provider state (verified this state is already registered in tests/contract/test_mcp_provider_verification.py:119).
• ADR-030 doc accurately describes the split between select_search_algorithm (strict) and resolve_search_algorithm (lenient, retained for the implicit-default path).

Not verified

I don't have shell execution permissions in this environment, so I couldn't independently re-run ruff/ty/pytest — I relied on static reading of the diff plus the PR author's reported results (26 + 3 + 4 + more passing, ruff/ty clean). If you'd like me to actually execute the suite, granting Bash tool access would let me confirm directly.

[cbcoutinho]

Round-1 findings addressed in 97c008a4:

• 🟡 422 wiring untested: added tests/unit/test_search_endpoint_algorithm_gate.py — drives the real unified_search / vector_search Starlette handlers through a TestClient (keyword-mode settings + patched auth) and asserts an explicit algorithm=semantic returns the 422 {error, requested, supported_search_types} body on both endpoints, plus that a supported bm25 passes the gate. No longer reliant on provider verification alone.
• 🟢 Duplication: extracted _unsupported_search_type_response(e); both endpoints now share one 422 payload builder so the shape can't drift.

4 new tests pass; ruff + ty-check green (pre-commit).

[cbcoutinho]

Round-2 nits addressed in 97c008a4:

• Reject-path log: select_search_algorithm now emits a logger.debug on the hard-reject branch, mirroring resolve_search_algorithm's coercion log.
• null caveat: documented that an explicit JSON "algorithm": null reads as absent (graceful default), a pre-existing body.get trait.
• Parametrized hybrid: the keyword-mode 422 tests (pure helper + endpoint wiring) now cover both semantic and hybrid.

32 tests pass; ruff/ty green.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[cbcoutinho]

Round-3 verdict is a pass — thanks. Both 🟢 nits are non-blocking and I'm leaving them as-is with rationale:

• requested: str narrower than raw JSON: cosmetic and pre-existing (the old body.get("algorithm", "hybrid") had the same looseness); %r/JSON serialization handle a non-str value fine, and in practice the field is always a string. Not worth widening the typed exception field.
• Gate runs after the empty-query short-circuit: confirmed astrolabe does not rely on the 422 firing for an empty query — its ApiController::search returns a 400 Missing required parameter: query before the gate, and the consumer pact's 422 interaction uses a non-empty query. So {query:"", algorithm:"semantic"} → 200 is fine and matches the pre-existing ordering.

No code change this round. Treating this as merge-ready pending the re-triggered CI going green.