Skip to content

fix(model-switch): distinct-named custom providers stay reachable in the picker and keep their own extra_body - #86180

Open
sfrmattos wants to merge 2 commits into
NousResearch:mainfrom
sfrmattos:fix/model-switch-custom-provider-extra-body-v2
Open

fix(model-switch): distinct-named custom providers stay reachable in the picker and keep their own extra_body#86180
sfrmattos wants to merge 2 commits into
NousResearch:mainfrom
sfrmattos:fix/model-switch-custom-provider-extra-body-v2

Conversation

@sfrmattos

Copy link
Copy Markdown

Closes #86179

Summary

Two related bugs, found while tracking down why a second providers: entry pointing at the same endpoint (used to toggle vLLM's chat_template_kwargs.enable_thinking for a hybrid-thinking model) was practically unusable:

1. Picker: distinctly-named providers: entries silently collapsed into one row

list_authenticated_providers() groups same-endpoint providers: entries into one picker row so a provider with several models doesn't produce N near-duplicate rows (the "Palantir Claude 4.6/4.7 Opus" → "Palantir Claude" case, from #36998). Section 3 (providers:) grouped purely on connection identity (api_url, credential, api_mode, headers) — never on name — while section 4 (custom_providers:) already grouped on a version-stripped name prefix too. That asymmetry meant two providers: entries at the same endpoint with deliberately different names (e.g. "vLLM" / "vLLM No-Think") collapsed into a single row, and the second entry's extra_body became unreachable from the picker even though it resolved correctly via --provider <name> or a direct config edit.

Fix: promote the existing (cosmetic-only) version-suffix-stripping heuristic into section 3's grouping key, mirroring section 4. Two entries whose names share no separator/version pattern now stay distinct rows; entries whose names differ only by an embedded version number (the Palantir case) still fold into one row with multiple models. extra_body/extra_headers remain additional identity signals for the rarer case of identical names with different configs.

2. Runtime: extra_body could apply to the wrong provider, or stop applying after a live switch

Independent of the picker, _merge_custom_provider_extra_body() (agent/agent_init.py) matched a named custom provider's extra_body using agent.provider, which is always the bare canonical "custom" for every providers:/custom_providers: entry — it carries no per-entry identity. With two entries sharing (base_url, model), the matcher fell through to "first entry with a non-empty extra_body" and could silently apply the WRONG entry's extra_body regardless of which one was actually selected.

While fixing that, found a second layer: a live /model switch resolves the new provider identity differently than agent-init does — hermes_cli/model_switch.py's pure switch_model() sets target_provider to the entry's own raw name (e.g. "vllm"), not "custom". The extra_body matcher only recognized the "custom" / "custom:<name>" shapes, so after ANY live switch to a named custom provider, extra_body resolution silently stopped applying at all. On top of that, switch_model() (agent/agent_runtime_helpers.py) never touched request_overrides['extra_body'], 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 a provider with none configured.

Fix:

  • _custom_provider_extra_body_for_agent() now accepts requested_provider and uses it (not agent.provider) to disambiguate entries at agent-init.
  • It also recognizes the live-switch convention: when neither provider nor requested_provider looks like "custom"/"custom:<name>", fall back to treating a bare name as the filter only when it matches a configured custom_providers identity (provider_key or display name) — never for a builtin provider, since those never appear in custom_providers.
  • _merge_custom_provider_extra_body() is now idempotent/repeatable: it tracks which keys it previously contributed (agent._custom_provider_extra_body_keys) and strips them before re-merging, so switching to a provider with no extra_body correctly clears the stale value while any unrelated caller-set override (e.g. an explicit fast-mode service_tier) survives.
  • switch_model() now calls this reconciliation on every live switch, using the freshly-loaded custom_providers list it already fetches for context-length resolution.

Repro

providers:
  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

Before this fix: the picker showed a single "vLLM" row with one model; vllm-no-think was unreachable from /model. Separately, even selecting vllm-no-think directly (or switching to it live), its extra_body could be silently dropped or applied to the wrong provider, and switching back to vllm mid-session left the enable_thinking: false override stuck on every subsequent request.

Test plan

  • tests/hermes_cli/test_provider_section3_grouping.py — picker grouping: distinct names stay separate rows (with and without differing extra_body), Palantir-style version-suffix names still collapse into one row.
  • tests/hermes_cli/test_model_switch_custom_providers.py — same invariants for the custom_providers: (section 4) grouping.
  • tests/agent/test_custom_provider_extra_body.py_merge_custom_provider_extra_body disambiguates by requested_provider (agent-init convention) and by bare raw name (live-switch convention), never leaks onto a builtin provider, clears stale keys on re-merge, preserves unrelated caller overrides.
  • tests/agent/test_custom_provider_extra_body_matching.py — existing matching tests still pass unchanged.
  • tests/run_agent/test_switch_model_extra_body.py (new) — end-to-end through the real switch_model(): switching to the no-think variant picks up its extra_body; switching away clears it; the agent-init "custom" convention still resolves correctly too.
  • Manually verified against the repro config above, on a real vLLM endpoint: picker shows both providers as separate rows; a fresh restart on either vllm or vllm-no-think sends the correct (or absent) chat_template_kwargs.enable_thinking; live /model switching back and forth in the same running session correctly toggles it on the outgoing request (checked via proxy).

Blackbeard added 2 commits August 14, 2026 12:58
…entity

providers:/custom_providers: entries sharing (api_url, credential, api_mode,
headers) but differing only by extra_body (e.g. a vLLM endpoint listed twice
with chat_template_kwargs.enable_thinking toggled) were silently collapsed
into a single picker row, and one of the two configured models disappeared.
Fold extra_body into the group identity, mirroring the existing
extra_headers handling in both grouping sections.
…the picker and keep their own extra_body

Two related bugs found while tracking down why a second providers: entry
pointing at the same endpoint (used to toggle vLLM's
chat_template_kwargs.enable_thinking for a hybrid-thinking model) was
practically unusable:

1. Picker (hermes_cli/model_switch.py): section 3 (providers:) grouped
   same-endpoint entries purely on connection identity, never on name,
   unlike section 4 (custom_providers:) which already grouped on a
   version-stripped name prefix. Two providers: entries with deliberately
   different names collapsed into one picker row, making the second
   entry's extra_body unreachable from /model. Promoted the existing
   (cosmetic-only) version-suffix-stripping heuristic into section 3's
   grouping key, mirroring section 4, while still folding
   Palantir-style "same provider, more models" names into one row.

2. Runtime (agent/agent_init.py, agent/agent_runtime_helpers.py):
   _merge_custom_provider_extra_body matched on agent.provider, which is
   always the bare canonical "custom" for every named custom provider —
   carrying no per-entry identity — so it could apply the WRONG entry's
   extra_body regardless of which was selected. A live /model switch
   also resolves the provider identity differently (the entry's own raw
   name, e.g. "vllm", not "custom"), which the matcher didn't recognize
   at all, and switch_model() never reconciled
   request_overrides['extra_body'], leaving a stale value from a
   previous provider stuck for the rest of the session.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard area/config Config system, migrations, profiles labels Aug 14, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(model-switch): distinct-named custom providers stay reachable in the picker and keep their own extra_body

The two-convention identity handling and the idempotent reconcile-on-switch are well covered by tests. Observations:

  1. agent/agent_init.py::_merge_custom_provider_extra_body: the idempotency bookkeeping (_custom_provider_extra_body_keys) unconditionally pops keys the previous call contributed on the next reconcile. If a caller sets one of those same keys between switches (a legitimate override), the next reconcile silently removes it. Tracking the last-written value per key (only pop when the current value still equals what we set) would let caller overrides survive.

  2. hermes_cli/model_switch.py::_group_display_prefix: the cut_at >= 2 guard means a version token at index 0 or 1 ("GPT-5", "Model 2 Pro", "Llama 4") keeps the whole name and versioned entries at the same endpoint no longer collapse into one row, while "Palantir Claude 4.7 Opus" (version at index ≥ 2) does. That asymmetry is deliberate per the docstring, but it is a behavior change for short-name versioned providers — worth confirming the "same provider, several models" case with short names is intended to stay split.

  3. agent/agent_runtime_helpers.py::switch_model: a reconcile failure is swallowed at logger.debug. Since this runs on every live /model switch, a regression here leaves stale/missing extra_body on all subsequent requests with no user-visible signal. A logger.warning would surface it.

No blocking issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(model-switch): same-endpoint providers: entries collapse in picker, extra_body can apply to wrong provider or get stuck

3 participants