Skip to content

Make PR closeout work-conserving across review and CI#212

Closed
ihabadham wants to merge 1 commit into
shakacode:mainfrom
ihabadham:ihabadham/fix/work-conserving-pr-closeout
Closed

Make PR closeout work-conserving across review and CI#212
ihabadham wants to merge 1 commit into
shakacode:mainfrom
ihabadham:ihabadham/fix/work-conserving-pr-closeout

Conversation

@ihabadham

@ihabadham ihabadham commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Why

PR closeout currently serializes bounded CI/reviewer polling ahead of review triage. That leaves runnable closeout work idle and can cause partial reviewer triage when only the hardcoded Claude check has completed.

What changed

  • split current-head checks into a complete reviewer cohort and an independent validation-CI cohort
  • wait for the full reviewer wave, then fetch and triage it once while unrelated validation may continue
  • require agents to exhaust runnable closeout work before another poll or sleep, and restart both cohorts after a push
  • make address-review fail closed on missing, pending, malformed, or unknown reviewer-wave state instead of fetching partial feedback
  • add a replay contract covering the canonical workflow, entry skills, continuation behavior, and workflow mirrors

Verification

  • ruby skills/pr-batch/bin/review-wave-contract-test.rb
  • ruby skills/pr-batch/bin/single_target_entrypoint_test.rb
  • ruby skills/pr-batch/bin/goal-completion-contract-test.rb
  • bash -n on the updated address-review Step 4 shell block
  • bin/validate on the rebased committed head (with a temporary Linux shasum -> sha256sum compatibility shim for existing tests)

Related: #179, #23. Neither issue tracks or is closed by this scheduling fix.

Summary by CodeRabbit

  • Bug Fixes

    • Improved pull request closeout by waiting for the complete current-head review wave before consolidated review triage.
    • Kept validation checks and independent closeout work progressing concurrently.
    • Prevented partial review feedback from being used when reviewer results are pending, unavailable, or incomplete.
    • Restarted review and validation tracking when new changes are pushed.
  • Documentation

    • Clarified review readiness, monitoring, continuation, and restart guidance for pull request workflows.
  • Tests

    • Added coverage to verify complete review-wave, validation, batching, and restart behavior.

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 688fa7d3-d7c6-4af7-a280-0be3b1c5aa97

📥 Commits

Reviewing files that changed from the base of the PR and between 10c2eeb and 75e061c.

📒 Files selected for processing (12)
  • CHANGELOG.md
  • bin/validate
  • docs/pr-batch-skills.md
  • skills/address-review/SKILL.md
  • skills/continue/SKILL.md
  • skills/plan-pr-batch/scripts/check_goal_prompt_size.rb
  • skills/pr-batch/SKILL.md
  • skills/pr-batch/bin/review-wave-contract-test.rb
  • skills/pr-batch/bin/single_target_entrypoint_test.rb
  • skills/pr-monitoring/SKILL.md
  • workflows/address-review.md
  • workflows/pr-processing.md

Walkthrough

The PR separates current-head reviewer checks from validation CI, requiring complete review-wave settlement and reviewer artifacts before consolidated triage. It permits independent closeout work, invalidates both cohorts after pushes, updates continuation guidance, and adds contract tests and validation wiring.

Changes

Review closeout coordination

Layer / File(s) Summary
Two-cohort closeout contracts
docs/pr-batch-skills.md, skills/pr-batch/..., skills/pr-monitoring/..., skills/continue/..., workflows/pr-processing.md, skills/plan-pr-batch/...
PR-batch and monitoring guidance separates review-agent checks from validation CI, allows independent runnable closeout work, consolidates triage after review settlement, and restarts both cohorts after a push.
Complete review-wave gating
workflows/address-review.md, skills/address-review/SKILL.md
Full-PR review flows resolve REVIEW_CHECK_NAMES_JSON, wait for all expected checks and reviewer artifacts, and return waiting-on-checks-or-review instead of using partial feedback.
Contract validation and release wiring
skills/pr-batch/bin/review-wave-contract-test.rb, skills/pr-batch/bin/single_target_entrypoint_test.rb, bin/validate, CHANGELOG.md
New contract assertions cover cohort barriers, artifact settlement, stale handoffs, and partial-review rejection; validation runs the test and the changelog records the behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant PRBatchCoordinator
  participant ReviewGate
  participant gh_pr_checks
  participant ReviewerArtifacts
  PRBatchCoordinator->>ReviewGate: resolve expected review cohort
  ReviewGate-->>PRBatchCoordinator: REVIEW_CHECK_NAMES_JSON
  PRBatchCoordinator->>gh_pr_checks: poll expected checks
  gh_pr_checks-->>PRBatchCoordinator: terminal check states
  PRBatchCoordinator->>ReviewerArtifacts: snapshot current-head artifacts
  ReviewerArtifacts-->>PRBatchCoordinator: reviewer outputs and dispositions
  PRBatchCoordinator->>PRBatchCoordinator: perform one consolidated fetch and triage
Loading

Possibly related PRs

Suggested reviewers: justin808

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the main change: making PR closeout work-conserving across reviewer triage and validation CI.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@ihabadham

Copy link
Copy Markdown
Contributor Author

Superseded by #213, recreated from an upstream-owned branch after repository write access was granted. The replacement uses the same validated commit and preserves the original scope.

@ihabadham ihabadham closed this Jul 20, 2026
@ihabadham
ihabadham deleted the ihabadham/fix/work-conserving-pr-closeout branch July 20, 2026 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant