fix(agent): drop vision payloads before compressing on a 413 - #89776
Open
AP3X-Dev wants to merge 1 commit into
Open
fix(agent): drop vision payloads before compressing on a 413#89776AP3X-Dev wants to merge 1 commit into
AP3X-Dev wants to merge 1 commit into
Conversation
A screenshot-heavy session could wedge in a 413 -> compress -> 413 loop far below its context window. The payload-too-large branch spent every recovery attempt on text summarization before it ever tried dropping image payloads, and text summarization cannot shrink base64: each pass cost a slow LLM round-trip, reported "no reduction" because the images were untouched, and consumed one of max_compression_attempts. A session on a 1M-token window was compacting repeatedly at ~84K tokens because the trigger was the provider's byte-size limit, not the token threshold. Retained vision parts are typically the bulk of an oversized body, so the strip now runs first: it is instant, local, and targets the actual bytes. Text compression still runs on the next pass when there was no image left to drop, so text-heavy 413s are unaffected. The strip is safe to hoist: api_messages is built once above the retry loop and _compress_context rewrites `messages` rather than api_messages, so the mutation survives the `continue`. It is also self-limiting — stripped tool content becomes a string, so a second call finds no list content and returns False rather than looping. remember_model=False is preserved: a 413 means this body was too large, not that the provider rejects list-type tool content in general. The post-compression strip retry is removed as dead code — by the time compression has failed to reduce, api_messages is already image-free. Fixes NousResearch#89286
alt-glitch
added
type/bug
11 tasks
19 tasks
19 tasks
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
3 participants
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.
What does this PR do?
Fixes the
413 -> compress -> 413loop a screenshot-heavy session can wedge in, far below its context window.The causal chain. In
agent/conversation_loop.pytheis_payload_too_largebranch was ordered:compression_attempts += 1agent._compress_context(...)— a slow LLM round-trip_try_strip_image_parts_from_tool_messages(...)A 413 is a verdict on the byte size of this request body. On a session carrying browser/vision tool results, retained base64
image_urlparts are typically the bulk of those bytes — and text summarization cannot shrink base64. So steps 1–3 reliably no-op, each pass costing ~50–70s of wall clock and one ofmax_compression_attempts, before the code reaches the one operation that would have helped. From the reported session (context window 1,000,000, threshold 468,000):Repeated compaction at ~84K tokens against a 1M window, because the trigger was the provider's byte cap, not the token threshold.
The fix inverts the order: attempt the vision-payload strip first (instant, local, targets the actual bytes), and fall through to text compression only when there was no image left to drop.
Why hoisting the strip is safe — verified rather than assumed:
api_messagesis built once above thewhile retry_count < max_retriesloop and is never reassigned inside it;_compress_contextrewritesmessages, notapi_messages. So the strip persists across thecontinue._try_strip_image_parts_from_tool_messagesonly rewrites tool messages whosecontentis a list containing image parts, and replaces that content with a string. A second call therefore finds no list content and returnsFalse, falling through to compression. At most one extra round-trip.False→ the branch proceeds to compression exactly as before, including the existing lock-defer and terminal paths.remember_model=Falseis preserved — already the convention here. A 413 means this body was too large, not that the provider rejects list-type tool content in general (the [Bug] computer_use multimodal tool message causes 400 error on providers that don't support multimodal tool content (e.g. Xiaomi MiMo) #27344 recovery path). The test asserts_no_list_tool_content_modelsstays empty.One deletion worth calling out: the post-compression strip retry (the old step 4) is removed as dead code. With the strip hoisted, by the time compression has failed to reduce,
api_messagesis already image-free and that call is provably a no-op — leaving it would also emit a status message ("Compression could not reduce the request further — removed retained vision payloads") that can no longer be true. A comment records why it is gone.Related Issue
Fixes #89286
Related: #59587 covers the threshold semantics (the
0.95progress gate) of the same branch. This change is about ordering and is independent — even with a perfect threshold, an image-heavy request never reached the stripper until compression had already failed.Type of Change
Changes Made
agent/conversation_loop.py— in theis_payload_too_largebranch, run_try_strip_image_parts_from_tool_messages(api_messages, remember_model=False)beforecompression_attempts += 1;continueon success. Removed the now-unreachable post-compression strip retry, with a comment explaining why it cannot fire.tests/run_agent/test_413_compression.py— updated the existing vision-strip test to the corrected ordering and added two tests (details below).How to Test
28 passed on this branch. On
main, two of them fail:test_413_strips_vision_payloads_before_compressing— the existing test, updated. It already proved images get evicted and text survives; it previously also asserted_compress_contextwas called once, which encoded the ordering this issue reports as wrong. It now asserts_compress_contextis not called: dropping the images alone recovers the request, with no summarization round-trip. Every outcome assertion it made before (image gone, text preserved, model not blacklisted) is unchanged.test_413_image_strip_does_not_consume_a_compression_attempt— new. Setsmax_compression_attempts = 0to stand in for a budget already exhausted by earlier futile passes, then sends a 413 with an image-bearing tool result. Onmainthis hard-fails with413 compression failed after 0 attemptswhile the base64 is still in the body; with the fix it recovers, because dropping images costs nothing.test_413_still_compresses_when_there_is_no_image_to_drop— new guard for the fallthrough. A text-only 413 must still reach compression, so the reorder cannot become a way to skip it. (This one passes both ways by design — it exists to stop a future change from turning the strip into an unconditional bypass.)Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass — left unchecked deliberately. I have not run the whole suite green on this Windows box; it carries pre-existing failures unrelated to this change. What I did run:tests/run_agent/test_413_compression.py→ 28 passedtests/run_agent/→ 266 passed, plustests/run_agent/test_callable_api_key.py, which fails identically (4 failed, 8 passed) on cleanmainwith my changes stashed — pre-existing and unrelated to this branch.Documentation & Housekeeping
docs/, docstrings) — inline comments record the ordering rationale and why the removed branch is unreachable; no user-facing docs describe this pathcli-config.yaml.exampleif I added/changed config keys — N/A, no config keysCONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/Ascripts/check-windows-footguns.py --diff mainis clean.Screenshots / Logs
Behavior on
mainwith an image-bearing 413 and no compression budget left — the request fails while the base64 is still in the body:With this change the same request recovers on the next round-trip, with no summarization call.