Files
6krrt/plans/upstream-failover-and-circuit-breaker.md
adlee-was-taken 3523dcf93e docs(plans): give every plan a Status line so the queue is greppable
plans/ held 58 documents and exactly one said whether it was open. The rest
mixed finished work, reviews of shipped work, parked specs and genuinely
pending ones, with nothing distinguishing them, so "how many plans are in
the queue" had no answer short of reading all 58.

Now `grep -H '^Status:' plans/*.md` is the answer:

    50 done   3 in progress   2 planned   2 reference   1 parked

Statuses were derived rather than guessed: CLAUDE.md's own built list and
"What's NOT built yet" section, plus checking the subject exists in the
code. A review of work that shipped counts as done -- it records what was
found, it is not a request for anything. `reference` separates the two docs
that are conventions rather than work items (admin-design-standards,
admin-work-framework), which otherwise read as permanently-open plans.

The vocabulary is deliberately five words. A larger one invites "mostly
done" and "blocked-ish", which is how the directory became unreadable.

test_plans_declare_status.py keeps it from rotting: a new plan without a
marker fails, as does an unknown status, one buried below the eighth line,
or an open status with no reason -- "planned" alone is the state that rots,
since nobody can tell later whether it waits on a decision, a dependency,
or just nobody's turn.

Also updates the sweep plan with what landed and what did not, including
that #9 was not a defect.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VRQXz5SYZYVWscxS1QqF6U
2026-09-08 18:55:16 -04:00

12 KiB

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:

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:

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):

@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

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_validators 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.