fix: preserve text block structure when externalizing large toolResult content#235
Merged
jalehman merged 2 commits intoMartian-Engineering:mainfrom Apr 3, 2026
Conversation
…t content
When a toolResult message contains a plain-text content block
({type: "text", text: "..."}) that exceeds the externalization
threshold, interceptLargeToolResults now keeps {type: "text", text: ref}
instead of rewriting to {type: "tool_result", output: ref}.
This prevents the amazon-bedrock provider from crashing on
sanitizeSurrogates(c.text) when c.text is undefined.
The assembler path also reads rawType from stored metadata so
reassembled blocks reconstruct the correct part type.
Fixes Martian-Engineering#196
Make the assembler reconstruct externalized plain-text tool results as
`{ type: "text", text: ... }` instead of forcing them back through the
`tool_result`/`output` shape. Tighten the regression tests so they assert
the exact assembled block shape, and add assembler coverage for the
externalized-text path.
Regeneration-Prompt: |
Review feedback on PR 235 showed the previous change only altered how
large plain-text tool results were stored, not how they were assembled
back into runtime messages. The bug report was that Bedrock reads
`c.text` for plain text tool-result content, and the PR still rebuilt
those externalized blocks as `tool_result` objects with `output`, so the
provider would still see `undefined`.
Fix the round-trip at the assembler layer with the smallest additive
change. Preserve existing behavior for structured tool results and
function_call_output blocks. Add regression tests that fail unless the
assembled block is actually `type: "text"` with a `text` field, and add
focused assembler coverage for the externalized plain-text case.
Contributor
|
Thank you! |
Contributor
Author
|
Appreciate the merge, @jalehman. |
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
2 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.
Summary
When
interceptLargeToolResultsexternalizes a large plain-text content block ({type: "text", text: "..."}) inside atoolResultmessage, it rewrites it to{type: "tool_result", output: "..."}, dropping thetextfield. The amazon-bedrock provider crashes onsanitizeSurrogates(c.text)becausec.textisundefined.Changes
src/engine.ts--interceptLargeToolResults()now branches onisPlainTextToolResult:{type: "text", text: "..."}) keep{type: "text", text: ref}after externalizationtool_result,function_call_output) keep existing{type: normalizedRawType, output: ref}behaviorrawTypemetadata field carries the original type through storage so the assembler can reconstruct the correct part type duringassemble()src/engine.ts--assemble()path readsrawTypefrom stored metadata instead of the block's runtimetype, so externalized text blocks round-trip correctly through storage and reassembly.test/engine.test.ts-- Updated the externalization test to checktextfield for plain-text blocks (was checkingoutput).Testing
npm testpasses (463/464, 1 pre-existing failure incircuit-breaker.test.tson main).Fixes #196
This contribution was developed with AI assistance (Codex).