Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions agent/bedrock_adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,13 @@ def convert_tools_to_converse(tools: List[Dict]) -> List[Dict]:
return result


# Placeholder text for empty/missing content blocks. Bedrock Converse rejects
# both empty strings ("text content blocks must be non-empty", #9486) AND
# whitespace-only strings ("text content blocks must contain non-whitespace
# text", #39829). Use a minimal non-whitespace placeholder that satisfies both.
_EMPTY_CONTENT_PLACEHOLDER = "(no content)"


def _convert_content_to_converse(content) -> List[Dict]:
"""Convert OpenAI message content (string or list) to Converse content blocks.

Expand All @@ -450,9 +457,9 @@ def _convert_content_to_converse(content) -> List[Dict]:
"text content blocks must be non-empty"). Ref: issue #9486.
"""
if content is None:
return [{"text": " "}]
return [{"text": _EMPTY_CONTENT_PLACEHOLDER}]
if isinstance(content, str):
return [{"text": content}] if content.strip() else [{"text": " "}]
return [{"text": content}] if content.strip() else [{"text": _EMPTY_CONTENT_PLACEHOLDER}]
if isinstance(content, list):
blocks = []
for part in content:
Expand All @@ -464,7 +471,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": text if text and text.strip() else _EMPTY_CONTENT_PLACEHOLDER})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This covers dict type: "text" parts, but plain string members in a content list are still appended without .strip() in the preceding branch. Please filter or replace those too, and add a regression test, so every emitted Converse text block satisfies the same contract.

elif part_type == "image_url":
image_url = part.get("image_url", {})
url = image_url.get("url", "") if isinstance(image_url, dict) else ""
Expand All @@ -486,7 +493,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_CONTENT_PLACEHOLDER}]
return [{"text": str(content)}]


Expand Down Expand Up @@ -574,7 +581,7 @@ def convert_messages_to_converse(
})

if not content_blocks:
content_blocks = [{"text": " "}]
content_blocks = [{"text": _EMPTY_CONTENT_PLACEHOLDER}]

# Merge with previous assistant message if needed (strict alternation)
if converse_msgs and converse_msgs[-1]["role"] == "assistant":
Expand All @@ -600,11 +607,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_CONTENT_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_CONTENT_PLACEHOLDER}]})

return (system_blocks if system_blocks else None, converse_msgs)

Expand Down
46 changes: 36 additions & 10 deletions tests/agent/test_bedrock_adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -1240,28 +1240,54 @@ def test_eu_claude(self):


class TestEmptyTextBlockFix:
"""Test that empty text blocks are replaced with space placeholders."""
"""Test that empty/whitespace text blocks are replaced with a non-whitespace placeholder.

def test_none_content_gets_space(self):
from agent.bedrock_adapter import _convert_content_to_converse
Bedrock rejects both empty (#9486) and whitespace-only (#39829) text blocks.
"""

def test_none_content_gets_placeholder(self):
from agent.bedrock_adapter import _convert_content_to_converse, _EMPTY_CONTENT_PLACEHOLDER
blocks = _convert_content_to_converse(None)
assert blocks[0]["text"] == " "
assert blocks[0]["text"] == _EMPTY_CONTENT_PLACEHOLDER
assert blocks[0]["text"].strip(), "placeholder must contain non-whitespace text"

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_CONTENT_PLACEHOLDER
blocks = _convert_content_to_converse("")
assert blocks[0]["text"] == " "
assert blocks[0]["text"] == _EMPTY_CONTENT_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_CONTENT_PLACEHOLDER
blocks = _convert_content_to_converse(" ")
assert blocks[0]["text"] == " "
assert blocks[0]["text"] == _EMPTY_CONTENT_PLACEHOLDER
assert blocks[0]["text"].strip()

def test_whitespace_only_text_part_gets_placeholder(self):
"""A text part inside a content list that is whitespace-only must also be replaced."""
from agent.bedrock_adapter import _convert_content_to_converse, _EMPTY_CONTENT_PLACEHOLDER
blocks = _convert_content_to_converse([{"type": "text", "text": " "}])
assert blocks[0]["text"] == _EMPTY_CONTENT_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_no_whitespace_only_blocks_in_padding(self):
"""First/last user-message padding must not produce whitespace-only blocks."""
from agent.bedrock_adapter import convert_messages_to_converse
# Assistant-first history triggers synthetic user padding at the front
_, converse_msgs = convert_messages_to_converse([
{"role": "assistant", "content": "I'll help."},
{"role": "user", "content": "thanks"},
])
for msg in converse_msgs:
for block in msg.get("content", []):
if "text" in block:
assert block["text"].strip(), f"whitespace-only block found: {block!r}"


# ---------------------------------------------------------------------------
# Stale-connection detection and per-region client invalidation
Expand Down
Loading