Skip to content

fix(reset_budget_job): write minimal reset updates - #28453

Closed
pyguy wants to merge 5 commits into
BerriAI:litellm_oss_stagingfrom
pyguy:fix/reset-budget-targeted-writes-27730-oss
Closed

fix(reset_budget_job): write minimal reset updates#28453
pyguy wants to merge 5 commits into
BerriAI:litellm_oss_stagingfrom
pyguy:fix/reset-budget-targeted-writes-27730-oss

Conversation

@pyguy

@pyguy pyguy commented May 21, 2026

Copy link
Copy Markdown

Summary

  • Fixes reset budget key/team writes by sending only identifier, spend, and budget_reset_at through update_data.
  • Adds dict payload support to PrismaClient.update_data update_many for key/team, preserving existing object payload behavior.
  • Adds regression coverage for key/team rows with object_permission_id and budget_limits, including dict/object update_data payload variants.

Fixes #27730.

Testing

  • uv run pytest tests/test_litellm/proxy/common_utils/test_reset_budget_job.py -x -vv (49 passed)
  • uv run pytest tests/litellm_utils_tests/test_proxy_budget_reset.py -x -vv (14 passed)
  • Live k8s dev validation on ai-tools-dev/litellm: unpatched pod reproduced the Prisma DataError for key and team reset rows with object_permission_id + budget_limits; patched pod reset both rows successfully with spend=0.0, advanced budget_reset_at, retained object_permission_id and budget_limits, and failure hook count 0. Both running dev pods were hot-patched with the implementation files afterward.

@codecov

codecov Bot commented May 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@greptile-apps

greptile-apps Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a Prisma DataError that occurred when resetting budgets for keys and teams that had object_permission_id or budget_limits populated — those fields were being forwarded wholesale to the DB update and rejected by Prisma. The fix narrows the write payload to only the three fields that should change: token/team_id, spend, and budget_reset_at.

  • reset_budget_job.py: Projects updated_keys / updated_teams into minimal dicts before passing to update_data, eliminating the stray field problem at the call site.
  • utils.py: Adds a dict-payload branch in the update_many handlers for both the key and team tables so the new minimal dicts are routed correctly alongside the existing object-based path.
  • Tests: Fixtures gain token/team_id fields; assertions tighten to check the exact set of keys written, and four new unit tests cover both dict and object payload variants through the updated update_data code.

Confidence Score: 4/5

Safe to merge — the change is narrowly scoped to the budget reset write path, the root-cause fix is correct, and live k8s validation plus 63 passing tests back it up.

The core fix is sound: extracting a minimal dict before the DB write prevents Prisma from receiving unexpected JSON/relation fields. One cosmetic issue exists in utils.py — data_json["token"] = token is assigned redundantly after jsonify_object already includes it from the dict comprehension — but this has no effect on runtime behaviour. No logic regressions were identified.

litellm/proxy/utils.py — the redundant token reassignment on line 3885 is worth cleaning up, but it has no runtime impact.

Important Files Changed

Filename Overview
litellm/proxy/common_utils/reset_budget_job.py Projects only token/team_id, spend, and budget_reset_at into minimal dicts before calling update_data, fixing the Prisma DataError caused by unexpected fields like object_permission_id and budget_limits.
litellm/proxy/utils.py Adds dict-payload branch to update_many for key and team tables; contains a redundant data_json["token"] = token assignment after the token is already present from the dict comprehension passed to jsonify_object.
tests/test_litellm/proxy/common_utils/test_reset_budget_job.py Adds token/team_id fields to test fixtures, updates assertions to reflect dict-payload shape, and adds four new direct unit tests for the update_data dict and object paths for both key and team tables.
tests/litellm_utils_tests/test_proxy_budget_reset.py Test fixtures updated to include token/team_id fields; assertions changed from id-based checks to token/team_id with key-set assertions that verify no extra fields leak into the DB payload.

Reviews (1): Last reviewed commit: "test(reset_budget_job): cover update_dat..." | Re-trigger Greptile

Comment thread litellm/proxy/utils.py Outdated
@oss-pr-review-agent-shin

Copy link
Copy Markdown
Contributor

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

Score: 3/5

Why blocked:

  • 1 PR-related CI failure (Size gate: tests (+286) exceed code (+75) by more than 3× — over-specified or feature too thin. Add the oversized-ok label if intentional.) (pr_related_failures, -2 pts)

Details: Score docked for: 1 PR-related CI failure (Size gate: tests (+286) exceed code (+75) by more than 3× — over-specified or feature too thin. Add the oversized-ok label if intentional.).

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.

@pyguy

pyguy commented May 21, 2026

Copy link
Copy Markdown
Author

All previously reported blockers are addressed on the latest commit: the size gate is within bounds (code +74, tests +222), Codecov patch coverage passes, and all GitHub checks are green. Requesting a fresh LiteLLM agent review.

@pyguy

pyguy commented May 25, 2026

Copy link
Copy Markdown
Author

@krrish-berri any chance to have this merged? thanks.

@pyguy

pyguy commented Jun 1, 2026

Copy link
Copy Markdown
Author

Fixed by #29358.

@pyguy pyguy closed this Jun 1, 2026
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.

1 participant