From 2abb287c3123e469b097f88776e3189f983460c0 Mon Sep 17 00:00:00 2001 From: "youchang.kim" Date: Sun, 5 Jul 2026 11:46:28 +0900 Subject: [PATCH 1/2] Shape PyMuPDF4LLM OCR config for upstream: drop proactive probe, resolve 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 --- docs/pipelines.md | 5 + .../inference/providers/parse/pymupdf4llm.py | 76 ++++++++--- .../providers/parse/test_pymupdf4llm.py | 128 +++++++++++++++++- 3 files changed, 186 insertions(+), 23 deletions(-) diff --git a/docs/pipelines.md b/docs/pipelines.md index e50be90..1269883 100644 --- a/docs/pipelines.md +++ b/docs/pipelines.md @@ -259,6 +259,11 @@ These run entirely locally with no external dependencies. | `pypdf_baseline` | PyPDF text extraction | None | | `pymupdf_text` | PyMuPDF text extraction | None | | `pymupdf_html` | PyMuPDF HTML extraction | None | +| `pymupdf4llm_markdown` | PyMuPDF4LLM layout Markdown, automatic OCR engine selection | `pymupdf4llm` | +| `pymupdf4llm_markdown_tesseract` | PyMuPDF4LLM Markdown, Tesseract OCR backend | `pymupdf4llm`, `tesseract` | +| `pymupdf4llm_markdown_rapidocr` | PyMuPDF4LLM Markdown, RapidOCR backend | `pymupdf4llm`, `rapidocr-onnxruntime` | +| `pymupdf4llm_markdown_no_ocr` | PyMuPDF4LLM Markdown, OCR disabled | `pymupdf4llm` | +| `pymupdf4llm_markdown_150dpi` | PyMuPDF4LLM Markdown, OCR at 150 DPI | `pymupdf4llm` | | `tesseract_eng` | Tesseract OCR (English) | `tesseract` installed | | `tesseract_fast` | Tesseract OCR (fast) | `tesseract` installed | | `tesseract_high_quality` | Tesseract OCR (high quality) | `tesseract` installed | diff --git a/src/parse_bench/inference/providers/parse/pymupdf4llm.py b/src/parse_bench/inference/providers/parse/pymupdf4llm.py index 70cd85b..cada5c0 100644 --- a/src/parse_bench/inference/providers/parse/pymupdf4llm.py +++ b/src/parse_bench/inference/providers/parse/pymupdf4llm.py @@ -3,6 +3,7 @@ import importlib import logging import math +from collections.abc import Callable from datetime import datetime from pathlib import Path from typing import Any @@ -49,6 +50,16 @@ } +# `ocr_backend` is a ParseBench-level config key (not a pymupdf4llm.to_markdown +# kwarg): it names which bundled OCR engine should back `ocr_function`. The +# engine callable is resolved from this map internally at call time (see +# `_resolve_ocr_function`) so it never enters a serialized options/config dict. +_OCR_BACKEND_MODULES = { + "rapidocr": "pymupdf4llm.ocr.rapidocr_api", + "tesseract": "pymupdf4llm.ocr.tesseract_api", +} + + @register_provider("pymupdf4llm") class PyMuPDF4LLMProvider(Provider): """Provider for PyMuPDF4LLM (markdown). AGPL — runtime dep only.""" @@ -56,7 +67,11 @@ class PyMuPDF4LLMProvider(Provider): def __init__(self, provider_name: str, base_config: dict[str, Any] | None = None): super().__init__(provider_name, base_config) - def _markdown_options(self, pymupdf: Any) -> dict[str, Any]: + def _markdown_options(self) -> dict[str, Any]: + # `use_ocr`, `force_ocr`, `ocr_dpi`, and `ocr_language` intentionally + # mirror pymupdf4llm.to_markdown's kwargs 1:1 and are forwarded verbatim + # below; keep these config keys aligned with the library API rather than + # renaming them to match other providers. options: dict[str, Any] = { "page_chunks": True, "show_progress": False, @@ -92,37 +107,47 @@ def _markdown_options(self, pymupdf: Any) -> dict[str, Any]: raw_backend = self.base_config.get("ocr_backend") if raw_backend is None: return options + # `ocr_backend` selects a bundled pymupdf4llm OCR engine. It is a + # ParseBench-level string, not a to_markdown kwarg: validate it here so a + # bad value fails fast, but resolve the engine callable lazily at call + # time (see `_resolve_ocr_function`) so it never enters this options + # dict. `auto` defers to pymupdf4llm's own engine selection. if not isinstance(raw_backend, str): raise ProviderConfigError("PyMuPDF4LLM 'ocr_backend' must be a string") - backend = raw_backend.strip().lower() - if backend == "auto": - return options - - backend_modules = { - "rapidocr": "pymupdf4llm.ocr.rapidocr_api", - "tesseract": "pymupdf4llm.ocr.tesseract_api", - } - module_name = backend_modules.get(backend) - if module_name is None: - supported = ", ".join(["auto", *backend_modules]) + if backend != "auto" and backend not in _OCR_BACKEND_MODULES: + supported = ", ".join(["auto", *_OCR_BACKEND_MODULES]) raise ProviderConfigError( f"Unsupported PyMuPDF4LLM OCR backend '{raw_backend}'. Supported backends: {supported}" ) + return options - if backend == "tesseract" and pymupdf.get_tessdata() is None: - raise ProviderConfigError("PyMuPDF4LLM Tesseract backend requires Tesseract language data") - + def _resolve_ocr_function(self) -> Callable[..., Any] | None: + """Resolve the configured OCR engine callable immediately before OCR. + + `ocr_backend` is validated in `_markdown_options`; here the selected + engine module is imported and its ``exec_ocr`` returned so it can be + handed to ``to_markdown(ocr_function=...)`` as a direct call-time kwarg + -- never stored in the serialized options/config dict. An absent or + ``auto`` backend returns ``None`` so pymupdf4llm performs its own engine + selection. Engine availability is discovered reactively: an unavailable + backend raises ProviderConfigError only when that backend is actually + requested, rather than probing eagerly for every request. + """ + raw_backend = self.base_config.get("ocr_backend") + if not isinstance(raw_backend, str): + return None + module_name = _OCR_BACKEND_MODULES.get(raw_backend.strip().lower()) + if module_name is None: + return None try: ocr_module = importlib.import_module(module_name) except (ImportError, RuntimeError) as e: - raise ProviderConfigError(f"PyMuPDF4LLM OCR backend '{backend}' is unavailable: {e}") from e - + raise ProviderConfigError(f"PyMuPDF4LLM OCR backend '{raw_backend}' is unavailable: {e}") from e ocr_function = getattr(ocr_module, "exec_ocr", None) if not callable(ocr_function): - raise ProviderConfigError(f"PyMuPDF4LLM OCR backend '{backend}' does not expose exec_ocr") - options["ocr_function"] = ocr_function - return options + raise ProviderConfigError(f"PyMuPDF4LLM OCR backend '{raw_backend}' does not expose exec_ocr") + return ocr_function def _extract(self, pdf_path: str) -> dict[str, Any]: try: @@ -132,8 +157,15 @@ def _extract(self, pdf_path: str) -> dict[str, Any]: raise ProviderConfigError("pymupdf4llm not installed. Run: pip install pymupdf4llm") from e try: - markdown_options = self._markdown_options(pymupdf) - page_chunks = pymupdf4llm.to_markdown(pdf_path, **markdown_options) + markdown_options = self._markdown_options() + # Resolve the OCR engine callable here, immediately before the call, + # and pass it as a direct kwarg so the callable never lives in the + # declarative options dict (ocr_backend stays a plain string key). + ocr_function = self._resolve_ocr_function() + if ocr_function is None: + page_chunks = pymupdf4llm.to_markdown(pdf_path, **markdown_options) + else: + page_chunks = pymupdf4llm.to_markdown(pdf_path, ocr_function=ocr_function, **markdown_options) with pymupdf.open(pdf_path) as document: page_dimensions = [(float(page.rect.width), float(page.rect.height)) for page in document] except ProviderConfigError: diff --git a/tests/parse_bench/inference/providers/parse/test_pymupdf4llm.py b/tests/parse_bench/inference/providers/parse/test_pymupdf4llm.py index 2973c91..93f68a8 100644 --- a/tests/parse_bench/inference/providers/parse/test_pymupdf4llm.py +++ b/tests/parse_bench/inference/providers/parse/test_pymupdf4llm.py @@ -1,6 +1,13 @@ -"""Tests for PyMuPDF4LLM layout normalization helpers.""" +"""Tests for PyMuPDF4LLM provider helpers.""" +import types + +import pytest + +import parse_bench.inference.providers.parse.pymupdf4llm as pymupdf4llm_module +from parse_bench.inference.providers.base import ProviderConfigError from parse_bench.inference.providers.parse.pymupdf4llm import ( + _OCR_BACKEND_MODULES, _PYMUPDF_CLASS_TO_CANONICAL, PyMuPDF4LLMProvider, ) @@ -29,3 +36,122 @@ def test_build_layout_page_maps_all_pymupdf_classes() -> None: assert page is not None assert [item.layout_segments[0].label for item in page.items] == list(_PYMUPDF_CLASS_TO_CANONICAL.values()) + + +def test_markdown_options_mirror_library_kwargs() -> None: + """Config keys mirror pymupdf4llm.to_markdown and are forwarded verbatim.""" + provider = PyMuPDF4LLMProvider( + "pymupdf4llm", + {"use_ocr": True, "force_ocr": True, "ocr_dpi": 150, "ocr_language": "deu"}, + ) + options = provider._markdown_options() + + assert options == { + "page_chunks": True, + "show_progress": False, + "use_ocr": True, + "force_ocr": True, + "ocr_dpi": 150, + "ocr_language": "deu", + } + + +def test_markdown_options_are_declarative_no_callable_injected() -> None: + """The options dict must stay serializable: no ocr_function/ocr_backend keys.""" + provider = PyMuPDF4LLMProvider("pymupdf4llm", {"ocr_backend": "tesseract"}) + options = provider._markdown_options() + + assert "ocr_function" not in options + assert "ocr_backend" not in options + assert all(not callable(value) for value in options.values()) + + +def test_markdown_options_does_not_probe_tessdata(monkeypatch: pytest.MonkeyPatch) -> None: + """Regression: the proactive pymupdf.get_tessdata() probe is gone. + + Selecting the tesseract backend must not eagerly probe the OCR engine + (the probe spawned a subprocess and cost ~350 ms/page). Building the + declarative options must succeed even if get_tessdata would raise. + """ + import pymupdf + + def _boom(*args: object, **kwargs: object) -> object: + raise AssertionError("get_tessdata must not be probed while building options") + + monkeypatch.setattr(pymupdf, "get_tessdata", _boom) + + provider = PyMuPDF4LLMProvider("pymupdf4llm", {"ocr_backend": "tesseract"}) + # Does not raise -> the probe was not invoked. + assert provider._markdown_options() == {"page_chunks": True, "show_progress": False} + + +@pytest.mark.parametrize("bad_backend", [123, ["tesseract"], object()]) +def test_markdown_options_rejects_non_string_backend(bad_backend: object) -> None: + provider = PyMuPDF4LLMProvider("pymupdf4llm", {"ocr_backend": bad_backend}) + with pytest.raises(ProviderConfigError, match="must be a string"): + provider._markdown_options() + + +def test_markdown_options_rejects_unsupported_backend() -> None: + provider = PyMuPDF4LLMProvider("pymupdf4llm", {"ocr_backend": "nonesuch"}) + with pytest.raises(ProviderConfigError, match="Unsupported PyMuPDF4LLM OCR backend"): + provider._markdown_options() + + +@pytest.mark.parametrize("config", [{}, {"ocr_backend": "auto"}, {"ocr_backend": "AUTO"}]) +def test_resolve_ocr_function_defers_to_library(config: dict[str, object]) -> None: + """Absent or 'auto' backend returns None so pymupdf4llm selects the engine.""" + provider = PyMuPDF4LLMProvider("pymupdf4llm", config) + assert provider._resolve_ocr_function() is None + + +@pytest.mark.parametrize("backend", sorted(_OCR_BACKEND_MODULES)) +def test_resolve_ocr_function_resolves_backend_internally(backend: str, monkeypatch: pytest.MonkeyPatch) -> None: + """The engine callable is resolved from the module map at call time.""" + imported: list[str] = [] + + def _sentinel_exec_ocr(*args: object, **kwargs: object) -> None: + return None + + fake_module = types.SimpleNamespace(exec_ocr=_sentinel_exec_ocr) + + def _fake_import(name: str) -> object: + imported.append(name) + return fake_module + + monkeypatch.setattr(pymupdf4llm_module.importlib, "import_module", _fake_import) + + provider = PyMuPDF4LLMProvider("pymupdf4llm", {"ocr_backend": backend}) + resolved = provider._resolve_ocr_function() + + assert resolved is _sentinel_exec_ocr + assert imported == [_OCR_BACKEND_MODULES[backend]] + + +def test_resolve_ocr_function_unavailable_backend_is_reactive() -> None: + """An unavailable engine fails only at resolve time, not at config time. + + rapidocr_onnxruntime is not installed in the test environment, so importing + the backend raises ImportError. Building the declarative options must still + succeed; the failure surfaces reactively from _resolve_ocr_function. + """ + provider = PyMuPDF4LLMProvider("pymupdf4llm", {"ocr_backend": "rapidocr"}) + + # Config/options stage stays clean. + assert provider._markdown_options() == {"page_chunks": True, "show_progress": False} + + # Resolution stage raises reactively. + with pytest.raises(ProviderConfigError, match="rapidocr.*unavailable"): + provider._resolve_ocr_function() + + +def test_resolve_ocr_function_missing_exec_ocr(monkeypatch: pytest.MonkeyPatch) -> None: + """A backend module without exec_ocr is a config error.""" + monkeypatch.setattr( + pymupdf4llm_module.importlib, + "import_module", + lambda name: types.SimpleNamespace(), + ) + provider = PyMuPDF4LLMProvider("pymupdf4llm", {"ocr_backend": "tesseract"}) + with pytest.raises(ProviderConfigError, match="does not expose exec_ocr"): + provider._resolve_ocr_function() From 7b13c9dea7ab03017159df955404a88e65e82728 Mon Sep 17 00:00:00 2001 From: "youchang.kim" Date: Wed, 8 Jul 2026 14:57:00 +0900 Subject: [PATCH 2/2] Fail loudly when the tesseract backend is requested but unavailable 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 Claude-Session: https://claude.ai/code/session_d5691145-f68e-4159-a913-572249137769 --- .../inference/providers/parse/pymupdf4llm.py | 11 +++++++ .../providers/parse/test_pymupdf4llm.py | 29 +++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/src/parse_bench/inference/providers/parse/pymupdf4llm.py b/src/parse_bench/inference/providers/parse/pymupdf4llm.py index cada5c0..9ef165b 100644 --- a/src/parse_bench/inference/providers/parse/pymupdf4llm.py +++ b/src/parse_bench/inference/providers/parse/pymupdf4llm.py @@ -147,6 +147,17 @@ def _resolve_ocr_function(self) -> Callable[..., Any] | None: ocr_function = getattr(ocr_module, "exec_ocr", None) if not callable(ocr_function): raise ProviderConfigError(f"PyMuPDF4LLM OCR backend '{raw_backend}' does not expose exec_ocr") + # The tesseract backend module imports cleanly even when Tesseract is + # missing: it warns once and its exec_ocr becomes a per-page no-op + # (pymupdf4llm/ocr/tesseract_api.py), so the import guard above never + # fires for it. A benchmark run must not silently score without the OCR + # the user asked for, so read the module's import-time availability + # marker (no extra subprocess probe) and fail loudly instead. + if getattr(ocr_module, "TESSDATA", True) is None: + raise ProviderConfigError( + f"PyMuPDF4LLM OCR backend '{raw_backend}' is unavailable: " + "Tesseract language data was not found (pymupdf.get_tessdata() returned None)" + ) return ocr_function def _extract(self, pdf_path: str) -> dict[str, Any]: diff --git a/tests/parse_bench/inference/providers/parse/test_pymupdf4llm.py b/tests/parse_bench/inference/providers/parse/test_pymupdf4llm.py index 93f68a8..b1e66f7 100644 --- a/tests/parse_bench/inference/providers/parse/test_pymupdf4llm.py +++ b/tests/parse_bench/inference/providers/parse/test_pymupdf4llm.py @@ -155,3 +155,32 @@ def test_resolve_ocr_function_missing_exec_ocr(monkeypatch: pytest.MonkeyPatch) provider = PyMuPDF4LLMProvider("pymupdf4llm", {"ocr_backend": "tesseract"}) with pytest.raises(ProviderConfigError, match="does not expose exec_ocr"): provider._resolve_ocr_function() + + +def test_resolve_ocr_function_tesseract_without_tessdata_raises(monkeypatch: pytest.MonkeyPatch) -> None: + """An explicitly requested tesseract backend must not silently skip OCR. + + pymupdf4llm's tesseract_api imports cleanly when Tesseract is missing (it + warns and its exec_ocr becomes a per-page no-op), so the ImportError guard + never fires. The provider must read the module's TESSDATA marker and raise + instead of letting the run quietly score without OCR. + """ + fake_module = types.SimpleNamespace(exec_ocr=lambda *a, **k: None, TESSDATA=None) + monkeypatch.setattr(pymupdf4llm_module.importlib, "import_module", lambda name: fake_module) + + provider = PyMuPDF4LLMProvider("pymupdf4llm", {"ocr_backend": "tesseract"}) + with pytest.raises(ProviderConfigError, match="Tesseract language data"): + provider._resolve_ocr_function() + + +def test_resolve_ocr_function_tesseract_with_tessdata_resolves(monkeypatch: pytest.MonkeyPatch) -> None: + """With Tesseract available (TESSDATA set), resolution succeeds.""" + + def _sentinel_exec_ocr(*args: object, **kwargs: object) -> None: + return None + + fake_module = types.SimpleNamespace(exec_ocr=_sentinel_exec_ocr, TESSDATA="/usr/share/tessdata") + monkeypatch.setattr(pymupdf4llm_module.importlib, "import_module", lambda name: fake_module) + + provider = PyMuPDF4LLMProvider("pymupdf4llm", {"ocr_backend": "tesseract"}) + assert provider._resolve_ocr_function() is _sentinel_exec_ocr