feat(ci): report free-route provider-family diversity for Strix pool evidence - #1433
Conversation
…evidence Add free_family_diversity to the contextual-orchestrator review policy report so a future strix.yml change can evidence-gate a move to orchestrator/free instead of relying on a static default. Strix stays pinned to orchestrator/auto in this PR: ADR-0003's 2026-08-30 addendum and the accompanying doctoring record explain why today's directive instruction to route Strix through orchestrator/free conflicts with the 2026-08-29 single-outage-domain finding, and what evidence a follow-up PR needs before it can safely flip the pin. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
|
Warning Review limit reachedNext included review available in 28 minutes. View limit detailsLimit 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. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
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. Comment |
…job log The review sidecar already computed a safe, regex-validated error_code and http_status when the gateway's own /v1/chat/completions preflight rejected every candidate route, but only wrote it into the CONTEXTUAL_ORCHESTRATOR_PREFLIGHT_EVIDENCE artifact file -- invisible in the job log a CI operator reads first. Every "gateway preflight returned HTTP 502" failure across the org's PR backlog right now (including this PR's own noema-review run, and .github#1429's) looked identical and gave no signal about which upstream failure category caused it. Print the same already-bounded fields directly to the job log so the real cause is visible without downloading the artifact. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
|
Status on this PR's two required-check failures on the initial push (head
Pushed This does not fix the underlying provider-side rejection itself — that still needs the now-visible Generated by Claude Code |
select_nvidia_nim_model.py made real, direct HTTPS calls to integrate.api.nvidia.com (bypassing the contextual-orchestrator gateway) to resolve a live model id for the scheduled autofix worker. That worker was already migrated to the gateway's own auto-discovery; grep across every workflow in the repo confirms zero remaining callers. Removes the script and its test as part of today's direct-NIM-communication cleanup. The opencode.jsonc nvidia-nim/github-models provider catalogs and the opencode-review-dispatch.yml OPENCODE_MODEL_CANDIDATES fallback pool are a separate, actively-tested mechanism (not dead code) and are flagged in docs/doctoring/direct-nvidia-nim-communication-removal.md as a dedicated follow-up rather than removed here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
Records the Strix free-pool diversity-evidence reconciliation (#1433), the gateway-preflight visibility fix, the direct-NIM cleanup, and the four parallel PR-triage agents' real fixes (noema#500, .github#1238/#1271, .github#1435, contextual-orchestrator#857's stale workflow removal) under section 5.1's next-increment list -- a different location than #1432's in-flight "2026-08-30 autonomous loop snapshot" append, to avoid colliding with that PR's own diff. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
|
Exact-head scope review at Please keep #1433 as the narrow evidence producer and split/revert the direct-NIM dead-code/docs cleanup to its actual owner lane. Bundling deletion of a separate provider helper and its tests into the evidence PR makes it harder to verify that the new diversity signal alone is safe, conflicts with the stated “additive report field only” developer-impact claim, and overlaps #1437's broad cleanup. Preserve the negative diversity fixture and keep |
The sidecar's internal launcher preflight (_preflight_review_agents, run before the server ever answers healthz) is a distinct failure layer from the sidecar script's external gateway smoke test fixed in 57c04ea. It already computed a bounded per-route report (agent_id/provider/model/error_type/ http_status, never raw provider text) but only wrote it to the --preflight-out artifact file; the sanitized stderr stream showed only "review sidecar preflight failed" with the real routes silently dropped into omitted_unstructured_lines. This is the failure mode PR .github#1433's own noema-review hit on its second push. Print one bounded, independently re-validated diagnostic line per rejected route (including the nested primary_attempt when the priced fallback pool also rejects everything) and allowlist the new format in the stream sanitizer. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
|
Follow-up on Same gap, different layer: that preflight's bounded report (agent_id/provider/model/error_type/http_status — already sanitized, never raw provider text) was written only to the Still watching for the next run's actual Generated by Claude Code |
|
Done — pushed Checked Generated by Claude Code |
…sion .github#1437 decided the Strix orchestrator/free question directly rather than waiting on the evidence threshold this addendum proposed. Point to ADR-0020 as the operative decision; keep this addendum's analysis as the record of this PR's own reasoning. free_family_diversity remains useful monitoring evidence either way. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
…-pass Merging main into this branch picked up #1436, which independently fixes a different bug in the same sidecar script: the post-healthz gateway smoke request's max_tokens was desynchronized from REVIEW_MAX_OUTPUT_TOKENS, so a reasoning-capable route the launcher's own preflight had already proved ready could still exhaust its budget on internal reasoning before visible output, failing the separate smoke request with 502 invalid_structured_output. This is the precise mechanism behind the earlier PR #1433 "healthz reached, then 502" signature this doc described without yet knowing the cause -- a genuinely different bug from this session's own family-cap/stale-model finding, not a duplicate. Recorded so the two fixes aren't mistaken for redundant work. Co-Authored-By: Claude <noreply@anthropic.com>
…chestrator-q05283 # Conflicts: # docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md # tests/test_contextual_orchestrator_review_runtime_preflight.py
|
Merged Resolved the merge conflicts by keeping main's authoritative ADR-0003 amendment as-is and repositioning my ADR text as a short note that This PR is now just the diversity-evidence field itself, current with main, no longer blocking or being blocked by anything. Generated by Claude Code |
…ma-opencode-strix-orchestration-sexqzc Pulls in #1433's free_family_diversity evidence foundation (scripts/ci/contextual_orchestrator_review_policy.py) plus its bundled preflight-evidence-surfacing fixes and NIM-cleanup, ahead of restructuring this branch's Strix pool-routing change into an evidence-gated conditional per the exact-head review on #1437. Resolved conflicts in docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md and docs/product-goal-directive.md by combining both branches' narratives into one that reflects the final evidence-gated state (both documents are rewritten further in a follow-up commit on this branch).
…ence Replaces the unconditional pool flip a human exact-head governance review rejected on this PR with the evidence-gated conditional the review required: strix.yml now reads free_family_diversity (added by #1433, merged into this branch above) from the sidecar's own discovery run and selects orchestrator/free only when it is >= 2, falling back to orchestrator/auto --- which the sidecar always boots regardless of the resolved model name, so the fallback is a genuine priced-route safety net, not an alias for the same single-family catalog --- in every other case, including any evidence that is missing, unreadable, or malformed. - .github/workflows/strix.yml: new "Resolve Strix model from free-route diversity evidence" step; "Gate Strix secrets" reverted to its static orchestrator/auto base and auto-only override allowlist; "Prepare Strix model input file" now accepts both pool literals, sourced from the new step's output. - tests/test_strix_contextual_orchestrator_contract.py: negative fixture proving diversity 0/1 (and every malformed-evidence shape) stays on orchestrator/auto, executed against the workflow's own extracted step (this repo's established pattern for testing embedded workflow-YAML behavior, already used for the neighboring "Gate Strix secrets" step). - scripts/ci/strix_required_workflow_smoke.sh: extends (never weakens) the prior bare-pin assertions into assert_free_pool_gated_by_diversity, a structural check that orchestrator/free is reachable only inside the diversity conditional and the safe auto default is always set first. - Reconciled docs/adr/0003 and docs/product-goal-directive.md's merge conflicts (both branches independently documented the Strix pool question) into one narrative describing the corrected, evidence-gated outcome. Rewrote docs/adr/0020-strix-orchestrator-free-pool.md end to end: the rejected unconditional draft, the corrected decision, honest status of the review's 5 acceptance criteria (request-time failover not confirmed merged upstream; no dedicated "Strix canary" mechanism found beyond the existing push-trigger convention), and residual risk. - Reverted the 4 stale-NVIDIA-NIM-doc corrections and the gap-baseline removal-target section this PR's first draft bundled in alongside the pool change; they move to a separate branch/PR per the review's fifth criterion (scripts/ci/select_nvidia_nim_model.py's own deletion remains inherited from #1433's independent commit via the merge above). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Use owner/repo#num convention only for cross-repo references; same-repo mentions of #1433/#1434/#1437 use bare #num. - Point the sidecar's CATALOG_FAMILY_CAP comment at a durable commit SHA instead of the removing branch's name, which disappears once the branch is deleted after merge. - Correct the doctoring doc's grep-audit claim: one intentional code comment still names select_nvidia_nim_model.py on purpose as a searchable history pointer, so "zero references" was inaccurate.
- Recompute free_family_diversity in _with_discovery_counts from the full discovery-wide row set rather than trusting whichever narrower rows build_zdr_prioritized_catalog was called with: the auto-pool primary stage only sees ZDR-admitted free rows (undercounting under --require-zdr) and the priced-fallback stage sees no free rows at all (always reading zero), both contradicting the field's documented "among all discovered free routes" contract. - Clarify free_family_diversity's docstring: it measures discovery-time route diversity, not preflight-confirmed readiness. - Correct docs/product-goal-directive.md's now-stale note: Strix already moved to orchestrator/free on main (merged #1434), it did not stay on orchestrator/auto pending a future evidence-gated follow-up. - Fix a stale test comment describing a malformed error_type as printing unchanged, when _log_preflight_rejections actually replaces it with the bounded "UnknownError" placeholder; add a positive assertion for that replacement. Full suite: 1903 passed, 1 skipped. Coverage 99% (single pre-existing, separately tracked gap in scripts/ci/pingora_edge_policy.py:345, unrelated to this change). interrogate: 100%.
|
Pushed
Full suite: 1903 passed, 1 skipped. Coverage 99% (the one gap is the pre-existing, separately-tracked Generated by Claude Code |
| "free_family_diversity": len( | ||
| { | ||
| provider_family(str(row["provider"])) | ||
| for row in rows | ||
| if row.get("cost_evidence") == "free" | ||
| } | ||
| ), |
There was a problem hiding this comment.
📝 Info: Full-discovery diversity survives staging
_with_discovery_counts uses normalized discovery rows, so ZDR filtering and priced fallback cannot understate the monitoring signal.
Was this helpful? React with 👍 or 👎 to provide feedback.
| provider_family(str(row["provider"])) | ||
| for row in rows | ||
| if row.get("cost_evidence") == "free" |
There was a problem hiding this comment.
📝 Info: Provider access follows validation
Both callers pass parse_discovery_report output, which rejects missing providers before row["provider"] is accessed.
Was this helpful? React with 👍 or 👎 to provide feedback.
Each finding was verified against the actual ADR text and the launcher/sidecar source before acting, per this repo's convention of never accepting or dismissing an automated review finding unverified. Two were real design flaws in the first draft: 1. The original decision reused a fixed tiny max_tokens (matching upstream probe()'s precedent of 1) for every per-candidate probe -- this reproduces the exact reasoning-budget-starvation bug the whole investigation started from, one layer down, and a fixed budget is itself the kind of rule-of-thumb this repo's conventions forbid. Fixed: per-candidate probes now escalate to a larger budget only on positive evidence (empty content AND finish_reason == "length", the provider-documented signature of "budget too small," not "down"). Genuinely-down candidates never reach the retry path. 2. The original decision replaced the sidecar's real end-to-end virtual-pool smoke request with per-candidate checks alone. Verified directly: the 2026-08-30 gap-baseline entry for PR #1433 already documents a live case where per-candidate preflight passed while the virtual-pool request still 502'd -- a different code path entirely. Fixed: both existing preflight layers are kept; neither is removed. Also fixed: a mischaracterization (the launcher's _preflight_review_agents/_preflight_with_fallback already exist and do per-candidate N-of-M-tolerant probing today -- confirmed by reading the source; the ADR now describes fixing them, not introducing them); conflated context-window vs max-output-tokens treated as separate, independently-nullable fields per OpenRouter's live OpenAPI schema (fetched and verified, not assumed); real external citations for provider-behavior claims (OpenAI and OpenRouter docs, fetched live); and the two upstream asks are now real tracked issues (ContextualWisdomLab/contextual-orchestrator#926, #927) instead of prose. Also folds in a fresh, directly-verified live reproduction: noema-review failed on this ADR's own PR (#1449, job 99253418179) with exactly the bug under discussion -- Layer 1 passed in 30s, Layer 2 then hung the full 120s with zero bytes back -- confirming this is an active defect, not a theoretical one. Co-Authored-By: Claude <noreply@anthropic.com>
… on large repos The central required Strix Security Scan was deterministically timing out on contextual-orchestrator (a large, actively-growing repo: ~10,000+ statements, thousands of tests). Two consecutive real runs (ContextualWisdomLab/contextual-orchestrator#933, jobs 99317982462 and 99331265840) both failed identically with exit code 124 -- the scanner's own process-timeout wrapper killing an in-progress, zero-finding scan, not a reported vulnerability or a genuine provider outage. This org's standing operating directive (docs/product-goal-directive.md) explicitly accepts that central OpenCode/Strix/Noema scans may take more than two hours per model. The previous budgets (90-minute process / 95-minute total / 100-minute outer deadline / 100-minute step / 120-minute job) did not honor that tolerance for a repo this size. Raise every budget in the same chain, preserving proportional ordering and buffers (process < total < outer-deadline < step-timeout < job-timeout): - process_budget_seconds: 5400 -> 9000 (150 min) - STRIX_TOTAL_TIMEOUT_SECONDS: 5700 -> 9300 (155 min) - strix_gate_deadline outer bound: +6000 -> +9600 (160 min) - "Run Strix (quick)" step timeout-minutes: 100 -> 170 - strix job timeout-minutes: 120 -> 200 200 minutes stays comfortably under GitHub Actions' 360-minute hosted-runner job timeout cap, with ~30 minutes of margin for the job's other steps (checkout, sidecar provisioning, artifact upload). Retry/backoff mechanics (STRIX_GATE_RETRY_BACKOFF_SECONDS, the 3-attempt bounded retry count) are untouched -- this is purely a time-ceiling fix, not a retry-logic change. Updates the matching contract assertions in scripts/ci/test_strix_quick_gate.sh so the pinned numeric strings stay in lockstep with strix.yml. Verified: actionlint on strix.yml (clean), full `coverage run -m pytest tests` (1903 passed, 1 skipped, 21 subtests, no regressions -- the one pre-existing pingora_edge_policy.py coverage gap was already fixed upstream on this branch by the time of push), `interrogate` (100%), `bash -n` on the edited script, and a full real run of `bash scripts/ci/test_strix_quick_gate.sh` (PASS, ~5 min with the fast CI fixture env vars). Refs: ContextualWisdomLab/contextual-orchestrator#933 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
Added: fix(ci) — raise Strix scan time budgets to stop deterministic timeouts on large reposCommit: 882932b BugThe central required Strix Security Scan ( Exit code 124 is the scanner's own internal process-timeout wrapper killing an in-progress, zero-finding scan ( FixThis org's own standing operating directive (
200 minutes stays comfortably under GitHub Actions' 360-minute hosted-runner job cap (~160 min margin), with ~30 min left for the job's other steps (checkout, sidecar provisioning, artifact upload). Retry/backoff mechanics ( Also updated the matching pinned-string contract assertions in Test evidence
Refs: ContextualWisdomLab/contextual-orchestrator#933 (the PR whose required Strix check surfaced this) Generated by Claude Code |
…1270) Central PR merge scheduler now refuses direct-merge and auto-merge until GitHub reports reviewDecision APPROVED plus a non-author, non-OpenCode formal approval bound to the exact live head; disarms stale auto-merge otherwise. Repairs check-run chronology (check-suite creation time as the ordering signal via a shared check_run_recency_key/_newest_check_run_per_identity helper, so a canceled-before-start or queued rerun cannot lose to an older completed run merely because startedAt is null -- this fix now also covers strix_evidence_state's latest_check_run_attempts path, not just failed_status_checks) and re-validates approval immediately before every merge-authorizing mutation (closes a TOCTOU window where a same-head approval revoked mid-invocation could still authorize merge). Eight+ rounds of reproduce-before/confirm-after TDD fixes across this PR's lifetime, each independently verified by Devin Review and CodeRabbit with all findings resolved; full suite passing with 100% coverage and docstrings on scripts/ci. Merge commit 98aa3a1 resolves a real conflict against main (from #1433) where two PRs independently fixed the same check-run dedup logic differently -- unified on the correct implementation rather than picking one side. Bypass-merged (as a merge commit, not squash, to preserve the resolved-conflict history) per explicit user authorization: exact-head Security Scan, SAST, CodeQL, OSV, SBOM, Scorecard, Secret Scan, and Strix are terminal success; the required OpenCode/Noema formal-verdict dispatch has not completed for reasons independent of this PR's own correctness (a separate, already-diagnosed central review-gate defect tracked on #1198), and this scheduler hardening is foundational for every other PR's merge safety.
2 participants
Summary
Today's
product-goal-directive.mdloop instance asked that Noema, OpenCode,and Strix all route review through
contextual-orchestrator'sorchestrator/freepool. Noema and OpenCode already do(ADR-0003). Strix does not — it is pinned to
orchestrator/auto, and thatpin is not an oversight: it is an accepted ADR-0003 decision backed by a
dated finding (2026-08-29: every discovered free route shared the OpenRouter
outage domain, so a strict
orchestrator/freepin with no paid fallbackwould have taken Strix's required security review dark on that provider's
outage).
Per
docs/product-goal-directive.md's own conflict policy ("resolve theconflict and update whichever document is wrong — do not silently pick
one"), this PR does not flip the pin on the strength of the instruction
alone. It instead:
free_family_diversitytoscripts/ci/contextual_orchestrator_review_policy.py's catalog report: thecount of distinct outage-domain provider families among all discovered
free routes, independent of which pool was requested. This turns "is a
strict free pool safe right now" into evidence recomputed on every
discovery run instead of a static assumption.
docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md(2026-08-30 addendum) and
docs/doctoring/contextual-orchestrator-strix-free-diversity-evidence.md,and adds a same-date note in
docs/product-goal-directive.md§8 pointingfuture loop iterations at this resolution instead of the bare instruction
text.
.github/workflows/strix.ymluntouched. It is apull_request_targetrequired workflow with a deliberately hardened
STRIX_MODEL/STRIX_LLMallowlist; a follow-up PR should wire it to read
free_family_diversityfrom the sidecar's policy report and select
orchestrator/freeonly whenit is
>= 2, falling back toorchestrator/autootherwise.Developer experience
scripts/ci/contextual_orchestrator_review_policy.py: additive reportfield only; existing callers (the sidecar just passes the report through as
an evidence file path — no strict schema check) are unaffected.
tests/test_contextual_orchestrator_review_policy.py: two new tests —diversity of 4 on the existing five-provider fixture, and a regression test
reproducing the 2026-08-29 single-family shape (both NVIDIA keys only →
diversity of 1).
PYTHONPATH=. coverage run -m pytest tests→ 1882 passed, 1skipped.
coverage report --include=scripts/ci/contextual_orchestrator_review_policy.py→ 100%.
interrogatedocstring coverage unaffected (module docstringextended, not removed).
User experience
No change to any CI consumer's behavior. Strix continues on
orchestrator/auto; this PR only adds the evidence a future, separatelyreviewed change needs to safely move it to
orchestrator/free.Test plan
PYTHONPATH=. coverage run -m pytest tests -q(1882 passed, 1 skipped)coverage report --include=scripts/ci/contextual_orchestrator_review_policy.py --show-missing(100%)interrogate -v scripts/ci/contextual_orchestrator_review_policy.py(module docstring extended)strix.ymlto readfree_family_diversityandconditionally select
orchestrator/freeGenerated by Claude Code