Repository navigation
fix(anthropic): keep tool_result blocks contiguous by grouping image blocks separately - #2243
Conversation
…blocks separately When a turn issued multiple tool_use blocks and tool results carried image blocks (read_file on images, screenshots, feedback images), presentAssistantMessage pushed each tool_result followed immediately by its images, yielding tool_result/image interleaving in userMessageContent. Task.ts then consumed the array verbatim, and the Anthropic API rejected the request with a 400: "tool_use ids were found without tool_result blocks immediately after" (Zoo-Code-Org#1307). pushToolResultToUserContent is the single choke point every tool_result push flows through, so it now maintains the invariant: when a new tool_result is appended, image blocks already accumulated are moved after it. The resulting user message is tool_results first (in tool_use order), then images, then other blocks — matching the shape the issue expects. A single tool result with images and image-free messages are unchanged.
…er-tool-results Textually clean merge; no conflict resolution was required.
…nterleave Addresses edelauna's two review asks on Zoo-Code-Org#1705. 1. presentAssistantMessage-images.spec.ts bound a hand-copied partition of Task.pushToolResultToUserContent, so the Zoo-Code-Org#1307 tests were tautological: deleting the grouping from Task.ts still passed them. The mock now binds the real Task.prototype.pushToolResultToUserContent via vi.importActual. Verified: removing the partition from Task.ts fails both Zoo-Code-Org#1307 tests. 2. Task.spec.ts only covered "images after the first result". Added the mixed interleave [image, text, r1, text, r2] -> [r1, r2, image, text, text2], which fails under an image-only grouping and only passes with the full tool_result-first partition. Also carries main's new policySnapshot argument at the 2 integration call sites, and drops this spec's no-explicit-any suppressions from 16 to 14.
|
Warning This pull request changes a CodeRabbit configuration file. Because it comes from a fork or its author is not a repository collaborator, reviews use only the configuration from the target branch. The proposed configuration will take effect after it is merged. No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Keep tool results contiguous src/core/task/Task.ts, src/core/task/__tests__/Task.spec.ts |
The method groups tool-result blocks before other content and preserves order within each group. Tests cover interleaved text and images, including a later tool result with no images. |
Test parallel tool responses src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts, src/eslint-suppressions.json |
The image-handling tests call the real task method and verify tool-result and image ordering for parallel tool responses. The configured suppression count decreases from 16 to 14. |
Test Guidance
| Layer / File(s) | Summary |
|---|---|
Update test instructions .coderabbit.yaml |
The TypeScript and JavaScript test instructions now cover test doubles that bind production logic, mixed-input tests for collection transformations, and handling surviving or equivalent Stryker mutants. |
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Medium
Possibly related issues
- Zoo-Code-Org/Zoo-Code#2098: Reports Anthropic rejection when tool results are not adjacent. This change moves other content after accumulated tool results, but does not address missing or orphan results.
- Zoo-Code-Org/Zoo-Code#2287: Concerns reporting surviving mutations in
Task.ts; this PR adds test guidance about surviving Stryker mutants.
Possibly related PRs
- Zoo-Code-Org/Zoo-Code#2113: Changes
Task.pushToolResultToUserContentto place tool results before existing content. This PR keeps accumulated tool results contiguous while preserving order.
Merge Risk: 
163a3
-
The change keeps tool results contiguous while preserving image and text order within their group. No actionable merge-blocking issue was identified.
Caution
Pre-merge checks failed
Please resolve all errors before merging. Addressing warnings is optional.
- Ignore (reviewers only)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check |  |
Issue #1307 requires all corresponding tool_result blocks to remain contiguous before image or text blocks. The PR description states that a pre-existing repetition-limit text block can still appear… |
Ensure every non-tool_result block, including repetition-limit text, is placed after the complete contiguous tool_result prefix before the request is sent. Add a regression test for a text block between two tool results. |
| Out of Scope Changes check | |
The production change and the Task and assistant-message tests support issue #1307. The eslint-suppressions.json update supports the test changes. The additional .coderabbit.yaml changes modify re… |
Remove the unrelated .coderabbit.yaml changes, or provide a separate directly linked objective that requires those repository review-instruction changes. |
-
Check name Status Explanation Title check The title names the main change: tool_result blocks are kept contiguous in Anthropic user messages by relocating image blocks after them. It matches the Task.pushToolResultToUserContent change, the im… Description check The description follows the repository template and covers the main required parts. It links the approved issue (Closes: #1307), explains the how (relocating image blocks in Task.pushToolResultToUserC…Regression Evidence Focused coverage exists at the lowest valid layer. Task.spec.tsverifies result contiguity, result order, image order, text relocation, a later image-free result, and duplicate/error-result behavior…Security Boundaries No changed path matches the security failure conditions. The production change in src/core/task/Task.ts:602-611only partitions the existing typeduserMessageContentarray intotool_resultand o…Persistence Integrity No changed persistence path matches the failure condition. The PR changes only Task.pushToolResultToUserContent(Task.ts:591-612), which synchronously partitions the in-memoryuserMessageContenta…Lifecycle Resource Cleanup No changed lifecycle path can leak a resource or duplicate work. The only production change is Task.pushToolResultToUserContent, which synchronously filters and reorders the existing `userMessageCon…Full details: Title check
-
Explanation
The title names the main change: tool_result blocks are kept contiguous in Anthropic user messages by relocating image blocks after them. It matches the Task.pushToolResultToUserContent change, the image-ordering tests, and the linked issue
#1307. It uses the conventional commit prefix and fits in one sentence. Minor nit: it is 86 characters, slightly longer than ideal, but still clear and specific.
Full details: Description check
-
Explanation
The description follows the repository template and covers the main required parts. It links the approved issue (Closes:
#1307), explains the how (relocating image blocks in Task.pushToolResultToUserContent and why the fix sits at that choke point), and gives concrete test procedure with commands and results. The checklist is completed and the visual-snapshot and video sections are marked N/A. Two gaps remain: the Get in Touch section contains a Discord username placeholder replaced by a GitHub handle, and the description uses some non-STE phrasing such as the parenthetical notes. These are non-critical, so the check passes.
Full details: Regression Evidence
-
Explanation
Focused coverage exists at the lowest valid layer.
Task.spec.tsverifies result contiguity, result order, image order, text relocation, a later image-free result, and duplicate/error-result behavior. The presenter integration tests invoke the realTask.prototype.pushToolResultToUserContentand cover parallel image results and an image-free later result. No visible UI change requires a Playwright snapshot.
Full details: Security Boundaries
-
Explanation
No changed path matches the security failure conditions. The production change in
src/core/task/Task.ts:602-611only partitions the existing typeduserMessageContentarray intotool_resultand other blocks, preserving order. It does not log, transmit, execute, or expose new data, and it does not alter tool validation or approval/allowlist checks. The other changes are tests, lint metadata, and review configuration.
Full details: Persistence Integrity
-
Explanation
No changed persistence path matches the failure condition. The PR changes only
Task.pushToolResultToUserContent(Task.ts:591-612), which synchronously partitions the in-memoryuserMessageContentarray. It adds no asynchronous operation, file write, default, or persistence transaction. The existing consumers still awaitsaveApiConversationHistory()and retain pending content when saving fails (Task.ts:1595-1603).
Full details: Lifecycle Resource Cleanup
-
Explanation
No changed lifecycle path can leak a resource or duplicate work. The only production change is
Task.pushToolResultToUserContent, which synchronously filters and reorders the existinguserMessageContentarray after a successful append. It does not create or retain listeners, watchers, timers, providers, or tasks, and it does not alter cancellation, disposal, or restart handling. All callers remain unchanged.
-
Full details: Linked Issues check
-
Explanation
Issue
#1307requires all correspondingtool_resultblocks to remain contiguous before image or text blocks. The PR description states that a pre-existing repetition-limit text block can still appear between tool results. The proposed relocation moves image blocks, but it does not establish the required result-first layout when that text block is present. The image-order tests support the reported image case, but they do not satisfy the full result-before-text requirement.Resolution
Ensure every non-
tool_resultblock, including repetition-limit text, is placed after the complete contiguoustool_resultprefix before the request is sent. Add a regression test for a text block between two tool results.
Full details: Out of Scope Changes check
-
Explanation
The production change and the Task and assistant-message tests support issue
#1307. Theeslint-suppressions.jsonupdate supports the test changes. The additional.coderabbit.yamlchanges modify repository review instructions and do not implement or verify tool-result ordering, so they are unrelated to the linked issue.Resolution
Remove the unrelated
.coderabbit.yamlchanges, or provide a separate directly linked objective that requires those repository review-instruction changes.
Finishing Touches
-
Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Review statusThanks for contributing. This comment tracks the review sequence and the next action. Current step: Required CI passed. Waiting for automated review of the latest commit. If automated review does not start, a maintainer must restart it. Review-state labels are managed by this workflow; do not edit them manually. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
github-actions
Bot
added
coderabbit-review-active
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
github-actions
Bot
added
awaiting-maintainer
…er-tool-results
github-actions
Bot
added
coderabbit-review-active
github-actions
Bot
added
awaiting-maintainer
github-actions
Bot
removed
the
awaiting-maintainer
github-actions
Bot
added
coderabbit-review-active
github-actions
Bot
added
awaiting-author
I added the out of scope changes intentionally.
github-actions
Bot
added
coderabbit-review-active
Supersedes #1705 (stale-bot closed; conflicts now resolved vs current main)
Related GitHub Issue
Closes: #1307
Description
When a turn executes multiple
tool_useblocks and at least one tool returns images,userMessageContentended up astool_result, image, tool_result, image. Anthropic rejects that user message with a hard 400 (tool_use ids were found without tool_result blocks immediately after) becausetool_resultblocks must be the first, contiguous content of the user message. Both consumers (Task.flushPendingToolResultsToHistoryand the main request assembly) passed the array through verbatim.Fix at the single choke point every
tool_resultpush flows through (Task.pushToolResultToUserContent, covering all 8 call sites: the main tool-result callback, MCP-native dispatch, rejection/validation-error paths, and the task-interruption pre-injection path): after appending a newtool_result, any image blocks already inuserMessageContentare relocated after it. Result shape:[trA, trB, imgA, imgB, text]— Anthropic accepts non-tool_resultcontent after the contiguous result run, so this is spec-compliant and matches the issue's expected shape.Why not at the image-push sites (as the issue suggested): if tool A returns images and a later tool result has no images,
[trA, imgA, trB]still interleaves — grouping at the result push is the only complete minimal fix, and it keeps the consumer untouched.Behavior preserved: multiple images keep per-tool order; single-tool-with-images and image-free turns are byte-identical; images with no tool results are never relocated.
Test Procedure
src/core/task/__tests__/Task.spec.ts(+1 regression test, 1 test updated to the new correct order with full block equality — stricter, not weaker):tr1 + 2 images + image-less tr2→ asserts[tr1, tr2, img1, img2].src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts(+2 integration tests through the realpresentAssistantMessage): two image-returning tools →[trA, trB, imgA, imgB]; first tool returns images, second returns none → images relocated after the second result. Both fail onmain.cd src && ./node_modules/.bin/vitest run core/assistant-message→ 67/67 passed.cd src && ./node_modules/.bin/vitest run core/task/__tests__/Task.spec.ts→ 151/151 passed.Pre-Submission Checklist
Visual Snapshots
N/A.
Videos (interaction / animation only)
N/A.
Documentation Updates
Additional Notes
Pre-existing follow-up (out of scope): a repetition-limit feedback text block can still land between tool results (
presentAssistantMessage.ts~684); this PR improves that case too (wastr, text, img, trbefore). Happy to address separately if it ever manifests.Get in Touch
GitHub: @myk1yt — please tag me here; I monitor notifications.