Skip to content

fix(extraction): graceful error for scanned PDFs with no text layer#10

Closed
aircode610 wants to merge 11 commits into
mainfrom
fix/issue-8-scanned-pdf-extraction
Closed

fix(extraction): graceful error for scanned PDFs with no text layer#10
aircode610 wants to merge 11 commits into
mainfrom
fix/issue-8-scanned-pdf-extraction

Conversation

@aircode610

@aircode610 aircode610 commented Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Summary

Fixes #8 — uploading a scanned, image-only PDF (no OCR text layer) returned a 500 / generic error instead of a user-friendly message.

Root cause

Traced the upload → extraction path:

  1. POST /letters (and /api/letters/{id}/extract) call extract_from_letter_file() in backend/app/services/extraction.py.
  2. The PDF is rendered to page images (split_to_image_bytes), which works fine even for scanned PDFs.
  3. The page images go to the Qwen vision model. For a scanned image-only PDF the model often returns no structured tool call.
  4. That hit the unguarded RuntimeError("Model returned no tool call; ...") at extraction.py:270, which bubbled up as a 500 (or a generic, unhelpful 502).

Fix

  • Add ExtractionError carrying a user-facing message + a machine-readable Klar ErrorCode — mirrors the existing ai.react_agent.ocr.OcrError pattern.
  • Replace the raw RuntimeError on the empty-tool-call path with an ExtractionError explaining the document may be a scanned image without readable text.
  • Guard split_to_image_bytes() (corrupt / password-protected PDF, missing poppler) and add an empty-page guard, both surfaced as PDF_RENDER_FAILED.
  • Surface ExtractionError.message + .code in both the /api/letters/{id}/extract and POST /letters endpoints, instead of the generic EXTRACTION_FAILED.

Now a scanned PDF returns a clean error envelope, e.g.:

{
  "code": "EXTRACTION_FAILED",
  "message": "Could not extract text from this document. It may be a scanned image without readable content — try a clearer photo or PDF."
}

Testing

  • ruff check app/ — passes
  • ruff format --check — passes
  • Module imports verified (no import cycle from the new app.errors import in extraction.py)

Closes #8

Follow-up hardening (commit 00956ed)

While verifying the fix end-to-end, I found two remaining gaps in the SSE /process pipeline OCR path (ai/react_agent/ocr.py) — the path the production upload flow actually uses (the sync /extract path uses extraction.py, fixed above):

  1. Asymmetric blank-content guard. The PDF branch of extract_text_from_image() raised OcrError when OCR yielded no readable text, but the single-image branch did not — a scanned/blank photo (.jpg/.png) silently returned empty text and fed it to the downstream agent. Now both branches behave symmetrically.
  2. Unguarded OCR response parsing. _ocr_image_bytes() blindly indexed result["choices"][0]["message"]["content"]. A 200 OK with an unexpected JSON shape (provider error body, null content) raised a raw KeyError/IndexError → 500. Now wrapped in a graceful OcrError.

Added regression tests for both (test_blank_image_raises_ocr_error, test_malformed_ocr_response_raises_ocr_error). Full suite: 6/6 passing; ruff check + ruff format --check clean.

aircode610 and others added 11 commits June 6, 2026 19:05
@claude
fix(form-fill): stronger image preservation in edit prompt
226d0e3 
Emphasize pixel-perfect preservation of original document. The model
should only ADD red handwritten overlays on blank fields, never
modify or regenerate any existing content.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude
refactor(form-fill): Pillow overlay instead of image generation
d59334a 
Two-step deterministic approach:
1. Qwen-VL detects blank form fields + coordinates (% of image)
2. Pillow draws red placeholder boxes + English text at those positions

No image regeneration — original document preserved pixel-perfect.
Robust JSON parsing handles LLM quirks (single quotes, trailing commas,
malformed coordinate pairs).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude
feat(form-fill): deterministic Pillow overlay with Qwen-VL field dete…
06e9ac7 
…ction

Two-step approach:
1. Qwen-VL-Max detects blank form fields → bbox_2d [x1,y1,x2,y2] in 0-1000 range
2. Pillow draws red highlight boxes + English placeholder text at exact positions

Tested on tax form (data/fields.png): all 8 form fields detected and annotated.
Original image preserved pixel-perfect — only red overlays added.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude
fix(form-fill): dynamic coordinate scaling from model's internal reso…
af2bfe1 
…lution

Qwen-VL returns bbox_2d in its internal processing resolution, not
a fixed 0-1000 range. Now derives scale factors from the max coordinate
values in the response, mapping accurately to actual image pixels.

Tested on data/fields.png: all 8 form fields correctly overlaid.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude
fix(form-fill): size overlay boxes to match placeholder text width
40e7fcf 
Boxes now measured from the actual text width instead of the full
detected bbox — no more covering existing printed labels.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Added ci
1264471
@claude
fix(lint): resolve all ruff check and format violations across backen…
b41229b 
…d/ and ai/

Remove unused imports (json, re, datetime, os, sys, timedelta, List, Dict,
Optional, Any, RagHit, generate_reply_text), replace bare except with
except Exception, move module-level imports to top of file, remove unused
variable assignments (classification_data, action_titles), strip
extraneous f-string prefixes, and apply ruff formatting throughout.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fix(ocr): render PDFs to images before OCR to prevent 500 crash
3e9cc15 
extract_text_from_image read raw file bytes and sent them to the
Qwen-VL-OCR model labeled as image/jpeg. For PDFs this handed the
model undecodable bytes, causing the /process SSE pipeline to fail
(notably for scanned image-only PDFs).

Now PDFs are rendered to one PNG per page via poppler/pdf2image and
each page is OCR'd and concatenated. Added an OcrError with
user-facing messages for corrupt/empty/scanned PDFs, surfaced through
the SSE error event as PDF_RENDER_FAILED instead of a raw 500.

Fixes #8
fix(extraction): graceful error for scanned PDFs with no text layer
58d003e 
A scanned, image-only PDF (no OCR text layer) made the Qwen vision model
return no structured tool call, which hit an unguarded RuntimeError in
extract_from_letter_file() and surfaced as a 500/generic 502 instead of a
helpful message.

- Add ExtractionError carrying a user-facing message + Klar ErrorCode
  (mirrors ai.react_agent.ocr.OcrError).
- Replace the raw RuntimeError on the empty-tool-call path with an
  ExtractionError explaining the document may be a scanned image without
  readable text.
- Guard split_to_image_bytes() (corrupt/password-protected PDF, missing
  poppler) and add an empty-page guard, both as PDF_RENDER_FAILED.
- Surface ExtractionError.message + .code in the /api/letters/{id}/extract
  and POST /letters endpoints instead of a generic EXTRACTION_FAILED.

Fixes #8
test(extraction): regression tests for graceful scanned-PDF handling (#8
5d198bf 
)

Lock in the issue #8 fix: a scanned, image-only PDF (no text layer) must
surface a user-facing error instead of a raw 500.

- Sync path: extract_from_letter_file raises ExtractionError with
  EXTRACTION_FAILED when the vision model returns no tool call, and
  PDF_RENDER_FAILED when rendering yields no pages / crashes.
- SSE/OCR path: extract_text_from_image raises OcrError on a blank scan.

Pure stdlib unittest (no pytest dep) and ruff-clean.
fix(ocr): graceful errors for blank single-image scans + malformed OC…
00956ed 
…R responses

Closes the remaining gaps in the scanned-PDF/no-text-layer fix (#8):

- The single-image (non-PDF) OCR branch lacked the blank-content guard the
  PDF branch already had — a scanned/blank image returned empty text silently
  and fed it to the downstream agent. Now raises a graceful OcrError, matching
  the PDF branch.
- _ocr_image_bytes() blindly indexed result["choices"][0]["message"]["content"];
  a 200 OK with an unexpected JSON shape (provider error body, null content)
  would surface as a raw KeyError/IndexError 500. Now wrapped in OcrError.

Adds regression tests for both paths.
@aircode610 aircode610 mentioned this pull request Jun 22, 2026
Closed
@aircode610

aircode610 commented Jun 22, 2026

Copy link
Copy Markdown
Owner Author

🛡️ Migration safety review — automated

I reviewed this PR specifically for new database migrations and their safety against production data.

✅ Result: No migration present — schema is unchanged

  • This project uses SQLModel.metadata.create_all(engine) (SQLite) and has no migration framework (no Alembic / versions/ directory), so there are no migration files to evaluate.
  • The only schema-adjacent files touched are backend/app/models.py and backend/app/database.py, and both changes are purely cosmetic (ruff formatting):
    • models.py: comment re-alignment + a line-wrap of GOVERNMENT_BENEFITS. The enum value is still "government_benefits" — no columns added/removed, no type changes.
    • database.py: one added blank line.
  • A scan of the entire PR diff for DDL / data-mutating statements (ALTER/CREATE/DROP TABLE, ADD/DROP COLUMN, RENAME, index ops, TRUNCATE, DELETE FROM, UPDATE ... SET, Alembic op.*, new Column(...)) returned nothing.

Checklist

  • Reversible / rollback path — N/A (no migration)
  • Downtime risk — none
  • Data-loss risk — none
  • Lock contention on large tables — none

Verdict: safe to run on production data — no schema migration is introduced by this PR.

Note (out of scope for this PR): the project relies on create_all, which only creates missing tables and does not apply column-level changes to existing tables. If future PRs modify existing model fields, they will need a real migration strategy.

🤖 Automated migration-safety check

This was referenced Jun 22, 2026
Closed
Closed
Merged
@aircode610

aircode610 commented Jun 22, 2026

Copy link
Copy Markdown
Owner Author

Closing in favour of #13 which has been merged.

@aircode610 aircode610 closed this Jun 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

500 error on /api/letters upload when PDF has no text layer

1 participant

@aircode610