From af0835a083fb85996013afaacc0e329e9d042eeb Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Sun, 28 Jun 2026 06:04:08 +0000 Subject: [PATCH 1/2] 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 From 94d952c3bb5643eaf779ccef723fa1747f5f436d Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Sun, 28 Jun 2026 05:57:56 +0000 Subject: [PATCH 2/2] fix: harden None providers config across onboarding/providers/routes (salvage of #3967) A config.yaml with an explicit empty/null `providers:` key parses to None in PyYAML. `cfg.get("providers", {})` only returns the {} default when the key is *absent*, so an explicit null yields None instead of an empty mapping. master already guards each read with `isinstance(providers_cfg, dict)`, but the value fed to that guard is still a footgun: the natural chained form `cfg.get("providers", {}).get(...)` (already fixed once in api/config.py) crashes with AttributeError on None. Harden every providers-key read in the three remaining files to `cfg.get("providers") or {}` so an explicit null degrades to an empty dict at the source, matching the existing api/config.py convention. Sites hardened: api/onboarding.py:578 (_provider_api_key_present) api/providers.py:1051 (_provider_has_shadowed_codex_oauth_value) api/providers.py:1219 (_provider_has_key) api/providers.py:1265 (_get_provider_api_key) api/providers.py:2348 (get_providers catalog) api/providers.py:2781 (_clean_provider_key_from_config) api/routes.py:5045 (_context_length_config_api_key_for_provider) api/routes.py:5110 (_context_length_lookup_inputs_for_model) api/routes.py:15916 (_handle_live_models) Adds tests/test_none_providers_config_guard.py: per-file source-form pins (fail on master / on any single-file revert) plus behavioural checks driving the real functions with `providers: None` to prove no crash and parity with the empty-mapping config. Updates the existing test_issue3717 routes-string pin to match the hardened form. This is a focused salvage of grab-bag PR #3967 -- only the None-providers hardening is taken; the PR's unrelated changes (account-usage worker-pool refactor, function removals) are intentionally dropped. Co-authored-by: lidi1011 --- api/onboarding.py | 5 +- api/providers.py | 10 +- api/routes.py | 6 +- ...e3717_context_length_provider_overrides.py | 5 +- tests/test_none_providers_config_guard.py | 189 ++++++++++++++++++ 5 files changed, 205 insertions(+), 10 deletions(-) create mode 100644 tests/test_none_providers_config_guard.py diff --git a/api/onboarding.py b/api/onboarding.py index 7cf1bbe1..ecaf1fbc 100644 --- a/api/onboarding.py +++ b/api/onboarding.py @@ -572,7 +572,10 @@ def _provider_api_key_present( if isinstance(model_cfg, dict) and str(model_cfg.get("api_key") or "").strip(): return True - providers_cfg = cfg.get("providers", {}) + # ``cfg.get("providers", {})`` only returns the default when the key is + # absent; an explicit ``providers:`` (null) in config.yaml yields ``None``. + # ``... or {}`` degrades that null to an empty mapping (salvage of #3967). + providers_cfg = cfg.get("providers") or {} if isinstance(providers_cfg, dict): provider_cfg = providers_cfg.get(provider, {}) if ( diff --git a/api/providers.py b/api/providers.py index ec4a1b49..489aedc8 100644 --- a/api/providers.py +++ b/api/providers.py @@ -1048,7 +1048,7 @@ def _provider_has_shadowed_codex_oauth_value(provider_id: str) -> bool: active_provider = str(model_cfg.get("provider") or "").strip().lower() if active_provider == provider_id: values.append(model_cfg.get("api_key")) - providers_cfg = cfg.get("providers", {}) + providers_cfg = cfg.get("providers") or {} if isinstance(providers_cfg, dict): provider_cfg = providers_cfg.get(provider_id, {}) if isinstance(provider_cfg, dict): @@ -1216,7 +1216,7 @@ def _provider_has_key(provider_id: str) -> bool: if _provider_value_counts_as_api_key(provider_id, model_cfg.get("api_key")): return True # Check providers..api_key - providers_cfg = cfg.get("providers", {}) + providers_cfg = cfg.get("providers") or {} if isinstance(providers_cfg, dict): provider_cfg = providers_cfg.get(provider_id, {}) if isinstance(provider_cfg, dict) and str(provider_cfg.get("api_key") or "").strip(): @@ -1262,7 +1262,7 @@ def _get_provider_api_key(provider_id: str) -> str | None: if model_key and active_provider == provider_id and _provider_value_counts_as_api_key(provider_id, model_key): return model_key - providers_cfg = cfg.get("providers", {}) + providers_cfg = cfg.get("providers") or {} if isinstance(providers_cfg, dict): provider_cfg = providers_cfg.get(provider_id, {}) if isinstance(provider_cfg, dict): @@ -2345,7 +2345,7 @@ def get_providers() -> dict[str, Any]: # Also detect providers from config.yaml providers section cfg = get_config() - providers_cfg = cfg.get("providers", {}) + providers_cfg = cfg.get("providers") or {} if isinstance(providers_cfg, dict): known_ids.update(providers_cfg.keys()) @@ -2778,7 +2778,7 @@ def _clean_provider_key_from_config(provider_id: str) -> None: return # 1. Clean providers..api_key - providers_cfg = cfg.get("providers", {}) + providers_cfg = cfg.get("providers") or {} if isinstance(providers_cfg, dict): provider_cfg = providers_cfg.get(provider_id, {}) if isinstance(provider_cfg, dict) and provider_cfg.get("api_key"): diff --git a/api/routes.py b/api/routes.py index b4daa03b..279b3ca0 100644 --- a/api/routes.py +++ b/api/routes.py @@ -5042,7 +5042,7 @@ def _context_length_config_api_key_for_provider( return resolved return "" - providers_cfg = cfg.get("providers", {}) + providers_cfg = cfg.get("providers") or {} if isinstance(providers_cfg, dict): for provider_key, provider_cfg in providers_cfg.items(): if not isinstance(provider_cfg, dict): @@ -5107,7 +5107,7 @@ def _context_length_lookup_inputs_for_model( custom_providers = None provider_context_length = None - providers_cfg = cfg.get("providers", {}) if isinstance(cfg, dict) else {} + providers_cfg = (cfg.get("providers") or {}) if isinstance(cfg, dict) else {} if isinstance(providers_cfg, dict): for provider_key, provider_cfg in providers_cfg.items(): if not isinstance(provider_cfg, dict): @@ -16243,7 +16243,7 @@ def _handle_live_models(handler, parsed): if _ep: try: import urllib.request - _providers_cfg = cfg.get("providers", {}) + _providers_cfg = cfg.get("providers") or {} _prov = _providers_cfg.get(provider, {}) if isinstance(_providers_cfg, dict) else {} # Only use a provider-scoped key. A top-level model.api_key # is safe here only when it belongs to the requested provider; diff --git a/tests/test_issue3717_context_length_provider_overrides.py b/tests/test_issue3717_context_length_provider_overrides.py index 39e7ab18..f1fe0c20 100644 --- a/tests/test_issue3717_context_length_provider_overrides.py +++ b/tests/test_issue3717_context_length_provider_overrides.py @@ -193,6 +193,9 @@ def test_streaming_fallbacks_use_shared_provider_context_helper(): def test_route_helper_keeps_all_context_length_sources_aligned(): assert "def _context_length_lookup_inputs_for_model" in ROUTES_PY - assert 'cfg.get("providers", {})' in ROUTES_PY + # Hardened against an explicit null ``providers:`` key (salvage of #3967): + # ``cfg.get("providers") or {}`` instead of ``cfg.get("providers", {})`` so + # a config.yaml with ``providers:`` (None) degrades to an empty mapping. + assert 'cfg.get("providers") or {}' in ROUTES_PY assert 'cfg.get("custom_providers")' in ROUTES_PY assert "_model_matches_configured_default" in ROUTES_PY diff --git a/tests/test_none_providers_config_guard.py b/tests/test_none_providers_config_guard.py new file mode 100644 index 00000000..8a5c8ce7 --- /dev/null +++ b/tests/test_none_providers_config_guard.py @@ -0,0 +1,189 @@ +"""Regression guard: a config with an explicit null ``providers:`` key must not +break the provider-config read paths in onboarding / providers / routes, and +must behave exactly as if ``providers`` were an empty mapping. + +Background (salvage of grab-bag PR #3967, thanks @lidi1011) +---------------------------------------------------------- +``cfg.get("providers", {})`` only returns the ``{}`` default when the key is +*absent*. When ``config.yaml`` contains an explicit empty/null key:: + + providers: + +PyYAML parses that as ``None``, so ``cfg.get("providers", {})`` yields ``None`` +(not ``{}``). master already wraps each of these reads in an +``isinstance(providers_cfg, dict)`` guard, so the None value silently disables +the provider lookup -- but the value handed to that guard is still a footgun: +any future maintainer who chains ``cfg.get("providers", {}).get(...)`` (the +natural-looking pattern, already used in api/config.py before its own #3967-style +fix) would crash with ``AttributeError: 'NoneType' object has no attribute +'get'``. + +The hardening replaces every such read with ``cfg.get("providers") or {}`` so an +explicit null degrades to an empty dict at the *source*, matching the existing +api/config.py convention. + +This module pins the fix two ways: + +1. **Source-form pins** (``test__hardens_providers_key_read``): assert that + onboarding / providers / routes no longer contain the unguarded + ``cfg.get("providers", {})`` form and *do* contain the hardened + ``cfg.get("providers") or {}`` form. These FAIL on master (where the + unguarded form is present) and would FAIL again if anyone reverts the guard + in any one of the three files -- so the test is non-vacuous per file. + +2. **Behavioural checks**: drive the real functions with ``providers: None`` and + assert they neither raise nor diverge from the ``providers: {}`` / absent-key + behaviour. This guards the runtime contract end to end. +""" + +import ast +import pathlib + +import pytest + +import api.config as config +import api.onboarding as onboarding +import api.providers as providers +import api.routes as routes + +_REPO = pathlib.Path(__file__).resolve().parents[1] + +_UNGUARDED = 'cfg.get("providers", {})' +_HARDENED = 'cfg.get("providers") or {}' + + +def _src(name: str) -> str: + return (_REPO / "api" / name).read_text(encoding="utf-8") + + +# --- source-form pins (fail on master / on any per-file revert) ----------- + + +@pytest.mark.parametrize("filename", ["onboarding.py", "providers.py", "routes.py"]) +def test_file_hardens_providers_key_read_against_none(filename): + """Each target file must read the providers key as ``... or {}``. + + Non-vacuous: on master the unguarded ``cfg.get("providers", {})`` form is + present in every one of these files, so this assertion fails there. Reverting + the guard in any single file re-fails it -- the guard is pinned per file. + """ + source = _src(filename) + # The hardened form must appear at least once. + assert _HARDENED in source, ( + f"api/{filename} should read the providers key as `{_HARDENED}` so an " + f"explicit null `providers:` degrades to an empty mapping (salvage #3967)." + ) + # The unguarded form must not appear as live code. It may legitimately appear + # inside an explanatory comment, so strip comment-only occurrences before + # asserting absence. + code_lines = [] + for line in source.splitlines(): + stripped = line.lstrip() + if stripped.startswith("#"): + continue + # Drop trailing inline comments crudely (good enough: the token we look + # for contains no '#'). + code_lines.append(line.split("#", 1)[0]) + code = "\n".join(code_lines) + assert _UNGUARDED not in code, ( + f"api/{filename} still reads the providers key as the unguarded " + f"`{_UNGUARDED}`; an explicit `providers:` (None) in config.yaml would " + f"yield None instead of an empty mapping (salvage #3967)." + ) + + +@pytest.mark.parametrize("filename", ["onboarding.py", "providers.py", "routes.py"]) +def test_target_files_parse(filename): + """Sanity: the hardened files are valid Python (guards real edits).""" + ast.parse(_src(filename), filename=filename) + + +# --- behavioural: onboarding ---------------------------------------------- + + +def _onboarding_cfg(providers_value): + return { + "model": {"provider": "custom", "api_key": "", "base_url": "http://localhost:1"}, + "providers": providers_value, + "custom_providers": [], + } + + +def test_onboarding_provider_api_key_present_handles_none_providers(): + """``_provider_api_key_present`` must treat ``providers: None`` like ``{}``.""" + none_cfg = _onboarding_cfg(None) + empty_cfg = _onboarding_cfg({}) + + for provider in ("custom", "openai", "anthropic"): + got_none = onboarding._provider_api_key_present(provider, none_cfg, {}) + got_empty = onboarding._provider_api_key_present(provider, empty_cfg, {}) + assert got_none == got_empty + assert got_none is False # no key configured anywhere + + +def test_onboarding_none_providers_does_not_mask_configured_key(): + """A provider key set under model.api_key is still found with providers: None.""" + cfg = _onboarding_cfg(None) + cfg["model"] = {"provider": "custom", "api_key": "sk-xyz", "base_url": "http://x"} + assert onboarding._provider_api_key_present("custom", cfg, {}) is True + + +# --- behavioural: providers (catalog + key lookup) ------------------------ + + +def _patch_get_config(monkeypatch, cfg): + """Force api.providers.get_config()/api.config.get_config() to return ``cfg``.""" + monkeypatch.setattr(providers, "get_config", lambda: cfg) + # Some helpers import get_config from api.config at call time. + monkeypatch.setattr(config, "get_config", lambda: cfg, raising=False) + + +def _providers_cfg(providers_value): + return { + "model": {"provider": "custom", "api_key": "", "base_url": "http://localhost:1"}, + "providers": providers_value, + "custom_providers": [], + } + + +def test_providers_has_key_handles_none_providers(monkeypatch): + """``_provider_has_key`` must not crash and must match the empty-dict result.""" + monkeypatch.setattr(providers, "_provider_env_var_for", lambda pid: None) + monkeypatch.setattr( + providers, "_load_env_file", lambda path: {}, raising=False + ) + + for providers_value, label in ((None, "none"), ({}, "empty")): + _patch_get_config(monkeypatch, _providers_cfg(providers_value)) + # Should return False (no key) without raising on the None config. + assert providers._provider_has_key("custom") is False, label + assert providers._get_provider_api_key("custom") is None, label + + +# --- behavioural: routes (context-length helper + key resolution) ---------- + + +def test_routes_context_length_helper_handles_none_providers(): + """``_context_length_lookup_inputs_for_model`` must accept providers: None.""" + cfg_none = { + "model": {"provider": "custom", "base_url": "http://localhost:1"}, + "providers": None, + "custom_providers": [], + } + cfg_empty = dict(cfg_none, providers={}) + + # Pass cfg explicitly so the helper does not hit real config/disk. + out_none = routes._context_length_lookup_inputs_for_model( + "@custom:my-model", "custom", cfg=cfg_none + ) + out_empty = routes._context_length_lookup_inputs_for_model( + "@custom:my-model", "custom", cfg=cfg_empty + ) + # No crash, and the None config produces the same lookup inputs as the + # empty-mapping config (no provider override available in either case). + # ``_ContextLengthLookupInputs`` is a __slots__ class without __eq__, so + # compare the materialised fields rather than object identity. + fields = ("config_context_length", "custom_providers", "base_url", "provider", "api_key") + snapshot_none = {f: getattr(out_none, f) for f in fields} + snapshot_empty = {f: getattr(out_empty, f) for f in fields} + assert snapshot_none == snapshot_empty