fix(bedrock): drop whitespace-only content blocks instead of padding them - #61318
Conversation
…them
Bedrock Converse rejects any text content block whose payload is empty
or whitespace-only:
ValidationException: text content blocks must contain non-whitespace text
The previous sanitizer (issue NousResearch#9486) tried to smuggle these through by
substituting a single space " " as a placeholder. That value is itself
whitespace-only and still gets rejected by Bedrock — reproduced in a
retro session with ~47k tokens / msgs=14 where a tool call returned
empty stdout and every subsequent turn failed all 3 retries.
New policy: don't post invalid content, drop it.
- user message with empty/whitespace content → drop the message
- assistant with whitespace-only text but real tool_calls → drop the
text block, keep the tool_calls (the turn is not empty)
- assistant with neither text nor tool_calls → drop the message
- system block with whitespace-only content → drop the block
- tool result with whitespace-only content → keep as
"[tool returned no output]" sentinel. Dropping a tool result would
orphan its matching toolUse in the prior assistant turn, and
Bedrock rejects orphan toolUse blocks too, so this is the one
place a sentinel is unavoidable. The sentinel is descriptive so
the model interprets it correctly.
- first/last-must-be-user guard (pathological path) now uses
"[conversation start]" / "[continue]" instead of " "
Also aligned the test suite with the real contract:
- TestEmptyTextBlockFix now asserts empty content is *dropped*, not
padded with a placeholder (change-detector test that locked in
the wrong contract is removed).
- Added TestNonWhitespaceContract regression suite covering empty
tool result, whitespace-only tool result, whitespace-only
assistant text, and whitespace-only user text parts — each
asserts the resulting Converse payload contains no
whitespace-only text blocks anywhere in the tree.
Verified end-to-end: a synthetic retro-shaped conversation (system +
user + 2× (toolUse/empty toolResult) + assistant + user) round-trips
through convert_messages_to_converse with 0 whitespace-only blocks,
0 role-alternation violations, and 0 orphan tool_use/tool_result IDs.
137 tests pass.
Related: #39829 (the bug), #39871 / #39850 / #41985 (competing fixes). This competes with the open cluster fixing #39829. Those PRs keep the pad-with-placeholder approach and just swap the single space |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the failure through the Bedrock serializer. The premise is verified on current main: agent/bedrock_adapter.py:505, :629, :655, and :659 still emit a single-space text block, while :590 passes empty tool output through unchanged.
Problems
- The new regression suite in
bb6c9bdb7e41only scans converted conversation messages. It does not validate the separate system-block return path changed from currentagent/bedrock_adapter.py:577-580, and it does not exercise the leading/trailing role-padding paths at:655and:659.
Suggested changes
- Add cases for whitespace-only system-list text, assistant-first history, and trailing assistant history; assert all emitted text blocks in both serializer return values are non-whitespace.
Automated hermes-sweeper review.
| msgs = [ | ||
| {"role": "user", "content": "go"}, | ||
| {"role": "assistant", "content": None, "tool_calls": [ | ||
| {"id": "t1", "type": "function", "function": {"name": "x", "arguments": "{}"}}, |
There was a problem hiding this comment.
All new callers scan only converse, so this regression suite never checks the separately returned system blocks or the first/last user-padding paths. Please add inputs that reach those changed branches and validate both serializer return values.
Symptom
Long conversations that go through the Bedrock Converse provider fail all 3 retries with:
Reproduced in a retro-style session (msgs=14, ~47k tokens, multiple tool calls) where at least one tool call returned empty stdout. Once the offending block was in history, every subsequent turn re-sent it and re-failed.
Root Cause
The previous sanitizer (added in #9486) tried to smuggle empty/None content through Converse by replacing it with a single space
" "placeholder. That value is itself whitespace-only and Bedrock rejects it too — the error string literally says non-whitespace, not "non-empty".Three code paths in
agent/bedrock_adapter.pyfed a" "(or unfiltered whitespace) into a{"text": ...}block:_convert_content_to_converseforNonecontent and empty list results.convert_messages_to_conversetool result branch — the rawcontentwas passed straight through with no whitespace check.convert_messages_to_converse.Fix
Change the policy from "pad invalid content with a placeholder" to "don't post invalid content at all" — with one unavoidable exception for tool results (see below).
" "placeholder message" "+ tool_calls" "placeholder message" "(rejected by Bedrock)"[tool returned no output]"sentinel" "(rejected by Bedrock)"[conversation start]"/"[continue]"Why tool results still need a sentinel: dropping a tool result would orphan its matching
toolUseblock in the prior assistant turn. Bedrock rejects orphantoolUseblocks as well, so the tool-use↔tool-result pairing is a hard invariant — this is the one place a sentinel is unavoidable. The sentinel is descriptive text so the model interprets it correctly instead of treating a bare" "as content.Cache safety: all changes happen at Converse serialization time in
bedrock_adapter.py. The stored OpenAI-shaped history is not mutated, so the per-conversation prompt cache prefix is unaffected.Tests
TestEmptyTextBlockFixrewritten from a change-detector ("placeholder is non-empty") to the real contract ("empty/whitespace input → empty block list output"). The previous assertions locked in the wrong contract that the production error disproved.TestNonWhitespaceContractregression suite scans the full Converse message tree (including nestedtoolResult.content) and asserts zero whitespace-only text blocks survive, across four inputs:"\n\n \n")test_empty_content_gets_placeholder→test_empty_content_message_is_droppedto match the new drop policy.137/137 tests pass in
tests/agent/test_bedrock_adapter.py.End-to-End Verification
Simulated the retro-shaped conversation that originally failed:
Result: zero whitespace-only text blocks anywhere in the Converse tree, strict role alternation preserved, and every
toolUsestill paired with its matchingtoolResult. The user confirmed the live agent stopped hitting the ValidationException after restarting the gateway with this patch.Related