diff --git a/.s2s/BACKLOG.md b/.s2s/BACKLOG.md index 3da37fb..0141245 100644 --- a/.s2s/BACKLOG.md +++ b/.s2s/BACKLOG.md @@ -974,7 +974,7 @@ Consequences: every reindex permanently doubles the storage for that namespace, ### DEBT-033: some endpoints bypass the REQ-010 error envelope -**Status**: planned | **Priority**: low | **Created**: 2026-07-14 +**Status**: completed | **Priority**: low | **Created**: 2026-07-14 | **Completed**: 2026-07-15 | **PR**: #112 **Origin**: DOCS-009 (2026-07-14). **Context**: REQ-010 defines a single error envelope (`{"error": {category, code, message, remediation, request_id, details}}`), and `docs/reference/api.md` presented it as universal. It is not. Reindex (`400`, `404`), conversations (`404`, `503`) and admin conversation turns (`404`, `501`, `503`) raise `HTTPException` with a plain string detail, so they return FastAPI's bare `{"detail": "..."}` — no code, no remediation, no request id. @@ -984,8 +984,28 @@ Consequences: every reindex permanently doubles the storage for that namespace, A client cannot branch on an error code for these endpoints, and the operator-facing reindex failures are exactly the ones where a machine-readable code would be worth having. The doc now warns that both shapes exist; the fix is to make the envelope actually universal. **Acceptance criteria**: -- [ ] The endpoints above raise envelope errors with proper `ERR-*` codes -- [ ] The "not every endpoint uses the envelope" caveat is removed from `docs/reference/api.md` +- [x] The endpoints above raise envelope errors with proper `ERR-*` codes +- [x] The "not every endpoint uses the envelope" caveat is removed from `docs/reference/api.md` + +**Resolution** (2026-07-15): the four scoped endpoints were enveloped, and a grep of the handlers (`HTTPException(..., detail="...")`) found the DOCS-009 caveat had **undercounted** — the same bare shape lived on `POST /api/v1/query` (registry / pipeline-not-configured), the admin auth dependency and the API-key creation path (registry / key-store-not-configured), and `GET /api/v1/admin/health?detail=full` (service-initializing). Removing the caveat as written would have re-asserted a universality those still broke, so all of them were enveloped too (approved scope expansion). Each raise keeps its prior HTTP status (shape-only change; no test asserted the bare shape). Codes: reused `ERR-CONFIG-001` (registry not initialized) and `ERR-CONFIG-002` (key store not configured), matching the global handler's treatment of internal faults; added `ERR-CONV-001/002/003`, `ERR-QUERY-005`, `ERR-ADMIN-008`, `ERR-INDEX-003/004`, each registered in `errors.py` + `_CODE_STATUS_OVERRIDE` + `docs/reference/error-codes.md`. The api.md caveat is removed. Spawned **DEBT-034**: the wire shape is `{"detail": {"error": {...}}}` for every `HTTPException`-based error (FastAPI wraps the detail), while api.md shows the logical top-level `{"error": {...}}`; that representation gap predates this work and affects every enveloped endpoint. + +--- + +### DEBT-034: the error envelope is nested under `detail` on the wire, but documented as top-level + +**Status**: planned | **Priority**: low | **Created**: 2026-07-15 +**Origin**: DEBT-033 (2026-07-15). + +**Context**: FastAPI serializes `raise HTTPException(status_code=..., detail=err.to_envelope())` as `{"detail": {"error": {...}}}` — the REQ-010 envelope sits one level down, under `detail`. This is how **every** enveloped endpoint behaves (auth, ingest, index, analytics, learn, admin, and the endpoints fixed in DEBT-033), and the whole test suite already asserts `resp.json()["detail"]["error"]["code"]`, so the nesting is the de-facto contract. The one exception is the global unhandled-exception handler in `vektra-app/main.py`, which returns top-level `{"error": {...}}` via `JSONResponse`. So there are two shapes after all — not "envelope vs bare" (DEBT-033 closed that), but "envelope under `detail`" (explicit raises) vs "envelope at top level" (uncaught 500s). `docs/reference/api.md` and `docs/reference/error-codes.md` show only the top-level form, so a client that follows the docs looks for `body.error.code` and finds nothing; the real path for almost every error is `body.detail.error.code`. + +**Options**: +1. **(Recommended)** Register a custom `StarletteHTTPException` handler that, when `exc.detail` is already an envelope dict, returns it at top level via `JSONResponse(status_code=exc.status_code, content=exc.detail)`. One handler, no per-endpoint change, and both error paths converge on the documented top-level `{"error": {...}}`. Rationale: makes the wire match the docs and REQ-010's own shape, rather than teaching clients an accidental FastAPI wrapping. +2. Document the nesting instead: change api.md/error-codes.md to show `{"detail": {"error": {...}}}` for `HTTPException`-based errors. Cheaper, but enshrines the artifact and leaves the two-shape split (nested vs top-level from the 500 handler) permanently in the contract. +3. Leave as-is. Rejected: the docs actively mislead about where the code lives. + +**Acceptance criteria**: +- [ ] The REQ-010 envelope is reachable at a single, documented JSON path across all error responses (explicit raises and uncaught 500s alike) +- [ ] `docs/reference/api.md` and `docs/reference/error-codes.md` match the actual wire shape --- diff --git a/CHANGELOG.md b/CHANGELOG.md index 134f3f0..6a554c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ Convention (Keep a Changelog 1.1.0): ### Fixed +- **api**: the REQ-010 error envelope (`{"error": {category, code, message, remediation, request_id}}`) is now returned by every API error path, not most of them (DEBT-033). `docs/reference/api.md` first presented it as universal, then (DOCS-009) walked that back with a caveat naming four endpoints that raised a bare FastAPI `{"detail": "..."}` instead — no code to branch on, no remediation. The caveat itself undercounted: grepping the handlers turned up the same gap on `POST /api/v1/query` (registry / pipeline-not-configured), the admin authentication dependency and the API-key creation path (registry / key-store-not-configured), and `GET /api/v1/admin/health?detail=full` (service-initializing) — none of which it listed, so removing the caveat as written would have made the doc claim a universality four more endpoints still broke. All of them now raise `ErrorResponse.to_envelope()` with a machine-readable code, at the same HTTP status they returned before (the change is shape-only, and no test asserted the bare shape): `ERR-CONFIG-001` is reused for an uninitialised provider registry and `ERR-CONFIG-002` for a missing key store — the codes the global unhandled-exception handler already assigns to internal faults — and `ERR-CONV-001/002/003` (conversation not found / store unavailable / store cannot decrypt turns), `ERR-QUERY-005` (query pipeline unavailable), `ERR-ADMIN-008` (deep-health auth not ready) and `ERR-INDEX-003/004` (reindex target equals the active version / reindex job not found) are new, each registered in `vektra_shared/errors.py`, `_CODE_STATUS_OVERRIDE` and `docs/reference/error-codes.md`. The api.md caveat is removed. One representation gap is left standing on purpose: the wire shape is `{"detail": {"error": {...}}}`, because FastAPI wraps an `HTTPException` detail under `detail`, so the envelope sits one level down for every enveloped endpoint (auth included), while the top-level `{"error": {...}}` only ever comes from the global 500 handler — the api.md example shows the logical envelope, and reconciling that is a separate follow-up. - **config**: `VEKTRA_LLM_PROVIDER=""` was accepted and the stack **booted and served with an empty LLM provider**, deferring the misconfiguration to the first query — which is the one thing the ARCH-057 startup sequence exists to prevent (REQ-011, NFR-009). The field is required, but typed as a bare `str`, and `""` is a perfectly valid `str`, so pydantic never raised and step 1 never fired. It is now `min_length=1`, so an empty value fails at startup with the same structured, remediable error an absent one already produced. Found by running `tests/test_startup.py` for the first time (DEBT-031): the test had asserted this exact behaviour since it was written, believing `docker run -e VAR=` *unset* the variable when it in fact sets it to the empty string — so it had been asserting against a value the product happily accepted. Nobody found out, because nothing ever ran it. This is the third time a suite in this repo turned out to be broken the moment someone executed it. The same test also caught **BUG-025** (filed, not fixed here): the structured error is emitted and then `raise SystemExit(1)` travels out of the ASGI lifespan into uvicorn, which logs a `Traceback` after it — so a typo'd env var yields the good message *and* a stack dump, where NFR-009 asks for the former instead of the latter. Fixing that means validating before `uvicorn.run()` rather than inside the lifespan, i.e. touching the boot path BUG-024 broke, so it is tracked on its own; the assertion is `xfail(strict=True)`, which turns the suite red the moment the fix lands and the marker is left behind. - **startup**: a failed ARCH-057 startup step printed a raw Python `Traceback` next to the structured `[STARTUP ERROR]`, where NFR-009 asks for the remediable message *instead of* the stack dump (BUG-025, REQ-011). The 11-step sequence ran inside the ASGI lifespan, so an aborting step's `raise SystemExit(1)` travelled out into uvicorn, which logged the exception before "Application startup failed." — and it applied to every step, not just config validation. The sequence now lives in a loop-agnostic `_run_startup()` that raises `StartupValidationError` (never `SystemExit`), and the container serves through a new `main()` entrypoint (`python -m vektra_app.main`, wired in `docker/entrypoint.sh`) that runs the validation in the very event loop that will serve, then starts uvicorn with `lifespan="off"`. A failed step logs its `[STARTUP ERROR]` and exits non-zero with no ASGI and no traceback; sharing one loop keeps the async resources built during validation (DB engine, HTTP clients) bound to the loop that serves, and `config.get_loop_factory()` preserves uvloop (the CLI default that uvicorn 0.36 stopped exposing through `setup_event_loop()`). The lifespan keeps validating and raising `SystemExit` for the in-process/TestClient path, so `test_missing_llm_provider_aborts_startup` is untouched; the `xfail(strict=True)` on `test_no_raw_traceback_on_startup_failure` is removed. Proven on the container for a config-validation failure and a database_connectivity failure (structured error, exit 1, no traceback) and a valid boot (all 11 steps logged, `startup_complete`, `Uvicorn running`). The required check names (`CI gate`, `Integration tests + NFR gates`) are unchanged. - **tests**: five suites that no runner executed now run, all five found by the guard above the moment it was pointed at `develop` (DEBT-031). Four are `integration`-marked — `vektra-admin/tests/test_integration.py`, `vektra-index/tests/test_integration.py`, `vektra-index/tests/test_benchmark.py`, `vektra-ingest/tests/test_integration.py` — so every unit run excluded them by `-m "not integration"`, while the only job that ran `-m integration` named `vektra-app/tests/` alone: DEBT-030 wired up vektra-app by hand and left the identical hole open in three neighbouring packages. The fifth is `tests/test_startup.py`, the ARCH-057 startup-validation suite, whose own docstring states it requires the stack "started by integration.yml" — it did not run there, because that workflow names `tests/integration/` and nothing named `tests/`. It was watching the precise surface BUG-024 broke, and it was watching nothing. The `app-integration` job becomes `package-integration` and targets `vektra-*/tests -m integration`, a glob on purpose: the hand-written package list is the thing that has now failed twice, and a package added tomorrow is picked up with no edit. `tests/test_startup.py` joins the job that already brings the stack up. Unlike vektra-app's suites in DEBT-030 these four had **not** rotted — 52 integration tests pass — but that was luck, not a property of the arrangement. The five suites under `tests/` were also missing the `integration` marker they require, which is what made them indistinguishable from unit tests `make test` had merely forgotten; `tests/nfr/test_performance.py` remains deliberately unrun (it needs a live LLM that GitHub Actions does not have) and is the single justified entry in the guard's `EXPECTED_UNRUN`, where a reviewer can see it. The required check names (`CI gate`, `Integration tests + NFR gates`) are unchanged. diff --git a/docs/reference/api.md b/docs/reference/api.md index 7da8e34..b20b962 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -977,12 +977,14 @@ Response (HTTP 202): The source version is not a parameter: the job always reads the active version, since that is the only version the store exposes for reading. -Errors: `400` if `target_index_version` equals the active version (reindexing a version onto itself would overwrite the live chunks instead of writing beside them). +Errors: `400` `ERR-INDEX-003` if `target_index_version` equals the active version (reindexing a version onto itself would overwrite the live chunks instead of writing beside them). ### GET /api/v1/reindex/{job_id}/status Poll a reindex job. +Errors: `404` `ERR-INDEX-004` if no job with that id is visible to the key. + **Scopes**: `admin` ```bash @@ -1315,7 +1317,7 @@ curl -s http://localhost:8000/metrics ## Error responses -Most errors follow a standard envelope (REQ-010): +Every endpoint returns errors in the same envelope (REQ-010): ```json { @@ -1330,12 +1332,5 @@ Most errors follow a standard envelope (REQ-010): } ``` -See [error codes reference](error-codes.md) for the complete list. - -Not every endpoint uses the envelope. `POST /api/v1/reindex`, `GET /api/v1/reindex/{job_id}/status`, conversations and admin conversation turns return FastAPI's bare shape instead: - -```json -{"detail": "Reindex job not found"} -``` - -Clients that parse errors must handle both. The envelope is the intended contract; the bare form is a gap (DEBT-033), not a second contract to rely on. `DELETE /api/v1/index-versions/{version}` uses the envelope throughout, including its `409`. +Clients can branch on `error.code`, which is stable, rather than parsing +`message`. See [error codes reference](error-codes.md) for the complete list. diff --git a/docs/reference/error-codes.md b/docs/reference/error-codes.md index 3b5bd6a..df2f727 100644 --- a/docs/reference/error-codes.md +++ b/docs/reference/error-codes.md @@ -92,8 +92,15 @@ These codes are used by specific components and are not part of the REQ-011 regi | ERR-LEARN-004 | vektra-learn | 409 | Duplicate enrollment (student already enrolled in course) | | ERR-LEARN-005 | vektra-learn | 404 | Conversation not found (GET conversations/turns) | | ERR-LEARN-006 | vektra-learn | 403 | Conversation belongs to a different course/namespace | +| ERR-CONV-001 | vektra-core / vektra-admin | 404 | Conversation not found (core `GET`/`DELETE conversations/{id}`, admin turns) | +| ERR-CONV-002 | vektra-core / vektra-admin | 503 | Persistent conversation store not available (starting up, or none configured) | +| ERR-CONV-003 | vektra-admin | 501 | Configured conversation store cannot return decrypted turns (in-memory store) | +| ERR-QUERY-005 | vektra-core | 503 | Query pipeline provider not available (starting up, or none configured) | +| ERR-ADMIN-008 | vektra-admin | 503 | Service initializing (deep health check auth: registry/key store not ready) | | ERR-INDEX-001 | vektra-index | 409 | Refused: the index version named for deletion is the one currently being served. Nothing was deleted (REQ-064) | | ERR-INDEX-002 | vektra-index | 400 | Invalid index version (below 1) | +| ERR-INDEX-003 | vektra-index | 400 | Reindex target must differ from the active version | +| ERR-INDEX-004 | vektra-index | 404 | Reindex job not found | ## Adding new error codes diff --git a/vektra-admin/src/vektra_admin/api.py b/vektra-admin/src/vektra_admin/api.py index a98caa2..24adf88 100644 --- a/vektra-admin/src/vektra_admin/api.py +++ b/vektra-admin/src/vektra_admin/api.py @@ -41,7 +41,13 @@ ErrorCategory, ErrorResponse, auth_invalid_token, + conversation_not_found, + conversation_store_unavailable, + conversation_turns_unsupported, http_status_for, + key_store_unavailable, + provider_registry_unavailable, + service_initializing, ) # --------------------------------------------------------------------------- @@ -111,12 +117,14 @@ async def _require_any_token( token = credentials.credentials registry = getattr(request.app.state, "registry", None) if registry is None: - raise HTTPException(status_code=500, detail="ProviderRegistry not initialized") + err = provider_registry_unavailable() + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) try: key_store: KeyStoreProvider = registry.get("key_store", "default") except ValueError: - raise HTTPException(status_code=500, detail="Key store not configured") + err = key_store_unavailable() + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) info = await key_store.lookup_by_token(token) if info is None: @@ -199,7 +207,10 @@ async def health( ) # Validate token; fail closed if registry/key_store unavailable if registry is None: - raise HTTPException(status_code=503, detail="Service initializing") + err = service_initializing() + raise HTTPException( + status_code=http_status_for(err), detail=err.to_envelope() + ) try: key_store = registry.get("key_store", "default") info = await key_store.lookup_by_token(credentials.credentials) @@ -211,7 +222,10 @@ async def health( # Expose key_id for AuditMiddleware (CR55) request.state.key_id = info.key_id except ValueError: - raise HTTPException(status_code=503, detail="Service initializing") + err = service_initializing() + raise HTTPException( + status_code=http_status_for(err), detail=err.to_envelope() + ) status_code = 503 if deep.status == "unhealthy" else 200 return Response( @@ -290,8 +304,9 @@ async def create_api_key( else: # Must be a valid admin-scoped key if registry is None: + err = provider_registry_unavailable() raise HTTPException( - status_code=500, detail="ProviderRegistry not initialized" + status_code=http_status_for(err), detail=err.to_envelope() ) try: key_store = registry.get("key_store", "default") @@ -516,22 +531,23 @@ async def get_conversation_turns( """Return decrypted conversation turns with full metadata (admin only).""" registry = getattr(request.app.state, "registry", None) if registry is None: - raise HTTPException(status_code=500, detail="ProviderRegistry not initialized") + err = provider_registry_unavailable() + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) try: conv_store = registry.get("conversation_store", "default") except ValueError: - raise HTTPException(status_code=503, detail="Conversation store not available") + err = conversation_store_unavailable() + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) if not hasattr(conv_store, "get_turns_detail"): - raise HTTPException( - status_code=501, - detail="Conversation decryption not available (in-memory store)", - ) + err = conversation_turns_unsupported() + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) turns = await conv_store.get_turns_detail(conversation_id) if turns is None: - raise HTTPException(status_code=404, detail="Conversation not found") + err = conversation_not_found(conversation_id) + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) # Audit log: sensitive content access request_id = _resolve_request_id(request) diff --git a/vektra-admin/tests/test_admin_turns.py b/vektra-admin/tests/test_admin_turns.py index a87f7b6..c479edf 100644 --- a/vektra-admin/tests/test_admin_turns.py +++ b/vektra-admin/tests/test_admin_turns.py @@ -77,6 +77,7 @@ async def test_returns_404_when_conversation_not_found(): with pytest.raises(HTTPException) as exc_info: await get_conversation_turns(uuid4(), request, MagicMock(), key_info) assert exc_info.value.status_code == 404 + assert exc_info.value.detail["error"]["code"] == "ERR-CONV-001" @pytest.mark.asyncio @@ -166,3 +167,4 @@ async def test_returns_501_for_inmemory_store(): with pytest.raises(HTTPException) as exc_info: await get_conversation_turns(uuid4(), request, MagicMock(), key_info) assert exc_info.value.status_code == 501 + assert exc_info.value.detail["error"]["code"] == "ERR-CONV-003" diff --git a/vektra-core/src/vektra_core/api.py b/vektra-core/src/vektra_core/api.py index cd6fad6..5faffc9 100644 --- a/vektra-core/src/vektra_core/api.py +++ b/vektra-core/src/vektra_core/api.py @@ -36,7 +36,11 @@ ERR_QUERY_003, ErrorCategory, ErrorResponse, + conversation_not_found, + conversation_store_unavailable, http_status_for, + provider_registry_unavailable, + query_pipeline_unavailable, ) from vektra_shared.namespace import resolve_citations_enabled, resolve_grounding_mode from vektra_shared.types import ( @@ -204,12 +208,14 @@ async def query( registry = getattr(request.app.state, "registry", None) if registry is None: - raise HTTPException(status_code=500, detail="ProviderRegistry not initialized") + err = provider_registry_unavailable() + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) try: pipeline = registry.get("query_pipeline", "default") except ValueError: - raise HTTPException(status_code=503, detail="Query pipeline not configured") + err = query_pipeline_unavailable() + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) # Safeguard pre_query (input validation trust boundary, REQ-044) try: @@ -411,21 +417,18 @@ def _get_conversation_store(request: Request) -> PersistentConversationStore: """ registry = getattr(request.app.state, "registry", None) if registry is None: - raise HTTPException(status_code=500, detail="ProviderRegistry not initialized") + err = provider_registry_unavailable() + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) try: store = registry.get("conversation_store", "default") except ValueError: - raise HTTPException( - status_code=503, - detail="Persistent conversation storage is not configured", - ) + err = conversation_store_unavailable() + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) if not isinstance(store, PersistentConversationStore): - raise HTTPException( - status_code=503, - detail="Persistent conversation storage is not configured", - ) + err = conversation_store_unavailable() + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) return store @@ -443,7 +446,8 @@ async def get_conversation( store = _get_conversation_store(request) meta = await store.get_metadata(conversation_id, namespace=_key.namespace_id) if meta is None or meta.get("deleted_at") is not None: - raise HTTPException(status_code=404, detail="Conversation not found") + err = conversation_not_found(conversation_id) + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) return ConversationMetadata( id=meta["id"], @@ -468,7 +472,8 @@ async def delete_conversation( store = _get_conversation_store(request) deleted = await store.soft_delete(conversation_id, namespace=_key.namespace_id) if not deleted: - raise HTTPException(status_code=404, detail="Conversation not found") + err = conversation_not_found(conversation_id) + raise HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) log.info( "conversation_deleted", diff --git a/vektra-core/tests/test_feedback_api.py b/vektra-core/tests/test_feedback_api.py index ab77b34..ce95806 100644 --- a/vektra-core/tests/test_feedback_api.py +++ b/vektra-core/tests/test_feedback_api.py @@ -295,6 +295,7 @@ async def test_get_conversation_not_found(): ) assert resp.status_code == 404 + assert resp.json()["detail"]["error"]["code"] == "ERR-CONV-001" async def test_get_conversation_soft_deleted_returns_404(): @@ -364,6 +365,7 @@ async def test_delete_conversation_not_found(): ) assert resp.status_code == 404 + assert resp.json()["detail"]["error"]["code"] == "ERR-CONV-001" async def test_conversation_no_persistent_store_returns_503(): @@ -381,6 +383,7 @@ async def test_conversation_no_persistent_store_returns_503(): ) assert resp.status_code == 503 + assert resp.json()["detail"]["error"]["code"] == "ERR-CONV-002" # --------------------------------------------------------------------------- diff --git a/vektra-index/src/vektra_index/reindex.py b/vektra-index/src/vektra_index/reindex.py index 38fc22c..68e25a6 100644 --- a/vektra-index/src/vektra_index/reindex.py +++ b/vektra-index/src/vektra_index/reindex.py @@ -31,6 +31,8 @@ from vektra_shared.errors import ( ERR_INDEX_001, ERR_INDEX_002, + ERR_INDEX_003, + ERR_INDEX_004, ActiveIndexVersionError, ErrorCategory, ErrorResponse, @@ -119,6 +121,44 @@ def _invalid_index_version(index_version: int) -> HTTPException: return HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) +def _reindex_target_conflict(target_version: int, active_version: int) -> HTTPException: + """400 for a reindex whose target is the version already being served. + + Reindexing a version onto itself would overwrite the live chunks in place + instead of writing a second copy beside them, defeating the zero-downtime + switch. A client can branch on this code to bump the target and retry. + """ + err = ErrorResponse( + category=ErrorCategory.PERMANENT, + code=ERR_INDEX_003, + message=( + f"target_index_version {target_version} must differ from the active " + f"index version {active_version}." + ), + remediation=( + "Pass a target_index_version different from the active version " + "(VEKTRA_ACTIVE_INDEX_VERSION)." + ), + details={ + "target_index_version": target_version, + "active_index_version": active_version, + }, + ) + return HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) + + +def _reindex_job_not_found(job_id: UUID) -> HTTPException: + """404 for a reindex job id that no key-visible job matches.""" + err = ErrorResponse( + category=ErrorCategory.PERMANENT, + code=ERR_INDEX_004, + message=f"Reindex job '{job_id}' not found.", + remediation="Verify the job_id returned by POST /api/v1/reindex.", + details={"job_id": str(job_id)}, + ) + return HTTPException(status_code=http_status_for(err), detail=err.to_envelope()) + + # --------------------------------------------------------------------------- # Background job # --------------------------------------------------------------------------- @@ -380,10 +420,7 @@ async def trigger_reindex( source_version = vs_config.active_index_version if body.target_index_version == source_version: - raise HTTPException( - status_code=400, - detail="target_index_version must differ from the active index version", - ) + raise _reindex_target_conflict(body.target_index_version, source_version) job_id = uuid4() job = ReindexJobOrm( @@ -431,7 +468,7 @@ async def reindex_status( result = await session.execute(select(ReindexJobOrm).where(*filters)) job = result.scalar_one_or_none() if job is None: - raise HTTPException(status_code=404, detail="Reindex job not found") + raise _reindex_job_not_found(job_id) return ReindexStatusResponse( job_id=str(job.id), diff --git a/vektra-index/tests/test_reindex.py b/vektra-index/tests/test_reindex.py index bb9c6cc..a4640da 100644 --- a/vektra-index/tests/test_reindex.py +++ b/vektra-index/tests/test_reindex.py @@ -282,3 +282,24 @@ def make_session_ctx(): ) assert call_count == 2 + + +class TestReindexErrorEnvelope: + """DEBT-033: the reindex endpoints now raise the REQ-010 envelope.""" + + def test_target_conflict_is_400_with_index_003(self): + from vektra_index.reindex import _reindex_target_conflict + + exc = _reindex_target_conflict(2, 2) + assert exc.status_code == 400 + assert exc.detail["error"]["code"] == "ERR-INDEX-003" + assert exc.detail["error"]["details"]["active_index_version"] == 2 + + def test_job_not_found_is_404_with_index_004(self): + from vektra_index.reindex import _reindex_job_not_found + + job_id = uuid4() + exc = _reindex_job_not_found(job_id) + assert exc.status_code == 404 + assert exc.detail["error"]["code"] == "ERR-INDEX-004" + assert exc.detail["error"]["details"]["job_id"] == str(job_id) diff --git a/vektra-shared/src/vektra_shared/errors.py b/vektra-shared/src/vektra_shared/errors.py index a1fc649..29f4c52 100644 --- a/vektra-shared/src/vektra_shared/errors.py +++ b/vektra-shared/src/vektra_shared/errors.py @@ -109,6 +109,7 @@ def __init__(self, namespace: str, index_version: int) -> None: ERR_QUERY_002 = "ERR-QUERY-002" # LLM unavailable (both primary and fallback failed) ERR_QUERY_003 = "ERR-QUERY-003" # Query too long (exceeds token limit) ERR_QUERY_004 = "ERR-QUERY-004" # Vector store read failed +ERR_QUERY_005 = "ERR-QUERY-005" # Query pipeline provider not registered/available # Configuration errors ERR_CONFIG_001 = "ERR-CONFIG-001" # Missing required configuration variable @@ -129,6 +130,12 @@ def __init__(self, namespace: str, index_version: int) -> None: ERR_ANALYTICS_001 = "ERR-ANALYTICS-001" # Analytics service not available ERR_ANALYTICS_002 = "ERR-ANALYTICS-002" # Trace not found +# Conversation errors (core + admin conversation surface, distinct from the +# learn vertical's ERR-LEARN-005/006 which are scoped to the widget's own path) +ERR_CONV_001 = "ERR-CONV-001" # Conversation not found +ERR_CONV_002 = "ERR-CONV-002" # Persistent conversation store not available +ERR_CONV_003 = "ERR-CONV-003" # Configured store cannot return decrypted turns + # Learn errors (e-learning vertical) ERR_LEARN_001 = "ERR-LEARN-001" # Learn service not available ERR_LEARN_002 = "ERR-LEARN-002" # Enrollment not found @@ -140,6 +147,12 @@ def __init__(self, namespace: str, index_version: int) -> None: # Index errors (ARCH-045, index versioning) ERR_INDEX_001 = "ERR-INDEX-001" # Refused: that index version is the one being served ERR_INDEX_002 = "ERR-INDEX-002" # Invalid index version (< 1) +ERR_INDEX_003 = "ERR-INDEX-003" # Reindex target equals the active version +ERR_INDEX_004 = "ERR-INDEX-004" # Reindex job not found + +# Admin errors (ERR-ADMIN-001..007 live in vektra-admin as string literals; +# 008 is shared because it backs a factory reused within that module) +ERR_ADMIN_008 = "ERR-ADMIN-008" # Deep-health auth: registry/key store not yet ready # --------------------------------------------------------------------------- @@ -167,8 +180,12 @@ def __init__(self, namespace: str, index_version: int) -> None: ERR_LEARN_004: 409, ERR_LEARN_005: 404, ERR_LEARN_006: 403, + ERR_CONV_001: 404, + ERR_CONV_003: 501, # Not Implemented: this store cannot decrypt turns ERR_INDEX_001: 409, # Conflict: the version is live, deleting it is refused ERR_INDEX_002: 400, + ERR_INDEX_003: 400, # Bad Request: target must differ from the active version + ERR_INDEX_004: 404, } @@ -210,3 +227,104 @@ def auth_insufficient_scope( ), request_id=request_id or uuid4(), ) + + +def provider_registry_unavailable(request_id: UUID | None = None) -> ErrorResponse: + """500 for a request that reached a handler before the app finished wiring. + + Reuses ERR-CONFIG-001, the same code the global unhandled-exception handler + assigns to internal failures: from a client's point of view a missing + provider registry is an internal setup fault, not something it can fix. + """ + return ErrorResponse( + category=ErrorCategory.CONFIGURATION, + code=ERR_CONFIG_001, + message="The provider registry is not initialized.", + remediation=( + "This indicates the service did not start up correctly. Check the " + "server logs and restart the service." + ), + request_id=request_id or uuid4(), + ) + + +def conversation_not_found( + conversation_id: object, request_id: UUID | None = None +) -> ErrorResponse: + return ErrorResponse( + category=ErrorCategory.PERMANENT, + code=ERR_CONV_001, + message=f"Conversation '{conversation_id}' not found.", + remediation="Verify the conversation id; it may have been deleted.", + request_id=request_id or uuid4(), + ) + + +def conversation_store_unavailable(request_id: UUID | None = None) -> ErrorResponse: + return ErrorResponse( + category=ErrorCategory.TRANSIENT, + code=ERR_CONV_002, + message="Persistent conversation storage is not available.", + remediation=( + "The service may be starting up, or no persistent conversation store " + "is configured (set VEKTRA_CONVERSATION_KEY). Retry shortly." + ), + request_id=request_id or uuid4(), + ) + + +def conversation_turns_unsupported(request_id: UUID | None = None) -> ErrorResponse: + """501 when the configured store cannot return decrypted turns. + + The in-memory store used in tests/dev has no decryption path; the request is + well-formed but this deployment cannot serve it. + """ + return ErrorResponse( + category=ErrorCategory.CONFIGURATION, + code=ERR_CONV_003, + message="The configured conversation store cannot return decrypted turns.", + remediation=( + "Configure a persistent conversation store with VEKTRA_CONVERSATION_KEY " + "set; the in-memory store cannot decrypt turn history." + ), + request_id=request_id or uuid4(), + ) + + +def query_pipeline_unavailable(request_id: UUID | None = None) -> ErrorResponse: + """503 when the query pipeline provider is not registered yet.""" + return ErrorResponse( + category=ErrorCategory.TRANSIENT, + code=ERR_QUERY_005, + message="The query pipeline is not available.", + remediation=( + "The service may be starting up, or no query pipeline is configured. " + "Retry shortly." + ), + request_id=request_id or uuid4(), + ) + + +def key_store_unavailable(request_id: UUID | None = None) -> ErrorResponse: + """500 when the API key store provider is not configured.""" + return ErrorResponse( + category=ErrorCategory.CONFIGURATION, + code=ERR_CONFIG_002, + message="The API key store is not configured.", + remediation=( + "Verify the key store provider is registered. Check the server logs " + "and restart the service." + ), + request_id=request_id or uuid4(), + ) + + +def service_initializing(request_id: UUID | None = None) -> ErrorResponse: + """503 for a deep-health auth check that ran before the store was ready.""" + return ErrorResponse( + category=ErrorCategory.TRANSIENT, + code=ERR_ADMIN_008, + message="The service is still initializing.", + remediation="Retry shortly; the service is starting up.", + request_id=request_id or uuid4(), + ) diff --git a/vektra-shared/tests/test_errors.py b/vektra-shared/tests/test_errors.py index 123d729..942dde5 100644 --- a/vektra-shared/tests/test_errors.py +++ b/vektra-shared/tests/test_errors.py @@ -3,11 +3,17 @@ from uuid import UUID, uuid4 from vektra_shared.errors import ( + ERR_ADMIN_008, ERR_AUTH_001, ERR_AUTH_002, ERR_AUTH_003, ERR_CONFIG_001, ERR_CONFIG_002, + ERR_CONV_001, + ERR_CONV_002, + ERR_CONV_003, + ERR_INDEX_003, + ERR_INDEX_004, ERR_INGEST_001, ERR_INGEST_002, ERR_INGEST_003, @@ -16,11 +22,19 @@ ERR_QUERY_002, ERR_QUERY_003, ERR_QUERY_004, + ERR_QUERY_005, ErrorCategory, ErrorResponse, auth_insufficient_scope, auth_invalid_token, + conversation_not_found, + conversation_store_unavailable, + conversation_turns_unsupported, http_status_for, + key_store_unavailable, + provider_registry_unavailable, + query_pipeline_unavailable, + service_initializing, ) @@ -200,3 +214,82 @@ def test_auth_insufficient_scope_mentions_scope(self): assert err.code == ERR_AUTH_003 assert "admin" in err.message assert err.remediation != "" + + +class TestDebt033Codes: + """DEBT-033: the pre-existing bare-detail endpoints now use the envelope. + + Each code preserves the HTTP status the endpoint returned before the fix, so + the change is shape-only, not a behaviour change. + """ + + def test_new_codes_have_expected_prefixes(self): + assert ERR_CONV_001.startswith("ERR-CONV-") + assert ERR_CONV_002.startswith("ERR-CONV-") + assert ERR_CONV_003.startswith("ERR-CONV-") + assert ERR_QUERY_005 == "ERR-QUERY-005" + assert ERR_ADMIN_008 == "ERR-ADMIN-008" + assert ERR_INDEX_003 == "ERR-INDEX-003" + assert ERR_INDEX_004 == "ERR-INDEX-004" + + def test_conversation_not_found_is_404(self): + err = conversation_not_found("abc") + assert err.code == ERR_CONV_001 + assert http_status_for(err) == 404 + assert "abc" in err.message + assert err.remediation != "" + + def test_conversation_store_unavailable_is_503(self): + err = conversation_store_unavailable() + assert err.code == ERR_CONV_002 + assert err.category == ErrorCategory.TRANSIENT + assert http_status_for(err) == 503 + assert err.remediation != "" + + def test_conversation_turns_unsupported_is_501(self): + err = conversation_turns_unsupported() + assert err.code == ERR_CONV_003 + assert http_status_for(err) == 501 + assert err.remediation != "" + + def test_query_pipeline_unavailable_is_503(self): + err = query_pipeline_unavailable() + assert err.code == ERR_QUERY_005 + assert http_status_for(err) == 503 + assert err.remediation != "" + + def test_service_initializing_is_503(self): + err = service_initializing() + assert err.code == ERR_ADMIN_008 + assert http_status_for(err) == 503 + assert err.remediation != "" + + def test_key_store_unavailable_is_500(self): + err = key_store_unavailable() + assert err.code == ERR_CONFIG_002 + assert http_status_for(err) == 500 + assert err.remediation != "" + + def test_index_003_maps_to_400(self): + err = ErrorResponse( + category=ErrorCategory.PERMANENT, + code=ERR_INDEX_003, + message="x", + remediation="y", + ) + assert http_status_for(err) == 400 + + def test_index_004_maps_to_404(self): + err = ErrorResponse( + category=ErrorCategory.PERMANENT, + code=ERR_INDEX_004, + message="x", + remediation="y", + ) + assert http_status_for(err) == 404 + + def test_provider_registry_unavailable_reuses_config_001(self): + err = provider_registry_unavailable() + assert err.code == ERR_CONFIG_001 + assert http_status_for(err) == 500 + assert err.remediation != ""