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..b75ed22e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,9 @@ 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-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 5ebae5d1..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. @@ -163,11 +193,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 +206,7 @@ Response (HTTP 200): ```json { "namespace_id": "default", - "config": {"grounding_mode": "hybrid"} + "config": {"grounding_mode": "hybrid", "show_sources": false} } ``` diff --git a/vektra-admin/src/vektra_admin/api.py b/vektra-admin/src/vektra_admin/api.py index 1fbe30e2..64d7c6e2 100644 --- a/vektra-admin/src/vektra_admin/api.py +++ b/vektra-admin/src/vektra_admin/api.py @@ -50,10 +50,27 @@ # 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: 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) +# 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__) @@ -534,6 +551,111 @@ 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], + defaults: dict[str, Any], +) -> dict[str, Any]: + """Apply the namespace > env > hardcoded-default chain to a loaded JSONB. + + 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). + """ + 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())}" + ) + 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( + "/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, + 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, + config=stored, + resolved=resolved, + ) + + @router.patch( "/api/v1/admin/namespaces/{namespace_id}/config", response_model=NamespaceConfigResponse, @@ -572,11 +694,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 +710,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..50c550bf 100644 --- a/vektra-admin/tests/test_integration.py +++ b/vektra-admin/tests/test_integration.py @@ -768,3 +768,216 @@ 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} + + +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 + + +# --------------------------------------------------------------------------- +# 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 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-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..038a924c 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,19 @@ 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"); + // 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.trim().toLowerCase() !== "false"; + if (!apiUrl || !courseId || !token) { console.error( "[vektra-chat] Missing required attributes: data-api-url, data-course-id, data-token" @@ -139,8 +157,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()); 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