fix(proxy): resolve provider from deployment for multi-provider defaultconfig (#27516) - #27517
Conversation
…ltconfig (BerriAI#27516) When the user-facing model name has no provider prefix (e.g. "claude-sonnet-4.6" instead of "bedrock/..."), _apply_credential_overrides_from_model_config left the provider hint as None, which caused _extract_credential_from_entry to fall through to the first credential in the dict — wrong whenever defaultconfig has more than one provider entry. Fix: when the request model has no '/', look up the deployment via llm_router.get_deployment_by_model_group_name() and resolve the provider from deployment.litellm_params.custom_llm_provider (or, as a fallback, the prefix of deployment.litellm_params.model). Both the post-alias and pre-alias names are tried. Single-provider defaultconfig entries (the historical use case) keep working unchanged because _extract_credential_from_entry still falls back to the only entry when there is no hint. Tests cover the new helper, the regression scenario from BerriAI#27516 (gemini+bedrock defaultconfig with 'claude-sonnet-4.6'), and the bypass when the request already has a provider/... prefix.
Greptile SummaryThis PR fixes a silent mis-routing bug in
Confidence Score: 4/5Safe to merge for the common case; the fix is additive and well-tested, with the only gaps being an unlogged swallowed exception and an undocumented first-deployment assumption that matters only in atypical multi-provider-per-group-name router configs. The core logic is correct and the new code path is only activated when a router is provided and the model name lacks a provider prefix. The two concerns — bare litellm/proxy/litellm_pre_call_utils.py around the exception handling in
|
| Filename | Overview |
|---|---|
| litellm/proxy/litellm_pre_call_utils.py | Adds _resolve_provider_from_deployment helper and threads llm_router through _apply_credential_overrides_from_model_config; the logic correctly falls back to the first deployment's provider and is gated behind llm_router is not None, preserving existing behaviour. Minor concerns: swallowed router exceptions are unlogged, and the first-deployment-only assumption is undocumented. |
| tests/test_litellm/proxy/test_litellm_pre_call_utils.py | Adds seven new mock-only tests covering the happy path, custom_llm_provider preference, no-match, router exception, pre-alias fallback, multi-provider regression, legacy single-provider, and provider-prefix-skips-router cases. No existing tests are modified. All tests use MagicMock; no real network calls. |
Reviews (1): Last reviewed commit: "fix(proxy): resolve provider from deploy..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| except Exception: | ||
| deployment = None |
There was a problem hiding this comment.
The bare except Exception: deployment = None discards every error from get_deployment_by_model_group_name without any log output. If the router raises unexpectedly (e.g., due to a programming error, corrupted model list, or wrong return type), the provider hint silently becomes None and credential selection silently falls back to dict-insertion-order — the very bug this PR is fixing. A verbose_proxy_logger.debug or warning call here would make production failures diagnosable without changing the graceful-fallback semantics.
| for name in candidates: | ||
| try: | ||
| deployment = llm_router.get_deployment_by_model_group_name( | ||
| model_group_name=name | ||
| ) | ||
| except Exception: | ||
| deployment = None | ||
| if deployment is None: | ||
| continue | ||
|
|
||
| litellm_params = getattr(deployment, "litellm_params", None) | ||
| if litellm_params is None: | ||
| continue | ||
|
|
||
| custom_provider = getattr(litellm_params, "custom_llm_provider", None) | ||
| if custom_provider: | ||
| return custom_provider | ||
|
|
||
| deployment_model = getattr(litellm_params, "model", "") or "" |
There was a problem hiding this comment.
get_deployment_by_model_group_name always returns model_list[indices[0]] — the first registered deployment for a model group name. If an operator configures the same model group name (e.g. claude-sonnet-4.6) with deployments across multiple providers (bedrock in one region, anthropic direct in another), the returned provider hint depends solely on registration order, not on which deployment would actually be selected at call time. For the scenario in the bug report (single provider per group name), this is fine, but the assumption is invisible to callers and could silently mis-select credentials if the router configuration changes. A code comment or docstring note that the method returns only the first deployment would make the limitation explicit.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
@Anai-Guo — could you add a screenshot or short video showing that this change works as expected? It really helps reviewers verify the fix quickly. Thanks! |
|
Hi @krrish-berri-2 — happy to share. This is a pure provider-routing fix on the Python side (no UI surface), so the cleanest "proof" is the unit test plus a minimal config-only repro. Let me show both: 1. Unit test output (the regression test from this PR)The test sets up a 2. Minimal config-only repro (what the original reporter would see)
model_list:
- model_name: claude-sonnet-4.6
litellm_params:
model: bedrock/us.anthropic.claude-sonnet-4-6
custom_llm_provider: bedrock
team_metadata:
model_config:
defaultconfig:
gemini: { litellm_credentials: gemini-team-1 } # <-- listed first
bedrock: { litellm_credentials: bedrock-team-1 }
The only behavioural change is for the previously-broken multi-provider case where the request model has no Let me know if you'd prefer a recorded terminal session instead — happy to capture one. |
0f908e6
into
BerriAI:shin_agent_oss_staging_05_09_2026
|
🤖 litellm-agent: Squash-merged into staging branch Triage Summary 227 lines across 2 files (+226 / -1) Merge Confidence: 5/5 ✅ READY All checks green. Greptile 4/5, no blocking pattern findings, no CircleCI runs (OSS-typical). |
…ltconfig (BerriAI#27516) (BerriAI#27517) Squash-merged by litellm-agent from Anai-Guo's PR.
3 participants
Summary
Closes #27516.
When a request's
modelfield has no provider prefix (e.g.claude-sonnet-4.6),_apply_credential_overrides_from_model_configleft the provider hint asNone._extract_credential_from_entrythen fell through to "first credential in the dict" — which silently routes to the wrong provider wheneverdefaultconfighas more than one provider entry.Repro (from the bug report)
curl /v1/chat/completions -d '{"model": "claude-sonnet-4.6", ...}'Before: gemini-team-1 (the first dict entry) gets used → Bedrock rejects the Gemini key.
After: bedrock-team-1 is selected, because the deployment for
claude-sonnet-4.6declaresbedrock.Approach
Option A from the issue: when the request model has no
/, look up the deployment viallm_router.get_deployment_by_model_group_name()and resolve the provider fromdeployment.litellm_params.custom_llm_provider, falling back to the prefix ofdeployment.litellm_params.model. Both the post-alias and pre-alias names are tried so team/key aliases are handled.Single-provider
defaultconfigentries (the historical case) keep working because_extract_credential_from_entrystill returns the only entry when there is no hint — so this is purely additive for the previously broken multi-provider case.Files changed
litellm/proxy/litellm_pre_call_utils.py— new_resolve_provider_from_deployment()helper, plumbed through_apply_credential_overrides_from_model_config(..., llm_router=...), and the caller inadd_litellm_data_to_requestnow passesllm_router.tests/test_litellm/proxy/test_litellm_pre_call_utils.py— unit tests for the new helper (router raises, no deployment, custom_llm_provider vs. model-prefix, pre-alias fallback) and an integration test for the exact regression scenario.Test plan
_resolve_provider_from_deploymentreturns the right value across the new unit testsazure/gpt-4) still bypasses the router lookup🤖 Generated with Claude Code