mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-03 02:22:24 +00:00
fix(bedrock): make document names unique across conversation turns (#30093)
* fix(bedrock): make document names unique across conversation turns PR #16275 derived Bedrock document names purely from a content hash so that names stay deterministic for prompt caching. When the same PDF or document appears in more than one conversation turn, every occurrence gets the identical name and Bedrock rejects the request with "Messages can not contain duplicate document names". Add _rename_duplicate_bedrock_document_names, a post-pass over the assembled message blocks that keeps the first occurrence's hash-based name and appends a positional suffix (_2, _3, ...) to later occurrences. Apply it in both _bedrock_converse_messages_pt and _bedrock_converse_messages_pt_async. Names remain deterministic across requests and the first occurrence is unchanged, so prompt cache prefixes stay stable. Fixes #29418. * fix(bedrock): avoid suffix collisions with organic document names A renamed duplicate could collide with a document whose hash-derived name already ends in the same positional suffix (e.g. an organic report_2 next to two documents named report). Collect every document name up front and bump the suffix until the candidate is unused, so renames can collide neither with organic names nor with each other.
This commit is contained in:
parent
48fb5f4e77
commit
365c2352c8
2 changed files with 133 additions and 2 deletions
|
|
@ -4290,6 +4290,49 @@ def _deduplicate_bedrock_tool_content(
|
|||
return _deduplicate_bedrock_content_blocks(tool_content, "toolResult")
|
||||
|
||||
|
||||
def _rename_duplicate_bedrock_document_names(
|
||||
contents: List[BedrockMessageBlock],
|
||||
) -> List[BedrockMessageBlock]:
|
||||
"""
|
||||
Rename duplicate document names across all messages in a Bedrock request.
|
||||
|
||||
Document names are derived from a content hash, so the same file appearing
|
||||
in multiple conversation turns produces identical names and Bedrock rejects
|
||||
the request with "Messages can not contain duplicate document names". The
|
||||
first occurrence keeps its original name so prompt-cache prefixes stay
|
||||
stable; later occurrences get a deterministic positional suffix
|
||||
(``_2``, ``_3``, ...), bumped further if the suffixed name already
|
||||
belongs to another document (e.g. an organic name ending in ``_2``).
|
||||
"""
|
||||
used_names: Set[str] = set()
|
||||
for message in contents:
|
||||
for block in message.get("content") or []:
|
||||
document = block.get("document")
|
||||
if isinstance(document, dict) and document.get("name"):
|
||||
used_names.add(document["name"])
|
||||
|
||||
name_counts: Dict[str, int] = {}
|
||||
for message in contents:
|
||||
for block in message.get("content") or []:
|
||||
document = block.get("document")
|
||||
if not isinstance(document, dict):
|
||||
continue
|
||||
name = document.get("name")
|
||||
if not name:
|
||||
continue
|
||||
count = name_counts.get(name, 0) + 1
|
||||
name_counts[name] = count
|
||||
if count > 1:
|
||||
suffix = count
|
||||
new_name = f"{name}_{suffix}"
|
||||
while new_name in used_names:
|
||||
suffix += 1
|
||||
new_name = f"{name}_{suffix}"
|
||||
used_names.add(new_name)
|
||||
document["name"] = new_name
|
||||
return contents
|
||||
|
||||
|
||||
def _sort_bedrock_assistant_content_blocks(
|
||||
blocks: List[BedrockContentBlock],
|
||||
) -> List[BedrockContentBlock]:
|
||||
|
|
@ -4938,7 +4981,7 @@ class BedrockConverseMessagesProcessor:
|
|||
llm_provider=llm_provider,
|
||||
)
|
||||
|
||||
return contents
|
||||
return _rename_duplicate_bedrock_document_names(contents)
|
||||
|
||||
@staticmethod
|
||||
def translate_thinking_blocks_to_reasoning_content_blocks(
|
||||
|
|
@ -5360,7 +5403,7 @@ def _bedrock_converse_messages_pt( # noqa: PLR0915
|
|||
llm_provider=llm_provider,
|
||||
)
|
||||
|
||||
return contents
|
||||
return _rename_duplicate_bedrock_document_names(contents)
|
||||
|
||||
|
||||
def make_valid_bedrock_tool_name(input_tool_name: str) -> str:
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ from litellm.litellm_core_utils.prompt_templates.factory import (
|
|||
BedrockImageProcessor,
|
||||
_bedrock_converse_messages_pt,
|
||||
_bedrock_tools_pt,
|
||||
_rename_duplicate_bedrock_document_names,
|
||||
_convert_to_bedrock_tool_call_invoke,
|
||||
_convert_to_bedrock_tool_call_result,
|
||||
anthropic_messages_pt,
|
||||
|
|
@ -2809,6 +2810,93 @@ def test_bedrock_converse_messages_pt_document_deterministic_name():
|
|||
assert name1 == name2
|
||||
|
||||
|
||||
def test_bedrock_converse_messages_pt_renames_duplicate_document_names():
|
||||
"""
|
||||
The same document in multiple turns must not produce duplicate names;
|
||||
Bedrock rejects requests with "Messages can not contain duplicate
|
||||
document names". The first occurrence keeps its hash-based name and
|
||||
later occurrences get a deterministic positional suffix.
|
||||
"""
|
||||
document_block = {
|
||||
"type": "document",
|
||||
"source": {
|
||||
"type": "base64",
|
||||
"media_type": "application/pdf",
|
||||
"data": "dGVzdA==",
|
||||
},
|
||||
}
|
||||
messages = [
|
||||
{
|
||||
"role": "user",
|
||||
"content": [document_block, {"type": "text", "text": "summarize this"}],
|
||||
},
|
||||
{"role": "assistant", "content": "It says test."},
|
||||
{
|
||||
"role": "user",
|
||||
"content": [document_block, {"type": "text", "text": "summarize again"}],
|
||||
},
|
||||
]
|
||||
|
||||
result1 = _bedrock_converse_messages_pt(
|
||||
messages, "anthropic.claude-sonnet-4-6", "bedrock"
|
||||
)
|
||||
result2 = _bedrock_converse_messages_pt(
|
||||
messages, "anthropic.claude-sonnet-4-6", "bedrock"
|
||||
)
|
||||
|
||||
names1 = [
|
||||
block["document"]["name"]
|
||||
for message in result1
|
||||
for block in message["content"]
|
||||
if "document" in block
|
||||
]
|
||||
names2 = [
|
||||
block["document"]["name"]
|
||||
for message in result2
|
||||
for block in message["content"]
|
||||
if "document" in block
|
||||
]
|
||||
|
||||
assert len(names1) == 2
|
||||
assert len(set(names1)) == 2
|
||||
assert names1[1] == f"{names1[0]}_2"
|
||||
assert names1 == names2
|
||||
|
||||
single_turn = _bedrock_converse_messages_pt(
|
||||
[messages[0]], "anthropic.claude-sonnet-4-6", "bedrock"
|
||||
)
|
||||
assert names1[0] == single_turn[0]["content"][0]["document"]["name"]
|
||||
|
||||
|
||||
def test_rename_duplicate_bedrock_document_names_skips_organic_suffixes():
|
||||
"""
|
||||
A renamed duplicate must not collide with a document whose organic name
|
||||
already carries the would-be suffix (e.g. an existing ``report_2``),
|
||||
regardless of whether that document appears before or after the rename.
|
||||
"""
|
||||
|
||||
def _contents(names):
|
||||
return [
|
||||
{
|
||||
"role": "user",
|
||||
"content": [{"document": {"name": name}} for name in names],
|
||||
}
|
||||
]
|
||||
|
||||
def _names(contents):
|
||||
return [block["document"]["name"] for block in contents[0]["content"]]
|
||||
|
||||
organic_first = _rename_duplicate_bedrock_document_names(
|
||||
_contents(["report", "report_2", "report"])
|
||||
)
|
||||
assert _names(organic_first) == ["report", "report_2", "report_3"]
|
||||
|
||||
organic_last = _rename_duplicate_bedrock_document_names(
|
||||
_contents(["report", "report", "report_2"])
|
||||
)
|
||||
assert _names(organic_last) == ["report", "report_3", "report_2"]
|
||||
|
||||
|
||||
def test_bedrock_converse_messages_pt_document_rejects_url_source():
|
||||
"""Test that a URL-type document source raises a clear error instead of KeyError."""
|
||||
messages = [
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue