Skip to content

Commit 5d7b1b2

Browse files
justin808claude
andcommitted
Address PR review: fix summary POST bug and clarify cutoff filters
- Step 10: replace `gh api --input` (which sends raw markdown) with `gh pr comment --body-file`; the API expected JSON, so the previous call would 422 and break the cutoff mechanism. - Step 4: split the ambiguous "created after" cutoff bullet into explicit per-type rules — issue comments and inline threads use `created_at`; review summaries use `submitted_at`. - Normalize shell snippets to use `${PR_NUMBER}`/`${REVIEW_ID}`/`${COMMENT_ID}`/`${THREAD_ID}` consistently so examples are directly runnable. - Step 10 timing: require posting the checkpoint only at the final stable stopping point, not after intermediate steps in a chain. - Readability polish on Action `f`/`f+i` steps. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent fc442cc commit 5d7b1b2

1 file changed

Lines changed: 21 additions & 20 deletions

File tree

commands/address-review.md

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ For full-PR scans (plain PR number or PR URL with no specific review/comment anc
5454
Fetch the latest summary comment before collecting review data:
5555

5656
```bash
57-
gh api --paginate repos/${REPO}/issues/{PR_NUMBER}/comments | jq -s '[.[].[] | select(((.body // "") | contains("<!-- address-review-summary -->"))) | {id: .id, created_at: .created_at, html_url: .html_url}] | sort_by(.created_at) | last'
57+
gh api --paginate repos/${REPO}/issues/${PR_NUMBER}/comments | jq -s '[.[].[] | select(((.body // "") | contains("<!-- address-review-summary -->"))) | {id: .id, created_at: .created_at, html_url: .html_url}] | sort_by(.created_at) | last'
5858
```
5959

6060
Cutoff rules:
@@ -70,17 +70,17 @@ Cutoff rules:
7070
**If a specific issue comment ID is provided (`#issuecomment-...`):**
7171

7272
```bash
73-
gh api repos/${REPO}/issues/comments/{COMMENT_ID} | jq '{body: .body, user: .user.login, created_at: .created_at, html_url: .html_url}'
73+
gh api repos/${REPO}/issues/comments/${COMMENT_ID} | jq '{body: .body, user: .user.login, created_at: .created_at, html_url: .html_url}'
7474
```
7575

7676
**If a specific review ID is provided (`#pullrequestreview-...`):**
7777

7878
```bash
7979
# Review body (often contains summary feedback)
80-
gh api repos/${REPO}/pulls/{PR_NUMBER}/reviews/{REVIEW_ID} | jq '{id: .id, body: .body, state: .state, user: .user.login, submitted_at: .submitted_at, html_url: .html_url}'
80+
gh api repos/${REPO}/pulls/${PR_NUMBER}/reviews/${REVIEW_ID} | jq '{id: .id, body: .body, state: .state, user: .user.login, submitted_at: .submitted_at, html_url: .html_url}'
8181

8282
# Inline comments for this review
83-
gh api --paginate repos/${REPO}/pulls/{PR_NUMBER}/reviews/{REVIEW_ID}/comments | jq -s '[.[].[] | {id: .id, node_id: .node_id, path: .path, body: .body, line: .line, start_line: .start_line, user: .user.login, in_reply_to_id: .in_reply_to_id, created_at: .created_at, html_url: .html_url}]'
83+
gh api --paginate repos/${REPO}/pulls/${PR_NUMBER}/reviews/${REVIEW_ID}/comments | jq -s '[.[].[] | {id: .id, node_id: .node_id, path: .path, body: .body, line: .line, start_line: .start_line, user: .user.login, in_reply_to_id: .in_reply_to_id, created_at: .created_at, html_url: .html_url}]'
8484
```
8585

8686
Include the review body as a general comment when it contains actionable feedback. When the review body contains actionable feedback, note that it cannot be replied to via the `/replies` endpoint — responses to review summary bodies must be posted as general PR comments (see Step 8).
@@ -89,21 +89,22 @@ Include the review body as a general comment when it contains actionable feedbac
8989

9090
```bash
9191
# Review summary bodies (can contain actionable feedback even without inline comments)
92-
gh api --paginate repos/${REPO}/pulls/{PR_NUMBER}/reviews | jq -s '[.[].[] | select((.body // "") != "") | {id: .id, type: "review_summary", body: .body, state: .state, user: .user.login, submitted_at: .submitted_at, html_url: .html_url}]'
92+
gh api --paginate repos/${REPO}/pulls/${PR_NUMBER}/reviews | jq -s '[.[].[] | select((.body // "") != "") | {id: .id, type: "review_summary", body: .body, state: .state, user: .user.login, submitted_at: .submitted_at, html_url: .html_url}]'
9393

9494
# Inline code review comments
95-
gh api --paginate repos/${REPO}/pulls/{PR_NUMBER}/comments | jq -s '[.[].[] | {id: .id, node_id: .node_id, type: "review", path: .path, body: .body, line: .line, start_line: .start_line, user: .user.login, in_reply_to_id: .in_reply_to_id, created_at: .created_at, html_url: .html_url}]'
95+
gh api --paginate repos/${REPO}/pulls/${PR_NUMBER}/comments | jq -s '[.[].[] | {id: .id, node_id: .node_id, type: "review", path: .path, body: .body, line: .line, start_line: .start_line, user: .user.login, in_reply_to_id: .in_reply_to_id, created_at: .created_at, html_url: .html_url}]'
9696

9797
# General PR discussion comments (not tied to specific lines)
98-
gh api --paginate repos/${REPO}/issues/{PR_NUMBER}/comments | jq -s '[.[].[] | {id: .id, node_id: .node_id, type: "issue", body: .body, user: .user.login, created_at: .created_at, html_url: .html_url}]'
98+
gh api --paginate repos/${REPO}/issues/${PR_NUMBER}/comments | jq -s '[.[].[] | {id: .id, node_id: .node_id, type: "issue", body: .body, user: .user.login, created_at: .created_at, html_url: .html_url}]'
9999
```
100100

101101
Include actionable review summary bodies from `/pulls/{PR_NUMBER}/reviews` as additional general comments. Like specific review bodies, they cannot be replied to via the `/replies` endpoint and must be answered as general PR comments (see Step 8).
102102

103103
When `REVIEW_CUTOFF_AT` is set for a full-PR scan:
104104

105105
- Fetch the full datasets above so you keep older context for unresolved threads.
106-
- Filter issue comments and review summaries to items created after `REVIEW_CUTOFF_AT`.
106+
- Filter issue comments by `created_at > REVIEW_CUTOFF_AT`.
107+
- Filter review summaries by `submitted_at > REVIEW_CUTOFF_AT` (review summary objects from `/pulls/{PR_NUMBER}/reviews` expose `submitted_at`, not `created_at`).
107108
- For inline review threads, keep an unresolved thread only when at least one comment in that thread has `created_at > REVIEW_CUTOFF_AT`.
108109
- Use the thread's top-level comment as the triage item, and use newer replies in that thread as the latest context.
109110
- Do not let older comments with no new activity re-enter triage unless the user asked for `check all reviews`.
@@ -113,7 +114,7 @@ When `REVIEW_CUTOFF_AT` is set for a full-PR scan:
113114
```bash
114115
OWNER=${REPO%/*}
115116
NAME=${REPO#*/}
116-
gh api graphql --paginate -f owner="${OWNER}" -f name="${NAME}" -F pr={PR_NUMBER} -f query='query($owner:String!, $name:String!, $pr:Int!, $endCursor:String) { repository(owner:$owner, name:$name) { pullRequest(number:$pr) { reviewThreads(first:100, after:$endCursor) { nodes { id isResolved comments(first:100) { nodes { id databaseId } } } pageInfo { hasNextPage endCursor } } } } }' | jq -s '[.[].data.repository.pullRequest.reviewThreads.nodes[] | {thread_id: .id, is_resolved: .isResolved, comments: [.comments.nodes[] | {node_id: .id, id: .databaseId}]}]'
117+
gh api graphql --paginate -f owner="${OWNER}" -f name="${NAME}" -F pr=${PR_NUMBER} -f query='query($owner:String!, $name:String!, $pr:Int!, $endCursor:String) { repository(owner:$owner, name:$name) { pullRequest(number:$pr) { reviewThreads(first:100, after:$endCursor) { nodes { id isResolved comments(first:100) { nodes { id databaseId } } } pageInfo { hasNextPage endCursor } } } } }' | jq -s '[.[].data.repository.pullRequest.reviewThreads.nodes[] | {thread_id: .id, is_resolved: .isResolved, comments: [.comments.nodes[] | {node_id: .id, id: .databaseId}]}]'
117118
```
118119

119120
**Filtering comments:**
@@ -196,8 +197,8 @@ Do not post the PR summary checkpoint during this triage-only phase. Post it onl
196197

197198
### Action `f` — Fix and merge-ready
198199

199-
1. Address all `MUST-FIX` items (make code changes, run checks). If there are no `MUST-FIX` items, skip directly to discuss/skipped handling.
200-
2. If local changes exist, commit and then ask for push confirmation before pushing. If there are no local changes, skip commit/push and continue decision flow.
200+
1. Address all `MUST-FIX` items (make code changes, run checks). Skip directly to discuss/skipped handling when there are no `MUST-FIX` items.
201+
2. Commit any local changes and ask for push confirmation before pushing; continue the decision flow when there are no local changes to commit.
201202
3. Reply to each addressed comment explaining the fix.
202203
4. Resolve the corresponding review threads.
203204
5. If `SKIPPED` items exist, ask for explicit confirmation before posting rationale replies and resolving those threads (for example: "Reply/resolve 3 skipped items? y/n").
@@ -207,11 +208,11 @@ Do not post the PR summary checkpoint during this triage-only phase. Post it onl
207208

208209
### Action `f+i` — Fix, follow-up issue, and merge-ready
209210

210-
1. Do everything in `f` for `MUST-FIX` items. If there are no `MUST-FIX` items, skip the fix phase and continue with deferred-item handling.
211+
1. Do everything in `f` for `MUST-FIX` items; skip the fix phase and continue with deferred-item handling when there are no `MUST-FIX` items.
211212
2. Create a **follow-up GitHub issue** (see Step 9) bundling all `DISCUSS` and non-trivial `SKIPPED` items.
212213
3. For each deferred item in the follow-up issue, post a reply in the original location referencing the issue (use review-comment replies for inline comments and issue comments for review summaries/general comments), and resolve the thread when one exists. For general PR comments and review summary bodies (which have no thread), the reply alone is sufficient.
213214
4. For trivial `SKIPPED` items that are not included in the follow-up issue (duplicates, factually incorrect suggestions, status noise), still post rationale replies and resolve those threads.
214-
5. If there are zero deferred items, skip issue creation and behave like `f`.
215+
5. Skip issue creation and behave like `f` when there are zero deferred items.
215216
6. No additional commit is required unless later steps introduce local changes; if they do, commit and ask for push confirmation before pushing.
216217
7. Tell the user the PR is merge-ready.
217218

@@ -253,23 +254,23 @@ If the user selects skipped/declined items for rationale replies, post those rep
253254
**For issue comments (general PR comments):**
254255

255256
```bash
256-
gh api repos/${REPO}/issues/{PR_NUMBER}/comments -X POST -f body="<response>"
257+
gh api repos/${REPO}/issues/${PR_NUMBER}/comments -X POST -f body="<response>"
257258
```
258259

259260
**For PR review comments (file-specific, replying to a thread):**
260261

261262
```bash
262-
gh api repos/${REPO}/pulls/{PR_NUMBER}/comments/{COMMENT_ID}/replies -X POST -f body="<response>"
263+
gh api repos/${REPO}/pulls/${PR_NUMBER}/comments/${COMMENT_ID}/replies -X POST -f body="<response>"
263264
```
264265

265266
Use the `/replies` endpoint for all existing review comments, including standalone top-level comments.
266267

267-
**For review summary bodies (from `/pulls/{PR_NUMBER}/reviews/{REVIEW_ID}`):**
268+
**For review summary bodies (from `/pulls/{PR_NUMBER}/reviews/${REVIEW_ID}`):**
268269

269270
Review summary bodies do not have a `comment_id` and cannot be replied to via the `/replies` endpoint. Instead, post a general PR comment referencing the review:
270271

271272
```bash
272-
gh api repos/${REPO}/issues/{PR_NUMBER}/comments -X POST -f body="<response>"
273+
gh api repos/${REPO}/issues/${PR_NUMBER}/comments -X POST -f body="<response>"
273274
```
274275

275276
The response should briefly explain:
@@ -287,7 +288,7 @@ After posting the reply, resolve the review thread when all of the following are
287288
Use GitHub GraphQL to resolve the thread:
288289

289290
```bash
290-
gh api graphql -f query='mutation($threadId:ID!) { resolveReviewThread(input:{threadId:$threadId}) { thread { id isResolved } } }' -f threadId="<THREAD_ID>"
291+
gh api graphql -f query='mutation($threadId:ID!) { resolveReviewThread(input:{threadId:$threadId}) { thread { id isResolved } } }' -f threadId="${THREAD_ID}"
291292
```
292293

293294
Do not resolve a thread if the fix is still pending, if you are unsure whether the reviewer concern is satisfied, or if the user asked to leave the thread open.
@@ -366,7 +367,7 @@ Rules for follow-up issues:
366367

367368
## Step 10: Post PR Summary Comment
368369

369-
After any chosen action or completed action chain (`f`, `f+i`, `d`, `r`, `m`, or direct item selection), post a consolidated PR comment that becomes the next default review cutoff.
370+
Post this comment only once, after the current run reaches its final stable stopping point — the full action or action chain (`f`, `f+i`, `d`, `r`, `m`, or direct item selection) is complete and no further pending work remains. Do not post after intermediate steps within a chain (for example, do not post after the `d` step of `f+i → d`).
370371

371372
Rules for the summary comment:
372373

@@ -396,7 +397,7 @@ summary_body_file="$(mktemp)"
396397
printf 'Next default scan starts after this comment. Say `check all reviews` to rescan the full PR.\n'
397398
} > "${summary_body_file}"
398399

399-
gh api repos/${REPO}/issues/${PR_NUMBER}/comments -X POST --input "${summary_body_file}"
400+
gh pr comment "${PR_NUMBER}" --repo "${REPO}" --body-file "${summary_body_file}"
400401
rm -f "${summary_body_file}"
401402
```
402403

0 commit comments

Comments
 (0)