feat: package Rust OCR bridge in LiteLLM wheel - #31265
Conversation
|
|
Greptile SummaryThis PR packages the Rust OCR bridge directly into the
Confidence Score: 3/5The Python and Rust logic changes are correct, but the pyproject.toml change inadvertently removes an explicit exclusion that kept the enterprise directory out of the OSS wheel; this should be resolved before merging. The new loader, protocol types, and test patches are all well-constructed. The riskiest part of the change is pyproject.toml: the old pyproject.toml — the missing
|
| Filename | Overview |
|---|---|
| pyproject.toml | Switches build backend from uv_build to maturin; removes source-exclude rules including the enterprise directory exclusion, risking bundling enterprise code into the OSS wheel. |
| litellm/rust_bridge/init.py | New thin package that re-exports get_native_bridge and native_bridge_available from the loader; clean and minimal. |
| litellm/rust_bridge/loader.py | New lazy loader for the compiled _native extension; correctly returns None on ImportError to preserve Python fallback. |
| litellm/ocr/rust_bridge.py | Re-routes OCR bridge loading through litellm.rust_bridge.get_native_bridge() instead of the former top-level litellm_python_bridge import; fallback behaviour unchanged. |
| Dockerfile | Adds rust to Alpine builder stage and embeds a Python smoke test for the native bridge; test is duplicated verbatim in the non-root Dockerfile. |
| docker/Dockerfile.non_root | Same Rust toolchain addition and identical smoke-test block as main Dockerfile; duplication is the only concern. |
| tests/test_litellm/ocr/test_rust_bridge.py | Test improvements: replaces brittle sys.modules injection with monkeypatch.setattr on get_native_bridge; adds missing monkeypatch for load_rust_ocr in the fallback test; no coverage regressions detected. |
| litellm-rust/crates/python-bridge/Cargo.toml | Renames the cdylib from litellm_python_bridge to _native to match the new litellm.rust_bridge._native module path. |
| litellm-rust/crates/python-bridge/src/lib.rs | Updates the #[pymodule] initializer name to _native; trivial rename, no logic change. |
| .dockerignore | Adds litellm-rust/target/ to prevent Rust build artifacts from entering the Docker build context; correct and necessary. |
Reviews (1): Last reviewed commit: "feat: package rust ocr bridge in litellm..." | Re-trigger Greptile
| [tool.maturin] | ||
| manifest-path = "litellm-rust/crates/python-bridge/Cargo.toml" | ||
| module-name = "litellm.rust_bridge._native" | ||
| python-source = "." | ||
| bindings = "pyo3" |
There was a problem hiding this comment.
Enterprise directory will be bundled into the OSS wheel
The old [tool.uv.build-backend] explicitly excluded litellm/proxy/enterprise from the built package. That block was removed when switching to maturin, but no equivalent exclude was added to [tool.maturin]. litellm/proxy/enterprise/ has an __init__.py and is tracked by git, so maturin (with python-source = ".") will include litellm.proxy.enterprise as a subpackage of the OSS wheel — the inverse of the old behaviour. Add an explicit exclude to [tool.maturin]: exclude = ["litellm/proxy/enterprise/**"]
| RUN python - <<'PY' | ||
| import importlib | ||
| import os | ||
|
|
||
| from litellm.ocr.rust_bridge import load_rust_ocr | ||
| from litellm.rust_bridge import native_bridge_available | ||
|
|
||
| os.environ.pop("MISTRAL_API_KEY", None) | ||
|
|
||
| native = importlib.import_module("litellm.rust_bridge._native") | ||
| assert native_bridge_available() | ||
| assert load_rust_ocr() is native.ocr | ||
|
|
||
| try: | ||
| native.ocr( | ||
| model="mistral-ocr-latest", | ||
| document={"type": "document_url", "document_url": "https://example.com/doc.pdf"}, | ||
| api_key=None, | ||
| api_base=None, | ||
| custom_llm_provider="mistral", | ||
| extra_headers={}, | ||
| optional_params={}, | ||
| timeout_seconds=1.0, | ||
| ) | ||
| except ValueError as exc: | ||
| assert "Missing Mistral API Key" in str(exc) | ||
| else: | ||
| raise AssertionError("expected Rust OCR bridge to reject missing Mistral API key") | ||
| PY | ||
|
|
||
| RUN find /app/.venv -type f -path "*/tornado/test/*" -delete && \ | ||
| find /app/.venv -type d -path "*/tornado/test" -delete |
There was a problem hiding this comment.
Identical smoke-test script copy-pasted into both Dockerfiles
The same 30-line Python block appears verbatim in Dockerfile and docker/Dockerfile.non_root. The two copies will drift if the Rust error message, function signature, or import path ever changes — a single edit site would be missed. Consider extracting the script to docker/smoke_test_rust_bridge.py and referencing it with COPY + RUN python docker/smoke_test_rust_bridge.py (or equivalent) in both files.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
3aea24e to
1ffe1d8
Compare
|
Addressed Greptile feedback and reduced PR scope: retargeted/rebased onto |
|
Superseded by #31267. Recreated from BerriAI/litellm:litellm_hotfix_rust_pip_binary so the source-branch guard passes. |
Summary
Synced directly on top of
litellm_internal_stagingThis makes the default
litellmpip package build and ship the Rust OCR bridge inside the existing Python package instead of publishing a separate Rust wheel packagematurinand builds the PyO3 crate aslitellm.rust_bridge._nativelitellm.rust_bridgeloader package so Python code does not import a top-level native moduleuv_buildsource excludes into[tool.maturin], includinglitellm/proxy/enterprisecargo testiflitellm-rust/crates/core/srcuses rawserde_json::Value,serde_json::Map, orjson!Related: #31263
Validation
uv lockuv build --sdist --wheellitellm/rust_bridge/loader.py, and excludelitellm/proxy/enterpriseuv run black litellm/rust_bridge litellm/ocr/rust_bridge.py tests/test_litellm/ocr/test_rust_bridge.pyuv run ruff check litellm/rust_bridge litellm/ocr/rust_bridge.py tests/test_litellm/ocr/test_rust_bridge.pyuv run pytest tests/test_litellm/ocr/test_rust_bridge.py -q(18 passed)(cd litellm-rust && cargo fmt --check && cargo clippy --workspace --all-targets --locked -- -D warnings && cargo test --workspace --locked)cargo test --workspace --lockedincludestyped_core_boundary::core_translation_code_does_not_use_raw_json_valuesdocker build -t litellm-rust-pip-binary-test .docker run --rm --entrypoint python litellm-rust-pip-binary-test -c "from litellm.rust_bridge import native_bridge_available; from litellm.ocr.rust_bridge import load_rust_ocr; import litellm.rust_bridge._native as native; assert native_bridge_available(); assert load_rust_ocr() is native.ocr; print('main docker rust bridge ok')"docker build -f docker/Dockerfile.non_root -t litellm-rust-pip-binary-non-root-test .docker run --rm --entrypoint python litellm-rust-pip-binary-non-root-test -c "from litellm.rust_bridge import native_bridge_available; from litellm.ocr.rust_bridge import load_rust_ocr; import litellm.rust_bridge._native as native; assert native_bridge_available(); assert load_rust_ocr() is native.ocr; print('non-root docker rust bridge ok')"git merge-tree $(git merge-base HEAD upstream/litellm_internal_staging) HEAD upstream/litellm_internal_stagingshowed no conflictsType
New Feature
Bug Fix
Test