fix(tools): use filtered messages list in async compaction#1124
fix(tools): use filtered messages list in async compaction#1124fede-kamel wants to merge 4 commits intoanthropics:mainfrom
Conversation
The async _check_and_compact() method was using self._params["messages"] instead of the local `messages` variable when building the compaction request. This caused the filtering logic (which removes tool_use blocks from the last assistant message) to be ignored. When compaction runs and the last message is an assistant with only tool_use blocks, those blocks should be filtered out before sending the summarization request. Without this fix, the API rejects with: "tool_use ids were found without tool_result blocks" The sync version correctly uses `*messages`, the async version was incorrectly using `*self._params["messages"]`. Added regression test that verifies tool_use filtering works correctly.
- Add cast() to fix pyright type error where lambda returns dict instead of ParseMessageCreateParamsBase - Fix bug: use params["messages"] (existing conversation) instead of messages (input variable)
fede-kamel
force-pushed
the
fix/async-compaction-bug-v2
branch
from
412ceba to
8684857
Compare
fede-kamel
force-pushed
the
fix/async-compaction-bug-v2
branch
from
8684857 to
d3b1dc2
Compare
|
@RobertCraigie @karpetrosyan @dtmeadows — would appreciate a review on this when you have a moment. This PR has been rebased on the latest The bug: Async Verified:
The fix is a one-line change. Happy to address any feedback. Thank you. |
|
@RobertCraigie @karpetrosyan Friendly follow-up - this is a one-line bug fix for async message compaction. The bug causes |
tests/lib/tools/test_runners.py
Outdated
|
|
||
|
|
||
| @pytest.mark.skipif(PYDANTIC_V1, reason="tool runner not supported with pydantic v1") | ||
| async def test_async_compaction_filters_tool_use(async_client: AsyncAnthropic) -> None: |
There was a problem hiding this comment.
LGTM! Let’s move forward without this test. I’m working on some logic to make runner tests way simpler—no mocks, and much shorter
There was a problem hiding this comment.
You're right, the cast was unnecessary - pyright passes without it. Removed it. Thanks for catching that!
The actual bug fix remains. The test is removed as @karpetrosyan is working on a new testing approach for runner tests that will be simpler.
fede-kamel
force-pushed
the
fix/async-compaction-bug-v2
branch
from
d3b1dc2 to
6b1d770
Compare
|
@karpetrosyan Done! Removed the test as requested. The PR now contains only the one-line bug fix. Ready for approval when you get a chance. Thanks! |
There was a problem hiding this comment.
@fede-kamel do we need to add a cast here? I think it should work with it
There was a problem hiding this comment.
You're right, the cast was unnecessary - pyright passes without it. Removed it. Thanks for catching that!
The cast was not needed - pyright passes without it.
karpetrosyan
left a comment
karpetrosyan
left a comment
There was a problem hiding this comment.
LGTM! Thanks @fede-kamel
2 participants
Summary
Fixes a bug where async
_check_and_compact()ignores the tool_use filtering logic.The Bug
When compaction runs and the last message is an assistant with only
tool_useblocks, those blocks should be filtered out before sending the summarization request. The sync version does this correctly, but the async version was usingself._params["messages"]instead of the filteredmessagesvariable.Without fix:
With fix: Compaction succeeds - tool_use is filtered out.
Code Change
Test Plan
test_async_compaction_filters_tool_use