fix(models): dict-shaped models is per-model metadata, not an allowlist (re-gate 9/23)
Re-review #7165 (nesquena-hermes 2026-09-23T06:21:57Z, CHANGES_REQUESTED). CORE: a dict-shaped custom_providers[].models mapping (what Hermes Agent's setup flow writes, e.g. {chat-a: {context_length: 128000}}) was treated as an allowlist by iterating its keys, collapsing the live /v1/models catalog to a single saved default while the CLI live-probed the full list. The Agent defines exactly this contract in hermes_cli/model_switch.py::_models_config_is_allowlist: a mapping is per-model metadata, NOT a catalog narrow. Fix (all 3 spec points): 1. api/routes.py::_custom_provider_allowlist_ids: added _custom_provider_models_discover_is_false(_cp) (mirrors api.config._provider_discover_allowed / Agent model_switch_providers.py:557: bool False or string false/no/0, case-insensitive). In the dict branch, when discovery is NOT explicitly false, return no allowlist so the live probe returns the full catalog. Only discover_models: false opts into treating dict keys as a pinned allowlist. List / JSON-array-string / Python-literal shapes stay plain allowlists. 2. tests/test_issue3718_live_models_custom_probe.py: removed the mapping-metadata param that asserted the regression; added two real-handler tests: test_dict_models_without_discover_false_does_not_gate (dict -> full _LIVE_CATALOG) and test_dict_models_with_discover_false_pins_to_keys (bool False / "false" / "no" / "0" -> pinned to keys). 3. docs/architecture/live-models-allowlist.md: documented that list and string shapes filter, and a dict filters only with discover_models: false. Companion test update (blocker reported by the fix itself): tests/test_byok_model_dropdown.py rightcode-codex entry encoded the removed dict-as-allowlist behavior (models: {"gpt-5.5-mini": {}} asserting the response collapsed to just that key). Added discover_models: False to preserve the provider-scoping intent (no sibling leak) under the new contract. Red/green verified: removing the dict-branch guard turns test_dict_models_without_discover_false_does_not_gate red (collapsed catalog); restoring it turns it green. Verification: 3718 18 passed; byok 24 passed; neighbour sweep (46 files) 520 passed (2 pre-existing order-dependent flakes reproduce identically on clean HEAD, verified via git stash); ruff curated gate OK. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
ff8e75c0be
commit
0fcd584274
4 files changed
+126
-5
No files matched your search
@@ -291,6 +291,13 @@ class TestLiveModelsCustomProviderFallback:
|
||||
"model": "gpt-5.5",
|
||||
"models": {"gpt-5.5-mini": {}},
|
||||
"base_url": "https://right.codes/codex/v1",
|
||||
# dict-shaped ``models`` is per-model metadata (Agent setup
|
||||
# flow) and only narrows the live catalog when discovery is
|
||||
# explicitly off — the new #7165 contract. This test still
|
||||
# wants provider-scoping (no sibling leak), so pin the
|
||||
# catalog via ``discover_models: false`` instead of relying
|
||||
# on the removed dict-as-allowlist behavior.
|
||||
"discover_models": False,
|
||||
},
|
||||
{
|
||||
"name": "infini-ai",
|
||||
|
||||
@@ -14,8 +14,12 @@ Covered shapes:
|
||||
* malformed serialized ``models`` → safe, treated as scalar name
|
||||
* allowlisted model absent from live catalog → appended
|
||||
* live probe failure → falls back to config ids
|
||||
* dict-shaped ``models`` (Agent setup) → live catalog NOT gated (per-model
|
||||
metadata, not an allowlist)
|
||||
* dict-shaped ``models`` + ``discover_models:
|
||||
false`` → pinned to dict keys (explicit opt-out)
|
||||
* ``models_discovered: true`` catalog → live catalog NOT gated (discovery
|
||||
metadata is not an allowlist)
|
||||
metadata is not an allowlist)
|
||||
* discovered + ``discover_models: false`` → still pinned (explicit opt-out)
|
||||
"""
|
||||
|
||||
@@ -116,7 +120,6 @@ _BASE_PROVIDER = {
|
||||
pytest.param(["chat-a", "chat-b"], id="native-list"),
|
||||
pytest.param('["chat-a","chat-b"]', id="json-array-string"),
|
||||
pytest.param("['chat-a', 'chat-b']", id="python-literal-string"),
|
||||
pytest.param({"chat-a": {}, "chat-b": {}}, id="mapping-metadata"),
|
||||
pytest.param(
|
||||
[{"id": "chat-a"}, {"model": "chat-b"}], id="list-of-dicts"
|
||||
),
|
||||
@@ -143,6 +146,57 @@ def test_allowlist_filters_live_catalog(monkeypatch, models_value):
|
||||
assert "audio-tts-1" not in _ids(payload)
|
||||
|
||||
|
||||
def test_dict_models_without_discover_false_does_not_gate(monkeypatch):
|
||||
"""A dict-shaped ``models`` is per-model metadata — NOT an allowlist (#7165).
|
||||
|
||||
Hermes Agent's setup flow and ``hermes_cli/model_switch.py::_save_custom_provider``
|
||||
persist per-model metadata as a mapping, e.g. ``{chat-a: {context_length: 128000}}``.
|
||||
Treating its keys as an allowlist would collapse the live picker to the saved
|
||||
default while the CLI live-probe shows the full catalog. Without an explicit
|
||||
``discover_models: false`` opt-out the dict must NOT gate — full live catalog wins.
|
||||
"""
|
||||
import api.routes as routes
|
||||
|
||||
provider = dict(
|
||||
_BASE_PROVIDER,
|
||||
models={"chat-a": {"context_length": 128000}, "chat-b": {"context_length": 32000}},
|
||||
)
|
||||
payload, requested = _run_live_models(
|
||||
monkeypatch, routes, provider, catalog=_LIVE_CATALOG
|
||||
)
|
||||
|
||||
assert requested, "live /v1/models probe was never attempted"
|
||||
assert _ids(payload) == _LIVE_CATALOG, (
|
||||
"dict-shaped models is per-model metadata, not an allowlist — the "
|
||||
"full live catalog must be surfaced"
|
||||
)
|
||||
|
||||
|
||||
def test_dict_models_with_discover_false_pins_to_keys(monkeypatch):
|
||||
"""``discover_models: false`` opts a dict-shaped ``models`` into an allowlist.
|
||||
|
||||
The explicit opt-out (bool ``False`` or the string forms ``"false"``/``"no"``/``"0"``)
|
||||
means the dict keys are a hand-pinned allowlist and must filter the live catalog.
|
||||
"""
|
||||
import api.routes as routes
|
||||
|
||||
for discover in (False, "false", "no", "0"):
|
||||
provider = dict(
|
||||
_BASE_PROVIDER,
|
||||
models={"chat-a": {"context_length": 128000}, "chat-b": {"context_length": 32000}},
|
||||
discover_models=discover,
|
||||
)
|
||||
payload, requested = _run_live_models(
|
||||
monkeypatch, routes, provider, catalog=_LIVE_CATALOG
|
||||
)
|
||||
assert requested, "live /v1/models probe was never attempted"
|
||||
assert _ids(payload) == ["chat-a", "chat-b"], (
|
||||
f"discover_models={discover!r} must pin the dict keys as the allowlist"
|
||||
)
|
||||
assert "image-gen-1" not in _ids(payload)
|
||||
assert "audio-tts-1" not in _ids(payload)
|
||||
|
||||
|
||||
def test_singular_model_only_does_not_gate_live_catalog(monkeypatch):
|
||||
"""``model:`` is default metadata, not an allowlist — keep full discovery."""
|
||||
import api.routes as routes
|
||||
|
||||
Reference in new issue
Block a user