From be9a2ea7c97f945dc961c357b80f8b44f30c7746 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Sat, 5 Sep 2026 16:20:20 -0700 Subject: [PATCH] fix(router): average every latency sample and drop the cost handler's dead division _average_latency skipped integer samples in the sum while counting them in the denominator, which contradicted its own Sequence[float | int] signature; it now averages every sample. Both success loggers in cost-based routing computed response_ms / completion_tokens and threw the result away, so a chat response with zero completion tokens raised ZeroDivisionError inside the handler. The proxy swallows and logs it, but the handler then skips that request's tpm and rpm update, so cost-based routing undercounts the deployment's usage. The QA run for the latency fix hit it on real gpt-5.5 traffic through /v1/chat/completions and /v1/messages --- litellm/router_strategy/lowest_cost.py | 11 +--- litellm/router_strategy/lowest_latency.py | 4 +- .../router_strategy/test_lowest_cost.py | 59 +++++++++++++++++++ .../router_strategy/test_lowest_latency.py | 2 - 4 files changed, 62 insertions(+), 14 deletions(-) create mode 100644 tests/test_litellm/router_strategy/test_lowest_cost.py diff --git a/litellm/router_strategy/lowest_cost.py b/litellm/router_strategy/lowest_cost.py index b927df0c438..6eb4d86d280 100644 --- a/litellm/router_strategy/lowest_cost.py +++ b/litellm/router_strategy/lowest_cost.py @@ -1,6 +1,6 @@ #### What this does #### # picks based on response time (for streaming, this is time to first token) -from datetime import datetime, timedelta +from datetime import datetime from typing import Final import litellm @@ -52,16 +52,12 @@ class LowestCostLoggingHandler(CustomLogger): precise_minute: Final = f"{current_date}-{current_hour}-{current_minute}" cost_key: Final = f"{model_group}_map" - response_ms: Final[timedelta] = end_time - start_time - total_tokens = 0 if isinstance(response_obj, ModelResponse): _usage: Final = getattr(response_obj, "usage", None) if _usage is not None and isinstance(_usage, litellm.Usage): - completion_tokens: Final = _usage.completion_tokens total_tokens = _usage.total_tokens - float(response_ms.total_seconds() / completion_tokens) # ------------ # Update usage @@ -131,18 +127,13 @@ class LowestCostLoggingHandler(CustomLogger): current_minute: Final = datetime.now().strftime("%M") precise_minute: Final = f"{current_date}-{current_hour}-{current_minute}" - response_ms: Final[timedelta] = end_time - start_time - total_tokens = 0 if isinstance(response_obj, ModelResponse): _usage: Final = getattr(response_obj, "usage", None) if _usage is not None and isinstance(_usage, litellm.Usage): - completion_tokens: Final = _usage.completion_tokens total_tokens = _usage.total_tokens - float(response_ms.total_seconds() / completion_tokens) - # ------------ # Update usage # ------------ diff --git a/litellm/router_strategy/lowest_latency.py b/litellm/router_strategy/lowest_latency.py index 4a1fb4f325a..bb6877a032b 100644 --- a/litellm/router_strategy/lowest_latency.py +++ b/litellm/router_strategy/lowest_latency.py @@ -26,10 +26,10 @@ class RoutingArgs(LiteLLMPydanticObjectBase): max_latency_list_size: int = 10 -def _average_latency(samples: Sequence[float | int]) -> float: +def _average_latency(samples: Sequence[float]) -> float: if not samples: return 0.0 - return sum(sample for sample in samples if isinstance(sample, float)) / len(samples) + return sum(samples) / len(samples) class LowestLatencyLoggingHandler(CustomLogger): diff --git a/tests/test_litellm/router_strategy/test_lowest_cost.py b/tests/test_litellm/router_strategy/test_lowest_cost.py new file mode 100644 index 00000000000..108053dddd9 --- /dev/null +++ b/tests/test_litellm/router_strategy/test_lowest_cost.py @@ -0,0 +1,59 @@ +from datetime import datetime + +import pytest + +import litellm +from litellm.caching.caching import DualCache +from litellm.router_strategy.lowest_cost import LowestCostLoggingHandler + +DEPLOYMENT_ID = "9876" +KWARGS = { + "litellm_params": { + "metadata": {"model_group": "gpt-5.5-pool"}, + "model_info": {"id": DEPLOYMENT_ID}, + } +} + + +def _chat_response_with_no_completion_tokens() -> litellm.ModelResponse: + return litellm.ModelResponse( + model="gpt-5.5", + choices=[{"index": 0, "message": {"role": "assistant", "content": ""}, "finish_reason": "length"}], + usage=litellm.Usage(prompt_tokens=12, completion_tokens=0, total_tokens=12), + ) + + +def _recorded_minute_counters(cache: DualCache) -> dict[str, int]: + cached = cache.get_cache(key="gpt-5.5-pool_map") or {} + minute_buckets = cached.get(DEPLOYMENT_ID, {}) + assert len(minute_buckets) == 1, f"expected one minute bucket, got {minute_buckets}" + return next(iter(minute_buckets.values())) + + +def test_log_success_event_counts_a_response_with_no_completion_tokens(): + cache = DualCache() + handler = LowestCostLoggingHandler(router_cache=cache) + + handler.log_success_event( + kwargs=KWARGS, + response_obj=_chat_response_with_no_completion_tokens(), + start_time=datetime(2026, 1, 1, 12, 0, 0), + end_time=datetime(2026, 1, 1, 12, 0, 2), + ) + + assert _recorded_minute_counters(cache) == {"tpm": 12, "rpm": 1} + + +@pytest.mark.asyncio +async def test_async_log_success_event_counts_a_response_with_no_completion_tokens(): + cache = DualCache() + handler = LowestCostLoggingHandler(router_cache=cache) + + await handler.async_log_success_event( + kwargs=KWARGS, + response_obj=_chat_response_with_no_completion_tokens(), + start_time=datetime(2026, 1, 1, 12, 0, 0), + end_time=datetime(2026, 1, 1, 12, 0, 2), + ) + + assert _recorded_minute_counters(cache) == {"tpm": 12, "rpm": 1} diff --git a/tests/test_litellm/router_strategy/test_lowest_latency.py b/tests/test_litellm/router_strategy/test_lowest_latency.py index 251b760a8d8..eb02459be68 100644 --- a/tests/test_litellm/router_strategy/test_lowest_latency.py +++ b/tests/test_litellm/router_strategy/test_lowest_latency.py @@ -172,8 +172,6 @@ def test_sync_chat_zero_completion_tokens_falls_back_to_seconds(): ids=["empty_latency_list", "minute_bucket_only_as_cost_based_routing_writes"], ) async def test_async_get_available_deployments_treats_missing_samples_as_zero_latency(cached_entry): - """A cached entry with no latency samples (cost-based routing shares the group's map key and writes - minute buckets only) must count as 0 latency, like an unseen deployment, instead of dividing by zero.""" cache = DualCache() handler = LowestLatencyLoggingHandler(router_cache=cache) cache.set_cache(