Skip to content

fix(router): allow usage-based routing to use a deployment up to its rpm limit - #33033

Open
abhay-codes07 wants to merge 1 commit into
BerriAI:litellm_oss_daily_2026_07_10from
abhay-codes07:fix/lowest-tpm-rpm-off-by-one
Open

fix(router): allow usage-based routing to use a deployment up to its rpm limit#33033
abhay-codes07 wants to merge 1 commit into
BerriAI:litellm_oss_daily_2026_07_10from
abhay-codes07:fix/lowest-tpm-rpm-off-by-one

Conversation

@abhay-codes07

Copy link
Copy Markdown

Relevant issues

Fixes #33032

Pre-Submission checklist

  • I have added meaningful tests
  • My PR passes lint / format / unit tests locally
  • My PR's scope is as isolated as possible; it only solves 1 specific problem

Screenshots / Proof of Fix

Pure routing-logic off-by-one, reproducible offline (no keys/network).

Before (base 3d63eda) — deployment excluded one request early:

current_rpm=8, limit=10 -> selectable? True
current_rpm=9, limit=10 -> selectable? False   # wrong: reaching exactly 10 is within budget
current_rpm=10, limit=10 -> selectable? False

After (this PR):

current_rpm=8, limit=10 -> selectable? True
current_rpm=9, limit=10 -> selectable? True    # fixed
current_rpm=10, limit=10 -> selectable? False

New boundary regression tests for both v1 and v2 (fail on base, pass here):

tests/test_litellm/router_strategy/test_lowest_tpm_rpm.py ........  8 passed

Type

🐛 Bug Fix

Changes

Change the RPM filter in lowest_tpm_rpm (v1) and lowest_tpm_rpm_v2 from rpm_dict[item] + 1 >= _deployment_rpm to rpm_dict[item] + 1 > _deployment_rpm, so a deployment stays selectable until a request would actually exceed its rpm limit. This matches the TPM check in the same function (strict >), the sibling lowest_cost / lowest_latency strategies (item_rpm + 1 > rpm), and the pre_call_check enforcement. Prevents premature exclusion, unnecessary fallbacks and wasted capacity.


cc @ishaan-jaff @krrish-berri-2 — one-character boundary fix in the two usage-based routers with regression tests. Thanks for maintaining LiteLLM!

…rpm limit

The lowest_tpm_rpm v1 and v2 strategies excluded a deployment from selection when rpm_dict[item] + 1 >= rpm, i.e. one request before the configured limit was reached. A request that brings usage to exactly the rpm limit is within budget: the TPM check in the same function uses strict '>', the sibling lowest_cost and lowest_latency strategies use 'item_rpm + 1 > rpm', and the rpm enforcement in pre_call_check raises only when the incremented count exceeds the limit.

Change both handlers to 'rpm_dict[item] + 1 > _deployment_rpm' so a deployment stays selectable until its rpm limit, preventing premature exclusion, unnecessary fallbacks and wasted capacity. Adds boundary regression tests for both v1 and v2.
Copilot AI review requested due to automatic review settings July 13, 2026 04:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@greptile-apps

greptile-apps Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes an off-by-one in the RPM eligibility check in both lowest_tpm_rpm (v1) and lowest_tpm_rpm_v2 routers, changing rpm_dict[item] + 1 >= _deployment_rpm to the strict rpm_dict[item] + 1 > _deployment_rpm. The previous condition excluded a deployment one request too early — when the next call would bring usage to exactly the configured limit, which should still be allowed.

  • The fix aligns the RPM gate with the existing TPM check (item_tpm + input_tokens > _deployment_tpm) and with the lowest_cost/lowest_latency sibling strategies that already use strict >.
  • New parameterized boundary tests are added for both v1 and v2, covering current_rpm at {8, 9, 10, 11} against a limit of 10; all tests are mock-only with no real network calls.

Confidence Score: 5/5

Minimal, well-tested change to routing logic with no behavioral regressions; safe to merge.

Two single-character operator changes in the routing filter, each backed by new parameterized tests that specifically exercise the boundary conditions. The fix is consistent with how the TPM check and sibling strategies already behave, and the tests are pure mock tests that will run correctly in CI.

No files require special attention.

Important Files Changed

Filename Overview
litellm/router_strategy/lowest_tpm_rpm.py One-character fix: >=> in RPM filter so a deployment stays selectable until a request would strictly exceed its rpm limit, aligning with the TPM check in the same function.
litellm/router_strategy/lowest_tpm_rpm_v2.py Same >=> fix applied to the v2 RPM filter; change is minimal and correct.
tests/test_litellm/router_strategy/test_lowest_tpm_rpm.py New test file with parameterized boundary tests for both v1 and v2 strategies covering current_rpm at {8, 9, 10, 11} against a limit of 10; uses only mocked DualCache with no real network calls, consistent with repository rules.

Reviews (1): Last reviewed commit: "fix(router): allow usage-based routing t..." | Re-trigger Greptile

@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@abhay-codes07

Copy link
Copy Markdown
Author

Heads up: the red auth-and-jwt check here is a pre-existing failure on the litellm_oss_daily_2026_07_10 base (the test_can_key_call_model_wildcard_access/test_can_team_access_model bedrockz/* cases), unrelated to this change. Root-caused and fixed separately in #33031; this PR's own changes are fully covered and green. Once #33031 lands in the base, this check goes green on re-run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants