fix: do not build phantom "Custom" group when active provider is set

[mbac]

Summary

When model.provider is a real provider (e.g. openai-codex) and model.base_url is configured, hermes_cli.models.list_available_providers() reports 'custom' as an authenticated provider because the base_url makes the "custom endpoint" path available. The WebUI model picker was then building a separate "Custom" group for it and parking the configured default_model there, while the configured provider's group (e.g. "OpenAI Codex") listed only its hardcoded _PROVIDER_MODELS entries. The TUI does not have this problem because it treats model.provider + model.default as a single source of truth.

Repro (real config)

model:
  default: gpt-5.4
  provider: openai-codex
  base_url: https://chatgpt.com/backend-api/codex

Before this patch, /api/models returns:

Custom:        gpt-5.4
OpenAI Codex:  codex-mini-latest
OpenRouter:    … fallback list …

After:

OpenAI Codex:  gpt-5.4, codex-mini-latest
OpenRouter:    … fallback list …

What the patch does

Two small edits in api/config.py inside get_available_models():

1. Suppress the phantom custom detected provider when active_provider is set and isn't custom itself — the base_url belongs to the active provider, not a separate bucket.
2. Replace the substring-based default-model injection check with an exact match against _PROVIDER_DISPLAY. The previous check active_provider.lower() in g.get('provider', '').lower() silently returned False for hyphenated provider IDs vs. space-separated display names ('openai-codex' in 'openai codex' is False), falling through to groups[0] and landing the model in the alphabetical first group.

Two regression tests added in tests/test_model_resolver.py. The second one mocks three authenticated providers (anthropic, openai-codex, custom) specifically so that anthropic sorts before openai-codex — this catches the fallback-to-groups[0] bug that a two-provider mock would miss.

⚠️ Important: authorship and review

This patch was written by Claude Opus 4.6 (Anthropic) during an investigation session. I (the GitHub user opening this PR) am not a confident-enough Python developer to vouch for the correctness of the change myself. The AI agent traced the bug, reproduced it via get_available_models() directly, and proposed this fix — but I can't independently verify edge cases.

I'd appreciate if maintainers could review the code carefully before merging, especially:

• Whether discarding 'custom' from detected_providers could affect users whose config legitimately wants a standalone Custom group (e.g. model.provider: custom with a local Ollama endpoint — the active_provider != 'custom' guard is meant to preserve this, but please double-check).
• Whether the _PROVIDER_DISPLAY-based injection match covers every provider ID in _PROVIDER_DISPLAY without breaking the existing test_no_duplicate_when_default_model_is_prefixed and similar tests.
• Whether the regression tests' monkeypatch.setitem(sys.modules, …) approach is idiomatic for this codebase, or if you'd prefer a different stubbing style.

Happy to iterate on the approach — just let me know what changes you'd like.

Test plan

• pytest tests/test_model_resolver.py -v passes (16/16, including the two new tests)
• pytest tests/ -q passes (508 passed, 41 skipped — the skipped tests all require hermes-agent which isn't in the test env)
• Manual verification against a live WebUI on the reporter's server: gpt-5.4 now appears under "OpenAI Codex" in the composer model dropdown, with no phantom "Custom" group.

🤖 Patch generated with Claude Code (Opus 4.6)

[nesquena-hermes]

Thanks for the detailed writeup — the bug analysis is accurate and the fix is correct.

What the bug is

Two independent problems both lurk in get_available_models():

1. Phantom "Custom" group

list_available_providers() reports 'custom' as authenticated whenever model.base_url is set, because the base_url makes the custom-endpoint path available. When a user has e.g. provider: openai-codex + base_url: https://..., detected_providers ends up containing both 'openai-codex' and 'custom', so the builder produces two groups. The custom group grabs the configured default model (which was never in _PROVIDER_MODELS['openai-codex']), and the real provider group only shows its hardcoded entries.

2. Hyphen/space mismatch in the "ensure default_model appears" pass

The original injection check was:

if active_provider and active_provider.lower() in g.get('provider', '').lower()

For active_provider = 'openai-codex', this tests 'openai-codex' in 'openai codex' — a hyphen vs. space mismatch — which is False. The loop falls through, injected stays False, and the model lands in groups[0] (whichever provider sorts alphabetically first).

Fix assessment

Fix 1 — discard phantom 'custom':

if active_provider and active_provider != 'custom':
    detected_providers.discard('custom')

This is the right guard. The active_provider != 'custom' condition correctly preserves the standalone "Custom" group for users who have explicitly set provider: custom (e.g. an Ollama endpoint with no named provider). For any other real provider, the base_url belongs to that provider and the phantom bucket is correctly suppressed.

One edge case worth noting but not a blocker: a user who sets provider: openai-codex and has separate custom_providers config entries will lose the Custom group. In practice this is an unusual mix, and the custom_providers models will still appear under the real provider group — but it's worth a note in the PR description for future reference.

Fix 2 — exact display-name match:

target_display = _PROVIDER_DISPLAY.get(active_provider, active_provider or '').lower()
...
if target_display and g.get('provider', '').lower() == target_display:

Using _PROVIDER_DISPLAY gives the exact string that the group was built with ('OpenAI Codex' → 'openai codex'), so the equality check is deterministic for every provider in the dict. The fallback active_provider or '' handles any future provider not yet in _PROVIDER_DISPLAY. ✅

Tests:

• test_no_phantom_custom_group_when_active_provider_is_set — directly exercises the bug with the exact repro config. ✅
• test_default_model_lands_under_active_provider_group — uses three detected providers (anthropic sorts before openai-codex) to catch the fallback-to-groups[0] path. This is a particularly good regression guard for the second fix. ✅

No blockers

The fix is minimal, well-targeted, and both test cases are solid regression guards. The active_provider != 'custom' guard preserves the existing standalone-Custom-provider use case correctly.

[nesquena]

Full Review: PR #191 — phantom Custom provider group fix

Thanks @mbac — excellent bug analysis and honest self-assessment. The AI-written code is actually quite solid.

Security Audit

Clean. Changes are limited to model dropdown group construction logic. No external resources, no injection vectors, no XSS.

Code Review

Fix 1: Suppress phantom custom provider — Correct. When active_provider is set (e.g. openai-codex) and isn't custom itself, detected_providers.discard('custom') removes the phantom bucket. The active_provider != 'custom' guard correctly preserves standalone custom providers (Ollama, LM Studio).

Fix 2: Display name matching — Correct. The old substring check active_provider.lower() in g.get('provider', '').lower() broke for openai-codex vs OpenAI Codex (hyphen vs space). The new code uses _PROVIDER_DISPLAY.get(active_provider) for exact display-name matching. This is the right fix — uses the existing mapping table rather than inventing new string manipulation.

To answer your questions:

• The active_provider != 'custom' guard looks correct — users with model.provider: custom will still get their Custom group.
• The _PROVIDER_DISPLAY-based match should cover all providers since _PROVIDER_DISPLAY is the authoritative mapping. If a provider isn't in the dict, active_provider itself is used as fallback, which is reasonable.
• The monkeypatch.setitem(sys.modules, ...) approach is fine for this codebase — it's a clean way to mock hermes_cli without it being installed.

Tests

508 passed, 0 failed, 41 skipped. No regressions. Two well-crafted regression tests — the second one cleverly adds anthropic to catch the groups[0] alphabetical fallback bug.

Merge Order Note

This PR and #189 both touch api/config.py and test_model_resolver.py but different functions. Recommend merging this one first since it's earlier in the pipeline (model group construction), then #189 (model routing) with a trivial rebase.

Verdict

Approved. Solid fix, well-tested, honest contribution. Ready to merge.

[nesquena-hermes]

Rebased onto current master (post v0.41.0, #189 merged). Both fixes are correct and the tests pass.

One additional fix in the rebase: _available_models_with_full_cfg test helper now clears HERMES_MODEL/OPENAI_MODEL/LLM_MODEL env vars during the call so real hermes profile env doesn't override the test fixture's default_model. Without this, test_default_model_lands_under_active_provider_group was failing in environments where those vars are set. The production code is unchanged — this is purely a test isolation improvement.

564 tests passing. Ready to merge.