diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 97a3967..e040f1c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,7 +27,7 @@ jobs: - name: Install dev dependencies run: pip install ".[dev]" - name: Ruff lint - run: ruff check backend ingestion tests + run: ruff check backend ingestion router db tests typecheck: timeout-minutes: 30 @@ -45,6 +45,23 @@ jobs: test: timeout-minutes: 30 runs-on: ubuntu-latest + # db/tests/test_schema.py::test_forward_then_rollback_restores_baseline runs + # a live migration round-trip against DATABASE_URL instead of skipping, so + # this job needs an actual Postgres server for that DSN to resolve. + services: + postgres: + image: postgres:16 + env: + POSTGRES_USER: test + POSTGRES_PASSWORD: test + POSTGRES_DB: test + ports: + - 5432:5432 + options: >- + --health-cmd="pg_isready -U test" + --health-interval=5s + --health-timeout=5s + --health-retries=10 env: DATABASE_URL: "postgresql+asyncpg://test:test@localhost:5432/test" QDRANT_URL: "http://localhost:6333" @@ -120,6 +137,26 @@ jobs: ) PY + frontend-test: + timeout-minutes: 30 + runs-on: ubuntu-latest + defaults: + run: + working-directory: frontend + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: "20" + - name: Install frontend dependencies + run: npm ci + - name: ESLint + run: npm run lint + - name: TypeScript typecheck + run: npm run typecheck + - name: Vitest + run: npm run test + frontend-security: timeout-minutes: 30 runs-on: ubuntu-latest diff --git a/.secrets.baseline b/.secrets.baseline index 357466a..97326e5 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -157,7 +157,7 @@ "filename": "backend/app/security/auth.py", "hashed_secret": "665b1e3851eefefa3fb878654292f16597d25155", "is_verified": false, - "line_number": 36 + "line_number": 38 } ], "deploy/k8s/ingress.yaml": [ diff --git a/Dockerfile b/Dockerfile index 13eaeb0..da4272e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -11,6 +11,8 @@ WORKDIR /build COPY pyproject.toml ./ COPY backend ./backend COPY ingestion ./ingestion +COPY router ./router +COPY db ./db COPY README.md ./ RUN pip install --prefix=/install . @@ -30,6 +32,8 @@ WORKDIR /app COPY --from=builder /install /usr/local COPY backend ./backend COPY ingestion ./ingestion +COPY router ./router +COPY db ./db USER app diff --git a/backend/app/adapters/__init__.py b/backend/app/adapters/__init__.py index 29f54cc..2a5d7c0 100644 --- a/backend/app/adapters/__init__.py +++ b/backend/app/adapters/__init__.py @@ -44,7 +44,10 @@ def __getattr__(name: str) -> object: AttributeError: When ``name`` is not a known adapter. """ _lazy_map = { - "SqlAlchemyDocumentStore": ("backend.app.adapters.document_store", "SqlAlchemyDocumentStore"), + "SqlAlchemyDocumentStore": ( + "backend.app.adapters.document_store", + "SqlAlchemyDocumentStore", + ), "ClaudeGenerationLLM": ("backend.app.adapters.generation", "ClaudeGenerationLLM"), "PipelineIngestor": ("backend.app.adapters.ingestor", "PipelineIngestor"), "RouterModuleAdapter": ("backend.app.adapters.router", "RouterModuleAdapter"), diff --git a/backend/app/adapters/generation.py b/backend/app/adapters/generation.py index 64f3cc2..effb245 100644 --- a/backend/app/adapters/generation.py +++ b/backend/app/adapters/generation.py @@ -18,12 +18,15 @@ from __future__ import annotations import asyncio +import logging import os from collections.abc import AsyncIterator from backend.app.api.interfaces import DependencyUnavailable, RoutedSection from backend.app.settings import get_settings +_logger = logging.getLogger(__name__) + DEFAULT_GENERATION_MODEL = os.environ.get("GENERATION_MODEL", "claude-opus-4-8") """Answer-synthesis model id (overridable via the GENERATION_MODEL env var).""" @@ -49,6 +52,10 @@ def _default_max_tokens() -> int: try: return get_settings().generation_max_tokens except Exception: + _logger.warning( + "Failed to read generation_max_tokens from settings; using fallback", + exc_info=True, + ) return 16000 @@ -61,6 +68,10 @@ def _default_thinking_budget() -> int: try: return get_settings().generation_thinking_budget_tokens except Exception: + _logger.warning( + "Failed to read generation_thinking_budget_tokens from settings; using fallback", + exc_info=True, + ) return 5000 diff --git a/backend/app/adapters/ingestor.py b/backend/app/adapters/ingestor.py index 1d2f28f..3e2e192 100644 --- a/backend/app/adapters/ingestor.py +++ b/backend/app/adapters/ingestor.py @@ -1,4 +1,4 @@ -"""Ingestor adapter binding ``ingestion.ingest_document`` to the backend Protocol. +"""Ingestor adapter binding ``ingestion.ingest_document`` to the backend Protocol. The backend :class:`Ingestor` Protocol is ``ingest_document(tenant_id, content, filename, title, domain, no_retention, residency_region, ocr) -> IngestOutcome``. @@ -247,6 +247,7 @@ async def ingest_document( toc = list(result.get("toc") or []) fallback_only = bool(result.get("fallback_only", False)) extracted_title = result.get("title") or title + return IngestOutcome( doc_id=doc_id_str, title=extracted_title, diff --git a/backend/app/api/answer.py b/backend/app/api/answer.py index bdfb760..2da4664 100644 --- a/backend/app/api/answer.py +++ b/backend/app/api/answer.py @@ -218,7 +218,7 @@ async def answer_query( raise document_not_found() if document.fallback_only: - # TODO: product owner to confirm — Option B (whole-document RAG) may replace this + # TODO: product owner to confirm - Option B (whole-document RAG) may replace this raise validation_error( detail=( "This document does not support section-level routing." diff --git a/backend/app/api/dependencies.py b/backend/app/api/dependencies.py index bb0a55a..8f59af4 100644 --- a/backend/app/api/dependencies.py +++ b/backend/app/api/dependencies.py @@ -22,6 +22,7 @@ from __future__ import annotations +import logging import threading from backend.app.api.interfaces import ( @@ -33,6 +34,8 @@ from backend.app.errors import service_unavailable from backend.app.settings import get_settings +logger = logging.getLogger(__name__) + _singleton_lock = threading.RLock() """Single re-entrant process-wide lock for all singleton builders (issue #222). @@ -146,10 +149,10 @@ def _generation_llm_singleton() -> GenerationLLM: from backend.app.settings import get_settings settings = get_settings() - _generation_llm_cache = ClaudeGenerationLLM( # type: ignore[assignment] + _generation_llm_cache = ClaudeGenerationLLM( thinking_budget_tokens=settings.generation_thinking_budget_tokens ) - return _generation_llm_cache # type: ignore[return-value] + return _generation_llm_cache def get_document_store() -> DocumentStore: @@ -207,4 +210,4 @@ async def _dispose_engines() -> None: if engine is not None: await engine.dispose() except Exception: - pass + logger.warning("Error disposing document store engine on shutdown", exc_info=True) diff --git a/backend/app/api/interfaces.py b/backend/app/api/interfaces.py index 5ec22da..a6945c8 100644 --- a/backend/app/api/interfaces.py +++ b/backend/app/api/interfaces.py @@ -18,7 +18,7 @@ from __future__ import annotations -from collections.abc import AsyncGenerator, AsyncIterator +from collections.abc import AsyncIterator from dataclasses import dataclass, field from typing import Protocol, runtime_checkable @@ -214,7 +214,8 @@ async def stream_answer( sections: list[RoutedSection], ) -> AsyncIterator[str]: """Yield answer token fragments for the routed sections.""" - ... + raise NotImplementedError + yield # pragma: no cover - unreachable, marks this an async generator class DependencyUnavailable(Exception): # noqa: N818 - maps to SERVICE_UNAVAILABLE, not an *Error diff --git a/backend/app/api/observability.py b/backend/app/api/observability.py index aa785f2..21f5516 100644 --- a/backend/app/api/observability.py +++ b/backend/app/api/observability.py @@ -1,4 +1,4 @@ -"""Observability surface: the ``/metrics`` scrape endpoint (PRD §21, NFR-009). +"""Observability surface: the ``/metrics`` scrape endpoint (PRD Sec. 21, NFR-009). Exposes the four product KPIs (token reduction %, answer accuracy, routing latency, fallback rate) in Prometheus text exposition format. LangSmith diff --git a/backend/app/api/routing.py b/backend/app/api/routing.py index b2a41c1..8036276 100644 --- a/backend/app/api/routing.py +++ b/backend/app/api/routing.py @@ -87,7 +87,7 @@ async def route_query( """ document_ids = list(dict.fromkeys(_target_document_ids(body))) - # Fix #262: rerank=True is not supported — reject with 422 instead of + # Fix #262: rerank=True is not supported - reject with 422 instead of # silently downgrading and returning HTTP 200. if body.rerank: raise validation_error( diff --git a/backend/app/api/schemas.py b/backend/app/api/schemas.py index eed6337..2f503ab 100644 --- a/backend/app/api/schemas.py +++ b/backend/app/api/schemas.py @@ -180,7 +180,7 @@ class Citation(_Strict): page_end: Annotated[int, Field(ge=1)] @model_validator(mode="after") - def _validate_page_range(self) -> "Citation": + def _validate_page_range(self) -> Citation: """Validate page_start <= page_end. Returns: @@ -267,7 +267,7 @@ class Document(_Strict): doc_id: DocumentId title: str | None = None total_pages: Annotated[int, Field(ge=0)] - domain: Annotated[str, Field(max_length=100, pattern=r"^[a-zA-Z0-9._-]+$")] | None = None + domain: str | None = None residency_region: ResidencyRegion fallback_only: bool created_at: str | None = None diff --git a/backend/app/errors.py b/backend/app/errors.py index 778c538..a33af04 100644 --- a/backend/app/errors.py +++ b/backend/app/errors.py @@ -105,11 +105,11 @@ def unauthorized( """Build a 401 UNAUTHORIZED problem. The ``WWW-Authenticate: Bearer`` header is included on every 401 response - per RFC 6750 §3 so that HTTP clients and API gateways can identify the + per RFC 6750 Sec. 3 so that HTTP clients and API gateways can identify the accepted credential scheme. This is intentional even when only API-key auth is configured: the header is informational and harmless, and omitting it would require threading settings through every error-raise site (issue - #183 — accepted as RFC-compliant behaviour). + #183 - accepted as RFC-compliant behaviour). Args: detail: Occurrence-specific explanation. diff --git a/backend/app/health.py b/backend/app/health.py index 27d270f..6ceea63 100644 --- a/backend/app/health.py +++ b/backend/app/health.py @@ -15,6 +15,8 @@ from fastapi import APIRouter, Request, Response, status from pydantic import BaseModel +from backend.app.errors import ProblemException +from backend.app.security.auth import resolve_principal from backend.app.settings import Settings, get_settings router = APIRouter(tags=["Operations"]) @@ -61,9 +63,10 @@ async def _check_postgres(database_url: str, timeout_seconds: float) -> bool: Returns: True if the authenticated query succeeded, otherwise False. """ - import asyncpg import re + import asyncpg + try: # asyncpg rejects SQLAlchemy dialect prefixes like postgresql+asyncpg:// clean_url = re.sub(r"^postgresql\+\w+://", "postgresql://", database_url) @@ -165,10 +168,13 @@ async def get_readiness(request: Request, response: Response) -> ReadinessStatus avoid disclosing infrastructure details (postgres/qdrant host reachability) to unauthenticated probers. K8s liveness and readiness probes do not send auth headers, so they still receive the correct status code; only the - ``dependencies`` breakdown is hidden from them. + ``dependencies`` breakdown is hidden from them. The credential is fully + verified via :func:`resolve_principal` (signature/expiry/key-hash checks) - + a caller cannot see the breakdown by merely sending an arbitrary header + value, since that would defeat the purpose of hiding it. Args: - request: The incoming request (used to detect auth headers). + request: The incoming request (used to verify the caller's credential). response: The response object whose status code is set to 503 on a degraded result. @@ -181,9 +187,8 @@ async def get_readiness(request: Request, response: Response) -> ReadinessStatus readiness = await evaluate_readiness(settings) if readiness.status != "ready": response.status_code = status.HTTP_503_SERVICE_UNAVAILABLE - has_auth = bool( - request.headers.get("X-API-Key") or request.headers.get("Authorization") - ) - if not has_auth: + try: + resolve_principal(request, settings) + except ProblemException: return ReadinessStatus(status=readiness.status, dependencies={}) return readiness diff --git a/backend/app/main.py b/backend/app/main.py index 0bc036c..b71fb55 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -2,7 +2,7 @@ Builds the real ASGI application: the liveness/readiness probe router (NFR-009, ADR-5), the /v1 routing, answer, and document/compliance routers -(openapi.yaml), the ``/metrics`` observability surface (PRD §21), and the +(openapi.yaml), the ``/metrics`` observability surface (PRD Sec. 21), and the RFC-7807 exception handlers. LangSmith tracing is configured from the environment at construction time. The factory is side-effect free with respect to external dependencies so it can be imported by the test suite and run @@ -16,8 +16,8 @@ from __future__ import annotations import logging +from collections.abc import AsyncIterator from contextlib import asynccontextmanager -from typing import AsyncIterator from fastapi import FastAPI from fastapi.middleware.cors import CORSMiddleware @@ -38,7 +38,7 @@ async def _lifespan(_app: FastAPI) -> AsyncIterator[None]: """Manage application startup and shutdown lifecycle. - On startup no eager initialisation is performed — heavyweight singletons + On startup no eager initialisation is performed - heavyweight singletons (SQLAlchemy engine, Qdrant client, etc.) are created lazily on first request so the process starts quickly and without requiring credentials in the test environment. diff --git a/backend/app/productization/__init__.py b/backend/app/productization/__init__.py index 06023df..33c2b0d 100644 --- a/backend/app/productization/__init__.py +++ b/backend/app/productization/__init__.py @@ -5,10 +5,10 @@ behavior: - ``roi_calculator``: projects monthly LLM-spend savings from the documented - 40-70% token-reduction range (PRD §15.1, §21) for sales/GTM modeling. + 40-70% token-reduction range (PRD Sec. 15.1, Sec. 21) for sales/GTM modeling. - ``metrics``: an in-process registry for the four product KPIs (token reduction %, answer accuracy, routing latency, fallback rate) exposed at - ``/metrics`` for scrape-based observability (PRD §21, NFR-009). + ``/metrics`` for scrape-based observability (PRD Sec. 21, NFR-009). """ from __future__ import annotations diff --git a/backend/app/productization/metrics.py b/backend/app/productization/metrics.py index 630107b..febe2bc 100644 --- a/backend/app/productization/metrics.py +++ b/backend/app/productization/metrics.py @@ -1,4 +1,4 @@ -"""In-process product-KPI metrics registry (PRD §21, NFR-009). +"""In-process product-KPI metrics registry (PRD Sec. 21, NFR-009). Tracks the four product KPIs the PRD measures the system against: diff --git a/backend/app/productization/roi_calculator.py b/backend/app/productization/roi_calculator.py index b044462..bc82137 100644 --- a/backend/app/productization/roi_calculator.py +++ b/backend/app/productization/roi_calculator.py @@ -4,7 +4,7 @@ documented token-reduction range when they place this refinement layer in front of their existing RAG stack. The savings band is anchored to the PRD-verified range of 40-70% average token reduction vs. full-document RAG -(PRD §15.1 / §21), with a documented conservative/expected/optimistic split. +(PRD Sec. 15.1 / Sec. 21), with a documented conservative/expected/optimistic split. This module is a pure-function GTM/sales utility. It performs no I/O, calls no provider, and does not touch routing, ingestion, or persistence. The dollar @@ -124,7 +124,8 @@ def calculate_roi( Args: current_monthly_spend_usd: Current monthly LLM spend in USD. Must be a - finite, non-negative number. + finite, positive number; a zero or negative spend has no + addressable savings to project. context_spend_fraction: Fraction (0.0-1.0] of the spend attributable to retrieved context that this layer can reduce. Defaults to 0.7, a common context-heavy RAG profile. @@ -134,8 +135,8 @@ def calculate_roi( projections plus the expected annual saving. Raises: - ValueError: If the spend is negative/non-finite or the context - fraction is outside the (0.0, 1.0] range. + ValueError: If the spend is not finite or not positive, or the + context fraction is outside the (0.0, 1.0] range. """ if not math.isfinite(current_monthly_spend_usd) or current_monthly_spend_usd <= 0: raise ValueError( diff --git a/backend/app/security/auth.py b/backend/app/security/auth.py index 31e0dd5..6f87757 100644 --- a/backend/app/security/auth.py +++ b/backend/app/security/auth.py @@ -23,14 +23,14 @@ import threading from dataclasses import dataclass -_RATE_LIMIT_KEY_HASH_LENGTH = 16 - import jwt from fastapi import Request from backend.app.errors import unauthorized from backend.app.settings import Settings, get_settings +_RATE_LIMIT_KEY_HASH_LENGTH = 16 + class PrincipalKind(enum.StrEnum): """The credential class a principal authenticated with.""" diff --git a/backend/app/settings.py b/backend/app/settings.py index 386c20a..e028a7c 100644 --- a/backend/app/settings.py +++ b/backend/app/settings.py @@ -64,7 +64,7 @@ class Settings(BaseSettings): default="HS256", alias="JWT_ALGORITHM", description=( - "JWT signing algorithm — restricted to HMAC-SHA variants" + "JWT signing algorithm - restricted to HMAC-SHA variants" " to prevent alg:none bypass." ), ) diff --git a/db/models.py b/db/models.py index b915105..ea920cc 100644 --- a/db/models.py +++ b/db/models.py @@ -118,14 +118,16 @@ class Document(Base): __table_args__ = ( CheckConstraint("total_pages >= 0", name="documents_total_pages_nonneg"), - # Canonical DDL: migrations/002_indexes_constraints.sql (#104). - # Keep the partial predicate (content_hash IS NOT NULL) in sync with that migration. + # Canonical DDL: migrations/004_fix_tenant_hash_partial_index.sql (#206). + # Keep the partial predicate in sync with that migration: it excludes + # rows with no hash AND tombstoned rows, so a soft-deleted document no + # longer blocks re-upload of the same content under the same tenant. Index( "uq_documents_tenant_hash", "tenant_id", "content_hash", unique=True, - postgresql_where=content_hash.isnot(None), + postgresql_where=content_hash.isnot(None) & tombstoned_at.is_(None), ), Index("idx_documents_tenant_id", "tenant_id"), Index("idx_documents_domain", "domain"), diff --git a/db/qdrant_bootstrap.py b/db/qdrant_bootstrap.py index e5f3e92..651df80 100644 --- a/db/qdrant_bootstrap.py +++ b/db/qdrant_bootstrap.py @@ -143,7 +143,14 @@ def _verify_vector_size(client: QdrantClient, collection_name: str) -> None: intervention (delete the collection and restart). """ collection_info = client.get_collection(collection_name) - actual_size = collection_info.config.params.vectors.size + vectors = collection_info.config.params.vectors + if not isinstance(vectors, qm.VectorParams): + raise RuntimeError( + f"Qdrant collection '{collection_name}' does not use a single unnamed " + "vector config (found named-vectors or no vectors); this collection was " + "not created by bootstrap_collection. Delete it and restart." + ) + actual_size = vectors.size if actual_size != VECTOR_SIZE: raise RuntimeError( f"Qdrant collection '{collection_name}' has vector size {actual_size} " @@ -179,7 +186,8 @@ def bootstrap_collection( collection_name=collection_name, vectors_config=vectors_config(), ) - _verify_vector_size(client, collection_name) + else: + _verify_vector_size(client, collection_name) indexed = ensure_payload_indexes(client, collection_name=collection_name) return BootstrapResult(created=not exists, indexed_fields=indexed) diff --git a/db/tests/conftest.py b/db/tests/conftest.py index d63ff1e..28dd9f7 100644 --- a/db/tests/conftest.py +++ b/db/tests/conftest.py @@ -5,6 +5,14 @@ import pathlib import sys +import pytest + _PROJECT_ROOT = pathlib.Path(__file__).resolve().parents[2] if str(_PROJECT_ROOT) not in sys.path: sys.path.insert(0, str(_PROJECT_ROOT)) + + +@pytest.fixture +def anyio_backend() -> str: + """Run anyio-based async tests on asyncio only (no trio dependency).""" + return "asyncio" diff --git a/db/tests/test_qdrant_bootstrap.py b/db/tests/test_qdrant_bootstrap.py index 74a081a..c627ab6 100644 --- a/db/tests/test_qdrant_bootstrap.py +++ b/db/tests/test_qdrant_bootstrap.py @@ -8,6 +8,8 @@ from __future__ import annotations +from types import SimpleNamespace + import pytest from qdrant_client import models as qm @@ -22,6 +24,32 @@ ) +def _fake_collection_info(size: int) -> SimpleNamespace: + """Build a minimal stand-in exposing only the attribute path bootstrap reads. + + Real ``qm.CollectionInfo`` construction requires many unrelated required + fields that drift across qdrant-client versions, so everything except + ``vectors`` is a plain stand-in; ``vectors`` itself must be a real + ``qm.VectorParams`` since ``_verify_vector_size`` does an ``isinstance`` + check against it (to reject named-vector collections it cannot support). + """ + return SimpleNamespace( + config=SimpleNamespace( + params=SimpleNamespace( + vectors=qm.VectorParams(size=size, distance=qm.Distance.COSINE) + ) + ) + ) + + +def test_existing_collection_with_mismatched_vector_size_raises() -> None: + """Bootstrap raises when an existing collection's vector size differs.""" + client = FakeQdrantClient(existing=True) + client.get_collection = lambda collection_name: _fake_collection_info(768) # type: ignore[method-assign] + with pytest.raises(RuntimeError, match="vector size"): + bootstrap_collection(client) # type: ignore[arg-type] + + class FakeQdrantClient: """Minimal in-memory stand-in for QdrantClient used in bootstrap tests. @@ -62,6 +90,10 @@ def create_payload_index( """Record a payload-index call for ``field_name``.""" self.payload_indexes.append(field_name) + def get_collection(self, collection_name: str) -> SimpleNamespace: + """Return collection info reporting a vector size matching VECTOR_SIZE.""" + return _fake_collection_info(VECTOR_SIZE) + def test_vector_size_matches_embedding_model() -> None: """Vector size is 1536 and distance is Cosine (text-embedding-3-small).""" diff --git a/db/tests/test_schema.py b/db/tests/test_schema.py index a9d55bc..f578604 100644 --- a/db/tests/test_schema.py +++ b/db/tests/test_schema.py @@ -38,7 +38,7 @@ _FORWARD_SQL_004 = _MIGRATIONS_DIR / "004_fix_tenant_hash_partial_index.sql" _DOWN_SQL_004 = _MIGRATIONS_DIR / "004_fix_tenant_hash_partial_index.down.sql" -# Ordered list of all forward migrations — used by the live integration test +# Ordered list of all forward migrations - used by the live integration test # to apply them in dependency order. _ALL_FORWARD_MIGRATIONS: list[pathlib.Path] = [ _FORWARD_SQL, @@ -268,7 +268,10 @@ def test_migration_002_declares_tenant_hash_unique_index() -> None: r"uq_documents_tenant_hash[\s\S]*content_hash\s+IS\s+NOT\s+NULL", ddl, re.IGNORECASE, - ), "uq_documents_tenant_hash in migration 002 must be a partial index on content_hash IS NOT NULL" + ), ( + "uq_documents_tenant_hash in migration 002 must be a partial index " + "on content_hash IS NOT NULL" + ) def test_migration_003_declares_erasure_outbox_fk() -> None: @@ -327,28 +330,62 @@ def test_migration_004_erasure_outbox_fk_is_cascade_in_model() -> None: not os.environ.get("DATABASE_URL"), reason="DATABASE_URL not set; live forward+rollback migration test skipped.", ) -def test_forward_then_rollback_restores_baseline() -> None: +@pytest.mark.anyio +async def test_forward_then_rollback_restores_baseline() -> None: """Apply all forward migrations in order, then rollback 001; expect clean state. Applies migrations 001 through 004 in dependency order against a live Postgres instance, then runs the 001 rollback. All migrations are tested, not just 001. Skipped without ``DATABASE_URL`` so the suite stays offline-green. + + ``DATABASE_URL`` carries the ``postgresql+asyncpg://`` async DSN (per + ``asyncpg`` being the project's only Postgres driver dependency), so this + uses ``create_async_engine`` rather than the sync ``create_engine`` - + mixing a sync engine with an async-only driver raises + ``sqlalchemy.exc.MissingGreenlet``. Each migration script is applied via + the raw asyncpg connection rather than ``conn.execute(text(...))``: + SQLAlchemy's asyncpg dialect always prepares statements, and Postgres's + extended query protocol rejects multiple commands in one prepared + statement, which every multi-statement migration file here is. """ - from sqlalchemy import create_engine, inspect, text - - engine = create_engine(os.environ["DATABASE_URL"]) - # Apply all migrations in order (001 → 002 → 003 → 004). - for migration_path in _ALL_FORWARD_MIGRATIONS: - migration_sql = _read(migration_path) - with engine.begin() as conn: - conn.execute(text(migration_sql)) - inspector = inspect(engine) - assert inspector.has_table("documents") - assert inspector.has_table("sections") - # Roll back 001 (which cascades to drop all dependent objects). - down = _read(_DOWN_SQL) - with engine.begin() as conn: - conn.execute(text(down)) - inspector = inspect(engine) - assert not inspector.has_table("sections") - assert not inspector.has_table("documents") + from sqlalchemy import inspect + from sqlalchemy.ext.asyncio import AsyncConnection, create_async_engine + + async def _exec_script(conn: AsyncConnection, sql: str) -> None: + """Run a (possibly multi-statement) SQL script via the raw driver connection.""" + raw_connection = await conn.get_raw_connection() + driver_connection = raw_connection.driver_connection + assert driver_connection is not None + await driver_connection.execute(sql) + + engine = create_async_engine(os.environ["DATABASE_URL"]) + try: + # Apply all migrations in order (001 -> 002 -> 003 -> 004). + for migration_path in _ALL_FORWARD_MIGRATIONS: + migration_sql = _read(migration_path) + async with engine.begin() as conn: + await _exec_script(conn, migration_sql) + async with engine.connect() as conn: + has_documents = await conn.run_sync( + lambda sync_conn: inspect(sync_conn).has_table("documents") + ) + has_sections = await conn.run_sync( + lambda sync_conn: inspect(sync_conn).has_table("sections") + ) + assert has_documents + assert has_sections + # Roll back 001 (which cascades to drop all dependent objects). + down = _read(_DOWN_SQL) + async with engine.begin() as conn: + await _exec_script(conn, down) + async with engine.connect() as conn: + sections_remain = await conn.run_sync( + lambda sync_conn: inspect(sync_conn).has_table("sections") + ) + documents_remain = await conn.run_sync( + lambda sync_conn: inspect(sync_conn).has_table("documents") + ) + assert not sections_remain + assert not documents_remain + finally: + await engine.dispose() diff --git a/deploy/k8s/ingress.yaml b/deploy/k8s/ingress.yaml index 9def19f..b40180b 100644 --- a/deploy/k8s/ingress.yaml +++ b/deploy/k8s/ingress.yaml @@ -5,6 +5,13 @@ # api and frontend Services are ClusterIP-only. SSE on /api/v1/answer requires # proxy buffering OFF and a generous read timeout (annotations below target the # NGINX ingress controller; adjust for your controller). +# +# The backend mounts its routers at /health, /ready, /metrics, and /v1/* +# (no /api prefix — see backend/app/main.py). rewrite-target + the regex +# capture group below strip the /api prefix before proxying, mirroring what +# Caddyfile.example does with handle_path. Without this, /api/v1/answer would +# be forwarded to the pod as a literal /api/v1/answer path, which does not +# exist on the backend and would 404. apiVersion: networking.k8s.io/v1 kind: Ingress metadata: @@ -15,6 +22,8 @@ metadata: nginx.ingress.kubernetes.io/force-ssl-redirect: "true" nginx.ingress.kubernetes.io/proxy-buffering: "off" nginx.ingress.kubernetes.io/proxy-read-timeout: "300" + nginx.ingress.kubernetes.io/use-regex: "true" + nginx.ingress.kubernetes.io/rewrite-target: /$2 nginx.ingress.kubernetes.io/configuration-snippet: | add_header Strict-Transport-Security "max-age=31536000; includeSubDomains" always; spec: @@ -27,8 +36,8 @@ spec: - host: app.example.com http: paths: - - path: /api - pathType: Prefix + - path: /api(/|$)(.*) + pathType: ImplementationSpecific backend: service: name: rag-api diff --git a/ingestion/chunker.py b/ingestion/chunker.py index 420bf85..4960ee4 100644 --- a/ingestion/chunker.py +++ b/ingestion/chunker.py @@ -99,7 +99,7 @@ def _section_page_text(doc: ParsedDocument, entry: TocEntry) -> list[tuple[int, continue if not page.text.strip(): logger.warning( - "Page %d in section '%s' has no extractable text (image-only?) — skipping", + "Page %d in section '%s' has no extractable text (image-only?) - skipping", page.number, getattr(entry, "title", "unknown"), ) diff --git a/ingestion/parser.py b/ingestion/parser.py index b8b8532..24211b4 100644 --- a/ingestion/parser.py +++ b/ingestion/parser.py @@ -19,7 +19,7 @@ logger = logging.getLogger(__name__) -MAX_PDF_BYTES: int = 100 * 1024 * 1024 # 100 MB — fix #246 +MAX_PDF_BYTES: int = 100 * 1024 * 1024 # 100 MB - fix #246 class ParseError(ValueError): diff --git a/ingestion/pipeline.py b/ingestion/pipeline.py index 801340d..edf24f3 100644 --- a/ingestion/pipeline.py +++ b/ingestion/pipeline.py @@ -1,4 +1,4 @@ -"""Ingestion pipeline orchestration (STORY-003/008/009/011). +"""Ingestion pipeline orchestration (STORY-003/008/009/011). Implements the data-engineer-owned ``ingest_document`` entry point that backend calls: parse -> TOC -> section-aware chunk -> embed -> upsert, writing section @@ -127,6 +127,7 @@ def upsert_document( content_hash_value: str | None, ingest_status: str, fallback_only: bool, + residency_region: str = "GLOBAL", ) -> None: """Create or update the document row (tenant-scoped). @@ -139,6 +140,7 @@ def upsert_document( content_hash_value: Content hash (None in no-retention mode). ingest_status: One of ``db.models.INGEST_STATUS_VALUES``. fallback_only: True when no structure was detected (Scenario C). + residency_region: DPDP data-residency region (FR-028); defaults to GLOBAL. """ ... @@ -188,11 +190,12 @@ def upsert_document_and_replace_sections( ... def update_residency_region(self, doc_id: str, tenant_id: str, residency_region: str) -> None: - """Update the residency_region for a document after ingestion (DPDP FR-028). + """Manually correct the residency_region for an already-ingested document (DPDP FR-028). - Called after the pipeline completes to stamp the data-residency region that - the synchronous pipeline cannot persist because ``IngestInput`` carries no - residency field. + ``ingest_document`` now persists ``residency_region`` on every write path via + ``upsert_document`` and ``upsert_document_and_replace_sections``, so this method + is no longer part of the ingest write path; it remains as an administrative + correction hook for out-of-band residency reassignment. Args: doc_id: Document primary key. @@ -544,17 +547,9 @@ def ingest_document( content_hash_value=hash_value, ingest_status="indexed", fallback_only=False, + residency_region=doc.residency_region, ) else: - # Fix #180: for no_retention documents, residency_region is not passed - # to upsert_document because the Protocol does not carry it yet. - # The ingestor adapter (PipelineIngestor) calls update_residency_region - # after the pipeline completes to stamp the region on the document row, - # but only when no_retention=False (DPDP FR-028). For true no_retention - # documents, content is not persisted so residency enforcement is - # inherently limited; the TODO below tracks the gap. - # TODO: extend upsert_document Protocol to accept residency_region so - # ephemeral documents still record their intended region for audit. section_store.upsert_document( doc_id=doc_id, tenant_id=doc.tenant_id, @@ -564,6 +559,7 @@ def ingest_document( content_hash_value=None if doc.no_retention else hash_value, ingest_status=ingest_status, fallback_only=fallback_only_flag, + residency_region=doc.residency_region, ) return IngestResult( diff --git a/ingestion/tests/conftest.py b/ingestion/tests/conftest.py index 8535577..cebcf93 100644 --- a/ingestion/tests/conftest.py +++ b/ingestion/tests/conftest.py @@ -138,6 +138,7 @@ def upsert_document( content_hash_value: str | None, ingest_status: str, fallback_only: bool, + residency_region: str = "GLOBAL", ) -> None: """Create or update a document row and index its content hash. @@ -150,6 +151,7 @@ def upsert_document( content_hash_value: Content hash (None in no-retention). ingest_status: Ingest status enum value. fallback_only: Scenario C flag. + residency_region: DPDP data-residency region; defaults to GLOBAL. """ self.documents[doc_id] = { "doc_id": doc_id, @@ -160,6 +162,7 @@ def upsert_document( "content_hash": content_hash_value, "ingest_status": ingest_status, "fallback_only": fallback_only, + "residency_region": residency_region, } if content_hash_value is not None: self.hash_index[(tenant_id, content_hash_value)] = doc_id @@ -222,6 +225,7 @@ def upsert_document_and_replace_sections( content_hash_value=content_hash_value, ingest_status=ingest_status, fallback_only=fallback_only, + residency_region=residency_region, ) return self.replace_sections(tenant_id, doc_id, rows) diff --git a/ingestion/toc_extractor.py b/ingestion/toc_extractor.py index bdb96c8..7f43db0 100644 --- a/ingestion/toc_extractor.py +++ b/ingestion/toc_extractor.py @@ -309,7 +309,7 @@ def extract_toc( refined = llm_refiner.refine(heuristic) if not refined and heuristic: logger.warning( - "LLM refiner returned empty result for %d heuristic entries — " + "LLM refiner returned empty result for %d heuristic entries - " "falling back to Scenario C. doc_id=%s", len(heuristic), getattr(doc, "doc_id", "unknown"), diff --git a/pyproject.toml b/pyproject.toml index 980047c..1b532a3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,6 +39,9 @@ dependencies = [ [project.optional-dependencies] dev = [ + # >=83.0.0 remediates PYSEC-2026-3447; pinned explicitly since CI's base + # image ships an older setuptools that pip does not otherwise upgrade. + "setuptools>=83.0.0", "ruff>=0.4,<1.0", "mypy>=1.10,<2.0", "pytest>=9.0.3,<10.0", @@ -50,7 +53,8 @@ dev = [ ] [build-system] -requires = ["setuptools>=68", "wheel"] +# >=83.0.0 remediates PYSEC-2026-3447. +requires = ["setuptools>=83.0.0", "wheel"] build-backend = "setuptools.build_meta" [tool.setuptools] diff --git a/router/graph.py b/router/graph.py index 9a62f4e..e653390 100644 --- a/router/graph.py +++ b/router/graph.py @@ -245,9 +245,6 @@ def _apply_threshold( high = [item for item in eligible if item.confidence >= confidence_threshold] if high: return high[:max_sections] - # Only fall back to mid-band for sub-HIGH_CONFIDENCE thresholds - if confidence_threshold >= HIGH_CONFIDENCE: - return [] # No section met the strict threshold -> fallback mid = [item for item in eligible if item.confidence < confidence_threshold] return mid[:max_sections] diff --git a/router/prompts.py b/router/prompts.py index 9a939e0..3eec665 100644 --- a/router/prompts.py +++ b/router/prompts.py @@ -146,7 +146,7 @@ def build_router_messages( A tuple ``(system_prompt, messages)`` where ``messages`` is the few-shot-primed user turn list for the Messages API. """ - safe_query = query.replace("<<<", "< < <").replace(">>>", "> > >") + safe_query = query.replace("<<", "< <").replace(">>", "> >") allowed_repr = "[" + ", ".join(f'"{sid}"' for sid in allowed_section_ids) + "]" user_turn = ( f"ALLOWED_SECTION_IDS: {allowed_repr}\n\n" diff --git a/router/schema.py b/router/schema.py index 920fb4f..a6e387f 100644 --- a/router/schema.py +++ b/router/schema.py @@ -41,7 +41,7 @@ class RankedSection(BaseModel): confidence: Router confidence in [0.0, 1.0] that the section is relevant. """ - model_config = ConfigDict(extra="ignore") + model_config = ConfigDict(extra="forbid") section_id: str confidence: float = Field(ge=0.0, le=1.0) diff --git a/router/tests/test_router.py b/router/tests/test_router.py index fdc603d..c377fa6 100644 --- a/router/tests/test_router.py +++ b/router/tests/test_router.py @@ -339,7 +339,7 @@ async def test_llm_error_during_route_triggers_fallback(golden_toc: list[dict]) "doc_abc123", golden_toc, tenant_id="tenant_test", - llm=error_llm, # type: ignore[arg-type] + llm=error_llm, confidence_threshold=0.7, max_sections=3, ) diff --git a/tests/test_health.py b/tests/test_health.py index d05e628..e196441 100644 --- a/tests/test_health.py +++ b/tests/test_health.py @@ -12,7 +12,9 @@ from backend.app import health as health_module from backend.app.main import create_app +from backend.app.security.auth import get_api_key_store from backend.app.settings import Settings +from tests.conftest import TENANT_A @pytest.fixture @@ -38,9 +40,11 @@ def test_ready_returns_503_when_dependency_down( ) -> None: """Readiness must return 503 when a configured dependency is down. - Sends an API-key header so the dependency breakdown is included in the - response (authenticated callers receive the full ``dependencies`` map per - the auth-gating logic in ``get_readiness``). + Sends a registered, valid API-key header so the dependency breakdown is + included in the response (authenticated callers receive the full + ``dependencies`` map per the auth-gating logic in ``get_readiness``, which + verifies the credential via :func:`resolve_principal` rather than merely + checking for header presence). """ async def fake_evaluate(settings: Settings) -> health_module.ReadinessStatus: @@ -50,6 +54,7 @@ async def fake_evaluate(settings: Settings) -> health_module.ReadinessStatus: ) monkeypatch.setattr(health_module, "evaluate_readiness", fake_evaluate) + get_api_key_store().register("probe-key", TENANT_A, "key_probe") response = client.get("/ready", headers={"X-API-Key": "probe-key"}) assert response.status_code == 503 body = response.json() @@ -57,6 +62,30 @@ async def fake_evaluate(settings: Settings) -> health_module.ReadinessStatus: assert body["dependencies"]["postgres"] == "down" +def test_ready_hides_dependencies_for_unregistered_api_key( + client: TestClient, monkeypatch: pytest.MonkeyPatch +) -> None: + """An unregistered/bogus API key must not unlock the dependency breakdown. + + The credential is fully verified (hash lookup), not merely checked for + presence, so an arbitrary header value is treated the same as no header + at all. + """ + + async def fake_evaluate(settings: Settings) -> health_module.ReadinessStatus: + return health_module.ReadinessStatus( + status="degraded", + dependencies={"postgres": "down", "qdrant": "up"}, + ) + + monkeypatch.setattr(health_module, "evaluate_readiness", fake_evaluate) + response = client.get("/ready", headers={"X-API-Key": "never-registered"}) + assert response.status_code == 503 + body = response.json() + assert body["status"] == "degraded" + assert body["dependencies"] == {} + + def test_ready_returns_200_when_deps_up( client: TestClient, monkeypatch: pytest.MonkeyPatch ) -> None: diff --git a/tests/test_health_internals.py b/tests/test_health_internals.py index 772fb36..d103a39 100644 --- a/tests/test_health_internals.py +++ b/tests/test_health_internals.py @@ -150,7 +150,7 @@ class TestEvaluateReadiness: async def test_unconfigured_dependencies_report_degraded(self) -> None: """Unconfigured DATABASE_URL/QDRANT_URL surface as degraded + down.""" - settings = Settings(database_url=None, qdrant_url=None, JWT_ISSUER="test-issuer") + settings = Settings(DATABASE_URL=None, QDRANT_URL=None, JWT_ISSUER="test-issuer") readiness = await health.evaluate_readiness(settings) assert readiness.status == "degraded" assert readiness.dependencies == {"postgres": "down", "qdrant": "down"}