mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-29 18:46:59 +00:00
fix(bedrock): use non-whitespace placeholder for empty text blocks
Bedrock Converse rejects text content blocks that are empty OR
whitespace-only (ValidationException: "text content blocks must
contain non-whitespace text"). The prior fix attempt substituted a
single space (" ") for missing content -- but a lone space IS
whitespace, so it was rejected by the exact same validation rule it
was meant to satisfy. This caused a deterministic, unrecoverable
retry-loop failure once any blank/whitespace assistant, tool, or user
turn entered history (most commonly via context-compaction rewriting
a turn to a blank string).
Adds _safe_text()/_EMPTY_TEXT_PLACEHOLDER ("(empty)") and applies it
everywhere a blank text block could reach the wire: user/assistant
content conversion, tool results, the assistant-empty-turn fallback,
and the first/last-message user-alternation padding. System-prompt
blocks are the one exception: blank parts are dropped entirely rather
than placeholder-filled, since a system prompt block should never
carry meaningless placeholder text.
Adds tests/agent/test_bedrock_empty_text_blocks.py (11 tests, was
already present uncommitted -- codifies the exact failing history
from issue #9486 and asserts no blank block ever reaches Bedrock).
Verified against the actual failed request dump from this session
(27-message payload) -- replaying it through the fixed converter now
produces zero blank/whitespace-only blocks.
This commit is contained in:
parent
d86500efea
commit
4618095ab9
2 changed files with 155 additions and 15 deletions
|
|
@ -490,6 +490,26 @@ def convert_tools_to_converse(tools: List[Dict]) -> List[Dict]:
|
||||||
return result
|
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]:
|
def _convert_content_to_converse(content) -> List[Dict]:
|
||||||
"""Convert OpenAI message content (string or list) to Converse content blocks.
|
"""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": "..."}]
|
- Plain text strings → [{"text": "..."}]
|
||||||
- Content arrays with text/image_url parts → mixed text/image blocks
|
- Content arrays with text/image_url parts → mixed text/image blocks
|
||||||
|
|
||||||
Filters out empty text blocks — Bedrock's Converse API rejects messages
|
Replaces empty/whitespace-only text blocks with a non-whitespace
|
||||||
where a text content block has an empty ``text`` field (ValidationException:
|
placeholder — Bedrock's Converse API rejects messages where a text
|
||||||
"text content blocks must be non-empty"). Ref: issue #9486.
|
content block is empty or whitespace-only (ValidationException:
|
||||||
|
"text content blocks must contain non-whitespace text"). Ref: issue #9486.
|
||||||
"""
|
"""
|
||||||
if content is None:
|
if content is None:
|
||||||
return [{"text": " "}]
|
return [{"text": _safe_text(content)}]
|
||||||
if isinstance(content, str):
|
if isinstance(content, str):
|
||||||
return [{"text": content}] if content.strip() else [{"text": " "}]
|
return [{"text": _safe_text(content)}]
|
||||||
if isinstance(content, list):
|
if isinstance(content, list):
|
||||||
blocks = []
|
blocks = []
|
||||||
for part in content:
|
for part in content:
|
||||||
|
|
@ -516,7 +537,7 @@ def _convert_content_to_converse(content) -> List[Dict]:
|
||||||
part_type = part.get("type", "")
|
part_type = part.get("type", "")
|
||||||
if part_type == "text":
|
if part_type == "text":
|
||||||
text = part.get("text", "")
|
text = part.get("text", "")
|
||||||
blocks.append({"text": text if text else " "})
|
blocks.append({"text": _safe_text(text)})
|
||||||
elif part_type == "image_url":
|
elif part_type == "image_url":
|
||||||
image_url = part.get("image_url", {})
|
image_url = part.get("image_url", {})
|
||||||
url = image_url.get("url", "") if isinstance(image_url, dict) else ""
|
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,
|
# Remote URL — Converse doesn't support URLs directly,
|
||||||
# include as text reference for the model.
|
# include as text reference for the model.
|
||||||
blocks.append({"text": f"[Image: {url}]"})
|
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)}]
|
return [{"text": _safe_text(content)}]
|
||||||
|
|
||||||
|
|
||||||
def convert_messages_to_converse(
|
def convert_messages_to_converse(
|
||||||
|
|
@ -569,14 +590,18 @@ def convert_messages_to_converse(
|
||||||
content = msg.get("content")
|
content = msg.get("content")
|
||||||
|
|
||||||
if role == "system":
|
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():
|
if isinstance(content, str) and content.strip():
|
||||||
system_blocks.append({"text": content})
|
system_blocks.append({"text": content})
|
||||||
elif isinstance(content, list):
|
elif isinstance(content, list):
|
||||||
for part in content:
|
for part in content:
|
||||||
if isinstance(part, dict) and part.get("type") == "text":
|
if isinstance(part, dict) and part.get("type") == "text":
|
||||||
system_blocks.append({"text": part.get("text", "")})
|
text = part.get("text", "")
|
||||||
elif isinstance(part, str):
|
if isinstance(text, str) and text.strip():
|
||||||
|
system_blocks.append({"text": text})
|
||||||
|
elif isinstance(part, str) and part.strip():
|
||||||
system_blocks.append({"text": part})
|
system_blocks.append({"text": part})
|
||||||
continue
|
continue
|
||||||
|
|
||||||
|
|
@ -587,7 +612,7 @@ def convert_messages_to_converse(
|
||||||
tool_result_block = {
|
tool_result_block = {
|
||||||
"toolResult": {
|
"toolResult": {
|
||||||
"toolUseId": tool_call_id,
|
"toolUseId": tool_call_id,
|
||||||
"content": [{"text": result_content}],
|
"content": [{"text": _safe_text(result_content)}],
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
# In Converse, tool results go in a "user" role message
|
# In Converse, tool results go in a "user" role message
|
||||||
|
|
@ -626,7 +651,7 @@ def convert_messages_to_converse(
|
||||||
})
|
})
|
||||||
|
|
||||||
if not content_blocks:
|
if not content_blocks:
|
||||||
content_blocks = [{"text": " "}]
|
content_blocks = [{"text": _EMPTY_TEXT_PLACEHOLDER}]
|
||||||
|
|
||||||
# Merge with previous assistant message if needed (strict alternation)
|
# Merge with previous assistant message if needed (strict alternation)
|
||||||
if converse_msgs and converse_msgs[-1]["role"] == "assistant":
|
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
|
# Converse requires the first message to be from the user
|
||||||
if converse_msgs and converse_msgs[0]["role"] != "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
|
# Converse requires the last message to be from the user
|
||||||
if converse_msgs and converse_msgs[-1]["role"] != "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)
|
return (system_blocks if system_blocks else None, converse_msgs)
|
||||||
|
|
||||||
|
|
|
||||||
115
tests/agent/test_bedrock_empty_text_blocks.py
Normal file
115
tests/agent/test_bedrock_empty_text_blocks.py
Normal file
|
|
@ -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)
|
||||||
Loading…
Add table
Add a link
Reference in a new issue