fix: make litellm_budget_table optional to prevent Field required 401 crash - #28700
Conversation
…_table Fixes BerriAI#28689 Pydantic v2 treats Optional[T] without a default as required. When budget_id is null, the DB join returns no litellm_budget_table key, causing model_validate to raise 'Field required' → 401 auth crash. Adding = None matches the behavior of budget_id and spend fields.
…udget_table default
Greptile SummaryThis PR fixes a Pydantic v2 "Field required" crash in
Confidence Score: 5/5The change is a targeted, backwards-compatible default-value addition that restores expected behaviour without altering any logic. The fix is a single-field default addition that exactly matches the root cause described in the issue. No logic paths are altered, no existing defaults are changed, and the first regression test directly validates the previously-crashing scenario. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/_types.py | Adds = None default to litellm_budget_table in LiteLLM_TeamMembership, fixing the Pydantic v2 "Field required" crash when the DB join returns no budget row. |
| tests/test_litellm/test__types.py | New regression tests for LiteLLM_TeamMembership; first test correctly covers the missing-key path; second test's docstring claims to verify a populated budget table but passes None instead. |
Reviews (3): Last reviewed commit: "fix: linting" | Re-trigger Greptile
| def test_team_membership_budget_table_present_still_works(): | ||
| """When budget_id exists, litellm_budget_table should still be populated.""" | ||
| data = { | ||
| "user_id": "test-user", | ||
| "team_id": "test-team", | ||
| "budget_id": "some-budget-id", | ||
| "litellm_budget_table": None, | ||
| } | ||
| result = LiteLLM_TeamMembership.model_validate(data) | ||
| assert result.litellm_budget_table is None No newline at end of file |
There was a problem hiding this comment.
The second test's docstring says "When budget_id exists, litellm_budget_table should still be populated," but the test body passes
litellm_budget_table: None and asserts None — it never actually validates that a real budget-table object is accepted and round-trips through model_validate. The test description is misleading and the scenario it claims to cover (budget table present) is untested.
| def test_team_membership_budget_table_present_still_works(): | |
| """When budget_id exists, litellm_budget_table should still be populated.""" | |
| data = { | |
| "user_id": "test-user", | |
| "team_id": "test-team", | |
| "budget_id": "some-budget-id", | |
| "litellm_budget_table": None, | |
| } | |
| result = LiteLLM_TeamMembership.model_validate(data) | |
| assert result.litellm_budget_table is None | |
| def test_team_membership_budget_table_present_still_works(): | |
| """When budget_id exists and litellm_budget_table data is present, it should be populated.""" | |
| data = { | |
| "user_id": "test-user", | |
| "team_id": "test-team", | |
| "budget_id": "some-budget-id", | |
| "litellm_budget_table": { | |
| "budget_id": "some-budget-id", | |
| "max_budget": 100.0, | |
| "rpm_limit": 10, | |
| "tpm_limit": 1000, | |
| }, | |
| } | |
| result = LiteLLM_TeamMembership.model_validate(data) | |
| assert result.litellm_budget_table is not None | |
| assert result.litellm_budget_table.max_budget == 100.0 |
Greptile SummaryThis PR fixes a Pydantic v2
Confidence Score: 5/5The one-line change is correct and well-targeted; adding the The fix is minimal and precise: it aligns The second test in
|
| Filename | Overview |
|---|---|
| litellm/proxy/_types.py | Adds = None default to litellm_budget_table in LiteLLM_TeamMembership, fixing a Pydantic v2 Field required crash when the DB join returns no budget row; also leaves two trailing blank lines after the field. |
| tests/test_litellm/test__types.py | New regression tests for the fix; test_team_membership_budget_table_present_still_works uses a misleading name — it passes litellm_budget_table: None rather than an actual populated budget table object, so it does not cover the "table is present" path it advertises. |
Reviews (2): Last reviewed commit: "test(proxy): add regression test for Lit..." | Re-trigger Greptile
| def test_team_membership_budget_table_present_still_works(): | ||
| """When budget_id exists, litellm_budget_table should still be populated.""" | ||
| data = { | ||
| "user_id": "test-user", | ||
| "team_id": "test-team", | ||
| "budget_id": "some-budget-id", | ||
| "litellm_budget_table": None, | ||
| } | ||
| result = LiteLLM_TeamMembership.model_validate(data) | ||
| assert result.litellm_budget_table is None No newline at end of file |
There was a problem hiding this comment.
The test name says "present still works", but
litellm_budget_table is set to None — so it only exercises the same absent-table path as the first test. An actual populated LiteLLM_BudgetTable or LiteLLM_BudgetTableFull object should be passed to verify that the field is accepted and deserialized correctly when a budget row IS returned from the DB join.
| def test_team_membership_budget_table_present_still_works(): | |
| """When budget_id exists, litellm_budget_table should still be populated.""" | |
| data = { | |
| "user_id": "test-user", | |
| "team_id": "test-team", | |
| "budget_id": "some-budget-id", | |
| "litellm_budget_table": None, | |
| } | |
| result = LiteLLM_TeamMembership.model_validate(data) | |
| assert result.litellm_budget_table is None | |
| def test_team_membership_budget_table_present_still_works(): | |
| """When budget_id exists and a budget row is returned, litellm_budget_table should be populated.""" | |
| from litellm.proxy._types import LiteLLM_BudgetTable | |
| data = { | |
| "user_id": "test-user", | |
| "team_id": "test-team", | |
| "budget_id": "some-budget-id", | |
| "litellm_budget_table": {"budget_id": "some-budget-id", "max_budget": 10.0}, | |
| } | |
| result = LiteLLM_TeamMembership.model_validate(data) | |
| assert result.litellm_budget_table is not None | |
| assert result.litellm_budget_table.max_budget == 10.0 |
| litellm_budget_table: Optional[Union[LiteLLM_BudgetTableFull, LiteLLM_BudgetTable]] = None | ||
|
|
||
|
|
||
|
|
||
| def safe_get_team_member_rpm_limit(self) -> Optional[int]: |
There was a problem hiding this comment.
Two trailing blank lines were left after the field definition. Only one blank line is needed to separate it from the method definitions.
| litellm_budget_table: Optional[Union[LiteLLM_BudgetTableFull, LiteLLM_BudgetTable]] = None | |
| def safe_get_team_member_rpm_limit(self) -> Optional[int]: | |
| litellm_budget_table: Optional[Union[LiteLLM_BudgetTableFull, LiteLLM_BudgetTable]] = None | |
| def safe_get_team_member_rpm_limit(self) -> Optional[int]: |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
@ishaan-jaff could you please review this when you get a chance? |
|
@Sameerlite, @krrish-berri-2, would you please help review? |
|
@greptileai review |
Relevant issues
Fixes #28689
Type
🐛 Bug Fix
Changes
Pydantic v2 treats
Optional[T]without a default as required. Whenbudget_idis null, the DB join returns no
litellm_budget_tablekey, causingmodel_validateto raise
Field required→ 401 auth crash. Adding= Nonematches the behaviorof
budget_idandspendfields.Pre-Submission checklist
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
CI (LiteLLM team)
Link:https://github.com/BerriAI/litellm/actions/runs/26336707362
Link:https://github.com/BerriAI/litellm/actions/runs/26336707362
Links:
Screenshots / Proof of Fix
Before fix:
pydantic_core._pydantic_core.ValidationError: Field required [type=missing]→ 401 auth crash when budget_id is nullAfter fix:
