diff --git a/hermes_cli/config_defaults.py b/hermes_cli/config_defaults.py index d4b4fd4612..6dd656fd30 100644 --- a/hermes_cli/config_defaults.py +++ b/hermes_cli/config_defaults.py @@ -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. diff --git a/tests/tools/test_vision_history_budget.py b/tests/tools/test_vision_history_budget.py index 2a75e0c2fb..13de7a6452 100644 --- a/tests/tools/test_vision_history_budget.py +++ b/tests/tools/test_vision_history_budget.py @@ -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 diff --git a/tools/vision_tools.py b/tools/vision_tools.py index 06f419a720..ca0f529922 100644 --- a/tools/vision_tools.py +++ b/tools/vision_tools.py @@ -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, diff --git a/tools/vision_tools_history_budget.py b/tools/vision_tools_history_budget.py index d72a91fcdc..70a758c79e 100644 --- a/tools/vision_tools_history_budget.py +++ b/tools/vision_tools_history_budget.py @@ -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.`` 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)))