Repository navigation
Gate Prose on the Merge Commit's Own Base, Not the Event's base.sha - #2529
Conversation
A pull request's checkout is GitHub's merge commit, built on the base branch's tip, while the event's base.sha can predate commits landed since, as after a close and reopen. Diffing against it scanned those commits as the pull request's own. Resolve the base as the merge commit's first parent, keeping the event's base.sha for a checkout that is no merge commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe pull-request workflow now selects a base for the prose gate, using ChangesProse-gate base selection
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~12 minutes Merge Risk: ⚪ Minimal · up to No identified issue blocks merging after normal checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 58.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 1 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 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 |
There was a problem hiding this comment.
🟢 Approval recommended
All reviewed changes are covered by regression tests with no unresolved blocking issues.
0 open findings
What changed in this PR
Updates the prose gate to compare against the merge commit’s actual base, avoiding unrelated base-branch changes.
Changes:
- Resolve the merge commit’s first parent as the prose base.
- Fall back to the event base for non-merge checkouts.
- Add regression tests for base resolution and wiring.
| File | Summary |
|---|---|
tests/test_prose_lint.py |
Adds regression coverage for base resolution and gate wiring. |
.github/workflows/validate-task.yml |
Resolves and passes the correct prose diff base. |
🧠 Review effort: Lite
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2529 +/- ##
==========================================
Coverage ? 58.40%
==========================================
Files ? 16
Lines ? 8090
Branches ? 0
==========================================
Hits ? 4725
Misses ? 3365
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
…2532) ## Summary Promotes one change from `develop`: - #2529: the prose gate in `validate-task.yml` diffs against the base the pull request's checkout was built on. The checkout is GitHub's merge commit, and the event's `base.sha` can be older than its first parent, as after a close and reopen, which made the gate scan commits landed on the base branch since as the pull request's own. A new `Resolve prose base step` uses the merge commit's first parent, and keeps the event's `base.sha` for a checkout that is no merge commit. A promotion still diffs against the default branch's tip. Fixes #2490 ## Verification - `TestTheGateDiffsAgainstTheMergeCommitsOwnBase` runs the workflow step's own script and the composite action's own script against a constructed repository whose base moved after the event, and shows the event's base reporting a landed commit's comment while the resolved base does not. Reverting the guard, and reverting the gate's input to `base.sha`, each failed the suite. - On #2529's own CI run the step logged its resolved base, and the gate's scope was the pull request's 2 files. - #2529 took two recorded local strict-review passes, the first finding fixed before the push, and a Copilot and a CodeRabbit round on its head with no findings. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Summary
The prose gate in
validate-task.ymldiffed againstgithub.event.pull_request.base.shawhile the lint job checks out GitHub's merge commit for the pull request. The merge commit is built on the base branch's current tip, and the event'sbase.shacan be older than that, as after a close and reopen. The diff then scanned every commit landed on the base branch in between as the pull request's own, and failed a pull request that added nothing the gate reports.A new
Resolve prose base stepresolves the base as the checkout's first parent where HEAD is a merge commit, which is by construction the base the checkout was built on, and keeps the event'sbase.shafor a checkout that is no merge commit. The gate step reads that output. A promotion still diffs against the default branch's tip, since that is the merge commit's first parent there.Tests
TestTheGateDiffsAgainstTheMergeCommitsOwnBaseintests/test_prose_lint.pyruns the workflow step's own script and the composite action's own script against a constructed repository, where a commit carrying a comment lands on the base after the event's base and before the merge:HEAD^1held apart from it so an unguarded first parent failsEach of the guard and the wiring was reverted in turn, and the suite failed each time. The step-script extraction is shared with the existing label-step helper.
Closes on promotion: #2490
🤖 Generated with Claude Code
Summary by CodeRabbit