fix(router): scan all deployments for weight/rpm/tpm in simple-shuffle

[mateo-berri]

Problem

simple_shuffle only consults healthy_deployments[0] when deciding whether to do a weighted random pick. A config like:

model_list:
  - model_name: gpt-4o-mini
    litellm_params:
      model: openai/gpt-4o-mini
      # ^ no weight/rpm/tpm declared
  - model_name: gpt-4o-mini
    litellm_params:
      model: openai/gpt-4o-mini
      rpm: 1000
  - model_name: gpt-4o-mini
    litellm_params:
      model: openai/gpt-4o-mini
      rpm: 500

silently falls back to uniform random across all three because the first entry has no rpm. Users perceive this as "deployment-level rpm: (or tpm: / weight:) is silently ignored under simple-shuffle" — the actual behavior is ordering-sensitive: if the first deployment happens to declare the weight field, everything works; if not, the rest are silently disregarded.

This was reported as part of a broader audit of v3 rate limit semantics (sibling to #27913, #27914, #27915). It is the only one of those audit findings that's a pre-existing router bug rather than a v3-limiter or exception-mapper issue.

Fix

Extract _pick_weight_field(healthy_deployments) which scans every deployment for the precedence-ordered weight fields (weight > rpm > tpm) and returns the first one declared by any of them. Deployments that don't declare the chosen field continue to get weight 0 (preserves the existing semantics for in-list-but-unweighted entries).

Also: defensive divide-by-zero guard when total_weight == 0 (every deployment declared the field as 0 — pathological config). Falls through to uniform random instead of crashing.

Docstring updated to be explicit that rpm and tpm here are static relative weights, not cap-enforced limits, and to point users at router_settings.enable_pre_call_checks (router-level RPM cap filter, gated on messages is not None) or the proxy's key-level v3 TPM/RPM limiter for actual cap enforcement under simple-shuffle.

Scope notes

This PR does not change the documented semantics of rpm/tpm under simple-shuffle (still relative weights). Adding actual cap enforcement to simple-shuffle would change load-balancing behavior for every user on that strategy and is out of scope here — leaving it for a follow-up.

Tests

tests/test_litellm/router_strategy/test_simple_shuffle.py — 10 new unit tests:

TestPickWeightField:

• test_returns_none_when_no_deployment_declares_anything
• test_returns_weight_when_declared_on_first
• test_returns_weight_when_declared_only_on_later_deployments — the headline regression test
• test_precedence_weight_over_rpm_over_tpm — covers two precedence pairs
• test_handles_missing_litellm_params — defensive

TestSimpleShuffle (statistical assertions over 6,000 picks):

• test_uniform_random_when_no_weights
• test_rpm_on_later_deployment_is_respected — end-to-end regression for silent-fallthrough
• test_zero_total_weight_falls_through_to_uniform_random — divide-by-zero defense
• test_single_deployment_always_picked
• test_weight_field_wins_over_rpm — precedence under simple_shuffle (not just helper)

$ uv run pytest tests/test_litellm/router_strategy/test_simple_shuffle.py -q
10 passed in 0.17s

Related

Companion PRs from the same audit:

• fix(rate-limit): stop v3 limiter from leaking internal stash to provider body #27913 — fix(rate-limit): stop v3 limiter from leaking internal stash to provider body
• fix(rate-limit): resolve router model_name aliases to real provider #27914 — fix(rate-limit): resolve router model_name aliases to real provider (stacked on feat: standardize rate limit errors with category, rate_limit_type, model, and llm_provider fields #27687)
• fix(exceptions): don't map upstream 400 to RateLimitError on substring match #27915 — fix(exceptions): don't map upstream 400 to RateLimitError on substring match

Slack Thread

  

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[greptile-apps]

Greptile Summary

This PR fixes a long-standing ordering-sensitive bug in simple_shuffle where the router would silently fall back to uniform random if the first deployment in the list lacked a weight/rpm/tpm field, even when later deployments declared one. The fix introduces _pick_weight_field, which scans every deployment and returns the first declared field by precedence (weight > rpm > tpm), plus a divide-by-zero guard for pathological all-zero configs.

• _pick_weight_field replaces the single-index check with a full scan, ensuring deployment order no longer silently suppresses weighted routing.
• A total_weight > 0 guard was added to fall through to uniform random instead of crashing when all weights resolve to zero.
• 10 new unit tests cover the headline regression, precedence, the divide-by-zero case, and edge-case defensive paths.

Confidence Score: 3/5

The core logic change is correct and well-tested, but a defect in the weights-list construction can cause a TypeError crash when any deployment has a weight field explicitly set to null in YAML alongside a sibling with a numeric value.

The weights list uses .get(weight_field, 0), which returns None when the key is present with a null value rather than returning the 0 default. Because _pick_weight_field may return the field name based on a sibling deployment's non-None value, the weights list for the null-valued deployment becomes None, causing sum() to raise a TypeError. The new tests do not cover this case.

litellm/router_strategy/simple_shuffle.py lines 81-84 need null-value coercion; tests/test_litellm/router_strategy/test_simple_shuffle.py is missing a case for field: null (key present, value None).

Important Files Changed

Filename	Overview
litellm/router_strategy/simple_shuffle.py	Core logic fix is correct (scan all deployments for weight fields, add divide-by-zero guard). One defect: when a weight key is present with value None in litellm_params, sum() raises TypeError. Edge case with field: 0 triggering weighted mode but resolving to uniform random may be surprising.
tests/test_litellm/router_strategy/test_simple_shuffle.py	10 new unit tests with good coverage of the regression case, precedence rules, zero-weight fallback, and single-deployment path. No real-network calls. Missing coverage for field: null (value present, set to None), which would have caught the sum() TypeError.

_{Reviews (1): Last reviewed commit: "fix(router): scan all deployments for we..." | Re-trigger Greptile}

[greptile-apps]

If a deployment's YAML sets a weight field to null (i.e. the key is present in litellm_params but its Python value is None), .get(weight_field, 0) returns None rather than 0 — the default is only used when the key is absent. Meanwhile _pick_weight_field returns the field name because the other deployment has a non-None value for it. The resulting weights list then contains None, causing sum() to raise TypeError: unsupported operand type(s) for +: 'int' and 'NoneType'. The fix is to coerce None to 0 explicitly.

Suggested change

	weights = [
	(m.get("litellm_params") or {}).get(weight_field, 0)
	for m in healthy_deployments
	]
	weights = [
	(m.get("litellm_params") or {}).get(weight_field) or 0
	for m in healthy_deployments
	]

[greptile-apps]

field: 0 triggers weighted mode but effective uniform random

_pick_weight_field returns a field when any deployment sets it to 0 (since 0 is not None). When this happens and no other deployment declares a positive value for that field, every entry defaults to weight 0, total_weight == 0, and the code falls through to uniform random — including the deployment with the explicit 0. A user who writes weight: 0 expecting to exclude one endpoint while letting the rest participate normally will instead get equal traffic to all deployments. The check could be tightened to litellm_params.get(field, 0) > 0 so that a field is only "detected" when at least one deployment declares a positive value for it.

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[devin-ai-integration]

🚅 Hi, thanks for the PR! I'm Agent Shin, the automated triage bot for this repository. What's this and why am I getting it?

I read the description against our contribution rubric. Here's how it lined up:

What you got right:

• ✅ Clear problem description
• ✅ Expected vs. actual behavior

What's still missing:

• End-to-end QA proof: the only run output is uv run pytest tests/test_litellm/router_strategy/test_simple_shuffle.py on the repo's own unit tests, which construct deployment dicts in-process and never route a real request through a live router/proxy. A short recording, a screenshot, or a real simple-shuffle proxy run with the three-deployment config from the description (showing the skewed distribution before and the weighted distribution after) would close this gap.
• Greptile's most recent confidence score on this PR is 3/5, below our 4/5 bar. Push fixes for its feedback and comment @greptileai for a fresh review.

The description is genuinely good on context: the ordering-sensitive simple_shuffle bug is spelled out with the exact config that triggers it, the root cause, and the precedence rules for the fix. What's missing is proof against the real system rather than the mocked unit tests, plus a Greptile score at or above our merge bar.

If the description isn't updated in the next 2 hours, I'll auto-close this PR. That's not us saying we don't care about the change; we want the open-PR list to mirror what a maintainer can act on right now, so contributors don't get lost in a backlog. A closed PR is a soft "park this for later," not a rejection. Take your time; everything below still works after the close.

During the grace period: just update the PR description with the missing pieces. No need to ping me; I'll re-check on the next sweep and skip the auto-close if it now passes. See what counts as QA proof for the full rubric (a linked issue alone isn't enough; it covers context, not proof).

If the PR does get auto-closed in 2 hours, you still have easy recovery paths:

• Comment @agent-shin reconsider after updating the description. I'll re-evaluate and reopen the PR if it now passes.
• Comment @greptileai to request a fresh Greptile review; that still works even after the PR is closed, and a stronger score is one of the signals that lifts the PR back into the queue. So a low Greptile score isn't a blocker either.

Internal BerriAI contributors: this rubric doesn't apply to you; ping a maintainer.

(I'm an LLM, so I'm not infallible. If you think I got this wrong, ping a maintainer; they'll override me.)

[devin-ai-integration]

🚅 Hi, thanks for the PR! I'm Agent Shin, the automated triage bot for this repository. What's this and why am I getting it?

I read the description against our contribution rubric. Here's how it lined up:

What you got right:

• ✅ Clear problem description
• ✅ Expected vs. actual behavior

What's still missing:

• End-to-end QA proof: the only run output is uv run pytest tests/test_litellm/router_strategy/test_simple_shuffle.py, which builds deployment dicts in-process and never routes a real request through a live router/proxy. A recording, a screenshot, or a real simple-shuffle proxy run with the three-deployment config from the description (skewed distribution before, weighted after) would close this gap.
• Greptile's most recent confidence score on this PR is 3/5, below our 4/5 bar.

Context is strong — the ordering-sensitive simple_shuffle bug, the triggering config, and the precedence rules for the fix are all clear. What's still absent 24h after the warning is proof against the real system rather than mocked unit tests, plus a Greptile score at or above the merge bar.

Closing this PR isn't a rejection of the change. We want the open-PR list to mirror what a maintainer can act on right now, so contributors don't get lost in a backlog. A closed PR is a soft "park this for later"; your work is still here, the diff is still here, and getting it reopened is one comment away. Take your time.

To bring this PR back:

• Update the description with the missing pieces, then comment @agent-shin reconsider on this PR. I'll re-evaluate and reopen if it now passes.
• Or Open a new PR with the same fix and the updated description. GitHub doesn't always let external contributors reopen a bot-closed PR, so a fresh PR is the most reliable path back into the review queue.
• If Greptile's most recent score on this PR was below 4/5, comment @greptileai to request a fresh review; that still works even after the PR is closed, and a stronger score is one of the signals that lifts the PR back into the queue. A low Greptile score isn't a blocker.

What "end-to-end QA proof" means, since it's the most common gap: at least one of a short before/after screen recording / video (the bug reproducing, then the fix working; for a brand-new feature, a recording of it working end-to-end), a screenshot (or before/after screenshots) of it working, or the exact commands you ran paired with their real output against the real system. Running pytest on the repo's unit tests doesn't count; those mock the LLM provider, DB, and network, so they aren't end-to-end. Output from a real, no-mocks integration run is what we look for. A linked issue alone isn't enough either: it covers context, not proof. See the full rubric.

Internal BerriAI contributors: this rubric doesn't apply to you; ping a maintainer.

(I'm an LLM, so I'm not infallible. If you think I got this wrong, comment @agent-shin reconsider or ping a maintainer; they'll override me.)