mirror of
https://github.com/BerriAI/litellm.git
synced 2026-08-28 05:25:59 +00:00
Merge pull request #36499 from BerriAI/litellm_bedrock_converse_pdf_text_block_fix
fix(bedrock): add text block to converse user messages carrying documents
This commit is contained in:
commit
422d925334
3 changed files with 170 additions and 6 deletions
|
|
@ -3969,6 +3969,36 @@ def _rename_duplicate_bedrock_document_names(
|
|||
return contents
|
||||
|
||||
|
||||
BEDROCK_DOCUMENT_PLACEHOLDER_TEXT: Final = "."
|
||||
|
||||
|
||||
def _with_text_when_document_only(message: BedrockMessageBlock) -> BedrockMessageBlock:
|
||||
blocks: Final = message["content"]
|
||||
needs_text: Final = (
|
||||
message["role"] == "user"
|
||||
and any("document" in block for block in blocks)
|
||||
and all("text" not in block for block in blocks)
|
||||
)
|
||||
if not needs_text:
|
||||
return message
|
||||
placeholder: Final = BedrockContentBlock(text=BEDROCK_DOCUMENT_PLACEHOLDER_TEXT)
|
||||
cut: Final = len(blocks) - 1 if "cachePoint" in blocks[-1] else len(blocks)
|
||||
return BedrockMessageBlock(role="user", content=[*blocks[:cut], placeholder, *blocks[cut:]])
|
||||
|
||||
|
||||
def _ensure_document_messages_have_text(
|
||||
contents: list[BedrockMessageBlock],
|
||||
) -> list[BedrockMessageBlock]:
|
||||
"""
|
||||
Bedrock Converse rejects any user message that carries a document block
|
||||
without a sibling text block ("A text block must be included when using
|
||||
documents"), e.g. Claude Code sends the PDF as a document-only user turn.
|
||||
Inject a placeholder text block, kept ahead of a trailing cachePoint so
|
||||
the caller's cache boundary stays the final block.
|
||||
"""
|
||||
return [_with_text_when_document_only(message) for message in contents]
|
||||
|
||||
|
||||
def _sort_bedrock_assistant_content_blocks(
|
||||
blocks: list[BedrockContentBlock],
|
||||
) -> list[BedrockContentBlock]:
|
||||
|
|
@ -4537,7 +4567,7 @@ class BedrockConverseMessagesProcessor:
|
|||
llm_provider=llm_provider,
|
||||
)
|
||||
|
||||
return _rename_duplicate_bedrock_document_names(contents)
|
||||
return _ensure_document_messages_have_text(_rename_duplicate_bedrock_document_names(contents))
|
||||
|
||||
@staticmethod
|
||||
def translate_thinking_blocks_to_reasoning_content_blocks(
|
||||
|
|
@ -4913,7 +4943,7 @@ def _bedrock_converse_messages_pt(
|
|||
llm_provider=llm_provider,
|
||||
)
|
||||
|
||||
return _rename_duplicate_bedrock_document_names(contents)
|
||||
return _ensure_document_messages_have_text(_rename_duplicate_bedrock_document_names(contents))
|
||||
|
||||
|
||||
def make_valid_bedrock_tool_name(input_tool_name: str) -> str:
|
||||
|
|
|
|||
|
|
@ -88,10 +88,6 @@ def _build_minimal_pdf(marker: str) -> bytes:
|
|||
return bytes(out)
|
||||
|
||||
|
||||
@pytest.mark.skip(
|
||||
reason="product bug LIT-4523: Bedrock Converse requires a text block with document; "
|
||||
"re-enable when document-only content is handled"
|
||||
)
|
||||
@pytest.mark.covers("llm.messages.bedrock_converse.pdf_input.nonstream.works")
|
||||
def test_pdf_input_bedrock_converse(compat_result, tmp_path):
|
||||
base_url, api_key = require_proxy(compat_result)
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ import pytest
|
|||
import litellm
|
||||
from litellm.litellm_core_utils.prompt_templates.factory import (
|
||||
BAD_MESSAGE_ERROR_STR,
|
||||
BEDROCK_DOCUMENT_PLACEHOLDER_TEXT,
|
||||
BedrockConverseMessagesProcessor,
|
||||
BedrockImageProcessor,
|
||||
_bedrock_converse_messages_pt,
|
||||
|
|
@ -3269,3 +3270,140 @@ def test_group_tool_exchanges_is_linear_in_message_count():
|
|||
|
||||
assert len(groups) == 100_000
|
||||
assert elapsed < 3.0, f"grouping 100k messages took {elapsed:.2f}s; suspect superlinear accumulation"
|
||||
|
||||
|
||||
_PDF_DATA_URI = "data:application/pdf;base64," + base64.b64encode(b"%PDF-1.4 regression fixture").decode()
|
||||
_PNG_DATA_URI = (
|
||||
"data:image/png;base64,"
|
||||
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNkYPhfDwAChwGA60e6kgAAAABJRU5ErkJggg=="
|
||||
)
|
||||
|
||||
|
||||
def _text_blocks(message):
|
||||
return [block["text"] for block in message["content"] if "text" in block]
|
||||
|
||||
|
||||
def test_bedrock_converse_pdf_only_user_message_gets_text_block():
|
||||
"""
|
||||
Regression for LIT-4523: Claude Code sends a PDF as a user turn whose only
|
||||
content is the document (an image_url part with a pdf data URI after the
|
||||
/v1/messages -> completion bridge). Bedrock Converse rejects any user
|
||||
message carrying a document without a sibling text block, so the builder
|
||||
must inject a placeholder text block.
|
||||
"""
|
||||
messages = [
|
||||
{
|
||||
"role": "user",
|
||||
"content": [{"type": "image_url", "image_url": {"url": _PDF_DATA_URI}}],
|
||||
}
|
||||
]
|
||||
|
||||
result = _bedrock_converse_messages_pt(
|
||||
messages, "anthropic.claude-haiku-4-5", "bedrock"
|
||||
)
|
||||
|
||||
assert len(result) == 1
|
||||
assert any("document" in block for block in result[0]["content"])
|
||||
assert _text_blocks(result[0]) == [BEDROCK_DOCUMENT_PLACEHOLDER_TEXT]
|
||||
|
||||
|
||||
def test_bedrock_converse_document_with_text_gets_no_extra_text_block():
|
||||
messages = [
|
||||
{
|
||||
"role": "user",
|
||||
"content": [
|
||||
{"type": "image_url", "image_url": {"url": _PDF_DATA_URI}},
|
||||
{"type": "text", "text": "summarize this"},
|
||||
],
|
||||
}
|
||||
]
|
||||
|
||||
result = _bedrock_converse_messages_pt(
|
||||
messages, "anthropic.claude-haiku-4-5", "bedrock"
|
||||
)
|
||||
|
||||
assert _text_blocks(result[0]) == ["summarize this"]
|
||||
|
||||
|
||||
def test_bedrock_converse_image_only_user_message_gets_no_text_block():
|
||||
messages = [
|
||||
{
|
||||
"role": "user",
|
||||
"content": [{"type": "image_url", "image_url": {"url": _PNG_DATA_URI}}],
|
||||
}
|
||||
]
|
||||
|
||||
result = _bedrock_converse_messages_pt(
|
||||
messages, "anthropic.claude-haiku-4-5", "bedrock"
|
||||
)
|
||||
|
||||
assert any("image" in block for block in result[0]["content"])
|
||||
assert _text_blocks(result[0]) == []
|
||||
|
||||
|
||||
def test_bedrock_converse_tool_round_trip_document_injects_text_before_cache_point():
|
||||
"""
|
||||
Claude Code shape: after a Read tool round trip, the document-only user
|
||||
turn (with cache_control) merges into the toolResult message. The injected
|
||||
text block must land before the trailing cachePoint so the cache boundary
|
||||
stays the final block, and earlier turns must stay untouched.
|
||||
"""
|
||||
messages = [
|
||||
{"role": "user", "content": "read the pdf"},
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": "",
|
||||
"tool_calls": [
|
||||
{
|
||||
"id": "tooluse_pdf1",
|
||||
"type": "function",
|
||||
"function": {"name": "Read", "arguments": "{}"},
|
||||
}
|
||||
],
|
||||
},
|
||||
{"role": "tool", "tool_call_id": "tooluse_pdf1", "content": "read ok"},
|
||||
{
|
||||
"role": "user",
|
||||
"content": [
|
||||
{
|
||||
"type": "document",
|
||||
"source": {
|
||||
"type": "base64",
|
||||
"media_type": "application/pdf",
|
||||
"data": "dGVzdA==",
|
||||
},
|
||||
"cache_control": {"type": "ephemeral"},
|
||||
}
|
||||
],
|
||||
},
|
||||
]
|
||||
|
||||
result = _bedrock_converse_messages_pt(
|
||||
messages, "anthropic.claude-haiku-4-5", "bedrock"
|
||||
)
|
||||
|
||||
assert _text_blocks(result[0]) == ["read the pdf"]
|
||||
document_message = result[-1]
|
||||
block_keys = [next(iter(block)) for block in document_message["content"]]
|
||||
assert block_keys == ["toolResult", "document", "text", "cachePoint"]
|
||||
assert _text_blocks(document_message) == [BEDROCK_DOCUMENT_PLACEHOLDER_TEXT]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_bedrock_converse_pdf_only_user_message_gets_text_block_async():
|
||||
messages = [
|
||||
{
|
||||
"role": "user",
|
||||
"content": [{"type": "image_url", "image_url": {"url": _PDF_DATA_URI}}],
|
||||
}
|
||||
]
|
||||
|
||||
result = await BedrockConverseMessagesProcessor._bedrock_converse_messages_pt_async(
|
||||
messages=messages,
|
||||
model="anthropic.claude-haiku-4-5",
|
||||
llm_provider="bedrock",
|
||||
)
|
||||
|
||||
assert len(result) == 1
|
||||
assert any("document" in block for block in result[0]["content"])
|
||||
assert _text_blocks(result[0]) == [BEDROCK_DOCUMENT_PLACEHOLDER_TEXT]
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue