feat: remove litellm dependency and bridge path#455
Conversation
- Delete litellm_bridge.py adapter, litellm_overrides.py, and their tests - Remove LiteLLM fallback branch and DATA_DESIGNER_MODEL_BACKEND env var from clients/factory.py; unknown provider_type now raises ValueError - Remove apply_litellm_patches() call from models/factory.py - Remove LiteLLM exception match arms and DownstreamLLMExceptionMessageParser from models/errors.py; port context window detail extraction to _extract_context_window_detail for native ProviderError path - Remove litellm from lazy_heavy_imports.py and pyproject.toml runtime deps - Remove flatten_extra_body parameter from TransportKwargs.from_request - Clean up LiteLLM references in docstrings, comments, and AGENTS.md - Add full ProviderErrorKind test coverage to test_model_errors.py - Update benchmark script to patch OpenAICompatibleClient instead of CustomRouter Made-with: Cursor
Greptile SummaryThis PR is the seventh in the model-facade overhaul series and completes the LiteLLM removal: it deletes the bridge adapter ( Key changes:
|
| Filename | Overview |
|---|---|
| packages/data-designer-engine/src/data_designer/engine/models/clients/factory.py | Removes _create_bridge_client, DATA_DESIGNER_MODEL_BACKEND env-var, and LiteLLM bridge fallback; unknown provider_type now raises DataDesignerError with a clear message. Docstring and Raises section are consistent with the code. |
| packages/data-designer-engine/src/data_designer/engine/models/errors.py | Removes all LiteLLM exception match arms and DownstreamLLMExceptionMessageParser; ports context-window detail extraction to new private _extract_context_window_detail helper. Logic is correct — uses case-insensitive search, extracts on original-case text, and handles the " Please reduce " split boundary. |
| packages/data-designer-engine/tests/engine/models/test_model_errors.py | Comprehensive replacement of LiteLLM exception test cases with ProviderError-based parametrized cases covering all ProviderErrorKind values; adds dedicated context-window detail tests (with and without OpenAI-style token-count text). Test IDs are descriptive and coverage is complete. |
| scripts/benchmarks/benchmark_engine_v2.py | Patches OpenAICompatibleClient.completion/acompletion instead of CustomRouter; introduces FakeCompletionResponse wrapping the canonical ChatCompletionResponse type; the previously flagged tools forwarding regression is now fixed. Class-level patching is correctly restored in the finally block. |
| packages/data-designer-engine/src/data_designer/engine/models/clients/types.py | Removes flatten_extra_body parameter and its False branch from TransportKwargs.from_request; body construction now always merges extra_body keys into the top level. Clean simplification with corresponding test removal. |
| packages/data-designer-engine/tests/engine/models/clients/test_factory.py | Removes all bridge-related tests and env-var override tests; adds test_unknown_provider_type_raises_data_designer_error which correctly validates DataDesignerError with a regex match. |
| packages/data-designer-engine/pyproject.toml | Removes litellm>=1.77.0,<1.80.12 from runtime dependencies; uv.lock is updated to drop the litellm and its transitive deps (distro, fastuuid, etc.). |
| plans/343/model-facade-overhaul-pr-7-architecture-notes.md | New architecture notes document for PR-7. Contains a minor discrepancy: multiple places state that unknown provider_type raises a ValueError, but the actual code (and its docstring and test) raise DataDesignerError. Not a runtime issue, but the notes are slightly stale on this point. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[create_model_client called] --> B{provider_type?}
B -->|"openai"| C[OpenAICompatibleClient]
B -->|"anthropic"| D[AnthropicClient]
B -->|"other / unknown"| E[DataDesignerError raised ✦ NEW]
C --> F{throttle_manager?}
D --> F
F -->|yes| G[ThrottledModelClient wrapper]
F -->|no| H[Return raw client]
subgraph REMOVED ["🗑️ Removed paths"]
R1["DATA_DESIGNER_MODEL_BACKEND=litellm_bridge → LiteLLMBridgeClient"]
R2["Unknown provider_type → _create_bridge_client fallback"]
end
style REMOVED fill:#ffeeee,stroke:#ff6666
style E fill:#fff3cd,stroke:#ffc107
Prompt To Fix All With AI
This is a comment left during a code review.
Path: plans/343/model-facade-overhaul-pr-7-architecture-notes.md
Line: 2382-2384
Comment:
**Architecture notes say `ValueError`, code raises `DataDesignerError`**
The notes state "unknown `provider_type` values raise a `ValueError`" in multiple places, but the actual implementation in `clients/factory.py` raises `DataDesignerError`, and the test (`test_unknown_provider_type_raises_data_designer_error`) correctly asserts `DataDesignerError`. The function's own `Raises:` docstring also documents `DataDesignerError`. The architecture notes are slightly stale on this point — worth a one-line fix so the planning docs stay accurate for future readers.
```suggestion
After this PR, unknown `provider_type` values raise a
`DataDesignerError` with a clear message listing supported types.
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (4): Last reviewed commit: "fix: address PR-7 review feedback" | Re-trigger Greptile
The old CustomRouter patch forwarded **kwargs (including tools) to _fake_response, but the new OpenAICompatibleClient patch only passed model and messages — silently disabling tool-call simulation in benchmark scenarios that exercise allow_tools. Made-with: Cursor
...es/data-designer-engine/src/data_designer/engine/dataset_builders/utils/async_concurrency.py
Show resolved
Hide resolved
suggestion: the |
...es/data-designer-engine/src/data_designer/engine/dataset_builders/utils/async_concurrency.py
Outdated
Show resolved
Hide resolved
|
Resolved in 1f0a36d — removed the |
johnnygreco
left a comment
There was a problem hiding this comment.
Review comment on factory.py — see inline.
packages/data-designer-engine/src/data_designer/engine/models/clients/factory.py
Show resolved
Hide resolved
andreatgretel
left a comment
There was a problem hiding this comment.
This looks great, approving!
One small follow-up I’d still like to clean up: provider_type is still free text in config and the CLI, so a typo or an old custom value can get saved and only fail later when preview/generation tries to build the client. Not a blocker here, but it’d be nice to validate that earlier in a follow-up PR!
- Return ChatCompletionResponse from benchmark fakes instead of FakeResponse to match the native client contract (facade expects .message, not .choices[0].message) - Add ids= to parametrize block in test_model_errors.py for readability - Remove unnecessary try/except from _extract_context_window_detail; the `if marker in` guard is sufficient - Make context window marker match case-insensitive - Replace stale httpx.AsyncClient callout in async_concurrency.py docstring with generic "async-stateful resources" Made-with: Cursor
1f0a36d to
2c82515
Compare
Let's explore this in PR-8. |
johnnygreco
left a comment
There was a problem hiding this comment.
thanks for jumping on this @nabinchha!!!
|
@nabinchha curious - what was litellm missing to solve your problem well? |
Hey @krrish-berri-2 — thanks for reaching out, and for building LiteLLM. It was really helpful early in the project. This move started several weeks ago as we worked through what the project needed as it matured:
For us, the tradeoff shifted once we needed tight control over transport lifecycle and throttling at the adapter level. |
|
Hey @nabinchha, thank you for the feedback.
Do you still see this? I thought we'd made several improvements on this in the past few months. |
📋 Summary
Seventh PR in the model facade overhaul series (plan, architecture notes). Removes the
litellmruntime dependency, bridge adapter, and all associated dead code paths. With PR-6 merged, all providers route to native HTTP adapters by default — the LiteLLM bridge was a migration safety net that is no longer needed.Previous PRs:
🔄 Changes
🗑️ Removed
litellm_bridge.py— bridge adapter wrapping LiteLLM's router behind theModelClientprotocollitellm_overrides.py—ThreadSafeCache,CustomRouter,apply_litellm_patches, and image URL schema patchlitellmruntime dependency frompyproject.tomlandlazy_heavy_imports.py(net ~1,470 lines removed)DATA_DESIGNER_MODEL_BACKENDenv-var escape hatch and_create_bridge_clienthelper fromclients/factory.pyDownstreamLLMExceptionMessageParserfrommodels/errors.pyapply_litellm_patches()call frommodels/factory.pyflatten_extra_bodyparameter fromTransportKwargs.from_request(only needed by bridge)test_litellm_bridge.py,test_litellm_overrides.pymock_router,bridge_clientfromclients/conftest.py🔧 Changed
clients/factory.py— unknownprovider_typenow raisesValueError(previously fell back to LiteLLM bridge)models/errors.py— ported context window detail extraction fromDownstreamLLMExceptionMessageParserto new_extract_context_window_detailhelper in the nativeProviderErrorpath; 403 now correctly maps toModelPermissionDeniedError(wasModelAuthenticationErrorin LiteLLM era)benchmark_engine_v2.py— patchesOpenAICompatibleClient.completion/acompletioninstead ofCustomRouter✨ Added
ProviderErrorKindtest coverage intest_model_errors.py— parametrized cases forAUTHENTICATION,API_CONNECTION,TIMEOUT,NOT_FOUND,INTERNAL_SERVER,UNPROCESSABLE_ENTITY,API_ERROR, and multimodalBAD_REQUESTValueErrortest intest_factory.py📚 Docs
AGENTS.md, andREADME.md🔍 Attention Areas
models/errors.py—_extract_context_window_detailports parsing logic from the removedDownstreamLLMExceptionMessageParser; verify the nativeProviderError.messagecarries the same detail the old LiteLLM exception didclients/factory.py— the only behavioral change: unknownprovider_typeraisesValueErrorinstead of silently falling back to LiteLLM bridgetest_model_errors.py— new parametrized test cases ensure everyProviderErrorKindis covered after the LiteLLM match arms were removedTest plan
uv run ruff checkon all changed source filesuv run pytest tests/engine/models/— all model error, factory, and parsing tests passlitellmno longer appears inuv.locktransitive depsimport litellmfails in the venv (not installed)make test-run-all-examplespasses without LiteLLM🤖 Generated with Cursor
Made with Cursor