diff --git a/agent/bedrock_adapter.py b/agent/bedrock_adapter.py index fbd91ce59446..a6aa210122f1 100644 --- a/agent/bedrock_adapter.py +++ b/agent/bedrock_adapter.py @@ -490,6 +490,26 @@ def convert_tools_to_converse(tools: List[Dict]) -> List[Dict]: return result +# Bedrock's Converse API rejects any text content block whose text is empty +# OR whitespace-only (ValidationException: "text content blocks must contain +# non-whitespace text"). A lone space is whitespace and is rejected too — the +# placeholder MUST itself be non-whitespace. Ref: issue #9486. +_EMPTY_TEXT_PLACEHOLDER = "(empty)" + + +def _safe_text(text) -> str: + """Return ``text`` if it's non-whitespace, else a non-whitespace placeholder. + + Handles None, empty string, and whitespace-only string (spaces, tabs, + newlines) — all of which Bedrock's Converse API rejects as text content. + """ + if text is None: + return _EMPTY_TEXT_PLACEHOLDER + if not isinstance(text, str): + text = str(text) + return text if text.strip() else _EMPTY_TEXT_PLACEHOLDER + + def _convert_content_to_converse(content) -> List[Dict]: """Convert OpenAI message content (string or list) to Converse content blocks. @@ -497,14 +517,15 @@ def _convert_content_to_converse(content) -> List[Dict]: - Plain text strings → [{"text": "..."}] - Content arrays with text/image_url parts → mixed text/image blocks - Filters out empty text blocks — Bedrock's Converse API rejects messages - where a text content block has an empty ``text`` field (ValidationException: - "text content blocks must be non-empty"). Ref: issue #9486. + Replaces empty/whitespace-only text blocks with a non-whitespace + placeholder — Bedrock's Converse API rejects messages where a text + content block is empty or whitespace-only (ValidationException: + "text content blocks must contain non-whitespace text"). Ref: issue #9486. """ if content is None: - return [{"text": " "}] + return [{"text": _safe_text(content)}] if isinstance(content, str): - return [{"text": content}] if content.strip() else [{"text": " "}] + return [{"text": _safe_text(content)}] if isinstance(content, list): blocks = [] for part in content: @@ -516,7 +537,7 @@ def _convert_content_to_converse(content) -> List[Dict]: part_type = part.get("type", "") if part_type == "text": text = part.get("text", "") - blocks.append({"text": text if text else " "}) + blocks.append({"text": _safe_text(text)}) elif part_type == "image_url": image_url = part.get("image_url", {}) url = image_url.get("url", "") if isinstance(image_url, dict) else "" @@ -538,8 +559,8 @@ def _convert_content_to_converse(content) -> List[Dict]: # Remote URL — Converse doesn't support URLs directly, # include as text reference for the model. blocks.append({"text": f"[Image: {url}]"}) - return blocks if blocks else [{"text": " "}] - return [{"text": str(content)}] + return blocks if blocks else [{"text": _EMPTY_TEXT_PLACEHOLDER}] + return [{"text": _safe_text(content)}] def convert_messages_to_converse( @@ -569,14 +590,18 @@ def convert_messages_to_converse( content = msg.get("content") if role == "system": - # System messages become the system prompt + # System messages become the system prompt. Blank/whitespace-only + # parts are dropped entirely (not placeholder-filled) since a + # system prompt made up of only placeholder text is meaningless. if isinstance(content, str) and content.strip(): system_blocks.append({"text": content}) elif isinstance(content, list): for part in content: if isinstance(part, dict) and part.get("type") == "text": - system_blocks.append({"text": part.get("text", "")}) - elif isinstance(part, str): + text = part.get("text", "") + if isinstance(text, str) and text.strip(): + system_blocks.append({"text": text}) + elif isinstance(part, str) and part.strip(): system_blocks.append({"text": part}) continue @@ -587,7 +612,7 @@ def convert_messages_to_converse( tool_result_block = { "toolResult": { "toolUseId": tool_call_id, - "content": [{"text": result_content}], + "content": [{"text": _safe_text(result_content)}], } } # In Converse, tool results go in a "user" role message @@ -626,7 +651,7 @@ def convert_messages_to_converse( }) if not content_blocks: - content_blocks = [{"text": " "}] + content_blocks = [{"text": _EMPTY_TEXT_PLACEHOLDER}] # Merge with previous assistant message if needed (strict alternation) if converse_msgs and converse_msgs[-1]["role"] == "assistant": @@ -652,11 +677,11 @@ def convert_messages_to_converse( # Converse requires the first message to be from the user if converse_msgs and converse_msgs[0]["role"] != "user": - converse_msgs.insert(0, {"role": "user", "content": [{"text": " "}]}) + converse_msgs.insert(0, {"role": "user", "content": [{"text": _EMPTY_TEXT_PLACEHOLDER}]}) # Converse requires the last message to be from the user if converse_msgs and converse_msgs[-1]["role"] != "user": - converse_msgs.append({"role": "user", "content": [{"text": " "}]}) + converse_msgs.append({"role": "user", "content": [{"text": _EMPTY_TEXT_PLACEHOLDER}]}) return (system_blocks if system_blocks else None, converse_msgs) diff --git a/tests/agent/test_bedrock_empty_text_blocks.py b/tests/agent/test_bedrock_empty_text_blocks.py new file mode 100644 index 000000000000..56511ed2f782 --- /dev/null +++ b/tests/agent/test_bedrock_empty_text_blocks.py @@ -0,0 +1,115 @@ +"""Regression tests for Bedrock Converse empty/whitespace text block rejection. + +Bedrock's Converse API raises:: + + ValidationException: The model returned the following errors: messages: + text content blocks must contain non-whitespace text + +for ANY text content block that is empty OR whitespace-only. Anthropic's native +API tolerates these; Bedrock does not. Such blocks most often appear after +context compression rewrites an assistant/tool turn into a blank string — once +in history the block is re-sent on every call and fails deterministically +(all retries fail identically). Ref: issue #9486. + +These tests assert convert_messages_to_converse never emits a blank text block +(including inside toolResult content) and never uses a whitespace-only +placeholder (a lone space would be rejected by the same validation). +""" +import pytest + +from agent.bedrock_adapter import ( + convert_messages_to_converse, + _convert_content_to_converse, + _safe_text, + _EMPTY_TEXT_PLACEHOLDER, +) + + +def _iter_text_blocks(msgs): + """Yield every text string that will be sent to Bedrock, incl. toolResult.""" + for m in msgs: + for b in m["content"]: + if "text" in b: + yield b["text"] + if "toolResult" in b: + for tb in b["toolResult"]["content"]: + if "text" in tb: + yield tb["text"] + + +def test_placeholder_is_non_whitespace(): + # The core lesson of #9486: a space is whitespace and is itself rejected. + assert _EMPTY_TEXT_PLACEHOLDER.strip(), "placeholder must be non-whitespace" + + +@pytest.mark.parametrize("value", ["", " ", "\n\n", "\t", None]) +def test_safe_text_blank_inputs_become_non_whitespace(value): + assert _safe_text(value).strip() + + +def test_safe_text_preserves_real_content(): + assert _safe_text("hello") == "hello" + assert _safe_text(" padded ") == " padded " # inner content kept verbatim + + +def test_no_blank_blocks_reach_bedrock(): + """The exact failing history: blank system/assistant/tool/user turns.""" + messages = [ + {"role": "system", "content": "You are helpful."}, + {"role": "system", "content": [{"type": "text", "text": " "}]}, + {"role": "user", "content": "search for foo"}, + {"role": "assistant", "content": "", + "tool_calls": [{"id": "tc1", + "function": {"name": "search", "arguments": "{}"}}]}, + {"role": "tool", "tool_call_id": "tc1", "content": ""}, # empty tool output + {"role": "assistant", "content": " \n\n "}, # whitespace-only (compaction) + {"role": "user", "content": [{"type": "text", "text": ""}]}, + {"role": "assistant", "content": None}, + ] + _system, msgs = convert_messages_to_converse(messages) + for text in _iter_text_blocks(msgs): + assert text.strip(), f"blank text block would be rejected by Bedrock: {text!r}" + + +def test_empty_tool_result_gets_placeholder(): + """A tool that returns no output must not produce a blank toolResult block.""" + messages = [ + {"role": "user", "content": "run it"}, + {"role": "assistant", "content": "", + "tool_calls": [{"id": "t1", "function": {"name": "sh", "arguments": "{}"}}]}, + {"role": "tool", "tool_call_id": "t1", "content": " "}, + ] + _system, msgs = convert_messages_to_converse(messages) + tool_msg = next(m for m in msgs + if any("toolResult" in b for b in m["content"])) + block = next(b for b in tool_msg["content"] if "toolResult" in b) + text = block["toolResult"]["content"][0]["text"] + assert text.strip() + + +def test_real_content_is_preserved_alongside_blank_siblings(): + messages = [ + {"role": "user", "content": [ + {"type": "text", "text": " "}, + {"type": "text", "text": "real question"}, + ]}, + ] + _system, msgs = convert_messages_to_converse(messages) + texts = list(_iter_text_blocks(msgs)) + assert "real question" in texts + assert all(t.strip() for t in texts) + + +def test_blank_system_blocks_dropped_not_blanked(): + messages = [ + {"role": "system", "content": [ + {"type": "text", "text": "keep me"}, + {"type": "text", "text": " "}, + ]}, + {"role": "user", "content": "hi"}, + ] + system, _msgs = convert_messages_to_converse(messages) + assert system is not None + for block in system: + assert block["text"].strip() + assert any(b["text"] == "keep me" for b in system)