feat: render a skipped check as skipped, not as a green tick - #260
Merged
Conversation
|
Warning Review limit reached
Next review available in: 43 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe change adds skipped-scope status handling, separate step-log and Markdown verdicts, skipped-scope markers, skip counts, tests, and documentation. Failures retain precedence over skipped results. ChangesSkipped scope reporting
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ScopeResult
participant render_report
participant StepLog
ScopeResult->>render_report: provide scope statuses
render_report->>render_report: select the run verdict
render_report->>StepLog: emit skipped markers and counts
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Contributor
Commit Check✅ All 6 checks passed Show all 6 checkscommit-check 2.13.1 · Rules reference |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@main.py`:
- Around line 723-725: Update the results filtering used by _markdown_table so
the failure table includes only scopes whose status is "fail", excluding both
"pass" and skipped scopes while preserving the existing failure summary and
table rendering flow.
- Around line 726-730: Update the all-skipped branch in the report verdict logic
to require at least one result/scope before matching skipped == total. Preserve
the existing “All checks skipped” message for non-empty result sets, while
allowing the empty-result handling to follow the same total guard used by the
step log.
- Around line 486-490: Update the commit-check reporting branch in the results
summary so partial skips do not print “all checks passed”; report the passed
count and skipped count separately when skipped is nonzero but less than total,
while preserving the all-skipped message for skipped == total.
- Around line 101-118: The completion paths must treat “skip” as non-failing,
matching run_commit_check’s fail-only failure contract. Update add_job_summary,
add_pr_comments, and set_result_output so skipped-only runs return success and
emit result status “skip,” while mixed pass/skip runs remain successful and
report the appropriate non-fail status; add coverage for both skipped-only and
partial-skip cases.
In `@README.md`:
- Around line 336-338: Update the commit-check dependency pin in
requirements.txt from 2.13.1 to 2.13.4 or newer, ensuring the shipped runtime
matches the README’s documented support for `"status": "skip"`.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: aa627c31-6b75-4a25-800b-55d0fbd62bc6
📒 Files selected for processing (3)
README.mdmain.pymain_test.py
shenxianpeng
force-pushed
the
claude/render-skipped-checks-602anc
branch
2 times, most recently
from
August 7, 2026 11:10
3b2bc13 to
0816365
Compare
PR #258 in this repository reported "All 5 checks passed" over five green ticks while validating nothing: its author is dependabot[bot], which the org config lists in ignore_authors, so every rule was bypassed. The report had no way to say so -- ScopeResult knew only pass and fail, and a skip arrived looking exactly like a pass. The bare labels in that summary were the only hint anything was different, and only because a skipped check reports no value. That was incidental, not a signal. commit-check 2.13.4 reports "status": "skip", so this consumes it: * ScopeResult.status gains "skip", set when every check in the scope skipped. One real verdict outranks the skips. * Skipped scopes render "⊘ <label> (skipped)" -- deliberately not a ✔, and with no value, because nothing was examined. * The headline distinguishes the three cases. All skipped reads "⊘ All N checks skipped — nothing was validated"; a partial skip reads "✅ 3 of 5 checks passed, 2 skipped" rather than claiming all passed. * The step log line follows the same rule. Back-compatible by construction: against an engine that never emits "skip" no branch here is reachable, and a test pins that the old rendering is unchanged. Adds a golden test for the fully skipped report so the layout stays exact, alongside tests for the partial-skip headline, failure precedence, and the one-real-verdict rule. The output specification comment and the README gain the skipped case, the README's rendered from the real renderer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn
Review found the same defect this PR set out to fix, one layer down. run_commit_check has always failed only on "fail", but add_job_summary, add_pr_comments (two call sites) and set_result_output each asked `all(scope.status == "pass")` instead. Those were equivalent only while pass and fail were the only statuses. With skip added, a skipped-only run rendered "⊘ All N checks skipped" and then exited 1 and emitted result status "fail" -- a bypassed policy turned into a broken build. Four copies of a two-state assumption is the same shape as the four copies of the reduce-to-overall rule in commit-check#537, so it gets the same remedy: overall_status() and exit_code_for(), defined once and used by every path. Three smaller findings, all real: * _markdown_table skipped only `pass` scopes, so a run with both a failure and a skip added a row with no value and no rule links -- a blank accusation under a "Failed checks" heading. It now takes failures only. * The all-skipped verdict fired on an empty result set, where `skipped == total` is trivially true, reporting "All 0 checks skipped". It now requires a scope, matching the guard the step log already had. * The step log said "all checks passed (2 skipped)" for a partial skip, contradicting its own report headline. It now reads "2 of 3 checks passed, 1 skipped". Each fix is pinned by a test that fails when the fix is reverted, checked one at a time. Not changed: the requirements.txt pin. The README documents 2.13.4 as the minimum for ⊘ to appear, and 2.13.4 does not exist yet -- commit-check#537 is still open. Pinning it now would install a version that cannot be resolved. The rendering is back-compatible by construction, so the bump belongs with the release, not here. The subject avoids "treat", which reads as imperative but is absent from the 396-verb list in commit-check 2.13.1 -- the version this action still pins. It was added by commit-check#527 and shipped in 2.13.2, so the whitelist on the runner is three releases behind the project's own. The pin bump in #258 fixes that; this only sidesteps it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn
shenxianpeng
force-pushed
the
claude/render-skipped-checks-602anc
branch
from
August 7, 2026 12:24
0816365 to
665ea86
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
#258 in this repository reported "All 5 checks passed" over five green ticks — while validating nothing:
Its author is
dependabot[bot], which the org config lists inignore_authors, so every rule was bypassed. The report had no way to say so:ScopeResultknew onlypassandfail, and a skip arrived looking exactly like a pass.The bare labels above were the only hint anything was different — and only because a skipped check reports no value. That was incidental, not a signal.
What changed
commit-check/commit-check#537 makes the engine report
"status": "skip". This consumes it:ScopeResult.statusgains"skip", set when every check in the scope skipped. One real verdict outranks the skips.⊘ <label> (skipped)— deliberately not a✔, and with no value, since nothing was examined.✅ All 5 checks passed⊘ All 5 checks skipped — nothing was validated✅ All 5 checks passed✅ 3 of 5 checks passed, 2 skipped❌ 1 of 5 checks failedBack-compatibility
Back-compatible by construction: against an engine that never emits
"skip", none of the new branches is reachable and the report is byte-identical. A test pins that.The
⊘rendering needs commit-check 2.13.4 or newer — which does not exist until #537 ships, so this can merge before or after without breaking anything.Verification
106 passed, black clean.mixedscope →pass), and the unchanged old rendering.render_reportitself, not written by hand.The output specification comment in
main.py— the one the two renderers are written against — gains the skipped case alongside the existing success and failure layouts.🤖 Generated with Claude Code
https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn
Generated by Claude Code
Summary by CodeRabbit
⊘indicator.