fix(config): default model empty string — no more unavailable OpenAI model for non-OpenAI users — closes #646 - #649
Conversation
Review: ✅ Approved — ready to mergeVerdict: LGTM Single-line fix for a clear root-cause problem: The fix: # Before
DEFAULT_MODEL = os.getenv("HERMES_WEBUI_DEFAULT_MODEL", "openai/gpt-5.4-mini")
# After
DEFAULT_MODEL = os.getenv("HERMES_WEBUI_DEFAULT_MODEL", "")Empty string default means the WebUI defers to the active provider's model — always valid. The Test count: 1362 passing, 4 pre-existing failures in The fix is minimal, correct, and closes #646. Ready to merge. |
Independent End-to-End Review — PR #649Independent second-pass review after the first approval. Pulled the branch, verified downstream empty-model handling, ran the full test suite. TL;DRMerge-ready. Minimal one-line fix for a real UX annoyance. Empty-string default is handled correctly by Security audit ✅N/A — one-character data change (the default value for an env var fallback). No new code paths. Downstream empty-model analysis ✅Traced
The key insight: Edge case: no provider configured AND empty defaultIf a user somehow reaches a state with no config.yaml provider, no env vars, and no saved settings, Test results ✅
Coverage gap worth noting: There's no test that exercises the "no env var set, no config" path — all tests run with the env var forced. Not a regression from this PR, but a test gap that predates it. Worth a future PR to add a conftest fixture that unsets the env var for a specific test to verify empty-default behavior end-to-end. Markdown + version
RecommendationApproved, ready to merge. No changes needed from me. The prior reviewer's approval stands. One future improvement (not for this PR):
|
Review follow-up: latent bug fixed + tests addedThe core Bug found during review: empty model creates blank dropdown entries Two code paths in
The What was pushed in the fix commit (fdf5fe6):
Test results: 3/3 new tests pass. Full suite: 4 failed (pre-existing test_sprint34.py OAuth stubs), 1372 passed. The fix is pushed to |
…odel for non-OpenAI users — closes #646
fdf5fe6 to
7e06e97
Compare
… for non-OpenAI users — closes nesquena#646 (PR nesquena#649) DEFAULT_MODEL now defaults to "" instead of "openai/gpt-5.4-mini". Guards added in model-list builder so empty default does not create blank model entries. Adds 3 tests in test_issue646.py. Independent review by @nesquena.
… for non-OpenAI users — closes nesquena#646 (PR nesquena#649) DEFAULT_MODEL now defaults to "" instead of "openai/gpt-5.4-mini". Guards added in model-list builder so empty default does not create blank model entries. Adds 3 tests in test_issue646.py. Independent review by @nesquena.
Problem
The hardcoded fallback
DEFAULT_MODEL = "openai/gpt-5.4-mini"caused the model dropdown to show "GPT-5.4 Mini (unavailable)" for any user whose configured provider isn't OpenAI — including custom providers, LM Studio, Anthropic-only setups, and third-party deployments like Agent37.The
(unavailable)label is intentional and correct behavior (it preserves stale session models). The problem is the server's own default triggering it on the very first session before the user has done anything wrong.Fix
Changed the default from
"openai/gpt-5.4-mini"to""(empty string).An empty default means the WebUI defers to the active provider's own default model, which is always valid. Users who want a pinned default can still set
HERMES_WEBUI_DEFAULT_MODELenv var or choose a model in Preferences — both paths are unaffected.Testing
Closes #646