From dff84f18901c3b3c082e7783ea03b7bb7b9ef6c7 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sun, 23 Aug 2026 12:56:47 +0530 Subject: [PATCH] fix(browser): cap browser_vision native embeds for history reuse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit browser_vision's native fast path base64-encoded screenshots at full resolution and baked them into the tool result uncapped — the exact sibling of the vision_analyze path #92699 fixed. Apply the same proactive 256KB/1568px resize before the embed enters reusable history. Fail-open by design: without Pillow the resize helper falls back to raw bytes and the compressor's keep-newest pass still retires stale embeds. Sibling-gap follow-up for the #92725 salvage; the shared-cap approach mirrors the policy-owner idea from #92748. Co-authored-by: joaomarcos --- agent/context_compressor.py | 17 ++++---- tests/tools/test_browser_console.py | 62 +++++++++++++++++++++++++++++ tools/browser_tool.py | 37 ++++++++++++++--- tools/vision_tools.py | 5 ++- 4 files changed, 107 insertions(+), 14 deletions(-) diff --git a/agent/context_compressor.py b/agent/context_compressor.py index aa3d7a7846..881e7cfe45 100644 --- a/agent/context_compressor.py +++ b/agent/context_compressor.py @@ -3942,16 +3942,19 @@ class ContextCompressor(ContextEngine): threshold (≈100K tokens on a 1M window) and would protect the entire session, pruning nothing. - ``_prune_old_tool_results`` runs all three deterministic passes: + ``_prune_old_tool_results`` runs all deterministic passes: (1) dedup byte-identical tool results — keeps the newest full copy and back-references older exact duplicates ANYWHERE in the list (including the protected tail), so no unique content is ever lost; (2) summarize non-tail tool results larger than ``min_prune_chars``; (3) truncate - oversized tool_call arguments on non-tail assistant messages. Only - pass (2)'s floor is raised by ``proactive_prune_min_result_chars``; - passes (1) and (3) keep their own fixed floors. The recent-tail - protection applies to passes (2) and (3); pass (1) is tail-agnostic by - design because dedup is lossless. + oversized tool_call arguments on non-tail assistant messages; + (3.5) retire image payloads on all but the newest + ``_MAX_KEEP_TOOL_IMAGES`` image-bearing tool results — tail-agnostic + and lossy by design (#92699). Only pass (2)'s floor is raised by + ``proactive_prune_min_result_chars``; passes (1) and (3) keep their + own fixed floors. The recent-tail protection applies to passes (2) + and (3); pass (1) is tail-agnostic by design because dedup is + lossless. PROMPT-CACHE CONTRACT: a committed prune rewrites message bodies the provider has already seen, invalidating the cached prefix from the @@ -3975,7 +3978,7 @@ class ContextCompressor(ContextEngine): before = sum(_estimate_msg_budget_tokens(m) for m in messages) if before < self._proactive_prune_rearm_tokens: return messages, 0 - # Capability gate BEFORE the expensive 3-pass scan: a bound store that + # Capability gate BEFORE the expensive multi-pass scan: a bound store that # can't persist the prune atomically (duck-typed/plugin session store # without archive_and_compact) makes every prune a permanent no-op, so # don't pay the scan for it on every eligible iteration. diff --git a/tests/tools/test_browser_console.py b/tests/tools/test_browser_console.py index abed1bb380..fc5c4ab71e 100644 --- a/tests/tools/test_browser_console.py +++ b/tests/tools/test_browser_console.py @@ -318,6 +318,68 @@ class TestBrowserVisionConfig: mock_get_vision_model.assert_not_called() mock_llm.assert_not_called() + def test_browser_vision_native_fast_path_caps_history_embed(self, tmp_path): + """Oversized screenshots are resized before entering history (#92699). + + browser_vision's native fast path bakes the data URL into the tool + result exactly like vision_analyze — without the proactive resize a + full-res screenshot rides every later request uncapped. + """ + pytest.importorskip("PIL") + import base64 + from io import BytesIO + + from PIL import Image + + from agent.auxiliary_client import clear_runtime_main, set_runtime_main + from tools.browser_tool import browser_vision + from tools.vision_tools import _EMBED_MAX_DIMENSION, _EMBED_TARGET_BYTES + + shots_dir = tmp_path / "browser_screenshots" + shots_dir.mkdir() + screenshot = shots_dir / "shot.png" + # Taller than the long-edge cap so the resize path must fire. + Image.new("RGB", (400, _EMBED_MAX_DIMENSION + 500), (0, 100, 0)).save( + screenshot, format="PNG" + ) + + set_runtime_main("brand-new-provider", "llava-v1.6") + try: + with ( + patch("hermes_constants.get_hermes_dir", return_value=shots_dir), + patch("tools.browser_tool._cleanup_old_screenshots"), + patch( + "tools.browser_tool._run_browser_command", + return_value={ + "success": True, + "data": {"path": str(screenshot)}, + }, + ), + patch( + "hermes_cli.config.load_config", + return_value={"model": {"supports_vision": True}}, + ), + patch("tools.browser_tool.call_llm") as mock_llm, + ): + result = browser_vision("what is on the page?", task_id="test") + finally: + clear_runtime_main() + + assert isinstance(result, dict) + assert result["_multimodal"] is True + url = next( + p["image_url"]["url"] + for p in result["content"] + if p.get("type") == "image_url" + ) + assert len(url) <= _EMBED_TARGET_BYTES, ( + f"embedded browser screenshot {len(url) / 1024:.0f} KB exceeds the " + f"history-reuse cap {_EMBED_TARGET_BYTES / 1024:.0f} KB" + ) + with Image.open(BytesIO(base64.b64decode(url.partition(",")[2]))) as img: + assert max(img.size) <= _EMBED_MAX_DIMENSION + mock_llm.assert_not_called() + def test_browser_vision_text_mode_blocks_native_fast_path(self, tmp_path): """Explicit text routing → aux LLM used even with supports_vision.""" from agent.auxiliary_client import clear_runtime_main, set_runtime_main diff --git a/tools/browser_tool.py b/tools/browser_tool.py index 3d191a874c..0d595f0f5a 100644 --- a/tools/browser_tool.py +++ b/tools/browser_tool.py @@ -4711,26 +4711,46 @@ def browser_vision(question: str, annotate: bool = False, task_id: Optional[str] ), }, ensure_ascii=False) - # Convert screenshot to base64 at full resolution. - _screenshot_bytes = screenshot_path.read_bytes() - _screenshot_b64 = base64.b64encode(_screenshot_bytes).decode("ascii") - data_url = f"data:image/png;base64,{_screenshot_b64}" + # NOTE: the full-resolution base64 encode is deliberately deferred. + # The native fast path below sizes its own history-reuse embed via + # _resize_image_for_vision (stat-based quick estimate — no full-res + # encode when oversized), and only the aux-LLM fallback path needs + # the one-shot full-res data URL. # Fast path: when native image routing is in effect for the active main # model, attach the screenshot directly instead of describing it through # an auxiliary vision LLM. The model inspects the pixels on its next # turn — no aux call, no information loss. Consistent with vision_analyze. from tools.vision_tools import ( + _EMBED_MAX_DIMENSION, + _EMBED_TARGET_BYTES, _build_native_vision_tool_result, + _resize_image_for_vision, _should_use_native_vision_fast_path, ) if _should_use_native_vision_fast_path(): + # History-reuse cap (#92699): this embed is baked into the tool + # result and re-sent on every later turn, exactly like + # vision_analyze's native path — apply the same proactive resize + # so full-res screenshots can't enter immutable history uncapped. + # The helper's internal stat/dimension quick-estimate skips the + # resize (and encodes directly) when the screenshot is already + # under both caps, so no full-res base64 is built just to be + # thrown away. Fail-open: without Pillow it falls back to the + # raw bytes and the compressor's keep-newest pass still retires + # stale embeds. + data_url = _resize_image_for_vision( + screenshot_path, + mime_type="image/png", + max_base64_bytes=_EMBED_TARGET_BYTES, + max_dimension=_EMBED_MAX_DIMENSION, + ) native_result = _build_native_vision_tool_result( image_url=str(screenshot_path), question=question, image_data_url=data_url, - image_size_bytes=len(_screenshot_bytes), + image_size_bytes=screenshot_path.stat().st_size, ) meta = native_result.setdefault("meta", {}) meta["screenshot_path"] = str(screenshot_path) @@ -4753,6 +4773,13 @@ def browser_vision(question: str, annotate: bool = False, task_id: Optional[str] f"Focus on answering the user's specific question." ) + # Aux-LLM path: one-shot analysis, not baked into history — encode at + # full resolution here (the pre-existing 5 MB oversize guard below + # still applies). + _screenshot_bytes = screenshot_path.read_bytes() + _screenshot_b64 = base64.b64encode(_screenshot_bytes).decode("ascii") + data_url = f"data:image/png;base64,{_screenshot_b64}" + # Use the centralized LLM router vision_model = _get_vision_model() logger.debug("browser_vision: analysing screenshot (%d bytes)", diff --git a/tools/vision_tools.py b/tools/vision_tools.py index 37898302ed..074dd14103 100644 --- a/tools/vision_tools.py +++ b/tools/vision_tools.py @@ -631,8 +631,9 @@ _MAX_BASE64_BYTES = 20 * 1024 * 1024 # safety nets; those are one-shot viewing limits, not history-reuse sizes. # A 4 MB / 7900px embed was observed at ~400K chars and ~100–260K billed # tokens per image (#92699), so we size for model reading instead: 256 KB -# is tens of KB after JPEG encode of a 1568px screenshot, well under every -# provider's per-image limit, and cheap enough to ride the session. +# keeps a 1568px screenshot cheap enough to ride the session (PNGs that +# exceed it are downscaled further by the byte-budget ladder), well under +# every provider's per-image limit. _EMBED_TARGET_BYTES = 256 * 1024 # Proactive embed dimension cap (px, longest side). Anthropic still rejects