ci(bench-gate): run the tier at the release cut, and report it per module (#1477) - #1494
Conversation
There was a problem hiding this comment.
Sorry @robotrocketscience, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Warning Review limit reached
Next review available in: 47 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe change classifies bench-gate results in pytest terminal summaries and documents release-cut execution, result interpretation, and mandatory inclusion of the complete summary in release PRs. Tests cover executed tests, module skips, whole-tier skips, and unrelated or malformed skip reasons. ChangesBench-gate release reporting
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
PR-size soft capThis PR is over the advisory size threshold:
Bigger PRs collide with more open work, which under the parallel-session workflow tends to produce repeated This is advisory only — nothing is blocked. If the size is intentional (large refactor, module removal, generated code), apply the |
Reviewer's GuideImplements per-module bench-gate reporting in pytest, formalizes when the bench-gate tier is run (at release cut only), documents the new process in RELEASING and CONTRIBUTING, and adds focused tests and changelog entry for the new reporting behavior. Sequence diagram for running bench-gate at release cut and reporting per modulesequenceDiagram
actor ReleaseEngineer
participant PublicRepo
participant BenchGateScript as scripts_run_bench_gate_sh
participant PytestBenchGateTier as pytest_bench_gate_tier
participant LabCorpus as lab_corpus
participant ReleasePR as release_pr
ReleaseEngineer->>PublicRepo: follow RELEASING_step_7
ReleaseEngineer->>BenchGateScript: run scripts/run_bench_gate_sh
BenchGateScript->>LabCorpus: read AELFRICE_CORPUS_ROOT (default lab corpus)
BenchGateScript->>PytestBenchGateTier: invoke pytest bench-gate tier
PytestBenchGateTier->>LabCorpus: load corpus modules
PytestBenchGateTier-->>BenchGateScript: bench_gate_tier summary block
note over PytestBenchGateTier: block separates executed, missing_module, empty_module, tier_skipped
BenchGateScript-->>ReleaseEngineer: display bench_gate_tier block
ReleaseEngineer->>ReleasePR: paste bench_gate_tier block into PR body
PublicRepo->>ReleasePR: enforce presence of bench_gate_tier block before merge
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/conftest.py`:
- Around line 76-124: Update _MODULE_SKIP_RE to include the expected corpus-root
prefix and use fullmatch() when classifying reasons in pytest_terminal_summary.
Replace the broad CORPUS_ENV_VAR substring check with an exact
BENCH_GATE_SKIP_REASON match, so only bench-gate skip messages contribute to
tier or module counts. Add regression coverage for unrelated skip reasons
containing the environment-variable name or an embedded module phrase.
- Around line 126-131: Update the executed-count calculation in the pytest
reporting logic to include the "xpassed" entry from TerminalReporter.stats, so
non-strict XPASS reports with the "bench_gated" keyword are counted. Add a
constructed xpassed report in test_bench_gate_module_report_1477.py to cover
this behavior.
🪄 Autofix
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: CHILL
Plan: Pro Plus
Run ID: aa537944-4224-4771-9f77-e108f30615be
📒 Files selected for processing (5)
CHANGELOG/unreleased/1477-bench-gate-at-release-cut.mdCONTRIBUTING.mddocs/concepts/RELEASING.mdtests/conftest.pytests/test_bench_gate_module_report_1477.py
8c7d142 to
165cb0d
Compare
|
This PR is now behind Auto-rebase was removed because the bot has no signing key; rebasing as the bot strips author signatures and the |
a0a69d8 to
4be187c
Compare
#1456 gave the tier an aggregate skip count, which is the right answer for this repository — with no corpus at all, '36 skipped' is the whole story. It is the wrong answer the moment a corpus exists. The corpus covers a minority of the modules scaffolded under tests/corpus/, so a lab-side run reports a healthy 'N passed' while most of the tier skipped for want of rows: the same misreading #1456 closed, one level in. Three states are now reported separately and by name — tests that executed against the corpus, tests skipped because a named module is missing or empty, and the whole tier skipped for want of a corpus root. Classification is by skip reason rather than by re-deriving which tests carry the marker, so an unrelated skip inside a bench-gated module is not folded in; executed tests are counted off the marker, the only place that signal survives to summary time. Driven against the hook with constructed reports, because the states that need pinning include ones this repository cannot produce — a module present and non-empty, so its gate actually ran — and a test that could only assert the locally reachable states would pin exactly the case that was never in doubt. Verified against a real run as well: with a stub corpus holding one module, the block names every other module as having no verdict. Mutation-checked: dropping the per-module lines reddens three tests, counting executed tests without reading the marker reddens two, collapsing missing and empty into one bucket reddens two. Refs #1477
Operator ruling 2026-08-11 on #1477: option (c). scripts/run_bench_gate.sh becomes a mandatory release step whose output is pasted into the release PR. Two findings decided it. The private repo has no CI of any kind — no .github/workflows directory at all — so 'runs lab-side' was never true of anything, and adding a job there is standing new infrastructure rather than a job. And the corpus covers a minority of the scaffolded modules, so a weekly cadence would institutionalise a mostly-skipping run and report it as coverage. The release cut is the only moment these verdicts are consumed, and a checklist step that blocks the cut is the one schedule that cannot die unnoticed — which is precisely the #1423 failure mode a cron on one laptop would reproduce. CONTRIBUTING's bench-gate section said the tier runs lab-side. It now says where it actually runs, and tells the reader to read the summary block rather than the pass count: a run reporting 'N passed' while most modules skipped is the normal case today. Refs #1477, #1456
4be187c to
c6f0c0c
Compare
|
merge-train: merged c6f0c0c → |
Closes #1477. Refs #1456 (AC3).
What was actually wrong
#1456 records that the bench-gate quality tier "runs lab-side only". It does not
run anywhere.
.github/workflowsdirectory at all. So"add a lab-side job" had no host to be added to — it is standing new
infrastructure, not a job.
scripts/run_bench_gate.shexists onmain, defaultsAELFRICE_CORPUS_ROOTto the lab corpus, and is invoked by nothing.
tests/corpus/,so even a job built today would leave most of the tier skipping for want of
rows.
Several defaults are held OFF pending exactly these verdicts. A gate that never
runs cannot deliver one, so those defaults are parked on a measurement that was
not scheduled to happen.
Where it runs now
Operator ruling 2026-08-11: at the release cut, and nowhere else.
docs/concepts/RELEASING.mdstep 7 makesscripts/run_bench_gate.shmandatoryand its output part of the release PR body.
That is the only moment the verdicts are consumed, and a checklist step that
blocks the cut is the one schedule that cannot die unnoticed — which is exactly
the #1423 failure mode a weekly cron on one machine would have reproduced. The
corpus is in any case too thin for a weekly cadence to produce a new verdict.
CONTRIBUTING.md's bench-gate section said the tier runs lab-side. It now sayswhere it actually runs.
Why the aggregate count was not enough (#1477 AC3)
#1456's aggregate is the right answer while the corpus is absent: with nothing
to run against, "36 skipped" is the whole story. It is the wrong answer the
moment a corpus exists — a run reports a healthy "N passed" while most of the
tier skipped for want of rows, which is the same misreading #1456 closed, one
level in.
The
bench-gate tierblock now separates three states and names them:per module with which of the two it was (they are different failures: no rows
delivered, versus a module that exists and is dead);
Classification is by skip reason, not by re-deriving which tests carry the
marker, so an unrelated skip inside a bench-gated module is not folded into the
missing-rows figure — inflating it is the direction that makes the tier look
more blocked than it is, so nobody would chase it. Executed tests are counted
off the marker, which is the only place that signal survives to summary time.
How it is tested
Driven against the hook with constructed reports rather than by running the tier
under a fixture corpus. The states that need pinning include ones this
repository cannot produce — a module present and non-empty, so its gate actually
ran — and a test that could only assert the locally reachable states would pin
exactly the case that was never in doubt.
Verified against a real run as well: with a stub corpus holding one module, the
block names every other module as having no verdict.
Out of scope
AC1 and AC2 as literally worded — a lab-side job on a stated cadence, and where
a public reader is pointed — are discharged by the ruling rather than by code:
the run is at the cut, and its output lands in the release PR, which is public.
Funding corpus rows for the modules that have none is a separate decision and is
not taken here.
Summary by Sourcery
Run the bench-gate quality tier at release time and expand its reporting so release PRs clearly show which modules did and did not produce corpus-backed verdicts.
Enhancements:
CI:
scripts/run_bench_gate.sha mandatory step in the release process and require pasting its bench-gate summary block into the release PR body.Documentation:
Tests:
Summary by CodeRabbit
New Features
Documentation
Tests
Changelog