Merge pull request #2267 from nesquena/stage-356

stage-356: small 2-PR follow-up batch — #2234 aux-model routing + #2265 mixed-case provider canonicalization (closes #2245)
This commit is contained in:
nesquena-hermes
2026-05-14 09:24:23 -07:00
committed by GitHub
5 changed files with 272 additions and 7 deletions
+8 -1
View File
@@ -4,6 +4,14 @@
### Fixed
- **PR #2265** by @Michaelyklam (closes #2245) — Model picker provider lookup now canonicalizes configured provider keys before loading their configured models. Pre-fix, custom provider keys with mixed casing or underscores such as `CLIPpoxy` or `snake_case_provider` were lower-cased during canonicalization, but the resulting canonical key didn't match the raw `config.yaml` key, so the model allowlist lookup silently returned empty and the model picker dropdown showed no models for that provider. The fix maps canonical provider IDs back to their raw config.yaml provider keys before loading `provider_cfg`. Original config keys are preserved for provider settings rendering. 242-line regression test covering CLIPpoxy + snake_case_provider plus built-in/fallback behavior.
- **PR #2234** by @Jordan-SkyLF (post-v0.51.62 rebase, follow-up to v0.51.62's category-refinement portion) — Update summary generation now routes through the documented `auxiliary.compression` text-model slot instead of a WebUI-only `auxiliary.update_summary` magic key. The reviewer concern was that `update_summary` would have been a non-discoverable WebUI-specific config key; using the existing documented compression/summarization slot keeps the PR self-contained to `hermes-webui` and gives users a way to override summary generation through an existing config surface. The existing main-model fallback is preserved if auxiliary resolution or generation fails. Adds route comment explaining why summary generation maps to compression instead of inventing a new task name.
## [v0.51.62] — 2026-05-14 — Release AL (stage-355 — 11-PR full sweep — metadata-only cache hit fixes + skill detail + phone UX + display-title projection + escaping + RFC update)
### Fixed
- **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.)
@@ -37,7 +45,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
View File
@@ -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
+5 -1
View File
@@ -5467,8 +5467,12 @@ def handle_post(handler, parsed) -> bool:
try:
from agent.auxiliary_client import get_text_auxiliary_client
# Update summaries are a short text-compression/summarization task.
# Reuse the documented auxiliary.compression slot instead of
# inventing a WebUI-only auxiliary task name that users cannot
# discover in the Hermes Agent setup/config UI.
aux_client, aux_model = get_text_auxiliary_client(
"update_summary",
"compression",
main_runtime=main_runtime,
)
if aux_client is not None and aux_model:
@@ -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"
+4 -3
View File
@@ -419,13 +419,14 @@ class TestForceUpdateRoute:
class TestUpdateSummaryRouteModelSelection:
"""Update summaries should use the auxiliary update-summary model before main model fallback."""
"""Update summaries should use a known text auxiliary model before main model fallback."""
def test_summary_route_prefers_update_summary_auxiliary_model(self):
def test_summary_route_prefers_documented_compression_auxiliary_model(self):
src = read('api/routes.py')
assert 'get_text_auxiliary_client' in src
assert '"update_summary"' in src
assert '"compression"' in src
assert '"update_summary"' not in src
assert 'main_runtime=main_runtime' in src
assert 'update summary auxiliary model failed; falling back to main model' in src
assert 'from run_agent import AIAgent' in src