diff --git a/CHANGELOG.md b/CHANGELOG.md index ca81dcea1d..46d599a320 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +### Review sidecar catalog interleaves credential accounts + +- `build_zdr_prioritized_catalog` now fills each free/ZDR tier round-robin across independently credentialed accounts instead of in provider-name order. The sidecar exports `ORCHESTRATOR_CATALOG_ACCOUNT_CAP=8` with `ORCHESTRATOR_CATALOG_LIMIT=12`, and the sorted fill took 8 `nvidia_nim` routes and 4 `nvidia_nim_sub` routes before any `openrouter` route was reached, so a review that admitted 62 free routes across three accounts served a NVIDIA-only catalog (`noema-review` run 33969842312: `free_pool_admitted_routes` 62, `free_selected_count` 12, runtime preflight `ready_count` 2 of 12) and the failover loop had no other account to leave a stalled NVIDIA endpoint for -- the `noema-review` 502 class tracked in contextual-orchestrator#1045. Tier order (free before priced, ZDR before non-ZDR), the account cap, the limit, and the discovery-order independence contract are unchanged; the same input now yields 4 + 4 + 4. Contrasts with #1476, which hardens `_routable_discovered_models` against a pin that regresses the OpenRouter `evidence_only` flag: on the current pin (`2e414d15`, includes contextual-orchestrator#949) OpenRouter rows already reach the catalog builder, and the selection was what dropped them. + ### Scheduler holds pre-review branch updates while checks are in flight - `inspect_pr` now decides `wait` instead of `update_branch` when a behind, unreviewed head still has queued or running check runs (`has_in_flight_check_runs`, built on the existing `latest_check_runs`/`running_check_state`). Under a saturated runner queue each PR's own delayed `pull_request_target` scheduler run merged `main` into the head before review dispatch, cancelling every queued check on the old head (22/28 on #1926, 21/30 on #1484) and requeueing the PR at the back, so no head ever completed its checks: 76 of the 77 PRs merged into this repository since 2026-09-04 had 0/12 required contexts satisfied at merge time. The hold has no age cap on purpose -- a check that never finishes keeps the head in place instead of restarting that loop, and the update resumes once every newest check run is terminal. `CLAUDE.md` now describes both update paths. Tracked in #1935. diff --git a/scripts/ci/contextual_orchestrator_review_policy.py b/scripts/ci/contextual_orchestrator_review_policy.py index 910b8da3a9..e609e67ff3 100644 --- a/scripts/ci/contextual_orchestrator_review_policy.py +++ b/scripts/ci/contextual_orchestrator_review_policy.py @@ -13,6 +13,7 @@ from __future__ import annotations import argparse +import itertools import json import math import re @@ -273,6 +274,21 @@ def _free_pool_source_admitted(row: Mapping[str, Any]) -> bool: ) +def _route_tier(row: Mapping[str, Any], zdr_endpoints: frozenset[str]) -> tuple[int, int]: + """Return the ``(cost rank, ZDR rank)`` tier a route is selected within. + + Free routes rank before priced ones and ZDR-attested routes before + unattested ones; the tier is what the catalog fill must never reorder, + while accounts inside one tier may be interleaved freely. + """ + attested = is_zdr_model( + str(row["provider"]), + model=str(row["model"]), + zdr_endpoints=zdr_endpoints, + ) + return (_COST_EVIDENCE_RANK[_cost_evidence(row)], 0 if attested else 1) + + def build_zdr_prioritized_catalog( rows: Iterable[Mapping[str, Any]], *, @@ -317,27 +333,36 @@ def build_zdr_prioritized_catalog( ] eligible_rows.sort( key=lambda row: ( - _COST_EVIDENCE_RANK[_cost_evidence(row)], - 0 - if is_zdr_model( - str(row["provider"]), - model=str(row["model"]), - zdr_endpoints=zdr_endpoints, - ) - else 1, + *_route_tier(row, zdr_endpoints), str(row["provider"]), str(row["model"]), ) ) + # Fill each (cost, ZDR) tier round-robin across independently credentialed + # accounts. A plain sorted fill let the alphabetically first account take + # its whole cap before the next account saw a slot: on 2026-09-05 the review + # sidecar admitted 62 free routes across three accounts and served + # 8 nvidia_nim + 4 nvidia_nim_sub + 0 openrouter (limit 12, cap 8), so a + # stalled NVIDIA endpoint had no other account to fail over to + # (ContextualWisdomLab/.github#1476, contextual-orchestrator#1045). per_account: Counter[str] = Counter() picked: list[Mapping[str, Any]] = [] - for row in eligible_rows: - account = provider_account(str(row["provider"])) - if per_account[account] >= account_cap: - continue - per_account[account] += 1 - picked.append(row) + for _tier, tier_rows in itertools.groupby( + eligible_rows, key=lambda row: _route_tier(row, zdr_endpoints) + ): + queues: dict[str, list[Mapping[str, Any]]] = {} + for row in tier_rows: + queues.setdefault(provider_account(str(row["provider"])), []).append(row) + while queues and len(picked) < limit: + for account in list(queues): + if per_account[account] >= account_cap or not queues[account]: + del queues[account] + continue + picked.append(queues[account].pop(0)) + per_account[account] += 1 + if len(picked) >= limit: + break if len(picked) >= limit: break diff --git a/tests/test_contextual_orchestrator_review_policy.py b/tests/test_contextual_orchestrator_review_policy.py index 4cda949897..41c1ea40b2 100644 --- a/tests/test_contextual_orchestrator_review_policy.py +++ b/tests/test_contextual_orchestrator_review_policy.py @@ -563,3 +563,92 @@ def test_private_catalog_fails_closed_without_attested_zdr_route() -> None: account_cap=4, require_zdr=True, ) + + +def _free_rows(provider: str, count: int, prefix: str) -> list[dict[str, object]]: + """Return ``count`` free discovery rows for one credential account.""" + return [ + { + "provider": provider, + "model": f"{prefix}{i}", + "agent_id": f"{prefix}_{i}", + "is_free": True, + **FREE_PRICE, + } + for i in range(count) + ] + + +def test_build_catalog_interleaves_accounts_within_a_tier() -> None: + """A bounded catalog spreads across admitted accounts instead of filling alphabetically. + + Measured on 2026-09-05 (``noema-review`` run 33969842312): 62 admitted free + routes across three accounts, limit 12, account cap 8, served as + 8 ``nvidia_nim`` + 4 ``nvidia_nim_sub`` + 0 ``openrouter`` because the + sorted fill reached the limit before the alphabetically last account got a + slot -- so a stalled NVIDIA endpoint had no other account to fail over to. + """ + report = { + "models": _free_rows("nvidia_nim", 8, "a") + + _free_rows("nvidia_nim_sub", 8, "b") + + _free_rows("openrouter", 8, "o") + } + result = policy.build_zdr_prioritized_catalog( + policy.parse_discovery_report(report), limit=12, account_cap=8 + ) + providers = [agent["provider_name"] for agent in result["agents"]] + assert providers[:3] == ["nvidia_nim", "nvidia_nim_sub", "openrouter"] + assert providers.count("nvidia_nim") == 4 + assert providers.count("nvidia_nim_sub") == 4 + assert providers.count("openrouter") == 4 + + +def test_build_catalog_interleaving_keeps_zdr_tier_first() -> None: + """Account interleaving never lifts a non-ZDR route above an attested one.""" + report = { + "models": _free_rows("nvidia_nim", 3, "a") + + [ + { + "provider": "openrouter", + "model": "deepseek/deepseek-r1:free", + "agent_id": "or_zdr", + "is_free": True, + **FREE_PRICE, + } + ] + + _free_rows("openrouter", 3, "o") + } + result = policy.build_zdr_prioritized_catalog( + policy.parse_discovery_report(report), + limit=4, + account_cap=8, + zdr_endpoints=ZDR_FEED, + ) + assert result["agents"][0]["model"] == "deepseek/deepseek-r1:free" + assert [agent["provider_name"] for agent in result["agents"]][1:] == [ + "nvidia_nim", + "openrouter", + "nvidia_nim", + ] + + +def test_build_catalog_interleaving_skips_exhausted_accounts() -> None: + """An account with fewer routes than its share hands its turns to the others.""" + report = { + "models": _free_rows("nvidia_nim", 5, "a") + + _free_rows("nvidia_nim_sub", 1, "b") + + _free_rows("openrouter", 2, "o") + } + result = policy.build_zdr_prioritized_catalog( + policy.parse_discovery_report(report), limit=12, account_cap=8 + ) + assert [agent["provider_name"] for agent in result["agents"]] == [ + "nvidia_nim", + "nvidia_nim_sub", + "openrouter", + "nvidia_nim", + "openrouter", + "nvidia_nim", + "nvidia_nim", + "nvidia_nim", + ]