fix(browser): cap browser_vision native embeds for history reuse
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 <joaomarcosdias444@gmail.com>
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
+32
-5
@@ -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)",
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user