fix(models): credit the managed local-models library in /model validation
The validation ladder had no llamacpp branch: a switch to a freshly downloaded local model fell through to the generic live-listing probe, which hard-rejects when the spawn-only /v1/models listing has not learned the new file yet — so the Local Models Use button and the composer picker could never succeed for a non-catalog model. The managed runtime now validates against the staged library on disk (the source of truth for what the user downloaded), case-insensitively; ids that were never staged keep the live-listing verdict. The activate flow's self-heal had the same blind spot: its rescan only ran when this process supervised the server, but ensure_local_runtime returns None when another process owns it — precisely the desktop situation after a download job bounced the router once. Probe the live listing through the persisted endpoint and bounce the router when it lacks the model.
This commit is contained in:
committed by
Teknium
parent
5141b3122b
commit
d02762ccca
@@ -449,6 +449,26 @@ def _profile_owns_catalog(normalized: str) -> bool:
|
||||
type(profile).fetch_models is not ProviderProfile.fetch_models or bool(profile.models_url))
|
||||
|
||||
|
||||
def _validate_managed_local(req: _Request) -> Optional[dict[str, Any]]:
|
||||
"""The managed llama.cpp runtime: the staged library on disk is the source of truth, not the
|
||||
live listing. The router's model list is spawn-only (a GGUF landed after its start is
|
||||
invisible to GET /models until a bounce), so validating a freshly downloaded model against
|
||||
the live listing rejects the very file the user just staged — the Local Models "Use" flow
|
||||
and the composer picker could never succeed for a non-catalog model. A staged id accepts
|
||||
(case-insensitive: typing matches the file name, the router registers the preset id);
|
||||
anything else falls through to the live listing, which stays authoritative for ids that
|
||||
were never downloaded here."""
|
||||
from hermes_cli.local_runtime.bootstrap import staged_model_ids
|
||||
|
||||
staged = {sid.lower() for sid in staged_model_ids()}
|
||||
if req.lookup.strip().lower() in staged:
|
||||
return _accept_with_note(
|
||||
f"Note: `{req.requested}` was not found in the live /v1/models listing "
|
||||
"but is downloaded in the managed local-models library — accepted."
|
||||
)
|
||||
return None
|
||||
|
||||
|
||||
def _validate_live_listing(req: _Request) -> Optional[dict[str, Any]]:
|
||||
"""Generic live /v1/models probe. Returns None when the API was unreachable (the caller then
|
||||
tries Bedrock discovery / the curated catalog). A profile that owns its catalog is validated
|
||||
@@ -557,8 +577,8 @@ def _for(*providers: str) -> Callable[[_Request], bool]:
|
||||
|
||||
# (gate, branch): the branch runs when the gate passes; the first non-None verdict wins. ORDER IS
|
||||
# BEHAVIOR: moa → whitespace → OpenRouter preset parse → LM Studio → Ollama native → custom →
|
||||
# codex/xai static → MiniMax → Anthropic native → Anthropic Messages → live listing → Bedrock →
|
||||
# curated-catalog fallback (always decides).
|
||||
# codex/xai static → MiniMax → managed local (staged library) → Anthropic native →
|
||||
# Anthropic Messages → live listing → Bedrock → curated-catalog fallback (always decides).
|
||||
_LADDER: tuple[tuple[Callable[[_Request], bool], Callable[[_Request], Optional[dict[str, Any]]]], ...] = (
|
||||
(_for("moa"), _validate_moa),
|
||||
(lambda req: True, _reject_whitespace),
|
||||
@@ -568,6 +588,7 @@ _LADDER: tuple[tuple[Callable[[_Request], bool], Callable[[_Request], Optional[d
|
||||
(_is_custom, _validate_custom),
|
||||
(_for("openai-codex", "xai-oauth"), _validate_static_catalog),
|
||||
(_for("minimax", "minimax-cn"), _validate_minimax),
|
||||
(_for("llamacpp", "llama.cpp", "llama-cpp"), _validate_managed_local),
|
||||
(_for("anthropic"), _validate_anthropic),
|
||||
(lambda req: req.api_mode == "anthropic_messages", _validate_anthropic_messages),
|
||||
(lambda req: True, _validate_live_listing),
|
||||
|
||||
@@ -263,13 +263,25 @@ def _ensure_server(job: Dict[str, Any], config: dict, model_id: str, *, fail_det
|
||||
_step(job, "starting-server", "Starting the local server")
|
||||
sup = _start_local_server(config, fail_detail)
|
||||
|
||||
def rescan_if_unknown() -> None:
|
||||
if model_id not in sup.models():
|
||||
def rescan_if_unknown(known: Dict[str, Any]) -> None:
|
||||
if model_id not in known:
|
||||
job["detail"] = "Refreshing the local server"
|
||||
bootstrap.refresh_local_runtime()
|
||||
|
||||
if sup is not None:
|
||||
_quiet(rescan_if_unknown, None, debug=skip_msg)
|
||||
_quiet(lambda: rescan_if_unknown(sup.models()), None, debug=skip_msg)
|
||||
return
|
||||
# A server owned by another process: ensure_local_runtime returned None, so the supervisor
|
||||
# path above never ran — but the same spawn-only listing gap applies. Probe the live
|
||||
# listing through the persisted endpoint and bounce when it lacks the model.
|
||||
endpoint = _state_endpoint()
|
||||
if endpoint is not None:
|
||||
|
||||
def _known_models() -> Dict[str, Any]:
|
||||
data = (_router_request(endpoint, "/models", timeout=10) or {}).get("data", [])
|
||||
return {m.get("id"): m for m in data}
|
||||
|
||||
_quiet(lambda: rescan_if_unknown(_known_models()), None, debug=skip_msg)
|
||||
|
||||
|
||||
def _assign_default(job: Dict[str, Any], model_id: str) -> None:
|
||||
|
||||
90
tests/hermes_cli/test_local_models_activate_rescan.py
Normal file
90
tests/hermes_cli/test_local_models_activate_rescan.py
Normal file
@@ -0,0 +1,90 @@
|
||||
"""The activate flow's rescan must fire even when another process owns the local server (#115237).
|
||||
|
||||
ensure_local_runtime returns None when the router is supervised elsewhere, so the supervisor
|
||||
path's rescan_if_unknown never ran — exactly the desktop situation after a download job already
|
||||
bounced the router once (presets fresh, listing still spawn-only and missing the new model).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import patch
|
||||
|
||||
|
||||
def test_activate_rescans_through_state_endpoint_when_supervisor_is_none(
|
||||
tmp_path, monkeypatch
|
||||
):
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes"))
|
||||
from hermes_cli.web_routers import local_models as lm
|
||||
|
||||
staged = tmp_path / "models"
|
||||
staged.mkdir(parents=True)
|
||||
(staged / "Qwen3-4B-Q4_K_M.gguf").write_bytes(b"GGUF\x00\x00\x00\x00")
|
||||
|
||||
calls = {"refresh": 0, "known": [{"id": "some-other-model"}]}
|
||||
|
||||
def fake_refresh():
|
||||
calls["refresh"] += 1
|
||||
|
||||
def fake_router_request(endpoint, path, *, timeout, payload=None):
|
||||
assert path == "/models"
|
||||
return {"data": calls["known"]}
|
||||
|
||||
job = lm._job("model-activate", "Qwen3-4B-Q4_K_M", model_id="Qwen3-4B-Q4_K_M")
|
||||
with (
|
||||
patch.object(lm.bootstrap, "ensure_local_runtime", return_value=None),
|
||||
patch.object(
|
||||
lm,
|
||||
"_state_endpoint",
|
||||
return_value={"base_url": "http://127.0.0.1:18434/v1", "api_key": "k"},
|
||||
),
|
||||
patch.object(lm.bootstrap, "refresh_local_runtime", fake_refresh),
|
||||
patch.object(lm, "_router_request", fake_router_request),
|
||||
patch.object(lm, "_set_runtime_enabled", lambda v: {}),
|
||||
patch(
|
||||
"hermes_cli.web_server_config._apply_model_assignment_sync",
|
||||
lambda *a, **k: {"ok": True},
|
||||
),
|
||||
):
|
||||
lm._ensure_server(
|
||||
job,
|
||||
{"local_runtime": {"enabled": True}},
|
||||
"Qwen3-4B-Q4_K_M",
|
||||
fail_detail="server failed",
|
||||
skip_msg="skipped",
|
||||
)
|
||||
|
||||
assert calls["refresh"] == 1, (
|
||||
"the router must be bounced when its listing lacks the model"
|
||||
)
|
||||
|
||||
|
||||
def test_activate_skips_rescan_when_owned_server_already_lists_the_model(
|
||||
tmp_path, monkeypatch
|
||||
):
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes"))
|
||||
from hermes_cli.web_routers import local_models as lm
|
||||
|
||||
calls = {"refresh": 0}
|
||||
|
||||
class FakeSup:
|
||||
def models(self):
|
||||
return {"Qwen3-4B-Q4_K_M": "loaded"}
|
||||
|
||||
job = lm._job("model-activate", "Qwen3-4B-Q4_K_M", model_id="Qwen3-4B-Q4_K_M")
|
||||
with (
|
||||
patch.object(lm.bootstrap, "ensure_local_runtime", return_value=FakeSup()),
|
||||
patch.object(
|
||||
lm.bootstrap,
|
||||
"refresh_local_runtime",
|
||||
lambda: calls.__setitem__("refresh", calls["refresh"] + 1),
|
||||
),
|
||||
):
|
||||
lm._ensure_server(
|
||||
job,
|
||||
{"local_runtime": {"enabled": True}},
|
||||
"Qwen3-4B-Q4_K_M",
|
||||
fail_detail="server failed",
|
||||
skip_msg="skipped",
|
||||
)
|
||||
|
||||
assert calls["refresh"] == 0, "no bounce when the listing already knows the model"
|
||||
98
tests/hermes_cli/test_model_validation_managed_local.py
Normal file
98
tests/hermes_cli/test_model_validation_managed_local.py
Normal file
@@ -0,0 +1,98 @@
|
||||
"""Tests for the managed llama.cpp runtime branch in /model validation (#115237)."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import patch
|
||||
|
||||
from hermes_cli.models_validate import validate_requested_model
|
||||
|
||||
|
||||
def _validate(model, staged=("Qwen3-4B-Q4_K_M",), live=("other-model",), **kw):
|
||||
"""validate against a managed-local world: staged files on disk + a live listing that
|
||||
(spawn-only as it is) hasn't learned the new file yet."""
|
||||
with (
|
||||
patch("hermes_cli.models.fetch_api_models", return_value=list(live)),
|
||||
patch(
|
||||
"hermes_cli.models.probe_api_models",
|
||||
return_value={
|
||||
"models": list(live),
|
||||
"probed_url": "http://127.0.0.1:18434/v1/models",
|
||||
"resolved_base_url": "http://127.0.0.1:18434/v1",
|
||||
"suggested_base_url": None,
|
||||
"used_fallback": False,
|
||||
},
|
||||
),
|
||||
patch(
|
||||
"hermes_cli.local_runtime.bootstrap.staged_model_ids",
|
||||
return_value=list(staged),
|
||||
),
|
||||
):
|
||||
return validate_requested_model(
|
||||
model, "llamacpp", base_url="http://127.0.0.1:18434/v1", **kw
|
||||
)
|
||||
|
||||
|
||||
class TestManagedLocalValidation:
|
||||
def test_staged_but_not_live_accepted(self):
|
||||
"""The Use button's exact case: downloaded, not yet in the spawn-only live listing."""
|
||||
result = _validate("Qwen3-4B-Q4_K_M")
|
||||
assert (result["accepted"], result["persist"], result["recognized"]) == (
|
||||
True,
|
||||
True,
|
||||
True,
|
||||
)
|
||||
assert "managed local-models library" in result["message"]
|
||||
|
||||
def test_typed_case_matches_staged_spelling(self):
|
||||
result = _validate("qwen3-4b-q4_k_m")
|
||||
assert result["accepted"] is True
|
||||
assert result["recognized"] is True
|
||||
|
||||
def test_not_staged_and_not_live_still_rejected(self):
|
||||
result = _validate("Llama-4-90B-Q4_K_M", staged=())
|
||||
assert result["accepted"] is False
|
||||
assert "was not found in this provider's model listing" in result["message"]
|
||||
|
||||
def test_live_hit_still_wins(self):
|
||||
"""A model the router already serves validates through the live listing, staged or not."""
|
||||
result = _validate("other-model", staged=())
|
||||
assert (result["accepted"], result["persist"], result["recognized"]) == (
|
||||
True,
|
||||
True,
|
||||
True,
|
||||
)
|
||||
assert result["message"] is None
|
||||
|
||||
def test_aliases_route_to_the_same_branch(self):
|
||||
for alias in ("llama.cpp", "llama-cpp"):
|
||||
result = _validate("Qwen3-4B-Q4_K_M")
|
||||
assert result["accepted"] is True, alias
|
||||
|
||||
def test_other_providers_do_not_read_the_staged_library(self):
|
||||
"""The branch is llamacpp-only: a cloud provider must keep its live-listing verdict."""
|
||||
with (
|
||||
patch("hermes_cli.models.fetch_api_models", return_value=["deepseek-v4.1"]),
|
||||
patch(
|
||||
"hermes_cli.models.probe_api_models",
|
||||
return_value={
|
||||
"models": ["deepseek-v4.1"],
|
||||
"probed_url": "x",
|
||||
"resolved_base_url": "x",
|
||||
"suggested_base_url": None,
|
||||
"used_fallback": False,
|
||||
},
|
||||
),
|
||||
patch(
|
||||
"hermes_cli.local_runtime.bootstrap.staged_model_ids",
|
||||
return_value=["Qwen3-4B-Q4_K_M"],
|
||||
),
|
||||
):
|
||||
result = validate_requested_model(
|
||||
"Qwen3-4B-Q4_K_M", "deepseek", base_url="https://api.deepseek.com/v1"
|
||||
)
|
||||
assert result["accepted"] is False
|
||||
|
||||
def test_empty_staged_library_falls_through(self):
|
||||
"""No staged files: nothing to credit, the live listing decides (None fall-through)."""
|
||||
result = _validate("other-model", staged=())
|
||||
assert result["accepted"] is True # live hit via _validate_live_listing
|
||||
Reference in New Issue
Block a user