From a7e911150cf20669ca28f66895b2ea4f49833d1d Mon Sep 17 00:00:00 2001 From: polyhistor Date: Sun, 19 Jul 2026 10:59:13 +1200 Subject: [PATCH] =?UTF-8?q?fix(bedrock):=20address=20review=20=E2=80=94=20?= =?UTF-8?q?route=20list-string=20items=20through=20=5Fsafe=5Ftext,=20updat?= =?UTF-8?q?e=20stale=20placeholder=20assertions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses hermes-sweeper review on PR #66167: 1. _convert_content_to_converse() still emitted {"text": part} directly for plain-string items inside a content list (as opposed to {"type": "text"} dicts), bypassing _safe_text() entirely. A whitespace-only string item (e.g. [" "]) could still reach Bedrock as a blank block. Now routed through _safe_text(). 2. tests/agent/test_bedrock_adapter.py::TestEmptyTextBlockFix asserted the pre-fix behavior (whitespace -> literal space " "), contradicting the new _safe_text()/_EMPTY_TEXT_PLACEHOLDER behavior added in 4618095a. Updated assertions to expect the non-whitespace placeholder, plus added a regression test for the list-string-item case above. --- agent/bedrock_adapter.py | 2 +- tests/agent/test_bedrock_adapter.py | 38 +++++++++++++++++++++-------- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/agent/bedrock_adapter.py b/agent/bedrock_adapter.py index 374adb9ac210..614f966e70c7 100644 --- a/agent/bedrock_adapter.py +++ b/agent/bedrock_adapter.py @@ -530,7 +530,7 @@ def _convert_content_to_converse(content) -> List[Dict]: blocks = [] for part in content: if isinstance(part, str): - blocks.append({"text": part}) + blocks.append({"text": _safe_text(part)}) continue if not isinstance(part, dict): continue diff --git a/tests/agent/test_bedrock_adapter.py b/tests/agent/test_bedrock_adapter.py index 62e5af44543c..afb7bbbc948f 100644 --- a/tests/agent/test_bedrock_adapter.py +++ b/tests/agent/test_bedrock_adapter.py @@ -1332,28 +1332,46 @@ class TestIsAnthropicBedrockModel: class TestEmptyTextBlockFix: - """Test that empty text blocks are replaced with space placeholders.""" + """Test that empty/whitespace-only text blocks are replaced with a + non-whitespace placeholder (not a literal space, which is itself + whitespace and gets rejected by the same Bedrock validation rule).""" - def test_none_content_gets_space(self): - from agent.bedrock_adapter import _convert_content_to_converse + def test_none_content_gets_placeholder(self): + from agent.bedrock_adapter import _convert_content_to_converse, _EMPTY_TEXT_PLACEHOLDER blocks = _convert_content_to_converse(None) - assert blocks[0]["text"] == " " + assert blocks[0]["text"] == _EMPTY_TEXT_PLACEHOLDER + assert blocks[0]["text"].strip() - def test_empty_string_gets_space(self): - from agent.bedrock_adapter import _convert_content_to_converse + def test_empty_string_gets_placeholder(self): + from agent.bedrock_adapter import _convert_content_to_converse, _EMPTY_TEXT_PLACEHOLDER blocks = _convert_content_to_converse("") - assert blocks[0]["text"] == " " + assert blocks[0]["text"] == _EMPTY_TEXT_PLACEHOLDER + assert blocks[0]["text"].strip() - def test_whitespace_only_gets_space(self): - from agent.bedrock_adapter import _convert_content_to_converse + def test_whitespace_only_gets_placeholder(self): + from agent.bedrock_adapter import _convert_content_to_converse, _EMPTY_TEXT_PLACEHOLDER blocks = _convert_content_to_converse(" ") - assert blocks[0]["text"] == " " + assert blocks[0]["text"] == _EMPTY_TEXT_PLACEHOLDER + assert blocks[0]["text"].strip() def test_real_text_preserved(self): from agent.bedrock_adapter import _convert_content_to_converse blocks = _convert_content_to_converse("Hello") assert blocks[0]["text"] == "Hello" + def test_whitespace_only_list_string_item_gets_placeholder(self): + """Regression: plain string items inside a content list (not + {"type": "text"} dicts) must also be routed through _safe_text().""" + from agent.bedrock_adapter import _convert_content_to_converse, _EMPTY_TEXT_PLACEHOLDER + blocks = _convert_content_to_converse([" "]) + assert blocks[0]["text"] == _EMPTY_TEXT_PLACEHOLDER + assert blocks[0]["text"].strip() + + def test_real_list_string_item_preserved(self): + from agent.bedrock_adapter import _convert_content_to_converse + blocks = _convert_content_to_converse(["Hello"]) + assert blocks[0]["text"] == "Hello" + # --------------------------------------------------------------------------- # Stale-connection detection and per-region client invalidation