fix(batches): attribute managed batch spend when the creating key has no user_id - #35850
Conversation
… no user_id CheckBatchCost put only user_api_key_user_id=job.created_by on the synthetic logging object it builds for a completed batch. Batches created by a virtual key without a user_id land in LiteLLM_ManagedObjectTable with created_by=NULL, so _should_track_cost_callback saw no key, user, team or end user and _PROXY_track_cost_callback dropped the row without logging anything, while the poller still flipped batch_processed to true; the batch was billed nowhere and never retried. Fall back to LITELLM_PROXY_ADMIN_NAME the same way the aretrieve_batch call already does, and forward the job's team_id so team budgets see batch spend. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Greptile SummaryThis PR repairs managed-batch spend attribution when the creating key has no user ID and propagates the batch’s stored team into the logging pipeline.
Confidence Score: 4/5The PR appears safe to merge, with only a non-blocking test-implementation issue around mutable callback capture state. The production change routes completed-batch cost through the existing user and team attribution fields and covers the null-creator path; the remaining feedback is limited to repository-required test style. Files Needing Attention: tests/proxy_unit_tests/test_check_batch_cost.py
|
| Filename | Overview |
|---|---|
| enterprise/litellm_enterprise/proxy/common_utils/check_batch_cost.py | Correctly supplies fallback user and team metadata to the existing spend callback pipeline; no blocking defect was identified. |
| tests/proxy_unit_tests/test_check_batch_cost.py | Adds meaningful end-to-end callback assertions, but the capture helper violates the repository’s immutability guidance. |
Reviews (1): Last reviewed commit: "fix(batches): attribute managed batch sp..." | Re-trigger Greptile
| from unittest.mock import patch | ||
|
|
||
| import litellm | ||
| from litellm.integrations.custom_logger import CustomLogger | ||
| from litellm.types.utils import Usage |
There was a problem hiding this comment.
Mutable callback capture state
The new callback capture uses a mutable list and captured.append(kwargs), contrary to the repository guidance requiring immutable local values and making the captured callback state less constrained.
Context Used: CLAUDE.md (source)
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!
There was a problem hiding this comment.
Good catch, switched to resolving an asyncio.Future; a second success event now fails loudly instead of piling up.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…le list Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
|
|
Closing: #35468 merged and fixes the same dropped spend row via the unattributed carve-out, including the team_id forwarding this PR also made |
TLDR
Problem this solves:
How it solves it:
Relevant issues
Fixes #35358
Linear ticket
Resolves LIT-5183
Pre-Submission checklist
Screenshots / Proof of Fix
Caveat on realism: this session's network policy only allowlists github, linear and pypi, so
api.openai.comis unreachable from the box. Everything in the run below is real (proxy, postgres, managed files and batches, theCheckBatchCostpoller, the spend logging pipeline) except the provider, which is a local OpenAI-compatible/v1/files+/v1/batchesstub that returnscompletedplus an output file withusageof 1000 prompt / 500 completion tokens on the first retrieve. That is enough to exercise the reported path end to end; the cost the proxy computes is real gpt-4o-mini batch pricing. Worth re-running against live OpenAI before merge on a box that can reach it.Setup, identical for both runs:
Before, at 956d517: the poller reconciles the batch and marks it processed, and no billed row is ever written
The proxy log shows the silent drop; the cost was computed, there was just nothing to attribute it to
After, at this PR's tree (local 0117345cb3, pushed as bb85f3c), same key with no user_id
0.000225 is correct gpt-4o-mini batch pricing for 1000 in / 500 out. A key that does carry a
user_idproduced the same row before and after, attributed to that user, so the existing behaviour is unchangedType
🐛 Bug Fix
Changes
CheckBatchCost._track_completed_batch_costbuilds its ownLiteLLMLoggingobject for the completed batch and put onlyuser_api_key_user_id = job.created_byin its metadata.managed_filesstorescreated_by = user_api_key_dict.user_id, so a batch submitted with a virtual key that has nouser_idgetscreated_by = NULL, and the emitted success event then carries no key hash, user, team or end user._PROXY_track_cost_callbackruns_should_track_cost_callback(None, None, None, None), gets False, and returns without writing anything and without logging a warning, while the poller goes on to setbatch_processed = true; the batch is never retried and the spend is gone.The poller already guards the
aretrieve_batchrouter call withjob.created_by or "default-user-id"; the spend-emitting path just never got the same treatment. It now falls back toLITELLM_PROXY_ADMIN_NAME(the samedefault_user_idthe rest of the proxy attributes unowned spend to) and additionally forwardsjob.team_id, which the managed object row already stores, so team budgets and the team spend views pick batch spend up.Two regression tests were added to
tests/proxy_unit_tests/test_check_batch_cost.py. Every existing test in that file mockslogging_obj.async_success_handler, so nothing there could catch this; the new ones let the realLoggingobject run, capture the event with aCustomLoggerregistered on the async success callbacks, and assert the captured kwargs would pass the exact_should_track_cost_callbackcheck_PROXY_track_cost_callbackapplies. Both fail onmainand pass with the fixNot addressed here:
LiteLLM_ManagedObjectTablenever stores the hash of the key that created the batch, so batch spend can still never be attributed touser_api_keyand per-keymax_budgetcannot see it. That needs a schema column and belongs in its own PRFinal Attestation