-
-
Notifications
You must be signed in to change notification settings - Fork 11.2k
fix(router): duration-only provider budget config no longer removes deployments #33366
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ce3ff5d
1fdcc0c
d646b9b
1ce9972
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| """Regression test for https://github.com/BerriAI/litellm/issues/33221""" | ||
| from litellm.main import responses_api_bridge_check | ||
|
|
||
|
|
||
| def test_gpt56_tools_bridged_to_responses_without_reasoning_effort(): | ||
| tools = [{"type": "function", "function": {"name": "get_weather", "description": "Get weather", "parameters": {"type": "object", "properties": {"location": {"type": "string"}}, "required": ["location"]}}}] | ||
| for model in ["gpt-5.6-sol", "gpt-5.6-luna", "gpt-5.6-terra", "gpt-5.6"]: | ||
| model_info, _ = responses_api_bridge_check(model=model, custom_llm_provider="openai", tools=tools, reasoning_effort=None) | ||
| assert model_info.get("mode") == "responses", f"{model} with tools should bridge to responses even without reasoning_effort" | ||
|
|
||
| def test_older_gpt5_with_tools_not_bridged_without_reasoning_effort(): | ||
| """gpt-5, gpt-5.1, gpt-5.3 with tools should NOT bridge without reasoning_effort.""" | ||
| tools = [{"type": "function", "function": {"name": "get_weather", "description": "test", "parameters": {"type": "object", "properties": {"location": {"type": "string"}}, "required": ["location"]}}}] | ||
| for model in ["gpt-5", "gpt-5.1", "gpt-5.3"]: | ||
| model_info, _ = responses_api_bridge_check(model=model, custom_llm_provider="openai", tools=tools, reasoning_effort=None) | ||
| assert model_info.get("mode") != "responses", f"{model} with tools but no reasoning_effort should NOT bridge" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| """Regression test for https://github.com/BerriAI/litellm/issues/33327""" | ||
| from litellm.router_strategy.budget_limiter import RouterBudgetLimiting | ||
| from litellm.types.utils import BudgetConfig | ||
|
|
||
|
|
||
| def test_duration_only_provider_config_keeps_deployment(): | ||
| """ | ||
| A provider budget entry with duration but no max_budget should NOT | ||
| remove deployments from the healthy set. | ||
| """ | ||
| limiter = RouterBudgetLimiting.__new__(RouterBudgetLimiting) | ||
| limiter.provider_budget_config = {"openai": BudgetConfig(budget_duration="1d", max_budget=None)} | ||
| limiter.deployment_budget_config = None | ||
| limiter.tag_budget_config = None | ||
|
|
||
| healthy_deployments = [{ | ||
| "model_name": "chat", | ||
| "litellm_params": { | ||
| "model": "openai/gpt-4o-mini", | ||
| "custom_llm_provider": "openai", | ||
| }, | ||
| "model_info": {"id": "deployment-1"}, | ||
| }] | ||
|
|
||
| provider_configs = {"openai": limiter.provider_budget_config["openai"]} | ||
|
|
||
| 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=[], | ||
| ) | ||
|
Comment on lines
+27
to
+35
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| assert len(result) == 1, ( | ||
| f"Expected 1 deployment, got {len(result)}. " | ||
| "duration-only provider config should not remove deployments." | ||
| ) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Check provider budgetcomment was introduced by the diff. One of them should be removed.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!