feat: OCR scanned PDFs with a vision model via LiteLLM - #161
Open
sandeep-selvaraj wants to merge 1 commit into
Open
feat: OCR scanned PDFs with a vision model via LiteLLM#161sandeep-selvaraj wants to merge 1 commit into
sandeep-selvaraj wants to merge 1 commit into
Conversation
Add ocr="llm" to the Python bindings, alongside ocr="hosted". When a PDF needs OCR, rasterise every page with pypdfium2 and transcribe it to Markdown through litellm.completion, so the document goes only to the provider the user configures.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 6 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="python/anydoc/_llm_ocr.py">
<violation number="1" location="python/anydoc/_llm_ocr.py:82">
P1: When callers provide `model=` without `ANYDOC_OCR_MODEL`, the override cannot work because settings validation fails before the override is applied. Pass the override into the settings constructor so it satisfies the required model field.</violation>
<violation number="2" location="python/anydoc/_llm_ocr.py:111">
P2: A large PDF page or `ANYDOC_OCR_DPI` value can exhaust memory during rasterization because rendering has no pixel/byte bound and all PNGs are retained. Enforce a maximum rendered size and use bounded/streamed page processing.</violation>
<violation number="3" location="python/anydoc/_llm_ocr.py:163">
P1: When any page completion is empty while another page has content, this filter silently returns a partial document. Treat an empty per-page response as an OCR failure, or preserve the page boundary, instead of checking only whether the entire document is empty.</violation>
</file>
<file name="python/anydoc/__init__.py">
<violation number="1" location="python/anydoc/__init__.py:54">
P2: The `ocr="llm"` path rasterizes pages and sends them to LiteLLM, so this sentence can falsely promise local privacy. Clarify that only local conversions stay on-device and OCR modes transmit content to their configured services.</violation>
</file>
<file name="python/pyproject.toml">
<violation number="1" location="python/pyproject.toml:27">
P2: The `llm` extra does not declare Pillow, but `_rasterise()` relies on `pypdfium2.PdfBitmap.to_pil()` to build the PNGs sent to the model. pypdfium2's `to_pil()` requires Pillow and does not install it transitively, so a clean `pip install firecrawl-anydoc[llm]` without a pre-existing Pillow fails during rasterisation (and `test_ocr_llm_transcribes_every_scanned_page_and_nothing_else` depends on Pillow being present). Add Pillow to the extra so the OCR path works on a fresh install.</violation>
</file>
<file name="python/tests/test_anydoc.py">
<violation number="1" location="python/tests/test_anydoc.py:187">
P3: The side_effect reads `completion.call_count` inside the threaded page pool, so both pages can observe the same value and the assertion `assertIn("# Page 1", markdown)` passes even if both pages came back as "# Page 1". Make the reply independent of thread timing (e.g. a per-call counter passed through the message content, or reply with a fixed distinct string per page) so the test actually verifies two distinct pages were transcribed.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| pool.map(lambda png: _transcribe_page(litellm, settings, png), pages) | ||
| ) | ||
|
|
||
| markdown = "\n\n".join(part.strip() for part in transcribed if part.strip()) |
There was a problem hiding this comment.
P1: When any page completion is empty while another page has content, this filter silently returns a partial document. Treat an empty per-page response as an OCR failure, or preserve the page boundary, instead of checking only whether the entire document is empty.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At python/anydoc/_llm_ocr.py, line 163:
<comment>When any page completion is empty while another page has content, this filter silently returns a partial document. Treat an empty per-page response as an OCR failure, or preserve the page boundary, instead of checking only whether the entire document is empty.</comment>
<file context>
@@ -0,0 +1,166 @@
+ pool.map(lambda png: _transcribe_page(litellm, settings, png), pages)
+ )
+
+ markdown = "\n\n".join(part.strip() for part in transcribed if part.strip())
+ if not markdown:
+ raise LlmOcrConfigError("the model returned no Markdown")
</file context>
|
|
||
| build = _build_settings_class() | ||
| try: | ||
| settings = build() |
There was a problem hiding this comment.
P1: When callers provide model= without ANYDOC_OCR_MODEL, the override cannot work because settings validation fails before the override is applied. Pass the override into the settings constructor so it satisfies the required model field.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At python/anydoc/_llm_ocr.py, line 82:
<comment>When callers provide `model=` without `ANYDOC_OCR_MODEL`, the override cannot work because settings validation fails before the override is applied. Pass the override into the settings constructor so it satisfies the required model field.</comment>
<file context>
@@ -0,0 +1,166 @@
+
+ build = _build_settings_class()
+ try:
+ settings = build()
+ except ValidationError as error:
+ missing = [
</file context>
Suggested change
| settings = build() | |
| settings = build(model=model_override) if model_override is not None else build() |
| ) | ||
| pages = [] | ||
| for page in pdf: | ||
| bitmap = page.render(scale=dpi / 72) |
There was a problem hiding this comment.
P2: A large PDF page or ANYDOC_OCR_DPI value can exhaust memory during rasterization because rendering has no pixel/byte bound and all PNGs are retained. Enforce a maximum rendered size and use bounded/streamed page processing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At python/anydoc/_llm_ocr.py, line 111:
<comment>A large PDF page or `ANYDOC_OCR_DPI` value can exhaust memory during rasterization because rendering has no pixel/byte bound and all PNGs are retained. Enforce a maximum rendered size and use bounded/streamed page processing.</comment>
<file context>
@@ -0,0 +1,166 @@
+ )
+ pages = []
+ for page in pdf:
+ bitmap = page.render(scale=dpi / 72)
+ image = bitmap.to_pil()
+ buffer = io.BytesIO()
</file context>
| Firecrawl Parse. `llm` rasterises the pages and transcribes them with a | ||
| vision model through LiteLLM, configured from `ANYDOC_OCR_*` environment | ||
| variables and needing the `llm` extra (`pip install firecrawl-anydoc[llm]`). | ||
| Documents anydoc converts itself never leave the machine.""" |
There was a problem hiding this comment.
P2: The ocr="llm" path rasterizes pages and sends them to LiteLLM, so this sentence can falsely promise local privacy. Clarify that only local conversions stay on-device and OCR modes transmit content to their configured services.
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 54:
<comment>The `ocr="llm"` path rasterizes pages and sends them to LiteLLM, so this sentence can falsely promise local privacy. Clarify that only local conversions stay on-device and OCR modes transmit content to their configured services.</comment>
<file context>
@@ -45,38 +45,51 @@
+Firecrawl Parse. `llm` rasterises the pages and transcribes them with a
+vision model through LiteLLM, configured from `ANYDOC_OCR_*` environment
+variables and needing the `llm` extra (`pip install firecrawl-anydoc[llm]`).
+Documents anydoc converts itself never leave the machine."""
</file context>
Suggested change
| Documents anydoc converts itself never leave the machine.""" | |
| Documents converted locally by anydoc never leave the machine; `ocr="hosted"` | |
| and `ocr="llm"` send document content to their configured service.""" |
| # `llm` powers `to_markdown(..., ocr="llm")`: LiteLLM calls the vision model, | ||
| # pypdfium2 rasterises the pages, pydantic-settings reads `ANYDOC_OCR_*`. | ||
| [project.optional-dependencies] | ||
| llm = ["litellm>=1.0", "pypdfium2>=4", "pydantic-settings>=2"] |
There was a problem hiding this comment.
P2: The llm extra does not declare Pillow, but _rasterise() relies on pypdfium2.PdfBitmap.to_pil() to build the PNGs sent to the model. pypdfium2's to_pil() requires Pillow and does not install it transitively, so a clean pip install firecrawl-anydoc[llm] without a pre-existing Pillow fails during rasterisation (and test_ocr_llm_transcribes_every_scanned_page_and_nothing_else depends on Pillow being present). Add Pillow to the extra so the OCR path works on a fresh install.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At python/pyproject.toml, line 27:
<comment>The `llm` extra does not declare Pillow, but `_rasterise()` relies on `pypdfium2.PdfBitmap.to_pil()` to build the PNGs sent to the model. pypdfium2's `to_pil()` requires Pillow and does not install it transitively, so a clean `pip install firecrawl-anydoc[llm]` without a pre-existing Pillow fails during rasterisation (and `test_ocr_llm_transcribes_every_scanned_page_and_nothing_else` depends on Pillow being present). Add Pillow to the extra so the OCR path works on a fresh install.</comment>
<file context>
@@ -21,6 +21,11 @@ classifiers = [
+# `llm` powers `to_markdown(..., ocr="llm")`: LiteLLM calls the vision model,
+# pypdfium2 rasterises the pages, pydantic-settings reads `ANYDOC_OCR_*`.
+[project.optional-dependencies]
+llm = ["litellm>=1.0", "pypdfium2>=4", "pydantic-settings>=2"]
+
[project.urls]
</file context>
Suggested change
| llm = ["litellm>=1.0", "pypdfium2>=4", "pydantic-settings>=2"] | |
| llm = ["litellm>=1.0", "pypdfium2>=4", "pydantic-settings>=2", "Pillow>=9"] |
| @unittest.skipUnless(_LLM_EXTRA, "the 'llm' extra is not installed") | ||
| def test_ocr_llm_transcribes_every_scanned_page_and_nothing_else(self): | ||
| with mock.patch("litellm.completion") as completion, llm_env(model="test/model"): | ||
| completion.side_effect = lambda **kw: _completion_reply( |
There was a problem hiding this comment.
P3: The side_effect reads completion.call_count inside the threaded page pool, so both pages can observe the same value and the assertion assertIn("# Page 1", markdown) passes even if both pages came back as "# Page 1". Make the reply independent of thread timing (e.g. a per-call counter passed through the message content, or reply with a fixed distinct string per page) so the test actually verifies two distinct pages were transcribed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At python/tests/test_anydoc.py, line 187:
<comment>The side_effect reads `completion.call_count` inside the threaded page pool, so both pages can observe the same value and the assertion `assertIn("# Page 1", markdown)` passes even if both pages came back as "# Page 1". Make the reply independent of thread timing (e.g. a per-call counter passed through the message content, or reply with a fixed distinct string per page) so the test actually verifies two distinct pages were transcribed.</comment>
<file context>
@@ -146,6 +181,41 @@ def test_the_keyless_limit_says_to_set_an_api_key(self):
+ @unittest.skipUnless(_LLM_EXTRA, "the 'llm' extra is not installed")
+ def test_ocr_llm_transcribes_every_scanned_page_and_nothing_else(self):
+ with mock.patch("litellm.completion") as completion, llm_env(model="test/model"):
+ completion.side_effect = lambda **kw: _completion_reply(
+ f"# Page {completion.call_count}"
+ )
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #141 — a provider-agnostic
ocr="llm"besideocr="hosted".Scope, and why Python only
#141 added
ocr="hosted": a PDF that needs OCR goes to Firecrawl Parse. This adds a second opt-in for people who would rather the document go to a model of their own choosing — OpenAI, Anthropic, Gemini, Bedrock, a local Ollama, a LiteLLM proxy — and not to Firecrawl.LiteLLM's SDK is Python, so this is the Python binding only. Node and wasm keep
hostedas their sole OCR path; the Rust core is untouched.to_markdown/to_markdown_bytesgain oneOcrliteral and one keyword — there is no newFormat, no.pyichange, no breaking change to exhaustive matches.Approach
It sits exactly where
_parse_hostedsits. Native conversion runs first; onNeedsOcrError, ifocr="llm", the newpython/anydoc/_llm_ocr.pytakes over:pypdfium2atANYDOC_OCR_DPI(200) to PNGlitellm.completionper page — a text instruction plus the page as a base64image_urldata URIThreadPoolExecutor(ANYDOC_OCR_PAGE_CONCURRENCY, 4);pool.mapkeeps them in order_parse_hostedlitellm,pypdfium2andpydantic-settingsare an optional extra —pip install "firecrawl-anydoc[llm]"— imported lazily, so the base wheel keeps its zero-dependency install.Configuration
Environment only, read through a
pydantic-settingsmodel withenv_prefix="ANYDOC_OCR_". The API key is aSecretStr, unwrapped only at thelitellm.completioncall.ANYDOC_OCR_MODELopenai/gpt-4o-mini,ollama/llama3.2-visionANYDOC_OCR_API_BASEANYDOC_OCR_API_KEYOPENAI_API_KEY, …)ANYDOC_OCR_PROMPTANYDOC_OCR_DPIANYDOC_OCR_MAX_PAGESANYDOC_OCR_PAGE_CONCURRENCYANYDOC_OCR_TIMEOUTto_markdown(..., model=...)overridesANYDOC_OCR_MODELper call — the one non-secret knob convenient to pass inline.Details worth flagging
Whole document, not only the flagged pages.
NeedsOcrError.pagesnames which pages need OCR, but a mixed PDF's text pages would be dropped if only the flagged ones went to the model, and the model reads better with the whole document. Same callhostedmakes. The alternative — transcribe only the scanned pages and splice them into locally extracted text — is a real option I did not take; noted below.New error, still a
ConvertError.LlmOcrError(ConvertError), soexcept anydoc.ConvertErrorkeeps catching everything. It covers three setup failures — extra not installed (message is thepip installline),ANYDOC_OCR_MODELunset (message names the var), page count overMAX_PAGES— and wraps anylitellmexception asllm OCR: <detail>. An internalLlmOcrConfigErrorcarries the setup cases out of_llm_ocr.py; callers only ever seeLlmOcrError.No async.
litellmhasacompletion, but the binding is synchronous and the GIL is released only in the Rust layer. Page fan-out is threads..pyiuntouched.LlmOcrError,Ocrand the settings live in the Python wrapper, not the compiled module — the same split asHostedErrortoday.test_the_stubs_cover_the_moduleasserts that split and is updated for the new name.Not wired into Node / wasm / CLI. Deliberate — LiteLLM is Python. An OpenAI-compatible HTTP path in
node/anydoc.jspointed at a LiteLLM proxy would be a separate PR and a different shape.Output
anydoc.to_markdown("scan.pdf", ocr="llm")returns the model's transcription verbatim, per-page results joined by a blank line and normalised to one trailing newline. Illustrative:Tests
Three added to
python/tests/test_anydoc.py, eachskipUnlessthe extra is importable.litellm.completionis mocked;pypdfium2rasterisation and thepydantic-settingsload run for real against the committedhandmade-mixed.pdffixture:completioncalls out, each carrying animage/pngbase64 data URI; a.docxwithocr="llm"converts locally and never calls the modelANYDOC_OCR_MODELunset →LlmOcrErrornaming it, and it is aConvertErrorcompletionraising →LlmOcrErrorcarrying the causeA
llm_envcontext manager saves, clears and restoresANYDOC_OCR_*around each, the wayhosted_stubdoes forFIRECRAWL_*.No integration test against a real model or a container — matching how
hostedis tested (loopback stub, never realapi.firecrawl.dev).python -m unittest discover -s tests— 14 passed (3 new, 0 skipped with the extra installed).cargo testandnode --testnot run: the Rust core, Node and wasm sources are untouched.Registered in
Ocrliteral,__all__,python/pyproject.tomloptional-dependencies,python/README.md(Scanned pages + error table), rootREADME.mdOCR section. NoFormatvariant, so no binding-stub edits and no breaking change.Open questions
hosted. Keeping local extraction for the text pages and sending only the flagged ones would change_parse_llmand the assembly test.model=is the only kwarg. Happy to surfaceapi_base/promptinline too; kept the surface minimal and the secret out of the signature.LlmOcrSettingsvisibility. Internal right now. Export it if callers should build or introspect it.Summary by cubic
Add
ocr="llm"to the Python bindings so scanned PDFs can be transcribed by a vision model of your choice via LiteLLM, alongside the existingocr="hosted".ocr="llm"rasterizes every page and sends each to a vision model through LiteLLM, returning the model's Markdown transcription. The whole document is sent, not just the flagged pages.llmextra (pip install "firecrawl-anydoc[llm]") and configuration viaANYDOC_OCR_*environment variables (ANYDOC_OCR_MODELis required).model=overridesANYDOC_OCR_MODELper call.LlmOcrError(aConvertError), covering missing extra, missing config, and model call failures.Written for commit fdc7f3f. Summary will update on new commits.