From 8ab98d3d62f8e36d9883b82c62fa19e4b210def2 Mon Sep 17 00:00:00 2001 From: aircode610 Date: Mon, 22 Jun 2026 10:40:26 +0200 Subject: [PATCH] fix(pipeline): robustly surface scanned-PDF OcrError on SSE path (#8) The SSE /process orchestrator is the production upload->process flow for issue #8: a scanned image-only PDF (no text layer) makes the OCR stage raise OcrError, which must reach the client as a graceful 'error' event (PDF_RENDER_FAILED), never a crashed stream / raw 500. - Detect OcrError via isinstance instead of a fragile type(exc).__name__ string match, so the guard survives subclassing and can't be silently broken by a refactor. OcrError is imported in the existing lazy-import block alongside extract_text_from_image. - Add the first regression test that exercises the orchestrator wiring (process_letter_stream), asserting an OcrError becomes exactly one graceful SSE error event with code PDF_RENDER_FAILED + its user-facing message. Previously only the leaf functions (extraction.py / ocr.py) were covered, leaving the production SSE path untested. Full suite: 7/7 passing; ruff check + format clean. --- backend/app/pipeline/orchestrator.py | 11 +++- backend/tests/test_scanned_pdf_graceful.py | 68 ++++++++++++++++++++++ 2 files changed, 76 insertions(+), 3 deletions(-) diff --git a/backend/app/pipeline/orchestrator.py b/backend/app/pipeline/orchestrator.py index b30b447..cccad63 100644 --- a/backend/app/pipeline/orchestrator.py +++ b/backend/app/pipeline/orchestrator.py @@ -214,7 +214,7 @@ async def process_letter_stream(letter_id: UUID, lang: str) -> AsyncIterator[str # BEFORE its first yield — that race causes the browser to see # ERR_INCOMPLETE_CHUNKED_ENCODING and infinitely reconnect. try: - from ai.react_agent.ocr import extract_text_from_image + from ai.react_agent.ocr import OcrError, extract_text_from_image from ai.react_agent.agent import run_react_agent except Exception as exc: logger.exception( @@ -540,8 +540,13 @@ async def process_letter_stream(letter_id: UUID, lang: str) -> AsyncIterator[str message: str | None = None module = type(exc).__module__ # OcrError carries a user-facing message (e.g. scanned/corrupt PDF) — - # surface it directly instead of a generic code. - if type(exc).__name__ == "OcrError": + # surface it directly instead of a generic code. This is the SSE + # production path's guard for issue #8: a scanned image-only PDF (no + # text layer) makes the OCR stage raise OcrError, which must reach + # the client as a graceful `error` event, never a crashed stream. + # Use isinstance (not a fragile `__name__` string match) so the guard + # survives subclassing and can't be silently broken by a refactor. + if isinstance(exc, OcrError): code = ErrorCode.PDF_RENDER_FAILED message = str(exc) or None elif "openai" in module or "httpx" in module or "langchain" in module: diff --git a/backend/tests/test_scanned_pdf_graceful.py b/backend/tests/test_scanned_pdf_graceful.py index 1341fa3..a101d53 100644 --- a/backend/tests/test_scanned_pdf_graceful.py +++ b/backend/tests/test_scanned_pdf_graceful.py @@ -8,6 +8,10 @@ ``ExtractionError`` (carrying a Klar ``ErrorCode``), never a bare 500. - SSE/OCR path: ``ai.react_agent.ocr.extract_text_from_image`` raises ``OcrError`` for a blank scan. +- SSE orchestrator (the production upload→process flow): + ``app.pipeline.orchestrator.process_letter_stream`` turns that ``OcrError`` + into a graceful SSE ``error`` event (``PDF_RENDER_FAILED``) instead of a + crashed stream / raw 500. Pure stdlib ``unittest`` — no pytest dependency — so it runs anywhere with:: @@ -17,6 +21,7 @@ import asyncio import os import sys +import types import unittest from unittest.mock import AsyncMock, MagicMock, patch @@ -137,5 +142,68 @@ def test_malformed_ocr_response_raises_ocr_error(self): _run(_ocr_image_bytes(fake_client, b"img-bytes", "image/png")) +class OrchestratorSsePathTest(unittest.TestCase): + """Covers app/pipeline/orchestrator.py — the SSE /process production path. + + In production, issue #8 is served by the SSE pipeline: a scanned image-only + PDF makes the OCR stage (``extract_text_from_image``) raise ``OcrError``. + The orchestrator MUST turn that into a graceful SSE ``error`` event + (``PDF_RENDER_FAILED``) — never let it propagate as a crashed stream / raw + 500. The leaf-function tests above never exercise this wiring, so a refactor + of the orchestrator's catch block could silently reintroduce the 500. + """ + + @staticmethod + def _drain(gen): + async def _collect(): + return [event async for event in gen] + + return _run(_collect()) + + def test_ocr_error_becomes_graceful_sse_error_event(self): + from uuid import uuid4 + + import ai.react_agent.ocr as ocr_mod + from app.pipeline import orchestrator + + # Stub the heavy agent module so the orchestrator's lazy + # `from ai.react_agent.agent import run_react_agent` import succeeds + # without pulling in the langchain/Tavily stack. + agent_stub = types.ModuleType("ai.react_agent.agent") + agent_stub.run_react_agent = lambda *a, **k: None # never reached here + + # A fake Letter row with a file set, so we reach the OCR stage. + fake_letter = MagicMock() + fake_letter.original_file = "/tmp/scan.pdf" + + fake_db = MagicMock() + fake_db.get.return_value = fake_letter + db_cm = MagicMock() + db_cm.__enter__.return_value = fake_db + db_cm.__exit__.return_value = False + + with ( + patch.dict(sys.modules, {"ai.react_agent.agent": agent_stub}), + patch.object( + ocr_mod, + "extract_text_from_image", + new=AsyncMock(side_effect=OcrError("scanned PDF, no text layer")), + ), + patch.object(orchestrator, "DBSession", return_value=db_cm), + ): + events = self._drain(orchestrator.process_letter_stream(uuid4(), "en")) + + # The stream must terminate with exactly one graceful error event — + # the generator must NOT raise. + error_events = [e for e in events if e.startswith("event: error")] + self.assertEqual(len(error_events), 1, f"events were: {events!r}") + payload = error_events[0] + # OcrError → PDF_RENDER_FAILED, carrying the user-facing message. + self.assertIn(ErrorCode.PDF_RENDER_FAILED.value, payload) + self.assertIn("scanned PDF, no text layer", payload) + # Regression guard: never the bare generic code on a known OcrError. + self.assertNotIn("event: ocr_result", "".join(events)) + + if __name__ == "__main__": unittest.main(verbosity=2)