Skip to content

Commit f166328

Browse files
routing show: a candidate is active only if its glob resolves against live model lists (#323)
* fix(routing show): a candidate is active only if its glob resolves live "Configured" is a property of the PROVIDER; a candidate's model is a GLOB that must ALSO match something that provider actually lists. `show` marked a candidate active on provider presence alone, so it claimed models the session could not serve. Measured 2026-09-07 in a DTU with only gpt-5.6-terra configured on openai: fast -> * openai / gpt-?.?-luna <- active # nothing would resolve this Fix: `_LiveModels` -- one memoized list_models() per DISTINCT matrix provider name, fetched lazily (only for candidates actually evaluated) and shared across roles, so a typical matrix costs a handful of calls. Alias-aware: a `provider: openai` candidate lists models from whichever openai-family backend is configured, canonical first -- the resolver's own preference. The resolved id is shown (`<- active (gpt-5.6-luna)`; `gpt-?.?-terra -> gpt-5.6-terra` in the table) so users see what will actually run. The id is chosen with the hook's own version-aware sort when importable, so display and runtime agree. Honest about what it cannot check. A provider whose list could not be fetched -- offline, no key, module not installed, empty list -- is UNVERIFIED, never "no match": it renders exactly as before this change and is named once in a footer. A glob that matches nothing a LISTED provider returns is "no model matches this glob" and the display falls through to the next candidate, as the resolver would. `_resolve_role` grows an optional `live` parameter; the `routing list` compatibility counts do not pass it and stay offline-fast and byte-identical. Two things found while making it real on this host: 1. WALL-CLOCK CAP. A provider with no usable credentials does not fail fast: provider-anthropic retries list_models() 5 times with backoff, measured at ~35s. `_list_models_bounded` runs each listing on a worker thread with a 6s cap (AMPLIFIER_ROUTING_LIST_TIMEOUT overrides); a miss is UNVERIFIED. Without this the routing test files went from 3s to 578s, and a user with one dead key would have waited half a minute per role. 2. `_list_models_for_provider` RETURNED REPRS, NOT IDS. ModelInfo carries `.id`; the helper read `getattr(m, "name", m)`, fell through to str(m), and returned `"id='gpt-5.6-terra' display_name=..."` -- a list that matches no glob. It had no caller until now, so it went unnoticed; the first live run showed "no model matches" for every openai, anthropic and gemini candidate on a host where all three resolve. Fixed to read `.id`. tests/conftest.py gains an autouse guard that stubs `_list_models_bounded` to "could not list" -- unit tests must never do live listing, and the stub makes every glob unverified, i.e. the pre-change output. The verification tests patch the same seam with an explicit table on top of it. Verified live on this host (15 provider instances): every openai glob resolves to its terra/luna id, gemini resolves, the whole detailed view takes 2.7s. Tests: tests/test_routing_show_verifies_globs.py (15) -- live match is active with the id; nothing-listed is no-match; exact names skip the lookup; unlistable is unverified and named; unconfigured never lists; picks the same id the hook would (suffix-free glob does not fall for -fast); one call per provider across roles; alias lists from the configured backend; _resolve_role falls through a no-match like the resolver, is unchanged without `live`, and yields None when nothing matches; details view marks no-match and promotes the next candidate; unverified renders as before plus footer; table shows the resolved id; the helper returns ids not reprs. Full suite 1932 passed in 19s, ruff clean on touched files. Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> * test(conftest): private MonkeyPatch in the autouse guard -- fixes Windows teardown The autouse `no_live_model_listing` guard requested the shared `monkeypatch` fixture. An autouse fixture is set up before every test's own fixtures, so that made `monkeypatch` set up BEFORE -- and torn down AFTER -- `temp_context_dirs` in tests/lib/mention_loading/test_resolver.py. Two tests there `monkeypatch.chdir()` into a TemporaryDirectory: teardown now deleted the directory while it was still the process cwd. Linux/macOS allow that; Windows raises PermissionError ("being used by another process"), deterministically, on both py3.11 and py3.12 (CI for #323, twice). Fix: a private `pytest.MonkeyPatch.context()` whose lifetime is this fixture's own. --setup-plan for test_resolver_cwd_file now shows the original order: TEARDOWN monkeypatch, then TEARDOWN temp_context_dirs. Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --------- Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
1 parent ea87c0c commit f166328

3 files changed

Lines changed: 568 additions & 15 deletions

File tree

‎amplifier_app_cli/commands/routing.py‎

Lines changed: 255 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
from __future__ import annotations
44

5+
import fnmatch
56
import logging
67
import re
78
from collections.abc import Mapping
@@ -447,18 +448,193 @@ def _check_compatibility(
447448
return covered, total
448449

449450

451+
_GLOB_CHARS = frozenset("*?[")
452+
453+
454+
def _is_glob(model: str) -> bool:
455+
return any(ch in model for ch in _GLOB_CHARS)
456+
457+
458+
def _best_glob_match(pattern: str, models: list[str]) -> str | None:
459+
"""The model id the routing hook would pick for *pattern* from *models*.
460+
461+
Reuses the hook's own version-aware sort when importable so the id shown
462+
here is the id that actually runs; falls back to a plain reverse sort
463+
(the same matcher, minus the date-suffix awareness) when it is not.
464+
"""
465+
lowered = pattern.lower()
466+
matched = [m for m in models if fnmatch.fnmatch(m.lower(), lowered)]
467+
if not matched:
468+
return None
469+
try:
470+
from amplifier_module_hooks_routing.resolver import _version_sort_key
471+
472+
return sorted(matched, key=_version_sort_key, reverse=True)[0]
473+
except Exception: # pragma: no cover - import environment dependent
474+
return sorted(matched, reverse=True)[0]
475+
476+
477+
# Wall-clock cap on ONE provider's live model listing, in seconds.
478+
#
479+
# Why a cap and not just "let it fail": a provider with no usable credentials
480+
# does not fail fast -- provider-anthropic, for one, retries list_models() up
481+
# to 5 times with backoff (its __init__.py: max_retries=5), which measured at
482+
# ~35s per provider. `show` is a display command; a user with one dead key
483+
# must not wait half a minute per role for it. Anything that misses the cap is
484+
# reported as UNVERIFIED, exactly like a provider that could not be listed at
485+
# all. Override with AMPLIFIER_ROUTING_LIST_TIMEOUT (seconds) when a slow but
486+
# healthy backend needs more.
487+
_LIVE_LIST_TIMEOUT_S = 6.0
488+
489+
490+
def _list_models_bounded(selector: str, settings: AppSettings) -> list[str]:
491+
"""`_list_models_for_provider` with a hard wall-clock cap.
492+
493+
Runs the listing on a worker thread and abandons it on timeout. The thread
494+
is left to finish on its own (``shutdown(wait=False)``) -- for a CLI that
495+
is about to exit that is the right trade, and it cannot leak into the
496+
process's own event loop because the listing helper builds its own.
497+
Returns ``[]`` on timeout, the same "could not list" value the helper
498+
itself uses for every other failure.
499+
"""
500+
import concurrent.futures
501+
import os
502+
503+
try:
504+
timeout = float(
505+
os.environ.get("AMPLIFIER_ROUTING_LIST_TIMEOUT", _LIVE_LIST_TIMEOUT_S)
506+
)
507+
except ValueError:
508+
timeout = _LIVE_LIST_TIMEOUT_S
509+
pool = concurrent.futures.ThreadPoolExecutor(max_workers=1)
510+
try:
511+
future = pool.submit(_list_models_for_provider, selector, settings)
512+
return future.result(timeout=timeout)
513+
except concurrent.futures.TimeoutError:
514+
logger.debug(
515+
"list_models() for %r exceeded %.1fs; treating as unverified",
516+
selector,
517+
timeout,
518+
)
519+
return []
520+
except Exception:
521+
return []
522+
finally:
523+
pool.shutdown(wait=False, cancel_futures=True)
524+
525+
526+
class _LiveModels:
527+
"""Memoized live ``list_models()`` per matrix provider name, for `show`.
528+
529+
WHY `show` NEEDS THIS. "Configured" is a property of the PROVIDER; a
530+
candidate's model is a GLOB that must also match something that provider
531+
actually lists. `show` used to mark a candidate active on provider presence
532+
alone, so it claimed models the session could not serve -- measured
533+
2026-09-07 in a DTU with only `gpt-5.6-terra` configured: `fast -> *
534+
openai / gpt-?.?-luna <- active`, a model nothing would resolve.
535+
536+
One ``list_models()`` per DISTINCT matrix provider name, fetched lazily
537+
(only for candidates that are actually evaluated) and memoized across
538+
roles, so a typical matrix costs a handful of calls, not one per candidate.
539+
Alias-aware: a `provider: openai` candidate lists models from whichever
540+
openai-family backend is configured, canonical first -- the same
541+
preference the resolver applies.
542+
543+
Honest about what it could not check: a provider whose list could not be
544+
fetched (offline, no key, module not installed, empty list) is UNVERIFIED,
545+
never "no match". Unverified candidates render exactly as before this
546+
class existed, and `show` names them once in a footer.
547+
"""
548+
549+
def __init__(self, settings: AppSettings) -> None:
550+
self._settings = settings
551+
self._cache: dict[str, list[str] | None] = {}
552+
self.unverified: set[str] = set()
553+
554+
def _configured_selector(self, provider: str) -> str | None:
555+
"""The configured provider (type or instance) a candidate name resolves to.
556+
557+
Canonical name first, then its family aliases -- mirrors the resolver.
558+
Checks against the RAW configured names (module types + instance ids),
559+
not the alias-expanded set `_get_configured_provider_types` returns:
560+
with only `openai-chatgpt` configured that set contains "openai" too,
561+
and listing models from a provider entry that does not exist would
562+
silently return [] and mark every openai candidate unverified.
563+
"""
564+
raw: set[str] = set()
565+
for p in self._settings.get_provider_overrides():
566+
module = str(p.get("module", ""))
567+
raw.add(module.removeprefix("provider-"))
568+
if p.get("id"):
569+
raw.add(str(p["id"]))
570+
for name in (provider, *_provider_family_aliases().get(provider, ())):
571+
if name in raw:
572+
return name
573+
return None
574+
575+
def models_for(self, provider: str) -> list[str] | None:
576+
if provider in self._cache:
577+
return self._cache[provider]
578+
selector = self._configured_selector(provider)
579+
models: list[str] | None = None
580+
if selector is not None:
581+
listed = _list_models_bounded(selector, self._settings)
582+
models = listed or None # [] means "could not list", not "lists nothing"
583+
if models is None:
584+
self.unverified.add(provider)
585+
self._cache[provider] = models
586+
return models
587+
588+
def resolve(self, provider: str, model: str) -> tuple[str, str | None]:
589+
"""Return ``(status, resolved_id)``.
590+
591+
status is ``"active"`` (an exact name, or a glob with a live match),
592+
``"no-match"`` (a glob that matches nothing the provider lists), or
593+
``"unverified"`` (the list could not be fetched -- treated as active,
594+
exactly as before, and named in the footer).
595+
"""
596+
if not _is_glob(model):
597+
# The resolver passes exact names straight to the API without
598+
# consulting list_models(); so does this.
599+
return "active", model
600+
models = self.models_for(provider)
601+
if models is None:
602+
return "unverified", None
603+
best = _best_glob_match(model, models)
604+
return ("active", best) if best else ("no-match", None)
605+
606+
def print_footer(self) -> None:
607+
if not self.unverified:
608+
return
609+
console.print(
610+
f"\n[dim]Model lists not fetched for: {', '.join(sorted(self.unverified))} "
611+
f"-- their globs are shown as configured but not verified live.[/dim]"
612+
)
613+
614+
450615
def _resolve_role(
451-
role_config: dict[str, Any], provider_types: set[str]
616+
role_config: dict[str, Any],
617+
provider_types: set[str],
618+
live: _LiveModels | None = None,
452619
) -> tuple[str | None, str | None]:
453620
"""Resolve a role to its first matching candidate.
454621
455622
Returns (model_pattern, provider_type) or (None, None) if unresolvable.
623+
With *live*, a configured candidate whose glob matches no live model is
624+
skipped -- the resolver would skip it too -- so the winner shown is the
625+
winner the session gets. Without *live* (the compatibility counts in
626+
`routing list`, which must stay offline-fast) this is provider-presence
627+
only, byte-identical to before.
456628
"""
457629
candidates = role_config.get("candidates", [])
458630
for candidate in candidates:
459631
provider = candidate.get("provider", "")
460-
if provider in provider_types:
461-
return candidate.get("model", "?"), provider
632+
if provider not in provider_types:
633+
continue
634+
model = candidate.get("model", "?")
635+
if live is not None and live.resolve(provider, model)[0] == "no-match":
636+
continue
637+
return model, provider
462638
return None, None
463639

464640

@@ -724,14 +900,42 @@ def _show_matrix_resolution(
724900
table.add_column("Model", style="green")
725901
table.add_column("Provider")
726902

727-
for role_name, role_config in roles.items():
728-
model, provider_type = _resolve_role(role_config, provider_types)
729-
if model and provider_type:
730-
table.add_row(role_name, model, provider_type)
731-
else:
732-
table.add_row(role_name, "[yellow]⚠ (no provider)[/yellow]", "[dim]-[/dim]")
903+
live = _LiveModels(settings)
904+
rows: list[tuple[str, str, str]] = []
905+
with console.status(
906+
"[dim]Checking globs against live model lists...[/dim]", spinner="dots"
907+
):
908+
for role_name, role_config in roles.items():
909+
model, provider_type = _resolve_role(role_config, provider_types, live)
910+
if model and provider_type:
911+
status, resolved = live.resolve(provider_type, model)
912+
shown = (
913+
f"{model} → {resolved}"
914+
if status == "active" and resolved and resolved != model
915+
else model
916+
)
917+
rows.append((role_name, shown, provider_type))
918+
elif any(
919+
c.get("provider", "") in provider_types
920+
for c in role_config.get("candidates", [])
921+
):
922+
# A provider IS configured; no candidate glob matches what it lists.
923+
rows.append(
924+
(
925+
role_name,
926+
"[yellow]⚠ (no matching model)[/yellow]",
927+
"[dim]-[/dim]",
928+
)
929+
)
930+
else:
931+
rows.append(
932+
(role_name, "[yellow]⚠ (no provider)[/yellow]", "[dim]-[/dim]")
933+
)
934+
for row in rows:
935+
table.add_row(*row)
733936

734937
console.print(table)
938+
live.print_footer()
735939

736940
# Show provider summary
737941
if provider_types:
@@ -776,6 +980,7 @@ def _show_matrix_details(
776980

777981
provider_types = _get_configured_provider_types(settings)
778982
roles = matrix_data.get("roles", {})
983+
live = _LiveModels(settings)
779984

780985
for role_name, role_config in roles.items():
781986
role_desc = role_config.get("description", "")
@@ -786,6 +991,7 @@ def _show_matrix_details(
786991

787992
candidates = role_config.get("candidates", [])
788993
winner_found = False
994+
any_configured = False
789995

790996
for candidate in candidates:
791997
provider = candidate.get("provider", "")
@@ -800,12 +1006,30 @@ def _show_matrix_details(
8001006
config_str = f" [dim]\\[{pairs}][/dim]"
8011007

8021008
is_configured = provider in provider_types
1009+
any_configured = any_configured or is_configured
1010+
status, resolved = (
1011+
live.resolve(provider, model)
1012+
if is_configured
1013+
else ("unconfigured", None)
1014+
)
8031015

804-
if is_configured and not winner_found:
1016+
if status == "no-match":
1017+
# The provider is here; nothing it lists matches this glob. The
1018+
# resolver would fall through, and so does this display.
1019+
line = (
1020+
f" [dim]✗ {provider} / {model}[/dim]"
1021+
f"{config_str} [yellow]no model matches this glob[/yellow]"
1022+
)
1023+
elif is_configured and not winner_found:
8051024
winner_found = True
1025+
tail = (
1026+
f" ({resolved})"
1027+
if status == "active" and resolved and resolved != model
1028+
else ""
1029+
)
8061030
line = (
8071031
f" [green]★ {provider} / {model}[/green]"
808-
f"{config_str} [green]← active[/green]"
1032+
f"{config_str} [green]← active{tail}[/green]"
8091033
)
8101034
elif is_configured:
8111035
line = f" [dim]✓ {provider} / {model}[/dim]{config_str}"
@@ -818,9 +1042,17 @@ def _show_matrix_details(
8181042
console.print(line)
8191043

8201044
if not winner_found:
821-
console.print(
822-
" [yellow]⚠ No configured provider can serve this role[/yellow]"
823-
)
1045+
if any_configured:
1046+
console.print(
1047+
" [yellow]⚠ A provider is configured but none of its models "
1048+
"matches any candidate glob[/yellow]"
1049+
)
1050+
else:
1051+
console.print(
1052+
" [yellow]⚠ No configured provider can serve this role[/yellow]"
1053+
)
1054+
1055+
live.print_footer()
8241056

8251057

8261058
# ============================================================
@@ -1246,7 +1478,15 @@ def _list_models_for_provider(
12461478
)
12471479
)
12481480
models = get_provider_models(provider_id, collected_config=collected_config)
1249-
return [str(getattr(m, "name", m)) for m in models]
1481+
# ModelInfo's identifier is `.id`, not `.name`. This helper had no
1482+
# caller until `_LiveModels` arrived, so the old `getattr(m, "name",
1483+
# m)` -- which fell through to str(m) and returned the whole dataclass
1484+
# repr (`"id='gpt-5.6-terra' display_name=..."`) -- went unnoticed: a
1485+
# list of reprs matches no glob, and every live-verified candidate
1486+
# showed "no model matches". Measured on this host, 2026-09-07.
1487+
return [
1488+
str(getattr(m, "id", None) or getattr(m, "name", None) or m) for m in models
1489+
]
12501490
except Exception:
12511491
return []
12521492

‎tests/conftest.py‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,37 @@
3232
# ---------------------------------------------------------------------------
3333

3434

35+
@pytest.fixture(autouse=True)
36+
def no_live_model_listing():
37+
"""`routing show` verifies candidate globs against live ``list_models()``.
38+
39+
Unit tests must never do that: a fixture provider with no credentials
40+
retries for ~35s before failing (measured: the routing test files went
41+
from 3s to 578s), and the result would depend on the network anyway. The
42+
default here is "could not list" -- every glob renders as unverified,
43+
which is byte-identical to the pre-verification output. Tests that
44+
exercise verification itself patch ``_list_models_bounded`` with an
45+
explicit table on top of this (tests/test_routing_show_verifies_globs.py).
46+
47+
Patched at the bounded wrapper, not at ``_list_models_for_provider``, so
48+
the tests that exercise THAT helper directly still see the real thing.
49+
50+
Uses a PRIVATE MonkeyPatch, deliberately not the shared ``monkeypatch``
51+
fixture. Requesting the shared one from an autouse fixture makes it set
52+
up before -- and therefore torn down after -- every test's own fixtures.
53+
Tests that ``monkeypatch.chdir()`` into a ``TemporaryDirectory`` then had
54+
that directory deleted while it was still the process cwd: fine on
55+
Linux/macOS, PermissionError on Windows, deterministically, in
56+
tests/lib/mention_loading/test_resolver.py. Measured on CI for #323.
57+
"""
58+
with pytest.MonkeyPatch.context() as mp:
59+
mp.setattr(
60+
"amplifier_app_cli.commands.routing._list_models_bounded",
61+
lambda _selector, _settings: [],
62+
)
63+
yield
64+
65+
3566
@pytest.fixture(autouse=True)
3667
def reset_skill_shortcuts():
3768
"""Clear SKILL_SHORTCUTS before and after every test in this suite."""

0 commit comments

Comments
 (0)