fix(pdf): raise clear error when fast strategy can't handle complex PDF - #4432
Open
Sohel2309 wants to merge 1 commit into
Open
fix(pdf): raise clear error when fast strategy can't handle complex PDF#4432Sohel2309 wants to merge 1 commit into
Sohel2309 wants to merge 1 commit into
Conversation
partition_pdf() skips pdfminer text extraction for PDFs flagged by is_pdf_too_complex() as mostly vector graphics (e.g. CAD/engineering drawings), since pdfminer is slow and unreliable on them. For strategy="auto" this correctly falls back to another strategy. But an explicitly-requested strategy="fast" has no fallback available, so it silently returned an empty element list with no indication anything went wrong. This raises a ValueError in that case instead, explaining why and suggesting strategy="hi_res" or strategy="auto". strategy="auto" is unaffected and continues to fall back gracefully, and normal (non- complex) PDFs with strategy="fast" are unaffected. Fixes Unstructured-IO#4260
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
No issues found across 3 files
Shadow auto-approve: would auto-approve. Focused bug fix: explicit fast strategy on a complex PDF now raises a clear ValueError instead of silently returning an empty list, with regression tests pinning the error and the preserved auto fallback. No new contract, schema, or operational tradeoffs introduced.
Re-trigger cubic
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
partition_pdf(..., strategy="fast")silently returns an empty element list — no exception, no warning — when the PDF trips the internalis_pdf_too_complex()heuristic.This change raises a clear
ValueErrorinstead.Fixes #4260
Problem
is_pdf_too_complex()exists to prevent pdfminer from running on documents where it performs poorly.When the check fires, extraction is skipped and
extracted_elementsremains empty.For
strategy="auto", the existing strategy-selection logic can fall back to another strategy. However, when the user explicitly requestsstrategy="fast", the strategy remainsfast, and the function eventually returns an empty list.This results in a silent failure where the caller receives no elements and no explanation.
Solution
When
is_pdf_too_complex()returnsTrueandstrategy="fast"was explicitly requested, raise a clearValueErrorsuggestingstrategy="hi_res"orstrategy="auto".This preserves the existing complexity protection instead of bypassing it.
Normal
fastPDFs and the existingautobehavior remain unchanged.Tests
Added a regression test covering explicit
faststrategy on a complex PDF.The regression test fails with the previous behavior and passes with the fix.
Also verified the relevant PDF and strategy tests, along with Ruff formatting and lint checks.
Related Issue
Fixes #4260