diff --git a/docs/USAGE.md b/docs/USAGE.md index adfd221..c4fbfed 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -254,6 +254,27 @@ agent-harness run --repo owner/name --work ./target-repo \ --endpoint https://api.your-gateway.example --check 'pytest -q' ``` +**A role may name several models, in preference order.** The first that +answers does the work; the others are tried only when it will not: + +```bash + --implementer deepseek-v4-flash,glm-5.2,gpt-5.4 --reviewer gpt-5.6 +``` + +This is not load spreading, it is availability. Measured on one gateway, 34 of +42 advertised models were unavailable simultaneously — a role with a single +name is a fleet that stops when that name is down. The whole chain is tried +before any backoff, because a model that is down answers immediately and +sleeping on it first would waste the alternatives entirely; the event stream +records which model actually answered, so a fleet quietly running on its third +choice says so. + +Two bounds worth knowing. A chain protects against a *model* being +unavailable, not against running out of budget: a spend cap belongs to the +account, so it parks every model behind that endpoint. And `/api/roles`, +readiness and the independence warning all report the *preferred* route — a +fallback that has not been needed is not what you configured. + ### What it guarantees either way - **Cheap checks run before the reviewer.** Paying a model to tell you the diff --git a/src/agent_harness/__main__.py b/src/agent_harness/__main__.py index 98a8d39..d7616cb 100644 --- a/src/agent_harness/__main__.py +++ b/src/agent_harness/__main__.py @@ -172,7 +172,7 @@ def _run(args: argparse.Namespace) -> int: from .executor import Checks, Executor from .github import GitHub - from .model_client import ModelClient, Route, routes_from_map + from .model_client import Chain, ModelClient, chains_from_map from .work import RUNNING, WorkQueue, WorkRecord # With a session host the CLI agent does the implementing, so only the @@ -263,8 +263,21 @@ def emit(event: dict[str, Any]) -> None: # Seed the shared map from the command line, then read it back per call so # `PUT /api/roles` takes effect without a restart. + # A role may name several models, comma-separated and in preference + # order. The first that answers does the work; the rest exist because on + # this endpoint 34 of 42 advertised models were unavailable at once, and a + # fleet with one name per role simply stops when that name is down. from_cli = { - name: {"model": model, "endpoint": args.endpoint, "provider": "claw-bay"} + name: { + # Both, on purpose: `models` is the chain, `model` is the + # preferred one, so a reader that predates fallbacks -- including + # the `RoleRoute` wire schema -- still sees a route rather than a + # role it thinks is unconfigured. + "models": (names := [m.strip() for m in model.split(",") if m.strip()]), + "model": names[0] if names else "", + "endpoint": args.endpoint, + "provider": "claw-bay", + } for name, model in roles.items() } stored_map = queue.get_setting(ROLE_MAP_KEY) or {} @@ -291,8 +304,8 @@ def emit(event: dict[str, Any]) -> None: if stored_map and filled: print(f"note: the stored role map had no route for {', '.join(filled)}; used the flags.") - def live_routes() -> dict[str, Route]: - return routes_from_map(queue.get_setting(ROLE_MAP_KEY) or {}, api_key=api_key) + def live_routes() -> dict[str, Chain]: + return chains_from_map(queue.get_setting(ROLE_MAP_KEY) or {}, api_key=api_key) # Nothing claims work until every role this run needs can be routed. The # alternative is finding out on the first model call -- after the project @@ -751,7 +764,7 @@ def _fleet_for_serve( from .events import KINDS, MODEL_CALL, Event from .fleet import Fleet from .github import GitHub - from .model_client import ModelClient, Route, effective_routes, routes_from_map + from .model_client import Chain, ModelClient, chains_from_map, effective_routes from .runtime import ExecutorRoles, session_executor_factory from .session_executor import AgentSpec from .session_host import HttpSessionHost @@ -774,10 +787,10 @@ def _fleet_for_serve( } queue.set_setting(ROLE_MAP_KEY, stored) - def live_routes() -> dict[str, Route]: - return routes_from_map(queue.get_setting(ROLE_MAP_KEY) or {}, api_key=api_key) + def live_routes() -> dict[str, Chain]: + return chains_from_map(queue.get_setting(ROLE_MAP_KEY) or {}, api_key=api_key) - def routes_for(project_id: str) -> dict[str, Route]: + def routes_for(project_id: str) -> dict[str, Chain]: """One project's effective map, read live on every call. The project row is read here rather than closed over so that a role @@ -787,7 +800,7 @@ def routes_for(project_id: str) -> dict[str, Route]: project = queue.get_project(project_id) return effective_routes( live_routes(), - routes_from_map(getattr(project, "roles", None) or {}, api_key=api_key), + chains_from_map(getattr(project, "roles", None) or {}, api_key=api_key), ) routes = live_routes() diff --git a/src/agent_harness/model_client.py b/src/agent_harness/model_client.py index a9e08ae..dafea00 100644 --- a/src/agent_harness/model_client.py +++ b/src/agent_harness/model_client.py @@ -195,6 +195,33 @@ class Route: options: Mapping[str, Any] = field(default_factory=dict) +#: One role's routes, in the order they are tried. A single `Route` is the +#: one-element case, which is why almost nothing outside this module had to +#: change: `route_for` still answers with the preferred one. +Chain = tuple[Route, ...] + + +def _as_chain(value: Route | Sequence[Route]) -> Chain: + return (value,) if isinstance(value, Route) else tuple(value) + + +def _chain_names(chain: Chain) -> str: + """The chain as an operator reads it: which models, in which order.""" + return ", ".join(f"{route.model} via {route.endpoint}" for route in chain) + + +def _fell_back(chain: Chain, used: Route) -> str | None: + """Said out loud when a call was served by anything but the first choice. + + A fleet quietly running on its third-choice model for a week is a fleet + whose costs and results nobody can explain, so the event stream records + which one answered rather than only that something did. + """ + if used is chain[0]: + return None + return f"fell back to {used.model} (preferred {chain[0].model})" + + @dataclass class Response: status: int @@ -218,21 +245,61 @@ def routes_from_map( """ routes: dict[str, Route] = {} for name, spec in (stored or {}).items(): - model, endpoint = spec.get("model"), spec.get("endpoint") - if not model or not endpoint: - continue - routes[name] = Route( - str(model), - str(endpoint), - P.PROVIDERS.get(str(spec.get("provider", "")), default_provider), - api_key=api_key, - ) + chain = _chain_from_spec(spec, api_key=api_key, default_provider=default_provider) + if chain: + routes[name] = chain[0] return routes -def effective_routes( - global_routes: Mapping[str, Route], project_routes: Mapping[str, Route] | None -) -> dict[str, Route]: +def chains_from_map( + stored: Mapping[str, Mapping[str, Any]] | None, + *, + api_key: str | None = None, + default_provider: Provider = P.CLAW_BAY, +) -> dict[str, Chain]: + """The persisted role map, as fallback chains. + + Same source as `routes_from_map`, which answers with each role's preferred + route for everything that *reports* on configuration. This one is what the + client calls with, because the second and third choices only matter at the + moment the first will not answer. + """ + chains: dict[str, Chain] = {} + for name, spec in (stored or {}).items(): + chain = _chain_from_spec(spec, api_key=api_key, default_provider=default_provider) + if chain: + chains[name] = chain + return chains + + +def _chain_from_spec( + spec: Mapping[str, Any], *, api_key: str | None, default_provider: Provider +) -> Chain: + """One role's stored spec as an ordered chain. + + Accepts `model` as a single name or as a list, so a map written before + fallbacks existed still reads correctly and a role that names one model is + not forced into list syntax. A role missing a model or an endpoint is + dropped rather than half-built: it is not a route, and preflight's job is + to name it as missing rather than to fail on the first call that uses it. + """ + endpoint = spec.get("endpoint") + models = spec.get("models") or spec.get("model") + if not endpoint or not models: + return () + if isinstance(models, str): + models = [models] + provider = P.PROVIDERS.get(str(spec.get("provider", "")), default_provider) + return tuple( + Route(str(model), str(endpoint), provider, api_key=api_key) + for model in models + if str(model).strip() + ) + + +def effective_routes[R]( + global_routes: Mapping[str, R], project_routes: Mapping[str, R] | None +) -> dict[str, R]: """The global role map with one project's overrides applied. Per role, not wholesale. Choosing one map or the other was the defect: @@ -314,7 +381,7 @@ class ModelClient: def __init__( self, - roles: Mapping[str, Route], + roles: Mapping[str, Route | Sequence[Route]], transport: Transport, policy: RetryPolicy | None = None, on_event: Callable[[dict[str, Any]], None] | None = None, @@ -324,9 +391,14 @@ def __init__( jitter: Callable[[], float] = random.random, parks: EndpointParks | None = None, run_id: str | None = None, - routes_provider: Callable[[], Mapping[str, Route]] | None = None, + routes_provider: Callable[[], Mapping[str, Route | Sequence[Route]]] | None = None, ) -> None: - self.roles = dict(roles) + # Either form, on purpose: this is a public attribute that callers and + # tests assign to, and a bare `Route` put there by hand is the + # one-element chain it looks like. Normalised on read. + self.roles: dict[str, Route | Chain] = { + name: _as_chain(value) for name, value in roles.items() + } # Consulted per call when set, so the role -> model map can be changed # while the fleet is running. The call site names a ROLE and never a # model, which is the whole reason that is possible; a provider lets @@ -359,9 +431,17 @@ def reviewer_independence(self, implemented_by: str = "") -> tuple[bool, str]: effective map -- which this client's own map may not be -- gets the same answer from the same code. """ - return reviewer_independence(self.roles, implemented_by=implemented_by) - - def routed_by(self, routes_provider: Callable[[], Mapping[str, Route]]) -> ModelClient: + # The preferred route per role: a fallback that has not been needed + # is not what the operator configured, and is not what they should be + # told about their reviewer. + preferred = { + name: _as_chain(value)[0] for name, value in self.roles.items() if _as_chain(value) + } + return reviewer_independence(preferred, implemented_by=implemented_by) + + def routed_by( + self, routes_provider: Callable[[], Mapping[str, Route | Sequence[Route]]] + ) -> ModelClient: """A sibling client that resolves routes differently. Transport, retry policy, prices, telemetry and — deliberately — the @@ -420,18 +500,39 @@ def answers(self, route: Route, *, timeout: float = 10.0) -> tuple[bool, str]: + (f": {verdict.message[:160]}" if verdict.message else ""), ) - def route_for(self, role: str) -> Route: + def routes_for(self, role: str) -> Chain: + """Every route for a role, preferred first. + + More than one is a fallback chain, not a pool: the first that answers + does the work, and the rest exist because a provider being down is a + normal Tuesday. Measured on the endpoint this runs against, 34 of 42 + advertised models were unavailable at once -- an ordering that names a + second and third choice is the difference between a fleet that pauses + and one that carries on. + """ if self.routes_provider is not None: live = self.routes_provider() if live: - self.roles = dict(live) + self.roles = {name: _as_chain(value) for name, value in live.items()} try: - return self.roles[role] + # Normalised on read as well as on write: `roles` is a plain + # attribute callers assign to, and a bare `Route` put there by + # hand is the one-element chain it looks like. + return _as_chain(self.roles[role]) except KeyError: raise KeyError( f"no route for role {role!r}; known roles: {sorted(self.roles)}" ) from None + def route_for(self, role: str) -> Route: + """The preferred route for a role: what this deployment means to use. + + Everything that *reports* on routing -- readiness, the role map, + reviewer independence -- asks this, because a fallback that has not + been needed is not what the operator configured. + """ + return self.routes_for(role)[0] + def call(self, role: str, messages: Sequence[Mapping[str, Any]], **options: Any) -> Response: """Call `role`'s model. Returns the successful response. @@ -439,82 +540,127 @@ def call(self, role: str, messages: Sequence[Mapping[str, Any]], **options: Any) if the provider refused and retrying cannot help, or `RetryExhausted` once the retryable attempt ladder is spent. """ - route = self.route_for(role) - merged = {**route.options, **options} + chain = self.routes_for(role) last: Classification | None = None + last_route = chain[0] for attempt in range(self.policy.max_attempts): if attempt > 0: delay = self.policy.delay_for( attempt, last.retry_after if last else None, self.jitter() ) - self._emit( - role, route, "retry_wait", last, attempt=attempt, detail=f"waiting {delay:.1f}s" - ) - self.sleep(delay) - - # This process's own park on this endpoint, from an earlier cap. - parked = self.parks.remaining(route.endpoint, self.now(), role) - if parked > 0: - self._emit( - role, route, "parked", None, attempt=attempt, detail=f"{parked:.0f}s remaining" - ) - self.sleep(parked) - - started = self.now() - response = self.transport(route, messages, merged) - latency = self.now() - started - - if 200 <= response.status < 300: self._emit( role, - route, - "ok", - None, + last_route, + "retry_wait", + last, attempt=attempt, - latency=latency, - usage=usage_fields(response.body, route.model, self.prices), - ) - return response - - verdict = route.provider.classify(response.status, response.headers, response.body) - last = verdict - self._emit(role, route, "error", verdict, attempt=attempt, latency=latency) - - if verdict.kind in P.CAPS: - # Out of budget. Retrying cannot help, so park this endpoint - # in this process and stop. Other workers and other endpoints - # are untouched; the fleet still resumes unattended when the - # window rolls over. - park = ( - self.policy.window_cap_park_seconds - if verdict.kind == P.WINDOW_CAP - else self.policy.terminal_cap_park_seconds - ) - self.parks.park(route.endpoint, park, self.now(), role) - raise CapExhausted( - f"{route.model} via {route.endpoint}: {verdict.message or verdict.kind}", - kind=verdict.kind, - endpoint=route.endpoint, + detail=f"waiting {delay:.1f}s", ) - if verdict.kind in (P.NON_RETRYABLE, P.FATAL): - # Refused, but nothing is exhausted — so do NOT park. Parking - # here would idle a healthy endpoint over one bad request. + self.sleep(delay) + + # One pass down the chain before any backoff. A model that is + # down answers immediately, so trying the alternatives first costs + # nothing and gets the work moving; backing off against a dead + # provider for minutes before even looking at the second choice + # would waste the fallback entirely. + kinds: list[str | None] = [] + for route in chain: + merged = {**route.options, **options} + last_route = route + + parked = self.parks.remaining(route.endpoint, self.now(), role) + if parked > 0 and len(chain) > 1: + # Somewhere else to go, so skip rather than sleep. + self._emit( + role, + route, + "skipped", + None, + attempt=attempt, + detail=f"parked {parked:.0f}s", + ) + kinds.append(P.WINDOW_CAP) + continue + if parked > 0: + self._emit( + role, + route, + "parked", + None, + attempt=attempt, + detail=f"{parked:.0f}s remaining", + ) + self.sleep(parked) + + started = self.now() + response = self.transport(route, messages, merged) + latency = self.now() - started + + if 200 <= response.status < 300: + self._emit( + role, + route, + "ok", + None, + attempt=attempt, + latency=latency, + usage=usage_fields(response.body, route.model, self.prices), + detail=_fell_back(chain, route), + ) + return response + + verdict = route.provider.classify(response.status, response.headers, response.body) + last = verdict + kinds.append(verdict.kind) + self._emit(role, route, "error", verdict, attempt=attempt, latency=latency) + + if verdict.kind in P.CAPS: + # Out of budget on this endpoint. Park it -- retrying it + # cannot help -- and try the next model rather than + # stopping, which is the whole point of naming one. + self.parks.park( + route.endpoint, + self.policy.window_cap_park_seconds + if verdict.kind == P.WINDOW_CAP + else self.policy.terminal_cap_park_seconds, + self.now(), + role, + ) + # A refusal is NOT parked: it says something about this + # request, not about the model's health, and idling a healthy + # endpoint over one bad prompt would be a self-inflicted + # outage. The next route is still tried, because a refusal + # from one vendor is routinely an answer from another. + + if not any(kind == P.TRANSIENT or kind == P.RPM for kind in kinds): + # Nothing another cycle could fix: every route was refused or + # is out of budget. Say which, in the terms the executor acts + # on -- a cap hands the item back untouched, a refusal is the + # model's answer. + if all(kind in P.CAPS for kind in kinds): + raise CapExhausted( + f"{role}: every route is out of budget " + f"({_chain_names(chain)}): {last.message if last else ''}", + kind=last.kind if last else P.WINDOW_CAP, + endpoint=last_route.endpoint, + ) raise RequestRefused( - f"{route.model} via {route.endpoint}: {verdict.message or verdict.kind}", - kind=verdict.kind, + f"{role}: every route refused ({_chain_names(chain)})" + f": {last.message if last else ''}", + kind=last.kind if last else P.NON_RETRYABLE, ) message = ( f"{role}: {self.policy.max_attempts} attempts exhausted against " - f"{route.model} via {route.endpoint}" + (f"; last was {last.kind}" if last else "") + f"{_chain_names(chain)}" + (f"; last was {last.kind}" if last else "") ) raise RetryExhausted( message, role=role, kind=last.kind if last else None, - endpoint=route.endpoint, - model=route.model, + endpoint=last_route.endpoint, + model=last_route.model, last=last, ) diff --git a/src/agent_harness/runtime.py b/src/agent_harness/runtime.py index 87e783b..fb83554 100644 --- a/src/agent_harness/runtime.py +++ b/src/agent_harness/runtime.py @@ -20,7 +20,7 @@ from __future__ import annotations import shlex -from collections.abc import Callable, Mapping +from collections.abc import Callable, Mapping, Sequence from dataclasses import dataclass from pathlib import Path from typing import Any @@ -110,7 +110,7 @@ def session_executor_factory( host: Any, agent: AgentSpec | None = None, reviewer: Any | None = None, - routes_for: Callable[[str], Mapping[str, Route]] | None = None, + routes_for: Callable[[str], Mapping[str, Route | Sequence[Route]]] | None = None, github_for: Callable[[str], Any] | None = None, ui_base_url: str = "", on_event: Callable[[dict[str, Any]], None] | None = None, diff --git a/tests/test_model_fallback.py b/tests/test_model_fallback.py new file mode 100644 index 0000000..dab9ab2 --- /dev/null +++ b/tests/test_model_fallback.py @@ -0,0 +1,253 @@ +"""A role can name more than one model, and the first that answers wins. + +Measured against the endpoint this runs on: 34 of 42 advertised models were +unavailable simultaneously, including two of the three an operator had chosen +for implementation. A role with one name per model is a fleet that stops when +that name is down — so a role holds an ordered chain, and the alternatives are +tried before any backoff, because a dead provider answers instantly and +sleeping on it first would waste the fallback entirely. +""" + +from __future__ import annotations + +import json +from collections.abc import Mapping, Sequence +from typing import Any + +import pytest + +from agent_harness import providers as P +from agent_harness.model_client import ( + CapExhausted, + ModelClient, + RequestRefused, + Response, + RetryExhausted, + RetryPolicy, + Route, + chains_from_map, +) + +ENDPOINT = "https://api.example/v1" + + +def chain(*models: str) -> tuple[Route, ...]: + return tuple(Route(m, ENDPOINT, P.CLAW_BAY, options={"role": "implementer"}) for m in models) + + +class Scripted: + """Answers per model, and records the order it was asked.""" + + def __init__(self, answers: Mapping[str, int | Exception]) -> None: + self.answers = dict(answers) + self.asked: list[str] = [] + + def __call__( + self, route: Route, messages: Sequence[Mapping[str, Any]], options: Mapping[str, Any] + ) -> Response: + self.asked.append(route.model) + status = self.answers.get(route.model, 200) + if isinstance(status, Exception): + raise status + body = json.dumps({"choices": [{"message": {"content": "ok"}}]}) + # A claw-bay upstream failure carries a JSON body naming the reason; + # the classifier reads it, so the fixture has to produce one. + if status >= 400: + body = json.dumps({"error": "model service unavailable", "code": "upstream_rejected"}) + return Response(status, {}, body) + + +def build(transport: Scripted, *models: str, attempts: int = 2) -> ModelClient: + return ModelClient( + roles={"implementer": chain(*models)}, + transport=transport, + policy=RetryPolicy(max_attempts=attempts, backoff_seconds=0.001), + sleep=lambda _s: None, + ) + + +def call(client: ModelClient) -> Response: + return client.call("implementer", [{"role": "user", "content": "hi"}]) + + +def test_the_preferred_model_is_used_when_it_answers() -> None: + transport = Scripted({}) + client = build(transport, "deepseek-v4-flash", "glm-5.2", "gpt-5.4") + + assert call(client).status == 200 + assert transport.asked == ["deepseek-v4-flash"], "a healthy first choice must not be skipped" + + +def test_an_unavailable_model_falls_through_to_the_next() -> None: + """The live case: two of three down, one answering.""" + transport = Scripted({"deepseek-v4-flash": 499, "glm-5.2": 503}) + client = build(transport, "deepseek-v4-flash", "glm-5.2", "gpt-5.4") + + assert call(client).status == 200 + assert transport.asked == ["deepseek-v4-flash", "glm-5.2", "gpt-5.4"] + + +def test_the_whole_chain_is_tried_before_any_backoff() -> None: + """A dead provider answers in milliseconds. Backing off against it before + looking at the second choice would spend minutes to learn nothing.""" + slept: list[float] = [] + transport = Scripted({"a": 503}) + client = ModelClient( + roles={"implementer": chain("a", "b")}, + transport=transport, + policy=RetryPolicy(max_attempts=3, backoff_seconds=10.0), + sleep=slept.append, + ) + + assert call(client).status == 200 + assert transport.asked == ["a", "b"] + assert slept == [], "the fallback was reached only after a backoff" + + +def test_a_refusal_still_tries_the_next_model() -> None: + """A refusal is about this request, and one vendor's refusal is routinely + another's answer.""" + transport = Scripted({"a": 400}) + client = build(transport, "a", "b") + + assert call(client).status == 200 + assert transport.asked == ["a", "b"] + + +def test_a_refusal_does_not_park_the_model_it_came_from() -> None: + """Falling back is not the same as declaring a model unhealthy. Parking a + working endpoint over one bad prompt would be a self-inflicted outage.""" + transport = Scripted({"a": 400}) + client = build(transport, "a", "b") + call(client) + + assert client.parks.remaining(ENDPOINT, client.now(), "implementer") == 0 + + +def test_when_every_route_refuses_the_refusal_is_raised() -> None: + transport = Scripted({"a": 400, "b": 400}) + client = build(transport, "a", "b") + + with pytest.raises(RequestRefused) as caught: + call(client) + assert "a" in str(caught.value) and "b" in str(caught.value) + + +def test_when_every_route_is_out_of_budget_the_cap_is_raised() -> None: + """`CapExhausted` is what hands an item back untouched, so it must survive + the chain rather than being flattened into a generic failure.""" + transport = Scripted({"a": 403, "b": 403}) + client = build(transport, "a", "b") + + with pytest.raises(CapExhausted): + call(client) + + +def test_a_transient_failure_everywhere_is_retried_then_given_up_on() -> None: + transport = Scripted({"a": 503, "b": 503}) + client = build(transport, "a", "b", attempts=2) + + with pytest.raises(RetryExhausted): + call(client) + # Two cycles over two routes. + assert transport.asked == ["a", "b", "a", "b"] + + +def test_a_parked_route_is_skipped_when_another_endpoint_can_serve() -> None: + """With somewhere else to go, sleeping out a park is time spent for no + reason.""" + slept: list[float] = [] + transport = Scripted({}) + elsewhere = "https://other.example/v1" + client = ModelClient( + roles={ + "implementer": ( + Route("a", ENDPOINT, P.CLAW_BAY, options={"role": "implementer"}), + Route("b", elsewhere, P.CLAW_BAY, options={"role": "implementer"}), + ) + }, + transport=transport, + policy=RetryPolicy(max_attempts=2, backoff_seconds=0.001), + sleep=slept.append, + ) + client.parks.park(ENDPOINT, 600.0, client.now(), "implementer") + + assert call(client).status == 200 + assert transport.asked == ["b"], "the parked route should have been skipped" + assert slept == [] + + +def test_models_on_one_endpoint_share_its_park() -> None: + """Worth pinning, because it bounds what a fallback chain can do. + + A spend cap belongs to the account, not the model, so parking the endpoint + parks every model behind it — which is right, and means a chain of models + on a single provider is insurance against *that model* being unavailable, + not against running out of budget. + """ + transport = Scripted({}) + client = build(transport, "a", "b") + client.parks.park(ENDPOINT, 600.0, client.now(), "implementer") + + with pytest.raises(CapExhausted): + call(client) + assert transport.asked == [], "nothing should have been called on a parked endpoint" + + +def test_falling_back_is_recorded() -> None: + """A fleet quietly running on its third choice for a week is a fleet whose + results nobody can explain.""" + events: list[dict[str, Any]] = [] + transport = Scripted({"a": 503}) + client = ModelClient( + roles={"implementer": chain("a", "b")}, + transport=transport, + policy=RetryPolicy(max_attempts=2, backoff_seconds=0.001), + sleep=lambda _s: None, + on_event=events.append, + ) + call(client) + + ok = [e for e in events if e["outcome"] == "ok"] + assert ok and ok[0]["model"] == "b" + assert "fell back to b" in (ok[0].get("detail") or "") + + +def test_the_preferred_route_is_what_is_reported() -> None: + """Readiness, the role map and independence all describe configuration, + not whichever alternative happened to answer.""" + client = build(Scripted({}), "deepseek-v4-flash", "glm-5.2") + + assert client.route_for("implementer").model == "deepseek-v4-flash" + assert [r.model for r in client.routes_for("implementer")] == ["deepseek-v4-flash", "glm-5.2"] + + +# ------------------------------------------------------------- the stored map + + +def test_a_stored_map_can_name_several_models() -> None: + chains = chains_from_map( + { + "implementer": { + "models": ["deepseek-v4-flash", "glm-5.2", "gpt-5.4"], + "endpoint": ENDPOINT, + } + } + ) + + assert [r.model for r in chains["implementer"]] == [ + "deepseek-v4-flash", + "glm-5.2", + "gpt-5.4", + ] + + +def test_a_map_written_before_fallbacks_existed_still_reads() -> None: + """`model` as a single name is the one-element chain it always was.""" + chains = chains_from_map({"reviewer": {"model": "gpt-5.6", "endpoint": ENDPOINT}}) + + assert [r.model for r in chains["reviewer"]] == ["gpt-5.6"] + + +def test_a_role_with_no_endpoint_is_dropped_rather_than_half_built() -> None: + assert chains_from_map({"reviewer": {"models": ["gpt-5.6"]}}) == {}