fix(router): discard oldest entry when trimming latency list in lowest_latency strategy by hatim-ez · Pull Request #25548 · BerriAI/litellm

hatim-ez · 2026-04-11T02:53:41Z

Relevant issues

N/A

Type

🐛 Bug Fix
✅ Test

Changes

Summary

The lowest_latency routing strategy maintains a rolling window of the last
max_latency_list_size latency and time-to-first-token measurements per
deployment. When a deployment's window was full, the trim logic discarded the
newest value instead of the oldest.

The root cause is an off-by-one in the slice expression used to drop an entry
before appending the new one:

# Before — keeps indices 0..N-2, drops the newest value at index N-1
request_count_dict[id]["latency"] = request_count_dict[id][
    "latency"
][: self.routing_args.max_latency_list_size - 1] + [final_value]

# After — drops index 0 (oldest) and keeps the rest
request_count_dict[id]["latency"] = request_count_dict[id][
    "latency"
][1:] + [final_value]

Since new values are always appended at the end (.append(...) is used for the
pre-full path, and ... + [final_value] for the trim path), index 0 is the
oldest and index -1 is the newest. The previous slice kept the oldest N-1
entries and threw away the freshly appended value — meaning once a deployment's
window filled up, the routing decision relied on stale data, and any initial
outlier (for example an early cold-start spike) lived in the window forever.

The same bug also silently swallowed timeout penalties: async_log_failure_event
appends 1000.0 to the latency list when a litellm.Timeout is raised, but
that penalty was being discarded on full windows, so repeatedly-timing-out
deployments never had their routing score degraded.

Fix

litellm/router_strategy/lowest_latency.py — change the slice from
[: self.routing_args.max_latency_list_size - 1] to [1:] in five
locations that all follow the same buggy pattern:

log_success_event — latency list
log_success_event — time_to_first_token list
async_log_failure_event — latency list (1000.0 timeout penalty)
async_log_success_event — latency list
async_log_success_event — time_to_first_token list

No other routing strategies (lowest_cost, lowest_tpm_rpm,
lowest_tpm_rpm_v2) use a rolling list — they aggregate per-minute counters —
so they are not affected.

Tests

Added 5 regression tests in
tests/local_testing/test_lowest_latency_routing.py, each exercising one of
the fixed call sites:

test_latency_list_trimming_discards_oldest_entry — sync log_success_event
path for the latency list.
test_latency_list_trimming_discards_oldest_entry_async — async counterpart.
test_ttft_list_trimming_discards_oldest_entry — streaming TTFT path
(uses litellm.ModelResponse because TTFT is only recorded for
ModelResponse instances).
test_timeout_penalty_discards_oldest_entry — async_log_failure_event
timeout penalty path; asserts the 1000.0 penalty lands at the end of the
list and the oldest normal entry is evicted.
test_list_order_preserved_after_multiple_trims — adds 10 entries with
max_latency_list_size=3 and asserts the final list is exactly the last 3
values in insertion order.

Each test uses routing_args={"max_latency_list_size": 3} so trimming is
triggered with only a handful of insertions, keeping the suite fast.

I verified the tests are load-bearing: reverting the source fix causes all 5
new tests to fail, and applying the fix makes them pass. The full test file
(tests/local_testing/test_lowest_latency_routing.py, 22 tests) passes.

$ uv run pytest tests/local_testing/test_lowest_latency_routing.py
======================= 22 passed, 2 warnings in 59.92s ========================

Pre-Submission checklist

I have added testing — 5 new regression tests in
tests/local_testing/test_lowest_latency_routing.py, one per fixed call
site. (The existing lowest_latency test suite already lives in
tests/local_testing/; the new tests were added alongside the existing
coverage to keep the suite cohesive.)
PR scope is isolated: one bug, five instances of the same off-by-one,
plus regression tests.
No unrelated formatting or refactoring changes — the source diff is
literally 5 single-line changes.

vercel · 2026-04-11T02:53:48Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
litellm	Ready	Preview, Comment	Apr 13, 2026 7:22pm

CLAassistant · 2026-04-11T02:53:51Z

All committers have signed the CLA.

codspeed-hq · 2026-04-11T02:56:04Z

Merging this PR will not alter performance

✅ 16 untouched benchmarks

_{Comparing hatim-ez:fix/lowest-latency-list-trimming (d5109e2) with main (d319cd8)}

greptile-apps · 2026-04-11T02:56:10Z

Greptile Summary

Fixes a sliding-window off-by-one in the lowest_latency routing strategy: every update site used list[:max_size - 1], which kept the oldest N-1 entries and discarded the previously-newest value instead of the oldest. The correct list[1:] slice is now used in all five locations (log_success_event latency, log_success_event TTFT, async_log_failure_event timeout-penalty latency, async_log_success_event latency, async_log_success_event TTFT). Six regression tests cover each fixed path, including the async TTFT path that was flagged in the prior review round.

Confidence Score: 5/5

Safe to merge — the fix is mechanically straightforward, all five sites are addressed consistently, and six targeted regression tests (including the async TTFT path flagged in the prior round) confirm the corrected behavior.

No P0 or P1 findings. All changes are identical one-liner corrections to the same pattern, tests are mock-only and cover every fixed call site, and no unrelated code is touched.

No files require special attention.

Important Files Changed

Filename	Overview
litellm/router_strategy/lowest_latency.py	Five identical off-by-one slice expressions corrected from `[:max_size-1]` to `[1:]`; all changed lines follow the same fix pattern and are consistent with each other.
tests/local_testing/test_lowest_latency_routing.py	Six new regression tests added (sync latency, async latency, sync TTFT, async TTFT, timeout penalty, multi-trim order); all use in-memory DualCache with no real network calls, satisfying the mock-only rule for this test folder.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[New latency value arrives] --> B{len latency_list < max_size?}
    B -- Yes --> C[list.append new_value]
    B -- No --> D["list = list[1:] + [new_value]"]
    D --> E[Index 0 oldest entry evicted]
    D --> F[new_value appended at end]
    C --> G[Cache updated]
    E --> G
    F --> G
    style D fill:#90EE90
    style E fill:#90EE90
    style F fill:#90EE90

_{Reviews (3): Last reviewed commit: "format" | Re-trigger Greptile}

greptile-apps · 2026-04-11T02:56:14Z

+@pytest.mark.asyncio
+async def test_latency_list_trimming_discards_oldest_entry_async():
+    """
+    Async counterpart: the oldest entry is discarded when the latency list is
+    trimmed.
+    """
+    max_size = 3
+    test_cache = DualCache()
+    lowest_latency_logger = LowestLatencyLoggingHandler(
+        router_cache=test_cache, routing_args={"max_latency_list_size": max_size}
+    )
+
+    model_group = "gpt-3.5-turbo"
+    deployment_id = "test-deployment"
+    kwargs = {
+        "litellm_params": {
+            "metadata": {
+                "model_group": model_group,
+                "deployment": "azure/gpt-4.1-mini",
+            },
+            "model_info": {"id": deployment_id},
+        }
+    }
+
+    latencies_to_add = []
+    for i in range(max_size + 1):
+        start_time = time.time()
+        response_obj = {"usage": {"total_tokens": 1, "completion_tokens": 1}}
+        expected_latency = float(i + 1)
+        end_time = start_time + expected_latency
+        latencies_to_add.append(expected_latency)
+
+        await lowest_latency_logger.async_log_success_event(
+            response_obj=response_obj,
+            kwargs=kwargs,
+            start_time=start_time,
+            end_time=end_time,
+        )
+
+    latency_key = f"{model_group}_map"
+    cached_data = await test_cache.async_get_cache(key=latency_key)
+    latency_list = cached_data[deployment_id]["latency"]
+
+    assert len(latency_list) == max_size
+
+    newest_latency = latencies_to_add[-1]
+    oldest_latency = latencies_to_add[0]
+    tolerance = 0.1
+
+    assert (
+        abs(latency_list[-1] - newest_latency) < tolerance
+    ), f"Newest latency {newest_latency} should be at end of list"
+
+    for latency in latency_list:
+        assert (
+            abs(latency - oldest_latency) > tolerance
+        ), f"Oldest latency {oldest_latency} should have been discarded"


Missing test for async TTFT trim path

Fix #5 (async_log_success_event → time_to_first_token list) doesn't have a dedicated test. test_latency_list_trimming_discards_oldest_entry_async only exercises the latency list because it passes a plain dict response_obj without stream=True or completion_start_time, so the TTFT branch is never entered. A test analogous to test_ttft_list_trimming_discards_oldest_entry but calling async_log_success_event with a ModelResponse and completion_start_time would close this gap.

codecov · 2026-04-11T02:56:42Z

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
litellm/router_strategy/lowest_latency.py	0.00%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

hatim-ez · 2026-04-11T08:37:25Z

The 2 failing checks are:

on the formatting, but that should be addressed by another PR: chore: apply black formatting to remaining files #25185
on an unrelated error: FAILED tests/test_litellm/proxy/prompts/test_prompt_endpoints.py::TestPromptVersionsEndpoint::test_get_prompt_versions_not_found - TypeError: object MagicMock can't be used in 'await' expression

…t_latency strategy The lowest_latency routing strategy keeps a rolling window of the most recent latency and time-to-first-token measurements per deployment. When the window is full, the strategy was discarding the *newest* value instead of the oldest, because the trim used `[: max_latency_list_size - 1]` (keeping indices 0..N-2) rather than `[1:]` (dropping index 0 and keeping indices 1..N-1). Since new values are appended at the end, the bug meant the most recent measurement was always dropped once the list reached capacity. The routing decisions then relied on stale data (including any early-spike values that never aged out), and timeout penalties written via `async_log_failure_event` were silently discarded as well. Fix the slice in all five call sites (sync + async log_success_event for both latency and time_to_first_token, and async_log_failure_event for the timeout penalty) and add regression tests covering each path.

… tests Adds test_ttft_list_trimming_discards_oldest_entry_async, an async counterpart to test_ttft_list_trimming_discards_oldest_entry that drives async_log_success_event with a ModelResponse and completion_start_time so the async time_to_first_token trim branch is actually exercised. Previously no test touched that code path: the sync TTFT test used log_success_event, and the async latency test passed a plain dict response_obj without stream/completion_start_time, so TTFT was never computed and the async trim was unreached. Verified load-bearing by reverting only the async TTFT slice — the new test fails and all others pass.

…t_latency strategy (#25548) * fix(router): discard oldest entry when trimming latency list in lowest_latency strategy The lowest_latency routing strategy keeps a rolling window of the most recent latency and time-to-first-token measurements per deployment. When the window is full, the strategy was discarding the *newest* value instead of the oldest, because the trim used `[: max_latency_list_size - 1]` (keeping indices 0..N-2) rather than `[1:]` (dropping index 0 and keeping indices 1..N-1). Since new values are appended at the end, the bug meant the most recent measurement was always dropped once the list reached capacity. The routing decisions then relied on stale data (including any early-spike values that never aged out), and timeout penalties written via `async_log_failure_event` were silently discarded as well. Fix the slice in all five call sites (sync + async log_success_event for both latency and time_to_first_token, and async_log_failure_event for the timeout penalty) and add regression tests covering each path. * test(router): cover async TTFT trim path in lowest_latency regression tests Adds test_ttft_list_trimming_discards_oldest_entry_async, an async counterpart to test_ttft_list_trimming_discards_oldest_entry that drives async_log_success_event with a ModelResponse and completion_start_time so the async time_to_first_token trim branch is actually exercised. Previously no test touched that code path: the sync TTFT test used log_success_event, and the async latency test passed a plain dict response_obj without stream/completion_start_time, so TTFT was never computed and the async trim was unreached. Verified load-bearing by reverting only the async TTFT slice — the new test fails and all others pass. * format

…t_latency strategy (BerriAI#25548) * fix(router): discard oldest entry when trimming latency list in lowest_latency strategy The lowest_latency routing strategy keeps a rolling window of the most recent latency and time-to-first-token measurements per deployment. When the window is full, the strategy was discarding the *newest* value instead of the oldest, because the trim used `[: max_latency_list_size - 1]` (keeping indices 0..N-2) rather than `[1:]` (dropping index 0 and keeping indices 1..N-1). Since new values are appended at the end, the bug meant the most recent measurement was always dropped once the list reached capacity. The routing decisions then relied on stale data (including any early-spike values that never aged out), and timeout penalties written via `async_log_failure_event` were silently discarded as well. Fix the slice in all five call sites (sync + async log_success_event for both latency and time_to_first_token, and async_log_failure_event for the timeout penalty) and add regression tests covering each path. * test(router): cover async TTFT trim path in lowest_latency regression tests Adds test_ttft_list_trimming_discards_oldest_entry_async, an async counterpart to test_ttft_list_trimming_discards_oldest_entry that drives async_log_success_event with a ModelResponse and completion_start_time so the async time_to_first_token trim branch is actually exercised. Previously no test touched that code path: the sync TTFT test used log_success_event, and the async latency test passed a plain dict response_obj without stream/completion_start_time, so TTFT was never computed and the async trim was unreached. Verified load-bearing by reverting only the async TTFT slice — the new test fails and all others pass. * format

vercel Bot deployed to Preview April 11, 2026 02:55 View deployment

greptile-apps Bot reviewed Apr 11, 2026

View reviewed changes

hatim-ez marked this pull request as draft April 11, 2026 02:58

vercel Bot deployed to Preview April 11, 2026 08:39 View deployment

hatim-ez marked this pull request as ready for review April 11, 2026 09:01

vercel Bot deployed to Preview April 11, 2026 09:02 View deployment

hatim-ez added 3 commits April 13, 2026 12:15

format

d5109e2

hatim-ez force-pushed the fix/lowest-latency-list-trimming branch from ef323d1 to d5109e2 Compare April 13, 2026 19:16

vercel Bot deployed to Preview April 13, 2026 19:22 View deployment

krrish-berri-2 changed the base branch from main to litellm_oss_staging_04_13_2026_p1 April 14, 2026 02:29

krrish-berri-2 merged commit fd6221c into BerriAI:litellm_oss_staging_04_13_2026_p1 Apr 14, 2026
48 of 51 checks passed

Navigation Menu

Uh oh!

Uh oh!

fix(router): discard oldest entry when trimming latency list in lowest_latency strategy - #25548

fix(router): discard oldest entry when trimming latency list in lowest_latency strategy#25548
krrish-berri-2 merged 3 commits into
BerriAI:litellm_oss_staging_04_13_2026_p1from
hatim-ez:fix/lowest-latency-list-trimming

hatim-ez commented Apr 11, 2026

Uh oh!

vercel Bot commented Apr 11, 2026 •

edited

Loading

Uh oh!

CLAassistant commented Apr 11, 2026 •

edited

Loading

Uh oh!

codspeed-hq Bot commented Apr 11, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Apr 11, 2026 •

edited

Loading

Important Files Changed

Uh oh!

greptile-apps Bot Apr 11, 2026

Uh oh!

codecov Bot commented Apr 11, 2026 •

edited

Loading

Uh oh!

hatim-ez commented Apr 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

hatim-ez commented Apr 11, 2026

Relevant issues

Type

Changes

Summary

Fix

Tests

Pre-Submission checklist

Uh oh!

vercel Bot commented Apr 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CLAassistant commented Apr 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codspeed-hq Bot commented Apr 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Uh oh!

greptile-apps Bot commented Apr 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

greptile-apps Bot Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Apr 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

hatim-ez commented Apr 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vercel Bot commented Apr 11, 2026 •

edited

Loading

CLAassistant commented Apr 11, 2026 •

edited

Loading

codspeed-hq Bot commented Apr 11, 2026 •

edited

Loading

greptile-apps Bot commented Apr 11, 2026 •

edited

Loading

codecov Bot commented Apr 11, 2026 •

edited

Loading