Skip to content

fix(model-switch): resolve provider-module plugin providers in switch path - #34368

Closed
Kyzcreig wants to merge 1 commit into
NousResearch:mainfrom
ANG-Ventures:fix/model-switch-plugin-provider-registry
Closed

Kyzcreig wants to merge 1 commit into
NousResearch:mainfrom
ANG-Ventures:fix/model-switch-plugin-provider-registry

Conversation

@Kyzcreig

Copy link
Copy Markdown
Contributor

Summary

resolve_provider_full() — the resolver behind /model <name>, --provider, and the interactive model picker — consults the models.dev catalog, Hermes overlays, and config.yaml providers: / custom_providers:. It never consults the provider-module plugin registry (plugins/model-providers/<name>/), even though hermes_cli.auth.PROVIDER_REGISTRY auto-extends from that same source at import time.

The two paths therefore use two different provider registries:

  • Startup / default-model resolutionruntime_providerauth.PROVIDER_REGISTRYknows plugin providers.
  • /model switch resolutionresolve_provider_fullget_provider() + config only → does not know plugin providers.

Symptom

A provider declared only as a plugin profile (e.g. a local Anthropic-compatible proxy registered via providers/) works fine as the configured default model — startup resolves it correctly — but switching into it via /model (or selecting it in the Discord/Telegram picker) fails with:

Unknown provider '<name>'. Check 'hermes model' for available providers,
or define it in config.yaml under 'providers:'.

Reproduction (pre-fix)

With a plugin provider under plugins/model-providers/<name>/ declaring api_mode="anthropic_messages":

from hermes_cli.model_switch import switch_model
r = switch_model(
    raw_input="my-proxy/some-model",
    current_provider="openrouter", current_model="x",
    current_base_url="", current_api_key="",
    is_global=False, explicit_provider="my-proxy",
)
assert r.success  # -> False: "Unknown provider 'my-proxy'"

Fix

Add resolution step 1b in resolve_provider_full that consults the providers/ plugin registry via get_provider_profile(), translating the ProviderProfile into a ProviderDef. api_mode maps back to transport via the inverse of TRANSPORT_TO_API_MODE (default openai_chat). Env-var splitting mirrors the same _BASE_URL/_URL heuristic auth.py already uses when it auto-extends the registry.

This reunifies the switch path with the startup path: a provider that works as a default now also works as a /model target. Built-in/models.dev/config resolution order is unchanged — the plugin layer slots in after built-ins and before config providers, matching the precedence auth.py uses.

Scope

  • IN — make the switch resolver see provider-module plugin providers.
  • OUT — collapsing the two provider registries into one. That's a larger refactor with its own tradeoffs; this PR keeps both registries but makes the switch path layer the plugin registry the same way auth.py does.

Verification

A standalone assertion script confirms:

  • resolve_provider_full("<plugin-provider>") returns a ProviderDef with correct transport (mapped from api_mode), base_url, and key env vars.
  • switch_model(... explicit_provider="<plugin-provider>") succeeds end-to-end with the right api_mode/base_url.
  • A genuinely unknown provider still resolves to None / fails (no over-broad matching).

Targeted suites pass (test_user_providers_model_switch, test_model_switch_opencode_anthropic, test_api_key_providers, test_runtime_provider_resolution, etc.). Three pre-existing failures in test_model_switch_custom_providers.py are caused by a model-catalog 403 in the sandbox and fail identically on origin/main with this change reverted — unrelated to this PR.

… path

resolve_provider_full() (the /model switch + --provider resolver) consulted
only the models.dev catalog, Hermes overlays, and config.yaml providers/
custom_providers. It never consulted the provider-module plugin registry
(plugins/model-providers/<name>/), even though hermes_cli.auth.PROVIDER_REGISTRY
auto-extends from that same source at import time.

Result: a provider declared only as a plugin profile (e.g. a local Anthropic
proxy registered via providers/) resolves fine during runtime startup — so it
works as the configured default model — but switching INTO it via /model fails
with "Unknown provider '<name>'. Check 'hermes model' ...". Two code paths,
two different provider registries.

Reproduction (pre-fix), with a plugin provider declaring api_mode=anthropic_messages:

    from hermes_cli.model_switch import switch_model
    r = switch_model(raw_input="my-proxy/some-model",
                     current_provider="openrouter", current_model="x",
                     current_base_url="", current_api_key="",
                     is_global=False, explicit_provider="my-proxy")
    assert r.success  # -> False, "Unknown provider 'my-proxy'"

Fix: add a resolution step (1b) in resolve_provider_full that consults the
providers/ plugin registry via get_provider_profile(), translating the
ProviderProfile into a ProviderDef. api_mode maps back to transport via the
inverse of TRANSPORT_TO_API_MODE (default openai_chat). This reunifies the
switch path with the startup path, so a provider that works as a default also
works as a /model target.

Scope: IN — make the switch resolver see plugin providers. OUT — refactoring
the two registries into one (larger change, own tradeoffs); this PR keeps both
but makes the switch path layer the plugin registry the same way auth.py does.

Verified: resolve_provider_full + switch_model now succeed for a plugin
provider with correct transport/base_url/api_mode; unknown providers still
return None. Pre-existing 3 failures in test_model_switch_custom_providers.py
(model-catalog 403 in sandbox) are unrelated and fail identically on origin/main.
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard comp/plugins Plugin system and bundled plugins labels May 29, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracing the split between the profile registry and the switch resolver. The missing lookup is present on current main: resolve_provider_full() falls through at hermes_cli/providers.py:742-779, and /model emits the unknown-provider error at hermes_cli/model_switch.py:862-889.

Problems

  • hermes_cli/providers.py:701 resolves a plugin before providers: / custom_providers:. That can shadow a user-configured endpoint with the same name, despite the file contract that config overrides are merged on top (hermes_cli/providers.py:14-15).
  • The api_modetransport conversion at hermes_cli/providers.py:770-771 does not reach the runtime switch result. switch_model() later calls resolve_runtime_provider() (hermes_cli/model_switch.py:1213-1220), whose generic registry path defaults to chat_completions unless config or URL heuristics override it (hermes_cli/runtime_provider.py:1466-1501).
  • The PR changes only hermes_cli/providers.py; please add a plugin-profile /model regression test, including an anthropic_messages profile.

Suggested changes

  • Preserve user-config precedence, then use profile lookup as fallback.
  • Resolve ProviderProfile.api_mode in the runtime path or explicitly carry the resolved transport through switching.

Automated hermes-sweeper review.

Comment thread hermes_cli/providers.py
# rejected with "Unknown provider" when the user tries to /model into
# it. resolve_provider_full and runtime startup resolution were
# consulting two different registries; this reunifies them.
plugin_pdef = _resolve_plugin_provider(canonical, name)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fallback is before the existing providers: and custom_providers: paths. A plugin named the same as a configured endpoint will now shadow that user override; keep config resolution ahead of the plugin fallback.

Comment thread hermes_cli/providers.py
return None

api_mode = getattr(profile, "api_mode", "") or "chat_completions"
transport = _API_MODE_TO_TRANSPORT.get(api_mode, "openai_chat")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mapped transport is not consumed by switch_model after it sets target_provider: runtime resolution independently defaults API-key registry providers to chat_completions. Please propagate ProviderProfile.api_mode into the runtime switch result and cover an anthropic_messages plugin.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
@WolframRavenwolf

Copy link
Copy Markdown
Contributor

Thank you for identifying the missing ProviderProfile lookup in the model-switch path. I consolidated this overlap into #52549, kept plugin discovery out of the built-in get_provider() hot path, added end-to-end regressions, and preserved your contribution through a Co-authored-by: Kyzcreig <kyzcreig@users.noreply.github.com> trailer. The coordinated branch also includes the complementary all-route api_mode work from #53055 with its original author intact.

@Kyzcreig

Copy link
Copy Markdown
Contributor Author

@WolframRavenwolf — thank you, and confirming your consolidation from our side after actually checking it rather than taking it on trust.

I consolidated this overlap into #52549 … and preserved your contribution through a Co-authored-by: Kyzcreig trailer.

Verified: 58f4030fe in #52549 carries authors=WolframRavenwolf,Kyzcreig. Credit is intact — thank you for the care.

More importantly, I checked whether #52549 actually resolves the two defects @teknium1's sweeper raised against this PR, since consolidation only helps if the consolidated version is the better one. It does, on both counts:

  1. Precedence. The sweeper's objection here was that hermes_cli/providers.py:701 resolved a plugin before providers:/custom_providers:, shadowing a user-configured endpoint of the same name and contradicting the file's own merge contract. In fix(providers): resolve ProviderProfile identity consistently #52549 profile lookup is step 2c — after built-ins, after providers:, after custom_providers: — and it gives an explicit user provider with the resolved profile ID one final chance to win. That is precisely the "preserve user-config precedence, then use profile lookup as fallback" ordering the review asked for, and it's the ordering my PR got wrong.

  2. Transport reaching the runtime switch. The sweeper's second objection was that the api_modetransport conversion never reached resolve_runtime_provider(), so switching defaulted to chat_completions. My PR touched only hermes_cli/providers.py, which is exactly why it couldn't fix this. fix(providers): resolve ProviderProfile identity consistently #52549 changes hermes_cli/runtime_provider.py (+73/-20) and hermes_cli/model_switch.py as well, and adds tests/hermes_cli/test_plugin_provider_api_mode.py — including the anthropic_messages profile regression the review asked me for and I never added.

So #52549 is strictly the better implementation and this PR should not land in preference to it.

One correction to the record, in your favour: an earlier internal triage pass of ours logged this as "already consolidated, nothing to do but close." That was wrong — #52549 is still OPEN, not merged (checked just now). Closing this PR on that basis would have retired the only other open implementation of the fix while the survivor was still unlanded. I'm leaving this open until #52549 merges; happy for a maintainer to close it the moment it does, and I'd rather it close as superseded-by-#52549 than get merged.

Nothing needed from you — flagging the open-vs-merged distinction so nobody else on our side acts on the same stale assumption.

WolframRavenwolf added a commit to WolframRavenwolf/hermes-agent that referenced this pull request Aug 11, 2026
Teach the full CLI provider resolution path to recognize ProviderProfile plugins and aliases without making the built-in get_provider() hot path perform plugin discovery.

This consolidates the overlap with NousResearch#34368 while preserving explicit user-config precedence, bare custom endpoint semantics, and provider metadata in /model results.

Co-authored-by: Kyzcreig <kyzcreig@users.noreply.github.com>
@Kyzcreig

Copy link
Copy Markdown
Contributor Author

Closing in favor of #52549 per the consolidation above — verified it carries the Co-authored-by trailer (58f4030fe) and resolves both sweeper-raised defects that this PR did not (my earlier comment has the point-by-point). Keeping our open-PR footprint reviewable; nothing here is lost.

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

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants