Skip to content

fix(ocr): render scanned PDF pages before embedded images - #2348

Open
Elioooon (Elioooon) wants to merge 2 commits into
microsoft:mainfrom
Elioooon:fix/scanned-pdf-page-ocr
Open

Elioooon (Elioooon) wants to merge 2 commits into
microsoft:mainfrom
Elioooon:fix/scanned-pdf-page-ocr

Conversation

@Elioooon

Copy link
Copy Markdown

Summary

Fixes #2343.

  • render zero-text PDF pages at 300 DPI and OCR the full page before considering embedded images
  • reuse the already-open pdfplumber page for image extraction instead of reopening the whole PDF once per page
  • share the existing full-page render/OCR path between per-page and document fallbacks

This keeps mixed PDFs intact: text pages still interleave extracted text and embedded-image OCR, while scanned pages no longer lose their body content to small decorative images.

Verification

  • python -m pytest packages/markitdown-ocr/tests/test_pdf_converter.py -q -k 'scanned_page_uses_full_page_ocr or convert_reuses_open_page_for_image_extraction' - 2 passed
  • python -m pytest packages/markitdown-ocr/tests -q - 37 passed, 1 pre-existing failure: test_pdf_multipage assumes pdfplumber rejects the fixture, but the current dependency version extracts it successfully; reproduced unchanged on main
  • black==23.7.0 --check packages/markitdown-ocr/src/markitdown_ocr/_pdf_converter_with_ocr.py packages/markitdown-ocr/tests/test_pdf_converter.py - passed
  • git diff --check - passed

Risk

Scoped to the OCR plugin's PDF path. Text-only conversion and non-PDF converters are unchanged. A page is treated as scanned only when page.extract_text() is empty or whitespace.

@Elioooon

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

@Elioooon
Elioooon (Elioooon) force-pushed the fix/scanned-pdf-page-ocr branch 5 times, most recently from 72a4f5d to 728a982 Compare September 16, 2026 18:34
@Elioooon
Elioooon (Elioooon) force-pushed the fix/scanned-pdf-page-ocr branch 2 times, most recently from c9ce9a1 to 085ba17 Compare September 21, 2026 21:34
@cagdasyurekli

Copy link
Copy Markdown
Contributor

I found a recovery regression at 085ba17 when full-page rendering fails. For an empty-text page, the new branch catches _ocr_page()'s exception, appends an error message and unconditionally continues. It never tries the embedded-image path, even when that image can be decoded directly without rendering the page.

I compared the parent and this head with the same controlled case: page.extract_text() is empty, page.to_image() raises, and _extract_page_images() can return a usable scan. The parent calls the image extractor and OCR service once and retains the transcription. This head calls neither and returns only the page heading and render error. No remote OCR was used. The 15 existing PDF converter tests pass, so they do not catch this regression.

Could full-page OCR remain the preferred path, but fall back to embedded-image extraction when rendering fails? A regression comparing that failure path would preserve recovery without returning to the decorative-image-first behavior this PR fixes.

@Elioooon

Copy link
Copy Markdown
Author

Addressed in 559933d.

Full-page OCR remains the preferred path for empty-text pages. If page rendering fails, the converter now tries OCR on usable embedded images from that same page before returning the render error, so the existing recovery path is retained without restoring image-first behavior.

The regression test injects a page.to_image() failure while providing a decodable embedded image; it verifies the embedded path is used and the OCR block is preserved.

Validation: python -m pytest packages/markitdown-ocr/tests/test_pdf_converter.py -q (16 passed); git diff --check clean.

@cagdasyurekli

Copy link
Copy Markdown
Contributor

Thanks, verified 559933d. The render-failure regression now passes and preserves the embedded-image OCR block. I also ran the full OCR plugin suite after installing its test dependency: 111 passed on Python 3.12 (16 in test_pdf_converter.py). No remote OCR calls were used. I will drop the duplicate local fix for this finding.

@psycmos

psycmos commented Sep 29, 2026

Copy link
Copy Markdown

Independently reproduced this on real-world data (a Turkish customs-style scanned
PDF mixed with a native-text invoice page, synthesized as a 2-page PDF: page 1
native, page 2 scanned). Before this patch: page 2's content was silently dropped
(the whole-document markdown was non-empty because of page 1, so the
_ocr_full_pages fallback never triggered). After applying this diff: page 2 is
correctly rendered and OCR'd, page 1's native extraction is untouched. Can confirm
this fixes the exact bug on an independent setup.

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.

markitdown-ocr: three PDF bugs — scanned pages never OCR'd, per-page PDF reopen, page-level scan misdetection

3 participants