fix(compressor): strip _multimodal tool-result images after compress() - #47412
Closed
srojk34 wants to merge 1 commit into
Closed
fix(compressor): strip _multimodal tool-result images after compress()#47412srojk34 wants to merge 1 commit into
srojk34 wants to merge 1 commit into
Conversation
browser_vision and computer_use return screenshots in a _multimodal
envelope {_multimodal: True, content: [...base64...], text_summary: "..."}.
Each screenshot can be several megabytes. _strip_historical_media only
stripped user-attached images, so _multimodal tool results accumulated
across turns and inflated every subsequent API request. Because providers
enforce payload limits on byte size rather than token count, sessions with
a handful of browser_vision calls produced HTTP 413 errors that survived
every compression pass even after the token estimate was well within limit.
Add _strip_all_multimodal_tool_results(), called inside compress() after
_sanitize_tool_pairs() and before _strip_historical_media(). It replaces
every _multimodal envelope with its text_summary string. Calling it post-
compress() is safe because the model always has an assistant response for
every tool result before compression runs — the base64 has already been
processed and keeping it serves no purpose.
Fixes NousResearch#47339.
Contributor
|
Thanks for the clear root-cause analysis and focused regression coverage. This is already implemented on current
|
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
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
browser_visionandcomputer_usereturn screenshots as a_multimodalenvelope:{"_multimodal": True, "content": [...base64...], "text_summary": "..."}Each screenshot can be several megabytes.
_strip_historical_mediaonly stripped user-attached images, so_multimodaltool results accumulated across turns and inflated every subsequent API request payload.Providers enforce limits on byte size, not token count. Sessions with a handful of
browser_visioncalls produced HTTP 413 errors that survived every compression pass even when the token estimate was well within the model's context limit (issue #47339: 101 → 13 messages, still 413).Fix
Add
_strip_all_multimodal_tool_results(), called insidecompress()after_sanitize_tool_pairs()and before_strip_historical_media().It replaces every
_multimodalenvelope with itstext_summarystring:Why it's safe to strip all of them (including the most recent):
compress()only runs between turns, never mid-turn. By the time compression triggers, the model always has an assistant response for every tool result in the conversation — the base64 has already been processed and retaining it serves no purpose.Tests
6 new tests in
TestStripAllMultimodalToolResults:text_summaryused as replacement texttext_summaryabsent_multimodaltool results unchangedAll 102 tests pass.
Fixes #47339.