From 30420455403cee431f2fb5a9e665649aef0efccd Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Thu, 18 Jun 2026 12:41:32 -0700 Subject: [PATCH] fix(picker): keep max_models=0 distinct from unlimited; lock cap semantics Follow-up to the cap-removal salvage. The contributor guarded the new unlimited default with `[:max_models] if max_models else ...`, which conflates max_models=0 (used by slug-only callers that want an empty model list) with None (unlimited). Tighten to `is not None` at all five slicing sites in list_authenticated_providers / list_picker_providers, and add a regression test asserting the three-way contract: None=full, 0=empty, N=first N. --- hermes_cli/model_switch.py | 10 ++-- tests/hermes_cli/test_model_catalog.py | 65 ++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 5 deletions(-) diff --git a/hermes_cli/model_switch.py b/hermes_cli/model_switch.py index 4da54beedf6..eae987fbbdf 100644 --- a/hermes_cli/model_switch.py +++ b/hermes_cli/model_switch.py @@ -1425,7 +1425,7 @@ def list_authenticated_providers( if hermes_id in _MODELS_DEV_PREFERRED: model_ids = _merge_with_models_dev(hermes_id, model_ids) total = len(model_ids) - top = model_ids[:max_models] if max_models else model_ids + top = model_ids[:max_models] if max_models is not None else model_ids slug = hermes_id pinfo = _mdev_pinfo(mdev_id) @@ -1588,7 +1588,7 @@ def list_authenticated_providers( if hermes_slug in _MODELS_DEV_PREFERRED: model_ids = _merge_with_models_dev(hermes_slug, model_ids) total = len(model_ids) - top = model_ids[:max_models] if max_models else model_ids + top = model_ids[:max_models] if max_models is not None else model_ids results.append({ "slug": hermes_slug, @@ -1663,7 +1663,7 @@ def list_authenticated_providers( if not _cp_model_ids: _cp_model_ids = curated.get(_cp.slug, []) _cp_total = len(_cp_model_ids) - _cp_top = _cp_model_ids[:max_models] if max_models else _cp_model_ids + _cp_top = _cp_model_ids[:max_models] if max_models is not None else _cp_model_ids results.append({ "slug": _cp.slug, @@ -1812,7 +1812,7 @@ def list_authenticated_providers( "name": "Custom endpoint", "is_current": True, "is_user_defined": True, - "models": _models[:max_models] if max_models else _models, + "models": _models[:max_models] if max_models is not None else _models, "total_models": len(_models), "source": "model-config", "api_url": str(current_base_url).strip().rstrip("/"), @@ -2082,7 +2082,7 @@ def list_picker_providers( except Exception: live_ids = list(p.get("models", [])) p = dict(p) - p["models"] = live_ids[:max_models] if max_models else live_ids + p["models"] = live_ids[:max_models] if max_models is not None else live_ids p["total_models"] = len(live_ids) has_models = bool(p.get("models")) diff --git a/tests/hermes_cli/test_model_catalog.py b/tests/hermes_cli/test_model_catalog.py index 7a1cbd54c30..b464fb046a9 100644 --- a/tests/hermes_cli/test_model_catalog.py +++ b/tests/hermes_cli/test_model_catalog.py @@ -423,6 +423,71 @@ class TestIntegrationWithModelsModule: assert nous_row is not None, "nous row must appear when authed" assert nous_row["models"] == expected + def test_picker_max_models_cap_semantics(self, tmp_path, monkeypatch): + """The cap argument has three distinct meanings on the real slicing + path: ``None`` = unlimited (the cap-removal fix, #48297), ``0`` = no + models (preserved for slug-only callers), an int N = first N. Guards + the ``is not None`` distinction the cap-removal follow-up introduced — + a ``if max_models`` (falsy) check would conflate ``0`` with unlimited. + """ + import importlib + from hermes_cli import model_catalog + from hermes_cli.models import get_curated_nous_model_ids + importlib.reload(model_catalog) + try: + from hermes_cli.model_switch import ( + list_authenticated_providers, + list_picker_providers, + ) + + active_home = Path(os.environ["HERMES_HOME"]) + (active_home / "auth.json").write_text( + json.dumps( + { + "providers": {"nous": {"access_token": "fake"}}, + "credential_pool": {}, + } + ) + ) + with patch.object( + model_catalog, "_fetch_manifest", return_value=_valid_manifest() + ), patch("hermes_cli.models.check_nous_free_tier", return_value=False), patch( + "hermes_cli.models.union_with_portal_free_recommendations", + side_effect=lambda ids, *a, **k: (ids, {}), + ), patch( + "hermes_cli.models.union_with_portal_paid_recommendations", + side_effect=lambda ids, *a, **k: (ids, {}), + ): + expected = get_curated_nous_model_ids() + full = list_picker_providers(current_provider="nous", max_models=None) + one = list_picker_providers(current_provider="nous", max_models=1) + # 0 is exercised on list_authenticated_providers (the slug-only + # path); the picker variant drops empty-model rows entirely, so + # the empty-list contract lives on the auth-providers call. + zero = list_authenticated_providers( + current_provider="nous", max_models=0 + ) + finally: + model_catalog.reset_cache() + + def _nous(rows): + return next((r for r in rows if r["slug"] == "nous"), None) + + # Only meaningful when the curated list actually exceeds 1 entry. + assert len(expected) > 1, "test needs a multi-model curated nous list" + + full_row = _nous(full) + assert full_row is not None and full_row["models"] == expected + + one_row = _nous(one) + assert one_row is not None and one_row["models"] == expected[:1] + + zero_row = _nous(zero) + # 0 means an empty model list — NOT unlimited. total_models still real. + assert zero_row is not None + assert zero_row["models"] == [] + assert zero_row["total_models"] == len(expected) + # ----------------------------------------------------------------------------- # Drift guard — prevent the in-repo curated lists from going out of sync with