feat(model-policy): exempt un-harvestable categories from the 10-case floor - #2873
Conversation
… floor approval_stage.minimum_cases_per_category applied a flat floor of 10 to all five required failure categories. Three of them — stale-verifier-claim, review-thread-debt, missing-acceptance-criterion — cannot be labelled from realized downstream outcomes, so tools/harvest_verifier_corpus.py never produces them (see its module docstring) and they only grow by hand. Holding them to the machine-harvestable floor made "approved" unreachable without ~22 owner-labelled cases, so no candidate could ever pass evaluate_model_benchmark.py and prepare_model_promotion.py could never prepare a promotion. Add approval_stage.minimum_cases_per_category_overrides and honour it in evaluate_model_benchmark.py. The three owner-sourced categories are set to 1: for them the floor asserts REPRESENTATION (at least one adjudicated example of the failure mode) rather than statistical power. Everything else is unchanged — the 75-case total, the machine-harvestable categories' floor of 10, and every Wilson bound / paired non-inferiority gate still run over the whole corpus. Guardrails: a floor below 1 is rejected (so a category can never be dropped entirely by setting 0), an override naming a non-required category is rejected, and a non-object overrides value is rejected. Effect on the real corpus (51 cases: clean-pass 40, stale-verifier-claim 5, follow-up-required 3, review-thread-debt 2, missing-acceptance-criterion 1): all three owner-sourced categories now pass. The two remaining gaps are both machine-harvestable — follow-up-required +7 (cap headroom 17) and the 75-case total +24 (headroom 37 across follow-up-required and regression-after-merge) — so approval becomes reachable by maint-79 alone as the fleet produces resolved follow-ups and reverts. It is not immediate, and no gate was weakened for the categories the harvester can actually grow. Refs #2819. Co-Authored-By: Claude Opus 5 <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. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 1 minute 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)
Comment |
Workflow source neededPR #2873 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. |
![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: ea86ffd080
ℹ️ 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".
| "approval_stage": { | ||
| "minimum_adjudicated_cases": 75, | ||
| "minimum_cases_per_category": 10, | ||
| "minimum_cases_per_category_overrides": { |
There was a problem hiding this comment.
The sync manifest distributes config/model_selection_policy.json under llm_config, for which scripts/sync_manifest_compiler.py::_source_candidates prefers templates/consumer-repo/ over the root file. Because templates/consumer-repo/config/model_selection_policy.json remains unchanged, maint-68 will keep sending consumers the flat 10-case floor, so this exemption—and its stated promotion-reachability benefit—only applies to Workflows itself. Update the consumer template with the override as well.
AGENTS.md reference: AGENTS.md:L33-L33
Useful? React with 👍 / 👎.
| # from realized outcomes (see tools/harvest_verifier_corpus.py) can never be | ||
| # grown by the harvester, so holding them to the machine-harvestable floor | ||
| # made `approved` unreachable without hand-labelling. See #2819. | ||
| raw_overrides = approval.get("minimum_cases_per_category_overrides") or {} |
There was a problem hiding this comment.
When a policy explicitly supplies an empty list, empty string, 0, false, or null, approval.get(...) or {} converts it to {} before the type check, so these malformed non-object values are silently accepted and the evaluator applies the flat floor instead. This can turn a configuration error into an unexplained failed benchmark with no recommendation; distinguish a missing field from a present value and validate the latter before defaulting.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR updates the verifier model selection policy and benchmark evaluator to allow per-category minimum-case floors to be overridden for owner-sourced categories that cannot be machine-harvested, making “approved” reachable without large amounts of hand-labeling.
Changes:
- Add
approval_stage.minimum_cases_per_category_overridesto exempt un-harvestable categories from the 10-case floor (representation-only floor of 1). - Update
tools/evaluate_model_benchmark.pyto apply per-category floors and validate override configuration. - Add tests covering the new override behavior and document the exemption in
docs/MODEL_SELECTION_POLICY.md.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tools/evaluate_model_benchmark.py | Implements per-category floor overrides and validation when computing the per-category gate. |
| tests/tools/test_evaluate_model_benchmark.py | Adds regression + behavior tests for the override-driven per-category gate logic. |
| docs/MODEL_SELECTION_POLICY.md | Documents the override-based exemption for owner-sourced categories. |
| config/model_selection_policy.json | Defines the new overrides (and an explanatory note) in the default policy profile. |
| raw_overrides = approval.get("minimum_cases_per_category_overrides") or {} | ||
| if not isinstance(raw_overrides, dict): | ||
| raise ValueError("approval_stage.minimum_cases_per_category_overrides must be an object") |
| - Run every candidate and baseline on the same cases and prompt version. | ||
| - Use at least 30 cases to retain a candidate and at least 75 cases, with 10 per | ||
| required failure category, to approve it. | ||
| required failure category, to approve it. Categories that cannot be labelled |
| "minimum_cases_per_category_overrides": { | ||
| "stale-verifier-claim": 1, | ||
| "review-thread-debt": 1, | ||
| "missing-acceptance-criterion": 1 | ||
| }, |
| def test_override_must_be_an_object(): | ||
| policy = _policy() | ||
| policy["profiles"]["verifier-balanced"]["approval_stage"][ | ||
| "minimum_cases_per_category_overrides" | ||
| ] = ["stale-verifier-claim"] | ||
|
|
||
| with pytest.raises(ValueError, match="must be an object"): | ||
| evaluator.evaluate_benchmark(_thin_payload(), policy) |
…template config/model_selection_policy.json is in the manifest's `llm_config` section, which sync_manifest_compiler.py resolves from templates/consumer-repo/, so the repo-root policy governs Workflows only. Without this the override would never reach consumers (same root-vs-template split that left the registry stale — see #2874). Surgical edit rather than a wholesale copy: the template deliberately omits Workflows-only sections (notably `corpus_growth`, a maint-79 harvester concern), so only the approval_stage override block and its note are added. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Automated Status SummaryHead SHA: 3c5dc53
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
|
GitHub Actions
Inactive
Implements the agreed resolution for #2819 — lower the per-category floor for the owner-sourced categories.
Why
approval_stage.minimum_cases_per_categoryapplied a flat floor of 10 to all five required failure categories (evaluate_model_benchmark.py:163-166). Three of them cannot be labelled from realized downstream outcomes, sotools/harvest_verifier_corpus.pynever produces them — its own docstring says they "cannot be labeled from realized outcomes and remain owner-sourced; this tool never [labels them]":stale-verifier-claimreview-thread-debtmissing-acceptance-criterionHolding them to the machine-harvestable floor made
approvedstructurally unreachable without roughly 22 hand-labelled cases (~1.1-1.8 h). Sinceprepare_model_promotion.py:135refuses anything whosestatus != "passed", that single unmet gate blocked every promotion the automation could ever prepare.What changed
config/model_selection_policy.json— newapproval_stage.minimum_cases_per_category_overrides, setting those three to 1, with an inline note recording the reasoning.tools/evaluate_model_benchmark.py— honour the overrides when buildinggate_results["minimum_cases_per_category"].docs/MODEL_SELECTION_POLICY.md— document the exemption and that the floors should be raised toward 10 as owner-labelled cases accumulate.For these three the floor now asserts representation (at least one adjudicated example of the failure mode) rather than statistical power. Deliberately unchanged: the 75-case total, the floor of 10 for the categories the harvester can grow (
clean-pass,follow-up-required), and every Wilson bound / paired non-inferiority gate — all still evaluated over the whole corpus.I used 1 rather than the 3 floated on the issue: at 3,
review-thread-debt(2) andmissing-acceptance-criterion(1) would still be short, leaving a residual blocking hand-labelling ask — exactly the kind of small-but-blocking task that stalls indefinitely. Raising these later is a one-line change.Guardrails
0.overridesvalue is rejected.Effect on the real corpus
51 cases —
clean-pass40,stale-verifier-claim5,follow-up-required3,review-thread-debt2,missing-acceptance-criterion1:Both remaining gaps are machine-harvestable —
follow-up-required+7 (cap headroom 17) and the 75-case total +24 (headroom 37 acrossfollow-up-requiredandregression-after-merge). Soapprovedbecomes reachable bymaint-79alone as the fleet produces resolved follow-ups and reverts. It is not immediate, and nothing was weakened for the categories the harvester can actually grow.Verification
Deliberate break → revert: reverting the gate to use the flat
minimum_per_categorymakestest_per_category_overrides_admit_thin_owner_sourced_categoriesFAIL; restoring it returns 20/20. A companion test (test_owner_sourced_categories_block_approval_without_overrides) pins the pre-change behaviour so the exemption cannot silently become the default.🤖 Generated with Claude Code