From 36bd54eaf26e45401f55a1656551d2f926fb14e6 Mon Sep 17 00:00:00 2001 From: AP3X Date: Tue, 18 Aug 2026 22:22:10 -0700 Subject: [PATCH] fix(agent): drop vision payloads before compressing on a 413 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 #89286 --- agent/conversation_loop.py | 45 +++++++-- tests/run_agent/test_413_compression.py | 122 ++++++++++++++++++++++-- 2 files changed, 147 insertions(+), 20 deletions(-) diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index ab8e60b6d07ea..661f778d9eb66 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -5449,6 +5449,32 @@ def _perform_api_call(next_api_kwargs): ) if is_payload_too_large: + # Drop retained vision payloads BEFORE spending a + # compression attempt. A 413 is a byte-size verdict on + # this request body, and on a screenshot-heavy session + # base64 image parts are the bulk of those bytes — text + # summarization cannot shrink them. Compressing first + # burns ~50-70s of LLM round-trip per attempt, reports + # "no reduction" because the images are untouched, and + # walks the session into a 413 -> compress -> 413 loop + # against a context window it is nowhere near (#89286). + # Stripping is instant, local, and targets the actual + # bytes, so it goes first; text compression still runs + # on the next pass when there was no image left to drop. + # + # remember_model=False: 413 means this BODY was too + # large, not that the provider rejects list-type tool + # content in general (the #27344 recovery path). + if agent._try_strip_image_parts_from_tool_messages( + api_messages, + remember_model=False, + ): + agent._buffer_status( + "📐 Request payload too large (413) — dropped retained " + "vision payloads and retrying before compressing..." + ) + continue + compression_attempts += 1 if compression_attempts > max_compression_attempts: # Terminal — surface the buffered retry trace. @@ -5512,16 +5538,15 @@ def _perform_api_call(next_api_kwargs): _retry.restart_with_compressed_messages = True break else: - if agent._try_strip_image_parts_from_tool_messages( - api_messages, - remember_model=False, - ): - agent._buffer_status( - "📐 Compression could not reduce the request further — " - "removed retained vision payloads and retrying..." - ) - continue - + # No image-strip retry here any more: the strip now + # runs at the top of this branch, so by the time + # compression has failed to reduce, api_messages is + # already image-free and a second call is a no-op. + # (api_messages is built once, above the retry loop, + # and _compress_context rewrites `messages` — not + # api_messages — so the strip above persists across + # this `continue`.) + # # Terminal — surface buffered context so the user # sees what compression attempts were made. agent._flush_status_buffer() diff --git a/tests/run_agent/test_413_compression.py b/tests/run_agent/test_413_compression.py index fcedeca0e1769..4f19b7f5cafe6 100644 --- a/tests/run_agent/test_413_compression.py +++ b/tests/run_agent/test_413_compression.py @@ -178,13 +178,16 @@ def test_413_triggers_compression(self, agent): - def test_413_strips_vision_payloads_when_compression_cannot_reduce_messages(self, agent): - """If compression leaves image payloads behind, strip them and retry. - - Browser vision tool results can contain base64 image parts. A 413 can - persist even after summarisation when the remaining recent tool result - still carries binary data; Hermes should evict the image payload and - keep the text/placeholder context instead of failing immediately. + def test_413_strips_vision_payloads_before_compressing(self, agent): + """A 413 evicts retained image payloads BEFORE spending a compression pass. + + Browser vision tool results carry base64 image parts, which are + typically the bulk of an oversized request body. Text summarisation + cannot shrink base64, so compressing first burns a slow LLM round-trip + that reliably reports "no reduction" and walks the session into a + 413 -> compress -> 413 loop (#89286). Stripping is instant and targets + the actual bytes, so it runs first and compression is not called at all + when dropping the images is enough to recover. """ err_413 = _make_413_error() ok_resp = _mock_response(content="Recovered after image eviction", finish_reason="stop") @@ -232,12 +235,15 @@ def _side_effect(**kwargs): patch.object(agent, "_save_trajectory"), patch.object(agent, "_cleanup_task_resources"), ): - # Simulate the bad production case: compression ran, but the - # recent vision tool message survived so message count did not drop. + # Kept as a guard: if the ordering ever regresses, compression + # runs here and (as in production) fails to shrink the base64, + # which the assert_not_called below catches. mock_compress.side_effect = lambda msgs, *_a, **_k: (msgs, "compressed prompt") result = agent.run_conversation("continue", conversation_history=prefill) - mock_compress.assert_called_once() + # The image strip alone recovered the request — no slow summarisation + # round-trip, and no compression attempt consumed. + mock_compress.assert_not_called() assert result["completed"] is True assert result["final_response"] == "Recovered after image eviction" assert len(request_payloads) == 2 @@ -248,6 +254,102 @@ def _side_effect(**kwargs): assert "Screenshot of the dashboard" in str(retried_tool["content"]) assert not getattr(agent, "_no_list_tool_content_models", set()) + def test_413_still_compresses_when_there_is_no_image_to_drop(self, agent): + """Text-only sessions must keep reaching compression (#89286). + + Running the image strip first must not become a way to skip + compression: on a text-heavy 413 the strip finds nothing, reports + False, and the branch falls through to summarisation exactly as + before. + """ + err_413 = _make_413_error() + ok_resp = _mock_response(content="Recovered after compression", finish_reason="stop") + agent.client.chat.completions.create.side_effect = [err_413, ok_resp] + + prefill = [ + {"role": "user", "content": "a very long text question"}, + {"role": "assistant", "content": "a very long text answer"}, + ] + + with ( + patch.object(agent, "_compress_context") as mock_compress, + patch.object(agent, "_persist_session"), + patch.object(agent, "_save_trajectory"), + patch.object(agent, "_cleanup_task_resources"), + ): + mock_compress.return_value = ( + [{"role": "user", "content": "summary"}], + "compressed prompt", + ) + result = agent.run_conversation("continue", conversation_history=prefill) + + mock_compress.assert_called_once() + assert result["completed"] is True + assert result["final_response"] == "Recovered after compression" + + def test_413_image_strip_does_not_consume_a_compression_attempt(self, agent): + """Dropping images must not burn one of max_compression_attempts. + + The pre-#89286 order spent an attempt on every futile summarisation + pass, so a screenshot-heavy session could exhaust the budget and hard + fail with "max compression attempts reached" while the images — the + actual bytes — were still in the body. A budget of 0 stands in for + that already-exhausted state: dropping images is free, so recovery + must not depend on having an attempt left to spend. + """ + agent.max_compression_attempts = 0 + err_413 = _make_413_error() + ok_resp = _mock_response(content="Recovered on a budget of one", finish_reason="stop") + calls = [] + + def _side_effect(**kwargs): + calls.append(kwargs) + if len(calls) == 1: + raise err_413 + return ok_resp + + agent.client.chat.completions.create.side_effect = _side_effect + + prefill = [ + {"role": "user", "content": "look at this"}, + { + "role": "assistant", + "content": None, + "tool_calls": [ + { + "id": "call_v", + "type": "function", + "function": {"name": "browser_vision", "arguments": "{}"}, + } + ], + }, + { + "role": "tool", + "tool_call_id": "call_v", + "name": "browser_vision", + "content": [ + {"type": "text", "text": "page text"}, + { + "type": "image_url", + "image_url": {"url": "data:image/png;base64," + ("b" * 4000)}, + }, + ], + }, + ] + + with ( + patch.object(agent, "_compress_context") as mock_compress, + patch.object(agent, "_persist_session"), + patch.object(agent, "_save_trajectory"), + patch.object(agent, "_cleanup_task_resources"), + ): + mock_compress.side_effect = lambda msgs, *_a, **_k: (msgs, "compressed prompt") + result = agent.run_conversation("continue", conversation_history=prefill) + + assert result["completed"] is True + assert result.get("compression_exhausted") is not True + assert result["final_response"] == "Recovered on a budget of one" + def test_413_clears_conversation_history_on_persist(self, agent): """After 413-triggered compression, _persist_session must receive None history.