fix: use non-empty placeholder in dropThinkingBlocks to prevent Bedrock ValidationException#71623
fix: use non-empty placeholder in dropThinkingBlocks to prevent Bedrock ValidationException#71623wujiaming88 wants to merge 1 commit into
Conversation
…ck ValidationException
When dropThinkingBlocks strips all thinking content from an assistant
message, it replaces the content with a synthetic text block to preserve
turn structure. Previously this placeholder used an empty string:
{ type: "text", text: "" }
However, both Bedrock's convertMessages and Anthropic's
convertAnthropicMessages filter out text blocks where
text.trim().length === 0. This causes the assistant message's content
array to become empty, which triggers a Bedrock ValidationException:
"The content field in the Message object at messages.N is empty.
Add a ContentBlock object to the content field and try again."
This was observed in production when an assistant message contained only
thinking blocks (no tool calls or visible text). After dropThinkingBlocks
ran, the empty placeholder was filtered by the provider converter,
leaving an empty content array that Bedrock rejected.
Fix: use "[thinking]" as the placeholder text so it survives downstream
empty-text filters across all providers.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Greptile SummaryThis PR fixes a production Confidence Score: 5/5Safe to merge — minimal, well-targeted fix that resolves a confirmed production error with updated tests. The change is a one-line value swap backed by thorough JSDoc explaining the invariant, a named constant to prevent future regressions, and an updated unit test. No unrelated code is touched, and the fix directly addresses the root cause observed in production logs. No files require special attention. Reviews (1): Last reviewed commit: "fix: use non-empty placeholder in dropTh..." | Re-trigger Greptile |
|
Thanks for the report and patch. I verified the underlying issue against current I landed the fix on
Validation run locally before push:
Closing this PR as superseded by the landed main fix. Changelog credits @wujiaming88. Thanks again. |
Problem
When
dropThinkingBlocksstrips all thinking content from an assistant message, it replaces the content with a synthetic text block to preserve turn structure. Previously this placeholder used an empty string:However, both Bedrock's
convertMessagesand Anthropic'sconvertAnthropicMessages(in@mariozechner/pi-ai) filter out text blocks wheretext.trim().length === 0:This causes the assistant message's content array to become empty after filtering, which triggers a Bedrock
ValidationException:Reproduction
claude-opus-4-6)dropThinkingBlocksruns on that historical messageObserved in production
Fix
Use
"[thinking]"as the placeholder text so it survives downstream empty-text filters across all providers.Changes
src/agents/pi-embedded-runner/thinking.ts— Replace empty string placeholder with"[thinking]"and add a named constant with documentationsrc/agents/pi-embedded-runner/thinking.test.ts— Update test expectation to match new placeholder