From 88236dbc26249ed2f6830bc92c43bf62fa414350 Mon Sep 17 00:00:00 2001
From: happy5318 <happy5318@users.noreply.github.com>
Date: Fri, 21 Aug 2026 00:15:44 +0800
Subject: [PATCH] docs(models): pin empty-allowlist semantics with regression
 coverage
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Greptile flagged that `models: []` / `models: "[]"` falls through the
non-empty allowlist gate and leaves the full live catalog selectable. The
symptom is real, but implementing the ask (gate on declared-ness instead)
measurably makes things worse: it emits an EMPTY picker and leaves the
provider unselectable.

Measured both branches against the real handler before deciding:

  models key absent   declared=False -> [chat-a, chat-b, image-gen-1, audio-tts-1]
  models: []          declared=True  -> []   EMPTY PICKER
  models: "[]"        declared=True  -> []   EMPTY PICKER
  models: [chat-a]    declared=True  -> [chat-a]

An unselectable provider is a harder failure than a few extra visible
models, and it is unrecoverable from the UI: `custom_providers` has no
WebUI write path, so it is hand-edited in config.yaml. There is also no
deny-all use case — a provider the user wants hidden is removed from
`custom_providers` outright, not left behind with an empty allowlist.

No production behavior change. Records the trade-off in the filter block
and adds `test_empty_allowlist_is_treated_as_not_configured` (native empty
list, serialized "[]", empty mapping, blank scalar) so a future "fix" of
this edge case fails loudly instead of silently emptying the picker.

Verified non-vacuous: applying the declared-ness gate fails all 4 new
cases with `assert [] == [...]`.
---
 api/routes.py                                 | 14 ++++++-
 ...test_issue3718_live_models_custom_probe.py | 37 +++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/api/routes.py b/api/routes.py
index 86a6769e..fc0c6f2f 100644
--- a/api/routes.py
+++ b/api/routes.py
@@ -22016,12 +22016,24 @@ def _handle_live_models(handler, parsed):
                 # providers (esp. New-API-style gateways) expose their ENTIRE
                 # catalog via /v1/models — including image/audio models that
                 # are not chat models.  The picker must not surface models the
-                # user never declared in config.yaml.  Only an explicit
+                # user never declared in config.yaml.  Only a NON-EMPTY explicit
                 # ``models`` allowlist gates the filter — a provider configured
                 # with just a singular ``model`` (no ``models`` list) keeps the
                 # unfiltered live catalog.  When no allowlist is configured,
                 # return the live list as-is (preserves the pre-filter
                 # discovery behaviour).
+                #
+                # An empty allowlist (``models: []``, ``models: "[]"``, or a
+                # value that decodes to no usable ids) is deliberately treated
+                # as "not configured", NOT as "allow nothing".  Gating on
+                # declared-ness instead would emit an EMPTY picker and make the
+                # provider unselectable — a harder failure than surfacing a few
+                # extra models, and unrecoverable from the UI because
+                # ``custom_providers`` is hand-edited in config.yaml (the WebUI
+                # has no write path for it).  ``[]`` in practice means a
+                # leftover/placeholder key, not an intentional deny-all, and no
+                # deny-all use case exists: a provider the user wants hidden is
+                # removed from ``custom_providers`` outright.
                 if ids:
                     if _allowlist_ids:
                         _allowlist_set = set(_allowlist_ids)
diff --git a/tests/test_issue3718_live_models_custom_probe.py b/tests/test_issue3718_live_models_custom_probe.py
index d5cab25e..39e5682d 100644
--- a/tests/test_issue3718_live_models_custom_probe.py
+++ b/tests/test_issue3718_live_models_custom_probe.py
@@ -192,3 +192,40 @@ def test_probe_failure_falls_back_to_config_ids(monkeypatch):
     )
 
     assert _ids(payload) == ["chat-default", "chat-a"]
+
+
+@pytest.mark.parametrize(
+    "models_value",
+    [
+        pytest.param([], id="native-empty-list"),
+        pytest.param("[]", id="serialized-empty-list"),
+        pytest.param({}, id="empty-mapping"),
+        pytest.param("   ", id="blank-scalar"),
+    ],
+)
+def test_empty_allowlist_is_treated_as_not_configured(monkeypatch, models_value):
+    """An empty ``models`` value must NOT be read as "allow nothing".
+
+    Pins a deliberate semantic decision. Gating the filter on *declared-ness*
+    rather than on a non-empty allowlist would make ``models: []`` emit an
+    EMPTY picker, leaving the provider unselectable — a harder failure than
+    surfacing a few extra models, and unrecoverable from the UI because
+    ``custom_providers`` has no WebUI write path (hand-edited config.yaml only).
+    There is also no deny-all use case: a provider the user wants hidden is
+    removed from ``custom_providers`` outright.
+
+    If someone later "fixes" the empty case into a deny-all gate, this test
+    fails and forces the trade-off to be re-argued rather than silently shipped.
+    """
+    import api.routes as routes
+
+    provider = dict(_BASE_PROVIDER, models=models_value)
+    payload, _ = _run_live_models(
+        monkeypatch, routes, provider, catalog=_LIVE_CATALOG
+    )
+
+    assert _ids(payload) == _LIVE_CATALOG, (
+        "empty allowlist must fall through to full live discovery, "
+        "never collapse the picker to zero models"
+    )
+    assert _ids(payload), "picker must never be emptied by an empty allowlist"