fix(bedrock): address review — route list-string items through _safe_text, update stale placeholder assertions

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.
This commit is contained in:
polyhistor 2026-07-19 10:59:13 +12:00 committed by Teknium
parent 9172048a2f
commit a7e911150c
2 changed files with 29 additions and 11 deletions

View file

@ -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

View file

@ -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