mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-05-25 03:00:23 +00:00
Merge pull request #2265 into stage-356
Fix configured provider models after key canonicalization (Michaelyklam, closes #2245)
This commit is contained in:
Hermes Agent
+2
-1
@@ -4,6 +4,8 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- **Issue #2245** — Model picker provider lookup now canonicalizes configured provider keys before loading their configured models. Custom provider keys such as `CLIPpoxy` or `snake_case_provider` still display their configured model allowlists after canonicalization while preserving the original config key for provider settings.
|
||||
|
||||
- **PR #2244** by @franksong2702 (fixes #2243) — `Archive Session` no longer fails when the in-memory session cache contains a metadata-only stub for the target. Pre-fix, the route loaded via `get_session(sid)` which returned the cached `_loaded_metadata_only=True` instance, then `Session.save()` correctly refused to write because the metadata stub's `messages=[]` would have overwritten the full transcript (#1558 guard). Now the archive route reloads the full session from disk before mutating `archived` and refreshes the cache. Existing CLI/imported-session fallback unchanged. 47-line regression test pinning the route-level behaviour.
|
||||
|
||||
- **PR #2249** by @franksong2702 (fixes #2248, follow-up to #2244) — Same metadata-only cache hit was happening at `/api/session/pin`, `/api/session/rename`, and `/api/personality/set`. Adds `_ensure_full_session_before_mutation()` helper in `api/routes.py` that reloads through `Session.load(sid)`, replaces the cached entry, preserves LRU ordering, and enforces `SESSIONS_MAX` eviction. Applied to all three routes. Parametrized regression coverage forces a metadata-only session into the cache for each route and verifies the saved session keeps its original messages while the cache is upgraded to a full session. (Archive Session in #2244 still uses an inline fix at the same site — a follow-up could refactor archive to use the helper too.)
|
||||
@@ -39,7 +41,6 @@
|
||||
- **PR #2228** by @franksong2702 (refs #749) — Profile creation now exposes the same configured model/provider choices users already see in the composer/settings model picker. New profiles can be created with an explicit `model.default` and `model.provider` written into that profile's own `config.yaml`, while clone/base-url/API-key creation behavior remains unchanged. Backend validates the chosen model/provider before profile creation so invalid values do not leave a half-created profile on disk. Adds locale entries for English, Chinese, Japanese, Korean, Russian, and Spanish (parity-tested). 74-line regression test pinning the form, backend, and locale-key contract.
|
||||
|
||||
### Fixed
|
||||
|
||||
- **PR #2234** by @Jordan-SkyLF (refines #2207, original v0.51.61 portion) — Three update-banner improvements: (1) Update summaries no longer repeat the same bullet under both "What you'll notice" and "Worth knowing" — visible notice items keep priority, and the secondary section is omitted when there is no distinct detail to show. (2) Update summaries now cap large commit lists (24 + probe item) before sending them to the summarizer and disclose when the summary uses only the latest commit subjects, while keeping the full comparison link available — bounds summarizer cost on large update ranges while remaining honest about coverage. (3) Update banners now wrap generated-summary links and long update text on narrow/mobile screens inside the banner instead of pushing controls off-screen. 108-line regression coverage for short-target dedup, repeated Agent-summary bullets, large-range capped input, and responsive wrapping. (A follow-up commit pushed AFTER stage-354 merged is now shipped in stage-355.)
|
||||
|
||||
- **PR #2236** by @jasonjcwu — Silent failure detection in `api/streaming.py` now scans only NEW messages, not the full conversation history. Pre-fix, the `_assistant_added` check at `_run_agent_streaming` scanned all messages in `result["messages"]` (including pre-turn history); if any prior turn contained an assistant response, `_assistant_added` was `True` and the apperror SSE event was silently skipped, leaving the user staring at a blank response after a provider 401/429/rate-limit error. Fix extracts a `_has_new_assistant_reply(all_messages, prev_count)` helper that only inspects messages beyond the pre-turn history offset (`_previous_context_messages`); applied to both the main detection path and the self-heal/retry `_heal_ok` check. 15-test regression suite covering empty/short/long-history scenarios, the heal path, and the `len < prev_count` edge-case fallback. Also includes a small alignment fix to `test_issue1857_usage_overwrite.py` so the FakeAgent message shape matches what the real agent produces.
|
||||
|
||||
+13
-2
@@ -2872,11 +2872,16 @@ def get_available_models() -> dict:
|
||||
# The same applies to mixed-case ids like ``OpenCode-Go`` and to
|
||||
# legitimate aliases like ``z-ai`` → ``zai``.
|
||||
_cfg_providers = cfg.get("providers", {})
|
||||
# Map canonical provider IDs back to raw config keys so the
|
||||
# generic-provider branch can preserve mixed-case/underscore
|
||||
# provider_cfg values (#2245).
|
||||
_canonical_to_raw_provider_key: dict[str, str] = {}
|
||||
if isinstance(_cfg_providers, dict):
|
||||
for _pid_key in _cfg_providers:
|
||||
_canonical = _canonicalise_provider_id(_pid_key)
|
||||
if not _canonical:
|
||||
continue
|
||||
_canonical_to_raw_provider_key.setdefault(_canonical, _pid_key)
|
||||
if _canonical in _PROVIDER_MODELS or _canonical in _cfg_providers or _pid_key in _cfg_providers:
|
||||
detected_providers.add(_canonical)
|
||||
|
||||
@@ -3502,8 +3507,14 @@ def get_available_models() -> dict:
|
||||
"models": models,
|
||||
}
|
||||
)
|
||||
elif pid in _PROVIDER_MODELS or pid in cfg.get("providers", {}):
|
||||
provider_cfg = cfg.get("providers", {}).get(pid, {})
|
||||
elif pid in _PROVIDER_MODELS or pid in _canonical_to_raw_provider_key:
|
||||
# Look up provider_cfg using the original raw key from
|
||||
# config.yaml so that mixed-case / underscore keys like
|
||||
# ``CLIPpoxy`` or ``snake_case_provider`` still resolve
|
||||
# (#2245). Fall back to the canonical pid for providers
|
||||
# that appear in _PROVIDER_MODELS but not in cfg.
|
||||
_raw_key = _canonical_to_raw_provider_key.get(pid, pid)
|
||||
provider_cfg = cfg.get("providers", {}).get(_raw_key, {})
|
||||
raw_models = []
|
||||
|
||||
# User-configured model allowlists are explicit local
|
||||
|
||||
@@ -0,0 +1,242 @@
|
||||
"""Regression tests for #2245 — mixed-case custom provider keys lose picker models.
|
||||
|
||||
When a user configures a provider key in ``config.yaml`` with mixed case
|
||||
(e.g. ``CLIPpoxy``) or underscores (e.g. ``snake_case_provider``), the
|
||||
WebUI model picker must still surface that provider's configured models.
|
||||
|
||||
Root cause: ``_build_available_models_uncached()`` iterates over
|
||||
*canonicalised* provider IDs (lowercase, hyphens) but looked up
|
||||
``cfg["providers"]`` using the canonical key — which doesn't match the
|
||||
raw mixed-case/underscore key in the config dict. The fix adds a
|
||||
``_canonical_to_raw_provider_key`` map so the generic-provider branch
|
||||
can resolve the original key and load ``provider_cfg`` correctly.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
import types
|
||||
|
||||
import pytest
|
||||
|
||||
import api.config as config
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fixtures
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _isolate_models_cache(tmp_path, monkeypatch):
|
||||
"""Invalidate the models TTL cache before and after every test."""
|
||||
monkeypatch.setattr(config, "_models_cache_path", tmp_path / "models_cache.json")
|
||||
config.invalidate_models_cache()
|
||||
yield
|
||||
config.invalidate_models_cache()
|
||||
|
||||
|
||||
def _stub_hermes_cli(monkeypatch):
|
||||
"""Stub hermes_cli so no real CLI/agent calls happen."""
|
||||
fake_models = types.ModuleType("hermes_cli.models")
|
||||
fake_models.list_available_providers = lambda: []
|
||||
fake_models.provider_model_ids = lambda _pid: []
|
||||
fake_auth = types.ModuleType("hermes_cli.auth")
|
||||
fake_auth.get_auth_status = lambda _pid: {"key_source": "none"}
|
||||
monkeypatch.setitem(sys.modules, "hermes_cli.models", fake_models)
|
||||
monkeypatch.setitem(sys.modules, "hermes_cli.auth", fake_auth)
|
||||
monkeypatch.setattr(
|
||||
config,
|
||||
"_get_auth_store_path",
|
||||
lambda: config.Path("/tmp/does-not-exist-auth.json"),
|
||||
)
|
||||
|
||||
|
||||
def _with_config(cfg_dict: dict):
|
||||
"""Replace ``config.cfg`` with *cfg_dict* and return a restore callable."""
|
||||
old_cfg = dict(config.cfg)
|
||||
old_mtime = config._cfg_mtime
|
||||
config.cfg.clear()
|
||||
config.cfg.update(cfg_dict)
|
||||
try:
|
||||
config._cfg_mtime = config.Path(config._get_config_path()).stat().st_mtime
|
||||
except Exception:
|
||||
config._cfg_mtime = 0.0
|
||||
|
||||
def restore():
|
||||
config.cfg.clear()
|
||||
config.cfg.update(old_cfg)
|
||||
config._cfg_mtime = old_mtime
|
||||
config.invalidate_models_cache()
|
||||
|
||||
return restore
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Test case 1 — mixed-case provider key
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_mixed_case_provider_key_produces_configured_models(monkeypatch):
|
||||
"""A provider key like ``CLIPpoxy`` must feed its configured models into
|
||||
the picker after canonicalisation to ``clippoxy``."""
|
||||
_stub_hermes_cli(monkeypatch)
|
||||
monkeypatch.setattr("socket.getaddrinfo", lambda *a, **k: [])
|
||||
|
||||
restore = _with_config(
|
||||
{
|
||||
"model": {
|
||||
"default": "my-model",
|
||||
"provider": "CLIPpoxy",
|
||||
},
|
||||
"providers": {
|
||||
"CLIPpoxy": {
|
||||
"models": ["my-model", "another-model"],
|
||||
},
|
||||
},
|
||||
}
|
||||
)
|
||||
try:
|
||||
result = config.get_available_models()
|
||||
finally:
|
||||
restore()
|
||||
|
||||
# Find the group for the canonicalised provider id "clippoxy"
|
||||
groups_by_pid = {g["provider_id"]: g for g in result["groups"]}
|
||||
assert "clippoxy" in groups_by_pid, (
|
||||
f"Expected canonical provider id 'clippoxy' in groups, "
|
||||
f"got: {list(groups_by_pid.keys())}"
|
||||
)
|
||||
group = groups_by_pid["clippoxy"]
|
||||
model_ids = [m["id"] for m in group["models"]]
|
||||
# Both configured models must appear (possibly with @clippoxy: prefix)
|
||||
# Strip any @provider: prefix for comparison
|
||||
bare_ids = []
|
||||
for mid in model_ids:
|
||||
if mid.startswith("@"):
|
||||
bare_ids.append(mid.split(":", 1)[-1] if ":" in mid else mid)
|
||||
else:
|
||||
bare_ids.append(mid)
|
||||
assert "my-model" in bare_ids, (
|
||||
f"Expected 'my-model' in model ids for clippoxy, got: {bare_ids}"
|
||||
)
|
||||
assert "another-model" in bare_ids, (
|
||||
f"Expected 'another-model' in model ids for clippoxy, got: {bare_ids}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Test case 2 — underscore provider key
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_underscore_provider_key_produces_configured_models(monkeypatch):
|
||||
"""A provider key like ``snake_case_provider`` must canonicalise to
|
||||
``snake-case-provider`` and still surface its configured models."""
|
||||
_stub_hermes_cli(monkeypatch)
|
||||
monkeypatch.setattr("socket.getaddrinfo", lambda *a, **k: [])
|
||||
|
||||
restore = _with_config(
|
||||
{
|
||||
"model": {
|
||||
"default": "model-a",
|
||||
"provider": "snake_case_provider",
|
||||
},
|
||||
"providers": {
|
||||
"snake_case_provider": {
|
||||
"models": ["model-a", "model-b"],
|
||||
},
|
||||
},
|
||||
}
|
||||
)
|
||||
try:
|
||||
result = config.get_available_models()
|
||||
finally:
|
||||
restore()
|
||||
|
||||
groups_by_pid = {g["provider_id"]: g for g in result["groups"]}
|
||||
canonical_pid = "snake-case-provider"
|
||||
assert canonical_pid in groups_by_pid, (
|
||||
f"Expected canonical provider id '{canonical_pid}' in groups, "
|
||||
f"got: {list(groups_by_pid.keys())}"
|
||||
)
|
||||
group = groups_by_pid[canonical_pid]
|
||||
model_ids = [m["id"] for m in group["models"]]
|
||||
bare_ids = []
|
||||
for mid in model_ids:
|
||||
if mid.startswith("@"):
|
||||
bare_ids.append(mid.split(":", 1)[-1] if ":" in mid else mid)
|
||||
else:
|
||||
bare_ids.append(mid)
|
||||
assert "model-a" in bare_ids, (
|
||||
f"Expected 'model-a' in model ids for {canonical_pid}, got: {bare_ids}"
|
||||
)
|
||||
assert "model-b" in bare_ids, (
|
||||
f"Expected 'model-b' in model ids for {canonical_pid}, got: {bare_ids}"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Test case 3 — built-in provider with lowercase key still works
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_builtin_provider_still_resolves(monkeypatch):
|
||||
"""A built-in provider like ``anthropic`` must still resolve through the
|
||||
same branch without regression."""
|
||||
_stub_hermes_cli(monkeypatch)
|
||||
monkeypatch.setattr("socket.getaddrinfo", lambda *a, **k: [])
|
||||
|
||||
restore = _with_config(
|
||||
{
|
||||
"model": {
|
||||
"default": "claude-sonnet-4-5",
|
||||
"provider": "anthropic",
|
||||
},
|
||||
"providers": {
|
||||
"anthropic": {
|
||||
"api_key": "sk-test-key",
|
||||
},
|
||||
},
|
||||
}
|
||||
)
|
||||
try:
|
||||
result = config.get_available_models()
|
||||
finally:
|
||||
restore()
|
||||
|
||||
groups_by_pid = {g["provider_id"]: g for g in result["groups"]}
|
||||
assert "anthropic" in groups_by_pid, (
|
||||
f"Expected 'anthropic' in groups, got: {list(groups_by_pid.keys())}"
|
||||
)
|
||||
# Should have at least one model (from _PROVIDER_MODELS fallback)
|
||||
group = groups_by_pid["anthropic"]
|
||||
assert len(group["models"]) > 0, "anthropic group should have models"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Test case 4 — _PROVIDER_MODELS fallback still works when no cfg key
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_provider_models_fallback_when_no_config_key(monkeypatch):
|
||||
"""A provider in _PROVIDER_MODELS but NOT in cfg["providers"] must
|
||||
still fall back to the static model list."""
|
||||
_stub_hermes_cli(monkeypatch)
|
||||
monkeypatch.setattr("socket.getaddrinfo", lambda *a, **k: [])
|
||||
|
||||
restore = _with_config(
|
||||
{
|
||||
"model": {
|
||||
"default": "deepseek-chat",
|
||||
"provider": "deepseek",
|
||||
},
|
||||
# No providers section at all
|
||||
}
|
||||
)
|
||||
try:
|
||||
result = config.get_available_models()
|
||||
finally:
|
||||
restore()
|
||||
|
||||
groups_by_pid = {g["provider_id"]: g for g in result["groups"]}
|
||||
assert "deepseek" in groups_by_pid, (
|
||||
f"Expected 'deepseek' in groups, got: {list(groups_by_pid.keys())}"
|
||||
)
|
||||
group = groups_by_pid["deepseek"]
|
||||
assert len(group["models"]) > 0, "deepseek group should have models from _PROVIDER_MODELS"
|
||||
Reference in New Issue
Block a user