From af0835a083fb85996013afaacc0e329e9d042eeb Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Sun, 28 Jun 2026 06:04:08 +0000 Subject: [PATCH] fix(streaming): delegate image routing to agent canonical, preserve unknown-model multimodal (salvage of #4113) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WebUI's api/streaming.py:_resolve_image_input_mode was a LOCAL re-implementation of agent/image_routing.py:decide_image_input_mode that had DIVERGED from the canonical function. The local copy never consulted the active model's vision capability — it only checked a handful of config heuristics — so it returned "native" (embedding the image as an image_url part) even for models KNOWN to lack vision. That silently sends pixels to a blind model (#21160-class failure) instead of routing through the text (vision_analyze) pipeline. Fix: delegate to the canonical agent.image_routing.decide_image_input_mode (single source of truth), with a try/except fallback that preserves the historical WebUI behaviour when the agent package is not importable (e.g. the WebUI standalone test environment). Carve-out preserved: the canonical router conservatively returns "text" for UNKNOWN/custom models (no models.dev capability data). WebUI historically forwards those NATIVELY and relies on the agent's strip-and-retry guard (_try_shrink_image_parts_in_messages / _strip_images_from_messages) to downgrade on a provider rejection. We keep that: a canonical "text" verdict is only honoured when there's a real signal — an explicit user choice (image_input_mode: text or a configured auxiliary.vision backend) or a model KNOWN to lack vision. Otherwise we forward native. Tests: - Existing gateway test (forwards_image_attachments_as_multimodal_parts) stays green — unknown "test-model" still forwards native. - New non-vacuous regression: a model KNOWN to be text-only (supports_vision == False) now routes "text" — this FAILS against master, where the old local copy returned "native" for the same config. - Added coverage for the unknown-model carve-out, known-vision native path, explicit-text-signal precedence, and the agent-unavailable fallback. Salvage of #4113 by @gkd2323c. The original PR delegated fully to canonical but did not add the unknown-model carve-out, which would have regressed the WebUI native-forwarding behaviour the gateway test encodes. Co-authored-by: gkd2323c --- api/streaming.py | 108 ++++++++++++++++----- tests/test_webui_gateway_chat_backend.py | 116 +++++++++++++++++++++++ 2 files changed, 200 insertions(+), 24 deletions(-) diff --git a/api/streaming.py b/api/streaming.py index c18159f1..ed11384f 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -1899,38 +1899,98 @@ def _is_valid_image(path: Path, mime: str) -> bool: return False -def _resolve_image_input_mode(cfg: dict) -> str: - """Return ``"native"`` or ``"text"`` based on config, mirroring - ``agent/image_routing.py:decide_image_input_mode``. +def _explicit_text_signal(cfg: dict) -> bool: + """True when the user has explicitly opted into the text (vision_analyze) + image pipeline. - The agent has this logic, but the WebUI's ``_build_native_multimodal_message`` - was unconditionally embedding images as native ``image_url`` parts, completely - bypassing ``image_input_mode``. This caused silent failures when the main model - does not support images and the fallback model is also text-only (#21160-related). + Two explicit signals, either of which means "the user chose text on + purpose" and we must honour it rather than forwarding images natively: + + * ``agent.image_input_mode: text`` — a direct mode override. + * a configured ``auxiliary.vision`` backend (provider not ``auto``/empty, + or an explicit model / base_url) — the user is paying for a dedicated + vision model and wants the text pipeline regardless of the main model. + + This mirrors the explicit-signal portion of + ``agent/image_routing.py:decide_image_input_mode`` and is used both to + interpret *why* the canonical router returned ``"text"`` (so the + unknown-model carve-out only fires when there's no explicit user choice) + and as the fallback decision when the agent package is unavailable. """ + if not isinstance(cfg, dict): + return False agent_cfg = cfg.get("agent") or {} - mode = str(agent_cfg.get("image_input_mode", "auto") or "auto").strip().lower() - if mode not in ("auto", "native", "text"): - mode = "auto" - - if mode == "native": - return "native" - if mode == "text": - return "text" - - # auto: if auxiliary.vision is explicitly configured → text mode - # (user opted into a dedicated vision backend) + if isinstance(agent_cfg, dict): + mode = str(agent_cfg.get("image_input_mode", "auto") or "auto").strip().lower() + if mode == "text": + return True aux = cfg.get("auxiliary") or {} - vision = aux.get("vision") or {} + vision = (aux.get("vision") or {}) if isinstance(aux, dict) else {} + if not isinstance(vision, dict): + return False provider = str(vision.get("provider") or "").strip().lower() model_name = str(vision.get("model") or "").strip() base_url = str(vision.get("base_url") or "").strip() - if provider not in ("", "auto") or model_name or base_url: - return "text" + return provider not in ("", "auto") or bool(model_name) or bool(base_url) - # No explicit vision config, no model-capability lookup available in WebUI. - # Default to native — the agent's ``_strip_images_from_messages`` guard will - # strip images on rejection and retry as text. + +def _resolve_image_input_mode(cfg: dict) -> str: + """Return ``"native"`` or ``"text"`` for current-turn image uploads. + + Delegates the routing decision to ``agent/image_routing.py: + decide_image_input_mode`` — the single source of truth — instead of the + local re-implementation that previously lived here. That copy had DIVERGED + from the canonical function: it returned ``"text"`` (dropping the image) in + cases the canonical router would have forwarded natively, because it never + consulted the active model's vision capability and instead hard-coded a + handful of config heuristics. + + The WebUI keeps one deliberate carve-out on top of the canonical decision: + for UNKNOWN / custom models (no models.dev capability data) the canonical + router conservatively returns ``"text"``, but the WebUI historically + forwards images NATIVELY and relies on the agent's strip-and-retry guard + (``run_agent._try_shrink_image_parts_in_messages`` / + ``_strip_images_from_messages``) to downgrade on a provider rejection. We + preserve that behaviour here: a canonical ``"text"`` verdict is only + honoured when there is a real signal — an explicit user choice + (``image_input_mode: text`` or a configured ``auxiliary.vision`` backend) + or a model KNOWN to lack vision. Otherwise we forward native. + + When the agent package is unavailable (e.g. the WebUI standalone test + environment, where ``import agent`` fails), we fall back to the historical + WebUI behaviour: honour an explicit text signal, otherwise native. + """ + if not isinstance(cfg, dict): + cfg = {} + + try: + from agent.image_routing import decide_image_input_mode, _lookup_supports_vision + from agent.auxiliary_client import _read_main_provider, _read_main_model + + provider = (_read_main_provider() or "").strip() + model = (_read_main_model() or "").strip() + + mode = decide_image_input_mode(provider, model, cfg) + if mode == "native": + return "native" + + # Canonical returned "text". Honour it only when it reflects a genuine + # signal; otherwise apply the WebUI unknown-model native carve-out. + if _explicit_text_signal(cfg): + return "text" + if _lookup_supports_vision(provider, model, cfg) is False: + # Model is KNOWN to be text-only — respect the canonical verdict. + return "text" + # Unknown / custom model (capability is None): WebUI forwards native + # and lets the agent's strip-and-retry guard downgrade on rejection. + return "native" + except Exception: + # Agent package unavailable or import error — preserve historical WebUI + # behaviour: explicit text signal wins, otherwise native. + pass + + if _explicit_text_signal(cfg): + return "text" return "native" diff --git a/tests/test_webui_gateway_chat_backend.py b/tests/test_webui_gateway_chat_backend.py index 6ca5c557..81fce5a9 100644 --- a/tests/test_webui_gateway_chat_backend.py +++ b/tests/test_webui_gateway_chat_backend.py @@ -829,6 +829,122 @@ def test_gateway_chat_worker_forwards_image_attachments_as_multimodal_parts(tmp_ assert content[1]["image_url"]["url"].startswith("data:image/png;base64,") +# ── #4113 (salvage): _resolve_image_input_mode delegates to the agent's +# canonical decide_image_input_mode, but preserves the WebUI carve-out that +# UNKNOWN/custom models still forward images NATIVELY. ────────────────────── +# +# The agent package is not importable in the WebUI standalone test environment +# (``import agent`` raises ModuleNotFoundError), so we inject fake +# ``agent.image_routing`` / ``agent.auxiliary_client`` modules to exercise the +# real delegation branch. The fakes let us control exactly what the canonical +# router returns and what capability lookup reports, so each behaviour is +# pinned independently of models.dev data. + +def _install_fake_agent_routing(monkeypatch, *, decision, supports, + provider="customcorp", model="mystery-9000"): + """Inject fake agent.image_routing + agent.auxiliary_client into sys.modules. + + ``decision`` is what the canonical ``decide_image_input_mode`` returns; + ``supports`` is what ``_lookup_supports_vision`` returns (True / False / + None — None means "unknown / no capability data"). + """ + import sys + import types + + img = types.ModuleType("agent.image_routing") + img.decide_image_input_mode = lambda p, m, cfg: decision + img._lookup_supports_vision = lambda p, m, cfg=None: supports + aux = types.ModuleType("agent.auxiliary_client") + aux._read_main_provider = lambda: provider + aux._read_main_model = lambda: model + pkg = types.ModuleType("agent") + + monkeypatch.setitem(sys.modules, "agent", pkg) + monkeypatch.setitem(sys.modules, "agent.image_routing", img) + monkeypatch.setitem(sys.modules, "agent.auxiliary_client", aux) + + +def test_resolve_image_input_mode_unknown_model_forwards_native(monkeypatch): + """WebUI carve-out: an UNKNOWN/custom model (no capability data) forwards + images natively even though the canonical router conservatively returns + ``"text"`` for it. + + This is the behaviour the gateway image-forwarding test relies on, and the + agent's strip-and-retry guard downgrades to text on a provider rejection. + """ + _install_fake_agent_routing(monkeypatch, decision="text", supports=None) + cfg = {"agent": {"image_input_mode": "auto"}, + "auxiliary": {"vision": {"provider": "auto"}}} + assert streaming._resolve_image_input_mode(cfg) == "native" + + +def test_resolve_image_input_mode_known_text_only_routes_text(monkeypatch): + """NON-VACUOUS regression for #4113's real divergence. + + The OLD local re-implementation never consulted model capability, so a + model KNOWN to lack vision (``supports_vision == False``) still got images + embedded as native ``image_url`` parts — silently sending pixels to a model + that cannot see them (#21160). Delegating to the canonical router fixes + this: a known text-only model now routes through the text (vision_analyze) + pipeline. + + Against master this assertion FAILS — the old code returns ``"native"`` for + this exact config (auto mode, no explicit vision backend) because it ignored + capability entirely. + """ + _install_fake_agent_routing(monkeypatch, decision="text", supports=False) + cfg = {"agent": {"image_input_mode": "auto"}, + "auxiliary": {"vision": {"provider": "auto"}}} + assert streaming._resolve_image_input_mode(cfg) == "text" + + +def test_resolve_image_input_mode_known_vision_model_forwards_native(monkeypatch): + """A model KNOWN to support vision forwards natively (canonical native).""" + _install_fake_agent_routing(monkeypatch, decision="native", supports=True) + cfg = {"agent": {"image_input_mode": "auto"}} + assert streaming._resolve_image_input_mode(cfg) == "native" + + +def test_resolve_image_input_mode_explicit_text_signal_honored(monkeypatch): + """An explicit user choice for the text pipeline is honoured even for an + unknown model — the carve-out only fires when there is NO explicit signal. + + Both an explicit ``agent.image_input_mode: text`` and a configured + ``auxiliary.vision`` backend count as explicit signals. + """ + _install_fake_agent_routing(monkeypatch, decision="text", supports=None) + assert streaming._resolve_image_input_mode( + {"agent": {"image_input_mode": "text"}}) == "text" + assert streaming._resolve_image_input_mode( + {"agent": {"image_input_mode": "auto"}, + "auxiliary": {"vision": {"provider": "openai", "model": "gpt-4o"}}}) == "text" + + +def test_resolve_image_input_mode_fallback_when_agent_unavailable(monkeypatch): + """When the agent package cannot be imported (standalone WebUI env), fall + back to historical WebUI behaviour: explicit text signal wins, else native. + """ + import sys + + # Ensure the delegation import fails: stub agent.image_routing as a module + # that raises on attribute access of the routing fn would still import, so + # instead force an ImportError by mapping the submodule to None. + monkeypatch.setitem(sys.modules, "agent", None) + monkeypatch.setitem(sys.modules, "agent.image_routing", None) + + # No explicit signal -> native (this is what keeps the gateway image test + # green, since agent is not importable there either). + assert streaming._resolve_image_input_mode( + {"agent": {"image_input_mode": "auto"}, + "auxiliary": {"vision": {"provider": "auto"}}}) == "native" + # Explicit text mode -> text. + assert streaming._resolve_image_input_mode( + {"agent": {"image_input_mode": "text"}}) == "text" + # Explicit auxiliary vision backend -> text. + assert streaming._resolve_image_input_mode( + {"auxiliary": {"vision": {"provider": "anthropic"}}}) == "text" + + def test_gateway_use_runs_api_is_default_off(): for env in ({}, {"HERMES_WEBUI_GATEWAY_USE_RUNS_API": ""}): assert _gateway_use_runs_api_enabled({}, env) is False