refactor(litellm-rust): dissolve providers into core + ai-gateway (strict 3-crate layers) - #31218
Conversation
…crate allowlist test
|
|
Greptile SummaryThis PR dissolves the
Confidence Score: 4/5Clean structural refactor with no logic changes; all existing tests pass and the new enforcement test adds a meaningful guardrail. The main thing to be aware of is the blocking reqwest client in an async-capable library, which is safe today but a latent footgun for future gateway callers. The refactor correctly redistributes code with no functional changes. Transforms stay pure, I/O stays async except OCR which is intentionally blocking for the Python bridge. Feature split is sound, test coverage is good, and the workspace allowlist test is a nice addition. Three non-blocking observations: the blocking reqwest Client in a public lib module could panic if a future route handler calls it directly; one test assertion is trivially true; and the hand-rolled TOML parser would misfire on a comment containing the word members. litellm-rust/crates/ai-gateway/src/io/ocr.rs — the run_ocr public function uses blocking reqwest in a library that the async gateway also links; worth documenting the panic condition clearly or restricting visibility.
|
| Filename | Overview |
|---|---|
| litellm-rust/crates/ai-gateway/src/io/ocr.rs | Moved from providers/src/ocr.rs; implements blocking OCR via reqwest. Two issues: blocking HTTP client can panic if called from an async context (future misuse risk), and the multibyte-char test assertion is trivially true. |
| litellm-rust/crates/ai-gateway/src/io/realtime.rs | Moved from providers/src/realtime.rs; async WebSocket splice logic with idle timeout. Clean refactor, good test coverage including an ignored live-API integration test. |
| litellm-rust/crates/ai-gateway/src/io/realtime_pool.rs | Pre-warmed WebSocket pool moved from providers; comprehensive unit tests with an in-process fake server, backoff logic, and liveness checks. Well-structured. |
| litellm-rust/crates/ai-gateway/src/lib.rs | New lib target for ai-gateway; cleanly gates server modules behind the server feature and python-config modules behind python-config, enabling the python-bridge to depend on only the io layer. |
| litellm-rust/crates/ai-gateway/Cargo.toml | Adds lib target; splits axum/serde/subtle behind server feature; reqwest and tokio-tungstenite become unconditional for the io layer. Feature split is sound. |
| litellm-rust/crates/core/tests/workspace_crate_allowlist.rs | New enforcement test; verifies workspace has exactly 3 crates. Hand-rolled TOML parser is fragile to comments containing the word "members" but safe for the current manifest shape. |
| litellm-rust/crates/core/src/providers/mistral/ocr/transformation.rs | Moved from providers crate; pure transform logic for Mistral OCR requests and responses. Well-tested with comprehensive unit tests. |
| litellm-rust/crates/core/src/providers/openai/realtime/transformation.rs | Moved from providers crate; pure passthrough transforms for OpenAI realtime events + URL construction with percent-encoding. Correct handling of scheme swapping and multibyte chars. |
| litellm-rust/crates/python-bridge/src/lib.rs | Import updated from litellm_providers to litellm_ai_gateway::io::ocr::run_ocr; otherwise unchanged. |
| litellm-rust/Cargo.toml | Removes providers crate from workspace members and replaces the workspace dependency with litellm-ai-gateway (default-features = false). |
Comments Outside Diff (2)
-
litellm-rust/crates/ai-gateway/src/io/ocr.rs, line 35-43 (link)reqwest::blockingin an async-capable library craterun_ocrusesreqwest::blocking::Client, which panics with "Cannot block the current thread from within a Tokio runtime" if called from any async context. Becauselitellm_ai_gateway::io::ocris a public, always-on module in the library that the async gateway also depends on, a future route handler that callsrun_ocrdirectly (withouttokio::task::spawn_blocking) will silently panic at runtime rather than failing to compile. The doc comment says "Blocking: intended to be called with the GIL released", but this constraint is invisible to callers inside the gateway crate. Consider adding a# Panicsrustdoc section that explicitly names the runtime constraint, or restricting visibility to only the Python bridge. -
litellm-rust/crates/ai-gateway/src/io/ocr.rs, line 127-133 (link)Trivially-true assertion doesn't exercise the intended invariant
truncated.is_char_boundary(truncated.len())is alwaystruefor any validString— the end position of aStringis always a char boundary by definition. The test never actually fails for a naively byte-sliced string because the panic from invalid UTF-8 would come at.collect::<String>(), not at the assertion. To actually assert the truncation didn't split a multibyte sequence, the test should verify the content of the prefix — for example that it contains exactlyERROR_BODY_MAX_CHARSchars and that each char is the expected character.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!
Reviews (1): Last reviewed commit: "refactor(litellm-rust): update workspace..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let open = after_members.find('[').expect("members should be an array"); | ||
| let close = after_members[open..] | ||
| .find(']') | ||
| .map(|offset| open + offset) | ||
| .expect("members array should be closed"); | ||
| let body = &after_members[open + 1..close]; | ||
|
|
||
| let mut members = BTreeSet::new(); | ||
| let mut rest = body; | ||
| while let Some(start) = rest.find('"') { | ||
| let after_quote = &rest[start + 1..]; | ||
| let end = after_quote | ||
| .find('"') | ||
| .expect("opening quote should be matched"); | ||
| members.insert(after_quote[..end].to_string()); | ||
| rest = &after_quote[end + 1..]; | ||
| } | ||
| members | ||
| } | ||
|
|
||
| /// The immediate subdirectory names under `crates/`. | ||
| fn crate_dirs(root: &Path) -> BTreeSet<String> { | ||
| fs::read_dir(root.join("crates")) | ||
| .expect("crates/ directory should exist") |
There was a problem hiding this comment.
split_once("members") matches the first substring, including comments
parse_members calls manifest.split_once("members") which will match the very first occurrence of the substring members anywhere in the file — including a TOML comment like # workspace members. If a comment containing the word "members" is ever added before the actual members = [...] key, the parser will consume the wrong slice and the resulting BTreeSet will either be empty or contain garbage, causing the allowlist test to fire a false alarm (or silently pass with wrong data). A more robust anchor would be to split on the literal members = [ or members=[.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
3 participants
Relevant issues
Locks
litellm-rustto a strict 3-crate layer structure before more providers/routes pile up. The workspace had drifted to 4 crates whereprovidersmixed pure transforms with network I/O. This dissolvesprovidersso each crate is a clean layer: translation / routes+I/O / Python binding. (Supersedes #31143, which predated theai-gatewaycrate.)Pre-Submission checklist
crates/core/tests/workspace_crate_allowlist.rs) + existing transform/route tests still passcargo test --workspace→ 39 passed, 1 ignored (the ignored one is the pre-existing live-OpenAI realtime test)cargo build -p litellm-ai-gateway --features serverandcargo build -p litellm-python-bridgeboth green; clippy-D warnings+fmt --checkcleanType
🧹 Refactoring
Changes
Dissolves the
providerscrate into the two layers it was straddling:litellm-corecore/src/providers/{mistral,openai}), routerlitellm-ai-gatewayai-gateway/src/io/{ocr,realtime,realtime_pool}) + the axum server (behindserverfeature)litellm-python-bridgeWhat moved:
mistral,openai)providers/→core/src/providers/ocr.rs,realtime.rs,realtime_pool.rs)providers/→ai-gateway/src/io/python-bridgenow importslitellm_ai_gateway::io::ocr::run_ocr;providerscrate deletedai-gatewaygains a lib target + feature split so the cdylib stays lean: theiolayer is always available, while axum + the server binary sit behindserver(required-features = ["server"]), and the existing embedded-CPython config stays behindpython-config.python-bridgedepends on it withdefault-features = false→ builds with no axum.Guardrails:
crates/core/tests/workspace_crate_allowlist.rsfails CI if the crate set drifts from the 3 allowlisted crates (so a revivedprovidersis caught).README.md+crates/ai-gateway/README.md; rule + table inAGENTS.md(top + per-crate);CLAUDE.mdboundary refreshed.Note: kept
litellm-rust/.cargo/config.toml(pyo3-undefined dynamic_lookup, apple-darwin only) so the cdylib links under plaincargo buildon macOS. No effect on Linux/CI or crate logic.