Conversation
`cleanup_none_field_in_message` dropped every key whose value was None,
so an assistant message carrying `tool_calls` lost its `content` key
entirely. That is the shape the OpenAI spec prescribes for a tool-call-only
turn, and providers that require the key to be present reject the request.
The helper's purpose, per its own docstring, is removing stray keys like
`{"function": None}` that trip provider validation — not `content`. Exempt
`content` when the assistant message carries `tool_calls` or a legacy
`function_call`; everything else still gets stripped, including a None
`content` on an assistant message with neither.
Reproduces with a plain `litellm.completion()` call, no proxy and no
credentials, via the intercept in the issue.
Fixes BerriAI#37711
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR adjusts OpenAI-message normalization so assistant tool-call and legacy function-call turns retain an explicit
Confidence Score: 4/5The PR appears safe to merge functionally, with only non-blocking repository-convention cleanup needed for the new tests. The normalization change is narrowly guarded and the added cases cover its intended behavior; the remaining concern is test organization and disallowed commentary rather than runtime correctness. Files Needing Attention: tests/test_litellm/test_cleanup_none_field_in_message.py
|
| Filename | Overview |
|---|---|
| litellm/utils.py | Preserves null content for assistant tool/function-call turns without changing cleanup of other null-valued fields. |
| tests/test_litellm/test_cleanup_none_field_in_message.py | Adds comprehensive regression coverage, but does so in a new test file and includes an ordinary explanatory comment contrary to repository conventions. |
Reviews (1): Last reviewed commit: "fix: keep content: null on an assistant ..." | Re-trigger Greptile
| @@ -0,0 +1,68 @@ | |||
| """`content: null` is the OpenAI-prescribed shape for an assistant tool-call turn.""" | |||
There was a problem hiding this comment.
Regression tests are misplaced
This bug fix creates a standalone test module for an existing helper and adds an ordinary explanatory comment at line 34, fragmenting the helper’s regression coverage and violating the repository’s test-placement and source-comment conventions. Move these cases into the existing mapped test module and remove the redundant comment.
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!
Fixes #37711.
Root cause
I traced the reporter's repro to
cleanup_none_field_in_messageinlitellm/utils.py:It drops every key whose value is
None. For an assistant message carryingtool_calls,content: nullis exactly the shape the OpenAI spec prescribesfor a tool-call-only turn, so the key is removed and providers that require it
to be present reject the request.
The helper's own docstring says the intent is removing stray keys like
{"function": None}that trip provider validation —contentwas collateral.It is reached via
validate_and_fix_openai_messages, which is why thisreproduces with a plain
litellm.completion()call: no proxy, no provider, noAPI key.
The fix
Exempt
contentwhen the assistant message carriestool_callsor a legacyfunction_call. Everything else is stripped exactly as before — including aNonecontenton an assistant message that has neither, so the originalbehaviour is preserved wherever it was not the bug.
Verification
Six tests in
tests/test_litellm/test_cleanup_none_field_in_message.py:content: nullsurvives on atool_callsturn, and on a legacyfunction_callturnNonefields on that same turn (function,name) are stillstripped — the helper's original purpose is intact
content: nullis still stripped from an assistant message with no tool callvalidate_and_fix_openai_messages, the callerthat was actually dropping the field
4 of the 6 fail without the source change, and the reporter's own repro script
now prints
content key present: True.Regression check. I ran the provider suites most likely to depend on the old
behaviour —
tests/test_litellm/llms/{openai,anthropic,databricks,mistral}—2169 passed. There are 54 failures in that set, but the identical 54 fail on a
clean checkout with nothing applied (verified by stashing and re-running:
54 failed, 268 passedon both), so none are caused by this change.ruff format --checkclean on both touched filesruff check litellm/utils.py: same findings as base, none addedOn the red
code-qualitycheck: it is not from this PR. It fails on everyPR against
litellm_internal_stagingright now, including ones that touch noworkflow files, because three unit shards in
.github/workflows/test-unit.ymlcap the job below the startup-safety invariant. Fixed independently in #38046;
this PR needs no change for it.