Skip to content

fix(proxy): coerce numeric budget settings from env var to float - #26867

Closed
xr843 wants to merge 1 commit into
BerriAI:litellm_oss_stagingfrom
xr843:fix/max-budget-env-var-type
Closed

fix(proxy): coerce numeric budget settings from env var to float#26867
xr843 wants to merge 1 commit into
BerriAI:litellm_oss_stagingfrom
xr843:fix/max-budget-env-var-type

Conversation

@xr843

@xr843 xr843 commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #26696. When litellm_settings.max_budget: os.environ/MAX_BUDGET is used in config.yaml, the env var resolves to a string and the litellm_settings loop falls through to the generic setattr(litellm, key, value), leaving litellm.max_budget as a str. The startup check litellm.max_budget > 0 (proxy_server.py:924) then raises TypeError.

The earlier fix (#23843 / #23855) only covered the CLI path (initialize(max_budget=...)), not config.yaml.

Changes

  • litellm/proxy/proxy_server.py: add explicit elif branches in the litellm_settings loop for max_budget, max_user_budget, and max_end_user_budget. Each coerces to float(value), preserving None for the two Optional[float] settings.
  • tests/test_litellm/proxy/test_max_budget_env_var.py: new regression test for the config.yaml path (test_max_budget_from_config_yaml_env_var) plus a parametrized test covering max_user_budget and max_end_user_budget (the symmetric defect — both are Optional[float] in litellm/__init__.py and used in numeric comparisons in litellm/proxy/utils.py).

Test plan

  • pytest tests/test_litellm/proxy/test_max_budget_env_var.py -v → 5/5 pass
  • Verified the new tests fail without the fix (got AssertionError: ... got str), confirming they catch the bug
  • Existing CLI-path tests still pass (test_max_budget_string_converted_to_float, test_max_budget_float_stays_float)

Notes

  • Bundled max_user_budget / max_end_user_budget into the same PR rather than as a follow-up — both have identical failure mode, the fix is one line per key, and they share the same test pattern.
  • Empty/whitespace env var handling: not added here; matches the behavior of the existing CLI-path coercion at proxy_server.py:5885 which is also a bare float(). Can be a separate hardening PR if desired.

@codspeed-hq

codspeed-hq Bot commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 16 untouched benchmarks


Comparing xr843:fix/max-budget-env-var-type (6ec0065) with main (934ecdc)

Open in CodSpeed

@codecov

codecov Bot commented Apr 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.36364% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
litellm/proxy/proxy_server.py 86.36% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

@greptile-apps

greptile-apps Bot commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a startup TypeError (issue #26696) where litellm_settings.max_budget set via os.environ/VAR in config.yaml resolves to a string, causing litellm.max_budget > 0 to blow up. The fix introduces a _coerce_budget_setting helper with proper empty-string and None handling, applied consistently across all five numeric budget keys.

  • Adds _coerce_budget_setting in proxy_server.py and wires it into max_budget, max_user_budget, max_end_user_budget, max_internal_user_budget, and default_max_internal_user_budget — also upgrading the two pre-existing float(value) bare-casts on the last two keys.
  • Adds a focused regression test file covering the config-yaml path, parametrized per-key variants, and direct unit tests of the helper for valid, empty, and invalid inputs.

Confidence Score: 4/5

Safe to merge after fixing the default_max_internal_user_budget propagation guard.

The default_max_internal_user_budget branch moves max_internal_user_budget = default_max_internal_user_budget outside the if coerced_default is not None: guard. When the env var is empty, the assignment to default_max_internal_user_budget is correctly skipped, but max_internal_user_budget can still be overwritten with a stale module-level default — silently applying a budget the user did not intend.

litellm/proxy/proxy_server.py — the default_max_internal_user_budget elif block

Important Files Changed

Filename Overview
litellm/proxy/proxy_server.py Adds _coerce_budget_setting helper and uses it for all five numeric budget keys; the default_max_internal_user_budget branch has a logic flaw where the max_internal_user_budget propagation step runs even when coerced_default is None.
tests/test_litellm/proxy/test_max_budget_env_var.py Adds regression tests for the config.yaml env-var budget coercion path, covering all affected keys and direct unit tests of the helper.

Reviews (2): Last reviewed commit: "fixup: extend safe coercion to internal ..." | Re-trigger Greptile

Comment thread litellm/proxy/proxy_server.py Outdated
@xr843

xr843 commented May 3, 2026

Copy link
Copy Markdown
Contributor Author

Pushed 6ec0065670 to extend the _coerce_budget_setting helper to max_internal_user_budget and default_max_internal_user_budget — greptile flagged that the existing branches at line 3344 had the same bare-float() gap, so this closes the consistency loop across all five budget settings. Also extended the parametrized regression test to cover max_internal_user_budget end-to-end. pytest tests/test_litellm/proxy/test_max_budget_env_var.py — 16/16 pass locally; black + ruff clean on touched files.

@xr843
xr843 changed the base branch from main to litellm_oss_staging May 5, 2026 06:05
@mateo-berri
mateo-berri deleted the branch BerriAI:litellm_oss_staging May 18, 2026 23:27
@xr843

xr843 commented May 21, 2026

Copy link
Copy Markdown
Contributor Author

I noticed this was closed unmerged. Was this superseded by another change, or would you prefer a narrower follow-up PR against the current staging branch?

The original failure mode was the env-var string path for numeric budget settings, and the branch had local regression coverage for the affected budget fields. I can rework or reopen if this is still wanted.

@mateo-berri

Copy link
Copy Markdown
Contributor

Hi, sorry this was closed automatically because the litellm_oss_staging branch was auto-deleted after it was merged in. We've since disabled auto-deletion of branches. Reopening

@mateo-berri mateo-berri reopened this May 23, 2026
Numeric budget settings (max_budget, max_user_budget, max_end_user_budget,
max_internal_user_budget, default_max_internal_user_budget) may arrive as
strings when loaded via os.environ/... in config.yaml. Coerce them to float
via a shared _coerce_budget_setting helper so downstream numeric comparisons
don't raise TypeError. Empty/whitespace env vars resolve to the unset default.

See GitHub issue BerriAI#26696.
@xr843
xr843 force-pushed the fix/max-budget-env-var-type branch from 6ec0065 to 6970f34 Compare May 29, 2026 09:33
@xr843

xr843 commented May 29, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for reopening, @mateo-berri 🙏

Rebased onto the current litellm_oss_staging and resolved the conflict. The drift was purely positional — upstream added the _scrub_db_overlay_remote_module_loads helpers in the same module-level block where this PR introduces _coerce_budget_setting, so both are kept side by side. I also collapsed the original four incremental commits into a single commit to keep the history clean.

All five budget settings (max_budget, max_user_budget, max_end_user_budget, max_internal_user_budget, default_max_internal_user_budget) now route through the shared helper, and the 16 regression tests in tests/test_litellm/proxy/test_max_budget_env_var.py pass locally. Ready for review whenever you have a cycle.

@xr843

xr843 commented May 30, 2026

Copy link
Copy Markdown
Contributor Author

Heads-up on the two red checks — both are pre-existing/infra and unrelated to this diff (which touches only litellm/proxy/proxy_server.py + a new tests/test_litellm/proxy/test_max_budget_env_var.py):

  1. misc / Run teststest_openapi_compliance.py::test_status_enum_values — pre-existing on litellm_oss_staging. The OpenAPI response-status enum already exposes budget_exceeded, but that test's expected_statuses on the staging branch hasn't been updated to include it (the list is already corrected on main). This PR doesn't modify the status enum or that compliance test.

  2. test-server-root-path — Docker build flake: the image build fails at prisma generate (npm install prisma@5.4.2 → exit code 127) before any test runs.

The two max_budget env-var tests and the rest of the suite pass. Happy to rebase again once the staging enum-test sync lands, if that's cleaner. 🙏

@mateo-berri
mateo-berri deleted the branch BerriAI:litellm_oss_staging June 2, 2026 15:48
@mateo-berri mateo-berri closed this Jun 2, 2026
@xr843
xr843 deleted the fix/max-budget-env-var-type branch June 3, 2026 03:55
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