[NOTICKET] fix: review cheap batch — F-19 SSE order, F-22/F-24 analysis_id, F-26, F-16, F-4

Six low-risk review findings, batched. No behaviour anyone depends on is removed.

F-19 — SSE order/presence did not match the contract. `check` and router-`help`
emitted NO `sources` event at all, and `structured_flow` emitted `status` BEFORE
`sources`, inverting the documented order on exactly the turns that take longest. A
frontend that initializes per-turn state on `sources` (documented always-present)
therefore never initialized on check/help turns and initialized late on slow ones —
every one of those a plausible FE bug that would have been blamed on the frontend.
Note `stream_help` (the /tools/help endpoint) already emitted it, so the two help
paths disagreed with each other. Purely additive; `sources` stays `[]`.

F-22 — `analysis_id` now 422s unless it parses as a UUID, on BOTH live endpoints
(chat and help kept deliberately identical — two endpoints taking the same field
should not disagree on what is valid). It was never usable: `analyses.id` is
`uuid NOT NULL`, so a non-UUID silently matched nothing, `state_store.ensure` failed
its INSERT, the caller logged a warning and continued with `analysis_state = None`,
and help/readiness/report-write-back all quietly no-op'd while the user still saw a
normal-looking answer.

F-24 — traceability and chart writes are now SKIPPED, and logged, when `analysis_id`
is falsy. Go migration 0007 declares `analysis_id UUID NOT NULL` on both tables (plus
an FK to `analyses(id)` on `message_charts`), and Python passed `analysis_id or ""` —
an empty string cannot cast to uuid, the insert failed, and both writes are
never-throw, so the row vanished with no user-visible symptom and GET /charts then
answered `not_found` for a turn that really did produce a chart. Both live endpoints
require `analysis_id`, so this cannot fire in production; it is the honest fallback.

F-26 — `QueryResult.error` uses `str(e) or repr(e)`. A Fernet InvalidToken has an
empty `str()`, so it reached the assembler prompt, the traceability record and the
report caveats as an EMPTY string while the log (already repr) stayed diagnosable.
Falling back only when str() is empty means no existing error text changes.

F-16 — retrieval cache key gains `settings.redis_prefix`, matching every other cache
key in the service. Two environments sharing one Redis — which the single shared .env
makes plausible — were cross-serving retrieval results. Not cross-tenant (`user_id`
is in the key), but cross-environment. One-time cache-miss storm, bounded by the 1h
TTL.

F-4 — non-Postgres sources are now refused at the executor. ZERO blast radius today:
Go's `database_clients.Service.Create` gates on `isSupportedActive` and only
`postgres` is active, so no such source can be registered. This is a tripwire. The
legacy branch it replaces had no read-only session and no statement_timeout, i.e.
only four of the five defense layers CLAUDE.md §2.5 states unconditionally — and
since the compiler is built with dialect="postgres" regardless of db_type, those
queries would fail on a parse error first, which the never-throw path degrades into
"data not available". The danger was someone fixing THAT without noticing the pooling
branch. Branch commented out per the house convention, with the orphaned import
commented alongside it.

Tests: two event-sequence assertions updated for F-19 (they pinned the missing
`sources`), and six traceability tests now pass an `analysis_id` — they called
`handle()` without one, a shape neither live endpoint can produce.

Verification: suite 434 passed / 0 failed / 7 skipped (was 424; +10 from F-8).
Ruff on touched paths identical to HEAD baseline (S324/I001/E501, all pre-existing).
`import main` OK. Readiness eval 17/17.

Docs: contract gains the corrected SSE guarantee + the 422 rule; DEV_PLAN rows 46-48
added (48 = the floor_08 decision, open for Rifqi).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

API_CONTRACT_BE_PYTHON.md

@@ -147,6 +147,8 @@ Behavior notes:
 - The router may classify messages into intents such as `chat`, `help`, `check`, `unstructured_flow`, or `structured_flow`.
 - `sources` in the stream is **always `[]`** (KM-691) — read the real `sources[]` from `GET /api/v1/traceability` after `done`.
 - `status` events are optional and should be safe for the frontend to ignore.
+- **Fixed 2026-07-23 — `sources` is now genuinely always first, on every path.** It previously did not match this document: the `check` and router-`help` turns emitted **no `sources` event at all**, and the `structured_flow` (slow) path emitted `status` *before* `sources`, inverting the documented order on exactly the turns that take longest. A frontend that initializes per-turn state on `sources` therefore never initialized on check/help turns and initialized late on slow turns. The change is **purely additive** — no event was removed or renamed, and `sources` is still `[]` — so no frontend change is required; behaviour simply now matches what this contract always said.
+- **`analysis_id` must be a UUID (2026-07-23).** `POST /api/v2/chat/stream` and `POST /api/v1/tools/help` now return **`422`** for a non-UUID `analysis_id` instead of accepting it. It was never actually usable: `analyses.id` is `uuid NOT NULL`, so a non-UUID id silently matched no analysis and the turn ran with no state — help, readiness, the report write-back, and the traceability/chart rows all quietly no-op'd while the user still saw a normal answer. The frontend passes Go-issued analysis ids, which are UUIDs, so this should never fire in practice.
 
 ## Tools
 

DEV_PLAN.md

@@ -247,6 +247,9 @@ plus the live report bug on analysis `966224d4…`. Same status legend as §0.
 | 43 | **Go identity contract** — what does Go forward, and when? | Rifqi ↔ Harry | ⬜ new | Needed to replace #37's shared secret with real per-user authorization. Until then the store predicates in #38 are defensive only |
 | 44 | **F-17 / F-18 / F-25 compiler-parity batch** | Rifqi | ✅ new | Shipped 2026-07-23. Three execution-verified review findings that had **no task row** — the tracker jumped from #39 to #40 and lost them. **F-17 (High):** the bare-select check was gated on `if ir.group_by`, so a mixed select with `group_by=[]` passed; Postgres then failed loudly but the pandas path silently DROPPED the column while `output_columns` still advertised it → a real-looking table with a fabricated all-null column. Fixed in `validator.py` (fires whenever any agg is present — no false positives possible) + a presence backstop in `TabularExecutor`. **F-18:** `astype(str)` ran before `na=False`, so `NULL LIKE '%an%'` matched the literal `"nan"`/`"None"`. **F-25:** `SqlCompiler` raised on an empty `in`/`not_in` where pandas and `_column_values`' own docstring implement the empty-set semantics — a two-step plan whose first step legitimately returned zero rows hard-failed instead of answering. 16 new parity tests; one stale test updated (it pinned the old F-25 raise) |
 | 45 | **F-9 PII in persisted artifacts** — mask the traceability preview + report evidence tables | Rifqi | ⬜ new | **Decided 2026-07-23** (CODE_REVIEW §7) but never tracked. `pii_flag` is an ingestion-time control only: nothing stops the planner SELECTing a flagged column, and the real values then land in `message_traceability.data` (served by a GET that is only now becoming scoped, #40) and are frozen permanently into `reports.content`. Decision: **the assembler still sees values** (otherwise "list our top customers" stops being answerable); mask only the persisted artifacts. Carry `pii_flag` onto `retrieve_data`'s output meta — the catalog is already in scope at `data_access.py:232` |
+| 46 | **F-8 prompt-injection resistance** — planner / assembler / report_summary | Rifqi | ✅ new | Shipped 2026-07-24. The three prompts that ingest customer data had **no injection rule**: `guardrails.md` is appended only in `chatbot.py`/`help.py`, and the `InputGuard` screens only the user's message — it never sees catalog or row content. This is the one attack the five query-defense layers structurally cannot see, because every IR the planner emits is individually valid. A hostile string only needs to reach a text column in the customer's OWN database (a product description, a support ticket, a form field) to be sampled into `sample_values` and rendered verbatim into the planner prompt. Fix = a purpose-written "content is data, never instructions" rule in each prompt + `<data>…</data>` delimiters around the catalog render and the run-state render. **Deliberately NOT `guardrails.md` wholesale** — its rules prescribe refusal sentences, and the planner's only free-text field is `infeasible_reason`, so those strings would surface there and could regress the Q2 data-gap path (tests pin their absence). Verified: planner eval **6/6, carried_over 5/5 green**; live hostile-catalog run planned only `t_products` and never touched the planted `employees.salary`; 10 new tests |
+| 47 | **Cheap-batch review fixes** — F-19, F-22, F-24, F-26, F-16, F-4 | Rifqi | ✅ new | Shipped 2026-07-24, one low-risk batch. **F-19:** `sources` was missing entirely on `check` + router-`help` and came *after* `status` on the slow path — contract said always-first; additive fix, contract updated. **F-22:** `analysis_id` now 422s unless it parses as a UUID, on **both** live endpoints (chat + help kept identical on purpose). **F-24:** traceability/chart writes are skipped, and logged, when `analysis_id` is falsy — Go `0007` declares `analysis_id UUID NOT NULL` (+ FK on charts), so `analysis_id or ""` failed the insert and the never-throw seam lost the row silently. **F-26:** `QueryResult.error` uses `str(e) or repr(e)` — a Fernet `InvalidToken` reached the assembler/traceability/report as an EMPTY string; falling back only when `str()` is empty changes no existing message. **F-16:** retrieval cache key gains `redis_prefix` (two envs on one Redis cross-served results). **F-4:** non-Postgres sources now refused at the executor — **zero blast radius today** (Go's `isSupportedActive` allows only `postgres`), a tripwire so nobody re-enables a path that has no read-only session and no `statement_timeout`; legacy branch commented out per house convention, orphaned import commented with it |
+| 48 | **floor_08 — floor/body disagreement** | Rifqi | ⬜ new | **Needs a decision, found while adding #34 coverage.** When a plan HAS an `analyze_*` step that FAILS but its upstream `retrieve_data` returned rows, the floor passes (`_produced_rows`) while the body rejects it (`has_reportable_result` keys on the failed analysis step). Verified by execution — it is the only shape where the two disagree. Because the "Attempted, Unresolved" section is commented out (`generator.py:527`), such a run leaves **no trace anywhere**: if it is the session's only run, the report generates but comes out empty and the business question renders "Unanswered" — exactly the #33 bug through a different door. Suggested fix: apply the floor's second arm only when the plan has NO analysis step, mirroring the body predicate; `floor_08` then flips to `expected_ready: false`. **Not changed — Rifqi owns the floor** |
 
 **Reading `eval/readiness/results/` (note for future sessions).** Four files are dated
 2026-07-23. `…_150632.json` scores **4/15 (26.7%)** — that is **not** a product

eval/readiness/results/readiness_result_2026-07-24_084154.json

@@ -0,0 +1,274 @@
+{
+  "run": {
+    "timestamp": "2026-07-24T08:41:54",
+    "dataset": "readiness_dataset.json",
+    "target": "src/agents/report/readiness.is_report_ready",
+    "total": 17,
+    "passed": 17,
+    "accuracy": 1.0,
+    "runtime_avg_ms": 0.0
+  },
+  "alignment_gap": {
+    "count": 2,
+    "ids": [
+      "align_01",
+      "align_02"
+    ]
+  },
+  "by_group": {
+    "floor": {
+      "n": 8,
+      "passed": 8,
+      "accuracy": 1.0
+    },
+    "delta": {
+      "n": 5,
+      "passed": 5,
+      "accuracy": 1.0
+    },
+    "edge": {
+      "n": 1,
+      "passed": 1,
+      "accuracy": 1.0
+    },
+    "alignment": {
+      "n": 3,
+      "passed": 3,
+      "accuracy": 1.0
+    }
+  },
+  "cases": [
+    {
+      "id": "floor_01",
+      "group": "floor",
+      "expected_ready": false,
+      "got_ready": false,
+      "expected_missing": [
+        "at least one completed analysis"
+      ],
+      "got_missing": [
+        "at least one completed analysis"
+      ],
+      "correct": true,
+      "aligned": false,
+      "gap": false,
+      "latency_ms": 0.0
+    },
+    {
+      "id": "floor_02",
+      "group": "floor",
+      "expected_ready": false,
+      "got_ready": false,
+      "expected_missing": [
+        "at least one completed analysis"
+      ],
+      "got_missing": [
+        "at least one completed analysis"
+      ],
+      "correct": true,
+      "aligned": false,
+      "gap": false,
+      "latency_ms": 0.0
+    },
+    {
+      "id": "floor_03",
+      "group": "floor",
+      "expected_ready": false,
+      "got_ready": false,
+      "expected_missing": [
+        "at least one completed analysis"
+      ],
+      "got_missing": [
+        "at least one completed analysis"
+      ],
+      "correct": true,
+      "aligned": false,
+      "gap": false,
+      "latency_ms": 0.0
+    },
+    {
+      "id": "floor_04",
+      "group": "floor",
+      "expected_ready": true,
+      "got_ready": true,
+      "expected_missing": [],
+      "got_missing": [],
+      "correct": true,
+      "aligned": true,
+      "gap": false,
+      "latency_ms": 0.0
+    },
+    {
+      "id": "floor_05",
+      "group": "floor",
+      "expected_ready": true,
+      "got_ready": true,
+      "expected_missing": [],
+      "got_missing": [],
+      "correct": true,
+      "aligned": true,
+      "gap": false,
+      "latency_ms": 0.0
+    },
+    {
+      "id": "floor_06",
+      "group": "floor",
+      "expected_ready": true,
+      "got_ready": true,
+      "expected_missing": [],
+      "got_missing": [],
+      "correct": true,
+      "aligned": true,
+      "gap": false,
+      "latency_ms": 0.0
+    },
+    {
+      "id": "floor_07",
+      "group": "floor",
+      "expected_ready": true,
+      "got_ready": true,
+      "expected_missing": [],
+      "got_missing": [],
+      "correct": true,
+      "aligned": true,
+      "gap": false,
+      "latency_ms": 0.0
+    },
+    {
+      "id": "floor_08",
+      "group": "floor",
+      "expected_ready": true,
+      "got_ready": true,
+      "expected_missing": [],
+      "got_missing": [],
+      "correct": true,
+      "aligned": true,
+      "gap": false,
+      "latency_ms": 0.0
+    },
+    {
+      "id": "delta_01",
+      "group": "delta",
+      "expected_ready": false,
+      "got_ready": false,
+      "expected_missing": [
+        "a new analysis since the last report"
+      ],
+      "got_missing": [
+        "a new analysis since the last report"
+      ],
+      "correct": true,
+      "aligned": true,
+      "gap": false,
+      "latency_ms": 0.0
+    },
+    {
+      "id": "delta_02",
+      "group": "delta",
+      "expected_ready": true,
+      "got_ready": true,
+      "expected_missing": [],
+      "got_missing": [],
+      "correct": true,
+      "aligned": true,
+      "gap": false,
+      "latency_ms": 0.0
+    },
+    {
+      "id": "delta_03",
+      "group": "delta",
+      "expected_ready": true,
+      "got_ready": true,
+      "expected_missing": [],
+      "got_missing": [],
+      "correct": true,
+      "aligned": true,
+      "gap": false,
+      "latency_ms": 0.0
+    },
+    {
+      "id": "delta_04",
+      "group": "delta",
+      "expected_ready": false,
+      "got_ready": false,
+      "expected_missing": [
+        "a new analysis since the last report"
+      ],
+      "got_missing": [
+        "a new analysis since the last report"
+      ],
+      "correct": true,
+      "aligned": true,
+      "gap": false,
+      "latency_ms": 0.0
+    },
+    {
+      "id": "delta_05",
+      "group": "delta",
+      "expected_ready": false,
+      "got_ready": false,
+      "expected_missing": [
+        "a new analysis since the last report"
+      ],
+      "got_missing": [
+        "a new analysis since the last report"
+      ],
+      "correct": true,
+      "aligned": true,
+      "gap": false,
+      "latency_ms": 0.0
+    },
+    {
+      "id": "edge_01",
+      "group": "edge",
+      "expected_ready": false,
+      "got_ready": false,
+      "expected_missing": [
+        "at least one completed analysis"
+      ],
+      "got_missing": [
+        "at least one completed analysis"
+      ],
+      "correct": true,
+      "aligned": false,
+      "gap": false,
+      "latency_ms": 0.0
+    },
+    {
+      "id": "align_01",
+      "group": "alignment",
+      "expected_ready": true,
+      "got_ready": true,
+      "expected_missing": [],
+      "got_missing": [],
+      "correct": true,
+      "aligned": false,
+      "gap": true,
+      "latency_ms": 0.0
+    },
+    {
+      "id": "align_02",
+      "group": "alignment",
+      "expected_ready": true,
+      "got_ready": true,
+      "expected_missing": [],
+      "got_missing": [],
+      "correct": true,
+      "aligned": false,
+      "gap": true,
+      "latency_ms": 0.0
+    },
+    {
+      "id": "align_03",
+      "group": "alignment",
+      "expected_ready": true,
+      "got_ready": true,
+      "expected_missing": [],
+      "got_missing": [],
+      "correct": true,
+      "aligned": true,
+      "gap": false,
+      "latency_ms": 0.0
+    }
+  ]
+}

src/agents/chat_handler.py

@@ -470,6 +470,11 @@ class ChatHandler:
                 yield {"event": "error", "data": f"Document retrieval failed: {e}"}
                 return
         elif intent == "check":
+            # The contract documents `sources` as always present and always first
+            # (it stays `[]` — real sources live in GET /traceability). This branch
+            # used to skip it, so an FE that initializes per-turn state on `sources`
+            # silently never initialized on a check turn. Purely additive. (F-19)
+            yield {"event": "sources", "data": json.dumps([])}
             try:
                 # Scope check to the analysis catalog: it holds only this room's
                 # bound sources and their real names (a DB shows as "xl test", not
@@ -516,6 +521,10 @@ class ChatHandler:
         #     yield {"event": "done", "data": ""}
         #     return
         elif intent == "help":
+            # Same as the check branch: `sources` is contractually always-first.
+            # Note `stream_help` (the dedicated /tools/help endpoint) already emits
+            # it, so the two help paths disagreed with each other until now. (F-19)
+            yield {"event": "sources", "data": json.dumps([])}
             try:
                 state = analysis_state or await self._load_analysis_state(analysis_id)
             except Exception as e:
@@ -670,8 +679,21 @@ class ChatHandler:
         """
         if pad.message_id is None:
             return
+        if not analysis_id:
+            # `message_traceability.analysis_id` is `UUID NOT NULL` (Go migration
+            # 0007). Passing `analysis_id or ""` sent an empty string, which cannot
+            # cast to uuid — the INSERT failed, the never-throw seam swallowed it, and
+            # the row vanished with no user-visible symptom. Skipping the write for a
+            # turn that genuinely has no analysis is the honest outcome, and it says
+            # so in the log instead of failing invisibly. (F-24)
+            logger.info(
+                "traceability flush skipped — no analysis_id",
+                degraded_seam="traceability_flush_no_analysis",
+                message_id=pad.message_id,
+            )
+            return
         try:
-            payload = pad.build(analysis_id or "", user_id, pad.message_id)
+            payload = pad.build(analysis_id, user_id, pad.message_id)
             await self._get_traceability_store().save(payload)
         except Exception as e:  # noqa: BLE001 — never break the answer on a trace slip
             logger.warning(
@@ -726,6 +748,13 @@ class ChatHandler:
         if ac:
             run_kw["assembler_callbacks"] = ac
 
+        # Contract order is `sources` -> `status`* -> `chunk`* -> `done`. This used to
+        # be emitted after the status loop, so the slow path (the ONLY path that emits
+        # `status`) inverted the documented order and the FE saw `status` first on
+        # exactly the turns that take longest. Sources live in traceability now
+        # (KM-691); the event stays `[]` and the stream stays text-only. (F-19)
+        yield {"event": "sources", "data": json.dumps([])}
+
         # R4: bridge the coordinator's per-stage progress callback to SSE `status`
         # events so the stream isn't silent for ~12s (and proxies don't drop the
         # idle connection). Status events only appear if the coordinator calls back.
@@ -763,9 +792,6 @@ class ChatHandler:
             yield {"event": "error", "data": f"Analysis failed: {e}"}
             return
 
-        # Sources live in traceability now (KM-691), derived from the run's
-        # retrieve_data calls; the stream stays text-only.
-        yield {"event": "sources", "data": json.dumps([])}
         yield {"event": "chunk", "data": result.chat_answer}
         try:
             # Stamp identity from the request scope: owner + the shared session id
@@ -790,13 +816,18 @@ class ChatHandler:
         # SPINE_V2_PLAN §4.4: chart rows are written before `done`; the FE fetches
         # GET /api/v1/charts unconditionally on every `done` (no polling race).
         try:
-            if pad is not None and pad.message_id:
+            # `message_charts.analysis_id` is `uuid NOT NULL REFERENCES analyses(id)`
+            # (Go migration 0007), so an empty string both fails the cast and violates
+            # the FK. The write is never-throw, so the chart row disappeared silently
+            # and GET /api/v1/charts then answered `not_found` for a turn that really
+            # did produce a chart. Skip rather than lose it invisibly. (F-24)
+            if pad is not None and pad.message_id and analysis_id:
                 for task_result in result.analysis_record.results_snapshot.values():
                     for output in task_result.outputs:
                         if output.kind == "chart" and isinstance(output.value, dict):
                             await self._get_chart_store().save(
                                 message_id=pad.message_id,
-                                analysis_id=analysis_id or "",
+                                analysis_id=analysis_id,
                                 user_id=user_id,
                                 record_id=result.analysis_record.record_id,
                                 envelope=output.value,

src/api/v1/help.py

@@ -20,7 +20,7 @@ import json
 import uuid
 
 from fastapi import APIRouter, Depends, HTTPException
-from pydantic import BaseModel
+from pydantic import BaseModel, field_validator
 from sqlalchemy.ext.asyncio import AsyncSession
 from sse_starlette.sse import EventSourceResponse
 
@@ -40,6 +40,24 @@ class HelpRequest(BaseModel):
     user_id: str
     analysis_id: str
 
+    @field_validator("analysis_id")
+    @classmethod
+    def _analysis_id_is_uuid(cls, v: str) -> str:
+        """Same boundary rule as `POST /api/v2/chat/stream`. (F-22)
+
+        Kept identical on purpose: a non-UUID id can never match a row in `analyses`
+        (`id uuid NOT NULL`), so the state read silently returns nothing and Help
+        answers from an empty state instead of saying the id was wrong. Two live
+        endpoints taking the same field should not disagree on what is valid.
+        """
+        try:
+            uuid.UUID(v)
+        except (ValueError, AttributeError, TypeError):
+            raise ValueError(
+                "analysis_id must be a UUID (the id of an existing analysis)"
+            ) from None
+        return v
+
 
 @router.post("/help")
 @log_execution(logger)

src/api/v2/chat.py

@@ -24,7 +24,7 @@ import uuid
 from typing import Any
 
 from fastapi import APIRouter, Depends, HTTPException, Request
-from pydantic import BaseModel
+from pydantic import BaseModel, field_validator
 from sqlalchemy.ext.asyncio import AsyncSession
 from sse_starlette.sse import EventSourceResponse
 
@@ -82,6 +82,31 @@ class ChatRequest(BaseModel):
     analysis_id: str
     message: str
 
+    @field_validator("analysis_id")
+    @classmethod
+    def _analysis_id_is_uuid(cls, v: str) -> str:
+        """422 on a non-UUID `analysis_id` instead of degrading silently. (F-22)
+
+        `analyses.id` is `uuid NOT NULL`, so a non-UUID value can never match a real
+        analysis. Without this the turn still ran: `state_store.ensure` failed its
+        INSERT, the caller logged a warning and continued with `analysis_state = None`,
+        and help, readiness and the report write-back all silently no-op'd for that
+        turn. The user saw a normal-looking answer whose analysis state was never
+        persisted. A clear client error is the honest outcome.
+
+        Also closes the F-24 half of the same problem: `message_traceability` and
+        `message_charts` declare `analysis_id UUID NOT NULL` (Go migration 0007, with
+        an FK to `analyses(id)` on the charts table), and both writes are never-throw
+        — so a non-UUID id silently lost the provenance and chart rows too.
+        """
+        try:
+            uuid.UUID(v)
+        except (ValueError, AttributeError, TypeError):
+            raise ValueError(
+                "analysis_id must be a UUID (the id of an existing analysis)"
+            ) from None
+        return v
+
 
 @router.post("/chat/stream")
 # Rate limit per client IP. `slowapi` needs a Starlette `Request` param named

src/query/executor/db.py

@@ -32,12 +32,16 @@ from ...database_client.database_client_service import database_client_service
 from ...database_client.engine import user_engine_cache
 from ...db.postgres.connection import AsyncSessionLocal
 from ...middlewares.logging import get_logger
-from ...pipeline.db_pipeline import db_pipeline_service
 from ...utils.db_credential_encryption import decrypt_credentials_dict
 from ..compiler.sql import CompiledSql, SqlCompiler
 from ..ir.models import QueryIR
 from .base import BaseExecutor, QueryResult
 
+# Orphaned by the F-4 tripwire (2026-07-23): the only consumer was the legacy
+# non-postgres branch in `_run_sync`, now commented out there. Restore this import
+# alongside that branch (kept out of the block above so import sorting stays clean).
+# from ...pipeline.db_pipeline import db_pipeline_service
+
 logger = get_logger("db_executor")
 
 _QUERY_TIMEOUT_SECONDS = 30
@@ -131,7 +135,13 @@ class DbExecutor(BaseExecutor):
                 source_id=ir.source_id,
                 backend="sql",
                 elapsed_ms=elapsed_ms,
-                error=str(e),
+                # `str(e) or repr(e)`, not bare repr: the log above already uses repr,
+                # but this payload reaches the assembler prompt, the traceability
+                # record and the report caveats — where a Fernet InvalidToken arrived
+                # as an EMPTY string, so the user-facing artifact said nothing while
+                # the log was diagnosable. Falling back only when str() is empty means
+                # no existing error text changes. (F-26)
+                error=str(e) or repr(e),
                 table_id=ir.table_id,
                 table_name=table_name,
                 source_name=source_name,
@@ -207,12 +217,36 @@ class DbExecutor(BaseExecutor):
                 result = conn.execute(text(compiled.sql), compiled.params)
                 return list(result.keys()), [dict(row) for row in result.mappings()]
 
-        # Legacy per-call path for non-postgres db_types (connect once, dispose).
-        # These never set read-only/timeout before, so behavior is unchanged.
-        with db_pipeline_service.engine_scope(db_type, creds) as eng:
-            with eng.connect() as conn:
-                result = conn.execute(text(compiled.sql), compiled.params)
-                return list(result.keys()), [dict(row) for row in result.mappings()]
+        # TRIPWIRE (F-4, 2026-07-23). Below this line was the legacy per-call path for
+        # non-postgres db_types, whose own comment conceded "these never set
+        # read-only/timeout before, so behavior is unchanged". That means such a source
+        # would get only four of the five documented defense layers: IR validation, the
+        # compiler whitelist, the sqlglot guard and LIMIT — but NO read-only session and
+        # NO statement_timeout. `CLAUDE.md` §2.5 states all five unconditionally; in
+        # truth they were conditional on db_type, and nothing said so where a reader
+        # would look.
+        #
+        # Zero blast radius today: Go's `database_clients.Service.Create` gates on
+        # `isSupportedActive`, and only `postgres` is `active` — mysql/sqlserver/
+        # bigquery/snowflake are all "Coming soon", so no such source can be registered.
+        # This refuses loudly the day someone flips that flag, instead of silently
+        # executing against a customer's database with two guardrails missing.
+        #
+        # Note the compiler is built with dialect="postgres" regardless of db_type and
+        # the sqlglot guard parses with read="postgres", so these queries would fail on
+        # a parse error anyway — which the never-throw path would degrade into "data not
+        # available", masquerading as a data problem. The danger is someone fixing THAT
+        # without noticing the pooling branch. Re-enabling this path requires
+        # dialect-correct compilation AND session hardening, not just a dialect string.
+        raise ValueError(
+            f"source type {db_type!r} is not supported for analysis yet — only "
+            "PostgreSQL sources can be queried safely (read-only session and query "
+            "timeout are not yet implemented for other database types)"
+        )
+        # with db_pipeline_service.engine_scope(db_type, creds) as eng:
+        #     with eng.connect() as conn:
+        #         result = conn.execute(text(compiled.sql), compiled.params)
+        #         return list(result.keys()), [dict(row) for row in result.mappings()]
 
     # ------------------------------------------------------------------
     # Speculative pre-connect (DB3)

src/query/executor/tabular.py

@@ -204,15 +204,18 @@ class TabularExecutor(BaseExecutor):
             elapsed_ms = int((time.perf_counter() - started) * 1000)
             logger.error(
                 "tabular executor failed",
+                degraded_seam="tabular_execute",
                 source_id=ir.source_id,
-                error=str(e),
+                error=repr(e),
                 elapsed_ms=elapsed_ms,
             )
             return QueryResult(
                 source_id=ir.source_id,
                 backend="tabular",
                 elapsed_ms=elapsed_ms,
-                error=str(e),
+                # See db.py: fall back to repr only when str() is empty, so no
+                # existing user-facing error text changes. (F-26)
+                error=str(e) or repr(e),
                 table_id=ir.table_id,
                 table_name=table_name,
                 source_name=source_name,

src/retrieval/router.py

@@ -13,6 +13,7 @@ import hashlib
 import json
 from dataclasses import asdict
 
+from src.config.settings import settings
 from src.db.redis.connection import get_redis
 from src.middlewares.logging import get_logger
 from src.retrieval.base import RetrievalResult
@@ -21,7 +22,13 @@ from src.retrieval.document import DocumentRetriever
 logger = get_logger("retrieval_router")
 
 _CACHE_TTL = 3600
-_CACHE_KEY_PREFIX = "retrieval"
+# Namespaced with `settings.redis_prefix`, like every other cache key in the service
+# (cf. `api/v1/chat.py::build_cache_key`). Without it, two environments pointed at the
+# same Redis — which the single shared `.env` makes entirely plausible — cross-serve
+# each other's retrieval results. Not cross-TENANT (`user_id` is in the key), but
+# cross-environment, which is confusing in exactly the way stale-cache bugs are. The
+# one-time cost is a cache-miss storm bounded by the 1h TTL. (F-16)
+_CACHE_KEY_PREFIX = f"{settings.redis_prefix}retrieval"
 
 
 class RetrievalRouter: