Providing Code Review

Wrong skill? File-protocol review → code-review-respond. Pre-commit review → progressive-code-review-gate. Processing feedback you received → receiving-code-review.

Source: superpowers-plus Part of: Engineering Rigor skill family

Invocation

If an argument was supplied with the trigger (PR URL, diff, branch name, or inline code), use it immediately — do not ask what to review. Examples:

• /sp-review https://github.com/org/repo/pull/123 → fetch the PR diff and begin review
• /sp-review #123 → treat as a short PR reference in the current repo; run gh pr diff 123
• /sp-review <pasted diff or code> → review the supplied content directly
• /sp-review (no argument) → ask: "What do you want me to review? (PR URL, diff, or branch name)"

Parse the argument from whatever the user typed after the trigger keyword. Do not dump the skill body and wait — act.

The Rule

The same engineering rigor that applies to your own work applies when reviewing others' work.

When reviewing a PR, check changes, or provide feedback on someone else's implementation, apply the same analytical rigor — not rubber-stamp approval.

When to Use

• Reviewing a PR, merge request, or diff someone else authored
• Asked to "check," "review," or "approve" code changes
• Pair-reviewing changes before merge in a shared branch workflow
• Evaluating a dependency upgrade or third-party contribution

Why This Gate Exists

Failure Pattern: Reviewing PRs superficially because "it's their code" — looking at the diff in isolation without:

• Tracing the full data flow
• Checking blast radius of changes
• Verifying integration points
• Running the actual changes locally

This allows the same bugs to slip through that disciplined pre-commit checks prevent in your own work.

Code Review Checklist

BEFORE approving or providing feedback:

1. Data Flow Analysis

For each changed function/component:
  WHERE does the input come from?
  WHERE does the output go?
  WHAT transforms happen in between?

If the PR adds a new field or parameter:

• Does it flow through ALL intermediate components?
• Are there "silent pass-through" files that need updating?
• Does the field reach its final destination?

2. Blast Radius Verification

# Search for OTHER usages of modified functions
grep -rn "functionName" --include="*.ts" .

# Check if changes break existing callers
# Are ALL callers updated?
# Are tests updated for changed signatures?

# 🔴 INBOUND REFERENCE SCAN (mandatory for renames/moves/deletes)
# When files are renamed, moved, or deleted — scan the ENTIRE repo,
# not just the changed directory. Other modules that reference old
# paths will silently break.
git diff --diff-filter=RD --name-only main..HEAD | while read old; do
  grep -rn "$(basename "$old")" . --include="*.md" --include="*.ts" \
    --include="*.sh" --include="*.json" | grep -v "$(dirname "$old")"
done

Questions to answer:

• How many places call this code?
• Did the PR update ALL of them?
• Are there cross-repo consumers not in this diff?
• If files were renamed/moved/deleted: Did the author grep the ENTIRE repo for references to the old paths — not just the directory they refactored?

3. Integration Point Check

4. CI/Build Verification

• Is CI actually passing? (Not just "mergeable" — actually green)
• Are there test failures being ignored?
• Did lint pass?

5. Factual Claims Verification

If the PR or associated documentation makes claims about external system state, verify each claim against the system of record. Do not treat metadata as stylistic — it is falsifiable.

PR platform gotcha: Some PR platforms generate a preview merge commit for every open PR. This does NOT mean the PR is merged. Always check the PR status field via the API before concluding a merge occurred.

The Review Gate Function

BEFORE approving any PR:

1. DATA FLOW: Did I trace where new fields/params flow FROM and TO?
2. BLAST RADIUS: Did I check for usages OUTSIDE the diff?
3. INTEGRATION: Did I verify the changes work at service boundaries?
4. TESTS: Are there tests for the new functionality?
5. BUILD: Is CI passing? (Not just "mergeable" — actually green)
6. FACTS: Did I verify every claim about external state (PR status, deploy status, ticket state, URLs)?

If I can't answer YES to all → don't approve, ask questions or flag gaps

Red Flags to Watch For

Output Format for Code Review

When providing code review, structure feedback as:

## Code Review: [PR Title]

### ✅ What's Good
- [Specific strength]

### ⚠️ Questions / Concerns
- [Question about data flow]
- [Missing integration check]

### 🚫 Must Fix Before Merge
- [Blocker issue]

### 📊 Engineering Rigor Check
- [ ] Data flow traced end-to-end
- [ ] Blast radius verified (checked for usages outside diff)
- [ ] Integration points verified
- [ ] CI passing (actual green, not just mergeable)
- [ ] Tests cover new functionality

If you can't check off all boxes, the review is incomplete.

Anti-Patterns

Failure Modes

Companion Skills

• code-review-battery: Parallel specialist reviews (heavier than this checklist)
• receiving-code-review: How the PR author should process your feedback
• progressive-code-review-gate: Pre-commit gate (uses this checklist internally)
• inter-agent-review-protocol: File-protocol review (requesting side)
• code-review-respond: File-protocol review (reviewer side)
• micro-harsh-review: Per-batch review (lighter)