refactor(cli): route every aux picker through one provider-inventory seam

Adds build_aux_picker_rows() + format_aux_picker_entries() to
hermes_cli/inventory.py and converts both auxiliary-task pickers to them,
so custom providers, exclusions, and picker visibility can no longer be
dropped one call site at a time.

The two salvaged commits each fixed one kwarg at these same two sites
(user/custom providers, then for_picker). Both were per-site patches, so
the next aux picker would have reintroduced the gap. The seam makes the
correct behaviour the default a new caller cannot forget:

- user `providers:` and saved `custom_providers:` entries
- `model_catalog.excluded_providers`, matching /model
- exhausted-credential-pool providers stay visible
- only the active custom endpoint is probed, so an offline saved local
  server can't hang the picker
- the virtual `moa` row is excluded (auxiliary_client unwraps it to the
  aggregator anyway, so offering it is a silently-rewritten choice)

The vision picker also gains the current-selection marker it never had.

Also threads for_picker through build_models_payload, which had no way to
express it despite list_authenticated_providers supporting it.
This commit is contained in:
Hermes Agent 2026-07-25 22:25:03 -07:00 committed by Teknium
parent 9aefa4c61c
commit 92549c9a6e
4 changed files with 378 additions and 40 deletions

View file

@ -124,6 +124,7 @@ def build_models_payload(
refresh: bool = False,
probe_custom_providers: bool = True,
probe_current_custom_provider: bool = False,
for_picker: bool = False,
max_models: int | None = None,
) -> dict:
"""Build the ``{providers, model, provider}`` shape every consumer
@ -168,6 +169,11 @@ def build_models_payload(
false, still live-probe the current custom endpoint. This keeps normal
GUI/TUI picker opens fast while making the active custom provider's model
list match the classic CLI picker.
- ``for_picker``: interactive-picker visibility. Keeps providers whose
credential pool exists but is entirely rate-limited (exhausted) in the
list. Rate limits are per-model, so a different model under the same
provider may still work; hiding the provider strands the user. Set for
any surface a human is choosing from, not for programmatic resolution.
"""
from hermes_cli.model_switch import list_authenticated_providers
@ -182,6 +188,7 @@ def build_models_payload(
refresh=refresh,
probe_custom_providers=probe_custom_providers,
probe_current_custom_provider=probe_current_custom_provider,
for_picker=for_picker,
excluded_providers=ctx.excluded_providers or [],
)
@ -295,6 +302,94 @@ def build_model_options_payload(
)
# ─── Public: auxiliary-task pickers ─────────────────────────────────────
def build_aux_picker_rows(
*,
current_provider: str = "",
current_model: str = "",
current_base_url: str = "",
max_models: int | None = None,
) -> list[dict]:
"""Provider rows for any auxiliary-task picker (vision, compression, …).
THE entry point for every aux picker present and future. Call this
instead of ``list_authenticated_providers()`` directly.
Aux pickers kept re-deriving their own kwargs and each one silently
dropped a different slice of the user's configuration. Two independent
contributor PRs landed against the same two call sites for exactly this:
#52642 (user ``providers:`` / ``custom_providers:`` entries never
appeared) and #66624 (providers with an exhausted credential pool were
hidden). Both were per-site kwarg patches, so the next aux picker would
have reintroduced the same gap. Routing through one function makes the
correct behaviour the default that a new caller cannot forget:
- user-defined ``providers:`` and saved ``custom_providers:`` entries
- ``model_catalog.excluded_providers`` honoured, matching ``/model``
- exhausted-credential-pool providers stay visible (``for_picker``)
- the active custom endpoint is probed, offline saved ones are not, so
the picker never blocks on a dead local server
The virtual ``moa`` row is excluded: auxiliary tasks must not run the
MoA reference fan-out, and ``auxiliary_client`` unwraps a ``moa``
provider to its aggregator slot anyway (see ``_resolve_auto``), so
offering it here would be a choice silently rewritten behind the user's
back. Mirrors the same filter in ``hermes_cli/moa_cmd.py``.
Rows are the standard ``list_authenticated_providers`` shape. Pair with
:func:`format_aux_picker_entries` to render them.
"""
ctx = load_picker_context().with_overrides(
current_provider=current_provider,
current_model=current_model,
current_base_url=current_base_url,
)
rows = build_models_payload(
ctx,
for_picker=True,
probe_custom_providers=False,
probe_current_custom_provider=True,
max_models=max_models,
)["providers"]
return [r for r in rows if str(r.get("slug") or "").strip().lower() != "moa"]
def format_aux_picker_entries(
rows: list[dict],
*,
current_provider: str = "",
current_base_url: str = "",
) -> list[tuple[str, str, list[str]]]:
"""Render aux-picker rows as ``(slug, label, models)`` menu entries.
Owns the label text and the `` current`` marker so every aux picker
presents providers identically. Callers add their own leading/trailing
entries (``auto``, ``Custom endpoint``, ``Back``) around this list.
A custom endpoint set via a raw ``base_url`` is "current" only through
that URL never through a provider slug so when ``current_base_url``
is set no provider row is marked, matching the pre-existing behaviour of
both call sites.
"""
entries: list[tuple[str, str, list[str]]] = []
current_slug = str(current_provider or "").strip().lower()
has_base_url = bool(str(current_base_url or "").strip())
for row in rows:
slug = str(row.get("slug") or "")
name = row.get("name") or slug
total = row.get("total_models") or len(row.get("models") or [])
model_hint = f"{total} models" if total else ""
marker = (
" ← current"
if slug.lower() == current_slug and current_slug and not has_base_url
else ""
)
entries.append((slug, f"{name}{model_hint}{marker}", list(row.get("models") or [])))
return entries
def _apply_capabilities(rows: list[dict]) -> None:
"""Attach a ``{model: {fast, reasoning}}`` map to each provider row.

View file

@ -3626,18 +3626,19 @@ def _aux_config_menu() -> None:
def _aux_select_for_task(task: str) -> None:
"""Pick a provider + model for a single auxiliary task and persist it.
Uses ``list_authenticated_providers()`` to only show providers the user
has already configured. This avoids re-running OAuth/credential flows
inside the aux picker users set up new providers through the normal
``hermes model`` flow, then route aux tasks to them here.
Provider rows come from ``build_aux_picker_rows()`` the shared aux-picker
substrate so this surface shows exactly what every other aux picker
shows: authenticated built-ins, the user's own ``providers:`` /
``custom_providers:`` endpoints, and providers whose credential pool is
temporarily exhausted. Only already-configured providers appear; users set
up new ones through the normal ``hermes model`` flow, then route aux tasks
to them here.
"""
from hermes_cli.config import get_compatible_custom_providers, load_config
from hermes_cli.model_switch import list_authenticated_providers
from hermes_cli.config import load_config
from hermes_cli.inventory import build_aux_picker_rows, format_aux_picker_entries
cfg = load_config()
aux = cfg.get("auxiliary", {}) if isinstance(cfg.get("auxiliary"), dict) else {}
user_providers = cfg.get("providers") if isinstance(cfg.get("providers"), dict) else {}
custom_providers = get_compatible_custom_providers(cfg)
task_cfg = aux.get(task, {}) if isinstance(aux.get(task), dict) else {}
current_provider = str(task_cfg.get("provider") or "auto").strip() or "auto"
current_model = str(task_cfg.get("model") or "").strip()
@ -3647,18 +3648,10 @@ def _aux_select_for_task(task: str) -> None:
# Gather authenticated providers (has credentials + curated model list)
try:
providers = list_authenticated_providers(
providers = build_aux_picker_rows(
current_provider=current_provider,
current_model=current_model,
current_base_url=current_base_url,
user_providers=user_providers,
custom_providers=custom_providers,
# Interactive picker: also show providers whose credential pool is
# entirely rate-limited (exhausted). Rate limits are per-model, and
# this persists an aux-task config the user will use later once the
# cooldown clears, so hiding the provider here is wrong — same
# rationale as the /model picker (#66584).
for_picker=True,
)
except Exception as exc:
print(f"Could not detect authenticated providers: {exc}")
@ -3671,16 +3664,13 @@ def _aux_select_for_task(task: str) -> None:
)
entries.append(("__auto__", f"auto (recommended){auto_marker}", []))
for p in providers:
slug = p.get("slug", "")
name = p.get("name") or slug
total = p.get("total_models", 0)
models = p.get("models") or []
model_hint = f"{total} models" if total else ""
marker = (
" ← current" if slug == current_provider and not current_base_url else ""
entries.extend(
format_aux_picker_entries(
providers,
current_provider=current_provider,
current_base_url=current_base_url,
)
entries.append((slug, f"{name}{model_hint}{marker}", list(models)))
)
# Custom endpoint (raw base_url)
custom_marker = " ← current" if current_base_url else ""

View file

@ -3810,22 +3810,33 @@ def _configure_vision_backend() -> None:
def _configure_vision_provider_model(config: dict, vision_cfg: dict) -> None:
"""Provider + model picker for vision, mirroring the ``/model`` surface.
Lists authenticated providers (same data source as the model switcher),
lets the user pick one and then a model from its curated list (or type a
custom id), and persists ``auxiliary.vision.provider`` + ``.model``.
Provider rows come from ``build_aux_picker_rows()`` the shared aux-picker
substrate so this picker lists exactly what the ``hermes model`` aux-task
picker lists, including the user's own ``providers:`` / ``custom_providers:``
endpoints. Lets the user pick a provider and then a model from its curated
list (or type a custom id), and persists ``auxiliary.vision.provider`` +
``.model``.
"""
try:
from hermes_cli.model_switch import list_authenticated_providers
from hermes_cli.inventory import (
build_aux_picker_rows,
format_aux_picker_entries,
)
except Exception as exc: # pragma: no cover - import guard
_print_warning(f" Could not load provider list: {exc}")
return
current_provider = str(vision_cfg.get("provider") or "").strip()
current_model = str(vision_cfg.get("model") or "").strip()
current_base_url = str(vision_cfg.get("base_url") or "").strip()
try:
# Interactive picker: include providers whose credential pool is
# entirely rate-limited (exhausted) so the user can still route vision
# here — rate limits are per-model and this persists a config used
# later. Same rationale as the /model picker (#66584).
providers = list_authenticated_providers(max_models=40, for_picker=True)
providers = build_aux_picker_rows(
current_provider=current_provider,
current_model=current_model,
current_base_url=current_base_url,
max_models=40,
)
except Exception as exc:
_print_warning(f" Could not detect providers: {exc}")
providers = []
@ -3837,11 +3848,14 @@ def _configure_vision_provider_model(config: dict, vision_cfg: dict) -> None:
)
return
provider_labels = []
for p in providers:
name = p.get("name") or p.get("slug")
total = p.get("total_models", len(p.get("models", [])))
provider_labels.append(f"{name} ({total} models)" if total else str(name))
provider_labels = [
label
for _slug, label, _models in format_aux_picker_entries(
providers,
current_provider=current_provider,
current_base_url=current_base_url,
)
]
provider_labels.append("Cancel")
pidx = _prompt_choice(" Choose vision provider:", provider_labels, 0)

View file

@ -0,0 +1,239 @@
"""Auxiliary-task pickers share one provider-inventory substrate.
Every aux picker (``hermes model`` Configure auxiliary models, the
``hermes tools`` vision picker, and any future one) must route through
``hermes_cli.inventory.build_aux_picker_rows()`` so it shows the same
provider universe as ``/model``.
Two independent contributor PRs fixed the same two call sites for exactly
this reason:
- #52642 (@deepjia) — user ``providers:`` / ``custom_providers:`` entries
were invisible because the aux picker never forwarded them.
- #66624 (@Drexuxux) — providers with a fully rate-limited credential pool
were hidden because the aux picker never forwarded ``for_picker``.
Both were per-call-site kwarg patches, so the next aux picker would have
reintroduced the gap. These tests pin the *shared substrate* behaviour and
guard the seam itself, not the kwargs at any one site.
"""
from pathlib import Path
from unittest.mock import patch
import pytest
import yaml
CONFIG = {
"model": {"provider": "openrouter", "default": "anthropic/claude-opus-4.6"},
"model_catalog": {"excluded_providers": ["copilot"]},
"providers": {
"my-llm": {
"name": "My LLM",
"base_url": "https://myllm.example.com/v1",
"key_env": "MYLLM_KEY",
"discover_models": False,
"models": {"big-model": {}, "small-model": {}},
}
},
"custom_providers": [
{
"name": "Legacy Box",
"base_url": "https://legacy.example.com/v1",
"key_env": "LEGACY_KEY",
"model": "legacy-1",
"discover_models": False,
}
],
}
@pytest.fixture
def configured_home(tmp_path, monkeypatch):
"""A HERMES_HOME with one ``providers:`` entry and one legacy
``custom_providers:`` entry, both credentialled via env."""
home = tmp_path / ".hermes"
home.mkdir()
(home / "config.yaml").write_text(yaml.safe_dump(CONFIG))
monkeypatch.setattr(Path, "home", lambda: tmp_path)
monkeypatch.setenv("HERMES_HOME", str(home))
monkeypatch.setenv("OPENROUTER_API_KEY", "sk-or-test")
monkeypatch.setenv("MYLLM_KEY", "sk-mine")
monkeypatch.setenv("LEGACY_KEY", "sk-legacy")
return home
# ─── The substrate contract ─────────────────────────────────────────────
def test_aux_picker_surfaces_user_defined_providers(configured_home):
"""Both config schemas for a user's own endpoint reach an aux picker.
This is #52642's bug: the aux picker built its own kwargs and passed
neither ``user_providers`` nor ``custom_providers``, so a user who had
configured their own endpoint could not route any auxiliary task to it.
"""
from hermes_cli.inventory import build_aux_picker_rows
slugs = {r["slug"] for r in build_aux_picker_rows()}
assert "my-llm" in slugs, (
"a keyed providers: entry must be selectable for auxiliary tasks"
)
assert "custom:legacy-box" in slugs, (
"a legacy custom_providers: entry must be selectable for auxiliary tasks"
)
def test_aux_picker_carries_configured_models_for_user_provider(configured_home):
"""A user provider arrives with its configured model list, so the
follow-up model prompt has something to offer."""
from hermes_cli.inventory import build_aux_picker_rows
row = next(r for r in build_aux_picker_rows() if r["slug"] == "my-llm")
assert set(row["models"]) == {"big-model", "small-model"}
def test_aux_picker_honors_excluded_providers(configured_home):
"""``model_catalog.excluded_providers`` applies to aux pickers too.
A provider the user hid from ``/model`` must not reappear in an aux
picker the exclusion is about the provider, not about one surface.
"""
from hermes_cli.inventory import build_aux_picker_rows
slugs = {str(r["slug"]).lower() for r in build_aux_picker_rows()}
assert "copilot" not in slugs
def test_aux_picker_omits_virtual_moa_row(configured_home):
"""MoA is not a real endpoint and auxiliary_client unwraps it to the
aggregator slot, so offering it in an aux picker would be a selection
silently rewritten behind the user's back."""
from hermes_cli.inventory import build_aux_picker_rows
cfg = dict(CONFIG)
cfg["moa"] = {"presets": {"opus-gpt": {}}}
(configured_home / "config.yaml").write_text(yaml.safe_dump(cfg))
slugs = {str(r["slug"]).lower() for r in build_aux_picker_rows()}
assert "moa" not in slugs
def test_aux_picker_requests_exhausted_pool_visibility(configured_home):
"""#66624: a provider whose credential pool is entirely rate-limited
must stay visible. Rate limits are per-model and the aux picker writes a
config the user runs later, once the cooldown has cleared."""
from hermes_cli import inventory
seen = {}
def _capture(**kwargs):
seen.update(kwargs)
return []
with patch("hermes_cli.model_switch.list_authenticated_providers", _capture):
inventory.build_aux_picker_rows()
assert seen.get("for_picker") is True
def test_aux_picker_does_not_block_on_offline_saved_endpoints(configured_home):
"""Saved custom endpoints are not live-probed on open (a dead local
server would hang the picker); only the active one is."""
from hermes_cli import inventory
seen = {}
def _capture(**kwargs):
seen.update(kwargs)
return []
with patch("hermes_cli.model_switch.list_authenticated_providers", _capture):
inventory.build_aux_picker_rows()
assert seen.get("probe_custom_providers") is False
assert seen.get("probe_current_custom_provider") is True
# ─── Shared rendering ───────────────────────────────────────────────────
def test_format_entries_marks_current_provider():
from hermes_cli.inventory import format_aux_picker_entries
rows = [
{"slug": "my-llm", "name": "My LLM", "models": ["a", "b"], "total_models": 2},
{"slug": "openrouter", "name": "OpenRouter", "models": ["x"], "total_models": 1},
]
entries = format_aux_picker_entries(rows, current_provider="my-llm")
assert entries[0] == ("my-llm", "My LLM — 2 models ← current", ["a", "b"])
assert entries[1] == ("openrouter", "OpenRouter — 1 models", ["x"])
def test_format_entries_base_url_owns_current_marker():
"""When the task points at a raw base_url, the current selection is the
URL no provider row may claim the marker."""
from hermes_cli.inventory import format_aux_picker_entries
rows = [{"slug": "my-llm", "name": "My LLM", "models": ["a"], "total_models": 1}]
entries = format_aux_picker_entries(
rows, current_provider="my-llm", current_base_url="https://elsewhere/v1"
)
assert "← current" not in entries[0][1]
# ─── Seam guard ─────────────────────────────────────────────────────────
def test_aux_pickers_route_through_the_shared_substrate(configured_home):
"""Neither aux picker may call ``list_authenticated_providers`` directly.
This is the regression that produced #52642 and #66624 as two separate
contributor PRs against the same two call sites: a picker calling the
low-level function re-derives its own kwargs and drops whichever slice
of user config the author didn't think about. Both pickers must reach
the provider list only through ``build_aux_picker_rows``.
"""
import hermes_cli.main as main
import hermes_cli.tools_config as tools_config
direct_calls = []
substrate_calls = []
def _direct(**kwargs):
direct_calls.append(kwargs)
return []
def _substrate(**kwargs):
substrate_calls.append(kwargs)
return []
# Abort each picker at its first prompt, right after it has asked for its
# provider list. The vision picker returns early on empty rows; the
# aux-task picker still renders auto/custom/back, so cancel its menu.
with (
patch("hermes_cli.model_switch.list_authenticated_providers", _direct),
patch("hermes_cli.inventory.build_aux_picker_rows", _substrate),
patch("hermes_cli.main._prompt_provider_choice", return_value=None),
):
main._aux_select_for_task("compression")
tools_config._configure_vision_provider_model({}, {})
assert len(substrate_calls) == 2, (
"both the aux-task picker and the vision picker must source providers "
f"from build_aux_picker_rows (got {len(substrate_calls)} calls)"
)
assert direct_calls == [], (
"aux pickers must not call list_authenticated_providers directly — "
"route through hermes_cli.inventory.build_aux_picker_rows so custom "
"providers, exclusions, and picker visibility stay consistent"
)