mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-29 01:42:19 +00:00
fix(router/proxy): address Greptile P1+P2 review comments on PR #28161
- router: raise ServiceUnavailableError (503) instead of RouterRateLimitErrorBasic (429) when a specifically-addressed deployment is administratively blocked; 429 misleads retry-enabled clients into spinning forever against a paused model - proxy_server: compute get_fully_blocked_model_names() once before both branches in model_list() instead of duplicating the call in each branch - deepseek: upgrade silent debug log to warning when injecting placeholder reasoning_content so callers are clearly notified of degraded multi-turn quality - tests: update two blocked-deployment assertions to expect ServiceUnavailableError Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
99f2487d3b
commit
683b421ba0
4 changed files with 36 additions and 20 deletions
|
|
@ -90,12 +90,15 @@ class DeepSeekChatConfig(OpenAIGPTConfig):
|
|||
cleaned.pop("reasoning_content", None)
|
||||
patched["provider_specific_fields"] = cleaned
|
||||
else:
|
||||
litellm.verbose_logger.debug(
|
||||
litellm.verbose_logger.warning(
|
||||
"DeepSeek thinking mode: assistant message is missing "
|
||||
"`reasoning_content`. Injecting a placeholder to satisfy "
|
||||
"API validation. For best results, preserve "
|
||||
"`reasoning_content` from the original assistant response "
|
||||
"when building multi-turn conversation history."
|
||||
"`reasoning_content` and none was saved in "
|
||||
"`provider_specific_fields`. A single-space placeholder "
|
||||
"is being injected to satisfy API validation, but the "
|
||||
"model will receive a blank reasoning chain for this turn, "
|
||||
"which may silently degrade multi-turn response quality. "
|
||||
"Preserve `reasoning_content` from the original assistant "
|
||||
"response when building multi-turn conversation history."
|
||||
)
|
||||
patched["reasoning_content"] = " "
|
||||
result.append(cast(AllMessageValues, patched))
|
||||
|
|
|
|||
|
|
@ -8090,6 +8090,11 @@ async def model_list(
|
|||
proxy_logging_obj=proxy_logging_obj,
|
||||
)
|
||||
|
||||
# Compute once — used in both branches below to hide paused models from the listing.
|
||||
blocked_names = (
|
||||
llm_router.get_fully_blocked_model_names() if llm_router is not None else set()
|
||||
)
|
||||
|
||||
# If scope=expand and user has admin privileges, return all proxy models
|
||||
if should_expand_scope:
|
||||
# Get all proxy models as if user is a proxy admin
|
||||
|
|
@ -8123,10 +8128,8 @@ async def model_list(
|
|||
)
|
||||
|
||||
# Hide paused models from the public listing (admins manage them via /model/info)
|
||||
if llm_router is not None:
|
||||
blocked_names = llm_router.get_fully_blocked_model_names()
|
||||
if blocked_names:
|
||||
all_models = [m for m in all_models if m not in blocked_names]
|
||||
if blocked_names:
|
||||
all_models = [m for m in all_models if m not in blocked_names]
|
||||
|
||||
# Build response data with all proxy models
|
||||
model_data = []
|
||||
|
|
@ -8162,10 +8165,8 @@ async def model_list(
|
|||
)
|
||||
|
||||
# Hide paused models from the public listing (admins manage them via /model/info)
|
||||
if llm_router is not None:
|
||||
blocked_names = llm_router.get_fully_blocked_model_names()
|
||||
if blocked_names:
|
||||
all_models = [m for m in all_models if m not in blocked_names]
|
||||
if blocked_names:
|
||||
all_models = [m for m in all_models if m not in blocked_names]
|
||||
|
||||
# Build response data
|
||||
model_data = []
|
||||
|
|
|
|||
|
|
@ -10163,7 +10163,11 @@ class Router:
|
|||
|
||||
if isinstance(healthy_deployments, dict):
|
||||
if (healthy_deployments.get("model_info") or {}).get("blocked") is True:
|
||||
raise RouterRateLimitErrorBasic(model=model)
|
||||
raise litellm.ServiceUnavailableError(
|
||||
message=f"Model '{model}' is administratively paused. Contact your proxy admin to unblock it.",
|
||||
model=model,
|
||||
llm_provider="",
|
||||
)
|
||||
return healthy_deployments
|
||||
|
||||
# Health-check-based filtering (before cooldown)
|
||||
|
|
@ -10591,7 +10595,11 @@ class Router:
|
|||
|
||||
if isinstance(healthy_deployments, dict):
|
||||
if (healthy_deployments.get("model_info") or {}).get("blocked") is True:
|
||||
raise RouterRateLimitErrorBasic(model=model)
|
||||
raise litellm.ServiceUnavailableError(
|
||||
message=f"Model '{model}' is administratively paused. Contact your proxy admin to unblock it.",
|
||||
model=model,
|
||||
llm_provider="",
|
||||
)
|
||||
return healthy_deployments
|
||||
|
||||
parent_otel_span: Optional[Span] = _get_parent_otel_span_from_kwargs(
|
||||
|
|
@ -10744,7 +10752,11 @@ class Router:
|
|||
# 2. If the returned is a specific deployment (Dict), verify and return directly
|
||||
if isinstance(healthy_deployments, dict):
|
||||
if (healthy_deployments.get("model_info") or {}).get("blocked") is True:
|
||||
raise RouterRateLimitErrorBasic(model=model)
|
||||
raise litellm.ServiceUnavailableError(
|
||||
message=f"Model '{model}' is administratively paused. Contact your proxy admin to unblock it.",
|
||||
model=model,
|
||||
llm_provider="",
|
||||
)
|
||||
litellm_params = healthy_deployments.get("litellm_params", {})
|
||||
if litellm_params.get("use_in_pass_through"):
|
||||
return healthy_deployments
|
||||
|
|
|
|||
|
|
@ -3788,10 +3788,10 @@ def test_public_get_available_deployment_skips_blocked_on_primary_path():
|
|||
|
||||
|
||||
def test_get_available_deployment_raises_when_addressed_dict_is_blocked():
|
||||
from litellm.types.router import RouterRateLimitErrorBasic
|
||||
import litellm
|
||||
|
||||
router = _router_with_two_deployments([True, True])
|
||||
with pytest.raises(RouterRateLimitErrorBasic):
|
||||
with pytest.raises(litellm.ServiceUnavailableError):
|
||||
router.get_available_deployment(model="dep-0", request_kwargs={})
|
||||
|
||||
|
||||
|
|
@ -3823,10 +3823,10 @@ def test_get_available_deployment_for_pass_through_skips_blocked():
|
|||
|
||||
|
||||
def test_get_available_deployment_for_pass_through_raises_when_dict_blocked():
|
||||
from litellm.types.router import RouterRateLimitErrorBasic
|
||||
import litellm
|
||||
|
||||
router = _router_with_two_pass_through_deployments([True, True])
|
||||
with pytest.raises(RouterRateLimitErrorBasic):
|
||||
with pytest.raises(litellm.ServiceUnavailableError):
|
||||
router.get_available_deployment_for_pass_through(
|
||||
model="pt-0", request_kwargs={}
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue