Merge pull request #298 from techdeveloper-org/fix/code-review-round-9

fix(review): resolve regressions from #147-#240 batch, harden auth + prompt-injection guards

Author: piyushmakhija28

.github/workflows/ci.yml

@@ -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"

.secrets.baseline

@@ -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"),

backend/app/adapters/__init__.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
 
 

backend/app/adapters/generation.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,

backend/app/adapters/ingestor.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."

backend/app/api/answer.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)

backend/app/api/dependencies.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

backend/app/api/interfaces.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

backend/app/api/observability.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(