diff --git a/hermes_cli/models.py b/hermes_cli/models.py index c1f7d580f1..4e5f4507a6 100644 --- a/hermes_cli/models.py +++ b/hermes_cli/models.py @@ -2649,6 +2649,13 @@ def nous_policy_allowed_ids(*, force_refresh: bool = False) -> Optional[set[str] return set(pricing) or None +# Above this many reachable models, an allowed set is treated as catalog-wide +# rather than as an allowlist worth enumerating in a picker. NAS caps an +# allowlist at 512, but a set this large is indistinguishable from the full +# catalog for display purposes. +_NOUS_POLICY_APPEND_MAX = 64 + + def restrict_to_nous_policy( model_ids: list[str], allowed: Optional[set[str]] ) -> list[str]: @@ -2665,12 +2672,33 @@ def restrict_to_nous_policy( """ if not allowed: return list(model_ids) - return [ + kept = [ mid for mid in model_ids if mid in allowed or mid.split(":", 1)[0] in allowed ] + # An allowlist can admit models the curated manifest has never heard of, and + # intersecting alone would then leave the user with nothing to pick at all — + # strictly worse than the unfiltered list. When the reachable set is no + # larger than what would have been shown anyway, it IS the list: append + # whatever it admits that the curated list is missing. + # + # Bounded by size, which is what separates the two kinds of policy: a model + # allowlist is human-authored and small, while a provider-only policy leaves + # the whole catalog reachable. Appending several hundred alphabetical + # vendor-prefixed ids would bury the curated order — the regression the + # pickers' curated branch exists to avoid. Past the cap the intersection + # stands on its own, and the picker's custom-model entry remains the way to + # reach anything it omits. + if len(allowed) <= _NOUS_POLICY_APPEND_MAX: + covered: set[str] = set() + for mid in kept: + covered.add(mid) + covered.add(mid.split(":", 1)[0]) + kept.extend(sorted(a for a in allowed if a not in covered)) + return kept + def get_pricing_for_provider(provider: str, *, force_refresh: bool = False) -> dict[str, dict[str, str]]: """Return live pricing for providers that support it (openrouter, nous, ai-gateway, novita).""" diff --git a/tests/hermes_cli/test_nous_policy_filter.py b/tests/hermes_cli/test_nous_policy_filter.py index edb7e6dc20..7da078c77e 100644 --- a/tests/hermes_cli/test_nous_policy_filter.py +++ b/tests/hermes_cli/test_nous_policy_filter.py @@ -63,7 +63,11 @@ class TestRestrictToNousPolicy: ) == ["vendor/model:free"] def test_drops_a_free_sibling_whose_base_is_blocked(self): - assert restrict_to_nous_policy(["vendor/model:free"], {"other/model"}) == [] + """The blocked sibling goes; the model the org may actually use takes + its place rather than leaving the picker empty.""" + assert restrict_to_nous_policy(["vendor/model:free"], {"other/model"}) == [ + "other/model" + ] class TestNousPolicyAllowedIds: @@ -184,3 +188,35 @@ class TestNousPolicyNotice: notice = account_mod.nous_policy_notice() assert "/" not in notice, f"looks like it names a model: {notice}" assert len(notice.splitlines()) == 1 + + +class TestAllowlistOutsideTheCuratedList: + """An allowlist can name a model the curated manifest has never heard of. + + Intersecting alone leaves the picker empty in that case — strictly worse + than showing an unfiltered list, because the one model the org may use is + the one that got dropped. + """ + + def test_surfaces_an_allowed_model_the_curated_list_lacks(self): + assert restrict_to_nous_policy( + ["vendor/a", "vendor/b"], {"amazon/nova-2-lite-v1"} + ) == ["amazon/nova-2-lite-v1"] + + def test_keeps_curated_order_then_appends_the_rest(self): + kept = restrict_to_nous_policy( + ["z/curated", "a/curated"], {"z/curated", "a/curated", "new/model"} + ) + assert kept == ["z/curated", "a/curated", "new/model"] + + def test_does_not_append_a_free_sibling_already_covered(self): + assert restrict_to_nous_policy(["vendor/m:free"], {"vendor/m"}) == [ + "vendor/m:free" + ] + + def test_a_provider_only_policy_does_not_bury_the_curated_order(self): + """Such a policy leaves the whole catalog reachable; appending it would + drop hundreds of alphabetical ids into the picker.""" + curated = ["vendor/one", "vendor/two"] + catalog = {f"vendor/model-{i}" for i in range(300)} | set(curated) + assert restrict_to_nous_policy(curated, catalog) == curated