fix(document): swap the AGPL rasterizer for pypdfium2 and bound OCR per page (#13896)

[mrveiss]

Thinking Path

Review of #14510 blocked on one thing: it adds PyMuPDF, which is AGPL-3.0-or-commercial, to a product that ships Apache-2.0. AGPL's copyleft reaches network use, which is exactly how a backend ingest service is consumed, and this repo has already answered the same question one notch weaker — 0d67a956e removed a plain GPL-3.0 dependency "to align with Apache-2.0".

Owner decision: swap the library. This is that swap, targeted at issue-13896 so #14510 can merge.

It deliberately does not touch that branch. A locked worktree belongs to the session that wrote it, so this lands as its own branch to merge or cherry-pick rather than a push under an active session.

What Changed

pypdfium2 (Apache-2.0/BSD, bundles pdfium, needs no system package) replaces PyMuPDF. media/document/ocr.py was the only consumer, so the change is contained:

	before	after
open	fitz.open(stream=raw, filetype="pdf")	pypdfium2.PdfDocument(raw)
pages	document.page_count	len(document)
render	load_page(n).get_pixmap(dpi=dpi)	document[n].render(scale=dpi / 72)
to image	Image.frombytes("RGB", (w, h), samples)	.to_pil()

render(scale=) takes a multiple of 72 DPI, pdfium's unit, and yields a PIL image directly — so the explicit frombytes round-trip is deleted rather than reimplemented, and the PIL.Image import goes with it.

Also: the per-page timeout review flagged

pytesseract.image_to_string accepts a timeout= that was not being passed, so it was unbounded on a path that accepts arbitrary uploads. DEFAULT_MAX_OCR_PAGES bounds page count; nothing bounded per-page cost, and one page with a pathological MediaBox renders a very large bitmap however few pages were requested.

New AUTOBOT_DOCUMENT_OCR_PAGE_TIMEOUT knob, following the file's existing _positive_int_setting convention exactly, default 60s. The rendered image is now closed explicitly too.

Verification — read this part

tests/test_ocr_fallback_13896.py    12 passed, 6 skipped
flake8 / black / isort              exit 0

The 6 skips are the end-to-end tests, and they cover exactly the code I rewrote. They need pytesseract, pypdfium2 and the tesseract binary; none is installed in this checkout, and installing them here is not something I will do. So:

• Verified locally: the availability probe, the config knobs including the new timeout, the page-ceiling logic, the born-digital short-circuit, and that nothing else in the tree still references fitz/PyMuPDF.
• NOT verified locally: the render path itself — PdfDocument(raw), len(document), document[i], render(scale=...).to_pil(), close(). Those are correct against the pypdfium2 v4 API by reading, not by executing.

CI installs the toolchain (setup-python-suite already installs tesseract-ocr, and requirements-ci/document.txt now installs pypdfium2), so the end-to-end tests will run there and are the real gate. Treat a green CI run as the verification this PR body cannot provide.

That the tests skip rather than silently pass is the property test_ci_actually_has_the_ocr_toolchain exists to protect — it fails loudly if the runner ever loses the toolchain, so a skip here cannot masquerade as a pass there.

Model Used

Opus 5

Refs #13896, unblocks #14510

[mrveiss]

Review — APPROVE-WITH-NITS

The licence question was checked independently, and further down than the PR itself went — which is the right depth for this, because a permissive wrapper around a copyleft blob would solve nothing:

	claim	verified
PyMuPDF	AGPL-3.0-or-commercial	PyPI metadata: "Dual Licensed - GNU AFFERO GPL 3.0 or Artifex Commercial License"
pypdfium2 wrapper	Apache-2.0/BSD	PyPI: BSD-3-Clause, Apache-2.0; upstream README confirms
bundled pdfium binary	not checked by the PR	BSD-style (Google)
pdfium's own bundled deps	not checked at all	BUILD_LICENSES/ — freetype, icu, lcms, libjpeg_turbo, libopenjpeg, libpng, libtiff, zlib, abseil, fast_float: all permissive, no AGPL/GPL/LGPL in the default non-XFA/non-V8 build that pypdfium2>=4.30.0 installs
repo licence	Apache-2.0	LICENSE confirmed
precedent 0d67a956e	"removed a GPL-3.0 dep to align with Apache-2.0"	real — it removed a GPL-3.0 JS/TS subtree rather than a pip package, so a different kind of dependency, but the characterisation is accurate

So the copyleft genuinely does not leak, all the way down. Good call, and correctly motivated.

The API translation is faithful, checked against pypdfium2's own source rather than inferred:

• document[number - 1] → get_page(i), documented zero-based — same 0-index conversion as load_page(number - 1). No off-by-one.
• scale = dpi / 72 matches pypdfium2's own docstring verbatim.
• render() defaults to BGR without alpha; to_pil() reverses BGR→RGB internally, so the result is a plain RGB PIL.Image — identical to the old Image.frombytes("RGB", ...), tesseract-compatible.
• Error parity holds: both call sites already catch except Exception, not a PyMuPDF-specific type, so PdfiumError/IndexError are still caught.
• document.close() and image.close() both in finally.

The per-page timeout is wired correctly. image_to_string(image, timeout=...) matches pytesseract's signature; on timeout it kills the subprocess and raises RuntimeError, caught cleanly and degraded to "". DEFAULT_OCR_PAGE_TIMEOUT = 60 is a named constant read through SSOT with the right alias — no literal at the call site.

The swap is complete: repo-wide grep for fitz / PyMuPDF across py/txt/yml/toml/md returns only two prose comments explaining the change. No surviving import, no stale pin. requirements.txt and requirements-ci/document.txt agree, and neither library needs a system package, so there is no CI provisioning drift.

Nits

1. test_only_the_unreadable_pages_are_ocred asserts keys, not content. It is the one multi-page test that could catch an off-by-one, but it only checks set(result.pages) == {2} — and _run_ocr populates recovered[number] for every in-range page regardless of what came back, so a silent off-by-one rendering a different in-range page leaves that assertion unchanged. Add assert "RECEIPT" in result.pages[2].upper(). Given the swap you just did, that is the assertion most worth having.
2. A timed-out page and a genuinely blank page are indistinguishable in the result — both empty, both absent from ocr_pages. Pre-existing shape, but the new timeout makes it reachable on purpose for the first time. A distinct ocr_failed_pages field would separate them.
3. The intermediate PdfBitmap from render() is never explicitly closed before .to_pil(). Not a leak given pypdfium2's finalizers, just less eager than the close() hygiene you added elsewhere.

What this does not close for #14510, confirmed unchanged

Stating it here so the stacked merge is not read as clearing my block:

• OCR still runs synchronously on the event loop. _recover_scanned_pages is a plain def called unawaited from async def _process_document. The per-page timeout bounds each page at 60s, but the worst case is still max_ocr_pages() × timeout — 50 × 60s = 3000s — of blocked event loop. Needs asyncio.to_thread plus an overall wait_for.
• Rasterization is still unbounded by page size. render(scale=dpi / 72) has no get_size()/pixel-area cap, and the timeout wraps the later image_to_string, so it cannot bound an OOM during render() itself.
• _positive_int_setting still has a floor and no ceiling — the new timeout knob inherits that.
• The empty_page_numbers vs has_usable_text_layer divergence is unchanged.

Both of the first two remain reachable from an ordinary PDF upload. Your body and code comments scope themselves accurately and do not claim otherwise, which I appreciate.

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No hardcoded values detected that have SSOT config equivalents!

[mrveiss]

CI: one gate fixed, one under investigation

Validate PR template sections — mine, fixed. The gate requires a literal Thinking Path heading and I had written Why this exists. Renamed; the other three sections were already OK.

Unit & Integration Tests — a frontend test, and this PR is Python-only. Recording the evidence rather than waving it through as known flakiness, since that is exactly how a real regression gets missed:

failing test : src/views/__tests__/AdvancedControlView.test.ts
               "calls emergencyStop and shows a success toast once"
this PR       : touches NO frontend files (ocr.py, requirements, ssot_config, one docstring)
issue-13896   : touches NO frontend files either
#14510        : same check, SAME base branch → SUCCESS
#14541        : same check → SUCCESS

So neither this change nor the branch it stacks on can plausibly reach that test, and the identical check passes on the base. Re-running the failed job to distinguish flaky from deterministic — if it reproduces, it is a real defect on this merge state and gets root-caused before this goes anywhere.

Not merging until it is green either way.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!