Skip to content

feat(pdf): report pages that need ocr - #140

Merged
tomsideguide merged 4 commits into
mainfrom
feat/pdf-needs-ocr
Aug 27, 2026
Merged

tomsideguide merged 4 commits into
mainfrom
feat/pdf-needs-ocr

Conversation

@tomsideguide

@tomsideguide tomsideguide commented Aug 27, 2026 •

Copy link
Copy Markdown
Member

A mixed PDF silently dropped its scanned pages from the output, and a fully scanned one failed as unsupported with nothing to branch on.

Conversion now fails with ConvertError::NeedsOcr naming the pages: needsOcr with pages and pageCount on the error in Node and wasm, NeedsOcrError with pages and page_count in Python, exit code 3 on the CLI.

Detection alone over-reports short or image-heavy text pages (the existing text.pdf fixture is one), so the page list is confirmed through per-page extraction. Watch the second pass on documents detection flags.


Summary by cubic

Reports PDF pages that need OCR as a distinct NeedsOcr error instead of silently dropping them from mixed documents or failing fully scanned ones as unsupported.

Error contract

  • Node and wasm reject with code: 'needsOcr' carrying pages and pageCount; Python raises NeedsOcrError with pages and page_count.
  • The CLI exits 3 for these documents.
  • Messages name the specific pages, like "page 2 of 2 needs OCR" or "all 2 pages need OCR".

Detection

  • Page lists are confirmed with a per-page extraction pass because detection over-reports short or image-heavy text pages.
  • Flagged documents now take a second pass before failing.

Written for commit b5ce6eb. Summary will update on new commits.

Review in cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 26 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread src/error.rs Outdated
Comment thread src/formats/pdf.rs Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant