Skip to content

feat(model_switch): honor discover_models in custom_providers section 4 (salvage #29810) - #40080

Merged
kshitijk4poor merged 2 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/discover-models-section4-29810
Jun 5, 2026
Merged

feat(model_switch): honor discover_models in custom_providers section 4 (salvage #29810)#40080
kshitijk4poor merged 2 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/discover-models-section4-29810

Conversation

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Summary

Salvages the still-relevant part of #29810 (which is 1,568 commits behind main and no longer applies). That PR bundled two changes:

  1. key_env resolution in section 4 — already landed independently in aa283d1e4 (custom-provider picker credential isolation), which replaced the old 2-tuple group key with (api_url, credential_identity, api_mode) and added the key_env fallback. Not carried here (redundant).
  2. discover_models in section 4 — still missing on main. This PR.

What this does

Section 3 (user providers:) already honors discover_models: false to skip live /models discovery and keep the explicit models: list. Section 4 (custom_providers: list) did not — should_probe ignored the field, so any grouped custom provider with an api_key always had its configured subset replaced by the full live /models catalog.

This adds the same discover_models support to section 4:

  • Default True — no behaviour change for existing configs.
  • discover_models: false keeps the explicit models: list even when an api_key is present.
  • String values ("false"/"no"/"0") are normalised to False, matching section 3.
  • If any entry in a grouped endpoint opts out, the whole grouped row opts out.

Use case

Endpoints that expose a full aggregator catalog via /models but only serve a configured subset:

custom_providers:
  - name: my-endpoint
    base_url: https://internal.example.com/v1
    api_key_env: MY_API_KEY
    discover_models: false
    models:
      DeepSeek-V4-Flash:
        context_length: 200000
        name: DeepSeek-V4-Flash
    model: DeepSeek-V4-Flash

Tests

Added two regression tests in tests/hermes_cli/test_model_switch_custom_providers.py:

  • discover_models: false + api_key preserves the explicit subset and never calls fetch_api_models.
  • String "false" is normalised to disable discovery.

Both pass. Three pre-existing failures in that file (test_..._groups_same_endpoint, test_..._distinct_endpoints_stay_separate, test_..._total_models_reflects_grouped_count) fail identically on bare origin/main — they probe a live local Ollama server because they don't mock fetch_api_models; unrelated to this change.

Credit to @ohMyJason (original author of #29810).

Closes #29810

ohMyJason and others added 2 commits June 6, 2026 01:04
Section 3 (user `providers:`) already honors `discover_models: false` to
skip live /models discovery and keep the explicit `models:` list. Section 4
(`custom_providers:` list) did not — `should_probe` ignored the field, so any
grouped custom provider with an api_key always had its configured subset
replaced by the full live /models catalog.

This adds the same `discover_models` support to section 4:
- Default True — no behaviour change for existing configs.
- `discover_models: false` keeps the explicit `models:` list even when an
  api_key is present.
- String values ("false"/"no"/"0") are normalised to False, matching
  section 3.
- If any entry in a grouped endpoint opts out, the whole group opts out.

Use case: endpoints that expose a full aggregator catalog via /models but
only serve a configured subset.

Salvaged from NousResearch#29810 — rebased onto current main. The PR's other change
(`key_env` resolution in section 4) landed independently in commit aa283d1
(custom provider picker credential isolation), so only the discover_models
portion is carried here.

Co-authored-by: ohMyJason <42903577+ohMyJason@users.noreply.github.com>
@alt-glitch alt-glitch added type/feature New feature or request P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard labels Jun 5, 2026
@kshitijk4poor
kshitijk4poor merged commit b5d42da into NousResearch:main Jun 5, 2026
22 of 23 checks passed
alt-glitch pushed a commit that referenced this pull request Jun 14, 2026
…section4-29810

feat(model_switch): honor discover_models in custom_providers section 4 (salvage #29810)
T02200059 pushed a commit to T02200059/hermes-agent that referenced this pull request Jun 18, 2026
…over-models-section4-29810

feat(model_switch): honor discover_models in custom_providers section 4 (salvage NousResearch#29810)
waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
…over-models-section4-29810

feat(model_switch): honor discover_models in custom_providers section 4 (salvage NousResearch#29810)
santhreal pushed a commit to santhreal/hermes-agent that referenced this pull request Jul 13, 2026
…over-models-section4-29810

feat(model_switch): honor discover_models in custom_providers section 4 (salvage NousResearch#29810)
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
…over-models-section4-29810

feat(model_switch): honor discover_models in custom_providers section 4 (salvage NousResearch#29810)
@kshitijk4poor
kshitijk4poor deleted the salvage/discover-models-section4-29810 branch August 5, 2026 07:08
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
…over-models-section4-29810

feat(model_switch): honor discover_models in custom_providers section 4 (salvage NousResearch#29810)
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 P2 Medium — degraded but workaround exists type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants