feat(parser): PDF OCR fallback + font-encoding recovery (#246)

[thewrz]

Why

Completes PDF spec ingest (#65, slice 3/3): scanned/image PDFs and font-corrupted text layers. After #245
(text-layer PDFs), a scanned PDF dropped into the demo's dropzone only emitted a "needs OCR" deferral — this makes
it actually OCR into the same AST + coordination panels, and repairs mojibake-corrupted text layers.

What

• OCR fallback (src/parser/pdf/ocr.ts): when feat(parser): PDF text-layer adapter — extract + normalize → text inference (slice 2/3 of #65) #245's detectPdfOcrNeed flags a scanned/mixed PDF,
  rasterize the affected pages (unpdf renderPageAsImage + @napi-rs/canvas) → tesseract.js → splice OCR text
  back per page → existing feat(parser): plaintext spec ingest — hierarchy inference from indent + numbering patterns #64 hierarchy inference. mixed keeps good text-layer pages and OCRs only the empty
  ones. The recognizer + renderer are dependency-injected.
• Font-encoding recovery (src/parser/pdf/font-encoding.ts): detects mojibake (char-frequency + chardet
  fingerprint), remaps via iconv-lite. Decodes items in coherent adjacent groups and rejects any remap that
  introduces a new �, so valid non-ASCII (70°F, ±) is never destroyed and sequences split across PDF items
  aren't corrupted. Genuinely unrecoverable corruption is flagged, never silently passed.
• Warnings: pdf-ocr-applied, pdf-ocr-low-confidence, pdf-ocr-unusable, pdf-font-encoding-remapped,
  pdf-font-encoding-unrecoverable.
• Graceful degradation: primary → pdfjs fallback → OCR / encoding remap → partial result with warnings[] →
  typed hard error. Never a silent empty/garbled parse.
• Deps per ADR-034 §3: tesseract.js 7 (Apache-2.0, WASM, offline) + @napi-rs/canvas (MIT, prebuilt). pnpm audit clean — 0 new advisories vs main. Model loaded from a gitignored cache (NOT vendored in git);
  tesseract's build script left unapproved.

Testing

• Build (tsc) clean
• Unit 1131 (OCR + font-encoding exercised via injected fakes — no model/network needed in CI)
• Integration 693 (orchestrator-run, isolated Postgres)
• Real-OCR end-to-end (orchestrator-run, SPECR_OCR_E2E=1): an image-only PDF → genuine tesseract model
  download + recognize → recovers PART 1 - GENERAL + pdf-ocr-applied. CI skips it (no model); skipIf-gated.
• Lint (eslint + tsc + prettier) + redocly lint openapi.yaml valid (9 pre-existing warnings)
• Manual: drop a scanned PDF into the demo dropzone → OCR'd into the tree (vs. the prior "needs OCR" deferral)
• CI green (runs when moved out of draft)

Review notes

• Implemented autonomously by Codex (gpt-5.5 xhigh) in an isolated worktree (TDD), under Claude Opus 4.8
  orchestration: dependency vetting + install, orchestrator-run integration + the real-OCR e2e, and an adversarial
  Codex review gate that caught 2 P2 font-encoding corruption bugs (valid non-ASCII destroyed; split-item
  mojibake) — both fixed here.
• The eng.traineddata model downloads on first OCR to a gitignored cache (offline after first fetch). For
  air-gapped deploys, pre-seed the cache or set OCR_LANG_PATH.

🤖 Co-authored by Codex (gpt-5.5 xhigh — implementation) under Claude Opus 4.8 (orchestration + review + deps).
Closes #246.

Summary by CodeRabbit

• New Features

  • Added improved PDF text extraction with OCR support and font-encoding recovery for harder-to-read documents.
  • Added new OCR-related configuration options to better control confidence thresholds, language files, cache location, and render scale.

• Bug Fixes

  • Replaced the generic “needs OCR” warning with more specific PDF warning types, including applied OCR, low confidence, unusable OCR output, and font-encoding issues.
  • Updated PDF parsing behavior to better preserve text quality and report more actionable warnings.

[coderabbitai]

Warning

Review limit reached

@thewrz, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 11 minutes and 31 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e64b9d2f-f878-4e75-a548-f5eca807a665

📥 Commits

Reviewing files that changed from the base of the PR and between adb9ec6 and 4dd3b9e.

📒 Files selected for processing (6)

• .env.example
• src/lib/env.ts
• src/parser/pdf/font-encoding.test.ts
• src/parser/pdf/font-encoding.ts
• src/parser/pdf/index.ts
• src/parser/pdf/ocr.ts

📝 Walkthrough

Walkthrough

The PR adds PDF OCR fallback and font-encoding recovery, expands ParseWarning codes and suggestions, wires new OCR dependencies and config, and updates parser and API tests for the new warning flow.

Changes

PDF OCR fallback and font-encoding recovery

Layer / File(s)	Summary
Warning contract updates openapi.yaml, src/ast/schemas.ts, src/ast/types.ts, src/parser/text/index.ts	The ParseWarning type surfaces and warning suggestion map now include the new PDF OCR and font-encoding codes.
OCR runtime config package.json, pnpm-workspace.yaml, src/lib/env.ts, src/lib/parse-worker.ts	The runtime adds OCR dependencies, blocks the Tesseract build script, extends OCR environment config, and forwards those settings into the worker parse call.
Parser dispatch and option mapping src/parser/index.ts	Top-level parse dispatch splits format handling into helpers and maps the OCR-related parse options into PDF parse options.
PDF OCR module src/parser/pdf/ocr.ts, src/parser/pdf/ocr.integration.test.ts	A PDF OCR module renders selected pages to images, runs Tesseract with configurable worker and render options, and is covered by an image-only PDF integration test.
Font-encoding recovery src/parser/pdf/font-encoding.ts, src/parser/pdf/font-encoding.test.ts	PDF text items are scored for garbled encodings, remapped when legacy decoding improves the text, and covered by recovery tests.
PDF parse flow and warnings src/parser/pdf/index.ts, src/parser/pdf/index.test.ts, src/api/parse.integration.test.ts	parsePdf now runs font-encoding recovery before OCR, applies OCR to selected pages, emits applied/low-confidence/unusable OCR warnings, merges them with extraction warnings, and the unit/API tests assert the new warning types and control flow.

Sequence Diagram(s)

sequenceDiagram
  participant parse
  participant parsePdf
  participant recoverPdfFontEncoding
  participant applyOcrIfNeeded
  participant recognizePdfPages
  parse->>parsePdf: parse(buffer, options)
  parsePdf->>recoverPdfFontEncoding: recoverPdfFontEncoding(pages)
  parsePdf->>applyOcrIfNeeded: applyOcrIfNeeded(need, pages, ocr)
  applyOcrIfNeeded->>recognizePdfPages: recognizePdfPages(buffer, pageNumbers, ocr)
  recognizePdfPages-->>applyOcrIfNeeded: OCR text and confidence
  applyOcrIfNeeded-->>parsePdf: updated pages and OCR warnings
  parsePdf-->>parse: ParseResult with merged warnings

Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related issues

• wrzonance/SpecR issue 65: The PR adds the OCR fallback and font-encoding recovery path described in the PDF ingest slice.

Possibly related PRs

• wrzonance/SpecR#113: Both PRs change the shared ParseWarning enum/type definitions in src/ast/schemas.ts and src/ast/types.ts.
• wrzonance/SpecR#287: This PR extends the same PDF warning and parsing flow introduced there with OCR fallback and font-encoding recovery.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 1.32% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and accurately summarizes the main change: PDF OCR fallback plus font-encoding recovery.
Linked Issues check	✅ Passed	The changes implement OCR fallback, font-encoding remapping, warnings, config, and tests, matching the linked issue scope.
Out of Scope Changes check	✅ Passed	The added dependencies, schema updates, config, and tests all support the OCR and font-recovery feature set; no unrelated changes stand out.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/issue-246

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/api/parse.integration.test.ts`:
- Around line 230-243: The regression test for blank PDF handling is currently
gated behind the OCR E2E flag, so the default suite no longer covers the crash
symptom. Move the `completes no-text PDF with an OCR warning instead of
crashing` case in `parse.integration.test.ts` back into the always-on
API-boundary suite, and make it deterministic by stubbing OCR or injecting a
fake recognizer instead of relying on `OCR_E2E_ENABLED`. Keep the real OCR
end-to-end coverage opt-in, but ensure this test remains unskipped in normal CI
and continues asserting the warning-based completion path through `postPdf` and
`blankPdf`.

In `@src/lib/env.ts`:
- Around line 9-12: The OCR_RENDER_SCALE schema in env validation is too
permissive because positive() allows unbounded values, which can later blow up
rasterization. Update the Zod definition in env validation to enforce a sane
maximum for OCR_RENDER_SCALE, keeping the existing default and failing fast on
invalid config so the process exits during boot. Use the OCR_RENDER_SCALE field
in src/lib/env.ts as the place to tighten the bound.

In `@src/parser/pdf/font-encoding.ts`:
- Around line 192-214: The aggregated pdf-font-encoding-remapped warning in
recoverPdfFontEncoding is using a single remapDecision for all recovered pages,
so the hint can become inaccurate when pages recover differently. Update
recoverPdfFontEncoding to keep the decision associated with each remapped page
(or avoid embedding a per-page decision in the aggregated warning), and make
sure the recoveryLineHint call reflects the correct page-specific decision from
remapPage/decideRecovery rather than the last one seen.

In `@src/parser/pdf/index.ts`:
- Around line 152-154: The OCR warning path is exposing raw exception text
through `errorMessage` and `lineHint`, which can leak internal filesystem/config
details to callers. Update the `errorMessage` helper and the related
parse-warning flow so callers only receive a generic `pdf-ocr-unusable` hint,
and make sure the full error is logged server-side via the pino logger from
`src/lib/logger.ts` rather than using any console output. Keep the public
surface in the `src/parser/pdf/index.ts` flow from returning `err.message`
directly, including the later OCR-init handling code referenced by the same
helper.

In `@src/parser/pdf/ocr.ts`:
- Around line 81-143: The OCR module is leaking raw filesystem/render/Tesseract
errors instead of a module-owned typed error. Add a `SpecrError` subclass for
this parser OCR module and catch failures in `createManagedRecognizer`,
`withRecognizer`, and `recognizePage` so you can rethrow with added stage/page
context using `cause`. Keep `recognizePdfPages` as the public surface that only
emits the typed OCR error, preserving the original error details while enriching
the message with the relevant page or OCR step.
- Around line 57-59: The pdfData helper in ocr.ts is unnecessarily cloning the
PDF buffer before OCR, which adds memory overhead on this hot path. Update
pdfData to return the incoming Buffer directly since it already satisfies
Uint8Array, and keep the change localized to the pdfData function used by the
OCR flow.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5e5f4103-123e-40ef-b9e3-4cc9658a9545

📥 Commits

Reviewing files that changed from the base of the PR and between 67bf1e6 and adb9ec6.

⛔ Files ignored due to path filters (1)

• pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

📒 Files selected for processing (16)

• openapi.yaml
• package.json
• pnpm-workspace.yaml
• src/api/parse.integration.test.ts
• src/ast/schemas.ts
• src/ast/types.ts
• src/lib/env.ts
• src/lib/parse-worker.ts
• src/parser/index.ts
• src/parser/pdf/font-encoding.test.ts
• src/parser/pdf/font-encoding.ts
• src/parser/pdf/index.test.ts
• src/parser/pdf/index.ts
• src/parser/pdf/ocr.integration.test.ts
• src/parser/pdf/ocr.ts
• src/parser/text/index.ts

[thewrz]

Codex (GPT-5.5, xhigh) adversarial review — additional eyes

Ran alongside CodeRabbit as a second reviewer. One confirmed finding:

[P1] OCR worker init can hang parse jobs offline when traineddata is uncached — src/parser/pdf/ocr.ts

Confirmed and reproduced. With OCR_LANG_PATH unset and an empty cache, tesseract.js fetches eng.traineddata from a CDN during createWorker. In a network-restricted env, Codex's run of ocr.integration.test.ts surfaced TypeError: fetch failed from tesseract.js/src/createWorker.js. In the project's offline target, a network stall (vs. fast failure) could leave the first scanned-PDF parse job hung instead of completing with pdf-ocr-unusable.

Decision — partially mitigated here, remainder tracked as a follow-up (#298):

• The fail-fast path is now handled by this PR's review fixes: createManagedRecognizer wraps init failures in a typed ParserError, which applyOcrIfNeeded catches → clean pdf-ocr-unusable degradation (no hang, no leaked paths).
• The indefinite-stall path (bounded init timeout) and offline provisioning (vendor/require local eng.traineddata, never depend on a CDN in production) are a deliberate packaging/config decision — binary asset, license, deployment docs — that warrants its own issue + ADR rather than being rushed into this feature PR. Filed as fix(parser): OCR worker init can hang parse jobs offline when traineddata is uncached (#246 follow-up) #298.
• .env.example now documents OCR_LANG_PATH as the operator-facing interim mitigation for offline deployments.

This isn't a regression: before this PR scanned PDFs weren't OCR'd at all, and the real-OCR path is gated behind SPECR_OCR_E2E (skipped in normal CI). The feature works when traineddata is available; #298 hardens the offline edge.

No other P1/P2 findings from Codex. CodeRabbit's 6 inline findings are all addressed in 4dd3b9e (5 fixed, 1 declined with rationale).