Add interactive PR walkthrough workflow - #278
Conversation
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
WalkthroughAdds the ChangesPR walkthrough and merge-authority flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Coordinator
participant OrdinaryGates
participant PRWalkthrough
participant Readiness
participant Human
Coordinator->>OrdinaryGates: Run ordinary readiness gates
OrdinaryGates->>Coordinator: Return clean gates
Coordinator->>PRWalkthrough: Start exact-diff walkthrough
PRWalkthrough->>Human: Present one conceptual change at a time
Human->>PRWalkthrough: Continue, stop, or skip
PRWalkthrough->>Readiness: Trigger diff identity and readiness refresh
Readiness->>Coordinator: Return refreshed status
Coordinator->>Human: Ask one final merge decision
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Review: Add interactive PR walkthrough workflowOverviewThis PR adds a new Strengths
Notes / minor concerns
Overall: well-scoped, consistent with the repo's existing dense-instruction conventions, and the guardrails against conflating "walkthrough" with "approval" are solid. The main ask is just to make the incidental bullet-compression edits explicit in the description if they weren't already intentional. |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
skills/pr-walkthrough/bin/pr-walkthrough-contract-test.rb (1)
13-39: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winStrengthen the contract tests beyond substring presence.
Whitespace-normalized
assert_includeschecks can pass contradictory text as long as each phrase appears somewhere. Assert ordering and coupled invariants—especially one-change-per-response, walkthrough-before-decision, explicit readiness, changed-head handling, and “not merge approval.”🤖 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 `@skills/pr-walkthrough/bin/pr-walkthrough-contract-test.rb` around lines 13 - 39, Strengthen test_skill_is_exact_head_interactive_and_complete and test_ask_authority_automatically_walks_through_before_merge_decision beyond independent substring checks: assert the required phrases appear in the correct order and validate their coupled invariants. Cover one conceptual change per response, walkthrough-before-merge decision, explicit readiness before advancing, changed-head refresh handling, and that walkthrough responses or positive reactions are not merge approval.
🤖 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 `@docs/pr-batch-skills.md`:
- Around line 247-252: Update the guidance around the $pr-walkthrough flow to
require a final readiness recheck before asking the merge question. If the exact
head changes or any gate newly fails, stop the flow instead of asking; ask only
when the refreshed head remains clean and ready, consistent with the contract in
workflows/pr-processing.md.
In `@workflows/pr-processing.md`:
- Line 1165: Propagate exact-head invalidation across all ask-mode consumers:
expand the compact Plan→Goal rule in workflows/pr-processing.md at lines
1165-1165; update the generated goal template in skills/pr-batch/SKILL.md at
lines 511-511 to require head/readiness refresh and invalidate evidence when the
SHA changes; add the changed-SHA restart-or-stop branch before the final
decision at skills/pr-batch/SKILL.md lines 764-774 and
skills/pr-monitoring/SKILL.md lines 119-128.
---
Nitpick comments:
In `@skills/pr-walkthrough/bin/pr-walkthrough-contract-test.rb`:
- Around line 13-39: Strengthen
test_skill_is_exact_head_interactive_and_complete and
test_ask_authority_automatically_walks_through_before_merge_decision beyond
independent substring checks: assert the required phrases appear in the correct
order and validate their coupled invariants. Cover one conceptual change per
response, walkthrough-before-merge decision, explicit readiness before
advancing, changed-head refresh handling, and that walkthrough responses or
positive reactions are not merge approval.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7037c616-e77c-480c-9206-1626c35bbd2a
📒 Files selected for processing (14)
CHANGELOG.mdREADME.mdbin/validatedocs/README.mddocs/pr-batch-skills.mdskills/plan-pr-batch/SKILL.mdskills/plan-pr-batch/scripts/check_goal_prompt_size.rbskills/pr-batch/SKILL.mdskills/pr-monitoring/SKILL.mdskills/pr-walkthrough/SKILL.mdskills/pr-walkthrough/agents/openai.yamlskills/pr-walkthrough/bin/pr-walkthrough-contract-test.rbskills/triage/SKILL.mdworkflows/pr-processing.md
![chatgpt-codex-connector[bot]](https://avatars.github.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: d1e0899eb4
ℹ️ 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".
|
Address-review pass complete for the full review history.
Next default scan starts after this comment; earlier items are terminal unless reopened. |
Review SummaryThis PR adds a new What it does
Strengths
Potential issue
Minor observations (non-blocking)
No security, correctness, or test-coverage blockers found. Verification claims in the PR description (bin/validate, targeted contract tests) look consistent with the actual changes. |
![claude[bot]](https://avatars.github.com/in/1236702?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: 5df07f548b
ℹ️ 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".
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 `@skills/plan-pr-batch/scripts/check_goal_prompt_size.rb`:
- Around line 158-163: Update the compact contract string
GOAL_MODE_COMPACT_CONTRACT to include the walk-through contract phrase required
by pr-walkthrough-contract-test.rb. Preserve the existing sequencing and
merge-approval wording while adding the missing phrase so the validated batch
goal prompt passes.
In `@skills/pr-batch/SKILL.md`:
- Around line 765-774: The final merge-decision condition in the PR walkthrough
flow must allow an explicitly skipped walkthrough. Update the logic around the
walkthrough completion/skip handling so the final decision requires the same
diff identity, clean readiness, and merge authorization, but not an explanation
when the walkthrough was skipped; preserve the existing explanation requirement
for completed walkthroughs.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a52ea08b-9413-495e-9892-229ca0e4fa1f
📒 Files selected for processing (10)
CHANGELOG.mddocs/pr-batch-skills.mdskills/plan-pr-batch/SKILL.mdskills/plan-pr-batch/scripts/check_goal_prompt_size.rbskills/pr-batch/SKILL.mdskills/pr-monitoring/SKILL.mdskills/pr-walkthrough/SKILL.mdskills/pr-walkthrough/bin/pr-walkthrough-contract-test.rbskills/triage/SKILL.mdworkflows/pr-processing.md
🚧 Files skipped from review as they are similar to previous changes (5)
- docs/pr-batch-skills.md
- skills/pr-walkthrough/SKILL.md
- skills/pr-monitoring/SKILL.md
- workflows/pr-processing.md
- CHANGELOG.md
ReviewReviewed via OverviewAdds a new standalone Strengths
Issues / suggestions
SecurityNo concerns beyond what's already called out as a strength above — the untrusted-input handling for the walkthrough's own inputs (PR text/diff) is appropriately cautious, and the skill is explicitly scoped read-only with no merge/approval authority. Test coverage
|
|
Final review wave triaged on
Next default scan starts after this comment; earlier items are terminal unless reopened. |
|
Completed-batch audit: replay evidence follows. |
1 participant
Summary
$pr-walkthroughskill that explains one conceptual change at a time, including rationale, tradeoffs, risk, and proofmerge_authority: askautomatically start a diff-identity walkthrough before its one final merge decision, using full coverage for large or complex PRsDesign rationale
The walkthrough first maps the complete diff, then presents concepts in dependency order rather than making a human reconstruct intent from file order. Each step pauses for questions and explicit readiness. A private coverage ledger prevents a friendly narrative from silently omitting changed paths.
The approach draws on Google’s guidance to understand the broad change first, inspect important parts in a logical sequence, and capture both what changed and why; GitHub’s guidance to identify purpose, important files, and review order; and research showing that coherent change decomposition supports reviewer understanding.
Compatibility note
Existing
merge_authority: askusers now receive a concise walkthrough before the final merge question; they can explicitly skip it. The compact generated prompt intentionally preserves the self-contained dispatch rule that reloads persisted state before preflight and persists output before resuming or launching work.Verification
PR_BATCH_GIT_PROBE_TIMEOUT_SECONDS=30 bin/validatequick_validate.py)Summary by CodeRabbit
New Features
$pr-walkthroughskill that explains a pull request one conceptual change at a time (read-only), using an exact-diff “diff identity” with coverage tracking and refresh/invalidation when the diff changes.merge_authority: askflows to automatically run the walkthrough after ordinary gates are clean, then request exactly one final merge decision (walkthrough is not merge approval), with concise/full modes for size/complexity.Documentation
Tests
askautomation.Completed-batch audit
Status: Clean — no outstanding findings or follow-ups. Durable receipt.