fix(sidecar): keep transient-rejected preflight routes as deferred failover instead of discarding them

[seonghobae]

Summary

The review sidecar's preflight discards a route the moment its 16-token probe answers with any exception — including 429 and 5xx — while the serving gateway it feeds treats exactly those statuses as transient, retrying the route and then failing over across it (provider_errors.PROVIDER_STATUS_SURFACES[429] = (…, retryable=True); orchestrator.TRANSIENT_HTTP_STATUS). Under concurrent CI load the probes themselves spend the per-key budgets, readiness collapses, and the served set has nowhere to fail over to. This PR keeps such routes as deferred, ranked after every ready route, and leaves everything else about the preflight unchanged.

Evidence

noema-review run 33993637015 (.github#1687, 2026-09-05T21:38Z, sidecar at 2e414d15, catalog already 4+4+4 after #1939), from the runtime preflight summary in the job log:

outcome	routes
ready	1 — nvidia_nim_sub deepseek-v4-flash
429	6 — all four openrouter free routes, plus nvidia_nim deepseek-v4-flash and deepseek-v4-pro (the sibling key's flash route was ready)
404	4 — gemma-3-12b / gemma-3-4b on both NVIDIA keys
500	1 — nvidia_nim_sub deepseek-v4-pro

attempts=1 for every rejected route, escalations_used 0/4, preflight finished in 20 s. The single ready route then spent 542 s — two rounds of three 90 s timeouts, the gateway's full retry budget for one silent agent — and the caller got a 502. Under this PR the same run serves 1 ready + 6 deferred.

Why escalations_used stayed 0: the escalation budget is only for the budget signature (finish_reason == "length" or reasoning-without-content, launcher.py:442-467); a probe exception goes _record_provider_exception → rejected → continue on the first attempt (:409-412), by design ("401/429/5xx are not budget evidence", :354). This PR does not touch escalation.

Change

• REVIEW_PREFLIGHT_DEFERRABLE_HTTP_STATUS = {408, 409, 425, 429, 500, 502, 503, 504, 529} — the vendored orchestrator's TRANSIENT_HTTP_STATUS, so preflight and serving read the same signal the same way; a status the gateway would not retry (401/403/404/…) is still rejected.
• After the probe loop, and only when at least one route is ready, every rejected row whose http_status is in that set becomes deferred; the agent is re-emitted with its catalog priority lowered by REVIEW_PREFLIGHT_DEFERRED_PRIORITY_PENALTY = 1000 (_demote_agent: dataclasses.replace for the frozen ModelAgent, copy-and-set for plain objects) and appended after the ready agents. With no ready route the stage fails exactly as before, so _preflight_with_fallback's "priced catalog only after every primary route rejects" (ADR-0005) is untouched.
• Report: ready_count unchanged (never inflated — it remains the discriminator peers measure), new deferred_count, rejected_count = probed − ready − deferred; routes[].status gains "deferred".
• _log_preflight_rejections prints preflight_route_deferred … next to preflight_route_rejected …, and the stream sanitizer's allow-list admits both event names with the same bounded fields.

Tests

• test_preflight_defers_transient_probe_statuses_behind_ready_routes: ready / 429 / 404 / 503 → served order ready, 429, 503, priorities 0, −1001, −1003, caller's agents untouched, counts 1/2/1, per-route statuses.
• test_preflight_still_fails_when_no_route_is_ready: all-429 → ReviewPreflightError, deferred_count 0, statuses stay rejected.
• test_demote_agent_handles_frozen_dataclasses_and_plain_objects.
• test_log_preflight_rejections_reports_deferred_routes and test_sidecar_stream_sanitizer_passes_deferred_preflight_lines (unknown event names still dropped; a planted token still scrubbed).
• Negative control: all five fail against main's launcher and sanitizer. Full gate in the commit message.

Tradeoff (raised by lane peer 1) and why it is bounded

A deferred route that does not recover adds to the serving walk. The first noema-sidecar-evidence trace (#1661 run 33995553859, table on #1939) shows what a route costs on the review path: a route that answers 429/5xx immediately costs seconds for its attempts; a silent route costs the gateway's full retry budget — three 90 s timeouts, twice (_invoke retries the same agent once) — 541 s measured. Deferral admits only routes that answered the probe with a transient status (a probe that timed out records no http_status and stays rejected), so it never admits, on the strength of the probe, the silent class that costs 541 s; the residual risk is a route that answered the probe and goes silent at request time, which a ready route carries equally.

The discriminator for this PR is the deferred routes' outcome at request time, read per route from the provider_attempt / provider_exhausted / provider_rejected_permanent lines in the artifact (#1943/#1944/#1945): if deferred routes mostly serve or fail fast, the change is pure gain; if they routinely go silent, a cap on deferred routes is the next commit. The same trace also shows the larger costs deferral does not touch — the same agents walked three times within one request, and two preflight-ready routes never attempted — which sit with contextual-orchestrator#1045 and #1948.

Not in this PR

The four permanent 404s (gemma-3 models NIM lists but does not serve) still cost a third of the catalog every run — that is a discovery-side question. The probe budget itself (every sidecar boot probing all twelve routes against per-key rate limits) is a policy value and stays with the owner. Related: #1939 (catalog composition), #1943/#1944/#1945 (trace + artifact + sanitizer), contextual-orchestrator#1045 (gateway failover cost).

Developer experience

One frozenset, one small helper, one post-loop pass; the existing probe branches and the escalation path are untouched, and the report keeps its old keys with their old meanings.

User experience

A review whose probes hit rate limits still gets served, and a stalled route has other routes to fall over to, instead of a 502 after the single survivor's retry budget runs out.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

Next included review available in 20 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: d890c9ef-0f51-40a0-b41d-bd350a9d5cc9

📥 Commits

Reviewing files that changed from the base of the PR and between efb8926 and e4c9c44.

📒 Files selected for processing (4)

• CHANGELOG.md
• scripts/ci/contextual_orchestrator_review_launcher.py
• scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py
• tests/test_contextual_orchestrator_review_runtime_preflight.py

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[seonghobae]

Sanitizer side checked on 0e42fbca (author of #1945 here): the diff against main@fe827e13 for scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py is exactly the _PREFLIGHT_ROUTE_REJECTED literal widened to preflight_route_(?P<event>rejected|deferred) plus the event substituted back into the summary — _LOG_PREFIX, _ORCHESTRATOR_EVENTS and the rest of the allowlist are untouched. On this head: the preflight/sanitizer suite is 73 passed; my two #1945 tests (admits_orchestrator_route_events, matches_real_formatter_output) pass unchanged; preflight_route_deferred provider=nvidia_nim error_type=ProviderUpstreamError http_status=429 upstream sk-secret → preflight_route_deferred provider=nvidia_nim error_type=ProviderUpstreamError http_status=429 (secret cut), preflight_route_rejected … unchanged, preflight_route_paused … → dropped. The allowlist principle holds: the event name is closed to two literals and every field keeps its bounded charset. No objection from the sanitizer lane; the deferred-route recovery rate read from the per-route artifact lines is the right ⑥ for this change.

[seonghobae]

Follow-up stacked on this branch: #1949 (lazy fill — probe the candidate list in order, stop at 8 ready or 16 probes). It needs this PR merged first; it will be retargeted to main afterwards.

[seonghobae]

Head-tree verification and merge under the chicken-and-egg clause (host 1, 2026-09-06 14:05 KST). The merge procedure was requested from host 2 at 10:40 KST and repeated at 13:08 and 13:35 KST by session name; no reply on either channel by 14:05 KST, so I am performing it, with the evidence here.

• Head: e4c9c44f (remote and local agree), base main@efb89269, 0 behind / 4 ahead. Three-dot delta vs main: 4 files, +191/−7 — scripts/ci/contextual_orchestrator_review_launcher.py, scripts/ci/sanitize_contextual_orchestrator_sidecar_stream.py, tests/test_contextual_orchestrator_review_runtime_preflight.py, CHANGELOG.md. The last commit is the merge of main's pin advance (efb89269), resolved at the CHANGELOG top-insert only.
• Gate on this exact head: 2909 passed, 1 skipped, coverage 100 % (0 statements missed), interrogate 100 %.
• Negative control: the five tests this PR adds fail on main's launcher and sanitizer (test_preflight_defers_transient_probe_statuses_behind_ready_routes, test_preflight_still_fails_when_no_route_is_ready, test_demote_agent_handles_frozen_dataclasses_and_plain_objects, test_log_preflight_rejections_reports_deferred_routes, test_sidecar_stream_sanitizer_passes_deferred_preflight_lines); the tests are unchanged since.
• Independent head verification: lane peer 1 verified 37a1129a (the code head; the merge commit added only main's pin bump and the CHANGELOG resolution) — sanitizer allow-list clean, deferral pairing correct. Lane jan's real-order simulation on #1949 reads the same walk.
• Checks on e4c9c44f: Bandit, Semgrep, gitleaks, Python Security, Security Scan, SAST Semgrep, admit-current-head, scheduler and runtime-quality success; Trivy and Scorecard neutral. CodeQL compatibility analysis (actions) failed at its dispatch hand-off step (#1929 actor gate, tree-independent; the python one is still queued). Strix Security Scan failed at sidecar provisioning (run 34008242133, 04:54–05:02Z): preflight with no ready route plus a Bytez discovery 500 — the same no-capacity hour as .github#1661 run 34008191123, not a finding on this diff. noema-review in progress, opencode-review queued.
• Why the clause applies: this PR changes the review sidecar's own launcher and sanitizer; the required review contexts run the base branch's scripts under pull_request_target and their verdict dispatch is blocked by the owner-gated #1929 variable, and this hour's provider capacity fails the sidecar at preflight regardless of the tree.
• Merge method: merge commit, so #1949 (stacked on this branch) retargets to main with only its own delta.

[seonghobae]

First post-merge sample (host 1). argos Strix run 34013128112, job 101437774115 (created 05:05Z right after this merge, checked out 46f5761a, so this PR without #1949; sidecar booted 06:43Z): preflight probed 12 / ready 4 / deferred 4 / rejected 4, escalations 2 — ready = deepseek-v4-flash and deepseek-v4-pro on both NVIDIA keys; deferred = all four OpenRouter routes (429), kept behind the ready ones as designed; rejected = the four gemma-3 404s. Exactly the structure lane jan projected on #1949. Deferral is live in production.

The run then failed inside Strix itself, not at the gateway: after Docker image ready the sandbox's Caido service on 127.0.0.1:48080 never accepted a connection, loginAsGuest failed after 10 attempts (curl exit 7), Strix exited after 240 s, and the gate printed STRIX_PROVIDER_UNAVAILABLE: … orchestrator/free exhausted on that exit code — so the served set (4 ready + 4 deferred) was never exercised and the verdict text blames the wrong component. I am checking how often recent Strix artifacts carry this sandbox signature versus real provider errors; if it is common, the gate needs a distinct sandbox unavailable signal so it neither counts against the gateway in the census nor spends the bounded replay budget on a non-provider failure.