Fix multipage PDF inference across parse providers - #108
SebasGarcia08 wants to merge 39 commits into
Conversation
Introduce a checkpoint-safe page adapter that renders every PDF page, invokes existing single-image provider implementations in document order, and combines normalized markdown and layout pages. Opt fourteen image-backed providers into the shared path while preserving their image-input behavior. Add focused tests for ordered page processing, serializable raw artifacts, layout page numbering, and single-image passthrough.
Inspect PDFs with pdf2image to determine the page count, then render exactly one page at a time using first_page/last_page bounds. Run each provider inference immediately in document order and close the rendered PIL image in a finally block, including converted image copies, so long documents retain at most one rendered page. Keep the existing checkpoint-safe multipage raw envelope and provider exception behavior. Report PDF inspection failures separately and identify the exact page for rendering failures or unexpected render counts. Add regression coverage proving render/inference ordering, prior-page image closure, checkpoint serialization, output ordering, and page-specific failure semantics. Verified with all 107 parse-provider tests, changed-file Ruff format/check, and isolated targeted mypy.
Centralize document image lifetime management in a bounded page iterator that inspects PDFs once, renders exactly one page per convert_from_path call, and closes both rendered and single-image inputs on success or failure. Migrate Amazon Nova, dots.ocr, Anthropic, Google, OpenAI, Tesseract, and Textract while preserving their provider-specific raw outputs, layout modes, usage accounting, retries, and OCR behavior. Validate multipage envelope versions and page counts, and cover real Nova PDF/image requests plus error cleanup and malformed artifacts.
Parameterize run and normalize hook checks across every image-backed provider using the shared multipage envelope. Add AST regressions that reject eager PDF conversion in the seven incremental providers and require first_page/last_page bounds in the shared rasterizer.
Close provider-opened PIL images in Gemma4, Nemotron Omni, and Qwen3.5 after reading dimensions. Make InfinityParser2 own and close loaded, converted, and derived crop images on success and failure paths, with focused ownership regression coverage for all four providers.
Propagate Dots OCR terminal failures after document-level retries and make Tesseract abort when any page fails instead of returning partial output. Strengthen provider-path tests to assert permanent failure propagation, transient document retry behavior, exhausted retry semantics, and page-image cleanup.
Preserve permanent and transient provider exceptions, classify timeout and client failures as retryable, and reject unexpected inference failures instead of returning error-shaped successful results.\n\nCover page-two permanent and transient failures through the real multipage adapter and assert no partial raw document escapes.
Replace eager KDL PDF rasterization with a context-managed page iterator that keeps at most one rendered page live. Close source and normalized images deterministically for successful, rendering-failure, and inference-failure paths while preserving permanent and transient provider classifications.\n\nAdd provider-level coverage for ordered multipage execution, bounded ownership, atomic page failures, render cleanup, and unchanged single-image semantics.
Introduce one authoritative manifest for all registered parse providers that rasterize PDFs locally. Derive adapter, direct-renderer, and KDL test matrices from it and structurally compare the inventory with provider registrations and rendering calls.\n\nExercise every adapter provider for ordered multipage handling, bounded image ownership, unchanged single-image behavior, and atomic permanent and transient page failures.
Apply Ruff formatting only to the newly changed KDL page-streaming methods while leaving the surrounding vendored module untouched.
Classify terminal HTTP failures, raise retry exhaustion with the original cause, and prevent empty stage content from becoming partial document output. Close every KDL page derivative across preprocessing, layout, crop, monochromatic skips, encoding, and concurrent recognition failures. Exercise the real page stage and HTTP retry boundary.
Raise retryable provider failures after Gemini exhausts its internal empty-response retry in image, layout, and PDF modes. Preserve classified exceptions so multipage inference aborts before returning diagnostic markdown or partial page payloads, with page-two regression coverage for image and layout paths.
Treat configured deep parsing as a required inference stage. Preserve classified provider failures, classify unexpected deep-stage errors, and abort rather than returning shallow content. Update ownership tests to assert failure propagation and crop/source cleanup.
Replace raster-call token detection with one exhaustive manifest that explicitly classifies every registered parse provider as locally page-rasterized or not. Resolve registration decorators through import aliases and require exact provider/module/class coverage so new registrations cannot bypass the multipage ownership guard by choosing another raster API.
Compare all changed Python files with origin/main using normalized Ruff and targeted mypy diagnostics, and reject formatter hunks that overlap branch-touched lines. Keep inherited vendored KDL debt measurable while preventing additions, and clean the KDL regions touched by the multipage fixes without reformatting the full vendored module.
Reject malformed layout responses and classified crop/preprocess failures instead of silently producing partial output. Materialize every source page, including empty pages, and preserve source page numbering through normalization.
Propagate malformed layout responses, retry only the failed billable page, and raise a terminal retry-exhausted error so the runner cannot replay completed pages in a second retry cycle.
Require list-shaped nonempty layout JSON, reject empty or diagnostic deep responses before replacing figure text, and classify invalid normalized output instead of converting it into empty success.
Replace decorator AST heuristics with exact comparison among the runtime registry, the authoritative parse-module load manifest, and the PDF classification inventory so aliases, keywords, star imports, and nested registrations are source-shape independent.
Resolve the baseline merge-base, archive the correct tree, validate Ruff/mypy/formatter status and output schemas, map formatter hunks to current lines, and compare diagnostics using source text. Add divergent-history, parsing, failure, and line-mapping tests.
Apply safe Python 3.12 annotation upgrades, explicit zip strictness, consolidated vendored imports, standard logging calls, and type narrowing so the merge-base static delta passes without masking KDL findings.
Accept a decoded empty layout list as a valid blank single-page result while keeping missing, malformed, diagnostic, and invalid-element payloads terminal. Preserve blank pages through multipage normalization and cover deep-parsing passthrough plus page identity.
Centralize transient and rate-limit retry ownership at the billable page boundary for adapter and direct raster providers, with permanent retry-exhaustion signaling that prevents document replay. Move KDL and Google stage retries under the page budget, preserve page ordering and aggregation, and cover successful page-2 recovery plus terminal exhaustion through the outer runner.
Wait for timed-out workers to terminate before resubmission, retain a usage-and-cost ledger for every physical page attempt, and retry KDL layout/recognition stages without replaying successful work. Normalize validated raw pages in one sorted order, emit explicit blank layout pages, and delete document-scoped Agentic Vision caches in finally. Add adversarial coverage for timeout overlap, billable failures, stage reuse, raw ordering, blank pages, and cache cleanup.
Preserve worker results that complete during timeout cancellation so sync and async runners never replay a completed billable document or contradict saved artifacts. Aggregate runner-owned failed usage and model-derived cost into ordered API attempt ledgers, persist every Textract page retry, and omit incomplete totals instead of undercounting them. Bound Textract resize memory by closing each superseded candidate before allocating the next and retain caller ownership of source images. Add adversarial coverage for late timeout completion, Google/OpenAI/Anthropic native-file retries, Textract transient retries, and progressive resize cleanup.
Propagate external async cancellation instead of converting it into a retryable timeout outcome, preventing a new document attempt from starting while the shielded worker may still be running. Correct process-result annotations to include structured provider errors and add a regression test for caller cancellation during timeout draining.
Preserve complete physical-attempt ledgers through terminal page failures, treat unavailable usage as unknown instead of zero-cost, and omit misleading aggregate token/cost totals when any attempt is incomplete. Also harden async cancellation classification, KDL canonical page views, Textract blank-page/transport behavior, and persisted retry diagnostics with adversarial coverage.
Retain complete terminal ledgers across runner, Agentic Vision, and KDL retry owners, including per-stage KDL usage and optional endpoint cost accounting. Treat missing usage and unknown model pricing as unknown instead of exact zero, preserving successful inference and known token totals while omitting unsupported cost fields. Drain externally cancelled workers, validate canonical text-page records, and cover terminal, cancellation, cache-cleanup, blank-page, ordering, and retry-accounting edge cases with adversarial tests.
Record permanent as well as retryable physical calls in the shared ledger and carry completed-page attempts into terminal payloads. Validate KDL and Dots layout responses inside their retry owners, preserve compatible API usage, and expose exact totals only when all attempts report them. Harden cancellation propagation, late-outcome handling, and token-cost accounting across file, page, cache, and Textract paths with adversarial terminal tests.
There was a problem hiding this comment.
Thanks for the contribution @SebasGarcia08
7 findings on the specific lines below — two correctness issues in the new static-quality gate, one behavior regression in the vision providers, one test that masks the gate bug, plus reuse/efficiency/altitude notes.
| return [] | ||
| result = _require_command( | ||
| _run([ruff, "format", "--diff", "--quiet", *files], cwd=root), | ||
| description="Ruff formatter", |
There was a problem hiding this comment.
Bug: this call crashes the whole gate on any formatting diff. _require_command here uses the default allowed_returncodes=(0,), but ruff format --diff exits 1 whenever a file would be reformatted (verified: exit 1 with a diff on stdout, exit 0 when clean). So on any changed .py that has a formatting difference — new, or pre-existing on a line the branch never touched — this raises StaticQualityError → parser.error → the script exits 2 with an argparse usage error. The branch-touched-line format_failures filter below is never reached, so pre-existing baseline format debt in a touched file blocks the PR — exactly the "conflating existing baseline debt" this script is meant to avoid.
Fix: allowed_returncodes=(0, 1), matching _ruff_diagnostics/_mypy_diagnostics.
| if result.returncode not in allowed_returncodes: | ||
| detail = result.stderr.strip() or result.stdout.strip() or "no output" | ||
| raise StaticQualityError(f"{description} failed with exit code {result.returncode}: {detail}") | ||
| if result.stderr.strip(): |
There was a problem hiding this comment.
Robustness: any subprocess stderr aborts the fail-closed gate. This raises even on returncode 0 when a tool writes to stderr. ruff can emit config/deprecation warnings, and mypy/git emit advisory notes, to stderr on otherwise-successful runs. A single benign stderr line then makes every PR fail with an opaque error until the tool is silenced. Consider only failing on stderr when the returncode is also unexpected, or filtering known-benign warnings.
| raise ProviderTransientError(f"OpenAI returned no message content for {context}") | ||
| content = message.content | ||
| if not isinstance(content, str) or not content.strip(): | ||
| raise ProviderTransientError(f"OpenAI returned empty message content for {context}") |
There was a problem hiding this comment.
Regression: a genuinely blank page now fails the whole document in plain markdown modes. In mode="image"/"file" an empty response is raised as ProviderTransientError → run_page_with_retries retries 3× on the same blank image → ProviderRetryExhaustedError → the entire document fails with no output for any page. Only the layout modes were given a [] blank-page sentinel; plain markdown modes have no valid representation for a blank page. Previously (content or "") yielded empty markdown and the doc succeeded. Same pattern in anthropic.py:251 (_extract_text) and google.py:558 (_parse_image). This contradicts the PR's stated goal that a genuinely blank page is valid content.
| num_pages = len(images) | ||
| ] | ||
| num_pages = 1 | ||
| else: |
There was a problem hiding this comment.
Reuse/altitude: this per-page mode-branch loop plus the token/cost aggregation below (openai.py:704-737) is duplicated nearly verbatim across google.py, anthropic.py, and amazon_nova.py (e.g. the summary block ≈ amazon_nova.py:431-465). The shared _multipage_image adapter only serves single-image providers, so the four layout-capable providers each re-implement hundreds of lines — the blank-page fix above, for instance, has to be made in four places. A shared helper for the page-loop scaffold + usage_summary would remove the drift risk.
| -old | ||
| +new | ||
| """ | ||
| monkeypatch.setattr(checker, "_run", lambda *args, **kwargs: _result(0, formatter_diff)) |
There was a problem hiding this comment.
Test masks the format-gate bug. This mocks _run to return _result(0, formatter_diff) — returncode 0 with a non-empty diff — which real ruff format --diff never produces (it returns 1 when a diff exists). So the only happy-path formatter test validates against a false tool contract; in production _require_command rejects the exit-1 and _format_hunks raises instead of returning hunks. test_formatter_failures_are_terminal only covers returncode 2, never the real returncode-1-with-valid-diff case. Recommend a test with _result(1, formatter_diff) (which currently fails).
|
|
||
| for page_number in range(1, page_count + 1): | ||
| try: | ||
| rendered = convert_from_path( |
There was a problem hiding this comment.
Efficiency: O(N²) PDF parsing. Calling convert_from_path once per page (with first_page=last_page=N) spawns a fresh pdftoppm for every page, each of which re-parses the entire PDF to reach page N — N process spawns and O(N²) parsing for an N-page doc (up to KDL's 400-page ceiling). Bounded memory (the goal here) can be kept with a single convert_from_path(..., output_folder=tmp, paths_only=True) that lazily opens page files, or PyMuPDF one-page rendering as KdlFrontierNanoProvider._iter_page_images already does — without the repeated full-document parse.
| break # Success (or handled provider error) - exit retry loop | ||
| except TimeoutError: | ||
| await self._cancel_inflight_and_drain_async(example_id, future) | ||
| drained_outcome = await self._cancel_inflight_and_drain_async(example_id, future) |
There was a problem hiding this comment.
Altitude: timeout path still replays the whole multipage document. On a per-file timeout the worker is cancelled and _process_document is resubmitted, re-running inference for all pages (up to timeout_retries times) — the exact "replay billable work" the PR's Why section says it prevents. The page-level retry ownership added elsewhere doesn't extend here, and there's no on-disk page checkpoint to resume from (run_pdf_pages uses an ephemeral TemporaryDirectory; raw output is saved only on full success). Since this PR makes docs process every page, timeouts are more likely and each replays completed billable pages.
2 participants
What
Fix ParseBench inference so image-backed PDF providers process every page in order instead of silently using only the first page.
This also fixes the inputs consumed by text evaluators: normalized Markdown now includes every source page in order, genuine blank pages remain explicit empty pages, and empty, malformed, or diagnostic provider responses are rejected instead of being treated as successful text. The branch also adds page-level retry and checkpoint handling, deterministic image ownership, provider-specific DPI selection, atomic document failure semantics, exhaustive provider classification checks, and a fail-closed static-quality delta gate.
Why
Text Content and other text-based evaluations need complete, page-aligned Markdown. First-page-only output, silently partial documents, or diagnostic strings represented as successful output can produce misleading evaluation scores. At the same time, a genuinely blank page is valid document content and must not be confused with a missing or malformed response.
Retry ownership and resource-lifecycle inconsistencies could also replay billable work, leak page images, or return partially parsed documents.
How
How to reproduce/test
From the repository root:
Expected results: