Skip to content

feat(pdf): convert the readable pages when others need ocr - #153

Open
larue-greenboard wants to merge 1 commit into
firecrawl:mainfrom
larue-greenboard:feat/pdf-skip-ocr-pages
Open

feat(pdf): convert the readable pages when others need ocr#153
larue-greenboard wants to merge 1 commit into
firecrawl:mainfrom
larue-greenboard:feat/pdf-skip-ocr-pages

Conversation

@larue-greenboard

@larue-greenboard larue-greenboard commented Aug 31, 2026

Copy link
Copy Markdown

A PDF with one image-only page returns nothing at all, so the pages that do carry text are lost along with it. One image on page 3 of an 80-page contract costs the other 79 pages.

Ocr::Skip converts the pages that carry text instead of failing the document, and names the ones it left out: Options::default().ocr(Ocr::Skip) returning Conversion { markdown, pages_needing_ocr, page_count } in Rust, { ocr: 'skip' } in Node, ocr="skip" in Python, toMarkdownBytesWith(bytes, format, 'skip') in wasm, --ocr skip on the CLI. A document where no page yielded text still fails with NeedsOcr — there is nothing to hand back, and the error is what tells a caller the document needs OCR rather than better parsing, so --ocr hosted and exit code 3 keep working as the signal for a real scan.

The default is untouched, byte for byte: to_markdown / to_markdown_bytes delegate to the new *_with forms with Options::default(), and all 66 existing snapshots are unchanged. Options is #[non_exhaustive] with a consuming ocr() setter, so #102 and #146 have somewhere to land later. No dependency bump: pdf-inspector 1.14.2 already returns per-page Markdown with a per-page needs_ocr.

Nothing is written into the Markdown

#144's third suggestion was an in-text marker where a page was dropped. This reports the pages on the result instead and leaves the output as content alone, because that Markdown is shown to readers and fed to models — a notice injected into it is corruption downstream, and unlike a marker, a page list can be branched on. The objection behind #140 was that the loss was silent, and it is not silent if the caller is handed the page numbers and had to opt in to get them. A test pins the Markdown as free of any such notice, so this cannot drift.

On the CLI the same reasoning puts the report on stderr, which already carries every diagnostic, so stdout stays Markdown alone:

$ anydoc contract.pdf --ocr skip > contract.md
anydoc: converted 79 of 80 pages; page 3 needs OCR

Why per-page extraction, and where it costs

Whole-document extraction is all-or-nothing, not merely lossy: process_pdf_mem classifies a mixed document as ImageBased and returns markdown: None for the whole thing, even where individual pages carry perfectly good text. So the readable pages have to come from extract_pages_markdown_mem, and the per-page needs_ocr verdicts decide both what is converted and what is reported — the two cannot disagree.

That pass only runs once detection and the existing confirmation pass agree a page is genuinely unreadable, so a document that converts today pays nothing new. Measured over 30 runs, an 80-page file:

default ocr: 'skip'
no page needs OCR 4.50 ms 4.53 ms
one page needs OCR 5.62 ms (returns nothing) 10.51 ms (returns 79 pages)

The second pass doubles the cost on documents that need it, against returning nothing at all. Worth a look if that trade is not the one you want.

Ocr::Skip is also decided before conversion rather than after a failure, unlike the hosted recovery in #141: no throw, no second file read, no double parse.

Tests

handmade-partly-scanned.pdf is a new 5-page fixture, TEXT, IMAGE, TEXT, TEXT, IMAGE with per-page text, generated by the existing handmade_pdf() helper. The existing one-text-one-image fixture cannot show that pages after an image page survive, which is the reported failure. Its default-path snapshot is the unchanged ERROR: pages 2, 5 of 5 need OCR.

Five assertions in tests/snapshots.rs cover the recovered pages, the absence of notices in the output, the fully scanned document still erroring, text.pdf being byte-identical in both modes, and Options::default() matching the plain functions. Plus the Node, Python and wasm equivalents, and CLI coverage of both the stderr report and exit 3.

cargo fmt --check clean · cargo clippy --workspace --all-targets --all-features -- -D warnings exit 0 · cargo clippy -p anydoc-wasm --target wasm32-unknown-unknown exit 0 · 301 Rust tests, 24 Node, 13 Python, 9 wasm pass · cargo doc clean with -D warnings · Node overloads typecheck under --strict.

Unrelated, noticed on the way

A PDF that detection does not flag but which yields no text returns Unsupported, not NeedsOcr (the final match in src/formats/pdf.rs). Since the hosted fallback keys on code === 'needsOcr', such a document never reaches Parse even with --ocr hosted. Left alone here; happy to file it separately.

Fixes #144


Summary by cubic

Fixes #144: a PDF with scanned pages previously returned nothing; now ocr: 'skip' converts the readable pages and reports the ones it left out. The report is data on the result and stderr for the CLI, never a notice in the Markdown, and the default path is byte-for-byte unchanged.

New Features

  • Adds Ocr::Skip to Options, exposed as ocr: 'skip' in Node and Python, --ocr skip on the CLI, and toMarkdownBytesWith(bytes, format, 'skip') in wasm.
  • Returns a Conversion carrying markdown, pages_needing_ocr, and page_count.
  • A document where no page yielded text still errors with NeedsOcr, so --ocr hosted and exit code 3 still signal a real scan.
  • Per-page extraction adds a second pass, roughly doubling conversion time for documents that need it.

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

Review in cubic

A PDF with one image-only page returns nothing at all, so the pages that
do carry text are lost along with it.

`Ocr::Skip` converts those pages instead of failing the document and names
the ones it left out on the result, leaving the default untouched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@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.

4 issues found across 28 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="skills/convert-documents-to-markdown/SKILL.md">

<violation number="1" location="skills/convert-documents-to-markdown/SKILL.md:25">
P3: Rule 5 adds that `--ocr skip` converts the readable pages and leaves the rest out, but does not say what exit code the process returns in that mode. `skip_pages_needing_ocr` returns success (exit 0) whenever even one page yields text, and only errors with exit 3 when nothing is readable. That makes rule 3's blanket contract "3 pages of a PDF need OCR" mode-dependent: a partially scanned PDF run with `--ocr skip` exits 0 while pages still need OCR. Clarify in this sentence that skip mode exits 0 when some pages convert (left-out pages are signalled only via the stderr message), so a caller that treats a non-zero exit as the detection signal is not misled.</violation>
</file>

<file name="python/anydoc/__init__.py">

<violation number="1" location="python/anydoc/__init__.py:65">
P3: When a caller stores the mode in the public `Ocr` type, both functions fail type checking because only literal overloads are exposed. Add fallback overloads accepting `Ocr` and returning `str | Conversion`.</violation>
</file>

<file name="wasm/test.mjs">

<violation number="1" location="wasm/test.mjs:95">
P3: A wasm conversion that drops readable pages 1 or 4 still passes this test because it checks only page 3. Assert all three readable pages so this binding test covers the per-page conversion guarantee.</violation>

<violation number="2" location="wasm/test.mjs:98">
P3: If the wasm error reports the wrong OCR pages or page count, this test still passes because it checks only `error.code`. Assert `[error.pages, error.pageCount]` equals `[[1, 2], 2]` to verify the documented error shape.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

3. Exit codes: 0 success, 1 the document could not be converted, 2 usage error, 3 pages of a PDF need OCR. Failures print one `anydoc: <message>` line to stderr. The CLI never prompts.
4. For a large document, write to a file with `-o` and read the parts you need instead of streaming everything into context.
5. Scanned and image-only pages need OCR, which anydoc does not do, so the document exits 3. Rerun with `--ocr hosted` to send it to [Firecrawl Parse](https://firecrawl.dev/parse). No signup needed. Pass `--api-key` or set `FIRECRAWL_API_KEY` for higher limits.
5. Scanned and image-only pages need OCR, which anydoc does not do, so the document exits 3. Rerun with `--ocr hosted` to send it to [Firecrawl Parse](https://firecrawl.dev/parse). No signup needed. Pass `--api-key` or set `FIRECRAWL_API_KEY` for higher limits. Where that is not an option, `--ocr skip` converts the pages that do carry text and names the pages it left out on stderr; stdout stays Markdown alone.

@cubic-dev-ai cubic-dev-ai Bot Aug 31, 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.

P3: Rule 5 adds that --ocr skip converts the readable pages and leaves the rest out, but does not say what exit code the process returns in that mode. skip_pages_needing_ocr returns success (exit 0) whenever even one page yields text, and only errors with exit 3 when nothing is readable. That makes rule 3's blanket contract "3 pages of a PDF need OCR" mode-dependent: a partially scanned PDF run with --ocr skip exits 0 while pages still need OCR. Clarify in this sentence that skip mode exits 0 when some pages convert (left-out pages are signalled only via the stderr message), so a caller that treats a non-zero exit as the detection signal is not misled.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At skills/convert-documents-to-markdown/SKILL.md, line 25:

<comment>Rule 5 adds that `--ocr skip` converts the readable pages and leaves the rest out, but does not say what exit code the process returns in that mode. `skip_pages_needing_ocr` returns success (exit 0) whenever even one page yields text, and only errors with exit 3 when nothing is readable. That makes rule 3's blanket contract "3 pages of a PDF need OCR" mode-dependent: a partially scanned PDF run with `--ocr skip` exits 0 while pages still need OCR. Clarify in this sentence that skip mode exits 0 when some pages convert (left-out pages are signalled only via the stderr message), so a caller that treats a non-zero exit as the detection signal is not misled.</comment>

<file context>
@@ -22,5 +22,5 @@ Rules:
 3. Exit codes: 0 success, 1 the document could not be converted, 2 usage error, 3 pages of a PDF need OCR. Failures print one `anydoc: <message>` line to stderr. The CLI never prompts.
 4. For a large document, write to a file with `-o` and read the parts you need instead of streaming everything into context.
-5. Scanned and image-only pages need OCR, which anydoc does not do, so the document exits 3. Rerun with `--ocr hosted` to send it to [Firecrawl Parse](https://firecrawl.dev/parse). No signup needed. Pass `--api-key` or set `FIRECRAWL_API_KEY` for higher limits.
+5. Scanned and image-only pages need OCR, which anydoc does not do, so the document exits 3. Rerun with `--ocr hosted` to send it to [Firecrawl Parse](https://firecrawl.dev/parse). No signup needed. Pass `--api-key` or set `FIRECRAWL_API_KEY` for higher limits. Where that is not an option, `--ocr skip` converts the pages that do carry text and names the pages it left out on stderr; stdout stays Markdown alone.
 6. Inside a Node, Python, or Rust codebase, prefer the library over shelling out: `@firecrawl/anydoc` on npm, `firecrawl-anydoc` on PyPI, `anydoc` on crates.io. Each exposes the same `to_markdown` / `toMarkdown` API.
</file context>
Suggested change
5. Scanned and image-only pages need OCR, which anydoc does not do, so the document exits 3. Rerun with `--ocr hosted` to send it to [Firecrawl Parse](https://firecrawl.dev/parse). No signup needed. Pass `--api-key` or set `FIRECRAWL_API_KEY` for higher limits. Where that is not an option, `--ocr skip` converts the pages that do carry text and names the pages it left out on stderr; stdout stays Markdown alone.
5. Scanned and image-only pages need OCR, which anydoc does not do, so the document exits 3. Rerun with `--ocr hosted` to send it to [Firecrawl Parse](https://firecrawl.dev/parse). No signup needed. Pass `--api-key` or set `FIRECRAWL_API_KEY` for higher limits. Where that is not an option, `--ocr skip` converts the pages that do carry text, exits 0 (or 3 if no page yields text), and names the pages it left out on stderr; stdout stays Markdown alone.
Fix with cubic

Comment thread python/anydoc/__init__.py


@overload
def to_markdown(

@cubic-dev-ai cubic-dev-ai Bot Aug 31, 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.

P3: When a caller stores the mode in the public Ocr type, both functions fail type checking because only literal overloads are exposed. Add fallback overloads accepting Ocr and returning str | Conversion.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At python/anydoc/__init__.py, line 65:

<comment>When a caller stores the mode in the public `Ocr` type, both functions fail type checking because only literal overloads are exposed. Add fallback overloads accepting `Ocr` and returning `str | Conversion`.</comment>

<file context>
@@ -45,31 +48,58 @@
 
 
+@overload
+def to_markdown(
+    path: "str | os.PathLike[str]",
+    *,
</file context>
Fix with cubic

Comment thread wasm/test.mjs
test("ocr: 'skip' converts the pages that carry text and names the rest", () => {
const { markdown, pagesNeedingOcr, pageCount } = toMarkdownBytesWith(PARTLY_SCANNED, 'pdf', 'skip')
assert.deepEqual([pagesNeedingOcr, pageCount], [[2, 5], 5])
assert.match(markdown, /Readable page three/)

@cubic-dev-ai cubic-dev-ai Bot Aug 31, 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.

P3: A wasm conversion that drops readable pages 1 or 4 still passes this test because it checks only page 3. Assert all three readable pages so this binding test covers the per-page conversion guarantee.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At wasm/test.mjs, line 95:

<comment>A wasm conversion that drops readable pages 1 or 4 still passes this test because it checks only page 3. Assert all three readable pages so this binding test covers the per-page conversion guarantee.</comment>

<file context>
@@ -86,6 +89,23 @@ test('conversion errors throw a coded Error', () => {
+test("ocr: 'skip' converts the pages that carry text and names the rest", () => {
+  const { markdown, pagesNeedingOcr, pageCount } = toMarkdownBytesWith(PARTLY_SCANNED, 'pdf', 'skip')
+  assert.deepEqual([pagesNeedingOcr, pageCount], [[2, 5], 5])
+  assert.match(markdown, /Readable page three/)
+  // The pages left out are reported, never written into the Markdown.
+  assert.doesNotMatch(markdown, /OCR|not converted/i)
</file context>
Suggested change
assert.match(markdown, /Readable page three/)
for (const page of ['one', 'three', 'four']) {
assert.match(markdown, new RegExp(`Readable page ${page}`))
}
Fix with cubic

Comment thread wasm/test.mjs
assert.match(markdown, /Readable page three/)
// The pages left out are reported, never written into the Markdown.
assert.doesNotMatch(markdown, /OCR|not converted/i)
assert.throws(() => toMarkdownBytesWith(SCANNED, 'pdf', 'skip'), (error) => {

@cubic-dev-ai cubic-dev-ai Bot Aug 31, 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.

P3: If the wasm error reports the wrong OCR pages or page count, this test still passes because it checks only error.code. Assert [error.pages, error.pageCount] equals [[1, 2], 2] to verify the documented error shape.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At wasm/test.mjs, line 98:

<comment>If the wasm error reports the wrong OCR pages or page count, this test still passes because it checks only `error.code`. Assert `[error.pages, error.pageCount]` equals `[[1, 2], 2]` to verify the documented error shape.</comment>

<file context>
@@ -86,6 +89,23 @@ test('conversion errors throw a coded Error', () => {
+  assert.match(markdown, /Readable page three/)
+  // The pages left out are reported, never written into the Markdown.
+  assert.doesNotMatch(markdown, /OCR|not converted/i)
+  assert.throws(() => toMarkdownBytesWith(SCANNED, 'pdf', 'skip'), (error) => {
+    assert.equal(error.code, 'needsOcr')
+    return true
</file context>
Fix with cubic

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.

0.2.4: one image-only page makes the whole PDF convert to nothing, including the pages that do have text

1 participant