Repository navigation
fix(ocr): keep inlined remote documents when the host restores retained fields - #41716
devin-ai-integration[bot] wants to merge 2 commits into
Conversation
…ed fields The Rust OCR path listed "document" in retained_fields unconditionally, so the Python bridge swapped the caller's raw document_url back into bodies that Azure AI, Vertex Mistral and Azure cohere_parse had already inlined, and their data-URI validation rejected every remote document. Retain the document only when the composed body still sends the caller's source, and cover the three inlining adapters at the facade
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6a9f6ff. Configure here.
… a legacy Logging adapter Extracted from BerriAI#41733 without the router loop, the cache machine layer, streaming, or the error, timeout and route-pruning work that moved to BerriAI#41745 litellm-callbacks holds the contract a native call and its host share: Machine, HostOp, CallEvent, the in-process run loop, and Passthrough, which is built only by comparing the caller's inputs with the body the route sends, so a route can never mark a key it rewrote. litellm-host-python (formerly python-interop) owns the CPython driver and the Execution handle, and litellm-callbacks-legacy is the @client wrapper as the native call sees it: function_setup, the deployment hooks, pre_call and post_call, the success and failure fan-out and the deferred proxy release. OCR is the one route on it, and the old core and bridge lifecycles are gone The passthrough rule is the structural fix for the bug BerriAI#41719 patched in core and BerriAI#41716 reworks: an inlined remote document no longer counts as the caller's value, so the legacy adapter never hands the caller's URL back into the body. core/tests/ocr/passthrough.rs pins it for every route and document source, including that unchanged values stay passthrough, and callbacks-legacy/tests/payload.rs pins the adapter side with a real pre_call callback Python OCR integration tests that only exercised core behavior now live as Rust tests, so tests/test_litellm_rust keeps the cases that need the full Python stack
TLDR
Problem this solves:
document_urlsince refactor(ocr): mirror Python provider layout and preserve tests #41550 ("invalid OCR document data URI")ocr_testingjob has been red on every branch since dc81cf5How it solves it:
User Flow
Before: a developer running OCR through the proxy against an Azure AI Foundry Mistral Document AI deployment sends a public PDF link and gets a 400 back, with no way to make the link work
azure_ai/mistral-document-ai-2512to the config asazure-ocrwith the Foundry api_base and key, and starts the proxy{"model":"azure-ocr","document":{"type":"document_url","document_url":"https://cdn.jsdelivr.net/gh/BerriAI/litellm@d769e81c/tests/llm_translation/fixtures/dummy.pdf"}}{"error":{"message":"litellm.BadRequestError: Azure_aiException - invalid OCR document data URI. Received Model Group=azure-ocr ...","type":"invalid_request_error","code":"400"}}back in about a second, and nothing in the error says what a valid document would look likedata:application/pdf;base64,...returns 200, so the only workaround is downloading and base64-encoding every document client-sideAfter: the same PDF link is accepted and the developer gets the OCR pages back
azure_ai/mistral-document-ai-2512to the config asazure-ocrwith the Foundry api_base and key, and starts the proxy{"model":"azure-ocr","document":{"type":"document_url","document_url":"https://cdn.jsdelivr.net/gh/BerriAI/litellm@d769e81c/tests/llm_translation/fixtures/dummy.pdf"}}"pages":[{"index":0,"markdown":"Test PDF File",...}],"usage_info":{"pages_processed":1,"doc_size_bytes":589}and"object":"ocr", the same response the data URI call returnedRelevant issues
Regression from #41550
Affected release
Linear ticket
Resolves LIT-8058
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
/qa run at PR tip 75ca8af (merge base 4c70cb4). Every checkpoint below booted its own worktree venv the way CI's
ocr_testingjob builds it (uv sync --frozen --all-groups --all-extras --python 3.12, so the Rust OCR path is on,native_bridge_available()printedTrueat every checkpoint), started a no-DB proxy with--num_workers 2on a random free port, and hit the real Azure AI Foundrymistral-document-ai-2512deployment (real $$$). This path holds no per-process state, so a single two-worker instance per checkpoint is the topology.run_test.pyloads the.env(AZURE_API_KEY, AZURE_API_BASE) and callspytest.mainwith the rest of argv. Proxy config at every checkpoint:Commit 6a9f6ff landed after this run and only rewrites four Rust tests plus their test-support builder (every hunk sits under
#[cfg(test)]), so it cannot change behavior and the proof keeps the hash it ran at.Regression table
Case 1 is
POST /v1/ocrwith a public PDF URL, case 2 the same PDF inlined as a data URI (control), case 3 CI's owntests/ocr_tests/test_ocr_azure_ai.py::TestAzureAIOCR::test_basic_ocr_with_url(both params).Before (4c70cb4, port 63900)
Case 1, run twice, byte-identical output both times:
Case 2, the data URI control, so the deployment itself is healthy:
Case 3, CI's own test from inside the worktree:
After (75ca8af, port 50844)
Case 1, same command against port 50844, run twice, byte-identical output both times:
usage_info.doc_size_bytes589 matches the fetched dummy.pdf, so the provider received the inlined document, not the URL.Case 2, the data URI control, same command against port 50844:
Case 3, CI's own test, same command:
The two regression-side checkpoints (1e7b03a and dc81cf5) ran the same three cases with the same commands and produced the table rows above: the first parent of #41550 accepted the URL (200, "Test PDF File", 2 passed) and the #41550 merge rejected it with the same
invalid OCR document data URI400 the merge base shows, with the data URI control at 200 on both.Surprises
base_ocr_unit_tests.py:47prints the api_key into captured stdout; PR leaves it aloneproject_idin credentials; identical both sides, pre-existingType
🐛 Bug Fix
Caveats (if any)
Low
mistral-ocrenabled, a fix for the pre-existing Rust Vertex auth gap (a credentials file withoutproject_idfails before any call, identical on base and head, filed separately), and a cohere parse Foundry deployment, which outweighs what the gap costspre_ocrandpost_ocrlogging payloads now carry the inlined data URI for remote documents on inlining providers, matching the wire body and the pre-refactor(ocr): mirror Python provider layout and preserve tests #41550 Python-path behavior; the URL is still what the caller sent and what non-inlining providers (Mistral) log. Verified live with a payload-recording callback on base, head, and head withLITELLM_RUST=0ci/circleci: litellm_utils_testing,llm_translation_testing, andpass_through_unit_testingare red on this PR and fail the same way on main's own scheduled pipeline; none of them run OCR code. Not required checksFinal Attestation