Dev Review Pr

Check GitHub Actions status, review feedback from automated reviewers (Code Rabbit, Claude), implement or respond to feedback, and merge the PR when all feedback is resolved.

> MANDATORY: Run /dev-local-review before every commit and push. > This applies to step 5 (commit and push fixes) and any other commit made > during this workflow. Do NOT push code that has not passed a local review. > Skipping this step wastes a GitHub review round on issues that could have > been caught locally for free.

The user provides:

• PR number: the pull request number to review (optional - infers from current branch)

Workflow

1. Check GitHub Actions status

Use gh pr checks to see if automated reviews are complete.

Important: This command returns exit code 8 if any checks are pending, and outputs them to stderr. Handle both stdout and stderr to capture all check statuses. Output format: check-name status duration url

If any checks are still running (pending):

• Report which checks are pending
• Exit with message: "GitHub Actions still running. Run this skill again when checks complete."
• DO NOT sleep or wait — let the user run the skill again later

If checks failed:

• Report which checks failed
• Show the failure URL from the output
• If markdown linting failed, offer to run it locally and fix issues (see step 1.5)
• Otherwise exit and ask user to investigate

If all checks passed:

• Proceed to step 2 (fetch review comments)

1.5. Fix markdown linting failures (if needed)

If the "Markdown Lint" check failed:

1. Run locally to see errors:

``bash npx markdownlint-cli2 "**/*.md" ``

1. Attempt auto-fix:

``bash npx markdownlint-cli2 --fix "**/*.md" ``

1. Manually fix remaining errors (especially MD040 - missing language tags)

1. Verify all errors resolved:

``bash npx markdownlint-cli2 "**/*.md" ``

1. Commit and push fixes:

```bash git add git commit -m "Fix markdown linting errors

Co-Authored-By: Claude " git push ```

1. Exit and wait for checks to re-run — user should invoke /dev-review-pr again after checks pass

1.7. Check CodeRabbit review status

CodeRabbit is configured to auto-pause after the first reviewed commit (reviews.auto_review.auto_pause_after_reviewed_commits: 1 in .coderabbit.yaml). This means after every push, CodeRabbit reviews once and then pauses. The "Reviews paused" banner in the PR comment is normal and expected — it does NOT mean something went wrong or that the review is missing.

Key insight: CodeRabbit completes its review before pausing. The review comments and review body are already available.

How to check:

Look at the CodeRabbit check status from step 1:

• If CodeRabbit shows pass — the review is done. **Proceed directly to

step 2** to fetch the review comments. Ignore the "Reviews paused" banner — it just means future commits won't be auto-reviewed.

• If CodeRabbit shows pending or no CodeRabbit check yet — the review

hasn't started or is still running. Exit and ask the user to re-run later.

After implementing feedback and pushing fixes (step 5):

Since auto-pause is on, CodeRabbit will NOT automatically review the new commit. Post a single comment to request a fresh review:

gh pr comment  --body "@coderabbitai review"

Do NOT post @coderabbitai resume — we want it to stay paused after each review to avoid review thrashing from rapid commits.

If CodeRabbit shows "Currently processing new changes":

• The review is actively running. Exit and tell the user to re-run later.
• Do NOT fetch review comments yet — they may be incomplete.

1.7a. Detect clean re-review (no actionable comments)

After CodeRabbit re-reviews a fix commit, it sometimes finds nothing new to flag. When this happens, CodeRabbit does not post an APPROVED review. Instead, it posts an issue comment (not a review comment) containing:

> No actionable comments were generated in the recent review.

The review state from step 6 may still show CHANGES_REQUESTED or COMMENTED from a previous round — that's stale and misleading.

How to detect:

# Get the latest "no actionable comments" timestamp
NO_ACTION_TS=$(gh api repos/{owner}/{repo}/issues/{pr-number}/comments --paginate \
  | jq -r -s '(add // []) | map(select(.user.login == "coderabbitai[bot]"
    and (.body | contains("No actionable comments"))))
    | sort_by(.created_at) | last | .created_at // empty')

# Get the latest CHANGES_REQUESTED or COMMENTED review timestamp
LAST_REVIEW_TS=$(gh api repos/{owner}/{repo}/pulls/{pr-number}/reviews --paginate \
  | jq -r -s '(add // []) | map(select(.user.login == "coderabbitai[bot]"
    and (.state == "CHANGES_REQUESTED" or .state == "COMMENTED")))
    | sort_by(.submitted_at) | last | .submitted_at // empty')

# Compare timestamps (lexicographic works for ISO 8601 dates)
if [[ -z "$NO_ACTION_TS" ]]; then
  echo "No 'no actionable comments' signal found"
elif [[ -z "$LAST_REVIEW_TS" || "$NO_ACTION_TS" > "$LAST_REVIEW_TS" ]]; then
  echo "Clean re-review detected — proceeding to step 6b"
else
  echo "Review feedback still pending"
fi

If the "no actionable comments" comment is newer than the last review with feedback, CodeRabbit is satisfied.

When detected, skip straight to step 6b (pre-merge branch update and resolve). Do not loop back to step 2 looking for feedback that does not exist.

2. Fetch review comments

Important: Inline review comments (the "tasks" users see in the GitHub UI) are NOT returned by gh pr view --json comments. You must use the API directly:

gh api repos/{owner}/{repo}/pulls/{pr-number}/comments

This returns an array of review comment objects with:

• path — file path
• line / start_line — line numbers
• body — comment body (markdown, may include severity, suggestions)
• html_url — link to view the conversation
• user.login — reviewer (e.g., "coderabbitai[bot]", "claude-code[bot]")

CodeRabbit comment format:

• Body starts with severity: _⚠️ Potential issue_ | _🟠 Major_, _🟡 Minor_, or other markers
• Includes suggested fixes in Suggested fix blocks
• May include committable suggestions between `` markers
• Important: CodeRabbit may leave nitpick/style comments alongside major issues—fetch ALL comments, not just the first few

Parse and categorize feedback by:

• Severity (🟠 Major, 🟡 Minor, nitpick/style)
• File and line number
• Reviewer (CodeRabbit, Claude, human)

Present ALL comments to the user sorted by severity (Major → Minor → Nitpick) so security/critical issues are addressed first.

2a. Correctly identify which comments belong to the latest review round

CRITICAL — this is the #1 cause of missed feedback. Do NOT filter comments by original_commit_id to find "new" comments. CodeRabbit posts comments against various commit SHAs depending on diff positioning, and a comment on commit A may actually be new feedback triggered by commit B's review.

The correct approach: use the review ID, not the commit ID.

1. First, fetch all reviews and find the latest CodeRabbit review:

``bash REVIEW_ID=$(gh api --paginate repos/{owner}/{repo}/pulls/{pr-number}/reviews \ | jq -s 'add | map(select(.user.login == "coderabbitai[bot]")) | sort_by(.submitted_at) | last | .id') echo "Latest CodeRabbit review: $REVIEW_ID" ``

1. Then, fetch comments and match them to reviews by pull_request_review_id:

``bash gh api --paginate repos/{owner}/{repo}/pulls/{pr-number}/comments \ | jq -s --argjson rid "$REVIEW_ID" \ 'add | map(select(.pull_request_review_id == $rid)) | map({id, path, line, body: .body[:200]})' ``

1. Comments from the latest review are the new comments for this round.

Comments from earlier reviews that were NOT auto-resolved are carried forward and still need action.

Why original_commit_id filtering is wrong:

• CodeRabbit may post a comment against commit X even when reviewing commit Y

(if the code at that location hasn't changed between commits)

• A single review round can produce comments with different original_commit_id

values

• Filtering by the latest commit SHA misses comments CodeRabbit posted against

older commit positions — this causes the agent to report "only 1 new comment" when there are actually 7

Alternative quick approach: If you don't want to join on review IDs, simply fetch ALL CodeRabbit comments, exclude those that have a human reply dismissing them (user said "out of scope"), and present the rest grouped by file. Let the user decide which are new vs. already handled.

2b. Check review bodies for duplicate and nitpick comments

Critical: CodeRabbit embeds additional feedback directly in review bodies — not as separate inline comment threads. These appear in collapsible sections titled "Duplicate comments" and "Nitpick comments" within the review. They do NOT create their own threads, so they will be missed if you only fetch inline comments.

Fetch review bodies from CodeRabbit:

gh api --paginate repos/{owner}/{repo}/pulls/{pr-number}/reviews \
  | jq -s 'add
    | map(select(.user.login == "coderabbitai[bot]"))
    | sort_by(.submitted_at)
    | map({id: .id, state: .state, body: .body, date: .submitted_at})'

Look for these sections in the review body:

• ♻️ Duplicate comments — Issues raised previously that CodeRabbit still

considers unresolved after re-review. These are NOT resolved just because the original thread was addressed — CodeRabbit is saying the fix was incomplete or the issue persists. Treat these as active feedback.

• 🧹 Nitpick comments — Low-severity suggestions that didn't warrant a

blocking review thread. Still present them to the user.

For each duplicate/nitpick comment found:

• Extract the file path, line numbers, severity, and description
• Include the suggested patch if present
• Add it to the feedback summary alongside inline comments
• Do not dismiss duplicate comments — they indicate CodeRabbit re-reviewed

and still found the issue. The fix from the previous round was likely incomplete.

When a review body contains duplicate comments but no new inline threads:

• The review state will be COMMENTED (not CHANGES_REQUESTED)
• The inline comment threads from the previous round may be auto-resolved
• But the duplicate section means there is still work to do — do NOT skip

to the merge step just because all threads show as resolved

3. Present feedback summary and classify into themes

Sort comments by severity: Major → Minor → Nitpick/Style, so critical issues (especially security) are addressed first.

Show the user the raw feedback list, then group comments into themes. A theme is a class of issue that may have multiple instances across the codebase. Examples:

| Raw comments | Theme | |---|---| | "Docs say circles but widget draws rectangles" × 3 files | Doc/code mismatch: shape description | | "No error check after fclose" in writer A | Unchecked I/O pattern | | "No error check after fclose" in writer B | (same theme) | | "Test only checks return value, not side effects" × 4 tests | Weak test assertions | | "Division by zero if rect width is 0" | Missing zero-guard pattern | | "fabsf() should be SDL_fabsf()" × 5 files | Bare C stdlib calls (portability) |

## PR Review Feedback Summary

### Pending conversations: X (grouped into Y themes)

**Theme 1: [description] (N comments, severity)**
- file.c:123 — [specific instance]
- file.c:456 — [specific instance]
- README.md:78 — [specific instance]

**Theme 2: [description] (N comments, severity)**
- ...

### Resolved conversations: Z

If no pending conversations, skip to step 6.

4. Build a verification plan (CRITICAL — do not skip)

This is the step that prevents 10+ round feedback loops. Before touching any code, build a comprehensive plan for each theme. The goal: fix every instance in one pass so CodeRabbit sees zero duplicates on re-review.

4a. For each theme, run an impact analysis

For each theme, before writing any fix, spawn an analysis agent (subagent_type: "Explore") to answer:

1. Where else does this pattern appear? Search all files changed in this

PR for the same class of issue — not just the line CodeRabbit flagged. Use grep/glob to find every instance.

Examples of what "same pattern" means:

• CodeRabbit says "docs say circles" → grep for "circle" in all .md,

.h, and .c files touched by this PR

• CodeRabbit says "unchecked fclose" → grep for every fclose call in

the file and check if any are unchecked

• CodeRabbit says "test only checks return value" → check ALL similar

tests for the same weakness, not just the one cited

• CodeRabbit says "division by zero if width is 0" → find ALL divisions

by width, height, or any user-derived value in the function

• CodeRabbit says "fabsf() should be SDL_fabsf()" → grep for ALL bare

C stdlib calls (fabsf, sinf, cosf, sqrtf, memset, memcpy, strcmp, strlen, malloc, free) across every .c and .h file in the PR — this is a critical cross-platform portability issue

1. What documentation describes this code? Identify every README,

API doc, header comment, and inline comment that references the behavior being changed. These MUST be updated when the code changes.

1. What tests cover this code? Identify existing tests. If the fix

changes behavior, those tests must be updated. If no test exists, one must be written.

1. Why did we get this wrong? Understanding the root cause prevents

adjacent bugs:

• Copy-paste without adaptation → check all copies
• Misunderstanding an API → check all uses of that API
• Missing a case in a switch/if chain → check the full chain
• Generated code from a template → check all generated instances

4b. Write the verification plan

For each theme, produce a checklist:

Theme: "Radio button shape — docs say circles, code draws rectangles"
Root cause: Copy-pasted checkbox description without adapting for radio buttons

Code fixes:
  [ ] common/ui/forge_ui.h:234 — change "circle" to "rectangle" in header doc
  [ ] common/ui/README.md:89 — update radio button description
  [ ] lessons/ui/15-dev-ui/README.md:156 — fix shape description
  [ ] OR: actually make radio buttons round in forge_ui_ctx_radio()

Doc sync:
  [ ] Verify all 3 doc locations match after fix
  [ ] Check if any diagram shows radio buttons (update if so)

Tests:
  [ ] Add test asserting radio button geometry is rectangular (or round)
  [ ] Verify existing radio button tests still pass

Grep verification:
  [ ] grep -r "circle" across all files in this PR — zero false matches remain

Present the full plan to the user before executing. The user may spot things the analysis missed or may prefer a different approach (e.g. "just make it round" vs "fix all the docs").

4c. Execute the plan with a team

For each theme that the user approves, spawn agents in parallel to handle the three concerns simultaneously:

1. Code agent (subagent_type: "coder", run_in_background: true) —

Fix the code issue across ALL instances found in the impact analysis. Not just the line CodeRabbit pointed at — every instance of the pattern.

1. Test agent (subagent_type: "tester", run_in_background: true) —

Write or update tests for the fix. For library changes in common/, this is mandatory. The test should:

• Verify the fix works (positive case)
• Verify edge cases (zero values, NULL, overflow)
• Verify the old bug does not regress

1. Doc agent (subagent_type: "coder", run_in_background: true) —

Update ALL documentation that describes the changed behavior:

…

Source & license

This open-source skill is cataloged on AgentStack and links to its original source — we do not rehost the code.

• Author: Nebulavenus
• Source: Nebulavenus/forge-gpu
• License: Zlib

Install and usage instructions live in the source repository linked above.