From 2b5a2af2032071608bf87f9a082ae637302f89c9 Mon Sep 17 00:00:00 2001 From: taoxee <61774203+taoxee@users.noreply.github.com> Date: Thu, 16 Jul 2026 19:04:11 +0800 Subject: [PATCH] fix(bedrock): use non-whitespace placeholder for empty Converse content blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bedrock's Converse/ConverseStream API rejects text content blocks whose text is empty OR whitespace-only: ValidationException: ... messages: text content blocks must contain non-whitespace text convert_messages_to_converse() used a single space {"text": " "} as its placeholder for otherwise-empty content in six places, and appended empty/whitespace text blocks for system messages, list string parts, and tool results. A single space is whitespace-only, so it trips the exact error above. The AnthropicBedrock SDK path tolerated these blocks, which is why this only surfaces on the Converse path. - Introduce _EMPTY_TEXT_PLACEHOLDER = "(empty)" and use it for all empty-content fallbacks (replacing the six {"text": " "} spots). - Add _nonblank_text(value) — isinstance(value, str) and value.strip() — and gate every text-block append with it, so empty/whitespace/non-string text (None, ints) is dropped or replaced, never emitted or raised on. Tests: update the pre-existing TestEmptyTextBlockFix / placeholder tests to assert the non-whitespace invariant, and add TestConverseNoWhitespaceOnlyTextBlocks with a recursive walk asserting no emitted text block (including nested toolResult content) is blank, plus a non-string text-part case that must be filtered rather than crash. --- agent/bedrock_adapter.py | 53 +++++++--- tests/agent/test_bedrock_adapter.py | 159 ++++++++++++++++++++++++++-- 2 files changed, 190 insertions(+), 22 deletions(-) diff --git a/agent/bedrock_adapter.py b/agent/bedrock_adapter.py index 51d0afbe3cbe..2e71c912880a 100644 --- a/agent/bedrock_adapter.py +++ b/agent/bedrock_adapter.py @@ -490,6 +490,24 @@ def convert_tools_to_converse(tools: List[Dict]) -> List[Dict]: return result +# Placeholder for otherwise-empty content blocks. Bedrock's Converse API +# rejects text content blocks that are empty OR whitespace-only with +# ``ValidationException: text content blocks must contain non-whitespace +# text``. A single space (the previous placeholder) is whitespace-only and +# still trips this — so the placeholder must contain a non-whitespace char. +_EMPTY_TEXT_PLACEHOLDER = "(empty)" + + +def _nonblank_text(value) -> bool: + """True only when ``value`` is a string with non-whitespace content. + + Used to gate every Converse text-block emit. Guards against non-string + ``text`` payloads (None, ints, nested dicts) that would raise on + ``.strip()`` — those are simply treated as blank and dropped/replaced. + """ + return isinstance(value, str) and bool(value.strip()) + + def _convert_content_to_converse(content) -> List[Dict]: """Convert OpenAI message content (string or list) to Converse content blocks. @@ -497,26 +515,29 @@ 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. + Filters out empty/whitespace-only text blocks — 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": _EMPTY_TEXT_PLACEHOLDER}] if isinstance(content, str): - return [{"text": content}] if content.strip() else [{"text": " "}] + return [{"text": content}] if content.strip() else [{"text": _EMPTY_TEXT_PLACEHOLDER}] if isinstance(content, list): blocks = [] for part in content: if isinstance(part, str): - blocks.append({"text": part}) + if part.strip(): + blocks.append({"text": part}) continue if not isinstance(part, dict): continue part_type = part.get("type", "") if part_type == "text": text = part.get("text", "") - blocks.append({"text": text if text else " "}) + if _nonblank_text(text): + blocks.append({"text": text}) elif part_type == "image_url": image_url = part.get("image_url", {}) url = image_url.get("url", "") if isinstance(image_url, dict) else "" @@ -547,7 +568,7 @@ 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 blocks if blocks else [{"text": _EMPTY_TEXT_PLACEHOLDER}] return [{"text": str(content)}] @@ -584,8 +605,10 @@ def convert_messages_to_converse( 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): + _t = part.get("text", "") + if _nonblank_text(_t): + system_blocks.append({"text": _t}) + elif _nonblank_text(part): system_blocks.append({"text": part}) continue @@ -593,6 +616,10 @@ def convert_messages_to_converse( # Tool result messages → merge into the preceding user turn tool_call_id = msg.get("tool_call_id", "") result_content = content if isinstance(content, str) else json.dumps(content) + # Converse rejects empty/whitespace-only text blocks (incl. tool + # results), so substitute a non-whitespace placeholder. + if not _nonblank_text(result_content): + result_content = _EMPTY_TEXT_PLACEHOLDER tool_result_block = { "toolResult": { "toolUseId": tool_call_id, @@ -635,7 +662,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": @@ -661,11 +688,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_adapter.py b/tests/agent/test_bedrock_adapter.py index 26a59a33e2a3..5a9cd2814ca5 100644 --- a/tests/agent/test_bedrock_adapter.py +++ b/tests/agent/test_bedrock_adapter.py @@ -326,8 +326,10 @@ def test_empty_content_gets_placeholder(self): from agent.bedrock_adapter import convert_messages_to_converse messages = [{"role": "user", "content": ""}] system, msgs = convert_messages_to_converse(messages) - # Empty string should get a space placeholder - assert msgs[0]["content"][0]["text"].strip() != "" or msgs[0]["content"][0]["text"] == " " + # Empty string must get a NON-whitespace placeholder — Converse rejects + # empty/whitespace-only text blocks (must contain non-whitespace text). + placeholder = msgs[0]["content"][0]["text"] + assert placeholder.strip() != "" def test_image_data_url_converted(self): from agent.bedrock_adapter import convert_messages_to_converse @@ -361,6 +363,142 @@ def test_multiple_system_messages_merged(self): assert system[1]["text"] == "Rule 2" +def _iter_converse_text_blocks(node): + """Yield every ``text`` value emitted anywhere in a Converse payload. + + Walks system blocks, message content blocks, and nested toolResult + content so the whitespace invariant can be asserted recursively. + """ + if isinstance(node, dict): + for key, value in node.items(): + if key == "text": + yield value + else: + yield from _iter_converse_text_blocks(value) + elif isinstance(node, list): + for item in node: + yield from _iter_converse_text_blocks(item) + + +class TestConverseNoWhitespaceOnlyTextBlocks: + """Bedrock's Converse API rejects empty/whitespace-only text blocks with + ``ValidationException: text content blocks must contain non-whitespace + text``. No emitted text block — including nested tool results — may be + blank. Ref: issue #9486, PR #64858.""" + + def _assert_all_text_nonblank(self, system, msgs): + texts = list(_iter_converse_text_blocks(system)) + list( + _iter_converse_text_blocks(msgs) + ) + # There must be at least one text block (or the walk is vacuous), and + # every one must be a non-whitespace string. + for text in texts: + assert isinstance(text, str), f"non-string text block: {text!r}" + assert text.strip() != "", f"whitespace-only text block: {text!r}" + + def test_empty_and_whitespace_user_content(self): + from agent.bedrock_adapter import convert_messages_to_converse + + for content in ("", " ", "\n\t ", None): + system, msgs = convert_messages_to_converse( + [{"role": "user", "content": content}] + ) + self._assert_all_text_nonblank(system, msgs) + + def test_whitespace_only_list_parts_dropped(self): + from agent.bedrock_adapter import convert_messages_to_converse + + messages = [{ + "role": "user", + "content": [ + {"type": "text", "text": " "}, + {"type": "text", "text": "\n"}, + " ", + {"type": "text", "text": "real content"}, + ], + }] + system, msgs = convert_messages_to_converse(messages) + self._assert_all_text_nonblank(system, msgs) + # The one real block survives; the blanks are dropped. + user_texts = [ + b["text"] for b in msgs[0]["content"] if "text" in b + ] + assert user_texts == ["real content"] + + def test_all_blank_list_parts_get_placeholder(self): + from agent.bedrock_adapter import convert_messages_to_converse + + messages = [{ + "role": "user", + "content": [{"type": "text", "text": " "}, {"type": "text", "text": ""}], + }] + system, msgs = convert_messages_to_converse(messages) + self._assert_all_text_nonblank(system, msgs) + + def test_empty_tool_result(self): + from agent.bedrock_adapter import convert_messages_to_converse + + for result in ("", " ", None): + messages = [ + {"role": "user", "content": "run it"}, + {"role": "tool", "tool_call_id": "t1", "content": result}, + ] + system, msgs = convert_messages_to_converse(messages) + self._assert_all_text_nonblank(system, msgs) + # The tool result block itself must be non-blank. + tool_texts = [ + inner["text"] + for m in msgs + for block in m["content"] + if "toolResult" in block + for inner in block["toolResult"]["content"] + if "text" in inner + ] + assert tool_texts and all(t.strip() for t in tool_texts) + + def test_empty_assistant_content(self): + from agent.bedrock_adapter import convert_messages_to_converse + + messages = [ + {"role": "user", "content": "hi"}, + {"role": "assistant", "content": ""}, + {"role": "user", "content": "still there?"}, + ] + system, msgs = convert_messages_to_converse(messages) + self._assert_all_text_nonblank(system, msgs) + + def test_whitespace_system_message(self): + from agent.bedrock_adapter import convert_messages_to_converse + + messages = [ + {"role": "system", "content": [ + {"type": "text", "text": " "}, + {"type": "text", "text": "Be concise."}, + " ", + ]}, + {"role": "user", "content": "hi"}, + ] + system, msgs = convert_messages_to_converse(messages) + self._assert_all_text_nonblank(system, msgs) + assert [b["text"] for b in (system or [])] == ["Be concise."] + + def test_non_string_text_part_does_not_raise(self): + """A non-string ``text`` payload must be filtered, not crash on .strip().""" + from agent.bedrock_adapter import convert_messages_to_converse + + messages = [{ + "role": "user", + "content": [ + {"type": "text", "text": None}, + {"type": "text", "text": 123}, + {"type": "text", "text": "ok"}, + ], + }] + system, msgs = convert_messages_to_converse(messages) + self._assert_all_text_nonblank(system, msgs) + assert [b["text"] for b in msgs[0]["content"] if "text" in b] == ["ok"] + + # --------------------------------------------------------------------------- # Response normalization: Converse → OpenAI # --------------------------------------------------------------------------- @@ -1305,22 +1443,25 @@ def test_eu_claude(self): class TestEmptyTextBlockFix: - """Test that empty text blocks are replaced with space placeholders.""" + """Empty/whitespace-only content is replaced with a NON-whitespace + placeholder. Bedrock's Converse API rejects text blocks that are empty or + whitespace-only ("must contain non-whitespace text"), so a single space + is not a valid placeholder. Ref: PR #64858.""" - def test_none_content_gets_space(self): + def test_none_content_gets_nonblank_placeholder(self): from agent.bedrock_adapter import _convert_content_to_converse blocks = _convert_content_to_converse(None) - assert blocks[0]["text"] == " " + assert blocks[0]["text"].strip() != "" - def test_empty_string_gets_space(self): + def test_empty_string_gets_nonblank_placeholder(self): from agent.bedrock_adapter import _convert_content_to_converse blocks = _convert_content_to_converse("") - assert blocks[0]["text"] == " " + assert blocks[0]["text"].strip() != "" - def test_whitespace_only_gets_space(self): + def test_whitespace_only_gets_nonblank_placeholder(self): from agent.bedrock_adapter import _convert_content_to_converse blocks = _convert_content_to_converse(" ") - assert blocks[0]["text"] == " " + assert blocks[0]["text"].strip() != "" def test_real_text_preserved(self): from agent.bedrock_adapter import _convert_content_to_converse