fix(compaction): also filter out server_tool_use blocks before compaction#1151
Open
stephaniepang97 wants to merge 3 commits intomainfrom
Open
fix(compaction): also filter out server_tool_use blocks before compaction#1151stephaniepang97 wants to merge 3 commits intomainfrom
stephaniepang97 wants to merge 3 commits intomainfrom
Conversation
…tion
When compaction is triggered, the code filters out tool_use blocks from
the last assistant message to avoid 400 errors (since tool_use requires
a corresponding tool_result). However, it wasn't filtering out
server_tool_use blocks, which also require corresponding result blocks
(web_fetch_tool_result, web_search_tool_result, etc.).
This caused compaction to fail with:
'messages.X: `web_fetch` tool use with id `srvtoolu_XXX` was found
without a corresponding `web_fetch_tool_result` block'
Fixed by changing the filter from:
block.get("type") != "tool_use"
to:
block.get("type") not in ("tool_use", "server_tool_use")
When compaction triggers, the current assistant message (which contains server tool results like web_fetch_tool_result) hasn't been appended to _params['messages'] yet. This causes compaction to fail because: 1. Message N has server_tool_use for some ID 2. Message N+1 (current, not yet appended) has the corresponding result 3. Compaction only sees messages up to N, missing the result 4. API rejects with 'tool use without corresponding result block' Fix: Pass the current message to _check_and_compact() so it can include the message (with server tool results) when building messages for the compaction API call.
When including the current message in compaction, the content blocks are pydantic objects. The filter logic checks isinstance(block, dict), which fails for pydantic objects, causing ALL blocks to be filtered out (including the server tool results we want to keep). Fix: Convert content blocks to dicts using model_dump() when appending the current message, so the existing dict-based filter logic works.
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
1 participant
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.
Problem
When compaction is triggered while the last assistant message contains
server_tool_useblocks (for server-side tools likeweb_fetchorweb_search), the compaction API call fails with:Root Cause
The compaction code filters out
tool_useblocks from the last assistant message before making the compaction request (since they require correspondingtool_resultblocks). However, it wasn't filtering outserver_tool_useblocks, which also require corresponding result blocks (web_fetch_tool_result,web_search_tool_result, etc.).Solution
Changed the filter from:
to:
Changes
BaseSyncToolRunner._check_and_compact()to filter out bothtool_useandserver_tool_useBaseAsyncToolRunner._check_and_compact()with the same fix