Skip to content

fix(proxy): use >= in _team_max_budget_check to match other budget checks - #28051

Closed
msxfXF wants to merge 6 commits into
BerriAI:litellm_internal_stagingfrom
msxfXF:fix/team-max-budget-gte
Closed

fix(proxy): use >= in _team_max_budget_check to match other budget checks#28051
msxfXF wants to merge 6 commits into
BerriAI:litellm_internal_stagingfrom
msxfXF:fix/team-max-budget-gte

Conversation

@msxfXF

@msxfXF msxfXF commented May 16, 2026

Copy link
Copy Markdown

Relevant issues

Fixes #28020

Pre-Submission checklist

  • I have Added testing in the tests/test_litellm/ directory — added test_team_budget_check_raises_when_spend_equals_max_budget in tests/test_litellm/proxy/auth/test_auth_checks.py
  • My PR's scope is as isolated as possible, it only solves 1 specific problem
  • My PR passes all unit tests on make test-unit (could not run the full suite locally; the new test follows the same pattern as the existing test_team_budget_check_reads_from_spend_counter directly above it)
  • Greptile review pending

Type

🐛 Bug Fix

Changes

_team_max_budget_check used > while every other budget enforcement path in the same file uses >=:

Check Line Operator
_virtual_key_max_budget_check (key) 3283 >=
Budget window (key) 3316 >=
_team_max_budget_check (team) 3653 > ← bug
Budget window (team) 3701 >=
_organization_max_budget_check (org) 4007 >=

A team whose spend equals exactly max_budget was therefore still allowed through, e.g. 10.0 > 10.0 is False. This is inconsistent with key/org/window enforcement and contradicts the natural meaning of "max budget".

Diff

-        if spend > team_object.max_budget:
+        if spend >= team_object.max_budget:

Test

Added test_team_budget_check_raises_when_spend_equals_max_budget that constructs a team with max_budget=10.0, mocks get_current_spend to return exactly 10.0, and asserts BudgetExceededError is raised with current_cost == 10.0 and max_budget == 10.0. Mirrors the existing test_team_budget_check_reads_from_spend_counter test pattern.

Scope note

This PR intentionally does not touch the other >-using sites (global_proxy_spend > litellm.max_budget at line 331, project_object.spend > max_budget at line 3815) — those are separate decisions and out of scope for #28020.


Disclosure: this PR was prepared with assistance from Claude (Anthropic). The bug location and proposed fix were both pre-specified in the linked issue; I verified the surrounding code matches the issue's description and added the regression test.

FabrizioCafolla and others added 4 commits May 15, 2026 10:36
Squash-merged by litellm-agent from FabrizioCafolla's PR.
Squash-merged by litellm-agent from tomdee's PR.
Squash-merged by litellm-agent from escon1004's PR.
@CLAassistant

CLAassistant commented May 16, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@greptile-apps

greptile-apps Bot commented May 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a one-character operator inconsistency in _team_max_budget_check: the comparison was > while every other budget enforcement path in the same file (_virtual_key_max_budget_check, budget windows, _organization_max_budget_check) uses >=. A team whose spend exactly equalled max_budget was therefore allowed to continue making requests.

  • litellm/proxy/auth/auth_checks.py: Changes spend > team_object.max_budget to spend >= team_object.max_budget on the single enforcement line in _team_max_budget_check.
  • tests/test_litellm/proxy/auth/test_auth_checks.py: Adds test_team_budget_check_raises_when_spend_equals_max_budget, a mock-only regression test that constructs a team at exactly its budget cap and asserts BudgetExceededError is raised with the correct current_cost and max_budget values.

Confidence Score: 5/5

Safe to merge — the change is a minimal, well-targeted operator correction that aligns team budget enforcement with every other budget check in the file.

The fix is isolated to a single comparison in _team_max_budget_check, directly matches the documented intent of 'max budget' enforcement, and is consistent with all parallel checks. The regression test covers the exact boundary case that was previously passing incorrectly, uses mocks throughout, and follows the established test pattern. No unrelated code is touched.

No files require special attention.

Important Files Changed

Filename Overview
litellm/proxy/auth/auth_checks.py Single-character operator fix: > changed to >= in _team_max_budget_check, bringing team budget enforcement in line with key, org, and window checks
tests/test_litellm/proxy/auth/test_auth_checks.py New regression test test_team_budget_check_raises_when_spend_equals_max_budget added; uses mocks only, mirrors existing test pattern, correctly asserts BudgetExceededError when spend equals max_budget

Reviews (2): Last reviewed commit: "fix(proxy): use >= in _team_max_budget_c..." | Re-trigger Greptile

@codspeed-hq

codspeed-hq Bot commented May 16, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 16 untouched benchmarks


Comparing msxfXF:fix/team-max-budget-gte (8a25348) with main (e58a561)

Open in CodSpeed

@codecov

codecov Bot commented May 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 68.59504% with 38 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
litellm/llms/deepseek/chat/transformation.py 23.33% 23 Missing ⚠️
litellm/router.py 76.66% 7 Missing ⚠️
litellm/cost_calculator.py 87.50% 4 Missing ⚠️
litellm/proxy/proxy_server.py 0.00% 3 Missing ⚠️
litellm/proxy/route_llm_request.py 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@oss-pr-review-agent-shin

Copy link
Copy Markdown
Contributor

🤖 litellm-agent: This PR is currently BLOCKED from merge.

Score: 0/5

Why blocked:

  • karpathy needs_human — The PR diff targets the context line if spend > team_object.max_budget: (without the math.isfinite guard), but the current main branch has already modified that same line to if math.isfinite(team_object.max_budget) and spend > team_object.max_budget:. The PR branch has not been rebased; GitHub will report a merge conflict. The correct post-merge result is if math.isfinite(team_object.max_budget) and spend >= team_object.max_budget:, but a human must confirm the rebase does not silently drop either guard (karpathy, -2 pts)
  • all Phase B agent checks non-approving (phase_b_none_approved, -5 pts)

Details: Score docked for: karpathy needs_human — The PR diff targets the context line if spend > team_object.max_budget: (without the math.isfinite guard), but the current main branch has already modified that same line to if math.isfinite(team_object.max_budget) and spend > team_object.max_budget:. The PR branch has not been rebased; GitHub will report a merge conflict. The correct post-merge result is if math.isfinite(team_object.max_budget) and spend >= team_object.max_budget:, but a human must confirm the rebase does not silently drop either guard; all Phase B agent checks non-approving (karpathy + security + coverage gap). 1 check also red on neighboring PRs (misc / Run tests) — infra-wide noise, no penalty.

Fix the issues above and push an update — the bot will re-review automatically.

Note: This bot is still in beta and might not always work as expected. Please share any feedback via Slack.

@msxfXF
msxfXF changed the base branch from main to litellm_internal_staging May 16, 2026 10:30
@msxfXF
msxfXF force-pushed the fix/team-max-budget-gte branch from 8a25348 to d278f59 Compare May 16, 2026 10:30
@msxfXF

msxfXF commented May 16, 2026

Copy link
Copy Markdown
Author

Thanks for the reviews. Pushed an updated version:

  • Retargeted base to litellm_internal_staging — I now see this is the real default branch and where maintainers merge; main is downstream of it.
  • Rebased on staging. The math.isfinite(team_object.max_budget) guard added in fix: tighten budget field validation and authorization checks #27897 is preserved; the only change is the operator inside it (>>=).

Final one-line diff:

-        if math.isfinite(team_object.max_budget) and spend > team_object.max_budget:
+        if math.isfinite(team_object.max_budget) and spend >= team_object.max_budget:

Regarding the misc / Run tests failure on the previous run — it was tests/test_litellm/interactions/test_openapi_compliance.py::TestResponseCompliance::test_interaction_response_fields failing with Output field 'steps' not in spec. That is unrelated to this change. Will check the new run.

Note: I have not signed the CLA yet — will do that.

@msxfXF

msxfXF commented May 16, 2026

Copy link
Copy Markdown
Author

@greptileai Could you re-run on the rebased commit (d278f59) and post a confidence score? The earlier auto-summary was on the pre-rebase tree. Thanks!

@oss-pr-review-agent-shin

Copy link
Copy Markdown
Contributor

🤖 litellm-agent: Auto-merge skipped — the staging branch shin_agent_oss_staging_05_16_2026 has 4 commit(s) not in your branch. Merging as-is would produce a confusing diff on the staging PR.

Please rebase your branch onto shin_agent_oss_staging_05_16_2026 and push; the agent will re-review automatically.

Divyansh8321 and others added 2 commits May 16, 2026 20:40
…conversations (BerriAI#28080)

Squash-merged by litellm-agent from Divyansh8321's PR.
…ecks

Fixes BerriAI#28020

_team_max_budget_check used `>` while every other budget enforcement
path (key, organization, budget windows) uses `>=`. A team whose spend
hit exactly `max_budget` was still allowed through.

Change `>` to `>=` and add a regression test that asserts
BudgetExceededError is raised when spend == max_budget.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

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.

@github-actions github-actions Bot added the stale label Aug 16, 2026
@github-actions github-actions Bot closed this Aug 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: _team_max_budget_check uses > instead of >= — inconsistent with key/org budget checks

7 participants