Skip to content

feat(vision): selectable pinned OCR model sets with a PP-OCRv5 Korean set - #496

Open
NUMORDO wants to merge 2 commits into
firecrawl:mainfrom
NUMORDO:feat/ocr-model-set-korean
Open

NUMORDO wants to merge 2 commits into
firecrawl:mainfrom
NUMORDO:feat/ocr-model-set-korean

Conversation

@NUMORDO

@NUMORDO NUMORDO commented Sep 4, 2026

Copy link
Copy Markdown

Summary

Adds OcrModelSet so the selective-OCR path can load a different pinned, checksum-verified model set without changing anything else in the pipeline. The default set (PpOcrV6Small) and all existing behavior are unchanged; the new PpOcrV5Korean set makes Hangul readable.

Motivation: the PP-OCRv6 Small dictionary contains no Hangul at all (18,709 entries, 0 in U+AC00–U+D7A3), so every Korean scanned page currently comes back empty even though the routing, rendering, fusion and Markdown assembly all work. The PP-OCRv5 Korean recogniser published with oar-ocr (already the crate's OCR backend) covers all 11,172 Hangul syllables plus Latin letters and digits, and pairs with the script-agnostic PP-OCRv5 mobile detector. Together the set is ~18 MB, smaller than the default.

What changed

  • vision::OcrModelSet (PpOcrV6Small default, PpOcrV5Korean) with stable identifiers, a lenient parse, and Display. #[non_exhaustive] so more sets can follow.
  • OcrOptions::model_set(...) / OcrOptions.model_set. Default still selects PP-OCRv6 Small.
  • vision::PP_OCR_V5_KOREAN manifest pinned to the oar-ocr v0.3.0 release files (SHA-256 and exact sizes), and OcrModelSet::manifest().
  • cached_ocr_engine now resolves, downloads, and caches by the selected manifest. The engine cache key already carried manifest id/revision/digests, so switching sets replaces the loaded sessions instead of mixing artifacts; each set also lives under its own <id>/<revision> cache directory.
  • CLI: pdf2md --ocr-model-set pp-ocrv5-korean (listed with the other OCR-only options; invalid values print the accepted identifiers).
  • Docs: docs/ocr-runtime.md "Model sets" table and examples, docs/rust-api.md, CHANGELOG.

Python and Node bindings are untouched in this PR (they keep the default set); happy to add a model_set option to both in a follow-up if you want parity in the same release.

Verification

  • cargo fmt, cargo clippy -- -D warnings, cargo clippy --features ocr -- -D warnings, cargo test, cargo test --features ocr --lib vision:: on the pinned 1.98.0 toolchain.
  • New unit tests: Korean manifest validates and has the three expected artifact kinds; every OcrModelSet resolves to a manifest whose id matches; sets use distinct cache roots; identifier parsing round-trips and rejects unknown values; OcrOptions default is unchanged.
  • End-to-end (Linux x64, PDFium native-v7988, ONNX Runtime 1.27.0, CPU): a Korean government PDF rasterised to a 150 dpi image-only PDF (3 pages, classified scanned) processed with --ocr auto --ocr-model-set pp-ocrv5-korean --ocr-offline --ocr-model-dir …. Results are in the comment below.

Notes for reviewers

  • Artifact digests were computed from the files downloaded from the oar-ocr v0.3.0 GitHub release (pp-ocrv5_mobile_det.onnx 4,826,518 B, korean_pp-ocrv5_mobile_rec.onnx 13,446,374 B, ppocrv5_korean_dict.txt 47,451 B).
  • No new dependencies. No change to the default feature set or to the text-only path.

Summary by cubic

Adds OcrModelSet so the selective-OCR path can load a different pinned, checksum-verified model set. The default PpOcrV6Small set is unchanged, and the new PpOcrV5Korean set makes Hangul readable — Korean scanned pages previously came back empty because the default dictionary has no Hangul.

  • PpOcrV5Korean pairs the script-agnostic PP-OCRv5 mobile detector with the Korean recogniser and dictionary (~18 MB total).
  • OcrOptions::model_set(...) and pdf2md --ocr-model-set pp-ocrv5-korean select the set; the engine cache is keyed by the selected set, so switching sets never mixes artifacts.
  • OcrOptions gained a public model_set field, so struct literals that spell out every field need the new field.
  • Python and Node bindings are untouched and keep the default set.

Written for commit 76cdc11. Summary will update on new commits.

Review in cubic

… set

Add OcrModelSet so the selective-OCR path can load a different pinned,
checksum-verified model set. PpOcrV6Small stays the default; the new
PpOcrV5Korean set pairs the script-agnostic PP-OCRv5 mobile detector with
the Korean PP-OCRv5 mobile recogniser and its dictionary (all 11,172 Hangul
syllables plus Latin letters and digits, ~18 MB), published with the
oar-ocr v0.3.0 release. The PP-OCRv6 Small dictionary has no Hangul, so
Korean scanned pages previously came back empty.

- OcrOptions::model_set / OcrModelSet::{id, parse, manifest}
- PP_OCR_V5_KOREAN manifest; engine cache and model cache are keyed per set
- pdf2md --ocr-model-set pp-ocrv5-korean
- docs: ocr-runtime model-set table, rust-api; CHANGELOG
- tests: manifest validity, id parity, distinct cache roots, parsing

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@NUMORDO

NUMORDO commented Sep 4, 2026

Copy link
Copy Markdown
Author

End-to-end measurement for the Korean set (Linux x64, 12-thread laptop CPU, release build with --features ocr, PDFium native-v7988, ONNX Runtime 1.27.0, offline model directory):

Input: a Korean government legal-interpretation PDF (text-based) rasterised to a 150 dpi greyscale image-only PDF, 3 pages, classified scanned with pages_needing_ocr = [1, 2, 3]. Ground truth = the original text layer of the same pages, extracted with pdf2md.

Run Result
--ocr auto --ocr-model-set pp-ocrv5-korean --ocr-offline --ocr-model-dir … all 3 pages routed to OCR, 1,434 Hangul characters recognised; character-level recall 0.918, precision 0.916 against the text layer (whitespace/markup-normalised difflib matching, 1,647 reference characters)
same input, default pp-ocrv6-small 0 Hangul characters (176 characters of non-Korean output), as expected from the dictionary
wall time (3 pages, warm cache) 32.6 s wall / 93.7 s user with the Korean set; 16.7 s wall with the default set on the same pages. --ocr-dpi 100/120 did not change either time or accuracy materially, so the cost is recognition of many short lines (dense legal text), not rendering

Clean text PDFs are unaffected (no renderer, model, or ORT initialisation happens before routing).

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 8 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/vision/contracts.rs">

<violation number="1" location="src/vision/contracts.rs:95">
P2: Adding this field breaks downstream `OcrOptions { ... }` struct literals because `OcrOptions` is public and not `#[non_exhaustive]`. Preserve source compatibility with a separate versioned options type/API, or treat this as a semver-breaking change and release it under the appropriate major-version policy.</violation>
</file>

Shadow auto-approve: would not auto-approve because issues were found.

Fix all with cubic | Re-trigger cubic

Comment thread src/vision/contracts.rs
/// Whether a missing pinned artifact may be downloaded.
pub model_downloads: ModelDownloadPolicy,
/// Which pinned model set the engine loads.
pub model_set: OcrModelSet,

@cubic-dev-ai cubic-dev-ai Bot Sep 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Adding this field breaks downstream OcrOptions { ... } struct literals because OcrOptions is public and not #[non_exhaustive]. Preserve source compatibility with a separate versioned options type/API, or treat this as a semver-breaking change and release it under the appropriate major-version policy.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/vision/contracts.rs, line 95:

<comment>Adding this field breaks downstream `OcrOptions { ... }` struct literals because `OcrOptions` is public and not `#[non_exhaustive]`. Preserve source compatibility with a separate versioned options type/API, or treat this as a semver-breaking change and release it under the appropriate major-version policy.</comment>

<file context>
@@ -40,6 +91,8 @@ pub struct OcrOptions {
     /// Whether a missing pinned artifact may be downloaded.
     pub model_downloads: ModelDownloadPolicy,
+    /// Which pinned model set the engine loads.
+    pub model_set: OcrModelSet,
 }
 
</file context>
Fix with cubic

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@NUMORDO

NUMORDO commented Sep 4, 2026

Copy link
Copy Markdown
Author

Re the automated note on OcrOptions.model_set:

  • OcrOptions is documented and constructed only through OcrOptions::new() / Default and the builder methods (docs/rust-api.md, docs/ocr-runtime.md, the CLI and the Python bindings all use that path); there is no struct-literal construction of it anywhere in the repo.
  • Adding a public field to a public struct in a minor release is the crate's existing practice: in this same unreleased cycle TextItem gained rotation and advance_known (feat(extractor): axis-aligned boxes and rotation for rotated text runs #486) and baseline_shift (feat(extractor): keep superscript/subscript runs on their line and expose baseline_shift #488).
  • Any way of attaching a new option to OcrOptions (public or private field) has the same effect on a fully spelled-out literal, so the choice is between following the existing practice or introducing a parallel options type. I kept the field and added a note under Changed in the CHANGELOG (pushed) so the release notes call it out.

If you would rather gate this behind #[non_exhaustive] on OcrOptions (which would also stop literals, but makes the policy explicit for the future) or a different shape, tell me and I will adjust.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 1 file (changes from recent commits).

Shadow auto-approve: would not auto-approve. Auto-approval blocked by 1 unresolved issue from previous reviews.

Re-trigger cubic

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant