From 07bffddbfa4240fcaf5705f2311345aeb1300ff3 Mon Sep 17 00:00:00 2001 From: Simon Lynch <44986346+srlynch1@users.noreply.github.com> Date: Sun, 1 Feb 2026 10:11:16 +1100 Subject: [PATCH] fix(bedrock): deduplicate toolResult and toolUse blocks in Converse message transformation (#20049) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bedrock rejects requests when toolResult or toolUse blocks within a single message contain duplicate IDs. The Converse message transformer merges consecutive tool/assistant messages without checking for duplicate toolUseId values, causing BedrockException errors. Add _deduplicate_bedrock_content_blocks() — a generalized helper that removes duplicate blocks by ID, logs a warning for each dropped duplicate via verbose_logger, and preserves non-tool blocks (e.g. cachePoint). Apply it at all four merge sites (sync/async × toolResult/ toolUse). The Anthropic /messages path was fixed in PR #19324; this applies the equivalent fix to the Bedrock Converse path. Fixes #20048 Co-authored-by: Claude Opus 4.5 --- .../prompt_templates/factory.py | 61 ++++ .../test_bedrock_converse_dedup_factory.py | 332 ++++++++++++++++++ 2 files changed, 393 insertions(+) create mode 100644 tests/litellm_core_utils/test_bedrock_converse_dedup_factory.py diff --git a/litellm/litellm_core_utils/prompt_templates/factory.py b/litellm/litellm_core_utils/prompt_templates/factory.py index 0e1637a65b..09b7c5374d 100644 --- a/litellm/litellm_core_utils/prompt_templates/factory.py +++ b/litellm/litellm_core_utils/prompt_templates/factory.py @@ -3399,6 +3399,59 @@ def _convert_to_bedrock_tool_call_result( return content_block +def _deduplicate_bedrock_content_blocks( + blocks: List[BedrockContentBlock], + block_key: str, + id_key: str = "toolUseId", +) -> List[BedrockContentBlock]: + """ + Remove duplicate content blocks that share the same ID under ``block_key``. + + Bedrock requires all toolResult and toolUse IDs within a single message to + be unique. When merging consecutive messages, duplicates can occur if the + same tool_call_id appears multiple times in conversation history. + + When duplicates exist, the first occurrence is retained and subsequent ones + are discarded. A warning is logged for every dropped block so that + upstream duplication bugs remain visible. + + Blocks that do not contain ``block_key`` (e.g., cachePoint, text) are + always preserved. + + Args: + blocks: The list of Bedrock content blocks to deduplicate. + block_key: The dict key to inspect (e.g. ``"toolResult"`` or ``"toolUse"``). + id_key: The nested key that holds the unique ID (default ``"toolUseId"``). + """ + seen_ids: Set[str] = set() + deduplicated: List[BedrockContentBlock] = [] + for block in blocks: + keyed = block.get(block_key) + if keyed is not None: + block_id = keyed.get(id_key) + if block_id: + if block_id in seen_ids: + verbose_logger.warning( + "Bedrock Converse: dropping duplicate %s block with " + "%s=%s. This may indicate duplicate tool messages in " + "conversation history.", + block_key, + id_key, + block_id, + ) + continue + seen_ids.add(block_id) + deduplicated.append(block) + return deduplicated + + +def _deduplicate_bedrock_tool_content( + tool_content: List[BedrockContentBlock], +) -> List[BedrockContentBlock]: + """Convenience wrapper: deduplicate ``toolResult`` blocks by ``toolUseId``.""" + return _deduplicate_bedrock_content_blocks(tool_content, "toolResult") + + def _insert_assistant_continue_message( messages: List[BedrockMessageBlock], assistant_continue_message: Optional[ @@ -3867,6 +3920,8 @@ class BedrockConverseMessagesProcessor: tool_content.append(cache_point_block) msg_i += 1 + # Deduplicate toolResult blocks with the same toolUseId + tool_content = _deduplicate_bedrock_tool_content(tool_content) if tool_content: # if last message was a 'user' message, then add a blank assistant message (bedrock requires alternating roles) if len(contents) > 0 and contents[-1]["role"] == "user": @@ -3980,6 +4035,8 @@ class BedrockConverseMessagesProcessor: msg_i += 1 + assistant_content = _deduplicate_bedrock_content_blocks(assistant_content, "toolUse") + if assistant_content: contents.append( BedrockMessageBlock(role="assistant", content=assistant_content) @@ -4230,6 +4287,8 @@ def _bedrock_converse_messages_pt( # noqa: PLR0915 tool_content.append(cache_point_block) msg_i += 1 + # Deduplicate toolResult blocks with the same toolUseId + tool_content = _deduplicate_bedrock_tool_content(tool_content) if tool_content: # if last message was a 'user' message, then add a blank assistant message (bedrock requires alternating roles) if len(contents) > 0 and contents[-1]["role"] == "user": @@ -4336,6 +4395,8 @@ def _bedrock_converse_messages_pt( # noqa: PLR0915 msg_i += 1 + assistant_content = _deduplicate_bedrock_content_blocks(assistant_content, "toolUse") + if assistant_content: contents.append( BedrockMessageBlock(role="assistant", content=assistant_content) diff --git a/tests/litellm_core_utils/test_bedrock_converse_dedup_factory.py b/tests/litellm_core_utils/test_bedrock_converse_dedup_factory.py new file mode 100644 index 0000000000..0969c77299 --- /dev/null +++ b/tests/litellm_core_utils/test_bedrock_converse_dedup_factory.py @@ -0,0 +1,332 @@ + +import sys +import os +import pytest + +sys.path.insert(0, os.path.abspath(".")) + +from litellm.litellm_core_utils.prompt_templates.factory import ( + _bedrock_converse_messages_pt, + _deduplicate_bedrock_content_blocks, + _deduplicate_bedrock_tool_content, + BedrockConverseMessagesProcessor, +) + + +MODEL = "anthropic.claude-v2" +PROVIDER = "bedrock_converse" + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_duplicate_tool_result_messages(): + """Return messages where two consecutive tool-role messages reference the + same tool_call_id, simulating the duplication scenario.""" + return [ + {"role": "user", "content": "What's the weather?"}, + { + "role": "assistant", + "content": None, + "tool_calls": [ + { + "id": "tooluse_abc123", + "type": "function", + "function": { + "name": "get_weather", + "arguments": '{"location": "Paris"}', + }, + } + ], + }, + { + "role": "tool", + "tool_call_id": "tooluse_abc123", + "content": '{"temp": 22}', + }, + { + "role": "tool", + "tool_call_id": "tooluse_abc123", # DUPLICATE + "content": '{"temp": 22}', + }, + ] + + +def _make_duplicate_tool_use_messages(): + """Return messages where two consecutive assistant messages carry tool_calls + with the same id, simulating assistant-side duplication.""" + return [ + {"role": "user", "content": "Do something"}, + { + "role": "assistant", + "content": None, + "tool_calls": [ + { + "id": "tool_1", + "type": "function", + "function": {"name": "fn_a", "arguments": "{}"}, + }, + ], + }, + { + "role": "assistant", + "content": None, + "tool_calls": [ + { + "id": "tool_1", # DUPLICATE + "type": "function", + "function": {"name": "fn_a", "arguments": "{}"}, + }, + ], + }, + # Need a tool result so the conversation is valid + { + "role": "tool", + "tool_call_id": "tool_1", + "content": '{"ok": true}', + }, + ] + + +def _extract_blocks(result, role, key): + """Extract all content blocks containing ``key`` from messages with ``role``.""" + return [ + block + for msg in result + if msg["role"] == role + for block in msg["content"] + if key in block + ] + + +# --------------------------------------------------------------------------- +# toolResult dedup tests +# --------------------------------------------------------------------------- + + +def test_bedrock_converse_deduplicates_tool_results(): + """Verify _bedrock_converse_messages_pt deduplicates toolResult blocks + with the same toolUseId when merging consecutive tool messages.""" + messages = _make_duplicate_tool_result_messages() + result = _bedrock_converse_messages_pt(messages, MODEL, PROVIDER) + + tool_results = _extract_blocks(result, "user", "toolResult") + ids = [tr["toolResult"]["toolUseId"] for tr in tool_results] + assert ids.count("tooluse_abc123") == 1 + + +@pytest.mark.asyncio +async def test_bedrock_converse_deduplicates_tool_results_async(): + """Verify the async path also deduplicates toolResult blocks with the + same toolUseId when merging consecutive tool messages.""" + messages = _make_duplicate_tool_result_messages() + result = await BedrockConverseMessagesProcessor._bedrock_converse_messages_pt_async( + messages, MODEL, PROVIDER + ) + + tool_results = _extract_blocks(result, "user", "toolResult") + ids = [tr["toolResult"]["toolUseId"] for tr in tool_results] + assert ids.count("tooluse_abc123") == 1 + + +def test_bedrock_converse_preserves_unique_tool_results(): + """Different toolUseIds should all be preserved.""" + messages = [ + {"role": "user", "content": "Weather and time?"}, + { + "role": "assistant", + "content": None, + "tool_calls": [ + { + "id": "tool_1", + "type": "function", + "function": {"name": "get_weather", "arguments": "{}"}, + }, + { + "id": "tool_2", + "type": "function", + "function": {"name": "get_time", "arguments": "{}"}, + }, + ], + }, + {"role": "tool", "tool_call_id": "tool_1", "content": '{"temp": 22}'}, + {"role": "tool", "tool_call_id": "tool_2", "content": '{"time": "14:00"}'}, + ] + + result = _bedrock_converse_messages_pt(messages, MODEL, PROVIDER) + + tool_results = _extract_blocks(result, "user", "toolResult") + assert len(tool_results) == 2 + ids = {tr["toolResult"]["toolUseId"] for tr in tool_results} + assert ids == {"tool_1", "tool_2"} + + +def test_bedrock_converse_dedup_preserves_cache_points(): + """cachePoint blocks should not be removed during dedup.""" + messages = [ + {"role": "user", "content": "Weather?"}, + { + "role": "assistant", + "content": None, + "tool_calls": [ + { + "id": "tool_1", + "type": "function", + "function": {"name": "get_weather", "arguments": "{}"}, + } + ], + }, + { + "role": "tool", + "tool_call_id": "tool_1", + "content": [ + { + "type": "text", + "text": "sunny", + "cache_control": {"type": "ephemeral"}, + } + ], + }, + { + "role": "tool", + "tool_call_id": "tool_1", # DUPLICATE + "content": '{"temp": 22}', + }, + ] + + result = _bedrock_converse_messages_pt(messages, MODEL, PROVIDER) + + tool_results = _extract_blocks(result, "user", "toolResult") + cache_points = _extract_blocks(result, "user", "cachePoint") + + assert len(tool_results) == 1 + assert len(cache_points) == 1 + + +# --------------------------------------------------------------------------- +# toolUse dedup tests +# --------------------------------------------------------------------------- + + +def test_bedrock_converse_deduplicates_tool_use_sync(): + """Verify the sync path deduplicates toolUse blocks with the same + toolUseId when merging consecutive assistant messages.""" + messages = _make_duplicate_tool_use_messages() + result = _bedrock_converse_messages_pt(messages, MODEL, PROVIDER) + + tool_uses = _extract_blocks(result, "assistant", "toolUse") + ids = [tu["toolUse"]["toolUseId"] for tu in tool_uses] + assert ids.count("tool_1") == 1 + + +@pytest.mark.asyncio +async def test_bedrock_converse_deduplicates_tool_use_async(): + """Verify the async path deduplicates toolUse blocks with the same + toolUseId when merging consecutive assistant messages.""" + messages = _make_duplicate_tool_use_messages() + result = await BedrockConverseMessagesProcessor._bedrock_converse_messages_pt_async( + messages, MODEL, PROVIDER + ) + + tool_uses = _extract_blocks(result, "assistant", "toolUse") + ids = [tu["toolUse"]["toolUseId"] for tu in tool_uses] + assert ids.count("tool_1") == 1 + + +@pytest.mark.asyncio +async def test_bedrock_converse_tool_use_sync_async_parity(): + """Sync and async paths should produce identical results for duplicate + toolUse blocks.""" + messages = _make_duplicate_tool_use_messages() + sync_result = _bedrock_converse_messages_pt(messages, MODEL, PROVIDER) + async_result = await BedrockConverseMessagesProcessor._bedrock_converse_messages_pt_async( + messages, MODEL, PROVIDER + ) + assert sync_result == async_result + + +# --------------------------------------------------------------------------- +# Generalized helper unit tests +# --------------------------------------------------------------------------- + + +def test_deduplicate_bedrock_content_blocks_tool_result(): + """Direct unit test: first occurrence wins, duplicates dropped, non-tool + blocks preserved.""" + blocks = [ + {"toolResult": {"toolUseId": "id_1", "content": [{"text": "a"}]}}, + {"cachePoint": {"type": "default"}}, + {"toolResult": {"toolUseId": "id_1", "content": [{"text": "b"}]}}, # duplicate + {"toolResult": {"toolUseId": "id_2", "content": [{"text": "c"}]}}, + ] + + result = _deduplicate_bedrock_content_blocks(blocks, "toolResult") + + assert len(result) == 3 # id_1, cachePoint, id_2 + tool_ids = [b["toolResult"]["toolUseId"] for b in result if "toolResult" in b] + assert tool_ids == ["id_1", "id_2"] + # First-wins: content "a" is kept, "b" is dropped + assert result[0]["toolResult"]["content"] == [{"text": "a"}] + + +def test_deduplicate_bedrock_content_blocks_tool_use(): + """Direct unit test of toolUse dedup via the generalized helper.""" + blocks = [ + {"toolUse": {"toolUseId": "id_1", "name": "fn_a", "input": {}}}, + {"text": "thinking..."}, + {"toolUse": {"toolUseId": "id_1", "name": "fn_a", "input": {}}}, # duplicate + {"toolUse": {"toolUseId": "id_2", "name": "fn_b", "input": {}}}, + ] + + result = _deduplicate_bedrock_content_blocks(blocks, "toolUse") + + assert len(result) == 3 # id_1, text, id_2 + tool_ids = [b["toolUse"]["toolUseId"] for b in result if "toolUse" in b] + assert tool_ids == ["id_1", "id_2"] + + +def test_deduplicate_preserves_blocks_with_missing_id(): + """Blocks where toolUseId is None or empty should pass through without + dedup tracking (they cannot be compared).""" + blocks = [ + {"toolResult": {"toolUseId": None, "content": [{"text": "a"}]}}, + {"toolResult": {"toolUseId": "", "content": [{"text": "b"}]}}, + {"toolResult": {"toolUseId": "id_1", "content": [{"text": "c"}]}}, + ] + + result = _deduplicate_bedrock_content_blocks(blocks, "toolResult") + + # All three should be preserved — None and "" are not tracked + assert len(result) == 3 + + +def test_deduplicate_bedrock_tool_content_convenience_wrapper(): + """The convenience wrapper should behave identically to calling the + generalized helper with block_key='toolResult'.""" + blocks = [ + {"toolResult": {"toolUseId": "id_1", "content": [{"text": "a"}]}}, + {"toolResult": {"toolUseId": "id_1", "content": [{"text": "b"}]}}, + ] + + assert _deduplicate_bedrock_tool_content(blocks) == _deduplicate_bedrock_content_blocks(blocks, "toolResult") + + +# --------------------------------------------------------------------------- +# Sync/async parity for toolResult +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_bedrock_converse_sync_async_parity_with_duplicates(): + """Sync and async paths should produce identical results with duplicate + tool results.""" + messages = _make_duplicate_tool_result_messages() + + sync_result = _bedrock_converse_messages_pt(messages, MODEL, PROVIDER) + async_result = await BedrockConverseMessagesProcessor._bedrock_converse_messages_pt_async( + messages, MODEL, PROVIDER + ) + + assert sync_result == async_result