fix(skills/review-loop): triage existing reviews instead of double-requesting#15
Merged
Merged
Conversation
…questing The loop opened by unconditionally requesting Copilot at Step 1, even when a complete un-triaged review already sat on the current head and the repo auto-reviews on push — stacking duplicate requests. Add an entry "take stock" gate, make Step 1 conditional, and fix Step 6 so a wait timeout on an auto-review-on-push repo is treated as latent (stop) rather than absent (re-trigger). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the review-loop skill to avoid redundant Copilot review requests by first triaging any already-present review on the current head and by treating auto-review-on-push timeouts as pending rather than absent.
Changes:
- Add an entry “take stock” gate to detect and triage an un-triaged Copilot review on the current head before requesting anything new.
- Make Step 1’s explicit reviewer request conditional (and update the loop diagram accordingly).
- Refine Step 6 timeout handling to prevent double-triggering on repos that auto-review on push, and add matching “Red flags” guidance.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+42
to
+55
| @@ -50,6 +51,8 @@ digraph review_loop { | |||
| "Done: clean" [shape=doublecircle]; | |||
| "Stop, report what remains" [shape=box]; | |||
|
|
|||
| "Take stock: un-triaged review on current head?\nrepo auto-reviews on push?" -> "Triage every open comment\n(receiving-code-review)" [label="review already present"]; | |||
| "Take stock: un-triaged review on current head?\nrepo auto-reviews on push?" -> "Capture watermark, request Copilot" [label="none to triage"]; | |||
|
|
||
| 1. **Capture the watermark, then request.** `SINCE=$(date -u +%Y-%m-%dT%H:%M:%SZ)` *before* `gh pr edit <pr> --add-reviewer "@copilot"`. The watermark is what distinguishes the new review from a stale one left by an earlier round. | ||
| 1. **Capture the watermark, then request — only when the entry check says you need to.** `SINCE=$(date -u +%Y-%m-%dT%H:%M:%SZ)` *before* `gh pr edit <pr> --add-reviewer "@copilot"`. The watermark is what distinguishes the new review from a stale one left by an earlier round. Skip this step and go straight to triage (Step 4) when a current un-triaged review already exists, or when a push you are about to make will auto-trigger the review. | ||
| 2. **Confirm Copilot attached.** `gh pr view <pr> --json reviewRequests`. If Copilot is **not** in the list, it is unavailable for this repo/plan (or `gh` is too old / this is GHES). Do **not** start waiting — there is no reviewer to produce a review. Instead do one triage pass over any human comments already on the PR (Step 3), state that Copilot review is unavailable, and exit. There is no loop without an automated reviewer to re-request. |
The take-stock diamond asked about both an existing review and auto-review-on-push but only branched on the former, so the "none to triage" edge routed to an explicit request — contradicting the prose that says an auto-review repo should wait for the queued review, not request. Split it into two diamonds so the auto-review-on-push path waits instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Copilot-unavailable fallback pointed at "(Step 3)" for the triage pass, but triage is Step 4 (Step 3 is the background wait). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
Description
review-loopopened every run by unconditionally requesting Copilot at Step 1.When a complete but un-triaged review already sat on the current head — and
especially on a repo that auto-reviews on push — that stacked a redundant
request on top of work that was already there, and the "re-requesting is
unreliable, never assume it fired" guidance then provoked an immediate retry,
producing two explicit requests in a row. This makes the loop triage what is
already on the PR before asking for anything new.
Changes
is this round's review (triage it, Step 4); if the repo auto-reviews on push,
pushing is the trigger and no explicit request is stacked on it.
with a matching branch added to the loop diagram.
treated as latent, not absent — stop and report the pending review instead
of re-triggering and double-firing.
timeout-driven re-trigger.
Challenges
The double-request had two distinct origins — the unconditional entry request
and a wait-timeout re-trigger on a repo whose auto-review simply lags the push
(by hours). The entry gate decapitates the first; the Step 6 rewrite closes the
second. Both were verified with isolated subagents: the pre-fix skill fires the
redundant request, the post-fix skill issues zero explicit requests under the
same impatience pressure and triages instead.
Testing
Developed via the
writing-skillsRED→GREEN→REFACTOR loop with isolatedsubagents driving the skill text against the exact incident state (a 4h-old
un-triaged review on head, repo auto-reviews on push with ~2h latency):
gh pr edit --add-reviewerat Step 1 despite theexisting review.
auto-review repo, even under explicit time pressure.
🤖 Generated with Claude Code