diff --git a/agent/agent_init.py b/agent/agent_init.py index 68fa17b0dff8..9ae224335a99 100644 --- a/agent/agent_init.py +++ b/agent/agent_init.py @@ -432,14 +432,55 @@ def _custom_provider_extra_body_for_agent( model: str, base_url: str, custom_providers: List[Dict[str, Any]], + requested_provider: str = "", ) -> Optional[Dict[str, Any]]: provider_norm = (provider or "").strip().lower() + requested_norm = (requested_provider or "").strip().lower() + if provider_norm == "custom": - provider_key_filter = "" + # ``agent.provider`` is the bare canonical "custom" for every named + # providers:/custom_providers: entry AT AGENT-INIT TIME — the runtime + # resolver (hermes_cli/runtime_provider.py::_resolve_named_custom_runtime) + # never emits "custom:". The entry's actual identity survives + # only on ``agent.requested_provider``. Without filtering on it, two + # entries sharing (base_url, model) — e.g. a vLLM endpoint listed + # twice as "vllm" / "vllm-no-think" with the same model id — are + # indistinguishable here and the matcher falls through to "first + # entry with a non-empty extra_body", silently applying the WRONG + # entry's extra_body to every provider at that endpoint regardless + # of which one is actually selected. + provider_key_filter = requested_norm if requested_norm not in ("", "custom") else "" + if provider_key_filter.startswith("custom:"): + provider_key_filter = provider_key_filter.split(":", 1)[1].strip() elif provider_norm.startswith("custom:"): provider_key_filter = provider_norm.split(":", 1)[1].strip() else: - return None + # A LIVE ``/model`` switch (hermes_cli/model_switch.py's pure + # switch_model(), which every gateway/TUI/CLI switch path calls + # before mutating the live agent) resolves ``target_provider`` to + # the entry's OWN identity ("vllm"), not "custom" — a different + # convention than agent-init resolution. So after any live switch, + # neither ``provider`` nor ``requested_provider`` ever looks like + # "custom"/"custom:" for a named custom provider, and every + # branch above misses. Recognize this shape explicitly: only engage + # when the bare name actually names a configured custom_providers + # entry (by provider_key or display name) — this still can't + # false-positive on a builtin provider (openai, anthropic, ...) + # since those never appear in ``custom_providers``. + known_identities = { + str(e.get("provider_key", "") or "").strip().lower() + for e in (custom_providers or []) if isinstance(e, dict) + } | { + str(e.get("name", "") or "").strip().lower() + for e in (custom_providers or []) if isinstance(e, dict) + } + known_identities.discard("") + if provider_norm in known_identities: + provider_key_filter = provider_norm + elif requested_norm in known_identities: + provider_key_filter = requested_norm + else: + return None target_url = _normalized_custom_base_url(base_url) if not target_url: @@ -472,22 +513,46 @@ def _custom_provider_extra_body_for_agent( def _merge_custom_provider_extra_body(agent, custom_providers: List[Dict[str, Any]]) -> None: + """(Re)apply the active custom provider's ``extra_body`` onto the agent. + + Called at agent init AND again on every live ``/model`` switch (see + ``agent.agent_runtime_helpers.switch_model``) — nothing else touches + ``agent.request_overrides['extra_body']`` on a switch, so without the + second call a provider switched-away-from with an ``extra_body`` (e.g. + a vLLM endpoint listed twice as "vllm" / "vllm-no-think", the latter + with ``chat_template_kwargs.enable_thinking``) leaves its extra_body + stuck on every request for the rest of the session, even after + switching to a provider with none. + + Idempotent/repeatable: each call first strips whatever keys the + *previous* call itself contributed (tracked via + ``agent._custom_provider_extra_body_keys``) before merging in the + newly resolved ``extra_body``, so keys set by something else entirely + (e.g. an explicit fast-mode ``service_tier`` override) are preserved + across the switch. + """ extra_body = _custom_provider_extra_body_for_agent( provider=agent.provider, model=agent.model, base_url=agent.base_url, custom_providers=custom_providers, - ) - if not extra_body: - return + requested_provider=getattr(agent, "requested_provider", ""), + ) or {} overrides = dict(getattr(agent, "request_overrides", {}) or {}) + existing_extra_body = dict(overrides.get("extra_body") or {}) + for key in getattr(agent, "_custom_provider_extra_body_keys", set()): + existing_extra_body.pop(key, None) + merged_extra_body = dict(extra_body) - existing_extra_body = overrides.get("extra_body") - if isinstance(existing_extra_body, dict): - merged_extra_body.update(existing_extra_body) - overrides["extra_body"] = merged_extra_body + merged_extra_body.update(existing_extra_body) + + if merged_extra_body: + overrides["extra_body"] = merged_extra_body + elif "extra_body" in overrides: + del overrides["extra_body"] agent.request_overrides = overrides + agent._custom_provider_extra_body_keys = set(extra_body.keys()) def init_agent( diff --git a/agent/agent_runtime_helpers.py b/agent/agent_runtime_helpers.py index a9bf1a1d38df..a32749097230 100644 --- a/agent/agent_runtime_helpers.py +++ b/agent/agent_runtime_helpers.py @@ -2755,6 +2755,22 @@ def _restore_snapshot() -> None: _runtime_context_length, ) + # ── Re-apply the active custom provider's extra_body ── + # Nothing else in this function touches request_overrides['extra_body'], + # so switching away from a providers:/custom_providers: entry that had one + # (e.g. a vLLM endpoint listed twice as "vllm" / "vllm-no-think", the + # latter carrying extra_body.chat_template_kwargs.enable_thinking) would + # otherwise leave that extra_body stuck on every request for the rest of + # the session, even after switching to a provider with none configured. + try: + from agent.agent_init import _merge_custom_provider_extra_body + _merge_custom_provider_extra_body(agent, _sm_custom_providers or []) + except Exception: + logger.debug( + "switch_model: custom-provider extra_body reconciliation skipped", + exc_info=True, + ) + # ── Re-evaluate prompt caching ── # Refresh the custom-provider snapshot from the config just loaded above # so the per-model ``prompt_caching`` capability lookup sees the same diff --git a/hermes_cli/model_switch.py b/hermes_cli/model_switch.py index d6cd312c9967..d3948e7bb0fd 100644 --- a/hermes_cli/model_switch.py +++ b/hermes_cli/model_switch.py @@ -1981,6 +1981,69 @@ def _extra_headers_from_config(entry: Any) -> dict[str, str]: return normalize_extra_headers(entry.get("extra_headers")) +def _extra_body_identity(entry: Any) -> str: + """Stable, hashable identity for an entry's ``extra_body``. + + Two entries can share (api_url, credential, api_mode) yet send different + request bodies — e.g. a vLLM endpoint listed twice with only + ``extra_body.chat_template_kwargs.enable_thinking`` differing between the + "think" and "no-think" variants. Without this in the group identity the + picker silently collapses them into one row and one of the two configured + ``providers:``/``custom_providers:`` entries becomes unreachable. + """ + if not isinstance(entry, dict): + return "" + extra_body = entry.get("extra_body") + if not isinstance(extra_body, dict) or not extra_body: + return "" + import json + + try: + return json.dumps(extra_body, sort_keys=True, default=str) + except TypeError: + return repr(sorted(extra_body.items(), key=lambda kv: str(kv[0]))) + + +def _group_display_prefix(raw_name: str) -> str: + """Version-stripped display prefix used to decide whether two same- + endpoint ``providers:`` entries are the same logical provider (several + models, one row) or genuinely distinct providers that happen to share a + connection. + + Strips a trailing " — " / " - " suffix (Hermes's own config writer uses + this to separate a provider label from its model, e.g. "Ollama — GLM + 5.1"), then drops trailing numeric/version tokens ("Palantir Claude 4.7 + Opus" → "Palantir Claude") so multi-model entries for the same provider + still collapse into one picker row — this mirrors the cosmetic label + logic section 4 (``custom_providers:``) already uses for its own display + prefix, applied here as part of the *grouping identity* instead of only + the label. + + A name with neither a separator nor a trailing digit token is returned + unchanged. That is what disambiguates a case like a vLLM endpoint listed + twice under distinct names ("vLLM" / "vLLM No-Think", e.g. to toggle + ``extra_body.chat_template_kwargs.enable_thinking`` for a hybrid-thinking + model) — those names carry no version pattern, so they stay distinct + picker rows rather than collapsing into one with only the first entry's + identity reachable. + """ + prefix = str(raw_name or "") + for sep in ("—", " - "): + if sep in prefix: + prefix = prefix.split(sep)[0].strip() + break + tokens = prefix.split() + cut_at = None + for i, tok in enumerate(tokens): + stripped_tok = tok.strip(".,()") + if stripped_tok and any(c.isdigit() for c in stripped_tok): + cut_at = i + break + if cut_at is not None and cut_at >= 2: + prefix = " ".join(tokens[:cut_at]).strip() + return prefix + + def prewarm_picker_cache_async() -> Optional["_threading.Thread"]: """Warm the provider-models disk cache in a background daemon thread. @@ -2658,19 +2721,39 @@ def _has_aws_sdk_creds_for_listing(slug: str) -> bool: # and one "custom:openrouter" from section 4, both labelled identically. _section3_emitted_pairs: set = set() if user_providers and isinstance(user_providers, dict): - # Group ``providers:`` entries by (api_url, key_env, api_mode) so that - # multiple keyed providers pointing at the same endpoint with the - # same credential and wire-protocol collapse into one picker row. - # Mirrors section-4's grouping for ``custom_providers:`` lists. - # Concrete case: a Palantir Foundry Anthropic-proxy with two - # configured models (claude-4.6 + claude-4.7) — both share the same - # api/key_env/api_mode and used to produce two near-duplicate rows - # labelled "Palantir Claude 4.6 Opus" and "Palantir Claude 4.7 Opus"; - # now they appear as a single "Palantir Claude" row with both models - # in the dropdown. Same-host entries with different ``key_env`` or - # ``api_mode`` (e.g. an OpenAI-compat gpt-5.4 alongside the Anthropic - # claude-4.7 on the same Palantir host) keep distinct rows since - # the wire protocol differs. + # Group ``providers:`` entries by (api_url, credential, api_mode, + # headers, extra_body, name-prefix) so that multiple keyed providers + # pointing at the same endpoint with the same credential and wire + # protocol collapse into one picker row — but ONLY when their names + # also identify them as the same logical provider. Mirrors (and now + # matches) section-4's grouping for ``custom_providers:`` lists. + # + # Concrete "same provider" case: a Palantir Foundry Anthropic-proxy + # with two configured models (claude-4.6 + claude-4.7) — both share + # api/key_env/api_mode and are named "Palantir Claude 4.6 Opus" / + # "Palantir Claude 4.7 Opus". The trailing version token is stripped + # (_group_display_prefix) so both fold into a single "Palantir + # Claude" row with both models in the dropdown, instead of two + # near-duplicate rows. + # + # Concrete "distinct providers" case: a vLLM endpoint listed twice — + # same base_url/model, no credential — under deliberately different + # names ("vLLM" / "vLLM No-Think") so the second entry can pin + # ``extra_body: {chat_template_kwargs: {enable_thinking: false}}`` + # for a hybrid-thinking model. Neither name carries a version + # pattern, so the prefix heuristic leaves them unchanged and they + # stay distinct rows. Without this — and without extra_body/headers + # also participating in the identity below — the group_key collapsed + # to one row keyed on the FIRST entry's slug, and the second entry's + # extra_body became permanently unreachable from the picker even + # though it resolved correctly via ``--provider vllm-no-think`` or a + # direct config edit (request-time resolution matches by slug, not + # by this grouping). + # + # Same-host entries with different ``key_env`` or ``api_mode`` (e.g. + # an OpenAI-compat gpt-5.4 alongside the Anthropic claude-4.7 on the + # same Palantir host) keep distinct rows since the wire protocol + # differs — unaffected by any of the above. from collections import OrderedDict as _OD3 from hermes_cli.config import is_provider_enabled @@ -2712,7 +2795,27 @@ def _has_aws_sdk_creds_for_listing(slug: str) -> bool: # URL, routed by header) and must keep distinct picker rows. entry_extra_headers = _extra_headers_from_config(ep_cfg) headers_identity = tuple(sorted(entry_extra_headers.items())) - group_key = (api_url_norm, credential_identity, api_mode, headers_identity) + # Per-provider extra_body participates in the group identity for + # the same reason as headers: two entries sharing + # (api_url, credential, api_mode, headers) but declaring different + # extra_body (e.g. chat_template_kwargs.enable_thinking) send + # different requests and must stay distinct picker rows. + body_identity = _extra_body_identity(ep_cfg) + # The version-stripped name prefix participates in the group + # identity too (see the module-level docstring on + # _group_display_prefix and the block comment above this loop): + # this is what tells "same provider, more models" (Palantir) + # apart from "distinct provider sharing a connection" (vLLM / + # vLLM No-Think). + group_prefix = _group_display_prefix(display_name) + group_key = ( + api_url_norm, + credential_identity, + api_mode, + headers_identity, + body_identity, + group_prefix.lower(), + ) # ``default_model`` is the legacy key; ``model`` matches what # custom_providers entries use, so accept either. @@ -2730,33 +2833,10 @@ def _has_aws_sdk_creds_for_listing(slug: str) -> bool: entry_models.append(model_id) if group_key not in ep_groups: - # Strip per-model suffix so "Palantir Claude 4.7 Opus" becomes - # "Palantir Claude". Em dash and " - " are the separators - # Hermes's own writer uses (mirrors section-4 grouping). - grp_display = display_name - for sep in ("—", " - "): - if sep in grp_display: - grp_display = grp_display.split(sep)[0].strip() - break - # Drop trailing numeric/version tokens that distinguish per-model - # entries ("Palantir Claude 4.7 Opus" → "Palantir Claude"). - # Keeps the row label short; the model dropdown carries the - # per-version detail. Heuristic: split at the first token whose - # stripped form contains a digit; keep the prefix only if it - # is at least 2 words (avoids over-trimming single-word names). - _toks = grp_display.split() - _cut_at = None - for _i, _t in enumerate(_toks): - _tl = _t.strip(".,()") - if _tl and any(c.isdigit() for c in _tl): - _cut_at = _i - break - if _cut_at is not None and _cut_at >= 2: - grp_display = " ".join(_toks[:_cut_at]).strip() grp_slug = ep_name # primary slug is the first ep_name encountered ep_groups[group_key] = { "slug": grp_slug, - "name": grp_display or display_name, + "name": group_prefix or display_name, "api_url": api_url, "models": [], "has_explicit_models": False, @@ -3022,6 +3102,15 @@ def _has_aws_sdk_creds_for_listing(slug: str) -> bool: entry_extra_headers = _extra_headers_from_config(entry) headers_identity = tuple(sorted(entry_extra_headers.items())) + # Per-provider extra_body participates in the group identity for + # the same reason as headers: two entries sharing + # (api_url, credential, api_mode, headers) but declaring different + # extra_body (e.g. a vLLM endpoint listed twice with only + # extra_body.chat_template_kwargs.enable_thinking differing) send + # different requests and must stay distinct picker rows rather + # than collapsing with only one of the two ``models:`` surviving. + body_identity = _extra_body_identity(entry) + # Display-name prefix (text before " — " / " - "), used both # as a grouping dimension and to derive the row's display name. _display_prefix = raw_name @@ -3030,7 +3119,14 @@ def _has_aws_sdk_creds_for_listing(slug: str) -> bool: _display_prefix = _display_prefix.split(sep)[0].strip() break - group_key = (api_url, credential_identity, api_mode, headers_identity, _display_prefix.lower()) + group_key = ( + api_url, + credential_identity, + api_mode, + headers_identity, + body_identity, + _display_prefix.lower(), + ) if group_key not in groups: # Reuse the prefix computed above as the row display name; # fall back to the raw name if stripping left it empty. diff --git a/tests/agent/test_custom_provider_extra_body.py b/tests/agent/test_custom_provider_extra_body.py index ea94c104f2a2..f0e2c22b1709 100644 --- a/tests/agent/test_custom_provider_extra_body.py +++ b/tests/agent/test_custom_provider_extra_body.py @@ -71,3 +71,208 @@ def test_named_custom_provider_extra_body_matches_provider_key(): ) assert agent.request_overrides == {"extra_body": {"enable_thinking": False}} + + +def test_bare_custom_provider_disambiguates_via_requested_provider(): + """Two providers: entries at the SAME (base_url, model) — one with + extra_body, one without — must not cross-contaminate. + + ``agent.provider`` is always the bare canonical "custom" for named + providers:/custom_providers: entries (the runtime resolver never emits + "custom:"); only ``agent.requested_provider`` carries the actual + selected entry's identity ("vllm" vs "vllm-no-think"). Without filtering + on it, the matcher fell through to "first entry with a non-empty + extra_body" and applied the WRONG entry's extra_body regardless of which + provider was actually selected (repro: a vLLM endpoint listed twice with + the same model id, only "vllm-no-think" carrying + extra_body.chat_template_kwargs.enable_thinking).""" + custom_providers = [ + { + "name": "vLLM", + "provider_key": "vllm", + "base_url": "http://192.168.15.115:8000/v1", + "model": "unsloth/Qwen3.6-35B-A3B-NVFP4", + }, + { + "name": "vLLM No-Think", + "provider_key": "vllm-no-think", + "base_url": "http://192.168.15.115:8000/v1", + "model": "unsloth/Qwen3.6-35B-A3B-NVFP4", + "extra_body": {"chat_template_kwargs": {"enable_thinking": False}}, + }, + ] + + agent_plain = SimpleNamespace( + provider="custom", + requested_provider="vllm", + model="unsloth/Qwen3.6-35B-A3B-NVFP4", + base_url="http://192.168.15.115:8000/v1", + request_overrides={}, + ) + _merge_custom_provider_extra_body(agent_plain, custom_providers) + assert agent_plain.request_overrides == {} + + agent_no_think = SimpleNamespace( + provider="custom", + requested_provider="vllm-no-think", + model="unsloth/Qwen3.6-35B-A3B-NVFP4", + base_url="http://192.168.15.115:8000/v1", + request_overrides={}, + ) + _merge_custom_provider_extra_body(agent_no_think, custom_providers) + assert agent_no_think.request_overrides == { + "extra_body": {"chat_template_kwargs": {"enable_thinking": False}} + } + + +def test_switching_away_from_extra_body_provider_clears_stale_value(): + """A second call for a provider WITHOUT extra_body must clear whatever + the previous call (e.g. at agent init, before a live /model switch) + applied — mirrors what switch_model() now does on every live switch.""" + custom_providers = [ + { + "name": "vLLM", + "provider_key": "vllm", + "base_url": "http://192.168.15.115:8000/v1", + "model": "unsloth/Qwen3.6-35B-A3B-NVFP4", + }, + { + "name": "vLLM No-Think", + "provider_key": "vllm-no-think", + "base_url": "http://192.168.15.115:8000/v1", + "model": "unsloth/Qwen3.6-35B-A3B-NVFP4", + "extra_body": {"chat_template_kwargs": {"enable_thinking": False}}, + }, + ] + agent = SimpleNamespace( + provider="custom", + requested_provider="vllm-no-think", + model="unsloth/Qwen3.6-35B-A3B-NVFP4", + base_url="http://192.168.15.115:8000/v1", + request_overrides={}, + ) + _merge_custom_provider_extra_body(agent, custom_providers) + assert agent.request_overrides == { + "extra_body": {"chat_template_kwargs": {"enable_thinking": False}} + } + + # Simulate a live /model switch to "vllm" (no extra_body). + agent.requested_provider = "vllm" + _merge_custom_provider_extra_body(agent, custom_providers) + assert agent.request_overrides == {} + + +def test_bare_raw_entry_name_as_provider_resolves_via_known_identities(): + """hermes_cli/model_switch.py's live /model-switch path sets + agent.provider to the entry's OWN raw identity (e.g. "vllm-no-think"), + not "custom" or "custom:vllm-no-think" — a different convention than + agent-init resolution. Matching must still work by recognizing the bare + name against the configured custom_providers identities.""" + custom_providers = [ + { + "name": "vLLM", + "provider_key": "vllm", + "base_url": "http://192.168.15.115:8000/v1", + "model": "unsloth/Qwen3.6-35B-A3B-NVFP4", + }, + { + "name": "vLLM No-Think", + "provider_key": "vllm-no-think", + "base_url": "http://192.168.15.115:8000/v1", + "model": "unsloth/Qwen3.6-35B-A3B-NVFP4", + "extra_body": {"chat_template_kwargs": {"enable_thinking": False}}, + }, + ] + + agent_vllm = SimpleNamespace( + provider="vllm", + requested_provider="vllm", + model="unsloth/Qwen3.6-35B-A3B-NVFP4", + base_url="http://192.168.15.115:8000/v1", + request_overrides={}, + ) + _merge_custom_provider_extra_body(agent_vllm, custom_providers) + assert agent_vllm.request_overrides == {} + + agent_no_think = SimpleNamespace( + provider="vllm-no-think", + requested_provider="vllm-no-think", + model="unsloth/Qwen3.6-35B-A3B-NVFP4", + base_url="http://192.168.15.115:8000/v1", + request_overrides={}, + ) + _merge_custom_provider_extra_body(agent_no_think, custom_providers) + assert agent_no_think.request_overrides == { + "extra_body": {"chat_template_kwargs": {"enable_thinking": False}} + } + + +def test_bare_provider_name_matching_builtin_never_leaks_custom_extra_body(): + """A builtin provider name (never present in custom_providers) must not + accidentally match — the known-identities fallback only engages for + names that actually belong to a configured custom_providers entry.""" + agent = SimpleNamespace( + provider="openai", + requested_provider="openai", + model="gpt-5.5", + base_url="https://api.openai.com/v1", + request_overrides={}, + ) + _merge_custom_provider_extra_body( + agent, + [ + { + "name": "vLLM No-Think", + "provider_key": "vllm-no-think", + "base_url": "https://api.openai.com/v1", + "model": "gpt-5.5", + "extra_body": {"chat_template_kwargs": {"enable_thinking": False}}, + } + ], + ) + assert agent.request_overrides == {} + + +def test_switching_preserves_unrelated_caller_override_across_switch(): + """A caller-set override key unrelated to the custom-provider config + (e.g. an explicit fast-mode service_tier) must survive a switch that + also changes the custom-provider-derived extra_body.""" + custom_providers = [ + { + "name": "A", + "provider_key": "a", + "base_url": "https://proxy.example.com/v1", + "model": "m", + "extra_body": {"chat_template_kwargs": {"enable_thinking": True}}, + }, + { + "name": "B", + "provider_key": "b", + "base_url": "https://proxy.example.com/v1", + "model": "m", + "extra_body": {"chat_template_kwargs": {"enable_thinking": False}}, + }, + ] + agent = SimpleNamespace( + provider="custom", + requested_provider="a", + model="m", + base_url="https://proxy.example.com/v1", + request_overrides={"extra_body": {"service_tier": "flex"}}, + ) + _merge_custom_provider_extra_body(agent, custom_providers) + assert agent.request_overrides == { + "extra_body": { + "chat_template_kwargs": {"enable_thinking": True}, + "service_tier": "flex", + } + } + + agent.requested_provider = "b" + _merge_custom_provider_extra_body(agent, custom_providers) + assert agent.request_overrides == { + "extra_body": { + "chat_template_kwargs": {"enable_thinking": False}, + "service_tier": "flex", + } + } diff --git a/tests/hermes_cli/test_model_switch_custom_providers.py b/tests/hermes_cli/test_model_switch_custom_providers.py index 381c5870c9bb..8f5986d3d6fa 100644 --- a/tests/hermes_cli/test_model_switch_custom_providers.py +++ b/tests/hermes_cli/test_model_switch_custom_providers.py @@ -715,6 +715,42 @@ def fake_fetch_api_models(api_key, base_url, **kwargs): assert models_by_row == {("model-a",), ("model-b",)} +def test_same_endpoint_different_extra_body_not_collapsed(monkeypatch): + """Entries sharing (api_url, credential, api_mode, headers) but declaring + different extra_body must NOT collapse into one picker row — e.g. a vLLM + endpoint listed twice where only extra_body.chat_template_kwargs + .enable_thinking differs between "think" and "no-think" variants.""" + monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda: {}) + monkeypatch.setattr("hermes_cli.providers.HERMES_OVERLAYS", {}) + monkeypatch.setattr("hermes_cli.models.fetch_api_models", lambda *a, **k: []) + + providers = list_authenticated_providers( + current_provider="openrouter", + current_base_url="https://openrouter.ai/api/v1", + custom_providers=[ + { + "name": "vLLM", + "base_url": "http://192.168.15.115:8000/v1", + "model": "unsloth/Qwen3.6-35B-A3B-NVFP4", + "discover_models": False, + "models": {"unsloth/Qwen3.6-35B-A3B-NVFP4": {}}, + }, + { + "name": "vLLM", + "base_url": "http://192.168.15.115:8000/v1", + "model": "unsloth/Qwen3.6-35B-A3B-NVFP4", + "discover_models": False, + "models": {"unsloth/Qwen3.6-35B-A3B-NVFP4": {}}, + "extra_body": {"chat_template_kwargs": {"enable_thinking": False}}, + }, + ], + max_models=50, + ) + + rows = [ + p for p in providers if p.get("api_url") == "http://192.168.15.115:8000/v1" + ] + assert len(rows) == 2, f"expected 2 rows, got {len(rows)}: {rows}" diff --git a/tests/hermes_cli/test_provider_section3_grouping.py b/tests/hermes_cli/test_provider_section3_grouping.py index 2889fc723c63..0449337607b9 100644 --- a/tests/hermes_cli/test_provider_section3_grouping.py +++ b/tests/hermes_cli/test_provider_section3_grouping.py @@ -6,6 +6,16 @@ mirroring section 4's grouping for ``custom_providers:``. These are invariant tests — grouping identity, header-routed separation, list-of-dict model declarations, and display-only RID stripping. + +Extended for the "distinct name" invariant: two entries at the same endpoint +whose names carry no shared version pattern (unlike "Palantir Claude 4.6/4.7 +Opus") are genuinely distinct providers, not "one provider, several models", +and must stay separate picker rows regardless of whether anything else in +their config differs — extra_body, extra_headers, or nothing at all. Before +this, section 3 grouped purely on connection identity (never on name), so a +second same-named-endpoint entry could become permanently unreachable from +the picker even when perfectly valid via ``--provider `` or a direct +config edit. """ import hermes_cli.providers as providers_mod @@ -80,6 +90,66 @@ def test_different_extra_headers_keep_distinct_rows(monkeypatch): assert len(rows) == 2 +def test_different_extra_body_keeps_distinct_rows(monkeypatch): + """Two providers: entries sharing (api_url, credential, api_mode, headers) + but declaring different extra_body must NOT collapse — e.g. a vLLM + endpoint listed twice where only extra_body.chat_template_kwargs + .enable_thinking differs between the "think" and "no-think" variants. + Regression test: previously these merged into one row and one of the two + ``models:`` entries silently disappeared from the picker.""" + rows = _user_rows(_providers(monkeypatch, { + "vllm": { + "name": "vLLM", + "base_url": "http://192.168.15.115:8000/v1", + "model": "unsloth/Qwen3.6-35B-A3B-NVFP4", + "discover_models": False, + "models": {"unsloth/Qwen3.6-35B-A3B-NVFP4": {}}, + }, + "vllm-no-think": { + "name": "vLLM No-Think", + "base_url": "http://192.168.15.115:8000/v1", + "model": "unsloth/Qwen3.6-35B-A3B-NVFP4", + "discover_models": False, + "models": {"unsloth/Qwen3.6-35B-A3B-NVFP4": {}}, + "extra_body": {"chat_template_kwargs": {"enable_thinking": False}}, + }, + })) + assert len(rows) == 2, f"expected 2 rows, got {len(rows)}: {rows}" + slugs = {row["slug"] for row in rows} + assert slugs == {"vllm", "vllm-no-think"} + + +def test_distinctly_named_entries_stay_separate_even_with_identical_config(monkeypatch): + """Two providers: entries at the same endpoint, with IDENTICAL config in + every other respect (no extra_body, no extra_headers, same credential), + but deliberately different names must still stay separate rows — name is + the primary signal, not a fallback used only when something else also + differs. Neither "vLLM" nor "vLLM No-Think" carries a version-number + pattern, so the Palantir-style suffix-stripping heuristic leaves both + unchanged and they don't collapse.""" + rows = _user_rows(_providers(monkeypatch, { + "vllm": { + "name": "vLLM", + "base_url": "http://192.168.15.115:8000/v1", + "model": "unsloth/Qwen3.6-35B-A3B-NVFP4", + "discover_models": False, + "models": {"unsloth/Qwen3.6-35B-A3B-NVFP4": {}}, + }, + "vllm-no-think": { + "name": "vLLM No-Think", + "base_url": "http://192.168.15.115:8000/v1", + "model": "unsloth/Qwen3.6-35B-A3B-NVFP4", + "discover_models": False, + "models": {"unsloth/Qwen3.6-35B-A3B-NVFP4": {}}, + }, + })) + assert len(rows) == 2, f"expected 2 rows, got {len(rows)}: {rows}" + slugs = {row["slug"] for row in rows} + assert slugs == {"vllm", "vllm-no-think"} + names = {row["name"] for row in rows} + assert names == {"vLLM", "vLLM No-Think"} + + class TestFormatModelForDisplay: def test_palantir_rid_stripped_to_trailing_slug(self): rid = "ri.language-model-service..language-model.anthropic-claude-4-7-opus" diff --git a/tests/run_agent/test_switch_model_extra_body.py b/tests/run_agent/test_switch_model_extra_body.py new file mode 100644 index 000000000000..1a8599c365e4 --- /dev/null +++ b/tests/run_agent/test_switch_model_extra_body.py @@ -0,0 +1,158 @@ +"""Regression tests: switch_model() must reconcile request_overrides['extra_body'] +on every live /model switch, for both agent-init and live-switch provider +identity conventions. + +Root-cause bugs fixed together: + +1. ``agent.provider`` carries the entry's identity DIFFERENTLY depending on + when it's resolved. At agent-init (hermes_cli/runtime_provider.py:: + _resolve_named_custom_runtime) it's always the bare canonical "custom", + with the real identity only on ``agent.requested_provider``. On a live + ``/model`` switch (hermes_cli/model_switch.py's pure switch_model(), which + every gateway/TUI/CLI switch path calls before mutating the live agent) + ``target_provider`` — and so ``agent.provider`` after the swap — is the + entry's OWN raw identity instead (e.g. "vllm"), never "custom". Matching + extra_body only recognized the first convention, so two providers: + entries sharing (base_url, model) — e.g. a vLLM endpoint listed twice as + "vllm" / "vllm-no-think", the latter pinning + extra_body.chat_template_kwargs.enable_thinking for a hybrid-thinking + model — were indistinguishable and the WRONG entry's extra_body could be + applied regardless of which was actually selected, or (for the live- + switch convention) extra_body silently stopped resolving at all. + +2. switch_model() never touched request_overrides['extra_body'] at all, so a + provider switched away from would leave its extra_body stuck on every + request for the rest of the session even after switching to one with none + configured. +""" + +from unittest.mock import MagicMock, patch + + +def _make_agent(provider="vllm", requested_provider=None, extra_body=None): + """Minimal AIAgent with just enough surface for switch_model()'s + non-anthropic (openai-client) branch, mirroring + tests/run_agent/test_switch_model_reapplies_headers.py.""" + from run_agent import AIAgent + + agent = AIAgent.__new__(AIAgent) + agent.model = "unsloth/Qwen3.6-35B-A3B-NVFP4" + agent.provider = provider + agent.requested_provider = requested_provider if requested_provider is not None else provider + agent.base_url = "http://192.168.15.115:8000/v1" + agent.api_key = "no-key-required" + agent.api_mode = "chat_completions" + agent.client = MagicMock() + agent.quiet_mode = True + agent._config_context_length = None + agent._client_kwargs = {"api_key": "no-key-required", "base_url": agent.base_url} + agent.request_overrides = dict(extra_body and {"extra_body": extra_body} or {}) + agent._custom_provider_extra_body_keys = set(extra_body or {}) + agent._credential_pool = None + agent._credential_pool_entry_id = None + agent._transport_cache = {} + agent.context_compressor = None + agent._primary_runtime = {} + agent._cached_system_prompt = None + agent._anthropic_api_key = "" + agent._anthropic_base_url = None + agent._is_anthropic_oauth = False + agent._anthropic_prompt_cache_policy = MagicMock(return_value=(False, False)) + agent._ensure_lmstudio_runtime_loaded = MagicMock(return_value=None) + agent._lmstudio_load_was_unverified = MagicMock(return_value=False) + agent._effective_lmstudio_context_length = MagicMock(return_value=None) + agent._create_openai_client = MagicMock(return_value=MagicMock()) + agent._apply_client_headers_for_base_url = MagicMock() + return agent + + +_CUSTOM_PROVIDERS = [ + { + "name": "vLLM", + "provider_key": "vllm", + "base_url": "http://192.168.15.115:8000/v1", + "model": "unsloth/Qwen3.6-35B-A3B-NVFP4", + }, + { + "name": "vLLM No-Think", + "provider_key": "vllm-no-think", + "base_url": "http://192.168.15.115:8000/v1", + "model": "unsloth/Qwen3.6-35B-A3B-NVFP4", + "extra_body": {"chat_template_kwargs": {"enable_thinking": False}}, + }, +] + + +@patch("agent.model_metadata.get_model_context_length", return_value=131_072) +@patch("hermes_cli.config.get_custom_provider_context_length", return_value=None) +@patch("hermes_cli.config.get_compatible_custom_providers", return_value=_CUSTOM_PROVIDERS) +@patch("hermes_cli.config.load_config", return_value={}) +def test_switch_to_no_think_variant_picks_up_its_extra_body( + mock_cfg, mock_cps, mock_ctx_len_cp, mock_ctx_len +): + """Real invocation convention: new_provider is the entry's OWN identity + ("vllm-no-think"), matching what hermes_cli/model_switch.py's + target_provider actually resolves to for a named providers: entry — NOT + the "custom" canonical form used at agent-init.""" + agent = _make_agent(provider="vllm") + + agent.switch_model( + "unsloth/Qwen3.6-35B-A3B-NVFP4", + "vllm-no-think", + base_url="http://192.168.15.115:8000/v1", + ) + + assert agent.request_overrides.get("extra_body") == { + "chat_template_kwargs": {"enable_thinking": False} + } + + +@patch("agent.model_metadata.get_model_context_length", return_value=131_072) +@patch("hermes_cli.config.get_custom_provider_context_length", return_value=None) +@patch("hermes_cli.config.get_compatible_custom_providers", return_value=_CUSTOM_PROVIDERS) +@patch("hermes_cli.config.load_config", return_value={}) +def test_switch_away_from_no_think_variant_clears_stale_extra_body( + mock_cfg, mock_cps, mock_ctx_len_cp, mock_ctx_len +): + agent = _make_agent( + provider="vllm-no-think", + extra_body={"chat_template_kwargs": {"enable_thinking": False}}, + ) + + agent.switch_model( + "unsloth/Qwen3.6-35B-A3B-NVFP4", + "vllm", + base_url="http://192.168.15.115:8000/v1", + ) + + assert agent.request_overrides.get("extra_body") is None + + +@patch("agent.model_metadata.get_model_context_length", return_value=131_072) +@patch("hermes_cli.config.get_custom_provider_context_length", return_value=None) +@patch("hermes_cli.config.get_compatible_custom_providers", return_value=_CUSTOM_PROVIDERS) +@patch("hermes_cli.config.load_config", return_value={}) +def test_agent_init_time_custom_convention_still_resolves_correctly( + mock_cfg, mock_cps, mock_ctx_len_cp, mock_ctx_len +): + """The OTHER convention — agent.provider == "custom" (bare canonical, + as set at agent-init by hermes_cli/runtime_provider.py) with the real + identity only on agent.requested_provider — must keep working too.""" + agent = _make_agent(provider="custom", requested_provider="vllm") + + agent.switch_model( + "unsloth/Qwen3.6-35B-A3B-NVFP4", + "custom", + base_url="http://192.168.15.115:8000/v1", + ) + agent.requested_provider = "vllm-no-think" + + agent.switch_model( + "unsloth/Qwen3.6-35B-A3B-NVFP4", + "custom", + base_url="http://192.168.15.115:8000/v1", + ) + + assert agent.request_overrides.get("extra_body") == { + "chat_template_kwargs": {"enable_thinking": False} + }