Shape PyMuPDF4LLM OCR config for upstream: reactive failure, declarative config#2
Open
veget-able wants to merge 2 commits into
Open
Conversation
…lve backend internally Clean the PyMuPDF4LLM provider's OCR surface so it can go to public upstream. Remove the proactive `pymupdf.get_tessdata()` probe (was in `_markdown_options` for `ocr_backend="tesseract"`). It was the only proactive engine probe in the tree and it spawns a `tesseract --list-langs` subprocess: ~350 ms/page as measured in review (~70 ms/call floor locally, two subprocess spawns), and under PYTHONUTF8=1 on a Korean (cp949) console its output fails UTF-8 decode inside subprocess reader threads, printing uncatchable "Exception in thread / UnicodeDecodeError" noise. Tesseract now fails reactively like the rest of upstream: an unavailable engine surfaces as ProviderConfigError only when that backend is actually requested (at import in `_resolve_ocr_function`), not eagerly for every request. The old probe carried no fallback semantics (it only raised), so nothing else was needed. Stop injecting the `ocr_function` callable into the options dict. `ocr_backend` stays a plain string config key (it is a ParseBench-level selector, not a to_markdown kwarg); the engine module/function is resolved internally from a module-level map at call time and passed to `to_markdown(ocr_function=...)` as a direct local kwarg, immediately before the call. `_markdown_options()` now returns only declarative, serializable library kwargs -- important because the runner json-dumps `pipeline.config` into run metadata; keeping callables out of any options/config dict keeps that serialization clean. Mirror the pymupdf4llm library API for config-key names: `use_ocr`, `force_ocr`, `ocr_dpi`, `ocr_language` map 1:1 to the fork build's `to_markdown` kwargs and are forwarded verbatim, so keep those names (documented in a code comment). This consciously answers the review suggestion to rename `ocr_dpi`->`dpi` / drop `force_ocr`: mirroring the library the provider wraps beats mirroring another provider's ad-hoc naming. docs/pipelines.md: add the five pipelines this branch introduces (pymupdf4llm_markdown + _tesseract/_rapidocr/_no_ocr/_150dpi). html_tables is intentionally left for the table-output PR. Tests: extend test_pymupdf4llm.py for the removed probe (get_tessdata is not called while building options), the declarative options dict (no ocr_function / ocr_backend keys, no callables), internal backend resolution via the module map, and reactive failure of an unavailable backend. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review follow-up (Hashir): dropping the proactive get_tessdata() probe also dropped the only explicit availability check for the tesseract backend. pymupdf4llm's tesseract_api imports cleanly without Tesseract (it warns once and exec_ocr becomes a per-page no-op), so the ImportError guard in _resolve_ocr_function never fires for it and an explicitly requested tesseract run would silently proceed without OCR - quietly mis-scoring the benchmark. (rapidocr is not affected: its module import raises when rapidocr_onnxruntime is missing.) Restore the fail-fast at the agreed layer: _resolve_ocr_function reads the backend module's import-time TESSDATA marker (no extra subprocess probe, no callables or side effects in the declarative options) and raises ProviderConfigError when it is None. Config stays pure and serializable; the failure is still reactive - it only surfaces when the tesseract backend is actually requested. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_d5691145-f68e-4159-a913-572249137769
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.
Note
Companion to #1 - independent, no ordering requirement. Both PRs auto-merge cleanly in the provider file (the test file has a trivial both-append conflict; happy to rebase whichever lands second).
Summary
Shapes the PyMuPDF4LLM provider's OCR handling for eventual upstream (run-llama) submission. One commit, 3 files.
pymupdf.get_tessdata()probe. It was the only proactive engine probe in the tree, cost ~two subprocess spawns per request (~70 ms floor, up to ~350 ms/page observed), and itstesseract --list-langsoutput breaks UTF-8 subprocess readers on non-UTF-8 Windows locales. Tesseract now fails reactively at use time (ImportError/missing tessdata ->ProviderConfigError), matching how the rest of the tree treats OCR engines.ocr_backendstays a plain string; the engine callable is resolved internally at call time (module map) and passed straight toto_markdown(ocr_function=...)- it never enters the options dict that gets serialized into run metadata.use_ocr,force_ocr,ocr_dpi,ocr_languageare 1:1to_markdownkwargs) - mirroring the wrapped library beats mirroring other providers' vocabulary; noted in a code comment.Note on residual stderr noise
Removing the probe silences the provider-level noise, but the default pipeline still triggers the same
get_tessdata()subprocess inside pymupdf4llm itself (library defaultuse_ocr=True+ocr_function=None). That residual is library-internal (no ParseBench frame in the stack), out of scope here, and unchanged from the base branch.pymupdf4llm_markdown_no_ocrruns with completely clean stderr.