diff --git a/tests/tools/test_computer_use_vision_routing.py b/tests/tools/test_computer_use_vision_routing.py index 17b8c4d38a..f69afd6d90 100644 --- a/tests/tools/test_computer_use_vision_routing.py +++ b/tests/tools/test_computer_use_vision_routing.py @@ -1,193 +1,83 @@ """Unit tests for tools.computer_use.vision_routing. -Cover the small ``should_route_capture_to_aux_vision`` policy helper that -decides whether a captured screenshot from ``computer_use(action='capture')`` -should be returned as a multimodal envelope (main model handles vision -natively) or pre-analysed via the ``auxiliary.vision`` pipeline so the -main model only sees text. - -The companion end-to-end regression for #24015 lives in -``tests/tools/test_computer_use_capture_routing.py``; this file pins the -unit contract of the helper in isolation so behaviour does not regress -silently if the surrounding ``computer_use`` plumbing is refactored. +``should_route_capture_to_aux_vision`` decides whether a ``computer_use`` capture is returned as a multimodal +envelope (the main model reads the pixels) or pre-analysed through ``auxiliary.vision`` (the main model sees text). +It is the negation of the shared native-tool-result gate, so these cases drive the real gate and patch only the +capability lookups underneath it. The end-to-end regression for #24015 lives in +``tests/tools/test_computer_use_capture_routing.py``. """ from __future__ import annotations from unittest.mock import patch +import pytest + +from tools.computer_use.vision_routing import should_route_capture_to_aux_vision -# --------------------------------------------------------------------------- -# _explicit_aux_vision_override -# --------------------------------------------------------------------------- +def _route(provider, model, cfg, *, catalog, provider_takes_media, veto=False): + with patch("agent.image_routing._lookup_supports_vision", return_value=catalog), \ + patch("tools.vision_tools._supports_media_in_tool_results", return_value=provider_takes_media), \ + patch("tools.vision_tools._profile_rejects_tool_media", return_value=veto): + return should_route_capture_to_aux_vision(provider, model, cfg) -class TestExplicitAuxVisionOverride: - """Mirror agent.image_routing — config detection must agree across paths.""" - - - - - def test_returns_false_when_vision_block_missing(self): - from tools.computer_use.vision_routing import _explicit_aux_vision_override - cfg = {"auxiliary": {"compression": {"provider": "openai"}}} - assert _explicit_aux_vision_override(cfg) is False - - - def test_returns_true_for_provider_auto_plus_explicit_model(self): - """``provider: auto`` + an explicit model still counts as override.""" - from tools.computer_use.vision_routing import _explicit_aux_vision_override - cfg = { - "auxiliary": { - "vision": {"provider": "auto", "model": "claude-3-haiku"}, - } - } - assert _explicit_aux_vision_override(cfg) is True - - - -# --------------------------------------------------------------------------- -# should_route_capture_to_aux_vision -# --------------------------------------------------------------------------- class TestRouteDecision: - """End-to-end policy: explicit override > tool-result support > vision caps.""" + def test_explicit_aux_backend_wins_over_a_vision_main_model(self): + """#24015: a configured auxiliary.vision backend is the de-facto image route in auto mode.""" + cfg = {"auxiliary": {"vision": {"provider": "openrouter", "model": "google/gemini-2.5-flash"}}} + assert _route("anthropic", "claude-opus-4-5", cfg, catalog=True, provider_takes_media=True) is True - def test_explicit_override_routes_to_aux_even_for_vision_main(self): - """Issue #24015 core repro: explicit aux config must win. + def test_provider_auto_plus_explicit_model_counts_as_an_aux_backend(self): + cfg = {"auxiliary": {"vision": {"provider": "auto", "model": "claude-3-haiku"}}} + assert _route("anthropic", "claude-opus-4-5", cfg, catalog=True, provider_takes_media=True) is True - Even if the main model fully supports vision (Anthropic / Claude), - an explicit ``auxiliary.vision`` block means the user wants their - configured backend used. Don't silently bypass it. - """ - from tools.computer_use import vision_routing + def test_vision_main_model_without_aux_backend_stays_native(self): + assert _route("anthropic", "claude-opus-4-5", None, catalog=True, provider_takes_media=True) is False - cfg = { - "auxiliary": { - "vision": { - "provider": "openrouter", - "model": "google/gemini-2.5-flash", - } - } - } - with patch.object(vision_routing, - "_provider_accepts_multimodal_tool_result", - return_value=True): - assert vision_routing.should_route_capture_to_aux_vision( - "anthropic", "claude-opus-4-5", cfg - ) is True + def test_catalog_text_only_model_on_a_media_provider_goes_to_aux(self): + """deepseek-v3.2 on OpenRouter: the provider carries tool-result media, the model cannot see. Routing it + native made the agent backstop swap the screenshot for a 'switch to a vision model' error.""" + assert _route("openrouter", "deepseek/deepseek-v3.2", {}, catalog=False, provider_takes_media=True) is True - def test_non_vision_main_model_routes_to_aux(self): - """The reported #24015 scenario: tencent/hy3-preview has no vision.""" - from tools.computer_use import vision_routing + def test_unknown_provider_and_model_fail_closed_to_aux(self): + assert _route("exotic-provider", "exotic-model", {}, catalog=None, provider_takes_media=False) is True - cfg = {"model": {"default": "tencent/hy3-preview", "provider": "openrouter"}} - with patch.object(vision_routing, - "_provider_accepts_multimodal_tool_result", - return_value=False): - assert vision_routing.should_route_capture_to_aux_vision( - "openrouter", "tencent/hy3-preview", cfg - ) is True + def test_image_input_mode_text_keeps_pixels_out(self): + cfg = {"agent": {"image_input_mode": "text"}} + assert _route("anthropic", "claude-opus-4-5", cfg, catalog=True, provider_takes_media=True) is True - def test_vision_main_model_no_override_keeps_multimodal(self): - """Default path: vision-capable main model + no aux override → native.""" - from tools.computer_use import vision_routing + def test_image_input_mode_native_keeps_a_catalog_unknown_model_native(self): + """A proxy alias the catalog does not know: native mode is the user's explicit word for it.""" + cfg = {"agent": {"image_input_mode": "native"}} + assert _route("anthropic", "my-proxy-claude", cfg, catalog=None, provider_takes_media=True) is False - with patch.object(vision_routing, - "_provider_accepts_multimodal_tool_result", - return_value=True): - assert vision_routing.should_route_capture_to_aux_vision( - "anthropic", "claude-opus-4-5", None - ) is False - - - def test_user_declared_vision_support_keeps_custom_provider_native(self): - """Local/custom VLMs use config as their tool-result image escape hatch.""" - from tools.computer_use import vision_routing - - cfg = { - "model": { - "default": "Qwen3.6-35B-A3B-local-vlm", - "provider": "omlx", - "supports_vision": True, - } - } - with patch.object(vision_routing, - "_provider_accepts_multimodal_tool_result", - return_value=False): - assert vision_routing.should_route_capture_to_aux_vision( - "custom", "Qwen3.6-35B-A3B-local-vlm", cfg - ) is False - - - def test_unknown_provider_capabilities_fail_closed(self): - """When tool-result lookup returns None, route to aux (safe default).""" - from tools.computer_use import vision_routing - - with patch.object(vision_routing, - "_provider_accepts_multimodal_tool_result", - return_value=None): - assert vision_routing.should_route_capture_to_aux_vision( - "exotic-provider", "exotic-model", {} - ) is True - - - def test_explicit_override_wins_over_unknown_caps(self): - """Explicit aux config wins regardless of unknown caps elsewhere.""" - from tools.computer_use import vision_routing - - cfg = {"auxiliary": {"vision": {"provider": "openrouter"}}} - with patch.object(vision_routing, - "_provider_accepts_multimodal_tool_result", - return_value=None): - assert vision_routing.should_route_capture_to_aux_vision( - "openrouter", "tencent/hy3-preview", cfg - ) is True - - -# --------------------------------------------------------------------------- -# Internal lookups — defensive paths -# --------------------------------------------------------------------------- - - - -# --------------------------------------------------------------------------- -# Module surface -# --------------------------------------------------------------------------- + def test_declared_supports_vision_keeps_a_local_vlm_native(self): + """Local/custom VLMs declare support in config; the real lookup reads it (no catalog patch).""" + cfg = {"model": {"default": "Qwen3.6-35B-A3B-local-vlm", "provider": "omlx", "supports_vision": True}} + with patch("tools.vision_tools._profile_rejects_tool_media", return_value=False): + assert should_route_capture_to_aux_vision("custom", "Qwen3.6-35B-A3B-local-vlm", cfg) is False + def test_profile_veto_routes_a_vision_model_to_aux(self): + assert _route("xiaomi", "mimo-v2.5", {}, catalog=True, provider_takes_media=False, veto=True) is True class TestGateAgreementWithVisionAnalyze: - """The capture route and the ``vision_analyze`` fast path derive from one predicate, so the lane never - depends on which tool asked (#115248: deepseek/deepseek-flash went native in one and aux in the other).""" + """Capture, vision_analyze, browser screenshots and MCP images share one predicate (#115248), so the lane never + depends on which tool produced the image.""" - def test_catalog_vision_model_off_the_provider_whitelist_stays_native(self): - from tools.computer_use import vision_routing + @pytest.mark.parametrize("cfg,catalog,provider_takes_media,veto", [ + ({}, True, False, False), # catalog vision off the provider whitelist + ({}, False, True, False), # text-only model on a media provider + ({"agent": {"image_input_mode": "text"}}, True, True, False), + ({"agent": {"image_input_mode": "native"}}, None, True, False), + ({}, True, False, True), # profile veto + ]) + def test_capture_route_is_the_shared_gate(self, cfg, catalog, provider_takes_media, veto): + from tools.vision_tools import _native_tool_result_images - cfg = {"agent": {"image_input_mode": "native"}} - with patch("agent.image_routing._lookup_supports_vision", return_value=True), \ - patch("tools.vision_tools._supports_media_in_tool_results", return_value=False), \ - patch("tools.vision_tools._profile_rejects_tool_media", return_value=False): - assert vision_routing.should_route_capture_to_aux_vision("deepseek", "deepseek-flash", cfg) is False - - def test_profile_veto_still_routes_a_catalog_vision_model_to_aux(self): - from tools.computer_use import vision_routing - - with patch("agent.image_routing._lookup_supports_vision", return_value=True), \ - patch("tools.vision_tools._supports_media_in_tool_results", return_value=False), \ - patch("tools.vision_tools._profile_rejects_tool_media", return_value=True): - assert vision_routing.should_route_capture_to_aux_vision("xiaomi", "mimo-v2.5", {}) is True - - def test_whitelisted_provider_with_catalog_unknown_model_matches_vision_analyze(self): - """provider on the tool-result-media whitelist, model absent from models.dev/config (a proxy alias): - vision_analyze embeds natively, so capture must stay native too instead of demanding a second - ``supports_vision is True`` from the catalog (review follow-up).""" - from tools.computer_use import vision_routing - from tools.vision_tools import _accepts_tool_result_images - - cfg = {"agent": {"image_input_mode": "native"}} - with patch("agent.image_routing._lookup_supports_vision", return_value=None), \ - patch("tools.vision_tools._supports_media_in_tool_results", return_value=True), \ - patch("tools.vision_tools._profile_rejects_tool_media", return_value=False): - assert _accepts_tool_result_images("anthropic", "my-proxy-claude", cfg) is True - assert vision_routing.should_route_capture_to_aux_vision("anthropic", "my-proxy-claude", cfg) is False + with patch("agent.image_routing._lookup_supports_vision", return_value=catalog), \ + patch("tools.vision_tools._supports_media_in_tool_results", return_value=provider_takes_media), \ + patch("tools.vision_tools._profile_rejects_tool_media", return_value=veto): + assert should_route_capture_to_aux_vision("p", "m", cfg) is (not _native_tool_result_images("p", "m", cfg)) diff --git a/tools/computer_use/vision_routing.py b/tools/computer_use/vision_routing.py index c29f4269a4..786f89df65 100644 --- a/tools/computer_use/vision_routing.py +++ b/tools/computer_use/vision_routing.py @@ -1,76 +1,25 @@ -"""Vision-routing decisions for ``computer_use`` capture results. ``capture`` (mode som|vision) returns a -``_multimodal`` screenshot envelope as the tool result. A text-only main model, or a provider that rejects multimodal -tool results, turns that into a hard 400/404 — even with a working ``auxiliary.vision`` model in config. This module -decides: multimodal envelope, or pre-analyse via aux vision so the main model only ever sees text? +"""Vision routing for ``computer_use`` capture results: return the ``_multimodal`` screenshot envelope, or +pre-analyse it via ``auxiliary.vision`` so the main model only sees text. A text-only main model, or a provider +that rejects multimodal tool results, turns the envelope into a hard 400/404. -Decision order (mirrors ``vision_analyze``): -1. ``auxiliary.vision`` explicitly configured (provider not ""/"auto", or model / base_url set) → aux routing; users - who pay for a vision model want it used. -2. User-declared ``supports_vision`` for the active route (escape hatch for custom/local VLMs absent from models.dev) - → honour it (True → multimodal). -3. The shared ``vision_analyze`` gate (profile veto, then provider tool-result media OR catalog vision) says yes → - multimodal — the same predicate, so the lane never depends on which tool asked. -4. Everything else (non-vision model, provider rejecting multimodal tool results, lookup failure) → aux routing. - -Fails *closed* toward aux routing when metadata is missing or ambiguous: a screenshot sent to a model that cannot read -it is a hard failure, while aux routing costs one extra LLM call and yields a usable description. +The decision IS the shared native-tool-result gate (``tools.vision_tools._native_tool_result_images``) that +``vision_analyze``, browser screenshots and MCP image results use: ``agent.image_input_mode``, an explicit +``auxiliary.vision`` backend (the de-facto route in ``auto``) or the capability lookup (config ``supports_vision`` → +catalog → local probes) picks ``native``, AND the route accepts images inside tool results (profile veto first). +One predicate, so a screenshot takes the same lane whichever tool produced it (#115248). Anything the gate cannot +vouch for goes to aux: one extra LLM call beats a screenshot the model cannot read. """ from __future__ import annotations -import logging from typing import Any, Dict, Optional -logger = logging.getLogger(__name__) - -def _explicit_aux_vision_override(cfg: Optional[Dict[str, Any]]) -> bool: - """True when ``auxiliary.vision`` carries a non-default user override; mirrors ``agent.image_routing`` so the capture - and user-attached-image paths agree. ``provider: "auto"``, blanks or a missing block are *not* explicit.""" - aux = cfg.get("auxiliary") if isinstance(cfg, dict) else None - vision = aux.get("vision") if isinstance(aux, dict) else None - if not isinstance(vision, dict): - return False - provider = str(vision.get("provider") or "").strip().lower() - return provider not in ("", "auto") or any(str(vision.get(k) or "").strip() for k in ("model", "base_url")) - -def _lookup_user_declared_supports_vision(provider: str, model: str, cfg: Optional[Dict[str, Any]]) -> Optional[bool]: - """Config-declared ``supports_vision`` for the active route (None on failure).""" - try: - from agent.image_routing import _supports_vision_override - return _supports_vision_override(cfg, provider, model) - except Exception as exc: # pragma: no cover - defensive - logger.debug("computer_use vision_routing: config override lookup failed: %s", exc) - return None - -def _provider_accepts_multimodal_tool_result(provider: str, model: str, cfg: Optional[Dict[str, Any]] = None) -> Optional[bool]: - """Whether *provider*+*model* may carry images inside tool-result messages — the SAME predicate the - ``vision_analyze`` fast path uses (#115248: the two gates disagreed for deepseek/deepseek-flash, so the route - depended on which tool asked). None on import failure so callers fall back to aux, not guess.""" - if not provider: - return None - try: - from tools.vision_tools import _accepts_tool_result_images - except Exception as exc: # pragma: no cover - defensive - logger.debug("computer_use vision_routing: tool-result support lookup failed: %s", exc) - return None - return bool(_accepts_tool_result_images(provider, model, cfg)) def should_route_capture_to_aux_vision(provider: str, model: str, cfg: Optional[Dict[str, Any]]) -> bool: """True iff the screenshot should be pre-analysed via aux vision; False keeps the multimodal envelope. *provider* is - the lower-case canonical id, *model* the slug sent to the provider, *cfg* the loaded ``config.yaml`` dict (or None). - Steps follow the module docstring's decision order.""" - # auto: an explicitly configured auxiliary.vision backend is the DE-FACTO choice — the user named a - # dedicated vision model, so that's what they want images to go through, even when the main model has - # native vision (maintainer decision, 2026-08-28, reversing #29135's fallback-only posture: config that - # only takes effect when the main model gets worse is a trap, not a setting). Native vision remains the - # default for unconfigured installs, and the fallback when the aux backend is unset. - if _explicit_aux_vision_override(cfg): - return True - user_declared = _lookup_user_declared_supports_vision(provider, model, cfg) - if isinstance(user_declared, bool): # True → multimodal, False → aux - return not user_declared - # The shared gate already folds the capability lookup in; demanding a second `is True` here made - # the two lanes disagree for whitelisted providers whose model the catalog does not know. - return not _provider_accepts_multimodal_tool_result(provider, model, cfg) + the lower-case canonical id, *model* the slug sent to the provider, *cfg* the loaded ``config.yaml`` dict (or None).""" + from tools.vision_tools import _native_tool_result_images + return not _native_tool_result_images(provider, model, cfg) + __all__ = ["should_route_capture_to_aux_vision"] diff --git a/tools/vision_tools.py b/tools/vision_tools.py index 63bb9d32e3..e8ceb32748 100644 --- a/tools/vision_tools.py +++ b/tools/vision_tools.py @@ -569,20 +569,22 @@ def _accepts_tool_result_images(provider: str, model: str, cfg: Optional[Dict[st return _lookup_supports_vision(provider, model, cfg) is True +def _native_tool_result_images(provider: str, model: str, cfg: Optional[Dict[str, Any]]) -> bool: + """THE gate for every tool that can hand the main model pixels (``vision_analyze``, browser and + ``computer_use`` screenshots, MCP ``ImageContent``): image routing resolves to ``native`` + (``agent.image_input_mode``, an explicit ``auxiliary.vision`` backend, the catalog) AND the + route accepts images inside tool results. One predicate, so the lane never depends on which + tool produced the image.""" + from agent.image_routing import decide_image_input_mode + return decide_image_input_mode(provider, model, cfg) == "native" and _accepts_tool_result_images(provider, model, cfg) + + def _should_use_native_vision_fast_path() -> bool: - """True when image routing resolves to ``native`` AND the provider accepts images in tool - results, or the user set the ``model.supports_vision`` override (escape hatch for - custom/local providers). Any failure → False.""" + """:func:`_native_tool_result_images` for the active main model; any failure → False.""" try: from agent.auxiliary_client import _read_main_provider, _read_main_model - from agent.image_routing import decide_image_input_mode from hermes_cli.config import load_config - provider = _read_main_provider() - model = _read_main_model() - cfg = load_config() - if decide_image_input_mode(provider, model, cfg) != "native": - return False - return _accepts_tool_result_images(provider, model, cfg) + return _native_tool_result_images(_read_main_provider(), _read_main_model(), load_config()) except Exception as exc: logger.debug("Native vision fast-path check failed: %s", exc) return False diff --git a/website/docs/user-guide/features/vision.md b/website/docs/user-guide/features/vision.md index 4e777c42e3..bff7b1dfa0 100644 --- a/website/docs/user-guide/features/vision.md +++ b/website/docs/user-guide/features/vision.md @@ -223,6 +223,8 @@ The `vision_analyze` tool itself follows the same routing. When the active main For text-only main models (or providers whose tool-result channel doesn't carry images), `vision_analyze` falls back to the legacy path: it asks the configured auxiliary vision model to describe the image and returns the description as plain text. Either way the calling tool signature is the same — the tool decides which path to take at runtime based on the active model. +Every other tool that hands the model an image uses this same decision: browser screenshots, `computer_use` captures (which are described by the auxiliary vision model when the main model can't take them natively), and [MCP image results](mcp.md#image-results). `agent.image_input_mode: text` therefore keeps pixels out of all of them. + ### SVG and other non-raster images on Responses backends Responses-style backends (for example `openai-codex`) accept only inline JPEG, PNG, GIF and WebP; any other `data:image/*` part makes them reject the **whole** request, and because the part stays in history every later turn fails the same way. Hermes handles this at the send layer: an inline **SVG** is rasterized to PNG when a rasterizer is installed (`cairosvg`, `svglib`+`reportlab`, `rsvg-convert`, or `inkscape` — the same soft dependencies `vision_analyze` uses), so the model still sees the drawing. Without a rasterizer, an SVG — and any other unsupported inline format such as BMP or TIFF — is replaced by a short text placeholder (`[image omitted: image/svg+xml is not a supported image format]`) while the valid images in the same message are still sent.