From 9114310cf775addfe7e88ca1e51884b9745a4c46 Mon Sep 17 00:00:00 2001 From: Francesco Vadicamo Date: Wed, 22 Apr 2026 21:54:58 +0000 Subject: [PATCH 1/8] feat(backend): configurable source citation visibility (FEAT-014) Introduces the show_sources flag end-to-end in the backend so instructors can hide the citations block per course without touching the widget. - vektra-shared: VEKTRA_LEARN_SHOW_SOURCES env (default true) + resolve_show_sources() matching the grounding_mode pattern (namespace JSONB config overrides env). - vektra-admin: PATCH /admin/namespaces/{id}/config now accepts show_sources alongside grounding_mode. New ALLOWED_CONFIG_TYPES map supports bool validation (ERR-ADMIN-007 rejects non-bool, including int/string coercion attempts). Partial-merge semantics unchanged. - vektra-app: expose learn_show_sources_default on app.state for the API layer to consume. - vektra-learn: resolve show_sources per request (namespace config > env > default), return the value in CourseQueryResponse, and tag the SSE "sources" event so the streaming widget sees the same flag as the JSON path. The API always returns the full sources list regardless of the flag; visibility is a presentation concern for the widget. Co-Authored-By: Claude Opus 4.7 (1M context) --- vektra-admin/src/vektra_admin/api.py | 25 +++- vektra-admin/tests/test_integration.py | 89 +++++++++++++ vektra-app/src/vektra_app/main.py | 3 + vektra-learn/src/vektra_learn/api.py | 29 ++++- vektra-learn/src/vektra_learn/query.py | 13 +- vektra-learn/tests/test_api.py | 129 +++++++++++++++++++ vektra-learn/tests/test_query.py | 37 ++++++ vektra-shared/src/vektra_shared/config.py | 5 + vektra-shared/src/vektra_shared/namespace.py | 35 ++++- 9 files changed, 357 insertions(+), 8 deletions(-) diff --git a/vektra-admin/src/vektra_admin/api.py b/vektra-admin/src/vektra_admin/api.py index 1fbe30e2..1cc94cfe 100644 --- a/vektra-admin/src/vektra_admin/api.py +++ b/vektra-admin/src/vektra_admin/api.py @@ -50,10 +50,18 @@ # Tight whitelist: unknown keys are rejected (not silently ignored) so that # typos from upstream clients (e.g. Moodle plugin) surface immediately. Each # key is a public contract — extend cautiously. -ALLOWED_CONFIG_KEYS: set[str] = {"grounding_mode"} +# +# Validation model: +# - ALLOWED_CONFIG_VALUES[key]: set of allowed values (enum-style key) +# - ALLOWED_CONFIG_TYPES[key]: required runtime type (type-validated key) +# A key must appear in exactly one of the two maps. +ALLOWED_CONFIG_KEYS: set[str] = {"grounding_mode", "show_sources"} ALLOWED_CONFIG_VALUES: dict[str, set[str]] = { "grounding_mode": {"strict", "hybrid"}, } +ALLOWED_CONFIG_TYPES: dict[str, type] = { + "show_sources": bool, +} log = structlog.get_logger(__name__) @@ -572,11 +580,12 @@ async def patch_namespace_config( ) raise HTTPException(status_code=400, detail=err.to_envelope()) - # --- Validate each value: must be in per-key enum or null (ERR-ADMIN-007) --- + # --- Validate each value: enum or runtime type per key, or null (ERR-ADMIN-007) --- for key, value in body.items(): if value is None: continue # null = remove the key allowed_values = ALLOWED_CONFIG_VALUES.get(key) + allowed_type = ALLOWED_CONFIG_TYPES.get(key) if allowed_values is not None and value not in allowed_values: err = ErrorResponse( category=ErrorCategory.PERMANENT, @@ -587,6 +596,18 @@ async def patch_namespace_config( ), ) raise HTTPException(status_code=400, detail=err.to_envelope()) + if allowed_type is not None and not isinstance(value, allowed_type): + # isinstance(True, bool) is True and isinstance(1, bool) is False, + # so a JSON integer cannot impersonate a bool here. + err = ErrorResponse( + category=ErrorCategory.PERMANENT, + code="ERR-ADMIN-007", + message=f"Invalid value for '{key}': {value!r}.", + remediation=( + f"Value must be of type {allowed_type.__name__}, or null to unset." + ), + ) + raise HTTPException(status_code=400, detail=err.to_envelope()) # --- Load namespace (ERR-ADMIN-005 if missing) --- result = await session.execute( diff --git a/vektra-admin/tests/test_integration.py b/vektra-admin/tests/test_integration.py index d52325f4..acc0b189 100644 --- a/vektra-admin/tests/test_integration.py +++ b/vektra-admin/tests/test_integration.py @@ -768,3 +768,92 @@ async def test_namespace_config_patch_resolves_via_shared_helper( mode = await resolve_grounding_mode(ns_id, fresh_engine, default_mode="strict") assert mode == "hybrid" + + +# --------------------------------------------------------------------------- +# FEAT-014: show_sources per-namespace override +# --------------------------------------------------------------------------- + + +async def test_namespace_config_patch_sets_show_sources_false( + client, bootstrap_key, fresh_engine +): + """PATCH with show_sources=false persists as a bool in namespaces.config.""" + admin_key = await _create_admin_key(client, bootstrap_key) + ns_id = "feat14-show-sources-false" + await _seed_namespace(fresh_engine, ns_id) + + resp = await client.patch( + f"/api/v1/admin/namespaces/{ns_id}/config", + json={"show_sources": False}, + headers={"Authorization": f"Bearer {admin_key}"}, + ) + assert resp.status_code == 200, resp.text + body = resp.json() + assert body["config"] == {"show_sources": False} + + await asyncio.sleep(0.2) + stored = await _read_namespace_config(fresh_engine, ns_id) + assert stored == {"show_sources": False} + + +async def test_namespace_config_patch_sets_show_sources_true( + client, bootstrap_key, fresh_engine +): + """PATCH with show_sources=true persists the explicit override.""" + admin_key = await _create_admin_key(client, bootstrap_key) + ns_id = "feat14-show-sources-true" + await _seed_namespace(fresh_engine, ns_id) + + resp = await client.patch( + f"/api/v1/admin/namespaces/{ns_id}/config", + json={"show_sources": True}, + headers={"Authorization": f"Bearer {admin_key}"}, + ) + assert resp.status_code == 200, resp.text + assert resp.json()["config"] == {"show_sources": True} + + +async def test_namespace_config_patch_rejects_show_sources_non_bool( + client, bootstrap_key, fresh_engine +): + """show_sources is strictly bool: strings and ints are rejected with ERR-ADMIN-007.""" + admin_key = await _create_admin_key(client, bootstrap_key) + ns_id = "feat14-show-sources-nonbool" + await _seed_namespace(fresh_engine, ns_id) + + for bad_value in ["true", 1, "yes"]: + resp = await client.patch( + f"/api/v1/admin/namespaces/{ns_id}/config", + json={"show_sources": bad_value}, + headers={"Authorization": f"Bearer {admin_key}"}, + ) + assert resp.status_code == 400, (bad_value, resp.text) + err = resp.json()["detail"]["error"] + assert err["code"] == "ERR-ADMIN-007" + + +async def test_namespace_config_patch_show_sources_partial_with_grounding( + client, bootstrap_key, fresh_engine +): + """show_sources and grounding_mode coexist; PATCH is partial across both keys.""" + admin_key = await _create_admin_key(client, bootstrap_key) + ns_id = "feat14-mixed-keys" + await _seed_namespace(fresh_engine, ns_id) + + r1 = await client.patch( + f"/api/v1/admin/namespaces/{ns_id}/config", + json={"grounding_mode": "hybrid", "show_sources": False}, + headers={"Authorization": f"Bearer {admin_key}"}, + ) + assert r1.status_code == 200, r1.text + assert r1.json()["config"] == {"grounding_mode": "hybrid", "show_sources": False} + + # Update only show_sources: grounding_mode must survive + r2 = await client.patch( + f"/api/v1/admin/namespaces/{ns_id}/config", + json={"show_sources": True}, + headers={"Authorization": f"Bearer {admin_key}"}, + ) + assert r2.status_code == 200, r2.text + assert r2.json()["config"] == {"grounding_mode": "hybrid", "show_sources": True} diff --git a/vektra-app/src/vektra_app/main.py b/vektra-app/src/vektra_app/main.py index e3175ebd..31bf2948 100644 --- a/vektra-app/src/vektra_app/main.py +++ b/vektra-app/src/vektra_app/main.py @@ -527,6 +527,9 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: # Expose grounding mode default for API layer resolution (FEAT-020) app.state.grounding_mode_default = settings.prompt_grounding_mode + # Expose show_sources default for learn API resolution (FEAT-014) + app.state.learn_show_sources_default = settings.learn_show_sources + if registry.has("learn", "default"): app.state.learn_service = registry.get("learn", "default") app.state.learn_require_enrollment = settings.learn_require_enrollment diff --git a/vektra-learn/src/vektra_learn/api.py b/vektra-learn/src/vektra_learn/api.py index 4d5bac26..ae925bc1 100644 --- a/vektra-learn/src/vektra_learn/api.py +++ b/vektra-learn/src/vektra_learn/api.py @@ -51,7 +51,7 @@ ErrorResponse, http_status_for, ) -from vektra_shared.namespace import resolve_grounding_mode +from vektra_shared.namespace import resolve_grounding_mode, resolve_show_sources from vektra_shared.types import trace_from_dict # Sentinel key_id for learn-originated conversations (JWT auth has no API key). @@ -69,8 +69,14 @@ async def _learn_sse_generator( db_session_factory: Any | None = None, namespace: str = "default", store_traces: bool = False, + show_sources: bool = True, ) -> AsyncGenerator[str, None]: - """Format QueryChunk events as SSE lines for learn endpoint.""" + """Format QueryChunk events as SSE lines for learn endpoint. + + The *show_sources* flag (FEAT-014) is attached to the ``sources`` event + so the widget can decide whether to render citations for this response. + The payload itself is always sent in full for analytics/debugging. + """ log = structlog.get_logger(__name__) try: async for chunk in stream: @@ -80,7 +86,10 @@ async def _learn_sse_generator( payload = json.dumps({"type": "token", "data": chunk.data}) yield f"data: {payload}\n\n" elif chunk.type in ("sources", "error", "trace"): - payload = json.dumps({"type": chunk.type, "data": chunk.data}) + event: dict[str, Any] = {"type": chunk.type, "data": chunk.data} + if chunk.type == "sources": + event["show_sources"] = show_sources + payload = json.dumps(event) yield f"data: {payload}\n\n" # Persist trace (best-effort, BUG-013) if ( @@ -739,6 +748,17 @@ async def course_query( _gm = _default_mode query_req.grounding_mode = _gm + # Resolve show_sources: namespace config > env var > default true (FEAT-014) + _default_show_sources = getattr( + request.app.state, "learn_show_sources_default", True + ) + if _db_factory_gm: + _show_sources = await resolve_show_sources( + namespace, _db_factory_gm, default_value=_default_show_sources + ) + else: + _show_sources = _default_show_sources + if registry is None: err = ErrorResponse( category=ErrorCategory.TRANSIENT, @@ -774,6 +794,7 @@ async def course_query( db_session_factory=_db_factory, namespace=namespace, store_traces=_store_traces, + show_sources=_show_sources, ), media_type="text/event-stream", headers={"Cache-Control": "no-cache", "X-Accel-Buffering": "no"}, @@ -794,4 +815,4 @@ async def course_query( exc_info=True, ) - return pipeline_response_to_course_response(response) + return pipeline_response_to_course_response(response, show_sources=_show_sources) diff --git a/vektra-learn/src/vektra_learn/query.py b/vektra-learn/src/vektra_learn/query.py index 1af025b6..6762d63c 100644 --- a/vektra-learn/src/vektra_learn/query.py +++ b/vektra-learn/src/vektra_learn/query.py @@ -35,6 +35,10 @@ class CourseQueryResponse(BaseModel): sources: list[dict[str, Any]] conversation_id: UUID | None no_relevant_context: bool = False + # FEAT-014: server-resolved citation visibility (namespace config > env > default). + # The API always returns the sources list so analytics and QueryTrace keep + # full information; the flag only instructs the widget whether to render them. + show_sources: bool = True def build_course_query( @@ -59,8 +63,14 @@ def build_course_query( def pipeline_response_to_course_response( resp: QueryResponse, + *, + show_sources: bool = True, ) -> CourseQueryResponse: - """Convert a QueryResponse to a CourseQueryResponse for the learn API.""" + """Convert a QueryResponse to a CourseQueryResponse for the learn API. + + *show_sources* is the server-resolved FEAT-014 flag. The API always + returns the full sources list; the flag is a hint for the widget. + """ return CourseQueryResponse( response_id=resp.response_id, answer=resp.answer, @@ -76,4 +86,5 @@ def pipeline_response_to_course_response( ], conversation_id=resp.conversation_id, no_relevant_context=resp.no_relevant_context, + show_sources=show_sources, ) diff --git a/vektra-learn/tests/test_api.py b/vektra-learn/tests/test_api.py index fe7aa2d3..bdcbf047 100644 --- a/vektra-learn/tests/test_api.py +++ b/vektra-learn/tests/test_api.py @@ -693,3 +693,132 @@ class _EmptyState: from uuid import UUID as _UUID assert isinstance(call_kwargs["request_id"], _UUID) + + +# --------------------------------------------------------------------------- +# FEAT-014: show_sources propagation +# --------------------------------------------------------------------------- + + +class TestShowSourcesPropagation: + """show_sources flag resolution and propagation to non-stream + SSE responses.""" + + def _make_pipeline_mock(self) -> tuple[MagicMock, MagicMock]: + mock_response = MagicMock() + mock_response.response_id = uuid4() + mock_response.answer = "answer" + mock_response.sources = [] + mock_response.conversation_id = None + mock_response.no_relevant_context = False + mock_pipeline = AsyncMock() + mock_pipeline.execute = AsyncMock(return_value=(mock_response, None)) + return mock_pipeline, mock_response + + def _make_app( + self, pipeline: MagicMock, *, show_sources_default: bool + ) -> MagicMock: + mock_app = MagicMock() + mock_app.state.learn_require_enrollment = False + mock_app.state.learn_show_sources_default = show_sources_default + # No DB factory: the resolver falls back to the env-default directly. + mock_app.state.db_session_factory = None + mock_registry = MagicMock() + mock_registry.get.return_value = pipeline + mock_app.state.registry = mock_registry + return mock_app + + async def test_non_stream_response_carries_env_default_true(self): + """When namespace has no override, the env-default (true) reaches the client.""" + from vektra_learn.api import course_query + from vektra_learn.query import CourseQueryRequest + + pipeline, _ = self._make_pipeline_mock() + mock_app = self._make_app(pipeline, show_sources_default=True) + mock_request = MagicMock() + mock_request.app = mock_app + req = CourseQueryRequest(question="Q") + + result = await course_query( + req, + mock_request, + {"sub": "s1", "course_id": "CS101"}, + MagicMock(), + AsyncMock(), + ) + assert result.show_sources is True + + async def test_non_stream_response_carries_env_default_false(self): + """VEKTRA_LEARN_SHOW_SOURCES=false is surfaced when no namespace override exists.""" + from vektra_learn.api import course_query + from vektra_learn.query import CourseQueryRequest + + pipeline, _ = self._make_pipeline_mock() + mock_app = self._make_app(pipeline, show_sources_default=False) + mock_request = MagicMock() + mock_request.app = mock_app + req = CourseQueryRequest(question="Q") + + result = await course_query( + req, + mock_request, + {"sub": "s1", "course_id": "CS101"}, + MagicMock(), + AsyncMock(), + ) + assert result.show_sources is False + + async def test_sse_sources_event_carries_show_sources(self): + """The SSE ``sources`` event payload includes the flag for the widget.""" + import json as _json + + from vektra_learn.api import _learn_sse_generator + + chunk = MagicMock() + chunk.type = "sources" + chunk.data = [{"doc_id": "d1", "chunk_id": "c1", "score": 0.9, "snippet": "s"}] + + async def _stream(): + yield chunk + + request = MagicMock() + request.is_disconnected = AsyncMock(return_value=False) + + events = [] + async for ev in _learn_sse_generator( + _stream(), request, "conv-1", show_sources=False + ): + events.append(ev) + + assert any("sources" in ev for ev in events) + sources_event = next(ev for ev in events if '"type": "sources"' in ev) + # Strip SSE framing ("data: ...\n\n") before parsing. + payload = _json.loads(sources_event.removeprefix("data: ").strip()) + assert payload["type"] == "sources" + assert payload["show_sources"] is False + assert payload["data"] == chunk.data + + async def test_sse_non_sources_events_do_not_include_flag(self): + """Only the ``sources`` event carries the flag; tokens and errors don't.""" + import json as _json + + from vektra_learn.api import _learn_sse_generator + + token_chunk = MagicMock() + token_chunk.type = "token" + token_chunk.data = "hello" + + async def _stream(): + yield token_chunk + + request = MagicMock() + request.is_disconnected = AsyncMock(return_value=False) + + events = [] + async for ev in _learn_sse_generator( + _stream(), request, "conv-1", show_sources=False + ): + events.append(ev) + + token_event = next(ev for ev in events if '"type": "token"' in ev) + payload = _json.loads(token_event.removeprefix("data: ").strip()) + assert "show_sources" not in payload diff --git a/vektra-learn/tests/test_query.py b/vektra-learn/tests/test_query.py index 019ebe92..b3c1f783 100644 --- a/vektra-learn/tests/test_query.py +++ b/vektra-learn/tests/test_query.py @@ -107,3 +107,40 @@ def test_handles_empty_sources(self): result = pipeline_response_to_course_response(resp) assert result.sources == [] + + def test_show_sources_defaults_to_true(self): + """FEAT-014: unset show_sources defaults to True (current behaviour).""" + resp = QueryResponse( + response_id=uuid4(), answer="x", sources=[], conversation_id=None + ) + result = pipeline_response_to_course_response(resp) + assert result.show_sources is True + + def test_show_sources_propagates_false(self): + """FEAT-014: explicit show_sources=False carries through to the response.""" + resp = QueryResponse( + response_id=uuid4(), answer="x", sources=[], conversation_id=None + ) + result = pipeline_response_to_course_response(resp, show_sources=False) + assert result.show_sources is False + + def test_show_sources_false_does_not_strip_sources(self): + """FEAT-014: the API still emits the full sources list regardless of the flag.""" + doc_id = uuid4() + resp = QueryResponse( + response_id=uuid4(), + answer="x", + sources=[ + SourceRef( + doc_id=doc_id, + chunk_id="c1", + score=0.9, + snippet="...", + citation_id=uuid4(), + ), + ], + conversation_id=None, + ) + result = pipeline_response_to_course_response(resp, show_sources=False) + assert len(result.sources) == 1 + assert result.show_sources is False diff --git a/vektra-shared/src/vektra_shared/config.py b/vektra-shared/src/vektra_shared/config.py index e0642522..5b602fae 100644 --- a/vektra-shared/src/vektra_shared/config.py +++ b/vektra-shared/src/vektra_shared/config.py @@ -531,6 +531,11 @@ class VektraSettings(BaseSettings): alias="VEKTRA_LEARN_REQUIRE_ENROLLMENT", description="Require Vektra enrollment record for learn queries. Set to false when an external LMS manages enrollment and authorization.", ) + learn_show_sources: bool = Field( + True, + alias="VEKTRA_LEARN_SHOW_SOURCES", + description="Default visibility of the source-citations section in the widget (FEAT-014). Per-namespace override via namespaces.config.show_sources.", + ) # Observability / retention audit_retention_days: int = Field(90, alias="VEKTRA_AUDIT_RETENTION_DAYS") diff --git a/vektra-shared/src/vektra_shared/namespace.py b/vektra-shared/src/vektra_shared/namespace.py index 7212b269..5229be6e 100644 --- a/vektra-shared/src/vektra_shared/namespace.py +++ b/vektra-shared/src/vektra_shared/namespace.py @@ -1,4 +1,4 @@ -"""Namespace configuration utilities shared across packages (FEAT-020). +"""Namespace configuration utilities shared across packages (FEAT-020, FEAT-014). Uses raw SQL to avoid importing ORM models from other packages (ADR-0005). Both vektra-core and vektra-learn can import from vektra-shared. @@ -47,3 +47,36 @@ async def resolve_grounding_mode( error=str(exc), ) return default_mode + + +async def resolve_show_sources( + namespace: str, + session_factory: Any, + default_value: bool = True, +) -> bool: + """Resolve show_sources: namespace JSONB config > default (FEAT-014). + + The *default_value* parameter should incorporate the env var resolution + (``VEKTRA_LEARN_SHOW_SOURCES``) done by the caller. + + Returns *default_value* on any error (namespace not found, DB error, + non-boolean value in config). + """ + try: + async with session_factory() as session: + result = await session.execute( + text("SELECT config FROM namespaces WHERE id = :ns"), + {"ns": namespace}, + ) + row = result.scalar_one_or_none() + if row and isinstance(row, dict): + ns_value = row.get("show_sources") + if isinstance(ns_value, bool): + return ns_value + except Exception as exc: + log.debug( + "show_sources_resolution_fallback", + namespace=namespace, + error=str(exc), + ) + return default_value From ad2741b044983e0e7ef543624f980289cacd53b0 Mon Sep 17 00:00:00 2001 From: Francesco Vadicamo Date: Wed, 22 Apr 2026 21:55:06 +0000 Subject: [PATCH 2/8] feat(widget): wire data-show-sources override + server-driven default (FEAT-014) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolution chain inside the widget: 1. data-show-sources attribute on the script tag (client-side force) 2. show_sources flag from the learn query response (server-resolved) 3. default true (legacy behaviour when both are absent) api-client.js forwards the server value as a second argument to the onSources callback for both the SSE and the JSON-fallback paths. index.js applies the resolution and only calls ui.addSources() when the effective value is true — keeping ChatUI agnostic. Co-Authored-By: Claude Opus 4.7 (1M context) --- vektra-learn/widget/src/api-client.js | 7 +++++-- vektra-learn/widget/src/index.js | 28 +++++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/vektra-learn/widget/src/api-client.js b/vektra-learn/widget/src/api-client.js index e2c86953..e0fecb8a 100644 --- a/vektra-learn/widget/src/api-client.js +++ b/vektra-learn/widget/src/api-client.js @@ -175,7 +175,9 @@ export class ApiClient { onToken(event.data); receivedTokens = true; } else if (event.type === "sources" && onSources) { - onSources(event.data); + // FEAT-014: forward the server-resolved show_sources hint + // alongside the sources list (undefined on older servers). + onSources(event.data, event.show_sources); } else if (event.type === "done") { if (event.data?.conversation_id) { this._conversationId = event.data.conversation_id; @@ -208,7 +210,8 @@ export class ApiClient { onToken(data.answer); } if (onSources && data.sources && data.sources.length > 0) { - onSources(data.sources); + // FEAT-014: pass server-resolved show_sources hint to caller. + onSources(data.sources, data.show_sources); } if (onDone) onDone(); } diff --git a/vektra-learn/widget/src/index.js b/vektra-learn/widget/src/index.js index 45b7f32e..a76c015d 100644 --- a/vektra-learn/widget/src/index.js +++ b/vektra-learn/widget/src/index.js @@ -19,6 +19,7 @@ * data-powered-by="true" * data-powered-by-text="Supported by University of X" * data-powered-by-url="https://univ.example/help" + * data-show-sources="true" * > * * White-label attributes (all optional): @@ -32,6 +33,10 @@ * Default: "Powered by Vektra" as a link. * data-powered-by-url - overrides the footer link target. Default: * https://vektralabs.github.io + * data-show-sources - "true"/"false" client-side override for the + * source citations section. Absent = defer to the + * server-resolved value (namespace config > env + * default). (FEAT-014) */ import { ApiClient } from "./api-client.js"; @@ -112,6 +117,15 @@ function _clearStored(courseId) { const poweredByText = scriptTag.getAttribute("data-powered-by-text") || null; const poweredByUrl = scriptTag.getAttribute("data-powered-by-url") || null; + // FEAT-014: client-side override for source citation visibility. When the + // attribute is absent, we defer to the server-resolved value that arrives + // with each query response; when present, "false" hides citations and any + // other string re-enables them (including an explicit "true" that forces + // visibility even when the namespace config disables them). + const showSourcesAttr = scriptTag.getAttribute("data-show-sources"); + const clientShowSourcesOverride = + showSourcesAttr === null ? null : showSourcesAttr.toLowerCase() !== "false"; + if (!apiUrl || !courseId || !token) { console.error( "[vektra-chat] Missing required attributes: data-api-url, data-course-id, data-token" @@ -139,8 +153,18 @@ function _clearStored(courseId) { onToken(tokenText) { ui.appendToken(stream, tokenText); }, - onSources(sources) { - ui.addSources(stream, sources); + onSources(sources, serverShowSources) { + // Resolution: client data-show-sources override > server hint > + // default true (legacy behaviour when the server omits the field). + const effective = + clientShowSourcesOverride !== null + ? clientShowSourcesOverride + : typeof serverShowSources === "boolean" + ? serverShowSources + : true; + if (effective) { + ui.addSources(stream, sources); + } }, onNoRelevantContext() { ui.appendToken(stream, ui.noRelevantContextMessage()); From 17bc43025828d71dcd2fbeda905f8fea0792543c Mon Sep 17 00:00:00 2001 From: Francesco Vadicamo Date: Wed, 22 Apr 2026 21:55:15 +0000 Subject: [PATCH 3/8] docs(feat-014): source citation visibility end-to-end - api.md: document show_sources on PATCH /admin/namespaces/{id}/config, covering the new bool type and the fallback to VEKTRA_LEARN_SHOW_SOURCES on null. - CHANGELOG: extend the v0.5.0 entry with the resolution chain and the rationale for keeping sources in the API payload. - BACKLOG: mark FEAT-014 completed (Moodle-side integration deferred to the sibling vektra-moodle plan). Co-Authored-By: Claude Opus 4.7 (1M context) --- .s2s/BACKLOG.md | 14 +++++++------- CHANGELOG.md | 3 ++- docs/reference/api.md | 5 +++-- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/.s2s/BACKLOG.md b/.s2s/BACKLOG.md index 1e9a46bb..07905421 100644 --- a/.s2s/BACKLOG.md +++ b/.s2s/BACKLOG.md @@ -1446,7 +1446,7 @@ For vertical deployments (e.g., a university running Vektra for their students), ### FEAT-014: Configurable source citation visibility -**Status**: draft | **Priority**: medium | **Created**: 2026-03-20 +**Status**: completed | **Priority**: medium | **Created**: 2026-03-20 | **Completed**: 2026-04-22 | **PR**: pending **Origin**: Moodle integration testing - source citations may not be appropriate for all courses **Context**: The widget always displays source citations (document/chunk reference, relevance score, snippet) below each assistant response. Some instructors may prefer to hide them: @@ -1467,12 +1467,12 @@ The API still returns sources in the response regardless of the flag (useful for **Traceability**: ADR-0025, ARCH-063, FEAT-008 -**Acceptance Criteria** (tentative): -- [ ] Global `show_sources` setting with default `true` -- [ ] Per-namespace override (metadata or JWT claim) -- [ ] Widget hides sources section when flag is `false` -- [ ] API response still includes sources regardless (no data loss) -- [ ] Moodle plugin exposes the setting in per-course block configuration +**Acceptance Criteria**: +- [x] Global `show_sources` setting with default `true` (`VEKTRA_LEARN_SHOW_SOURCES`) +- [x] Per-namespace override via `namespaces.config.show_sources` (writable through the existing `PATCH /api/v1/admin/namespaces/{id}/config` whitelist) +- [x] Widget hides sources section when flag is `false` (resolution chain: `data-show-sources` client override > server-resolved value > default `true`) +- [x] API response still includes sources regardless (no data loss) +- [ ] Moodle plugin exposes the setting in per-course block configuration — **deferred to the vektra-moodle sibling plan** --- diff --git a/CHANGELOG.md b/CHANGELOG.md index 6dcebbc0..a3203624 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Added -- **vektra-admin**: `PATCH /api/v1/admin/namespaces/{id}/config` endpoint for instructor configuration. Admin-scoped, whitelisted (v0.5.0 accepts `grounding_mode` only), partial updates, null removes a key. Backs the Moodle block form that lets teachers toggle strict/hybrid RAG per course without admin intervention. +- **vektra-admin**: `PATCH /api/v1/admin/namespaces/{id}/config` endpoint for instructor configuration. Admin-scoped, whitelisted (v0.5.0 accepts `grounding_mode` and `show_sources`), partial updates, null removes a key. Backs the Moodle block form that lets teachers toggle strict/hybrid RAG and source-citation visibility per course without admin intervention. +- **vektra-learn / widget**: configurable source citation visibility (FEAT-014). Resolution chain: `data-show-sources` attribute (client override) > `namespaces.config.show_sources` (per-course) > `VEKTRA_LEARN_SHOW_SOURCES` env var > default `true`. The learn query response (both JSON and the SSE `sources` event) now carries a `show_sources` flag the widget uses to decide whether to render the citations section. The API always returns the full sources list so analytics and QueryTrace keep complete data. - **vektra-learn**: `GET /api/v1/learn/conversations/{id}/turns` JWT-scoped endpoint so the widget can restore a conversation after a page reload. Returns decrypted question/answer + created_at; admin-only metadata is not exposed. 403 on namespace mismatch, 404 on missing. - **vektra-core**: `document_name` field on every source citation, joined from `source_documents.filename` so the widget renders `[1] lecture-07.pdf` instead of chunk UUIDs. Propagates through both `SimpleQueryPipeline` and `AdvancedQueryPipeline`, JSON and SSE paths. Soft-deleted source documents (REQ-057) keep their citation with an `(archived)` suffix so answers stay traceable. - **vektra-learn**: content-access audit entry (`learn_conversation_turns_read`) written on every successful turns fetch via the shared `vektra_shared.audit` interface (NFR-007). diff --git a/docs/reference/api.md b/docs/reference/api.md index 5ebae5d1..fdb9571c 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -163,11 +163,12 @@ Request body (flat dict, one entry per config key): | Field | Type | Allowed values | Description | |-------|------|----------------|-------------| | `grounding_mode` | string or null | `"strict"`, `"hybrid"`, `null` | RAG grounding policy. `null` removes the key and falls back to `VEKTRA_PROMPT_GROUNDING_MODE`. | +| `show_sources` | bool or null | `true`, `false`, `null` | Widget citation visibility (FEAT-014). `null` removes the key and falls back to `VEKTRA_LEARN_SHOW_SOURCES`. The API always returns the full sources list; the flag only instructs the widget whether to render them. | Behavior: - **Partial update**: keys not present in the body are preserved. - **Unknown keys rejected** with `400 ERR-ADMIN-006` (not silently ignored — surfaces typos early). -- **Invalid values rejected** with `400 ERR-ADMIN-007`. +- **Invalid values rejected** with `400 ERR-ADMIN-007` (includes type mismatches, e.g. a JSON string passed for a boolean key). - **Missing namespace** returns `404 ERR-ADMIN-005`. Response (HTTP 200): @@ -175,7 +176,7 @@ Response (HTTP 200): ```json { "namespace_id": "default", - "config": {"grounding_mode": "hybrid"} + "config": {"grounding_mode": "hybrid", "show_sources": false} } ``` From f9b4ccb9b0dccddddfc09547b4b70356702cacfb Mon Sep 17 00:00:00 2001 From: Francesco Vadicamo Date: Fri, 24 Apr 2026 17:17:43 +0000 Subject: [PATCH 4/8] refactor(admin): derive ALLOWED_CONFIG_KEYS from validation maps A hand-maintained whitelist alongside the validation maps was a single forgotten edit away from letting a new key bypass validation entirely. Derive it from the union of ALLOWED_CONFIG_VALUES and ALLOWED_CONFIG_TYPES so the relationship cannot drift, and add a module-level disjoint check that surfaces a duplicated registration at import time. Addresses Gemini inline 3127135275 and CodeRabbit nitpick on api.py:53-64. Co-Authored-By: Claude Opus 4.7 (1M context) --- vektra-admin/src/vektra_admin/api.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/vektra-admin/src/vektra_admin/api.py b/vektra-admin/src/vektra_admin/api.py index 1cc94cfe..a94ae38d 100644 --- a/vektra-admin/src/vektra_admin/api.py +++ b/vektra-admin/src/vektra_admin/api.py @@ -51,17 +51,26 @@ # typos from upstream clients (e.g. Moodle plugin) surface immediately. Each # key is a public contract — extend cautiously. # -# Validation model: +# Validation model: every allowed key MUST be declared in exactly one of the +# two maps below. # - ALLOWED_CONFIG_VALUES[key]: set of allowed values (enum-style key) # - ALLOWED_CONFIG_TYPES[key]: required runtime type (type-validated key) -# A key must appear in exactly one of the two maps. -ALLOWED_CONFIG_KEYS: set[str] = {"grounding_mode", "show_sources"} +# ALLOWED_CONFIG_KEYS is derived (not hand-maintained) so a key cannot reach +# the whitelist without a corresponding validation rule. The disjoint check +# below fails at import if the same key is wired into both maps. ALLOWED_CONFIG_VALUES: dict[str, set[str]] = { "grounding_mode": {"strict", "hybrid"}, } ALLOWED_CONFIG_TYPES: dict[str, type] = { "show_sources": bool, } +assert ALLOWED_CONFIG_VALUES.keys().isdisjoint(ALLOWED_CONFIG_TYPES.keys()), ( + "Each config key must appear in exactly one of " + "ALLOWED_CONFIG_VALUES / ALLOWED_CONFIG_TYPES, never both." +) +ALLOWED_CONFIG_KEYS: set[str] = ( + ALLOWED_CONFIG_VALUES.keys() | ALLOWED_CONFIG_TYPES.keys() +) log = structlog.get_logger(__name__) From 79514127bccf049851ec8eb312f0b44ab12bcd94 Mon Sep 17 00:00:00 2001 From: Francesco Vadicamo Date: Fri, 24 Apr 2026 17:17:51 +0000 Subject: [PATCH 5/8] fix(widget): trim data-show-sources before lowercase comparison Whitespace from server-side templating (e.g. " false ") was silently treated as truthy because the comparison ran on the unmodified attribute value. Trim before lowercasing so the explicit override behaves as documented. Addresses CodeRabbit inline 3127144765. Co-Authored-By: Claude Opus 4.7 (1M context) --- vektra-learn/widget/src/index.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/vektra-learn/widget/src/index.js b/vektra-learn/widget/src/index.js index a76c015d..038a924c 100644 --- a/vektra-learn/widget/src/index.js +++ b/vektra-learn/widget/src/index.js @@ -123,8 +123,12 @@ function _clearStored(courseId) { // other string re-enables them (including an explicit "true" that forces // visibility even when the namespace config disables them). const showSourcesAttr = scriptTag.getAttribute("data-show-sources"); + // Trim before lowercasing so " false " (or surrounding whitespace from + // server-side templating) is honoured, not silently treated as truthy. const clientShowSourcesOverride = - showSourcesAttr === null ? null : showSourcesAttr.toLowerCase() !== "false"; + showSourcesAttr === null + ? null + : showSourcesAttr.trim().toLowerCase() !== "false"; if (!apiUrl || !courseId || !token) { console.error( From c806afc318fc68cd1fab4b2025ed993c48d2406a Mon Sep 17 00:00:00 2001 From: Francesco Vadicamo Date: Fri, 24 Apr 2026 17:18:00 +0000 Subject: [PATCH 6/8] test(admin): cover resolve_show_sources end-to-end via PATCH Mirror test_namespace_config_patch_resolves_via_shared_helper for the FEAT-014 resolver: PATCH show_sources=False, then call the shared helper with default_value=True and assert the namespace override wins. Closes the loop between the write path (admin PATCH) and the read path (vektra_shared.namespace.resolve_show_sources) with no cache between. Addresses CodeRabbit nitpick on test_integration.py:778-859. Co-Authored-By: Claude Opus 4.7 (1M context) --- vektra-admin/tests/test_integration.py | 29 ++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/vektra-admin/tests/test_integration.py b/vektra-admin/tests/test_integration.py index acc0b189..392a2953 100644 --- a/vektra-admin/tests/test_integration.py +++ b/vektra-admin/tests/test_integration.py @@ -857,3 +857,32 @@ async def test_namespace_config_patch_show_sources_partial_with_grounding( ) assert r2.status_code == 200, r2.text assert r2.json()["config"] == {"grounding_mode": "hybrid", "show_sources": True} + + +async def test_namespace_config_patch_resolves_show_sources_via_shared_helper( + client, bootstrap_key, fresh_engine +): + """End-to-end: PATCH show_sources=False → resolve_show_sources returns False. + + Mirrors test_namespace_config_patch_resolves_via_shared_helper for the + grounding_mode path. Proves the FEAT-014 write/read loop works with no + cache or stale resolver in between. + """ + from vektra_shared.namespace import resolve_show_sources + + admin_key = await _create_admin_key(client, bootstrap_key) + ns_id = "feat14-resolver-end-to-end" + await _seed_namespace(fresh_engine, ns_id) + + resp = await client.patch( + f"/api/v1/admin/namespaces/{ns_id}/config", + json={"show_sources": False}, + headers={"Authorization": f"Bearer {admin_key}"}, + ) + assert resp.status_code == 200, resp.text + await asyncio.sleep(0.2) # let audit background task complete before re-using pool + + # default_value=True would be the env-var fallback; the namespace override + # must take precedence and force False. + value = await resolve_show_sources(ns_id, fresh_engine, default_value=True) + assert value is False From 843eaaab36904d825519409f384f3220d2c42257 Mon Sep 17 00:00:00 2001 From: Francesco Vadicamo Date: Fri, 24 Apr 2026 18:28:38 +0000 Subject: [PATCH 7/8] feat(admin): GET /admin/namespaces/{id}/config symmetric to PATCH MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the read side of the namespace-config contract so upstream plugins (starting with the Moodle block edit form) can render "Use default" vs "Override" without re-implementing the resolver chain client-side. The response separates two views: - config: the raw JSONB stored in namespaces.config (empty when nothing has been PATCHed) - resolved: the value queries actually observe, computed via the same fallback chain (namespace > env > hardcoded default) used at runtime The resolved values are computed inline from the loaded ORM row to keep the endpoint to a single DB session — the standalone resolvers in vektra_shared.namespace remain for callers that hold only a namespace id and need to open their own session. The validation logic (enum membership for grounding_mode, isinstance for show_sources) is the same in both paths so behaviour cannot drift. Auth: admin scope. Errors mirror PATCH (404 ERR-ADMIN-005 on missing). 4 integration tests cover empty/stored/missing/scope. Documented in docs/reference/api.md and CHANGELOG. Motivated by the contract checklist in vektra-internal/moodle/feat14-moodle-contract-checklist.md (G1). Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 1 + docs/reference/api.md | 30 ++++++++ vektra-admin/src/vektra_admin/api.py | 95 ++++++++++++++++++++++++++ vektra-admin/tests/test_integration.py | 95 ++++++++++++++++++++++++++ 4 files changed, 221 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a3203624..b75ed22e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - **vektra-admin**: `PATCH /api/v1/admin/namespaces/{id}/config` endpoint for instructor configuration. Admin-scoped, whitelisted (v0.5.0 accepts `grounding_mode` and `show_sources`), partial updates, null removes a key. Backs the Moodle block form that lets teachers toggle strict/hybrid RAG and source-citation visibility per course without admin intervention. - **vektra-learn / widget**: configurable source citation visibility (FEAT-014). Resolution chain: `data-show-sources` attribute (client override) > `namespaces.config.show_sources` (per-course) > `VEKTRA_LEARN_SHOW_SOURCES` env var > default `true`. The learn query response (both JSON and the SSE `sources` event) now carries a `show_sources` flag the widget uses to decide whether to render the citations section. The API always returns the full sources list so analytics and QueryTrace keep complete data. +- **vektra-admin**: `GET /api/v1/admin/namespaces/{id}/config` symmetric to the PATCH. Returns `{namespace_id, config: , resolved: }`. The `resolved` block is what the upstream plugin form (e.g. Moodle block edit) needs to render the "Use default" / "Override" toggle without re-implementing the fallback chain. - **vektra-learn**: `GET /api/v1/learn/conversations/{id}/turns` JWT-scoped endpoint so the widget can restore a conversation after a page reload. Returns decrypted question/answer + created_at; admin-only metadata is not exposed. 403 on namespace mismatch, 404 on missing. - **vektra-core**: `document_name` field on every source citation, joined from `source_documents.filename` so the widget renders `[1] lecture-07.pdf` instead of chunk UUIDs. Propagates through both `SimpleQueryPipeline` and `AdvancedQueryPipeline`, JSON and SSE paths. Soft-deleted source documents (REQ-057) keep their citation with an `(archived)` suffix so answers stay traceable. - **vektra-learn**: content-access audit entry (`learn_conversation_turns_read`) written on every successful turns fetch via the shared `vektra_shared.audit` interface (NFR-007). diff --git a/docs/reference/api.md b/docs/reference/api.md index fdb9571c..493fe120 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -142,6 +142,36 @@ Returns HTTP 204 (no body). ## Namespaces +### GET /api/v1/admin/namespaces/{namespace_id}/config + +Read a namespace's stored config and the effective values after env-default fallback. Symmetric to the PATCH endpoint below. + +**Scopes**: `admin` + +```bash +curl -s -H "Authorization: Bearer $VEKTRA_API_KEY" \ + http://localhost:8000/api/v1/admin/namespaces/default/config | python3 -m json.tool +``` + +Response (HTTP 200): + +```json +{ + "namespace_id": "default", + "config": {"grounding_mode": "hybrid"}, + "resolved": { + "grounding_mode": "hybrid", + "show_sources": true + } +} +``` + +- `config` mirrors the raw JSONB stored in `namespaces.config` (`{}` when no key has ever been set). +- `resolved` is the value queries actually observe at runtime, after the namespace > env > hardcoded-default chain. Always includes every key in the whitelist. Use this for "Use default" / "Override" form rendering in upstream plugins (e.g. the Moodle block edit form). + +Errors: +- `404 ERR-ADMIN-005` if the namespace does not exist. + ### PATCH /api/v1/admin/namespaces/{namespace_id}/config Partially update a namespace's behavioral config (JSONB). Requires `admin` scope. diff --git a/vektra-admin/src/vektra_admin/api.py b/vektra-admin/src/vektra_admin/api.py index a94ae38d..4e18e006 100644 --- a/vektra-admin/src/vektra_admin/api.py +++ b/vektra-admin/src/vektra_admin/api.py @@ -551,6 +551,101 @@ class NamespaceConfigResponse(BaseModel): config: dict[str, Any] +class NamespaceConfigDetailResponse(BaseModel): + """Full namespace config plus the effective values after env-default fallback. + + *config* mirrors what is physically stored in ``namespaces.config``. + *resolved* is what queries actually see at runtime, after the resolvers + in :mod:`vektra_shared.namespace` apply the env-var fallback chain + (namespace JSONB > env var > hardcoded default). Consumers like the + Moodle block edit form use *resolved* to render the "Use default" / + "Override" toggle without having to re-implement the chain client-side. + """ + + namespace_id: str + config: dict[str, Any] + resolved: dict[str, Any] + + +def _resolved_from_stored( + stored: dict[str, Any], + *, + grounding_default: str, + show_sources_default: bool, +) -> dict[str, Any]: + """Apply the namespace > env > hardcoded-default chain to a loaded JSONB. + + This duplicates the trivial validation that + :func:`vektra_shared.namespace.resolve_grounding_mode` / + :func:`resolve_show_sources` perform, but operates on an already-loaded + config dict so the GET endpoint can avoid opening extra DB sessions + (the runtime resolvers exist for callers that hold only a namespace id). + """ + raw_grounding = stored.get("grounding_mode") + grounding = ( + raw_grounding if raw_grounding in {"strict", "hybrid"} else grounding_default + ) + raw_show_sources = stored.get("show_sources") + show_sources = ( + raw_show_sources if isinstance(raw_show_sources, bool) else show_sources_default + ) + return { + "grounding_mode": grounding, + "show_sources": show_sources, + } + + +@router.get( + "/api/v1/admin/namespaces/{namespace_id}/config", + response_model=NamespaceConfigDetailResponse, +) +async def get_namespace_config( + namespace_id: str, + request: Request, + session: AsyncSession = Depends(get_session), + key_info: ApiKeyInfo = Depends(require_scope("admin")), +) -> NamespaceConfigDetailResponse: + """Return the stored namespace config and the effective resolved values. + + Behavior: + - 404 with ``ERR-ADMIN-005`` if the namespace does not exist. + - ``config`` is the raw JSONB (``{}`` when no key has ever been set). + - ``resolved`` is the value that queries observe, computed via the + same resolvers used by the runtime path. Always includes every + key in ``ALLOWED_CONFIG_KEYS``. + """ + from vektra_admin.models import NamespaceOrm # late import + + result = await session.execute( + select(NamespaceOrm).where(NamespaceOrm.id == namespace_id) + ) + ns = result.scalar_one_or_none() + if ns is None: + err = ErrorResponse( + category=ErrorCategory.PERMANENT, + code="ERR-ADMIN-005", + message=f"Namespace '{namespace_id}' not found.", + remediation="Verify the namespace id or create it via the admin UI.", + ) + raise HTTPException(status_code=404, detail=err.to_envelope()) + + stored = dict(ns.ns_config or {}) + resolved = _resolved_from_stored( + stored, + grounding_default=getattr( + request.app.state, "grounding_mode_default", "strict" + ), + show_sources_default=getattr( + request.app.state, "learn_show_sources_default", True + ), + ) + return NamespaceConfigDetailResponse( + namespace_id=namespace_id, + config=stored, + resolved=resolved, + ) + + @router.patch( "/api/v1/admin/namespaces/{namespace_id}/config", response_model=NamespaceConfigResponse, diff --git a/vektra-admin/tests/test_integration.py b/vektra-admin/tests/test_integration.py index 392a2953..50c550bf 100644 --- a/vektra-admin/tests/test_integration.py +++ b/vektra-admin/tests/test_integration.py @@ -886,3 +886,98 @@ async def test_namespace_config_patch_resolves_show_sources_via_shared_helper( # must take precedence and force False. value = await resolve_show_sources(ns_id, fresh_engine, default_value=True) assert value is False + + +# --------------------------------------------------------------------------- +# GET /admin/namespaces/{id}/config (read-side symmetric to PATCH) +# --------------------------------------------------------------------------- + + +async def test_namespace_config_get_returns_empty_with_resolved_defaults( + client, bootstrap_key, fresh_engine +): + """Fresh namespace: config={} and resolved exposes hardcoded defaults.""" + admin_key = await _create_admin_key(client, bootstrap_key) + ns_id = "feat14-get-empty" + await _seed_namespace(fresh_engine, ns_id) + + resp = await client.get( + f"/api/v1/admin/namespaces/{ns_id}/config", + headers={"Authorization": f"Bearer {admin_key}"}, + ) + assert resp.status_code == 200, resp.text + body = resp.json() + assert body["namespace_id"] == ns_id + assert body["config"] == {} + # Hardcoded resolver defaults: grounding_mode="strict", show_sources=True. + # In CI the env vars may shift these; assert the keys are present and bool/string-typed + # rather than a fixed value, so the test stays independent of test env config. + assert set(body["resolved"]) == {"grounding_mode", "show_sources"} + assert body["resolved"]["grounding_mode"] in {"strict", "hybrid"} + assert isinstance(body["resolved"]["show_sources"], bool) + + +async def test_namespace_config_get_reflects_stored_overrides( + client, bootstrap_key, fresh_engine +): + """After PATCH, GET returns the stored values in config and they win in resolved.""" + admin_key = await _create_admin_key(client, bootstrap_key) + ns_id = "feat14-get-overrides" + await _seed_namespace(fresh_engine, ns_id) + + # Store both keys + patch = await client.patch( + f"/api/v1/admin/namespaces/{ns_id}/config", + json={"grounding_mode": "hybrid", "show_sources": False}, + headers={"Authorization": f"Bearer {admin_key}"}, + ) + assert patch.status_code == 200, patch.text + await asyncio.sleep(0.2) + + resp = await client.get( + f"/api/v1/admin/namespaces/{ns_id}/config", + headers={"Authorization": f"Bearer {admin_key}"}, + ) + assert resp.status_code == 200, resp.text + body = resp.json() + assert body["config"] == {"grounding_mode": "hybrid", "show_sources": False} + # Resolver must mirror the stored value (no env fallback when key is set) + assert body["resolved"]["grounding_mode"] == "hybrid" + assert body["resolved"]["show_sources"] is False + + +async def test_namespace_config_get_404_on_missing(client, bootstrap_key): + """Missing namespace → 404 ERR-ADMIN-005, mirroring the PATCH error.""" + admin_key = await _create_admin_key(client, bootstrap_key) + + resp = await client.get( + "/api/v1/admin/namespaces/does-not-exist/config", + headers={"Authorization": f"Bearer {admin_key}"}, + ) + assert resp.status_code == 404, resp.text + err = resp.json()["detail"]["error"] + assert err["code"] == "ERR-ADMIN-005" + + +async def test_namespace_config_get_requires_admin_scope( + client, bootstrap_key, fresh_engine +): + """A non-admin (query-only) key must not be able to read the config.""" + admin_key = await _create_admin_key(client, bootstrap_key) + ns_id = "feat14-get-scope" + await _seed_namespace(fresh_engine, ns_id) + + # Create a query-only key and attempt the GET with it + create = await client.post( + "/api/v1/api-keys", + json={"label": "query-only", "scopes": ["query"]}, + headers={"Authorization": f"Bearer {admin_key}"}, + ) + assert create.status_code == 201, create.text + query_key = create.json()["key"] + + resp = await client.get( + f"/api/v1/admin/namespaces/{ns_id}/config", + headers={"Authorization": f"Bearer {query_key}"}, + ) + assert resp.status_code == 403, resp.text From 437736008e130c8b4ddba890f882e7144f0efd51 Mon Sep 17 00:00:00 2001 From: Francesco Vadicamo Date: Fri, 24 Apr 2026 18:38:37 +0000 Subject: [PATCH 8/8] refactor(admin): derive _resolved_from_stored from whitelist maps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous helper hardcoded the grounding_mode whitelist and built the return dict by hand, so adding a new key to ALLOWED_CONFIG_KEYS would have silently bypassed the GET endpoint's "Always includes every key" invariant — the same drift risk the round 1 review flagged for the whitelist itself. Iterate ALLOWED_CONFIG_KEYS, look up each key's validator in ALLOWED_CONFIG_VALUES / ALLOWED_CONFIG_TYPES, and accept a single defaults dict (asserted to cover every key). Adding a new whitelist key now flows through automatically; the assertion fails fast at runtime if the call site forgot to provide its default. Existing GET tests still pass — response shape is unchanged. Addresses CodeRabbit nitpick on api.py:570-595. Co-Authored-By: Claude Opus 4.7 (1M context) --- vektra-admin/src/vektra_admin/api.py | 60 ++++++++++++++++------------ 1 file changed, 35 insertions(+), 25 deletions(-) diff --git a/vektra-admin/src/vektra_admin/api.py b/vektra-admin/src/vektra_admin/api.py index 4e18e006..64d7c6e2 100644 --- a/vektra-admin/src/vektra_admin/api.py +++ b/vektra-admin/src/vektra_admin/api.py @@ -569,30 +569,38 @@ class NamespaceConfigDetailResponse(BaseModel): def _resolved_from_stored( stored: dict[str, Any], - *, - grounding_default: str, - show_sources_default: bool, + defaults: dict[str, Any], ) -> dict[str, Any]: """Apply the namespace > env > hardcoded-default chain to a loaded JSONB. - This duplicates the trivial validation that - :func:`vektra_shared.namespace.resolve_grounding_mode` / - :func:`resolve_show_sources` perform, but operates on an already-loaded - config dict so the GET endpoint can avoid opening extra DB sessions - (the runtime resolvers exist for callers that hold only a namespace id). + Operates on an already-loaded config dict so the GET endpoint can avoid + opening extra DB sessions (the standalone resolvers in + :mod:`vektra_shared.namespace` remain for callers that hold only a + namespace id). + + Validation is derived from the module-level ``ALLOWED_CONFIG_VALUES`` / + ``ALLOWED_CONFIG_TYPES`` maps and the result is built by iterating + ``ALLOWED_CONFIG_KEYS``, so adding a new whitelist key automatically + flows through here without further edits — the only requirement is that + *defaults* provides an entry for the new key (assertion below). """ - raw_grounding = stored.get("grounding_mode") - grounding = ( - raw_grounding if raw_grounding in {"strict", "hybrid"} else grounding_default - ) - raw_show_sources = stored.get("show_sources") - show_sources = ( - raw_show_sources if isinstance(raw_show_sources, bool) else show_sources_default + assert ALLOWED_CONFIG_KEYS <= defaults.keys(), ( + "defaults must provide a fallback for every key in " + "ALLOWED_CONFIG_KEYS; missing: " + f"{sorted(ALLOWED_CONFIG_KEYS - defaults.keys())}" ) - return { - "grounding_mode": grounding, - "show_sources": show_sources, - } + resolved: dict[str, Any] = {} + for key in ALLOWED_CONFIG_KEYS: + raw = stored.get(key) + allowed_values = ALLOWED_CONFIG_VALUES.get(key) + allowed_type = ALLOWED_CONFIG_TYPES.get(key) + if allowed_values is not None and raw in allowed_values: + resolved[key] = raw + elif allowed_type is not None and isinstance(raw, allowed_type): + resolved[key] = raw + else: + resolved[key] = defaults[key] + return resolved @router.get( @@ -632,12 +640,14 @@ async def get_namespace_config( stored = dict(ns.ns_config or {}) resolved = _resolved_from_stored( stored, - grounding_default=getattr( - request.app.state, "grounding_mode_default", "strict" - ), - show_sources_default=getattr( - request.app.state, "learn_show_sources_default", True - ), + defaults={ + "grounding_mode": getattr( + request.app.state, "grounding_mode_default", "strict" + ), + "show_sources": getattr( + request.app.state, "learn_show_sources_default", True + ), + }, ) return NamespaceConfigDetailResponse( namespace_id=namespace_id,