Closes #1538, #1539. Two related dropdown-staleness bugs reported by Deor (Discord, May 03 2026). #1538 — Nous Portal picker showed only 4 hardcoded models ========================================================= The Settings → Default Model picker, the composer model dropdown, the /model slash command, and the Settings → Providers card all showed only four Nous models (Claude Opus 4.6, Claude Sonnet 4.6, GPT-5.4 Mini, Gemini 3.1 Pro Preview) because `_PROVIDER_MODELS["nous"]` had four hardcoded entries and `_build_available_models_uncached()` fell through to the generic `pid in _PROVIDER_MODELS` branch. The actual Nous Portal catalog has 30 models live — Claude Opus 4.7, GPT-5.5, Kimi K2.6, MiniMax M2.7, Gemini 3.1 Pro/Flash, several Xiaomi/Tencent/StepFun entries, and more. Fix: - New `_format_nous_label()` helper in `api/config.py` — reuses the `_format_ollama_label()` token rules, drops the vendor namespace, and appends ` (via Nous)` so labels disambiguate from same-named direct- provider entries (e.g. "Claude Opus 4.7" via direct Anthropic). - New `elif pid == "nous":` branch in `_build_available_models_uncached()` mirroring the Ollama Cloud pattern: live-fetch through `hermes_cli.models.provider_model_ids("nous")`, prefix every id with `@nous:` (matches the existing routing convention from PR-era #854 and pinned in tests/test_nous_portal_routing.py), fall back to the curated 4-entry static list when hermes_cli is unavailable. - Same fix applied to `api/providers.py:get_providers()` — that's the separate code path that builds Settings → Providers card models, and it had the identical bug shape. #1539 — Removed provider lingered in dropdowns until restart ============================================================ After Settings → Providers → Remove, the provider's models still appeared in every model dropdown until the page was reloaded. The server-side TTL cache was correctly flushed (`set_provider_key()` calls `invalidate_models_cache()` on both add and remove) but JS-side caches were never dropped: - `_slashModelCache` / `_slashModelCachePromise` (commands.js) — feeds the `/model` slash-command suggestions. - `_dynamicModelLabels` / `window._configuredModelBadges` (ui.js) — populated by `populateModelDropdown()` on app boot and profile switch. Pre-fix, `_removeProviderKey()` only called `loadProvidersPanel()` which refreshed the providers card list but never asked any consumer to re-fetch /api/models. Fix: - `static/commands.js`: new `_invalidateSlashModelCache()` helper that nulls both cache slots, exposed on `window` (typeof-guarded so the module remains importable in headless vm contexts — needed by the existing tests/test_cli_only_slash_commands.py harness). - `static/panels.js`: new `_refreshModelDropdownsAfterProviderChange()` helper that calls the invalidator + `populateModelDropdown()`, wrapped in try/catch so the providers panel update never breaks if a downstream module hasn't loaded yet. Both `_saveProviderKey` and `_removeProviderKey` invoke it (defense-in-depth: same staleness shape applies to the add path too). Tests ----- - `tests/test_issue1538_nous_live_catalog.py` (12 tests): live-fetch surfaces ≥20 entries, every id starts with `@nous:`, every label ends with ` (via Nous)`, recent flagships (Opus 4.7, GPT-5.5, Kimi K2.6, Gemini 3.1 Pro, MiniMax M2.7) reach the dropdown, static fallback works when hermes_cli raises, label formatter unit tests (vendor namespace stripping, variant rendering, MiniMax mixed-case), the curated static list and its routing invariants are preserved. - `tests/test_issue1539_provider_removal_dropdown_invalidation.py` (11 tests): invalidator helper exists and clears both cache slots, exposed on window with typeof guard, both save and remove paths invoke the dropdown flush, helper calls both invalidator and populateModelDropdown, helper is resilient to missing modules, helper does not block panel refresh, server-side `set_provider_key → invalidate_models_cache` invariant pinned. Verified live on port 8789: `/api/models` Nous group returns 30 models (was 4); browser `document.getElementById('modelSelect')` exposes 30 options under the "Nous Portal" group; the dropdown-flush helper is callable from the browser and round-trip rebuild keeps the dropdown at 30 options. Test counts: - Full pytest: 4013 passed, 2 skipped, 3 xpassed, 0 failures (was 3990 → 4013, +23 from this PR). - QA harness pytest: 20 passed. - Browser API sanity: 11/11 passed. - Agent Browser CDP: 21/23 passed (the 2 SSE liveness failures reproduce on master and are unrelated to this PR).
226 lines
11 KiB
Python
226 lines
11 KiB
Python
"""Regression tests for #1539 — removing a provider in Settings must invalidate
|
|
every dropdown surface that caches /api/models, so the removed provider
|
|
disappears immediately without a server restart or page reload.
|
|
|
|
The bug
|
|
-------
|
|
Pre-fix, ``_removeProviderKey()`` in ``static/panels.js`` only called
|
|
``loadProvidersPanel()`` after deletion. That refreshed the providers card
|
|
list but left these JS-side caches stale:
|
|
|
|
* ``_slashModelCache`` / ``_slashModelCachePromise`` (``static/commands.js``) —
|
|
cache for the ``/model`` slash-command suggestions.
|
|
* ``_dynamicModelLabels`` / ``window._configuredModelBadges`` (``static/ui.js``) —
|
|
populated by ``populateModelDropdown()`` on boot and on profile switch.
|
|
|
|
Layered server-side cache via ``api/config.invalidate_models_cache`` was
|
|
already flushed (``set_provider_key`` calls it on both add + remove), so the
|
|
next ``/api/models`` request would return the correct list — but no consumer
|
|
was triggering one.
|
|
|
|
The fix
|
|
-------
|
|
``static/commands.js`` exposes an ``_invalidateSlashModelCache()`` helper on
|
|
``window``. ``static/panels.js`` calls it from a shared
|
|
``_refreshModelDropdownsAfterProviderChange()`` helper after both the save
|
|
and the remove paths, plus invokes ``populateModelDropdown()`` to rebuild
|
|
the composer / Settings dropdowns and ``_configuredModelBadges`` map.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import re
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
|
|
REPO = Path(__file__).resolve().parent.parent
|
|
|
|
|
|
def _read_static(name: str) -> str:
|
|
return (REPO / "static" / name).read_text(encoding="utf-8")
|
|
|
|
|
|
def _extract_function_body(src: str, signature: str) -> str:
|
|
"""Return the source of a top-level ``async function NAME(...)`` /
|
|
``function NAME(...)`` declaration via brace-balance — robust to nested
|
|
blocks (try/catch/await) and not dependent on indentation.
|
|
"""
|
|
idx = src.find(signature)
|
|
if idx == -1:
|
|
raise AssertionError(f"signature {signature!r} not found in source")
|
|
open_idx = src.find("{", idx)
|
|
if open_idx == -1:
|
|
raise AssertionError(f"could not find opening brace after {signature!r}")
|
|
depth = 0
|
|
for i in range(open_idx, len(src)):
|
|
c = src[i]
|
|
if c == "{":
|
|
depth += 1
|
|
elif c == "}":
|
|
depth -= 1
|
|
if depth == 0:
|
|
return src[idx : i + 1]
|
|
raise AssertionError(f"unbalanced braces in {signature!r}")
|
|
|
|
|
|
class TestSlashModelCacheInvalidator:
|
|
"""``static/commands.js`` must export the helper to ``window`` so
|
|
``static/panels.js`` can drop the slash-command cache without poking
|
|
module-local lets across module boundaries."""
|
|
|
|
def test_invalidator_helper_defined(self):
|
|
src = _read_static("commands.js")
|
|
assert "function _invalidateSlashModelCache(" in src, (
|
|
"_invalidateSlashModelCache helper missing from static/commands.js. "
|
|
"Without it static/panels.js cannot drop the /model slash-command "
|
|
"cache when a provider is added/removed (#1539)."
|
|
)
|
|
|
|
def test_invalidator_clears_both_cache_slots(self):
|
|
src = _read_static("commands.js")
|
|
body = _extract_function_body(src, "function _invalidateSlashModelCache(")
|
|
# Cache slots from static/commands.js:84-85 — keep both null'd.
|
|
assert "_slashModelCache=null" in body, (
|
|
"_invalidateSlashModelCache must null _slashModelCache so the next "
|
|
"/model autocomplete refetches /api/models."
|
|
)
|
|
assert "_slashModelCachePromise=null" in body, (
|
|
"_invalidateSlashModelCache must null _slashModelCachePromise so an "
|
|
"in-flight load doesn't resolve into the stale cache slot after "
|
|
"invalidation."
|
|
)
|
|
|
|
def test_invalidator_exposed_on_window(self):
|
|
src = _read_static("commands.js")
|
|
# Exposed on window via a typeof-guarded assignment so the module is
|
|
# also importable in headless test contexts (vm.runInContext) that
|
|
# don't define a window global.
|
|
assert "window._invalidateSlashModelCache=_invalidateSlashModelCache" in src, (
|
|
"_invalidateSlashModelCache must be exposed on window so static/panels.js "
|
|
"can invoke it across module boundaries."
|
|
)
|
|
assert "typeof window!=='undefined'" in src, (
|
|
"The window-export assignment must be guarded by `typeof window!=='undefined'` "
|
|
"so static/commands.js stays importable in headless vm contexts (the "
|
|
"tests/test_cli_only_slash_commands.py harness has no window global)."
|
|
)
|
|
|
|
|
|
class TestProviderRemoveInvalidatesDropdowns:
|
|
"""The remove path in ``static/panels.js`` must trigger the dropdown-cache
|
|
flush and rebuild — otherwise the dropped provider lingers in every
|
|
/model dropdown until the page reloads (#1539)."""
|
|
|
|
def test_remove_path_invokes_dropdown_flush(self):
|
|
src = _read_static("panels.js")
|
|
body = _extract_function_body(src, "async function _removeProviderKey(")
|
|
assert "_refreshModelDropdownsAfterProviderChange()" in body, (
|
|
"_removeProviderKey must call _refreshModelDropdownsAfterProviderChange() "
|
|
"after a successful delete. Without this, the JS-side caches "
|
|
"(_slashModelCache, _dynamicModelLabels, _configuredModelBadges) "
|
|
"still offer the deleted provider's models until reload (#1539)."
|
|
)
|
|
|
|
def test_save_path_invokes_dropdown_flush(self):
|
|
"""Defense-in-depth: adding a key has the same staleness shape — the
|
|
new provider's models won't show up until reload without this call.
|
|
Bundled in #1539."""
|
|
src = _read_static("panels.js")
|
|
body = _extract_function_body(src, "async function _saveProviderKey(")
|
|
assert "_refreshModelDropdownsAfterProviderChange()" in body, (
|
|
"_saveProviderKey must also call _refreshModelDropdownsAfterProviderChange() "
|
|
"so a newly-configured provider's models appear in every dropdown "
|
|
"without a reload. Same staleness shape as the remove path (#1539)."
|
|
)
|
|
|
|
def test_dropdown_flush_helper_defined(self):
|
|
src = _read_static("panels.js")
|
|
assert "function _refreshModelDropdownsAfterProviderChange(" in src, (
|
|
"_refreshModelDropdownsAfterProviderChange must be defined in "
|
|
"static/panels.js (single helper used by both save + remove paths)."
|
|
)
|
|
|
|
def test_dropdown_flush_calls_slash_cache_invalidator(self):
|
|
src = _read_static("panels.js")
|
|
body = _extract_function_body(src, "function _refreshModelDropdownsAfterProviderChange(")
|
|
# Must invoke the commands.js helper — directly poking module-local
|
|
# lets across module boundaries is brittle.
|
|
assert "_invalidateSlashModelCache" in body, (
|
|
"_refreshModelDropdownsAfterProviderChange must call "
|
|
"window._invalidateSlashModelCache() so the /model slash-command "
|
|
"cache is dropped (covers the slash-command surface from #1539)."
|
|
)
|
|
|
|
def test_dropdown_flush_calls_populate_model_dropdown(self):
|
|
src = _read_static("panels.js")
|
|
body = _extract_function_body(src, "function _refreshModelDropdownsAfterProviderChange(")
|
|
assert "populateModelDropdown" in body, (
|
|
"_refreshModelDropdownsAfterProviderChange must call "
|
|
"populateModelDropdown() so the composer model picker, Settings → "
|
|
"Default Model dropdown, _dynamicModelLabels, and "
|
|
"_configuredModelBadges all rebuild from a fresh /api/models "
|
|
"response (covers the dropdown + badge surfaces from #1539)."
|
|
)
|
|
|
|
def test_dropdown_flush_is_resilient_to_missing_modules(self):
|
|
"""If commands.js or ui.js failed to load, the providers panel must
|
|
still update — the dropdown flush is best-effort (#1539)."""
|
|
src = _read_static("panels.js")
|
|
body = _extract_function_body(src, "function _refreshModelDropdownsAfterProviderChange(")
|
|
# Outer try/catch wraps the whole helper so a runtime error inside
|
|
# populateModelDropdown / cache flush cannot surface as an unhandled
|
|
# rejection that breaks the surrounding save/remove flow.
|
|
assert re.search(r"\btry\s*\{", body), (
|
|
"_refreshModelDropdownsAfterProviderChange must wrap its work in "
|
|
"try/catch — if commands.js or ui.js failed to load, a missing "
|
|
"function should not break the providers panel update (#1539)."
|
|
)
|
|
# And the populateModelDropdown call must be guarded by typeof — the
|
|
# dropdown rebuild is best-effort.
|
|
assert "typeof populateModelDropdown" in body, (
|
|
"populateModelDropdown lookup must use typeof so it gracefully "
|
|
"skips when ui.js hasn't loaded yet."
|
|
)
|
|
|
|
def test_dropdown_flush_does_not_block_panel_refresh(self):
|
|
"""populateModelDropdown is async; its result must not be awaited
|
|
synchronously inside the helper — otherwise a slow /api/models would
|
|
delay the providers panel re-render (#1539)."""
|
|
src = _read_static("panels.js")
|
|
body = _extract_function_body(src, "function _refreshModelDropdownsAfterProviderChange(")
|
|
# The helper itself is non-async (signature checked indirectly: the
|
|
# source begins with 'function _refresh...', not 'async function').
|
|
# Anything async is fired with Promise.resolve(...).catch(...) so the
|
|
# provider panel re-render is not blocked.
|
|
assert body.startswith("function _refreshModelDropdownsAfterProviderChange"), (
|
|
"_refreshModelDropdownsAfterProviderChange should be a sync helper "
|
|
"that fires-and-forgets populateModelDropdown — not an async one "
|
|
"the save/remove paths await."
|
|
)
|
|
|
|
|
|
class TestServerSideInvariantPreserved:
|
|
"""Server-side ``invalidate_models_cache()`` is the load-bearing invariant
|
|
that lets the next /api/models request return correct data; #1539 was a
|
|
pure frontend bug, but pin the server-side wiring so a refactor of
|
|
``set_provider_key`` cannot silently regress it."""
|
|
|
|
def test_set_provider_key_invalidates_cache(self):
|
|
src = (REPO / "api" / "providers.py").read_text(encoding="utf-8")
|
|
# set_provider_key is the canonical write path — both add and remove
|
|
# flow through it (remove_provider_key calls set_provider_key(pid, None)).
|
|
m = re.search(
|
|
r"def set_provider_key\([^)]*\).*?(?=\ndef |\Z)",
|
|
src,
|
|
re.DOTALL,
|
|
)
|
|
assert m, "set_provider_key not found in api/providers.py"
|
|
body = m.group(0)
|
|
assert "invalidate_models_cache()" in body, (
|
|
"set_provider_key must call invalidate_models_cache() so the "
|
|
"server-side TTL cache is flushed on every add/remove. Without "
|
|
"this, even a perfectly-cached frontend would receive stale data."
|
|
)
|