From f48c6bc67601dba81152861aab8356c3e586c949 Mon Sep 17 00:00:00 2001 From: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> Date: Fri, 23 Oct 2026 08:28:46 -0700 Subject: [PATCH 1/4] fix(recall-report): return an honest empty report instead of demo scores A tenant with no traceability data used to get hardcoded demo dimensions, a canned action plan and a fake 4.2-hour response estimate. It now gets no dimensions, no action items, score 0 with grade N/A and status no_data, and a 'No data yet' summary. demo_mode/demo_disclaimer stay in the model for API compatibility but are always false/null. Tests are data-free. Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> --- services/ingestion/app/recall_report.py | 222 ++++-------------- .../tests/test_recall_report_1342.py | 69 +++--- 2 files changed, 84 insertions(+), 207 deletions(-) diff --git a/services/ingestion/app/recall_report.py b/services/ingestion/app/recall_report.py index 368c6ed3d..e33f534e4 100644 --- a/services/ingestion/app/recall_report.py +++ b/services/ingestion/app/recall_report.py @@ -16,7 +16,6 @@ from pydantic import BaseModel, Field from shared.tenant_context import TenantContext -from .disclaimers import DEMO_DATA_DISCLAIMER from .webhook_compat import require_tenant_context logger = logging.getLogger("recall-report") @@ -224,9 +223,9 @@ async def generate_report( # Try to query real data scoring_data = _query_scoring_data(tenant_id) - use_demo_mode = scoring_data is None or scoring_data.get("cte_count", 0) == 0 + has_data = scoring_data is not None and scoring_data.get("cte_count", 0) > 0 - if not use_demo_mode: + if has_data: # Compute real scores based on actual data cte_count = scoring_data.get("cte_count", 0) cte_types = scoring_data.get("cte_types", 0) @@ -380,185 +379,53 @@ async def generate_report( ), ] else: - # Use hardcoded demo scenarios - dimensions = [ - RecallDimension( - id="trace_speed", - name="Trace Speed", - score=78, - grade="C", - status="needs_improvement", - findings=[ - "Average trace-back time: 4.2 hours", - "3 lots required manual lookups (non-electronic records)", - "Receiving CTEs averaged 3.1 hour entry delay", - ], - recommendations=[ - "Reduce CTE entry delay to under 2 hours", - "Digitize remaining manual records via CSV upload or API", - "Run monthly mock audit drills to improve response time", - ], - ), - RecallDimension( - id="data_completeness", - name="Data Completeness", - score=85, - grade="B", - status="good", - findings=[ - "92% of CTEs have all required KDEs", - "GLN coverage: 78% (missing for 3 supplier locations)", - "TLC format consistency: 95%", - ], - recommendations=[ - "Collect missing GLNs from 3 supplier locations", - "Standardize TLC format across all facilities", - "Enable automated KDE validation rules", - ], - ), - RecallDimension( - id="chain_integrity", - name="Chain Integrity", - score=95, - grade="A", - status="excellent", - findings=[ - "SHA-256 hash chain: 100% verified", - "No gaps in event sequence", - "All immutability triggers active", - ], - recommendations=[ - "Maintain current integrity practices", - "Consider adding independent third-party verification", - ], - ), - RecallDimension( - id="supplier_coverage", - name="Supplier Coverage", - score=65, - grade="D", - status="critical", - findings=[ - "Only 60% of suppliers actively using portal", - "2 suppliers have not submitted in 30+ days", - "1 supplier still using paper-based records", - ], - recommendations=[ - "Send portal link reminders to inactive suppliers", - "Offer supplier training sessions", - "Consider switching non-compliant suppliers", - ], - ), - RecallDimension( - id="export_readiness", - name="Export Readiness", - score=88, - grade="B", - status="good", - findings=[ - "FDA sortable spreadsheet export: functional", - "EPCIS 2.0 JSON-LD export: functional", - "Export includes SHA-256 verification hashes", - ], - recommendations=[ - "Test export with target retailer portals (Walmart, Kroger)", - "Validate EPCIS schema compliance with GS1 validator", - ], - ), - RecallDimension( - id="team_readiness", - name="Team Readiness", - score=72, - grade="C", - status="needs_improvement", - findings=[ - "Last mock drill: 2 weeks ago (score: C)", - "2 of 5 team members completed FSMA training", - "No documented recall SOP on file", - ], - recommendations=[ - "Generate SOP via /api/v1/sop/generate", - "Schedule monthly mock audit drills", - "Complete FSMA 204 training for remaining 3 team members", - ], - ), - ] - - overall = int(sum(d.score for d in dimensions) / len(dimensions)) - - # Build executive summary from computed dimension scores - _dim_scores = {d.id: d.score for d in dimensions} - _chain_score = _dim_scores.get("chain_integrity", overall) - _supplier_score = _dim_scores.get("supplier_coverage", overall) - _team_score = _dim_scores.get("team_readiness", overall) - executive_summary = ( - f"This recall readiness assessment evaluates {tenant_id}'s preparedness " - f"to respond to an FDA traceability records request under 21 CFR 1.1455. " - f"The overall readiness score is {overall}/100 (Grade {_grade(overall)}). " - f"Chain integrity score: {_chain_score}/100. " - f"Supplier coverage score: {_supplier_score}/100. " - f"Team readiness score: {_team_score}/100. " - f"Addressing any gaps below 80 would raise the overall grade to B or above." - ) - - # #3042: the improvement plan must come from the tenant's actual - # computed gaps. Only the demo scenario ships the canned narrative - # (its dimension findings are equally illustrative). - if use_demo_mode: - action_items = [ - { - "priority": "HIGH", - "action": "Collect missing GLNs from 3 supplier locations", - "impact": "+5 to Data Completeness", - "effort": "Low", - }, - { - "priority": "HIGH", - "action": "Re-engage 2 inactive suppliers via portal reminders", - "impact": "+10 to Supplier Coverage", - "effort": "Low", - }, - { - "priority": "MEDIUM", - "action": "Generate and distribute FSMA 204 SOP", - "impact": "+8 to Team Readiness", - "effort": "Low", - }, - { - "priority": "MEDIUM", - "action": "Complete FSMA training for 3 remaining team members", - "impact": "+10 to Team Readiness", - "effort": "Medium", - }, - { - "priority": "MEDIUM", - "action": "Reduce CTE entry delay to under 2 hours", - "impact": "+7 to Trace Speed", - "effort": "Medium", - }, - { - "priority": "LOW", - "action": "Test EPCIS exports with retailer portals", - "impact": "+5 to Export Readiness", - "effort": "Low", - }, - ] + # No tenant data: no dimensions to score (no hardcoded demo scenarios). + dimensions: list[RecallDimension] = [] + + if dimensions: + overall = int(sum(d.score for d in dimensions) / len(dimensions)) + overall_grade = _grade(overall) + overall_status = _status(overall) + + # Build executive summary from computed dimension scores + _dim_scores = {d.id: d.score for d in dimensions} + _chain_score = _dim_scores.get("chain_integrity", overall) + _supplier_score = _dim_scores.get("supplier_coverage", overall) + _team_score = _dim_scores.get("team_readiness", overall) + executive_summary = ( + f"This recall readiness assessment evaluates {tenant_id}'s preparedness " + f"to respond to an FDA traceability records request under 21 CFR 1.1455. " + f"The overall readiness score is {overall}/100 (Grade {overall_grade}). " + f"Chain integrity score: {_chain_score}/100. " + f"Supplier coverage score: {_supplier_score}/100. " + f"Team readiness score: {_team_score}/100. " + f"Addressing any gaps below 80 would raise the overall grade to B or above." + ) else: - action_items = _derive_action_items(dimensions) + # No tenant traceability data yet: say so instead of inventing scores. + overall = 0 + overall_grade = "N/A" + overall_status = "no_data" + executive_summary = ( + "No data yet. Recall readiness is scored from this tenant's own " + "traceability records; ingest CTE events to generate a report." + ) + + # #3042: the improvement plan comes only from the tenant's actual + # computed gaps. A tenant with no data gets an empty plan. + action_items = _derive_action_items(dimensions) return RecallReport( tenant_id=tenant_id, generated_at=now.isoformat(), report_title=f"Recall Readiness Report — {tenant_id}", overall_score=overall, - overall_grade=_grade(overall), - overall_status=_status(overall), - # #3042: the previous real-mode value (overall/100*24 hours) was a - # score formula formatted as a latency measurement. Response time - # is not measured anywhere in this service, so say so. - time_to_respond_estimate="4.2 hours (target: < 24 hours)" - if use_demo_mode - else "Not yet measured — run a recall drill to establish a baseline (target: < 24 hours per 21 CFR 1.1455(c))", + overall_grade=overall_grade, + overall_status=overall_status, + # #3042: response time is not measured anywhere in this service. + time_to_respond_estimate=( + "Not yet measured — run a recall drill to establish a baseline (target: < 24 hours per 21 CFR 1.1455(c))" + ), dimensions=dimensions, executive_summary=executive_summary, action_items=action_items, @@ -568,6 +435,7 @@ async def generate_report( "21 CFR 1.1325-1.1350 — CTE/KDE Requirements", "FSMA Section 204 — Food Traceability Rule", ], - demo_mode=use_demo_mode, - demo_disclaimer=DEMO_DATA_DISCLAIMER if use_demo_mode else None, + # Kept for API compatibility; reports are always built from tenant data. + demo_mode=False, + demo_disclaimer=None, ) diff --git a/services/ingestion/tests/test_recall_report_1342.py b/services/ingestion/tests/test_recall_report_1342.py index e8fb00cbc..a1a0554af 100644 --- a/services/ingestion/tests/test_recall_report_1342.py +++ b/services/ingestion/tests/test_recall_report_1342.py @@ -5,8 +5,8 @@ - _query_scoring_data: happy path with gaps, chain_length=0 path (no max_seq query), SessionLocal raise → None, execute raise → None - Pydantic models — RecallDimension, RecallReport -- GET /{tenant_id}/report: demo mode (None scoring), demo mode - (cte_count=0), real mode (low suppliers + no chain + no export), +- GET /{tenant_id}/report: no data (None scoring or cte_count=0) → empty report, + real mode (low suppliers + no chain + no export), real mode (medium suppliers + chain gaps + has export), real mode (high suppliers + clean chain + has export) @@ -16,6 +16,7 @@ from __future__ import annotations from types import SimpleNamespace +from uuid import uuid4 import pytest from app import recall_report as rr @@ -292,35 +293,29 @@ def test_execute_raises_returns_none(self, monkeypatch): # --------------------------------------------------------------------------- -# GET /{tenant_id}/report — demo mode +# GET /{tenant_id}/report — tenant with no data gets an honest empty report # --------------------------------------------------------------------------- -class TestGenerateReportDemoMode: - def test_demo_mode_when_scoring_returns_none(self, client, monkeypatch): - monkeypatch.setattr(rr, "_query_scoring_data", lambda tid: None) +class TestGenerateReportNoData: + @pytest.mark.parametrize("scoring", [None, {"cte_count": 0}]) + def test_no_data_returns_empty_report(self, client, monkeypatch, scoring): + monkeypatch.setattr(rr, "_query_scoring_data", lambda tid: scoring) + tenant_id = str(uuid4()) - r = client.get("/api/v1/recall/acme/report") + r = client.get(f"/api/v1/recall/{tenant_id}/report") assert r.status_code == 200 body = r.json() - assert body["demo_mode"] is True - assert body["demo_disclaimer"] is not None - assert body["tenant_id"] == "acme" - # Demo mode uses hardcoded "4.2 hours" estimate - assert "4.2 hours" in body["time_to_respond_estimate"] - # Demo dimensions total is (78+85+95+65+88+72)/6 = 80.5 → 80 - assert body["overall_score"] == 80 - - def test_demo_mode_when_cte_count_zero(self, client, monkeypatch): - monkeypatch.setattr(rr, "_query_scoring_data", lambda tid: {"cte_count": 0}) - - r = client.get("/api/v1/recall/empty-tenant/report") - assert r.status_code == 200 - body = r.json() - assert body["demo_mode"] is True - assert body["demo_disclaimer"] is not None - # Overall score is demo dimensions average - assert body["overall_grade"] in {"B", "C"} + assert body["tenant_id"] == tenant_id + assert body["demo_mode"] is False + assert body["demo_disclaimer"] is None + assert body["dimensions"] == [] + assert body["action_items"] == [] + assert body["overall_score"] == 0 + assert body["overall_grade"] == "N/A" + assert body["overall_status"] == "no_data" + assert body["executive_summary"].startswith("No data yet") + assert body["time_to_respond_estimate"].startswith("Not yet measured") # --------------------------------------------------------------------------- @@ -575,17 +570,31 @@ def test_real_mode_zero_suppliers(self, client, monkeypatch): class TestReportShape: def test_summary_and_citations_present(self, client, monkeypatch): - monkeypatch.setattr(rr, "_query_scoring_data", lambda tid: None) - r = client.get("/api/v1/recall/abc/report") + # Real-mode scoring with gaps; no data is invented by the report. + monkeypatch.setattr( + rr, + "_query_scoring_data", + lambda tid: { + "cte_count": 1, + "tlc_count": 0, + "cte_types": 0, + "has_export": False, + "supplier_count": 0, + "chain_length": 0, + "chain_gap_count": 0, + }, + ) + tenant_id = str(uuid4()) + r = client.get(f"/api/v1/recall/{tenant_id}/report") body = r.json() # Executive summary mentions tenant id + score - assert "abc" in body["executive_summary"] + assert tenant_id in body["executive_summary"] assert "21 CFR 1.1455" in body["executive_summary"] or "21 CFR 1.1455" in " ".join(body["regulatory_citations"]) # Regulatory citations fully populated assert len(body["regulatory_citations"]) == 4 - # Action items populated - assert len(body["action_items"]) >= 5 + # Action items derived from the tenant's own sub-80 dimensions + assert body["action_items"] # All action items have required fields for item in body["action_items"]: assert item["priority"] in {"HIGH", "MEDIUM", "LOW"} From 7d17e972592d46927930695dde0543ced6c21d43 Mon Sep 17 00:00:00 2001 From: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> Date: Fri, 23 Oct 2026 12:16:54 -0700 Subject: [PATCH 2/4] fix(ingestion): surface recall scoring DB errors as 503 instead of no_data Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> --- services/ingestion/app/recall_report.py | 181 +++++++++--------- .../tests/test_recall_report_1342.py | 66 +++++-- 2 files changed, 143 insertions(+), 104 deletions(-) diff --git a/services/ingestion/app/recall_report.py b/services/ingestion/app/recall_report.py index e33f534e4..c62bf685b 100644 --- a/services/ingestion/app/recall_report.py +++ b/services/ingestion/app/recall_report.py @@ -12,7 +12,7 @@ import logging from datetime import datetime, timezone -from fastapi import APIRouter, Depends +from fastapi import APIRouter, Depends, HTTPException from pydantic import BaseModel, Field from shared.tenant_context import TenantContext @@ -21,99 +21,99 @@ logger = logging.getLogger("recall-report") -def _query_scoring_data(tenant_id: str) -> dict | None: - """Query actual tenant data for recall readiness scoring.""" +def _query_scoring_data(tenant_id: str) -> dict: + """Query actual tenant data for recall readiness scoring. + + Database errors propagate to the caller; they are never turned into an + empty result, so only a real ``cte_count == 0`` means "no data". + """ + from shared.database import SessionLocal + from sqlalchemy import text + + db = SessionLocal() try: - from shared.database import SessionLocal - from sqlalchemy import text - - db = SessionLocal() - try: - # Count events from the canonical store of record (#2506). The - # legacy ``fsma.cte_events`` table is a dual-write copy being - # retired; the hash-chain verifier now anchors on - # ``fsma.traceability_events``, so these scoring readers read the - # same source of truth. Columns ``traceability_lot_code`` and - # ``event_type`` are identical on both tables, and no status - # filter is applied here (matching the prior legacy behavior, - # which counted every row regardless of validation status). - cte_count = ( - db.execute( - text("SELECT COUNT(*) FROM fsma.traceability_events WHERE tenant_id = :tid"), {"tid": tenant_id} - ).scalar() - or 0 - ) + # Count events from the canonical store of record (#2506). The + # legacy ``fsma.cte_events`` table is a dual-write copy being + # retired; the hash-chain verifier now anchors on + # ``fsma.traceability_events``, so these scoring readers read the + # same source of truth. Columns ``traceability_lot_code`` and + # ``event_type`` are identical on both tables, and no status + # filter is applied here (matching the prior legacy behavior, + # which counted every row regardless of validation status). + cte_count = ( + db.execute( + text("SELECT COUNT(*) FROM fsma.traceability_events WHERE tenant_id = :tid"), {"tid": tenant_id} + ).scalar() + or 0 + ) - # Count distinct TLCs - tlc_count = ( - db.execute( - text( - "SELECT COUNT(DISTINCT traceability_lot_code) " - "FROM fsma.traceability_events WHERE tenant_id = :tid" - ), - {"tid": tenant_id}, - ).scalar() - or 0 - ) + # Count distinct TLCs + tlc_count = ( + db.execute( + text( + "SELECT COUNT(DISTINCT traceability_lot_code) " + "FROM fsma.traceability_events WHERE tenant_id = :tid" + ), + {"tid": tenant_id}, + ).scalar() + or 0 + ) - # Count CTE types covered - cte_types = ( - db.execute( - text("SELECT COUNT(DISTINCT event_type) FROM fsma.traceability_events WHERE tenant_id = :tid"), - {"tid": tenant_id}, - ).scalar() - or 0 - ) + # Count CTE types covered + cte_types = ( + db.execute( + text("SELECT COUNT(DISTINCT event_type) FROM fsma.traceability_events WHERE tenant_id = :tid"), + {"tid": tenant_id}, + ).scalar() + or 0 + ) - # Check FDA export capability - has_export = ( - db.execute( - text("SELECT COUNT(*) FROM fsma.fda_export_log WHERE tenant_id = :tid"), {"tid": tenant_id} - ).scalar() - or 0 - ) + # Check FDA export capability + has_export = ( + db.execute( + text("SELECT COUNT(*) FROM fsma.fda_export_log WHERE tenant_id = :tid"), {"tid": tenant_id} + ).scalar() + or 0 + ) - # Count suppliers - supplier_count = ( - db.execute( - text("SELECT COUNT(*) FROM fsma.tenant_suppliers WHERE tenant_id = :tid"), {"tid": tenant_id} - ).scalar() - or 0 - ) + # Count suppliers + supplier_count = ( + db.execute( + text("SELECT COUNT(*) FROM fsma.tenant_suppliers WHERE tenant_id = :tid"), {"tid": tenant_id} + ).scalar() + or 0 + ) - # Chain integrity: count chain entries and detect sequence gaps - chain_length = ( + # Chain integrity: count chain entries and detect sequence gaps + chain_length = ( + db.execute( + text("SELECT COUNT(*) FROM fsma.hash_chain WHERE tenant_id = :tid"), {"tid": tenant_id} + ).scalar() + or 0 + ) + + chain_gap_count = 0 + if chain_length > 0: + max_seq = ( db.execute( - text("SELECT COUNT(*) FROM fsma.hash_chain WHERE tenant_id = :tid"), {"tid": tenant_id} + text("SELECT MAX(sequence_num) FROM fsma.hash_chain WHERE tenant_id = :tid"), {"tid": tenant_id} ).scalar() or 0 ) - - chain_gap_count = 0 - if chain_length > 0: - max_seq = ( - db.execute( - text("SELECT MAX(sequence_num) FROM fsma.hash_chain WHERE tenant_id = :tid"), {"tid": tenant_id} - ).scalar() - or 0 - ) - # Gaps = expected entries minus actual entries - chain_gap_count = max(0, max_seq - chain_length) - - return { - "cte_count": cte_count, - "tlc_count": tlc_count, - "cte_types": cte_types, - "has_export": has_export > 0, - "supplier_count": supplier_count, - "chain_length": chain_length, - "chain_gap_count": chain_gap_count, - } - finally: - db.close() - except (RuntimeError, OSError, AttributeError, TypeError, ValueError) as exc: - logger.warning("recall_scoring_query_failed error=%s", str(exc)) - return None + # Gaps = expected entries minus actual entries + chain_gap_count = max(0, max_seq - chain_length) + + return { + "cte_count": cte_count, + "tlc_count": tlc_count, + "cte_types": cte_types, + "has_export": has_export > 0, + "supplier_count": supplier_count, + "chain_length": chain_length, + "chain_gap_count": chain_gap_count, + } + finally: + db.close() router = APIRouter(prefix="/api/v1/recall", tags=["Recall Readiness"]) @@ -221,9 +221,16 @@ async def generate_report( tenant_id = ctx.tenant_id now = datetime.now(timezone.utc) - # Try to query real data - scoring_data = _query_scoring_data(tenant_id) - has_data = scoring_data is not None and scoring_data.get("cte_count", 0) > 0 + # Query real data. A query failure is a server error, not "no data". + try: + scoring_data = _query_scoring_data(tenant_id) + except Exception as exc: + logger.error("recall_scoring_query_failed error_type=%s", type(exc).__name__) + raise HTTPException( + status_code=503, + detail="Recall readiness data is temporarily unavailable; please retry.", + ) from exc + has_data = scoring_data.get("cte_count", 0) > 0 if has_data: # Compute real scores based on actual data diff --git a/services/ingestion/tests/test_recall_report_1342.py b/services/ingestion/tests/test_recall_report_1342.py index a1a0554af..78cad6b4f 100644 --- a/services/ingestion/tests/test_recall_report_1342.py +++ b/services/ingestion/tests/test_recall_report_1342.py @@ -3,9 +3,9 @@ Locks: - _grade / _status helpers — all threshold bands - _query_scoring_data: happy path with gaps, chain_length=0 path - (no max_seq query), SessionLocal raise → None, execute raise → None + (no max_seq query), SessionLocal raise and execute raise propagate (not None) - Pydantic models — RecallDimension, RecallReport -- GET /{tenant_id}/report: no data (None scoring or cte_count=0) → empty report, +- GET /{tenant_id}/report: cte_count=0 → empty report; DB error → 503 (never no_data), real mode (low suppliers + no chain + no export), real mode (medium suppliers + chain gaps + has export), real mode (high suppliers + clean chain + has export) @@ -207,7 +207,7 @@ def test_happy_path_with_gaps(self, monkeypatch): monkeypatch.setattr(db_mod, "SessionLocal", lambda: sess) - data = _query_scoring_data("t1") + data = _query_scoring_data(str(uuid4())) assert data is not None assert data["cte_count"] == 50 assert data["tlc_count"] == 8 @@ -225,7 +225,7 @@ def test_happy_path_no_gaps(self, monkeypatch): monkeypatch.setattr(db_mod, "SessionLocal", lambda: sess) - data = _query_scoring_data("t1") + data = _query_scoring_data(str(uuid4())) assert data["chain_gap_count"] == 0 def test_has_export_false_when_count_zero(self, monkeypatch): @@ -234,7 +234,7 @@ def test_has_export_false_when_count_zero(self, monkeypatch): monkeypatch.setattr(db_mod, "SessionLocal", lambda: sess) - data = _query_scoring_data("t1") + data = _query_scoring_data(str(uuid4())) assert data["has_export"] is False def test_chain_length_zero_skips_max_seq(self, monkeypatch): @@ -250,7 +250,7 @@ def test_chain_length_zero_skips_max_seq(self, monkeypatch): monkeypatch.setattr(db_mod, "SessionLocal", lambda: sess) - data = _query_scoring_data("t1") + data = _query_scoring_data(str(uuid4())) assert data["chain_length"] == 0 assert data["chain_gap_count"] == 0 @@ -262,24 +262,25 @@ def test_scalar_returns_none_coalesces_to_zero(self, monkeypatch): monkeypatch.setattr(db_mod, "SessionLocal", lambda: sess) - data = _query_scoring_data("t1") + data = _query_scoring_data(str(uuid4())) assert data is not None assert data["cte_count"] == 0 assert data["chain_length"] == 0 assert data["chain_gap_count"] == 0 - def test_sessionlocal_raises_returns_none(self, monkeypatch): - """SessionLocal() itself raising is caught and returns None.""" + def test_sessionlocal_raises_propagates(self, monkeypatch): + """SessionLocal() raising is NOT swallowed into None/no-data.""" import shared.database as db_mod def _boom(): raise RuntimeError("connection refused") monkeypatch.setattr(db_mod, "SessionLocal", _boom) - assert _query_scoring_data("t1") is None + with pytest.raises(RuntimeError): + _query_scoring_data(str(uuid4())) - def test_execute_raises_returns_none(self, monkeypatch): - """An execute() exception is caught and returns None.""" + def test_execute_raises_propagates_and_closes(self, monkeypatch): + """An execute() exception propagates; db.close() still runs.""" sess = _ScoringSession( raise_on={"COUNT(*) FROM fsma.traceability_events": ValueError("boom")}, ) @@ -287,8 +288,8 @@ def test_execute_raises_returns_none(self, monkeypatch): monkeypatch.setattr(db_mod, "SessionLocal", lambda: sess) - assert _query_scoring_data("t1") is None - # db.close() was still called via finally + with pytest.raises(ValueError): + _query_scoring_data(str(uuid4())) assert sess.closed is True @@ -298,9 +299,8 @@ def test_execute_raises_returns_none(self, monkeypatch): class TestGenerateReportNoData: - @pytest.mark.parametrize("scoring", [None, {"cte_count": 0}]) - def test_no_data_returns_empty_report(self, client, monkeypatch, scoring): - monkeypatch.setattr(rr, "_query_scoring_data", lambda tid: scoring) + def test_zero_ctes_returns_empty_report(self, client, monkeypatch): + monkeypatch.setattr(rr, "_query_scoring_data", lambda tid: {"cte_count": 0}) tenant_id = str(uuid4()) r = client.get(f"/api/v1/recall/{tenant_id}/report") @@ -318,6 +318,38 @@ def test_no_data_returns_empty_report(self, client, monkeypatch, scoring): assert body["time_to_respond_estimate"].startswith("Not yet measured") +# --------------------------------------------------------------------------- +# GET /{tenant_id}/report — a DB error is a 5xx, never an empty "no_data" report +# --------------------------------------------------------------------------- + + +class TestGenerateReportDbError: + @pytest.mark.parametrize("exc_type", [RuntimeError, OSError, ValueError]) + def test_db_error_returns_5xx_not_no_data(self, client, monkeypatch, exc_type): + def _fail(tid): + raise exc_type(str(uuid4())) + + monkeypatch.setattr(rr, "_query_scoring_data", _fail) + + r = client.get(f"/api/v1/recall/{uuid4()}/report") + assert r.status_code == 503 + body = r.json() + assert "overall_status" not in body + assert "no_data" not in r.text + assert "No data yet" not in r.text + + def test_sessionlocal_failure_end_to_end_is_5xx(self, client, monkeypatch): + import shared.database as db_mod + + def _boom(): + raise RuntimeError(str(uuid4())) + + monkeypatch.setattr(db_mod, "SessionLocal", _boom) + r = client.get(f"/api/v1/recall/{uuid4()}/report") + assert r.status_code == 503 + assert "no_data" not in r.text + + # --------------------------------------------------------------------------- # GET /{tenant_id}/report — real mode # --------------------------------------------------------------------------- From 29354a1c74fc4ab64344031e248ba2a1a86caeef Mon Sep 17 00:00:00 2001 From: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> Date: Fri, 23 Oct 2026 18:18:54 -0700 Subject: [PATCH 3/4] chore(ci): retrigger checks Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> From 1ec1ba1921137c3dbf529162c9a97b9719cb50e9 Mon Sep 17 00:00:00 2001 From: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> Date: Sun, 25 Oct 2026 21:25:42 -0700 Subject: [PATCH 4/4] test(ingestion): assert 503 recall report never leaks exception text Signed-off-by: Christopher <131207452+PetrefiedThunder@users.noreply.github.com> --- services/ingestion/tests/test_recall_report_1342.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/services/ingestion/tests/test_recall_report_1342.py b/services/ingestion/tests/test_recall_report_1342.py index 78cad6b4f..deb378866 100644 --- a/services/ingestion/tests/test_recall_report_1342.py +++ b/services/ingestion/tests/test_recall_report_1342.py @@ -326,8 +326,10 @@ def test_zero_ctes_returns_empty_report(self, client, monkeypatch): class TestGenerateReportDbError: @pytest.mark.parametrize("exc_type", [RuntimeError, OSError, ValueError]) def test_db_error_returns_5xx_not_no_data(self, client, monkeypatch, exc_type): + sentinel = str(uuid4()) + def _fail(tid): - raise exc_type(str(uuid4())) + raise exc_type(sentinel) monkeypatch.setattr(rr, "_query_scoring_data", _fail) @@ -337,17 +339,20 @@ def _fail(tid): assert "overall_status" not in body assert "no_data" not in r.text assert "No data yet" not in r.text + assert sentinel not in r.text def test_sessionlocal_failure_end_to_end_is_5xx(self, client, monkeypatch): + sentinel = str(uuid4()) import shared.database as db_mod def _boom(): - raise RuntimeError(str(uuid4())) + raise RuntimeError(sentinel) monkeypatch.setattr(db_mod, "SessionLocal", _boom) r = client.get(f"/api/v1/recall/{uuid4()}/report") assert r.status_code == 503 assert "no_data" not in r.text + assert sentinel not in r.text # ---------------------------------------------------------------------------