fix(model-switch): keep bare custom endpoint visible when not the active provider - #59808
fix(model-switch): keep bare custom endpoint visible when not the active provider#59808TheTom wants to merge 4 commits into
Conversation
Related: competing fix for #59702. This PR fixes the bare-custom picker row at the backend data source ( |
1cee780 to
a797f3f
Compare
Context-window override leak (NousResearch#62153, fixed here), session model switches persisting globally (NousResearch#61192), and bare custom endpoints hidden in the picker (NousResearch#59808, cherry-pick branch on this fork).
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing this to the shared backend inventory path. The reported premise is still present on current main: hermes_cli/model_switch.py:2131-2146 emits a bare custom row only when the live provider is custom, while tui_gateway/server.py:12962-12968 overlays picker context with the live session provider.
Problems
- Blocking: PR
hermes_cli/model_switch.py:2115leaves a truthy legacy scalarmodel:value as a string, then PR line 2119 calls_model_cfg.get(...). Currenthermes_cli/inventory.py:94-97explicitly supports scalarconfig.modelvalues, so opening a picker with a non-custom live provider can raiseAttributeError.
Suggested changes
- Normalize the loaded
modelvalue to{}unless it is a dict, and add a scalar-config regression test alongside the new provider-switch test.
The patch otherwise applies cleanly to current main (git apply --check). This is an automated hermes-sweeper review.
| else: | ||
| try: | ||
| from hermes_cli.config import load_config as _load_config_3b | ||
| _model_cfg = _load_config_3b().get("model") or {} |
There was a problem hiding this comment.
load_config() can return a legacy scalar model: value; current hermes_cli/inventory.py:94-97 explicitly supports that shape. Guard this value with isinstance(..., dict) before the .get() calls below, or a non-custom live provider will make picker listing raise AttributeError.
|
Pushed 2 fixups addressing hermes-sweeper's blocking finding:
|
c706ef0 to
c9a810b
Compare
…ive provider The bare model.provider: custom + model.base_url form only surfaced its picker row when it matched the current session's active provider. Once a session switched to any other model, the row vanished and never marked itself seen, so the unconfigured-canonical-provider fallback silently inserted a misleading 0-model placeholder in its place instead. Named custom_providers entries never had this problem since they are surfaced unconditionally. This adds the same static config.yaml lookup for the bare endpoint case, falls back to a live /models probe (mirroring section 4's policy) when the caller has no current_model to rely on, and computes is_current from an actual comparison instead of hardcoding it. Verified end to end against a live Desktop instance: captured the raw model.options WebSocket frame at the exact moment the row was missing and confirmed the payload itself carried models: [] for the custom slug, not a rendering issue. Also adds an isolated ModelMenuPanel test proving the frontend grouping and render logic was not at fault. Fixes NousResearch#59702
…lback Adds the backend regression the fix actually changes behavior for: the bare custom row must survive the session being on a different provider (fails on main, passes with the fix), and the config-sourced fallback must defer to a named custom_providers entry covering the same base_url. Also tightens the ModelMenuPanel test: asserts the exact humanized model display names instead of hedged either-form checks, and corrects the header comment to say what these tests pin down (the panel renders a complete payload correctly; the NousResearch#59702 bug was the payload itself).
hermes-sweeper flagged that the config-sourced bare-custom-endpoint fallback called .get() on config['model'] unconditionally. Older configs can carry a bare string there (see inventory.py's ConfigContext loader), which raised AttributeError and crashed the picker whenever the live session was on a non-custom provider.
c9a810b to
adb66ba
Compare
What does this PR do?
Fixes the bare
model.provider: custom+model.base_urlpicker row (no namedcustom_providers:/providers:entry) so it stays visible in the model picker regardless of which provider the current session happens to be on.Previously, section 3b of
list_authenticated_providersonly built this row whencurrent_provider == "custom", i.e. only while it was the session's active model. As soon as the session switched to any other provider, the row disappeared and never marked itself inseen_slugs, so the unconfigured-canonical-provider fallback ininventory.pysilently inserted a misleading 0-model "runhermes modelto configure" placeholder in its place, even though the endpoint was fully configured and reachable.Named
custom_providers:entries never had this problem, since section 4 surfaces them unconditionally.I verified this against a live Desktop instance by capturing the raw
model.optionsWebSocket frame (via CDP, Network domain) at the exact moment the picker showed the row missing, and confirmed the payload itself carriedmodels: []for thecustomslug. This rules out a rendering/clipping issue: the frontend was correctly reflecting empty data, not hiding present data.Related Issue
Fixes #59702
Changes Made
hermes_cli/model_switch.py: section 3b now falls back to reading the staticmodel:config block directly when the caller-suppliedcurrent_provider/current_base_urldo not identify a bare custom endpoint (i.e. the session is on a different provider). Also adds a live/modelsprobe in that fallback path (mirroring section 4's policy) since there is no caller-suppliedcurrent_modelto rely on there, and computesis_currentby comparison instead of hardcoding it toTrue.tests/hermes_cli/test_model_switch_custom_providers.py: two backend regressions for the fix.test_list_authenticated_providers_bare_custom_endpoint_survives_provider_switchfails on currentmainand passes with this change (verified both ways); the second pins that the config-sourced fallback defers to a namedcustom_providersentry covering the samebase_urlinstead of duplicating it.apps/desktop/src/app/shell/model-menu-panel.custom-providers.test.tsx(new): isolatedModelMenuPaneltest using the exact live payload shape from Desktop model picker shows only some named custom_providers entries even though backend data is correct #59702 (one bare custom endpoint plus two namedcustom_providersentries), exercised through both the REST fallback and the realgateway.requestpath. These pass against currentmainas well, pinning down that the panel renders a complete payload correctly and the bug was the payload itself.How to Test
model.provider: custom+model.base_urlinconfig.yamlwithout a matchingcustom_providers:/providers:entry, then switch the active session to a different provider (e.g. an OAuth provider).hermes modelto configure" placeholder.pytest tests/hermes_cli/test_model_switch_custom_providers.py -q(35 passed; the new survives-provider-switch regression fails when run againstmain'smodel_switch.py) and the broaderpytest tests/hermes_cli/ -q -k "model_switch or model_options or custom_provider"(226 passed).npm run test:ui -- src/app/shell/model-menu-panel.custom-providers.test.tsx(3 passed), plus eslint and prettier on the new test file.Checklist
fix(scope):)pytest tests/hermes_cli/test_model_switch_custom_providers.py -qand all tests pass