maint-86: prepare (not auto-apply) verifier model promotions (#2819 move 3) - #2834
Conversation
…ove 3) Move 3 of the self-feeding verifier-model promotion system. Moves 1-2 made the evaluation evidence flow on its own; this turns a passing benchmark into a *prepared* selection change that a human approves by merging a PR — human_approval_required stays true. - tools/prepare_model_promotion.py: find_promotions() prepares a candidate only when it is the SAME FAMILY as the incumbent (openai gpt-<major>, anthropic claude-<line>), PASSED every quality gate (incl. paired non-inferiority), and costs <= the incumbent per accepted review. apply_promotion() writes the registry mutation and records the prior selection in selection_history (new). find_rollbacks()/apply_rollback() are the inverse: revert to the prior selection when the active model shows a failed workload-benchmark (quality-gate breach). Pure/deterministic given an injected `today`; CLI exits 10 when a change is prepared. - maint-86: dispatch-only workflow that evaluates a benchmark artifact, prepares the change, and opens a PR that is NOT auto-merged (merge = approval). The schedule trigger + pilot->benchmark bridge are deliberately deferred until the corpus reaches the approval minimum (grown by maint-79) so this cannot fabricate a promotion from thin data. - Registered maint-86 in EXPECTED_NAMES + the WORKFLOWS.md / WORKFLOW_SYSTEM.md inventory; documented the mechanism in MODEL_SELECTION_POLICY.md. Verified: 12 promoter tests incl. a promote->breach->rollback round-trip; CI pinned black/ruff/mypy clean; against the real registry it correctly prepares claude-opus-4-6 -> claude-opus-4-8 (same-family, passed, cheaper). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
GitHub Actions
Inactive
Workflow source neededPR #2834 needs either a linked GitHub issue or one valid non-issue Workflow Source before PR metadata automation can manage it safely. Please do one of:
Once a valid source is present, this warning will not be reposted. |
📝 WalkthroughWalkthroughAdds deterministic benchmark-driven model promotion and rollback preparation, a manually dispatched GitHub Actions workflow that opens human-reviewed registry PRs, comprehensive tests, and supporting policy and workflow documentation. ChangesModel promotion preparation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Maintainer
participant GitHubActions
participant prepare_model_promotion
participant ModelRegistry
participant HumanReviewPR
Maintainer->>GitHubActions: dispatch workflow with benchmark_path and mode
GitHubActions->>prepare_model_promotion: run promotion preparation
prepare_model_promotion->>ModelRegistry: evaluate and write prepared selection
prepare_model_promotion-->>GitHubActions: return code 10 when changed
GitHubActions->>HumanReviewPR: open registry update PR
HumanReviewPR-->>Maintainer: require human merge approval
Possibly related issues
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| runs-on: ubuntu-latest | ||
| timeout-minutes: 15 | ||
| steps: | ||
| - uses: actions/checkout@v7 |
| - uses: actions/checkout@v7 | ||
| with: | ||
| persist-credentials: false | ||
| - uses: actions/setup-python@v6 |
| exit 0 | ||
| - name: Open promotion PR (human merges to approve) | ||
| if: steps.prep.outputs.rc == '10' | ||
| uses: peter-evans/create-pull-request@v8 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44d1d2755f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| --write config/model_registry.json | tee prepare.log | ||
| rc=$? |
There was a problem hiding this comment.
In maint-86-model-promotion-prepare.yml, this pipeline returns tee's status because pipefail is not enabled, so a successful preparation where Python exits 10 records rc=0 and the PR step at line 70 never runs; input errors are likewise swallowed. Capture ${PIPESTATUS[0]} or enable set -o pipefail before reading the status.
Useful? React with 👍 / 👎.
| evidence_ids = list(selection.get("evidence_ids", [])) | ||
| if promotion["evidence_id"] not in evidence_ids: | ||
| evidence_ids.append(promotion["evidence_id"]) | ||
| selection["model_id"] = promotion["to_model_id"] | ||
| selection["evidence_ids"] = evidence_ids |
There was a problem hiding this comment.
When a candidate qualifies, this adds its evidence ID to the selection but never copies the corresponding registry_evidence object into the registry's evidence array. The generated PR therefore triggers maint-77-model-registry-freshness.yml, whose missing_evidence check treats that dangling ID as a blocking structural finding, so every promotion PR will fail its model-registry gate.
Useful? React with 👍 / 👎.
| f"{promotion['to_model_id']} ({promotion['reason']})" | ||
| ) | ||
| mutated = apply_promotion(mutated, promotion, today=today) | ||
| for rollback in rollbacks: |
There was a problem hiding this comment.
In auto mode, a report can contain a failed active baseline and a cheaper passing same-family candidate, causing both promotions and rollbacks to be populated. The promotion loop first inserts the failed incumbent at the front of history; this subsequent rollback then reads that mutated history and restores the failed incumbent rather than the prior model identified by find_rollbacks, producing a prepared PR that performs neither intended action. Auto mode should select one path or apply rollback against its explicit target.
Useful? React with 👍 / 👎.
Automated Status SummaryHead SHA: 1446583
Coverage Overview
Coverage Trend
Top Coverage Hotspots (lowest coverage)
Low Coverage Files (<50.0%)
Updated automatically; will refresh on subsequent CI/Docker completions. Keepalive checklistScopeNo scope information available Tasks
Acceptance criteria
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/maint-86-model-promotion-prepare.yml (2)
69-89: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPin the third-party
create-pull-requestaction to a commit SHA.
peter-evans/create-pull-request@v8(line 71) is pinned to a mutable major-version tag rather than a commit SHA. Since this workflow is synced fleet-wide across consumer repos, a compromised or re-pointed tag on this third-party action would replicate everywhere.🔒 Proposed fix
- - uses: peter-evans/create-pull-request@v8 + - uses: peter-evans/create-pull-request@<full-length-commit-sha> # v8.x.xAs per path instructions, "Flag template-injection, unpinned third-party actions, and spoofable bot-actor checks — this workflow YAML is synced across the consumer repos, so one bug replicates fleet-wide."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/maint-86-model-promotion-prepare.yml around lines 69 - 89, Update the create-pull-request action in the “Open promotion PR (human merges to approve)” step to reference a full immutable commit SHA instead of the mutable `@v8` tag. Preserve the existing action version and all surrounding workflow inputs.Source: Path instructions
1-89: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winMirror this workflow to
templates/consumer-repo/.github/workflows/maint-86-model-promotion-prepare.yml. The consumer template has no matching copy, so this new workflow won’t propagate to consumer repos.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/maint-86-model-promotion-prepare.yml around lines 1 - 89, Copy the complete Maint 86 model promotion prepare workflow, including its workflow_dispatch inputs, preparation step, outputs, and pull-request creation step, into templates/consumer-repo/.github/workflows/maint-86-model-promotion-prepare.yml so consumer repositories receive the same workflow.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/maint-86-model-promotion-prepare.yml:
- Around line 45-68: Update the “Prepare promotion/rollback” step’s piped Python
command so rc captures the Python process exit status rather than tee’s status,
using PIPESTATUS[0] or explicitly configuring shell: bash. Preserve the existing
rc-based handling, including the input-error path and promotion-gate status.
In `@tests/tools/test_prepare_model_promotion.py`:
- Around line 121-130: Extend the promotion tests around find_promotions to
cover equal-cost same-family candidates, asserting the candidate with lower p95
latency wins, and add a report containing qualifying candidates from two
providers, asserting one independent promotion is returned for each provider.
Preserve the existing cost-first and one-winner-per-provider behavior, including
deterministic to_model_id ordering when cost and latency also tie.
In `@tools/prepare_model_promotion.py`:
- Around line 109-169: Prevent promotion and rollback proposals from targeting
the same profile/provider in a single run. In main(), after evaluating
find_promotions and find_rollbacks against the original registry, suppress the
conflicting rollback (or otherwise give promotion precedence) before sequential
application. Ensure the resulting rollback rationale and from_model_id cannot
describe a rollback that was skipped or applied against a mutated selection.
- Around line 162-169: Update the sorting key in the proposal-selection logic to
treat only a missing p95_latency_ms value as infinite, while preserving
legitimate zero latency as the fastest value. Keep the existing candidate_cost,
latency, and to_model_id ordering and best_by_provider selection unchanged.
---
Outside diff comments:
In @.github/workflows/maint-86-model-promotion-prepare.yml:
- Around line 69-89: Update the create-pull-request action in the “Open
promotion PR (human merges to approve)” step to reference a full immutable
commit SHA instead of the mutable `@v8` tag. Preserve the existing action version
and all surrounding workflow inputs.
- Around line 1-89: Copy the complete Maint 86 model promotion prepare workflow,
including its workflow_dispatch inputs, preparation step, outputs, and
pull-request creation step, into
templates/consumer-repo/.github/workflows/maint-86-model-promotion-prepare.yml
so consumer repositories receive the same workflow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 036b7fc4-d42b-4b6e-a2ff-a3ea363c0f50
📒 Files selected for processing (7)
.github/workflows/maint-86-model-promotion-prepare.ymldocs/MODEL_SELECTION_POLICY.mddocs/ci/WORKFLOWS.mddocs/ci/WORKFLOW_SYSTEM.mdtests/tools/test_prepare_model_promotion.pytests/workflows/test_workflow_naming.pytools/prepare_model_promotion.py
| - name: Prepare promotion/rollback | ||
| id: prep | ||
| env: | ||
| BENCHMARK_PATH: ${{ inputs.benchmark_path }} | ||
| MODE: ${{ inputs.mode }} | ||
| run: | | ||
| set +e | ||
| python -m tools.prepare_model_promotion \ | ||
| --benchmark "$BENCHMARK_PATH" \ | ||
| --mode "$MODE" \ | ||
| --write config/model_registry.json | tee prepare.log | ||
| rc=$? | ||
| echo "rc=$rc" >> "$GITHUB_OUTPUT" | ||
| { | ||
| echo '## Model promotion prepare' | ||
| echo '```' | ||
| cat prepare.log | ||
| echo '```' | ||
| } >> "$GITHUB_STEP_SUMMARY" | ||
| if [ "$rc" = "2" ]; then | ||
| echo "::error::prepare_model_promotion input error." | ||
| exit 2 | ||
| fi | ||
| exit 0 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== workflow snippet ==\n'
sed -n '35,80p' .github/workflows/maint-86-model-promotion-prepare.yml
printf '\n== template workflow snippet ==\n'
sed -n '35,80p' templates/consumer-repo/.github/workflows/maint-86-model-promotion-prepare.yml 2>/dev/null || true
printf '\n== shell/default hints ==\n'
rg -n 'defaults:|shell:' .github/workflows/maint-86-model-promotion-prepare.yml templates/consumer-repo/.github/workflows/maint-86-model-promotion-prepare.ymlRepository: stranske/Workflows
Length of output: 1923
🌐 Web query:
GitHub Actions default shell ubuntu-latest bash --noprofile --norc -eo pipefail unspecified shell documentation
💡 Result:
In GitHub Actions, the command used to execute bash script steps depends on whether you specify the shell explicitly [1]. If you do not specify a shell (i.e., it is unspecified/default), GitHub Actions uses the following command on Linux and macOS [1]: bash -e {0} If you explicitly specify shell: bash, GitHub Actions uses a more restrictive set of flags [1]: bash --noprofile --norc -eo pipefail {0} The differences between these two are significant: 1. --noprofile --norc: When shell: bash is specified, these flags are included to ensure a clean shell environment by preventing the loading of system-wide or user-specific profile/initialization files (e.g., ~/.bashrc, ~/.profile) [2][3][4]. The default/unspecified shell does not explicitly include these flags [1]. 2. pipefail: When shell: bash is specified, -o pipefail is enabled [5][1]. This causes a pipeline to return a non-zero exit code if any command in the pipeline fails [5]. The default/unspecified shell (-e only) only tracks the exit code of the final command in the pipeline [6][1]. In both cases, the -e (errexit) flag is used, which causes the step to exit immediately if a command returns a non-zero status [5][6][2]. Top results: [5][6][1]
Citations:
- 1: https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax
- 2: https://onmyway133.com/posts/how-to-handle-shell-script-with-github-action/
- 3: https://boostsecurityio.github.io/lotp/tool/bash
- 4: https://stackoverflow.com/questions/9357464/how-to-start-a-shell-without-any-user-configuration
- 5: https://copdips.com/2023/11/github-actions-bash-shell--e--o-pipefail.html
- 6: Fail-fast/pipefail behaviour for default shell inconsistent with documentation actions/runner#1955
Capture the pipeline exit code explicitly
rc=$? here reads tee’s status because the step uses the default Linux shell (bash -e), so both the rc == 2 error path and the rc == 10 promotion gate can be skipped. Use rc=${PIPESTATUS[0]} or set the step to shell: bash.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/maint-86-model-promotion-prepare.yml around lines 45 - 68,
Update the “Prepare promotion/rollback” step’s piped Python command so rc
captures the Python process exit status rather than tee’s status, using
PIPESTATUS[0] or explicitly configuring shell: bash. Preserve the existing
rc-based handling, including the input-error path and promotion-gate status.
| def test_cheapest_same_family_candidate_wins_per_provider(): | ||
| report = _report( | ||
| [ | ||
| _result("claude-opus-4-6", "anthropic", status="passed", cost=0.10), | ||
| _result("claude-opus-4-8", "anthropic", status="passed", cost=0.09), | ||
| _result("claude-opus-4-7", "anthropic", status="passed", cost=0.05), | ||
| ] | ||
| ) | ||
| props = pmp.find_promotions(report, _registry()) | ||
| assert len(props) == 1 and props[0]["to_model_id"] == "claude-opus-4-7" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Missing coverage for latency tie-break and multi-provider selection.
find_promotions documents "cheapest, then lowest-latency" per-provider winner selection and iterates across all providers in the report, but no test here exercises the latency tie-break (two same-family candidates with equal cost) or a report with two different providers each qualifying for an independent promotion. These are exactly the branches the "one winner per provider" sort key (candidate_cost, p95_latency_ms or float("inf"), to_model_id) is meant to protect.
As per path instructions, "Prioritize correctness, error handling, and test coverage. Flag new or changed behavior with no accompanying test."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/tools/test_prepare_model_promotion.py` around lines 121 - 130, Extend
the promotion tests around find_promotions to cover equal-cost same-family
candidates, asserting the candidate with lower p95 latency wins, and add a
report containing qualifying candidates from two providers, asserting one
independent promotion is returned for each provider. Preserve the existing
cost-first and one-winner-per-provider behavior, including deterministic
to_model_id ordering when cost and latency also tie.
| def find_promotions( | ||
| report: dict[str, Any], registry: dict[str, Any], *, profile: str = DEFAULT_PROFILE | ||
| ) -> list[dict[str, Any]]: | ||
| """Return prepared same-family, passing, cost<= promotions for the profile. | ||
|
|
||
| At most one promotion per provider (the cheapest, then lowest-latency, of the | ||
| qualifying same-family candidates). | ||
| """ | ||
| incumbent_id = str(report.get("baseline_model_id", "")).strip() | ||
| if not incumbent_id: | ||
| return [] | ||
| incumbent_result = _result_for(report, incumbent_id) | ||
| incumbent_cost = _cost(incumbent_result) if incumbent_result else None | ||
|
|
||
| proposals: list[dict[str, Any]] = [] | ||
| for result in report.get("results", []): | ||
| model_id = str(result.get("model_id", "")).strip() | ||
| provider = str(result.get("provider", "")).strip() | ||
| if not model_id or model_id == incumbent_id: | ||
| continue | ||
| selection = _selection_for(registry, profile, provider) | ||
| if selection is None: | ||
| continue | ||
| # Only prepare a swap of the *actual* incumbent for this provider/profile. | ||
| if str(selection.get("model_id", "")).strip() != incumbent_id: | ||
| continue | ||
| if result.get("status") != "passed": | ||
| continue | ||
| if model_family(provider, model_id) != model_family(provider, incumbent_id): | ||
| continue | ||
| cand_cost = _cost(result) | ||
| if cand_cost is None or incumbent_cost is None or cand_cost > incumbent_cost: | ||
| continue | ||
| evidence_id = _evidence_id(report, provider, model_id) | ||
| if not evidence_id: | ||
| continue | ||
| proposals.append( | ||
| { | ||
| "profile": profile, | ||
| "provider": _normalize_provider(provider), | ||
| "from_model_id": incumbent_id, | ||
| "to_model_id": model_id, | ||
| "evidence_id": evidence_id, | ||
| "incumbent_cost": incumbent_cost, | ||
| "candidate_cost": cand_cost, | ||
| "p95_latency_ms": (result.get("metrics") or {}).get("p95_latency_ms"), | ||
| "reason": ( | ||
| f"same-family ({model_family(provider, model_id)}) non-inferior pass at " | ||
| f"cost/accepted {cand_cost} <= incumbent {incumbent_cost}" | ||
| ), | ||
| } | ||
| ) | ||
|
|
||
| # One winner per provider: cheapest, then lowest latency. | ||
| best_by_provider: dict[str, dict[str, Any]] = {} | ||
| for proposal in sorted( | ||
| proposals, | ||
| key=lambda p: (p["candidate_cost"], p["p95_latency_ms"] or float("inf"), p["to_model_id"]), | ||
| ): | ||
| best_by_provider.setdefault(proposal["provider"], proposal) | ||
| return list(best_by_provider.values()) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift
Promotion and rollback can silently collide for the same profile/provider in one run.
find_promotions and find_rollbacks are both evaluated (mode auto, the CLI and workflow default) against the original registry, then applied sequentially in main() (lines 313-325). If the incumbent's own result is "failed" (triggering a rollback for that profile/provider) while a different, cheaper same-family candidate simultaneously "passed" (triggering a promotion for the same profile/provider), both proposals target the same selection:
apply_promotionruns first, moving the incumbent intoselection_history[0]and setting the active model to the new candidate.apply_rollbackthen re-resolvesprior_indexon the now-mutated history (line 254-262) and finds index 0 — the just-superseded, failed incumbent — and reverts the active model back to it.
The prepared PR ends up reverting the legitimate promotion back to the model that just failed, while rollback["reason"]/rationale still reference the stale, pre-mutation from_model_id, making this hard for a human reviewer to spot in the diff/PR body.
🐛 Suggested guard
promotions = (
find_promotions(report, registry, profile=args.profile)
if args.mode in {"promote", "auto"}
else []
)
rollbacks = (
find_rollbacks(report, registry, profile=args.profile)
if args.mode in {"rollback", "auto"}
else []
)
+ # A profile/provider that is being promoted this run should never also be
+ # rolled back in the same run (rollback would revert the promotion using
+ # stale from/to ids and a stale history index).
+ promoted_keys = {(p["profile"], p["provider"]) for p in promotions}
+ rollbacks = [r for r in rollbacks if (r["profile"], r["provider"]) not in promoted_keys]No test in the provided snippets exercises this combined case; consider adding one alongside the fix.
Also applies to: 206-232, 282-334
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/prepare_model_promotion.py` around lines 109 - 169, Prevent promotion
and rollback proposals from targeting the same profile/provider in a single run.
In main(), after evaluating find_promotions and find_rollbacks against the
original registry, suppress the conflicting rollback (or otherwise give
promotion precedence) before sequential application. Ensure the resulting
rollback rationale and from_model_id cannot describe a rollback that was skipped
or applied against a mutated selection.
Source: Path instructions
| # One winner per provider: cheapest, then lowest latency. | ||
| best_by_provider: dict[str, dict[str, Any]] = {} | ||
| for proposal in sorted( | ||
| proposals, | ||
| key=lambda p: (p["candidate_cost"], p["p95_latency_ms"] or float("inf"), p["to_model_id"]), | ||
| ): | ||
| best_by_provider.setdefault(proposal["provider"], proposal) | ||
| return list(best_by_provider.values()) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Falsy-zero bug in the latency tiebreaker.
p["p95_latency_ms"] or float("inf") maps a legitimate 0 latency to infinity (same as None), because 0 is falsy in Python. For the "cheapest, then lowest-latency" tiebreak, this would wrongly rank a genuinely fastest candidate (0ms) as worst.
🐛 Proposed fix
- key=lambda p: (p["candidate_cost"], p["p95_latency_ms"] or float("inf"), p["to_model_id"]),
+ key=lambda p: (
+ p["candidate_cost"],
+ p["p95_latency_ms"] if p["p95_latency_ms"] is not None else float("inf"),
+ p["to_model_id"],
+ ),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # One winner per provider: cheapest, then lowest latency. | |
| best_by_provider: dict[str, dict[str, Any]] = {} | |
| for proposal in sorted( | |
| proposals, | |
| key=lambda p: (p["candidate_cost"], p["p95_latency_ms"] or float("inf"), p["to_model_id"]), | |
| ): | |
| best_by_provider.setdefault(proposal["provider"], proposal) | |
| return list(best_by_provider.values()) | |
| # One winner per provider: cheapest, then lowest latency. | |
| best_by_provider: dict[str, dict[str, Any]] = {} | |
| for proposal in sorted( | |
| proposals, | |
| key=lambda p: ( | |
| p["candidate_cost"], | |
| p["p95_latency_ms"] if p["p95_latency_ms"] is not None else float("inf"), | |
| p["to_model_id"], | |
| ), | |
| ): | |
| best_by_provider.setdefault(proposal["provider"], proposal) | |
| return list(best_by_provider.values()) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/prepare_model_promotion.py` around lines 162 - 169, Update the sorting
key in the proposal-selection logic to treat only a missing p95_latency_ms value
as infinite, while preserving legitimate zero latency as the fastest value. Keep
the existing candidate_cost, latency, and to_model_id ordering and
best_by_provider selection unchanged.
Source: Path instructions
2 participants
Completes #2819 — move 3 of 3. Builds on move 1 (#2831) + move 2 (#2832), now on main.
What this is
Moves 1–2 made the evaluation evidence flow on its own (registry-derived candidates + a corpus that grows from realized PR outcomes). Move 3 turns a passing benchmark into a prepared selection change that a human approves by merging a PR — per your choice of auto-prepare + one-click approve + auto-rollback.
human_approval_requiredstays true; the machine does everything expensive, the human just merges.Guardrails — a promotion is prepared only when ALL hold
gpt-<major>, anthropicclaude-<line>). Cross-family swaps are never auto-prepared — those still need a human to initiate.apply_promotion()writes the registry mutation and records the prior selection in a newselection_historyarray. The inverse path (find_rollbacks/apply_rollback) reverts to the prior selection when the active model shows a failed workload-benchmark (a quality-gate breach).maint-86 workflow
Dispatch-only: it evaluates a benchmark artifact, prepares the change, and opens a PR that is not auto-merged — merging is the approval. The schedule trigger + the pilot→benchmark bridge are deliberately deferred until the corpus reaches the approval minimum (grown weekly by maint-79), so this cannot fabricate a promotion from thin data. That deferral is the honest state: the logic is built and tested now; it goes live when real approval-grade evidence exists.
Verification (local, CI-pinned)
config/model_registry.json, a synthetic passing benchmark correctly preparesclaude-opus-4-6 → claude-opus-4-8(same-family, passed, cheaper) — and correctly refuses a cross-family (claude-sonnet-5) or more-expensive candidate.With this, #2819's three moves are complete: candidates derive themselves (1), the corpus grows itself (2), and a qualifying same-family upgrade prepares itself for one-click human approval with auto-rollback (3).
Summary by CodeRabbit
New Features
Documentation
Tests