fix(batches): don't crash logging when a completed batch has no output file - #34067
Conversation
Greptile SummaryThis PR prevents all-error completed batches from crashing spend logging.
Confidence Score: 4/5The PR does not yet appear safe to merge because completed Bedrock batches with missing output metadata can be logged as having zero billable usage. The current guard equates every absent output file with an all-error batch, while the Bedrock normalization path can leave Files Needing Attention: litellm/batches/batch_utils.py
|
| Filename | Overview |
|---|---|
| litellm/batches/batch_utils.py | Adds an early zero-accounting return for missing output files, but the condition also covers supported provider states that do not establish an all-error batch. |
| tests/test_litellm/batches/test_batch_utils.py | Adds focused regression coverage for the intended all-error case, including verification that no output fetch occurs. |
Reviews (2): Last reviewed commit: "fix(batch): avoid reading a nonexistent ..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Per repo convention PRs should target litellm_internal_staging rather than litellm_oss_daily_2026_07_20. Please retarget the base branch so this lands on the default staging line. |
0c1e22e to
492730f
Compare
|
I rebased this PR onto litellm_internal_staging and updated the target branch accordingly. Since _handle_completed_batch now reads completed batch data through _fetch_batch_output_file_content, the nil-check has been moved to the call site before that helper is invoked, and the regression test has been updated to stub the helper instead of the previous implementation. The functional change is otherwise identical to the original proposal: 2 modified files with a net addition of 32 lines. I also reran the entire tests/test_litellm/batches/test_batch_utils.py suite, and all 85 tests complete successfully on my machine. |
… batches Completed batches that contain only failed requests do not generate an output file, leaving output_file_id unset while the failures are recorded through error_file_id instead. The completion handler attempted to read the output payload regardless of whether an output file actually existed. During retrieve polling this caused the logging pipeline to fail with "Output file id is None cannot retrieve file content", preventing normal completion bookkeeping from running. Skip output retrieval when no output file is available and return an empty batch summary (zero usage, zero cost, no model entries). The lower-level file retrieval helper still reports an error if it is called directly with an invalid or missing file identifier. Closes BerriAI#33987
492730f to
f91e698
Compare
|
bugbot run |
PR SummaryCursor Bugbot is generating a summary for commit f91e698. Configure here. |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit f91e698. Configure here.
mateo-berri
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the contribution!
2bc87ec
into
BerriAI:litellm_internal_staging
TLDR
Problem this solves:
How it solves it:
User Flow
Before: every status poll of the all-error batch vanishes from the gateway's spend records, and the gateway console prints a file-retrieval error each time
After: every status poll is recorded at zero spend and the console stays clean
Relevant issues
Fixes #33987
Linear ticket
Resolves LIT-4852
Pre-Submission checklist
@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
Live e2e against real OpenAI batches, all request lines carrying an out-of-range
"temperature": 42so every line fails provider-side. Both legs ran the identical setup and differ only in the commit the proxy was booted from: fresh Postgres DB, config withgpt-5.5(openai/gpt-5.5) plus afiles_settingsentry for the plain-provider flow,PROXY_BATCH_POLLING_ENABLED=false(the OSS shape from #33987, no enterprise cost poller), and a user-bound virtual keyBefore, merge base
add095b494cc75440543a240618a88174ea4a445(proxy port 25200): polls never reach the spend logsManaged flow (upload with
target_model_names=gpt-5.5, then create and poll):3 polls made, zero
aretrieve_batchrows. Plain-provider flow (POST /v1/files?provider=openai, raw ids) behaves the same:(the empty-call_type row is from an earlier rejected 403 poll, hours before this run; see observations below)
Every one of the 5 polls across both flows produced a
LoggingWorker error: ... ValueError: Output file id is None cannot retrieve file contenttraceback in the proxy console, 1:1 with the polls, at any batch status, not just completedAfter, PR head
f91e698adbd00b88c3114a276b8a3d0095302ffc(proxy port 52847): every poll is accounted at $0.00Same commands, same flows. Managed flow, 6 polls to the same terminal shape:
Plain-provider flow, 5 polls:
Zero "Output file id is None" occurrences and zero tracebacks in the proxy console across all 11 after-leg polls. Row counts read 4 of 6 and 3 of 5 because the spend log keys retrieve rows on a hash of the response, so consecutive polls observing an identical batch state dedup to one row; that behavior predates this PR, and the row for the completed-observing poll, the one the bug erased, is present in both flows
Observed during QA, all pre-existing and untouched by this PR unless noted:
Type
🐛 Bug Fix