fix(compressor): strip _multimodal tool-result images in _strip_historical_media - #47398
Closed
srojk34 wants to merge 1 commit into
Closed
fix(compressor): strip _multimodal tool-result images in _strip_historical_media#47398srojk34 wants to merge 1 commit into
srojk34 wants to merge 1 commit into
Conversation
…rical_media browser_vision and computer_use (native-vision fast path) return a ``_multimodal`` envelope as the tool result. ``_strip_historical_media`` only searched for image-bearing *user* messages when choosing the anchor, so sessions that accumulated multiple browser_vision screenshots never stripped any base-64 payload from tool results. After two compression passes (e.g. 101 → 13 messages) those screenshots were still present in the tail and kept the outgoing payload above Copilot's HTTP 413 limit even though the estimated token count appeared safe. Fix: - Add ``_tool_result_has_media`` to recognise ``_multimodal`` envelopes and OpenAI-style list content in tool-result messages. - Add ``_strip_tool_result_media`` to replace a ``_multimodal`` envelope with its ``text_summary`` and to strip image parts from list-style tool results. - Extend the anchor search in ``_strip_historical_media`` to also consider tool-result messages (user-image anchoring is preserved as-is). - Strip tool-result images for all messages before the anchor alongside the existing user-image stripping. - Add ``TestStripHistoricalMediaToolResults`` with five cases covering ``_multimodal`` envelopes, list-style images, legacy user-image stripping, single-screenshot no-op, and mixed scenarios.
Collaborator
|
Related: #47397, #42509, #37412 (all fix #47339 — context compression not evicting image/vision payloads, leading to HTTP 413). This PR extends the merged |
Contributor
Author
|
Closing in favor of a more complete fix that strips all _multimodal tool-result images after compress(), not just pre-anchor ones. |
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
_strip_historical_mediaonly searched for image-bearing user messages whenselecting the anchor. Sessions driven by
browser_visionorcomputer_use(native-vision fast path) accumulate screenshots exclusively in tool result
messages as
_multimodalenvelopes — their user turns carry plain text. Becausethe anchor search never matched those tool results, no base-64 payload was ever
stripped. After two compression passes (e.g. 101 → 13 messages) the screenshots
were still present in the tail, pushing the outgoing payload above the provider's
HTTP 413 limit even though the estimated token count looked safe. Reported in #47339.
Fix
_tool_result_has_media()to recognise_multimodalenvelopes andOpenAI-style list content in tool-result messages.
_strip_tool_result_media()to replace a_multimodalenvelope with itstext_summarystring and to strip image parts from list-style tool results._strip_historical_mediato also considertool-result messages (user-image anchoring is preserved as-is).
existing user-image stripping.
Tests
TestStripHistoricalMediaToolResults(5 cases):_multimodalenvelope in tool result before anchor → stripped totext_summary