fix(sidecar): fill the preflight served set lazily to a readiness target

[seonghobae]

Was stacked on #1947, which merged as main@46f5761a on 2026-09-06; this PR now targets main directly and the diff is its own delta (7 files, +407/−26).

Summary

#1939 made catalog selection diverse (round-robin across accounts inside each tier, four per account), and that exposed a second defect: the per-account slice is filled from an alphabetically sorted model list, and for both NVIDIA keys the first four models are deepseek-v4-flash, deepseek-v4-pro, gemma-3-12b, gemma-3-4b — the last two answer 404 on every run. Each key therefore served two working routes, both the most contended, while the pre-#1939 eight-slot fill had reached the llama-3.2 / muse-glimmer routes that were ready in every artifact of that afternoon. This PR makes the catalog a candidate list and lets the preflight fill lazily: probe in catalog order, stop at 8 ready routes or 16 probes, whichever first. A dead candidate costs one probe instead of a served slot; a healthy pool stops early. ADR-0029 records the decision and amends ADR-0003's stage-budget sentence.

Evidence (lane jan's census on #1948, verdict-step conclusions only, draft skips excluded)

window	preflight ready of 12	noema-review in .github
before #1939 (main@f2f91b80, 2026-09-05T17:25Z)	6, 6, 5 (16:37–16:56Z Strix artifacts)	7 success / 14 failure
after	1–3 (23:47Z onward)	0 / 22

Evening rate-limit pressure is a confound; the mechanism is not — the same post-#1939 artifacts show preflight_route_rejected provider=nvidia_nim … http_status=404 ×4 (both gemma-3 on both keys) in every boot. #1812's preflight (lane peer 1) is the account-skip lever's measured case: OpenRouter 429 ×4, NIM gemma-3 404 ×4, ready 4 of 12 — under this PR the two OpenRouter probes after the skip threshold are exactly the two that reach the NIM llama tail.

Change

• REVIEW_PREFLIGHT_MAX_TOTAL_ROUTES 12 → 24 (candidates), REVIEW_PREFLIGHT_PRIMARY_ROUTE_LIMIT 8 → 16, new REVIEW_PREFLIGHT_TARGET_READY = 8, REVIEW_PREFLIGHT_MAX_PROBES = 16; the sidecar's ORCHESTRATOR_CATALOG_LIMIT default 12 → 24. Per-account cap stays 8, so each account's list continues past its dead entries in the same round-robin order #1939 established.
• _preflight_review_agents: one break at the top of the probe loop; report gains candidate_count, target_ready, probe_budget; probed_count counts probes actually sent; rejected_count = probed − ready − deferred. #1947's deferral pass is unchanged and pairs exactly the probed prefix.
• _preflight_with_fallback arithmetic unchanged in shape: 16 free candidates, remainder priced, both stages inside one budget; each stage's list fits the probe cap, so the worst case stays "every candidate probed once" (24 base + 4 escalations, the existing shared-budget test asserts 28).
• Account skip after two consecutive 429s (REVIEW_PREFLIGHT_ACCOUNT_SKIP_AFTER_429 = 2): a 429 at preflight is a per-key answer, so once an account has answered it twice in a row its remaining candidates are skipped without a probe and the walk moves to the other accounts' next candidates; the two probed routes are still deferred. Under the real 2026-09-06 order (lane jan's table below) this is the difference between about five ready routes and the target of eight, and a fully rate-limited hour costs two probes per account instead of the whole budget. The production free pool therefore lists all 24 candidates while probing at most 16 — the tail past the cap is reachable exactly when an account is skipped — and the report separates skipped_count from the unreached remainder, which answers lane peer 1's earlier objection to an unreachable tail. Report gains skipped_count and account_skip_after_429.
• Sidecar job-log echo of the preflight JSON: sed -n '1,400p' (was 160 — 16 probed routes at 8–10 lines each plus the header overran it exactly in the dead hour; the artifact copy was always complete). Also peer 1.
• ADR-0029 (new — 0028 is already carried by open #1668), ADR-0003 §2 sentence amended, CHANGELOG.

Tests

• test_preflight_fills_lazily_and_stops_at_the_readiness_target — 12 ready candidates → 8 probes, served = first 8, counts 12/8/8/0/0, report carries target_ready/probe_budget, 8 route rows.
• test_preflight_dead_candidates_cost_a_probe_not_a_served_slot — two 404s at the head → probed 10, ready 8, rejected 2.
• test_preflight_probe_budget_bounds_a_dead_hour — 24 × 404, nothing ready → exactly 16 probes, ReviewPreflightError, 24/16/0/16/0, no skips.
• test_preflight_skips_an_account_after_consecutive_429s — three accounts × 8, all 429 → 6 probes (two per account), 18 skipped, ReviewPreflightError.
• test_preflight_reaches_both_keys_llama_routes_under_the_artifact_order — the candidate order and answers rebuilt from jan's table (both NVIDIA keys: deepseek ×2, gemma-3 ×2 → 404, gemma-4-31b → empty, llama ×2, muse; OpenRouter all 429; tier round-robin, 8 per account) → served set contains a llama route on each key, ready = 8 within 16 probes, deferred = 2, skipped ≥ 3, and the two deferred agents are exactly the two probed OpenRouter routes. Fails on the previous head of this PR (5 ready, sub key never reaches llama).
• test_preflight_deferral_pairs_rows_with_probed_agents_after_skips — once an account is skipped, routes no longer lines up with agents, so the deferral pass pairs rows with the list of agents actually probed (probed); this test has a 429 route after the first skip and asserts it, not a skipped candidate, is the one demoted. Caught in self-review: the artifact-order test alone passed on the misaligned version because its skips all fell after its deferred rows.
• test_preflight_lazy_fill_keeps_deferral_for_probed_transient_routes — a 429 on the way is served last with the −1000 penalty; candidates past the stop get no row.
• test_preflight_stage_limits_share_one_startup_budget updated to (16, 8) plus the fits-the-probe-cap invariants, including the free pool's single stage (= 16).
• Negative control: the four new tests and the updated one all fail on #1947's tree. Full gate (coverage 100 / interrogate 100) in the commit message.

Tradeoff

In an hour where nothing is ready the sidecar spends up to 16 probes per stage where it spent 12 — a third more against keys that are already exhausted, and a silent candidate's probe costs up to one transport timeout (peer 1's artifact: 19 probes, 805 s; #1661 run 34008191123: two 16-token probes held for 90.06 s and 90.10 s), so the cap bounds wall time too — a fully silent hour costs at most 16 × 90 s = 24 minutes of preflight against 18 today, and a rate-limited hour two probes per account. That is the price of finding routes past the dead ones; #1948's shared rate ledger is the lever above it. Not touched: a route that answers the probe and goes silent at request time still costs the gateway's full retry budget (contextual-orchestrator#1045); the discovery-side question of why NIM lists models it does not serve stays open.

Discriminator

Per boot after merge, from the runtime preflight summary in the job log or the noema-sidecar-evidence artifact: probed_count vs candidate_count, and ready_count of the served set against the table above. If a healthy hour shows probed_count near 16 with ready_count under 8, the dead entries are denser than two per key and the target/cap need re-reading from the census, not guessing.

Developer experience

Four named constants, one break, three report keys; the probe branches, the escalation path and #1947's deferral are untouched.

User experience

A review boots with a served set that reaches past the dead catalog entries, so a rate-limited or stalled route has other routes to fail over to instead of a 502 after the lone survivor's retry budget runs out.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

Next included review available in 21 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: 1ccc99dd-fe1d-4c4c-8e36-d9420c0b8f3c

📥 Commits

Reviewing files that changed from the base of the PR and between ff9848a and 27d478d.

📒 Files selected for processing (7)

• .github/actions/orchestrator-free-sidecar/action.yml
• CHANGELOG.md
• docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md
• docs/adr/0029-sidecar-preflight-lazy-fill.md
• scripts/ci/contextual_orchestrator_review_launcher.py
• scripts/ci/contextual_orchestrator_review_sidecar.sh
• 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]

Head b270bde1 — review fixes from lane peer 1's verification of 6d897c99 (their gate: 77 tests in this file pass with GITHUB_ACTIONS=true; negative control 5 failures on #1947's launcher; ① the break pairs exactly the probed prefix with the deferral zip, confirmed).

• ②(b), the real one: the production pool is free (sidecar default; auto is rejected there), and on that path _bounded_primary_catalog_limit returned the full 24 while the probe cap is 16 — candidates 17–24 could never be probed, and candidate_count − probed_count would have carried a permanent 8 instead of evidence. Every stage's list is now min(total, REVIEW_PREFLIGHT_MAX_PROBES): the free pool lists 16 candidates (12 before), the auto pool keeps 16 free + up to 8 priced. The stage-limit test asserts the free pool's single stage equals the probe cap.
• ②(a): the launcher's own ORCHESTRATOR_CATALOG_LIMIT default now matches the sidecar (24).
• ③: the sidecar's job-log echo of the preflight JSON grows from 160 to 400 lines — 16 probed routes at 8–10 lines each plus the header overran the old cap exactly in the dead hour the summary matters; the artifact copy was always complete.
• ④: ADR-0029 is deliberate — open #1668 already carries docs/adr/0028-strix-orchestrator-free-only-model-boundary.md, so 0028 would collide the moment either merged (main's tree alone says 0027; open PRs are part of the tree).
• Also recorded from peer 1's artifact: a silent candidate's probe costs up to one transport timeout (19 probes, 805 s), so the probe cap bounds preflight wall time, not just request count.

Gate on this head: 2913 passed, 1 skipped, coverage 100 % (13150 statements, 0 missed), interrogate 100 %.

[seonghobae]

Verification against the real candidate order (lane jan). The change is right and I have no correctness finding; this is what the 16-probe walk will actually reach under today's discovery, derived from #1938's Strix artifact (contextual-orchestrator-discovery.json for each account's alphabetical list, contextual-orchestrator-agents.json for the tier round-robin pattern the first 12 slots follow; positions 13–16 are extrapolated from that pattern, not observed).

probe	candidate	expected at preflight
1, 2	nvidia_nim / nvidia_nim_sub deepseek-v4-flash	ready or 429/529
3	openrouter cohere/north-mini-code	429 → deferred
4, 5	both keys deepseek-v4-pro	ready or 429
6	openrouter dots-3-note	429 → deferred
7, 8	both keys gemma-3-12b	404
9	openrouter gemma-4-26b	429 → deferred
10, 11	both keys gemma-3-4b	404
12	openrouter gemma-4-31b	429 → deferred
13, 14	both keys gemma-4-31b	rejected without status in every pre-#1939 artifact (empty response, cheap)
15	openrouter 5th free (ling-3.0-flash-fin)	429 → deferred
16	nvidia_nim llama-3.2-11b	ready

So in a healthy hour the served set becomes about 5 ready (four deepseek routes plus one llama-11b) + 5 deferred (all OpenRouter) with 6 rejected, versus 2–4 ready today — a real gain, but the readiness target of 8 is not reachable with this order, and the routes that were ready in every pre-#1939 artifact (llama-3.2-90b, muse-glimmer-30b, and the sub key's llama routes) stay past the cap, because the round-robin spends a third of the 16 probes on an account whose free routes have answered 429 at preflight in every artifact since 21:00Z and another quarter on the two dead gemma-3 entries per key.

Two cheap layers, either of which fits this PR's shape and needs no model name:

1. Skip an account after K consecutive transient rejections at preflight (K = 2): OpenRouter's 429 is per key, so after two of them the remaining three probes on that account are spent for a known answer; those three probes then reach nvidia_nim llama-3.2-90b, nvidia_nim_sub llama-3.2-11b and muse-glimmer-30b. Deferral of the two probed routes is unchanged.
2. Family interleave inside each account's list before the cap (the fallback I floated on Review sidecar probes and replays spend the per-key free-tier rate budget the review itself needs #1948): each NVIDIA key's first six candidates become deepseek-flash, gemma-3-12b, llama-11b, muse, minimax-m3, mistral-7b — four working routes of six per key inside the same 16-probe budget, and the readiness target becomes reachable.

A test that would pin either: build the candidate list from the artifact's discovery order (two NVIDIA accounts × 21 models, OpenRouter × 20, 404 for gemma-3-*, 429 for every OpenRouter route) and assert the served set contains at least one non-deepseek NVIDIA route on each key within REVIEW_PREFLIGHT_MAX_PROBES. Today's tree would fail it for the sub key.

Not opening a competing change; the numbers are here so the choice is yours.

---

Generated by Claude Code

[seonghobae]

Taken — head 82ed98d7 (host 1). Your first layer is in, as REVIEW_PREFLIGHT_ACCOUNT_SKIP_AFTER_429 = 2: once an account has answered 429 to two consecutive probes, its remaining candidates are skipped without a probe and the walk continues with the other accounts' next candidates; the two probed routes stay deferred. Replaying your table through the new walk: probes 1–8 as in your rows, OpenRouter skipped from its third slot on, so probes 9–16 land on both keys' gemma-3-4b (404), gemma-4-31b (empty), llama-3.2-11b and llama-3.2-90b — ready 8 at probe 16, deferred 2, skipped 4. That projection is now a test built from your table (test_preflight_reaches_both_keys_llama_routes_under_the_artifact_order: a llama route on each key, ready = target within the probe cap, deferred = exactly the two probed OpenRouter routes); it fails on the previous head with 5 ready. A fully rate-limited hour now costs two probes per account instead of the whole budget.

Two consequences worth stating. The free pool lists all 24 candidates again (probe cap 16): the tail is reachable exactly when an account is skipped, and the report separates skipped_count from the unreached remainder, which answers lane peer 1's earlier unreachable-tail objection rather than reintroducing it. And the skip broke a positional pairing downstream — the deferral pass zipped rows with agents, so a 429 answered after an account had been skipped would have demoted a skipped candidate instead of the route that answered; caught in self-review, fixed by pairing rows with the agents actually probed, pinned by test_preflight_deferral_pairs_rows_with_probed_agents_after_skips. Your artifact-order test alone did not catch it because its deferred rows come before its first skip.

Your second layer (family interleave inside each account's list) is recorded in ADR-0029 as the reserve alternative: it needs a model-family equivalence derived from names, which ADR-0003/#1468 deliberately avoid, so it waits for the post-merge census to show same-family contention as the residual. Gate on this head: 2916 passed, coverage 100 %, interrogate 100 %.

[seonghobae]

One datum and one question for the walk at 82ed98d7, from #1913's 04:21Z artifact (9982569956, pre-bump pin): both keys' deepseek-v4-pro answered TimeoutError at the 90 s probe bound, flash 429 on both, 0 of 12 ready — the first artifact in which the route that had been carrying reviews times out instead of answering.

Replayed through the walk: NVIDIA key A probes flash (429, streak 1), then pro (timeout — line 484 clears the streak, since only a 429 answer restores it), then gemma-3-12b (404, streak stays 0), and so on; the same on key B. So neither NVIDIA account ever reaches REVIEW_PREFLIGHT_ACCOUNT_SKIP_AFTER_429, and each key spends 90 s on pro before its 404s. That is bounded by MAX_PROBES (worst case 16 × 90 s), so it is not a correctness finding. The question is whether a timed-out probe should clear the streak at all: a timeout is not evidence the key recovered, only that the model did not answer. Treating a timeout as neutral (streak unchanged) and letting only an answer — any HTTP status — reset or extend it keeps the rule's meaning ("this key said 429 twice in a row") intact; a pinning test would be flash 429 → pro timeout → gemma 429 on one key, expecting the account skipped after the third probe (today's code leaves it at streak 1). Your call.

---

Generated by Claude Code

[seonghobae]

Head-tree verification and merge under the chicken-and-egg clause (host 1, 2026-09-06 14:1x KST). Same procedure as #1947 and #1950; host 2 has not responded to merge requests since 10:40 KST.

• Head: 27d478d7 (ls-remote = local), base main@ff9848a3 (after #1947 and #1950), 0 behind / 7 ahead. Three-dot delta vs main: 7 files, +407/−26 — launcher, sidecar script, composite action default, ADR-0029 (new; 0028 is carried by open #1668), ADR-0003 §2 sentence, CHANGELOG (own entry on top), runtime-preflight tests. The last two commits are main merges with zero and with #1950's three-file delta; the PR's own seven files are byte-identical to the verified code head 82ed98d7 plus the docs-only 3a1e10b8.
• Gates on every head of this PR: 82ed98d7 2916 passed / coverage 100 % (0 missed) / interrogate 100 %; 27d478d7 2920 passed / 100 % / 100 %.
• Negative controls: the four lazy-fill tests plus the updated stage-limit test fail on #1947's tree; the account-skip and artifact-order tests fail on this PR's previous code head; the row/agent pairing test fails on the pre-fix walk.
• Independent verification: lane peer 1 verified 6d897c99, b270bde1 and 82ed98d7 (streak logic, pairing, free-pool bound; negative control 4/4 on their side) and confirmed 1a49cc93's tree; lane jan rebuilt the real 2026-09-06 candidate order from #1938's artifact — the plain walk reaches about five ready routes, the account skip reaches both keys' llama routes and the target of eight — and that projection is the test_preflight_reaches_both_keys_llama_routes_under_the_artifact_order test.
• Checks on 27d478d7: all queued at merge time (pushed minutes ago). The clause applies as for #1947: this PR changes the review sidecar's launcher, the required review contexts run the base branch's scripts and their verdict dispatch is blocked by the owner-gated #1929 variable, and the current provider capacity fails sidecars at preflight regardless of the tree (#1661 run 34008191123, 04:48Z: ready 0 of 12).
• Why not wait for a #1947 sample first: no sidecar run created after 14:05 KST has completed anywhere in the org (all queued), and the two changes are measured by separable report fields — deferred_count (#1947) versus candidate_count/probed_count/skipped_count (#1949) — so the same artifacts discriminate both, while #1939's regression (ready 1–3 of 12) costs reviews every hour.
• Rollback: a revert PR through the same path if the first post-merge artifacts show ready_count below the pre-fix(review-policy): fill the sidecar catalog round-robin across credential accounts #1939 level. Merge method: merge commit.

[seonghobae]

First post-merge sample (host 1). .github run 34013668803, job 101438328025 (noema-review, created 05:18Z after fb2ae81d, sidecar booted ~06:45Z, failed 07:19Z after 1689 s of serving).

Preflight, from the job log's runtime preflight summary:

field	value
candidate_count	24
probed_count	16
skipped_count	4 — OpenRouter, after its first two probes answered 429
deferred_count	2 — the two probed OpenRouter routes (429)
ready_count	6 — deepseek-v4-flash, deepseek-v4-pro, meta/llama-3.2-11b on both NVIDIA keys
rejected_count	8 — gemma-3-12b/4b 404 ×4, plus four NIM routes silent at the 16-token probe (TimeoutError, 90 s each)
escalations_used	2

Readiness is back at the pre-#1939 level (5–6 of 12) from the post-#1939 1–3, and the routes the four-slot slice could never reach are in the served set, so the rollback rule is not triggered. The target of 8 was missed only because four probes were spent on silent NIM routes, which is the capacity hour, not the walk.

Serving then failed as it did before this work: 45 provider_attempt lines (42 × 1/1, 3 × 1/3), 28 failures — 21 TimeoutError, 7 HTTPError — 26 circuit_* lines (the breaker is now recording), ending in request_failed status=502 code=provider_connection_error after a deepseek-flash timeout. That is the walk-end 502 class at the new pin's 90 s-per-attempt cost: routes that answer a 16-token probe still go silent on the real request in this hour. The lever above this PR stays #1948 (capacity / response-start deadline, owner) and contextual-orchestrator#1045.

[seonghobae]

Cross-check of the first true #1949 sample (run 34013668803) by a second, independent parser on the noema-sidecar-evidence artifact: candidate 24 / probed 16 / skipped 4 / deferred 2 / ready 6 / rejected 8, escalations 2/4 — all six fields agree with host1's read above (ready = 3 nvidia_nim + 3 nvidia_nim_sub incl. llama-3.2-11b on both keys; rejected = 4 HTTPError + 4 TimeoutError; deferred = 2 openrouter). ready 6/12 is the pre-#1939 level → the rollback line is not crossed. Two parsers, one artifact: this is agreement on the read, not a second sample.