fix: filter out empty text block objects from content blocks#380
fix: filter out empty text block objects from content blocks#380manikanta5827 wants to merge 7 commits intostrands-agents:mainfrom
Conversation
cagataycali
left a comment
There was a problem hiding this comment.
✅ LGTM - Approving this bug fix
Summary
This PR correctly fixes the ValidationException: The text field in the ContentBlock object is blank issue (#373) by filtering out empty TextBlock objects before sending them to the model.
Code Review
The implementation looks solid:
- Change from
consttolet- Necessary to allow reassignment during filtering - Filter logic - Correctly targets empty
TextBlockinstances usinginstanceofcheck - Placement - Filtering happens at the right spot (before
stoppedMessagecreation)
Verification
- The fix addresses the root cause identified by @manikanta5827 in the issue comments
- Works around models (GPT/Qwen on Bedrock) that insert empty text blocks alongside reasoning/tool blocks
Suggestions (non-blocking)
Consider adding a unit test for this edge case to prevent regression:
it("should filter out empty TextBlock objects from content blocks", async () => {
// Test that empty text blocks are removed before message creation
});Great work on debugging and fixing this! 👍🦆
strands-agent
left a comment
There was a problem hiding this comment.
Review Summary
👍 Looks good - Solid fix for issue #373! Just waiting for integration tests to complete.
Problem Being Solved
Some Bedrock models (GPT/Qwen) insert empty TextBlock objects in responses, causing:
ValidationException: The text field in the ContentBlock object is blank
Solution Assessment
✅ Correct approach: Filter empty TextBlock instances before API calls
✅ Type-safe: Uses proper instanceof check
✅ Minimal change: Only touches affected code path
✅ Addresses root cause: Based on issue discussion analysis
Implementation Review
The fix properly identifies and removes empty text blocks:
// Filter out empty TextBlock objects
let contentBlocks = contentBlocks.filter(
(block) => !(block instanceof TextBlock && block.text === '')
);This is the right pattern:
- ✅ Checks type with
instanceof(not just property existence) - ✅ Only removes TextBlock with empty string (not other empty content)
- ✅ Preserves all other block types
Suggestions (Non-blocking)
- Edge case test: Consider adding a unit test for this scenario
it('should filter empty TextBlock objects', () => {
const blocks = [
new TextBlock({ text: 'Hello' }),
new TextBlock({ text: '' }), // Should be filtered
new TextBlock({ text: 'World' })
];
const result = filterEmptyTextBlocks(blocks);
expect(result).toHaveLength(2);
});- Documentation: If this is a known Bedrock model behavior, might be worth a code comment explaining why this filter exists.
CI Status
⏳ Integration tests are pending - this is normal for external contributor PRs and requires maintainer approval to run.
Backward Compatibility
✅ No breaking changes - only prevents errors that would have occurred anyway.
Great work identifying and fixing this! The implementation looks solid. 🎉
🦆
🤖 This is an experimental AI agent response from the Strands team, powered by Strands Agents. We're exploring how AI agents can help with community support and development. Your feedback helps us improve! If you'd prefer human assistance, please let us know.
|
👋 Friendly follow-up! This PR is:
This is a nice fix for issue #373 (ValidationException with blank text field). The implementation cleanly filters out empty Thanks @manikanta5827 for the contribution! 🙏 🤖 This comment was generated by an AI agent using strands-agents. Workflow Run: 20944495454 |
|
Implemented a unit test for the identified edge case to ensure that empty text blocks are correctly filtered from the model response. |
… into manikanta-fix_content_blocks_validation_exception
b15d534 to
a130d7e
Compare
|
added empty commit to re run the failed jobs |
Description
This PR fixes an issue where content blocks were not properly filtering out empty text block objects, leading to potential inconsistencies or unexpected behavior in downstream processing.
The change ensures that any text block objects with empty content are removed before the content block is finalized. This improves data cleanliness and prevents unnecessary empty elements from being processed.
Related Issues
#373
Documentation PR
N/A
Type of Change
Bug fix
Testing
How have you tested the change?
npm run checkChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Before
After