fix(pipeline): robustly surface scanned-PDF OcrError on the SSE /process path (#8) - #11
Closed
aircode610 wants to merge 1 commit into
Closed
Conversation
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.
This was referenced Jun 22, 2026
Owner
Author
|
Closing in favour of #13 which has been merged. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up hardening for #8 (500 on
/api/lettersupload when a PDF has no text layer), stacked on #10.#10 fixed the leaf functions (
extraction.py,ai/react_agent/ocr.py) and added the orchestrator'sOcrErrorhandling — but two things were left thin on the SSE/processpath, which #10 itself calls "the path the production upload flow actually uses":OcrErrorwith a fragiletype(exc).__name__ == "OcrError"string match in its catch-all.Changes
backend/app/pipeline/orchestrator.pyOcrErrorin the existing lazy-import block (alongsideextract_text_from_image, so no new eager import of the AI stack).isinstance(exc, OcrError)instead of the brittle__name__string match — survives subclassing and can't be silently broken by a rename/refactor.backend/tests/test_scanned_pdf_graceful.pyOrchestratorSsePathTest: drivesprocess_letter_streamwith the OCR stage raisingOcrError(DB + agent import mocked) and asserts the stream emits exactly one graceful SSEerrorevent carryingPDF_RENDER_FAILED+ the user-facing message — and never raises.Why it matters
This closes the test-coverage gap on the exact production path the issue describes. Verified the test is a real guard: temporarily bypassing the
OcrErrorbranch makes it fail (the stream falls back to the genericEXTRACTION_FAILED), and it passes with the fix.Testing
python backend/tests/test_scanned_pdf_graceful.py— 7/7 passing (was 6/6).ruff check+ruff format --check— clean.Refs #8. Builds on #10.