Revert "fix: strip empty text content blocks in /v1/messages endpoint" - #23232
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR reverts #23097 ( Key issues:
Confidence Score: 1/5
|
| Filename | Overview |
|---|---|
| litellm/llms/custom_httpx/llm_http_handler.py | Removes _sanitize_anthropic_messages_empty_text_blocks and its call site, re-introducing issue #22930 where empty text content blocks in multi-turn tool-use conversations cause 400 errors from the Anthropic API. The original test failure had a narrow fix that didn't require a full revert. |
| tests/test_litellm/llms/anthropic/test_v1_messages_empty_text_sanitization.py | Entire test file with 10 unit tests for empty-text-block sanitization is deleted, removing all coverage for the /v1/messages multi-turn tool-use scenario described in issue #22930. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Client sends /v1/messages request\nwith multi-turn tool-use history] --> B{Messages contain\nempty text blocks?}
B -- No --> C[Forward to Anthropic API]
B -- Yes --> D_before["BEFORE revert:\n_sanitize_anthropic_messages_empty_text_blocks()"]
D_before --> E{All blocks\nempty text?}
E -- No --> F["Remove empty text blocks\n(keep tool_use, etc.)"]
E -- Yes --> G["Replace with placeholder\n{type: text, text: ...}"]
F --> C
G --> C
C --> H[✅ Anthropic returns 200]
B -- Yes --> D_after["AFTER revert (this PR):\nNo sanitization"]
D_after --> I["Forward empty text blocks\nas-is to Anthropic"]
I --> J["❌ Anthropic returns 400\n'text content blocks must be non-empty'"]
style J fill:#f55,color:#fff
style H fill:#5a5,color:#fff
Comments Outside Diff (1)
-
tests/test_litellm/llms/anthropic/test_v1_messages_empty_text_sanitization.pyTest coverage for a real bug is being deleted
This file contains 10 unit tests that validate the sanitization of empty text content blocks — a behaviour that is required for multi-turn tool-use conversations to work correctly with the Anthropic API (issue [Bug]:
/v1/messagesendpoint does not sanitize empty text content blocks #22930). Deleting it means there is now no test coverage ensuring those scenarios are handled, and anyone who later re-introduces the sanitization fix will have to re-write all these tests.If the revert is necessary as a temporary measure, the test file should be kept and its import updated (or the function temporarily stubbed) to preserve coverage, rather than dropped entirely.
Last reviewed commit: c1b860b
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| LiteLLMLoggingObj = Any | ||
|
|
||
|
|
||
| def _sanitize_anthropic_messages_empty_text_blocks( | ||
| messages: List[Dict], | ||
| ) -> List[Dict]: | ||
| """ | ||
| Strip empty text content blocks from Anthropic-format messages. | ||
|
|
||
| Claude's API returns assistant messages with ``{"type": "text", "text": ""}`` | ||
| alongside ``tool_use`` blocks, but rejects them when sent back in subsequent | ||
| requests. This helper removes those empty text blocks so the /v1/messages | ||
| native path doesn't forward them as-is. | ||
|
|
||
| - If a content list contains a mix of empty text blocks and other blocks | ||
| (e.g. tool_use), the empty text blocks are removed. | ||
| - If *all* blocks in a content list are empty text, the content is replaced | ||
| with a single non-empty placeholder to avoid sending an empty array. | ||
|
|
||
| Ref: https://github.com/BerriAI/litellm/issues/22930 | ||
| """ | ||
| sanitized: List[Dict] = [] | ||
| for message in messages: | ||
| content = message.get("content") | ||
| if not isinstance(content, list): | ||
| sanitized.append(message) | ||
| continue | ||
|
|
||
| filtered = [ | ||
| block | ||
| for block in content | ||
| if not ( | ||
| isinstance(block, dict) | ||
| and block.get("type") == "text" | ||
| and not block.get("text", "").strip() | ||
| ) | ||
| ] | ||
|
|
||
| if filtered == content: | ||
| # Nothing was removed — keep original message as-is. | ||
| sanitized.append(message) | ||
| elif filtered: | ||
| # Some empty text blocks removed, but other content remains. | ||
| new_message = message.copy() | ||
| new_message["content"] = filtered | ||
| sanitized.append(new_message) | ||
| else: | ||
| # All blocks were empty text blocks. Replace with a placeholder | ||
| # so we don't send an empty content array. | ||
| new_message = message.copy() | ||
| new_message["content"] = [{"type": "text", "text": "..."}] | ||
| sanitized.append(new_message) | ||
|
|
||
| return sanitized | ||
|
|
||
|
|
||
| class BaseLLMHTTPHandler: | ||
| async def _make_common_async_call( | ||
| self, |
There was a problem hiding this comment.
Revert re-introduces issue #22930
This revert removes the fix for a confirmed production bug: when Claude returns assistant messages with {"type": "text", "text": ""} alongside tool_use blocks during multi-turn agentic conversations, forwarding those messages back to the API fails with a 400 "text content blocks must be non-empty" error.
The root cause of the test breakage was a narrow, trivially-fixable bug in the sanitization function itself. test_bad_request_error_handling_streaming passes messages=["hi"] (a list containing a bare string, not a dict). The function iterated over messages and called message.get("content"), which raises AttributeError on a string, crashing before the request reached Anthropic's API.
The correct minimal fix would have been a single isinstance guard:
for message in messages:
if not isinstance(message, dict):
sanitized.append(message)
continue
content = message.get("content")
...A full revert is too broad — it undoes a valid fix and re-exposes all users doing multi-turn tool-use conversations via /v1/messages to the original 400-error regression.
|
@Sameerlite Is this issue fixed in any latest version? |
…ze-empty-text-blocks-v1-messages Revert "fix: strip empty text content blocks in /v1/messages endpoint"
2 participants
Reverts #23097
Breaks test_bad_request_error_handling_streaming