fix(router): duration-only provider budget config no longer removes deployments - #33366
Conversation
… reasoning_effort gpt-5.6 family models fail on /v1/chat/completions when function tools are present because OpenAI applies a default reasoning_effort server-side. Bridge to /v1/responses unconditionally for is_model_gpt_5_4_plus_model with tools. Fixes: BerriAI#33221
…eployments When provider_budget_config has budget_duration but no max_budget, the deployment was silently removed from the healthy set due to a misplaced continue statement advancing the deployment loop. Fix: guard the budget check with config.max_budget is not None so a missing max_budget means no numeric cap and the deployment stays eligible. Fixes BerriAI#33327
Greptile SummaryThis PR bundles two independent fixes: correcting
Confidence Score: 3/5The production logic fix in The core
|
| Filename | Overview |
|---|---|
| litellm/router_strategy/budget_limiter.py | Core bug fix: wraps the budget comparison in if config.max_budget is not None so a duration-only provider config no longer causes deployments to be silently excluded. Logic is correct; a duplicate comment was introduced in the same hunk. |
| tests/test_provider_budget_fix.py | Regression test for the budget fix, but it passes the same list object for both potential_deployments and healthy_deployments, which causes an infinite loop when the method appends to the list while iterating over it. Should use potential_deployments=[]. |
| litellm/main.py | Logic update to responses_api_bridge_check: for gpt-5.4+ models, tools alone now unconditionally bridge to the Responses API (removing the prior requirement that reasoning_effort also be set). Unrelated to the PR title but the change is self-consistent and tested. |
| tests/test_litellm/test_gpt56_bridge.py | New mock tests for the gpt-5.6 bridge change: one positive case (gpt-5.6 + tools without reasoning_effort bridges) and one negative case (older gpt-5 models with tools but no reasoning_effort do not bridge). Both tests are pure unit tests with no network calls. Missing newline at end of file. |
Reviews (1): Last reviewed commit: "fix(router): duration-only provider budg..." | Re-trigger Greptile
| result, _ = limiter._filter_out_deployments_above_budget( | ||
| potential_deployments=healthy_deployments, | ||
| healthy_deployments=healthy_deployments, | ||
| provider_configs=provider_configs, | ||
| deployment_configs={}, | ||
| deployment_providers=["openai"], | ||
| spend_map={}, | ||
| request_tags=[], | ||
| ) |
There was a problem hiding this comment.
Same list passed to both
potential_deployments and healthy_deployments causes an infinite loop
potential_deployments=healthy_deployments passes the same Python list object for both parameters. Inside _filter_out_deployments_above_budget, the method appends each eligible deployment to potential_deployments while iterating over healthy_deployments. Because both refer to the same list, the first eligible deployment is appended mid-iteration, the iterator sees the new element, processes it again, appends again, and so on indefinitely. In production, potential_deployments is always initialized as a separate empty list (potential_deployments: List[Dict] = [] at line 141 of budget_limiter.py). The test should pass potential_deployments=[] to match the production call pattern and avoid this hang.
| # Check provider budget | ||
| # Check provider budget | ||
| if self.provider_budget_config: |
There was a problem hiding this comment.
Duplicate
# Check provider budget comment was introduced by the diff. One of them should be removed.
| # Check provider budget | |
| # Check provider budget | |
| if self.provider_budget_config: | |
| # Check provider budget | |
| if self.provider_budget_config: |
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✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Closing in favor of #34044. The budget_limiter fix for #33327 is correct, but this branch was cut off #33237 so it bundled the gpt-5.6 main.py change (inheriting the gpt-5.4 test failures) and the regression test aliased potential_deployments to healthy_deployments (infinite loop). #34044 is a clean standalone branch off staging with the one-line fix and a test that passes potential_deployments=[]. |
Problem
A provider budget entry with
budget_durationbut nomax_budgetsilently removes all deployments for that provider from the healthy set.
Root cause: inside
_filter_out_deployments_above_budget, the block:uses
continuewhich advances the deployment loop, not just skipsthe provider check. The deployment never gets appended to the result.
Fix
Wrapped the budget comparison inside
if config.max_budget is not None:so a missing max_budget means no numeric cap — deployment stays eligible.
Test
Added regression test confirming a duration-only provider config
leaves the deployment in the healthy set.
Fixes #33327