6krrt/plans/upstream-failover-and-circuit-breaker.md

# Spec: immediate per-request failover + a passive circuit breaker for upstream model outages

Status: done -- circuit_breaker.py

**Origin.** Found live: NeuralWatt returned `503` for `gemma-4-31b`
("All servers for model 'nvidia/Gemma-4-31B-IT-NVFP4' are currently
unavailable... Retry later") on four consecutive requests. Every one of
those requests had already computed a ranked candidate list — `cand=10` in
the route log — and none of the other nine were ever tried. Two related but
separable problems, addressed together because the second only matters once
the first exists:

1. **No failover within a single request.** When the picked model's upstream
   call itself errors (not a bad *answer* — the call never produced one),
   the router surfaces the raw error to the client instead of trying the
   next-ranked candidate.
2. **No memory across requests.** Even with #1 fixed, every subsequent
   request during an outage would still try the dead model first and pay a
   wasted round-trip before falling over — for as long as the outage lasts.

No code changes accompany this document — this is the spec opencode builds
from.

---

## Why these are a different failure class from what `iteration.py` already handles

`iteration.py`'s retry budget (`attempts_by_tier`) is a **quality** budget:
it exists to pay for a corrective attempt after the model *answered badly*
(`truncated`, `malformed`). An upstream `5xx` means the model never got to
answer at all — this is an **availability** failure, not a quality one, and
it must not be charged against the same budget. A tier-1 interactive
request has a 0-attempt quality budget (`CLAUDE.md`'s own table), which is
correct for "don't pay to fix a bad answer on cheap work" — but it would be
wrong for that to also mean "a tier-1 request gets zero chances to route
around a dead replica." Availability failover needs its own, separate cap.

Convenient fact found while reading the existing code: `decision.runners_up`
is already capped at 3 (`ranked[1:4]`, `dispatcher.py:838`), so "try every
candidate this request already ranked" is naturally bounded at 4 total
attempts (the primary pick + 3 runners-up) with **no new config knob
required** for the cap itself.

## Mechanism, part 1: immediate per-request failover

### Non-streaming path (`dispatcher.py`, the `while True:` loop around line 2415)

This loop already exists, already tracks `current_model` and an
`alternatives` list built from `decision.runners_up`
(`dispatcher.py:2402-2409`), and already reassigns `current_model = plan.model_id`
on a quality-retry. The only change: today, `resp.status_code >= 400`
(line 2425) immediately does `raise HTTPException(...)` — bypassing the loop
entirely. Instead, treat it as an availability failure inside the same loop:

```python
if resp.status_code >= 400:
    logs.error("upstream", model=current_model, status=resp.status_code,
               detail=resp.text[:200], ms=upstream_ms)
    circuit_breaker.record_failure(current_model, provider)   # part 2
    if not alternatives:
        raise HTTPException(resp.status_code, resp.text[:500])
    current_model, _ceiling = alternatives[0]
    alternatives = alternatives[1:]
    continue   # try the next candidate; does NOT consume `attempts_used`
```

Critically, this must **not** increment `attempts_used` or check it against
`budget` — that variable is the quality-retry budget from `iteration.py` and
stays reserved for verification failures, per the section above. An
availability failover loop needs its own bound, and reusing
`len(alternatives)` (already ≤3) is sufficient; no new counter needed.

### Streaming path (`proxy()` generator, `dispatcher.py:2546` onward)

This is the harder half, and the ordering matters. Today, the generator
opens the upstream connection and checks its status *inside* the streamed
response body's own generator function. That's too late to retry
transparently: once `StreamingResponse(proxy())` is returned from the route
handler, the client has already been sent a `200` and headers — there is no
way to swap in a different upstream after that without the client seeing a
broken stream.

The fix is to move connection **and status check** for each candidate
*before* `StreamingResponse` is ever constructed, using the fact that
`requests.post(..., stream=True)` returns as soon as headers arrive, without
consuming the body:

```python
def _open_upstream(model_id: str, ...) -> requests.Response:
    """POST with stream=True; caller decides whether to consume or discard."""
    return requests.post(url, headers=headers, json={**upstream_body, "model": model_id}, stream=True, timeout=600)

candidates = [target] + [c.model_id for c in decision.runners_up]  # decision.runners_up already capped at 3
upstream = None
for model_id in candidates:
    attempt = _open_upstream(model_id, ...)
    if attempt.status_code < 400:
        upstream = attempt
        target = model_id
        break
    logs.error("upstream", model=model_id, status=attempt.status_code, ...)
    circuit_breaker.record_failure(model_id, provider)
    attempt.close()   # release the connection; nothing was ever sent to the client
if upstream is None:
    raise HTTPException(attempt.status_code, attempt.text[:500])
# ... proceed to construct StreamingResponse(proxy_over(upstream)) as today,
# with `proxy()` now taking the already-opened, already-healthy `upstream`
# instead of opening its own.
```

The `proxy()` generator keeps its existing defensive status check as a
belt-and-suspenders case (a healthy-looking connection can still fail
mid-stream — that's a much rarer, already-partially-committed situation this
spec doesn't try to solve), but the common case — a replica that's flatly
down, which is what `503 "no healthy replicas"` is — gets caught before the
client ever sees a byte.

## Mechanism, part 2: passive circuit breaker

A new module, `circuit_breaker.py`, matching `session_cache.py`'s exact
shape (pure, in-memory, no imports of `dispatcher`/`config`):

```python
@dataclass(frozen=True)
class CircuitState:
    down_until: float       # time.time() value; now < down_until means "skip"
    cooldown_seconds: float # what the NEXT failure's cooldown will be (doubled from this one)

def is_down(model_id: str, provider: str, now: float) -> bool: ...
def record_failure(model_id: str, provider: str, initial_cooldown: float,
                    max_cooldown: float, backoff_multiplier: float) -> None: ...
def record_success(model_id: str, provider: str) -> None: ...   # clears the entry entirely
def clear() -> None: ...   # test isolation, matching session_cache.clear()
```

**Recovery is passive, by design — no pinger.** `is_down` only ever
compares against `down_until`; nothing proactively re-checks a down model.
The next real request that would otherwise have picked that model, once
`down_until` has passed, simply isn't excluded anymore and becomes the
natural recovery probe. If it succeeds, `record_success` clears the entry.
If it fails, `record_failure` runs again with the cooldown doubled (capped
at `max_cooldown_seconds`) — this is deliberately **not** a background
service: an active health-check would spend real billed quota probing a
model nobody is currently asking for, which is the opposite of this
project's own standing rule that a wasted attempt costs energy against a
fixed quota. Demand already provides the probe for free.

**Where it plugs into routing.** `routing.py`'s `select_candidates` /
`rejection_reason` already hard-filters on `exclude_stale` /
`exclude_deprecated` (`routing.py:117-119`) — this is one more hard filter
of the same shape, not a new mechanism. `dispatcher.py` computes the
excluded set from `circuit_breaker` state once per request (`now =
time.time()`, check every candidate row) and passes it in, keeping
`routing.py` itself free of any import of `circuit_breaker` or `time` —
same separation `metrics.py`'s docstring already insists on to avoid an
import cycle.

**Where it's fed.** Both failover loops in part 1 call
`circuit_breaker.record_failure(...)` on every upstream `5xx`, and record
`record_success(...)` on the eventual successful attempt for that request —
so the circuit breaker's state comes entirely from real dispatch traffic,
never a separate check.

### Config

```yaml
circuit_breaker:
  # Off by default, matching every other new-and-unproven knob in this
  # project. Unlike most of them, this one has a low-risk failure mode even
  # when wrong — see the Recommendation section — so it's a reasonable
  # candidate to flip on sooner than most.
  enabled: false
  initial_cooldown_seconds: 30
  max_cooldown_seconds: 600
  backoff_multiplier: 2.0
```

`CircuitBreakerConfig(StrictModel)` with `field_validator`s requiring
`initial_cooldown_seconds > 0`, `max_cooldown_seconds >= initial_cooldown_seconds`,
and `backoff_multiplier > 1.0` (a multiplier ≤1 would never grow the
cooldown, defeating the point).

## What this does NOT do

- Does not retry a request whose upstream call *succeeded* but whose
  *answer* was bad — that's `iteration.py`'s job, unchanged.
- Does not add a background health-check service, on purpose (see above).
- Does not change anything about `POST /outcome` or `feedback.py` — an
  availability failure is not a proficiency signal about the model's
  quality, so it should never be folded into `proficiency` the way a
  verification failure is.

## Testing

- `circuit_breaker.py`: pure, offline, same shape as `tests/test_session_cache.py`
  already tests `session_cache.py` — inject `now` explicitly
  rather than relying on real `time.time()` in tests, so cooldown expiry is
  deterministic. Cover: first failure sets `initial_cooldown_seconds`;
  second consecutive failure doubles it; cooldown never exceeds
  `max_cooldown_seconds`; a success clears the entry outright (next failure
  after a success starts back at `initial_cooldown_seconds`, not wherever
  the backoff had climbed to).
- `routing.py`: a test asserting a circuit-broken model is excluded from
  `select_candidates` the same way a stale/deprecated one already is —
  extend the existing test shape for those two filters rather than
  inventing a new one.
- Non-streaming failover: `monkeypatch.setattr(dispatcher.requests, "post", ...)`
  returning a `503` for the first candidate and `200` for the second,
  asserting the response actually came from the second model and
  `attempts_used`/quality budget was untouched.
- Streaming failover: same idea against `_open_upstream`, asserting the
  discarded first connection's `.close()` was called and the client-visible
  stream came from the second candidate with a `200` from the very first
  byte (i.e., confirming the retry genuinely happened before
  `StreamingResponse` was constructed, not after).

## Recommendation

Build part 1 (immediate failover) regardless of how part 2 is scoped — it
has essentially no downside: on a provider-wide outage where every candidate
is down, the end result is identical to today (an error, after trying every
candidate instead of one), and on a partial outage like the one that
prompted this, it turns four visible failures into four invisible
successes. This one is a reasonable candidate to ship default-on rather than
behind the project's usual off-by-default caution, precisely because its
worst case matches current behavior rather than introducing a new one.

Build part 2 alongside it — without it, every request during an outage
still pays one wasted round-trip before failing over, for as long as the
outage lasts. Ship it behind `circuit_breaker.enabled: false` per the
project's standing pattern for new knobs, watch `route_decisions` /
`upstream` log lines for excluded-candidate behavior on real traffic, then
decide the default.