chore(model-registry): advance provisional verifier selections to current catalog - #2852
Conversation
|
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. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 27 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe PR refreshes registry-backed verifier selections and model evaluation roles for three providers. Client tests and README verification guidance now resolve reviewed selections. Fleet attempt metadata records the new timestamp and PR number. ChangesModel registry refresh
Estimated code review effort: 2 (Simple) | ~10 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 |
Workflow source neededPR #2852 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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db4e091753
ℹ️ 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".
| "model_id": "gpt-5.6-terra", | ||
| "status": "provisional", | ||
| "decided_at": "2026-07-31", | ||
| "review_by": "2026-08-30", | ||
| "evidence_ids": ["catalog-review-2026-07-10"], | ||
| "rationale": "Provisional selection advanced to the current catalog generation; positioning 'balanced' matches the verifier-balanced profile. This is NOT a benchmark-proven promotion: no passing workload-benchmark evidence exists for this model or for the superseded incumbent, so status stays provisional until the paired pilot runs. Human-approved by merging this PR." |
There was a problem hiding this comment.
Keep catalog-only candidates out of runtime selections
These replacements become the live verifier models immediately because select_model_for_profile() accepts provisional decisions, even though their only evidence is explicitly catalog-only. This contradicts docs/MODEL_SELECTION_POLICY.md:88-92 and 119-122, which require the provisional incumbents to remain the runtime baseline and prohibit changing a selection until paired workload evidence passes; human approval is an additional gate, not a substitute for that evidence. A regression in false-PASS behavior would therefore affect verifier decisions across every synced consumer before the pilot detects it.
Useful? React with 👍 / 👎.
| "as_of": "2026-07-10", | ||
| "review_by": "2026-07-24", | ||
| "review_by": "2026-08-30", |
There was a problem hiding this comment.
Preserve the catalog-facts freshness deadline
The catalog facts and baselines remain dated 2026-07-10, but advancing the top-level review_by to 2026-08-30 makes _review_date() prefer that deadline over as_of + 30 days. Consequently, maint-77 reports the registry as fresh through August 30 even though the catalog has not been queried for up to 51 days; for example, strict evaluation on 2026-08-15 returns no finding. Only the per-selection deadlines should move unless the catalog facts are actually refreshed.
Useful? React with 👍 / 👎.
| "selections": [ | ||
| { | ||
| "profile": "verifier-balanced", | ||
| "provider": "openai", | ||
| "model_id": "gpt-5.6-terra", |
There was a problem hiding this comment.
Update the consumer template with the new selections
This registry is consumer-managed through .github/sync-manifest.yml, but templates/consumer-repo/config/model_registry.json still selects gpt-5.4, claude-opus-4-6, and codex-mini-latest. A consumer initialized from the repository template therefore receives different verifier defaults until a later fleet sync overwrites the file; align the template in this change as required for consumer-affecting updates.
AGENTS.md reference: AGENTS.md:L63-L65
Useful? React with 👍 / 👎.
| ### Verification Pipeline | ||
|
|
||
| After PR merge, applying a `verify:*` label (typically `verify:evaluate` via auto-pilot, or `verify:compare` for dual-model mode) triggers the verifier. In `compare` mode, two LLM providers (gpt-5.4 + claude-opus-4-6) independently evaluate the diff against acceptance criteria with unanimous PASS required. On CONCERNS or FAIL, maintainers or automation can apply the `verify:create-new-pr` label to trigger a 4-round LLM pipeline that generates a follow-up issue (analyze -> tasks -> acceptance criteria -> format). | ||
| After PR merge, applying a `verify:*` label (typically `verify:evaluate` via auto-pilot, or `verify:compare` for dual-model mode) triggers the verifier. In `compare` mode, two LLM providers (the reviewed OpenAI + Anthropic `verifier-balanced` selections in `config/model_registry.json`) independently evaluate the diff against acceptance criteria with unanimous PASS required. On CONCERNS or FAIL, maintainers or automation can apply the `verify:create-new-pr` label to trigger a 4-round LLM pipeline that generates a follow-up issue (analyze -> tasks -> acceptance criteria -> format). |
There was a problem hiding this comment.
Update all verifier-default contract docs
Changing the README to describe registry-backed selections leaves the workflow contract inconsistent: docs/ci/WORKFLOWS.md:167 and docs/MODEL_MANAGEMENT.md:44-47 still identify gpt-5.4 as the OpenAI verifier default. After this commit that model is no longer selected, so operators following those docs will diagnose or configure the wrong runtime; convert those references to the registry-backed wording as part of the same contract update.
AGENTS.md reference: AGENTS.md:L63-L63
Useful? React with 👍 / 👎.
Automated Status SummaryHead SHA: 7e17a84
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
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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_langchain_client.py`:
- Around line 14-23: Update the _reviewed_model helper to explicitly request the
verifier-balanced profile when calling select_model_for_profile, using the
existing named constant if one is available. Keep the provider argument and
assertion behavior unchanged so tests remain tied to the intended reviewed
profile.
🪄 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: 2b20e667-9632-44ae-bf0c-f05eafa437ff
📒 Files selected for processing (5)
README.mdconfig/model_eval_candidates.jsonconfig/model_registry.jsonlangsmith-fleet-worker-attempt.jsontests/tools/test_langchain_client.py
| def _reviewed_model(provider: str) -> str: | ||
| """Return the registry's reviewed selection for ``provider``. | ||
|
|
||
| Tests assert the resolver serves the *reviewed selection*, not a hardcoded | ||
| model id. Pinning literals here made every selection change — including an | ||
| auto-prepared maint-86 promotion PR — fail CI for no real defect. | ||
| """ | ||
| model = llm_registry.select_model_for_profile(provider=provider) | ||
| assert model, f"no reviewed selection for {provider}; registry/policy is misconfigured" | ||
| return model |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Pin _reviewed_model to verifier-balanced.
The helper relies on select_model_for_profile’s default profile. If that default changes, these tests can validate a different profile while still passing. Pass profile="verifier-balanced" or use a named constant for this profile.
Proposed fix
- model = llm_registry.select_model_for_profile(provider=provider)
+ model = llm_registry.select_model_for_profile(
+ provider=provider,
+ profile="verifier-balanced",
+ )As per path instructions, this Python test change must preserve correctness and test coverage.
📝 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.
| def _reviewed_model(provider: str) -> str: | |
| """Return the registry's reviewed selection for ``provider``. | |
| Tests assert the resolver serves the *reviewed selection*, not a hardcoded | |
| model id. Pinning literals here made every selection change — including an | |
| auto-prepared maint-86 promotion PR — fail CI for no real defect. | |
| """ | |
| model = llm_registry.select_model_for_profile(provider=provider) | |
| assert model, f"no reviewed selection for {provider}; registry/policy is misconfigured" | |
| return model | |
| def _reviewed_model(provider: str) -> str: | |
| """Return the registry's reviewed selection for ``provider``. | |
| Tests assert the resolver serves the *reviewed selection*, not a hardcoded | |
| model id. Pinning literals here made every selection change — including an | |
| auto-prepared maint-86 promotion PR — fail CI for no real defect. | |
| """ | |
| model = llm_registry.select_model_for_profile( | |
| provider=provider, | |
| profile="verifier-balanced", | |
| ) | |
| assert model, f"no reviewed selection for {provider}; registry/policy is misconfigured" | |
| return model |
🤖 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_langchain_client.py` around lines 14 - 23, Update the
_reviewed_model helper to explicitly request the verifier-balanced profile when
calling select_model_for_profile, using the existing named constant if one is
available. Keep the provider argument and assertion behavior unchanged so tests
remain tied to the intended reviewed profile.
Source: Path instructions
…rent catalog Move the three verifier-balanced provisional selections from the 2026-07-10 incumbents to the current catalog generation: openai gpt-5.4 -> gpt-5.6-terra (positioning "balanced") anthropic claude-opus-4-6 -> claude-sonnet-5 (positioning "balanced") github-models codex-mini-latest -> openai/gpt-5 This is NOT a benchmark-proven promotion. No passing workload-benchmark evidence exists for either the new models or the superseded incumbents, so status stays "provisional" and the paired pilot remains the path to "approved". The prior selections are recorded in selection_history following the same shape tools/prepare_model_promotion.py writes, so the rollback path still works. Registry and per-selection review_by move to 2026-08-30 (today + the policy's maximum_days_between_reviews: 30). as_of stays 2026-07-10 because no provider catalog was re-queried; the reused catalog-review-2026-07-10 evidence already attests availability of all three chosen models. Also: - Regenerate config/model_eval_candidates.json via tools/refresh_model_eval_candidates.py --write (its --check drift gate fails otherwise). The superseded incumbents correctly become paired candidates. - tests/tools/test_langchain_client.py: derive the three expected default models from the reviewed selection instead of hardcoding ids. Pinned literals meant every selection change — including an auto-prepared maint-86 promotion PR — would fail CI with no real defect. - README: describe compare mode by reviewed selection rather than naming models. Verified: freshness gate reports "Model registry is fresh" (0 findings, exit 0 with --strict too); resolve_slots() serves the new trio; 164 model/registry tests and 88 followup/template-sync tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tests/docs/test_workflow_source_docs.py::test_readme_verify_compare_models_match_langchain_defaults derives the expected pair from langchain_client._default_slots() and REQUIRES the README to contain "<openai> + <anthropic>". Describing the pair abstractly broke that enforced sync, so keep the literals and advance them instead — the test is what prevents them drifting. Also update the same stale claim in docs/ci/WORKFLOWS.md, which still named the defunct claude-sonnet-4-6. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
54ee27c to
bf9490f
Compare
langsmith-fleet-worker-attempt.json is written per-run by reusable-codex-run.yml:1173 and uploaded as a build artifact; nothing reads it from the repo. While tracked, every run that rewrote it produced a diff that codex-autofix then committed onto whatever PR happened to be open — two such commits landed on PR #2852 while it was in review. Remove it and add it to the existing .gitignore block for per-run CI snapshots. It was also not on config/root-allowlist.txt, which health-40's "Root allowlist guard" enforces. While auditing that, four other tool-mandated root config files turned out to be unlisted as well, so add them: .coderabbit.yaml, .release-please-manifest.json, release-please-config.json, renovate.json. The guard has not been reporting any of this because it is skipped — the earlier "Snapshot branch protection state" step fails first (see PR body). Deliberately NOT changed: testgen_gate.py (a one-off #2620 acceptance gate, referenced by nothing) is left for an explicit keep-or-delete decision rather than silently blessed by an allowlist entry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
langsmith-fleet-worker-attempt.json is written per-run by reusable-codex-run.yml:1173 and uploaded as a build artifact; nothing reads it from the repo. While tracked, every run that rewrote it produced a diff that codex-autofix then committed onto whatever PR happened to be open — two such commits landed on PR #2852 while it was in review. Remove it and add it to the existing .gitignore block for per-run CI snapshots. It was also not on config/root-allowlist.txt, which health-40's "Root allowlist guard" enforces. While auditing that, four other tool-mandated root config files turned out to be unlisted as well, so add them: .coderabbit.yaml, .release-please-manifest.json, release-please-config.json, renovate.json. The guard has not been reporting any of this because it is skipped — the earlier "Snapshot branch protection state" step fails first (see PR body). Deliberately NOT changed: testgen_gate.py (a one-off #2620 acceptance gate, referenced by nothing) is left for an explicit keep-or-delete decision rather than silently blessed by an allowlist entry. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…mplate (#2874) #2852 advanced the verifier-balanced selections in the repo-root config/model_registry.json, but that file is not what consumers receive. The sync manifest places config/model_registry.json in the `llm_config` section, which sync_manifest_compiler.py treats as TEMPLATE-sourced, so the compiled entry resolves to: source: config/model_registry.json resolved_source: templates/consumer-repo/config/model_registry.json sync_mode: null (overwrite) The template copy still carried the pre-#2852 selections (gpt-5.4 / claude-opus-4-6 / codex-mini-latest) and the lapsed review_by 2026-07-24. Because the mode is overwrite, maint-68 was actively re-asserting the OLD selections onto consumers: Manager-Database and Inv-Man-Intake still resolve the old trio today, after three successful maint-68 runs on 2026-08-01. Copy the root registry to the template. Verified the two files were otherwise byte-identical: the entire diff was exactly #2852's change (review_by, the three selections, and the new selection_history), so nothing consumer-specific is clobbered. Also refresh the stale "Next decision review" header in docs/MODEL_SELECTION_POLICY.md, which still said 2026-07-24. Consumers' config/llm_slots.json remains create_only and already profile-driven in the template, so slot resolution follows the reviewed selection with no further change (Manager-Database#1501, Inv-Man-Intake#870). Refs #2852. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
What
Advances the three
verifier-balancedprovisional selections from the 2026-07-10 incumbents to the current catalog generation:gpt-5.4gpt-5.6-terraclaude-opus-4-6claude-sonnet-5codex-mini-latestopenai/gpt-5balancedpositioning matches theverifier-balancedprofile. Both replaced OpenAI/Anthropic incumbents recorded pricing only as "observed billed cost", so no cost delta is claimed here.What this is NOT
Not a benchmark-proven promotion. No passing
workload-benchmarkevidence exists for the new models or for the superseded incumbents — the only evidence in the registry iscatalog-review-2026-07-10(catalog-only, which the registry itself says "cannot approve a selection"). So:statusstaysprovisionalon all three.approved.Merging this PR is the
human_approval_requiredstep for a human-initiated selection change. The anthropic move is cross-family (claude-opus→claude-sonnet), whichtools/prepare_model_promotion.pynever auto-prepares by design — hence human-initiated.Why now
config/model_registry.jsonselections had gone 7 days pastreview_by(advisory-only, non-blocking) while the catalog baseline already listed newer current models. The incumbents were pinned to a generation the registry itself no longer lists as the current catalog.Changes
config/model_registry.json— the three selections,decided_at2026-07-31,review_by2026-08-30 (today + the policy's ownmaximum_days_between_reviews: 30). Prior selections recorded in a newselection_historyusing the exact shapetools/prepare_model_promotion.py:181-190writes, so the rollback path keeps working.as_ofstays 2026-07-10 — no provider catalog was re-queried.config/model_eval_candidates.json— regenerated withtools/refresh_model_eval_candidates.py --write; its--checkdrift gate fails otherwise. The superseded incumbents correctly become paired candidates.tests/tools/test_langchain_client.py— three assertions now derive the expected model from the reviewed selection instead of hardcodinggpt-5.4/claude-opus-4-6/codex-mini-latest. Pinned literals meant any selection change — including an auto-preparedmaint-86promotion PR — would fail CI with no real defect.README.md— describe compare mode by reviewed selection instead of naming two model ids that drift.Verification
claude-sonnet-5correctly resolves as temperature-rejecting via the #2822 logic (verified), so no client change is needed.Follow-ups (not in this PR)
config/model_registry.jsonsyncs and overwrites, but both consumers ship pinnedconfig/llm_slots.jsonfiles, and the resolver deliberately retains an unprofiled pin while the pinned model is stilllifecycle: current— sogpt-5.4andcodex-mini-latestwould persist there (only the invalidclaude-sonnet-4-6pin falls through to the selection). That file issync_mode: create_only, so each repo must migrate to the profile form: feat(llm): support profile slots so the chat path tracks reviewed selections Manager-Database#1501 (profile support + migration) and chore(config): make llm_slots profile-driven so slots track reviewed selections Inv-Man-Intake#870 (migration). Both are order-independent w.r.t. this PR.codex-mini-latest → openai/gpt-5can never be auto-prepared:model_family()uses the exact model id for providers other than openai/anthropic, so github-models swaps always need a human until someone teaches it a rule.langsmith-fleet-worker-attempt.jsonat the repo root looks like a stray committed CI artifact from run 2832 (written at runtime byreusable-codex-run.yml:1173). Untouched here.🤖 Generated with Claude Code
Summary by CodeRabbit