fix(bedrock): use non-whitespace placeholder for empty content blocks - #41985
Open
JiaDe-Wu wants to merge 1 commit into
Open
fix(bedrock): use non-whitespace placeholder for empty content blocks#41985JiaDe-Wu wants to merge 1 commit into
JiaDe-Wu wants to merge 1 commit into
Conversation
Bedrock Converse rejects whitespace-only text blocks with 'text content blocks must contain non-whitespace text' (NousResearch#39829). The previous fix for NousResearch#9486 replaced empty strings with a single space, which passes the non-empty check but fails the non-whitespace check — breaking resume of assistant-first history and any padding path. Replace all 6 single-space placeholders with a module-level _EMPTY_CONTENT_PLACEHOLDER = '(no content)' that satisfies both checks. Also strip-guard text parts inside content lists. 6 tests updated/added. Closes NousResearch#39829.
alt-glitch
added
type/bug
teknium1
reviewed
Jul 14, 2026
teknium1
left a comment
teknium1
left a comment
Contributor
There was a problem hiding this comment.
Thanks for tracing the six placeholder sites; current main still has the reported whitespace-padding defect (agent/bedrock_adapter.py:505-507, 655-659).
Problems
- The patch does not cover the tool-result branch:
agent/bedrock_adapter.py:586-591serializes empty or whitespace-only tool output directly as nestedtoolResult.content[].text. That can still trigger the same Bedrock validation error while preserving a required toolUse/toolResult pair. - The new guard handles dict text parts, but plain string items in a content list are still appended without a
.strip()check atagent/bedrock_adapter.py:511-513.
Suggested changes
- Apply the non-whitespace contract to nested tool results as well, using a descriptive sentinel if needed to preserve the pairing invariant.
- Add regression coverage that walks the complete Converse payload, including nested
toolResult.content, plus whitespace-only string list items.
This is an automated hermes-sweeper review.
| 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}) |
Contributor
There was a problem hiding this comment.
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.
teknium1
added
sweeper:risk-compatibility
This was referenced Aug 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sequel to #9486. That issue fixed
text content blocks must be non-emptyby replacing empty strings with a single space" ". But Bedrock Converse also rejects whitespace-only blocks:The single-space placeholder passes the non-empty check but fails the non-whitespace check — breaking resume of assistant-first history (which triggers synthetic user-message padding) and any other empty-content path. Reported in #39829.
Fix
Replace all 6 single-space placeholders with a module-level
_EMPTY_CONTENT_PLACEHOLDER = "(no content)"that satisfies both the non-empty and non-whitespace checks. Also adds a.strip()guard to text parts inside content lists (previously a whitespace-only text part would slip through).E2E verification
Tested on EC2 (us-east-2) with
us.anthropic.claude-opus-4-8:6 tests updated/added. Closes #39829.