fix(vision): delegated subagents stop re-loading the same image on the native fast path
A delegate_task child asked to transcribe five screenshots called vision_analyze 158 times on those files (full loads alternating with region crops) until its provider quota ran out; every native load bakes the image into history and is re-sent on each later API call, and nothing refused the repeat (#112095). tools/vision_tools_history_budget.py now keeps a per-(session id, resolved source) embed counter. Region crops share their file's key. When the cap is reached _vision_analyze_native returns a tool error that says the image has already been loaded N times and to answer from what is visible or ask the user, instead of another embed. Only successful embeds count, so a failed read never eats the budget. Default: `vision.max_calls_per_image` unset caps only delegated subagents (agent.delegation_context.is_delegated_child_process_context) at 3 — they run unattended and CLI /steer targets the parent — while the main agent stays unlimited; an explicit value applies everywhere and 0 means unlimited. Co-authored-by: Evi Nova <66773372+Tranquil-Flow@users.noreply.github.com> Co-authored-by: gumclaw <gumclaw@gumroad.com>
This commit is contained in:
@@ -1168,6 +1168,10 @@ DEFAULT_CONFIG = {
|
||||
# Byte budget for one embedded image (clamped 64 KiB..4 MiB). Raise it for dense phone
|
||||
# screenshots of tables the model calls "unreadable" at 256 KB.
|
||||
"embed_target_bytes": 256 * 1024,
|
||||
# How often vision_analyze may embed the SAME image (region crops included) per session.
|
||||
# null = 3 inside delegated subagents (they run unattended), unlimited for the main agent;
|
||||
# an explicit number applies everywhere; 0 = unlimited.
|
||||
"max_calls_per_image": None,
|
||||
},
|
||||
# "Hey Hermes" hands-free wake word: always-on, on-device hotword detection that starts a fresh
|
||||
# voice session. Off by default; toggle with /wake.
|
||||
|
||||
@@ -1,15 +1,18 @@
|
||||
"""Invariants for the native-embed history budgets (#112095).
|
||||
|
||||
A native ``vision_analyze`` result is re-sent on every later API call, so the per-embed byte budget
|
||||
must follow ``vision.embed_target_bytes`` instead of a hardcoded 256 KB.
|
||||
A native ``vision_analyze`` result is re-sent on every later API call, so (1) a delegated subagent
|
||||
may not embed the same image without limit and (2) the per-embed byte budget must follow
|
||||
``vision.embed_target_bytes`` instead of a hardcoded 256 KB.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import json
|
||||
import random
|
||||
|
||||
import pytest
|
||||
|
||||
from agent.delegation_context import delegated_child_context
|
||||
from hermes_cli.config import get_config_path
|
||||
from tools import vision_tools_history_budget as budget
|
||||
from tools.vision_tools import _vision_analyze_native
|
||||
@@ -17,6 +20,15 @@ from tools.vision_tools import _vision_analyze_native
|
||||
PIL = pytest.importorskip("PIL.Image")
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _fresh_counters():
|
||||
with budget._repeat_lock:
|
||||
budget._repeat_counts.clear()
|
||||
yield
|
||||
with budget._repeat_lock:
|
||||
budget._repeat_counts.clear()
|
||||
|
||||
|
||||
def _write_config(text: str) -> None:
|
||||
path = get_config_path()
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
@@ -37,10 +49,42 @@ def _load(image, region=None):
|
||||
return asyncio.get_event_loop().run_until_complete(_vision_analyze_native(image, "q", region=region))
|
||||
|
||||
|
||||
def _embedded(result) -> bool:
|
||||
return isinstance(result, dict) and result.get("_multimodal") is True
|
||||
|
||||
|
||||
def _embed_len(result) -> int:
|
||||
return len(next(p["image_url"]["url"] for p in result["content"] if p.get("type") == "image_url"))
|
||||
|
||||
|
||||
class TestRepeatCap:
|
||||
def test_delegated_subagent_is_refused_after_three_loads_of_one_image(self, tmp_path):
|
||||
"""Full loads and region crops of the same file share one counter; the refusal names the
|
||||
knob and tells the model to answer from what it already has (no fourth embed)."""
|
||||
shot = _png(tmp_path / "shot.png")
|
||||
with delegated_child_context("child-session"):
|
||||
assert _embedded(_load(shot))
|
||||
assert _embedded(_load(shot, region=[0, 0, 8, 8]))
|
||||
assert _embedded(_load(shot))
|
||||
refused = _load(shot, region=[4, 4, 12, 12])
|
||||
other = _load(_png(tmp_path / "other.png"))
|
||||
assert isinstance(refused, str)
|
||||
payload = json.loads(refused)
|
||||
assert payload["success"] is False
|
||||
assert "already been loaded" in payload["error"] and "max_calls_per_image" in payload["error"]
|
||||
assert _embedded(other), "a different image in the same session is not affected"
|
||||
|
||||
def test_main_agent_is_unlimited_unless_configured(self, tmp_path):
|
||||
shot = _png(tmp_path / "shot.png")
|
||||
assert all(_embedded(_load(shot)) for _ in range(5))
|
||||
|
||||
_write_config("vision:\n max_calls_per_image: 1\n")
|
||||
with budget._repeat_lock:
|
||||
budget._repeat_counts.clear()
|
||||
assert _embedded(_load(shot))
|
||||
assert json.loads(_load(shot))["success"] is False
|
||||
|
||||
|
||||
class TestEmbedTargetBytes:
|
||||
def test_native_embed_follows_configured_budget(self, tmp_path):
|
||||
"""A 400x400 noisy PNG (~160 KB base64) rides under the 256 KB default untouched, and is
|
||||
|
||||
@@ -36,7 +36,11 @@ def _load_auxiliary_client() -> None:
|
||||
from hermes_constants import get_hermes_dir
|
||||
from tools.debug_helpers import DebugSession
|
||||
from tools.website_policy import check_website_access
|
||||
from tools.vision_tools_history_budget import resolve_embed_target_bytes as _resolve_embed_target_bytes
|
||||
from tools.vision_tools_history_budget import (
|
||||
record_embed as _record_embed,
|
||||
repeat_refusal as _repeat_refusal,
|
||||
resolve_embed_target_bytes as _resolve_embed_target_bytes,
|
||||
)
|
||||
from tools.vision_tools_image_prep import (
|
||||
_VISION_MAX_VALIDATED_AGGREGATE_PIXELS,
|
||||
_VISION_MAX_VALIDATED_FRAME_COUNT,
|
||||
@@ -585,6 +589,9 @@ async def _vision_analyze_native(
|
||||
or a JSON error string (the normal tool-result contract) on failure."""
|
||||
if not isinstance(image_url, str) or not image_url.strip():
|
||||
return tool_error("image_url is required", success=False)
|
||||
refusal = _repeat_refusal(image_url)
|
||||
if refusal is not None:
|
||||
return refusal
|
||||
prepared: Optional[_PreparedImage] = None
|
||||
try:
|
||||
from tools.interrupt import is_interrupted
|
||||
@@ -612,6 +619,7 @@ async def _vision_analyze_native(
|
||||
# Reject rather than embed a session-wedging payload.
|
||||
if len(image_data_url) > _MAX_BASE64_BYTES:
|
||||
return tool_error(_too_large_message(image_data_url), success=False)
|
||||
_record_embed(image_url)
|
||||
return _build_native_vision_tool_result(
|
||||
image_url=image_url, question=question, image_data_url=image_data_url,
|
||||
image_size_bytes=prepared.size_bytes,
|
||||
|
||||
@@ -1,17 +1,35 @@
|
||||
"""History-reuse budgets for native vision embeds (config section ``vision``).
|
||||
|
||||
A native ``vision_analyze`` result bakes the image into conversation history, where it is
|
||||
re-sent on every later API call. ``vision.embed_target_bytes`` bounds that cost
|
||||
(how large one embed may be); see #112095 for why 256 KB is a budget, not a constant.
|
||||
re-sent on every later API call. Two knobs bound that cost: ``vision.embed_target_bytes``
|
||||
(how large one embed may be) and ``vision.max_calls_per_image`` (how often the same image
|
||||
may be embedded per session). See #112095: a delegated subagent re-loaded five screenshots
|
||||
158 times in 15 minutes because nothing refused the repeat.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import hashlib
|
||||
import os
|
||||
import threading
|
||||
from pathlib import Path
|
||||
from typing import Optional
|
||||
|
||||
from tools.registry import tool_error
|
||||
|
||||
# 256 KB keeps a 1568px screenshot cheap enough to ride the session (#92699); the clamp keeps one
|
||||
# setting from turning every later request into a multi-megabyte resend or a useless thumbnail.
|
||||
_DEFAULT_EMBED_TARGET_BYTES = 256 * 1024
|
||||
_MIN_EMBED_TARGET_BYTES = 64 * 1024
|
||||
_MAX_EMBED_TARGET_BYTES = 4 * 1024 * 1024
|
||||
|
||||
# Delegated subagents run unattended and cannot be steered mid-loop from the CLI, so they get a
|
||||
# cap by default; the main agent stays unlimited unless the user sets ``vision.max_calls_per_image``.
|
||||
_SUBAGENT_REPEAT_CAP = 3
|
||||
_REPEAT_COUNTS_MAX_KEYS = 4096
|
||||
|
||||
_repeat_counts: dict[tuple[str, str], int] = {}
|
||||
_repeat_lock = threading.Lock()
|
||||
|
||||
|
||||
def _cfg_vision(key: str, default=None):
|
||||
"""``vision.<key>`` from config.yaml; ``default`` when config is unavailable."""
|
||||
@@ -32,3 +50,64 @@ def resolve_embed_target_bytes() -> int:
|
||||
except (TypeError, ValueError, OverflowError):
|
||||
return _DEFAULT_EMBED_TARGET_BYTES
|
||||
return min(max(target, _MIN_EMBED_TARGET_BYTES), _MAX_EMBED_TARGET_BYTES)
|
||||
|
||||
|
||||
def resolve_repeat_cap() -> int:
|
||||
"""Per-image embed cap for this session; 0 = unlimited.
|
||||
|
||||
``vision.max_calls_per_image`` unset (or unparseable) → ``_SUBAGENT_REPEAT_CAP`` inside a
|
||||
delegated subagent, unlimited for the main agent. An explicit value applies everywhere.
|
||||
"""
|
||||
raw = _cfg_vision("max_calls_per_image")
|
||||
try:
|
||||
if raw is not None and raw != "" and not isinstance(raw, bool):
|
||||
return max(int(raw), 0)
|
||||
except (TypeError, ValueError):
|
||||
pass
|
||||
from agent.delegation_context import is_delegated_child_process_context
|
||||
return _SUBAGENT_REPEAT_CAP if is_delegated_child_process_context() else 0
|
||||
|
||||
|
||||
def _image_key(image_url: str) -> str:
|
||||
"""Stable identity for an image source: local paths resolve (symlinks, ``~``, ``file://``),
|
||||
URLs drop their fragment, data URLs hash. Region crops are NOT part of the key — the incident
|
||||
loop alternated full loads and crops of the same files, and both re-embed the image."""
|
||||
if image_url.startswith("data:"):
|
||||
return "data:" + hashlib.sha256(image_url.encode("utf-8", "ignore")).hexdigest()[:32]
|
||||
stripped = image_url.split("#", 1)[0].removeprefix("file://")
|
||||
if "://" in stripped:
|
||||
return "url:" + stripped
|
||||
return "file:" + str(Path(os.path.expanduser(stripped)).resolve())
|
||||
|
||||
|
||||
def _count_key(image_url: str) -> tuple[str, str]:
|
||||
from gateway.session_context import get_session_env
|
||||
return get_session_env("HERMES_SESSION_ID", ""), _image_key(image_url)
|
||||
|
||||
|
||||
def repeat_refusal(image_url: str) -> Optional[str]:
|
||||
"""Tool-error JSON when this image already hit its per-session embed cap, else ``None``."""
|
||||
cap = resolve_repeat_cap()
|
||||
if cap <= 0:
|
||||
return None
|
||||
with _repeat_lock:
|
||||
count = _repeat_counts.get(_count_key(image_url), 0)
|
||||
if count < cap:
|
||||
return None
|
||||
return tool_error(
|
||||
f"vision_analyze refused: this image has already been loaded into context {count} time(s) "
|
||||
"in this session (region crops of the same file count too), and every native load re-sends "
|
||||
"the full image on each later API call. Answer from what you can already see, or ask the "
|
||||
f"user. (vision.max_calls_per_image = {cap}; 0 = unlimited)",
|
||||
success=False,
|
||||
)
|
||||
|
||||
|
||||
def record_embed(image_url: str) -> None:
|
||||
"""Count one successful native embed of ``image_url`` for the current session."""
|
||||
key = _count_key(image_url)
|
||||
with _repeat_lock:
|
||||
_repeat_counts[key] = _repeat_counts.get(key, 0) + 1
|
||||
# Bound long-lived gateway memory: evict the oldest (session, image) entries.
|
||||
while len(_repeat_counts) > _REPEAT_COUNTS_MAX_KEYS:
|
||||
_repeat_counts.pop(next(iter(_repeat_counts)))
|
||||
|
||||
Reference in New Issue
Block a user