maint-78: derive pilot candidates from the registry + auto-trigger (#2819 move 1) - #2831
Conversation
Move 1 of the self-feeding verifier-model promotion system (#2819). The pilot candidate list was hand-maintained and had already drifted: it listed models that were no longer current while omitting now-current ones (e.g. claude-opus-4-8). A stale candidate list means a superseded model stays the incumbent forever because nothing ever pilots its replacement. - tools/refresh_model_eval_candidates.py: derive_candidates() builds the candidate set straight from config/model_registry.json — incumbent = the profile's reviewed selection, candidates = every other current, non-blocked, same-provider model whose positioning is a plausible verifier tier (excludes efficient / coding-worker-profile). --write regenerates the config; --check is a drift gate. - tests: derivation logic + a gate test asserting the committed config/model_eval_candidates.json equals the derivation, so a catalog change that forgets to refresh candidates reddens CI. - config/model_eval_candidates.json: regenerated from the registry (adds claude-fable-5, claude-opus-4-8, and a github-models candidate). - maint-78: auto-run on catalog change (push to main touching the registry) + weekly cron, and refresh candidates from the registry before each run so the pilot always tests every now-current model. Promotion is unchanged — the pilot only narrows candidates; approval still requires the 75-case corpus + policy gate. 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. |
Workflow source neededPR #2831 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. |
📝 WalkthroughWalkthroughThe PR adds registry-driven candidate derivation, CLI drift checks, tests for filtering and ordering, updated candidate configuration, and workflow triggers that refresh candidates before running the evaluation pilot. ChangesModel evaluation pilot
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbb2113703
ℹ️ 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".
| workflow_dispatch: {} | ||
| # Auto-run when the catalog changes so a newly-current model is piloted without | ||
| # anyone editing the candidate list (stranske/Workflows#2819, move 1). | ||
| push: |
There was a problem hiding this comment.
Document the automatic pilot triggers
The workflow now runs automatically on matching pushes and a weekly schedule, but the inspected inventory still describes it as workflow_dispatch / “Manual evaluation” in docs/ci/WORKFLOW_SYSTEM.md:748, while docs/ci/WORKFLOWS.md:209 also omits the new cadence and registry-derived behavior. This leaves operators relying on the documented topology unaware that the credential-backed, 30-case evaluation will execute automatically; update both contract documents with these trigger changes.
AGENTS.md reference: AGENTS.md:L62-L65
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@tests/tools/test_refresh_model_eval_candidates.py`:
- Around line 21-65: Extend the model fixture used by the refresh-candidate test
with a current same-provider entry whose positioning is coding-worker-profile,
then update the relevant assertions to verify that model is excluded from the
candidates. Keep the existing efficient, non-current, and blocked exclusion
coverage unchanged.
In `@tools/refresh_model_eval_candidates.py`:
- Around line 99-102: Update the args.write branch in the CLI flow to catch
OSError from args.candidates.write_text, print a useful diagnostic to stderr,
and return a non-zero exit code; preserve the existing success message and
return 0 when writing succeeds.
- Around line 66-67: Update _load and the derive_candidates entry path to
validate that registry JSON is an object with correctly typed selections and
models fields, rejecting arrays and malformed values before candidate
derivation. Route JSON parsing and shape-validation failures through the
existing error-message path and return exit code 2 instead of allowing uncaught
exceptions.
🪄 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: 045b5614-69aa-4708-ba20-bc487929656a
📒 Files selected for processing (4)
.github/workflows/maint-78-model-evaluation-pilot.ymlconfig/model_eval_candidates.jsontests/tools/test_refresh_model_eval_candidates.pytools/refresh_model_eval_candidates.py
| "models": [ | ||
| { | ||
| "provider": "openai", | ||
| "model_id": "gpt-5.4", | ||
| "lifecycle": "current", | ||
| "positioning": "incumbent-verifier", | ||
| }, | ||
| { | ||
| "provider": "openai", | ||
| "model_id": "gpt-5.6-terra", | ||
| "lifecycle": "current", | ||
| "positioning": "balanced", | ||
| }, | ||
| { | ||
| "provider": "openai", | ||
| "model_id": "gpt-5.6-luna", | ||
| "lifecycle": "current", | ||
| "positioning": "efficient", | ||
| }, # excluded | ||
| { | ||
| "provider": "openai", | ||
| "model_id": "gpt-5.5", | ||
| "lifecycle": "compatibility", | ||
| "positioning": "frontier", | ||
| }, # excluded (not current) | ||
| { | ||
| "provider": "openai", | ||
| "model_id": "gpt-blocked", | ||
| "lifecycle": "current", | ||
| "positioning": "frontier", | ||
| "blocked": True, | ||
| }, # excluded (blocked) | ||
| { | ||
| "provider": "anthropic", | ||
| "model_id": "claude-opus-4-6", | ||
| "lifecycle": "current", | ||
| "positioning": "incumbent-verifier", | ||
| }, | ||
| { | ||
| "provider": "anthropic", | ||
| "model_id": "claude-opus-4-8", | ||
| "lifecycle": "current", | ||
| "positioning": "high-capability", | ||
| }, | ||
| ], |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Cover the coding-worker-profile exclusion.
The fixture and assertions only exercise efficient; a regression that admits coding-worker-profile models passes all tests. Add a current same-provider model with that positioning and assert it is absent. As per path instructions, “Flag new or changed behavior with no accompanying test.”
Also applies to: 78-82
🤖 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_refresh_model_eval_candidates.py` around lines 21 - 65,
Extend the model fixture used by the refresh-candidate test with a current
same-provider entry whose positioning is coding-worker-profile, then update the
relevant assertions to verify that model is excluded from the candidates. Keep
the existing efficient, non-current, and blocked exclusion coverage unchanged.
Source: Path instructions
| def _load(path: Path) -> dict[str, Any]: | ||
| return json.loads(path.read_text(encoding="utf-8")) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Reject structurally invalid registry JSON cleanly.
A valid JSON array or malformed selections/models value reaches derive_candidates() and raises an uncaught exception. Validate the expected registry shape and return exit code 2 with the existing error-message path. As per path instructions, “Prioritize correctness, error handling, and test coverage.”
Also applies to: 88-94
🤖 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/refresh_model_eval_candidates.py` around lines 66 - 67, Update _load
and the derive_candidates entry path to validate that registry JSON is an object
with correctly typed selections and models fields, rejecting arrays and
malformed values before candidate derivation. Route JSON parsing and
shape-validation failures through the existing error-message path and return
exit code 2 instead of allowing uncaught exceptions.
Source: Path instructions
| if args.write: | ||
| args.candidates.write_text(_serialize(derived), encoding="utf-8") | ||
| print(f"wrote {len(derived['candidates'])} candidate rows to {args.candidates}") | ||
| return 0 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Handle candidate-file write failures.
An unwritable target or nonexistent parent raises OSError and produces a traceback, rather than a controlled CLI failure. Catch it, print a stderr diagnostic, and return a non-zero exit code. As per path instructions, “Prioritize correctness, error handling, and test coverage.”
🤖 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/refresh_model_eval_candidates.py` around lines 99 - 102, Update the
args.write branch in the CLI flow to catch OSError from
args.candidates.write_text, print a useful diagnostic to stderr, and return a
non-zero exit code; preserve the existing success message and return 0 when
writing succeeds.
Source: Path instructions
Automated Status SummaryHead SHA: dbc3550
Coverage Overview
Failure triageDetected failure types: pytest.
Updated automatically; will refresh on subsequent CI/Docker completions. Keepalive checklistScopeNo scope information available Tasks
Acceptance criteria
|
Part of #2819 (self-feeding verifier-model promotion) — move 1 of 3.
Problem
The verifier-pilot candidate list (
config/model_eval_candidates.json) was hand-maintained and had already drifted: it listed models that were no longercurrentwhile omitting now-current ones (notablyclaude-opus-4-8). A stale candidate list is why a superseded model stays incumbent forever — nothing ever pilots its replacement.Change
tools/refresh_model_eval_candidates.py—derive_candidates()builds the candidate set straight fromconfig/model_registry.json: incumbent = the profile's reviewedselectionsentry; candidates = every othercurrent, non-blocked, same-provider catalogued model whosepositioningis a plausible verifier tier (excludesefficient/coding-worker-profile).--writeregenerates the config;--checkis a drift gate.config/model_eval_candidates.json— regenerated from the registry (addsclaude-fable-5,claude-opus-4-8, and a github-models candidate that the hand list had dropped).maint-78— auto-runs on catalog change (push tomaintouching the registry) + weekly cron, and refreshes candidates from the registry before each run so the pilot always tests every now-current model.Not in scope (later moves)
pr_verifierdecisions labeled by realized PR outcomes.Promotion semantics are unchanged here: the pilot only narrows candidates; approval still requires the 75-case corpus +
model_selection_policy.jsongate (new_catalog_models_auto_promote=false).Verification
black==26.5.1 --check --line-length 100clean on both new filesruff==0.15.20cleanmypy==2.1.0clean on the new modulepytest tests/tools/test_refresh_model_eval_candidates.py— 7 passedSummary by CodeRabbit
New Features
Tests