Tutorial Overview

Series index: Superpowers Tutorial Series

Superpowers includes two code review skills: requesting-code-review and receiving-code-review. Used together, they make the review process both technically rigorous and efficient.

What you will learn

• ✅ When requesting-code-review triggers and how it works
• ✅ How to handle feedback with receiving-code-review
• ✅ How to classify review severity
• ✅ Why technical rigor matters more than performative agreement
• ✅ Real-world review case studies

Why Is Code Review So Important?

Without review

flowchart LR
    A[Finish code] --> B[Commit directly]
    B --> C[Problem goes unnoticed]
    C --> D[Production bug]
    D --> E[Urgent fix]
    E --> F[New problem introduced]
    
    style B fill:#ffcccc
    style C fill:#ffcccc
    style D fill:#ffcccc

With review

flowchart TD
    A[Finish implementation] --> B[requesting-code-review]
    B --> C[Review issue list]
    C --> D{Critical issue?}
    D -->|Yes| E[Block the commit and fix it]
    D -->|No| F[Record technical debt]
    E --> G[receiving-code-review]
    G --> H[Apply the fix]
    H --> I[Review again]
    I --> J[Commit approved]
    
    style B fill:#e1f5ff
    style E fill:#ffebee
    style G fill:#e8f5e9
    style J fill:#e8f5e9

Review impact

requesting-code-review Skill

When it triggers

requesting-code-review triggers automatically in these situations:

1. When a task is finished - after completing the work in a plan
2. When a feature is finished - after the main feature is implemented
3. Before merging - before merging into the main branch

Review request flow

sequenceDiagram
    participant D as Developer (AI)
    participant R as requesting-code-review
    participant C as Reviewer (AI/Human)
    
    D->>R: Task done, request review
    R->>R: Prepare review checklist
    R->>C: Send review request + context
    
    Note over C: Review items:
    Note over C: - Does it follow the design spec?
    Note over C: - Is the code quality acceptable?
    Note over C: - Is test coverage sufficient?
    Note over C: - Are there hidden issues?
    
    C->>R: Return review feedback
    R->>R: Classify by severity
    R->>D: Return review results
    
    alt Critical issues exist
        D->>D: Fix the issues
        D->>R: Request review again
    else No critical issues
        D->>D: Move on to the next task
    end

Review checklist

requesting-code-review prepares this checklist automatically:

## Code Review Request

**Task**: Task 5 - Implement the tag API endpoint

**Design doc**: `.project/designs/feature-tag-system.md`

**Changed files**:
- `app/Controllers/TagController.php` (new, 156 lines)
- `routes/api.php` (modified, +8 lines)
- `tests/Feature/TagApiTest.php` (new, 234 lines)

**Self-checks completed**:
- [x] Unit tests passed
- [x] Integration tests passed
- [x] Manual tests passed
- [x] Lint checks passed

**Review focus**:
1. Does the API design follow REST conventions?
2. Is error handling complete?
3. Is input validation sufficient?

**Potential issues**:
- Pagination may need optimization
- Error response format needs to be standardized

Severity levels

Review feedback is classified into four severity levels:

🔴 Critical

Definition: Must be fixed; blocks the commit

Examples:

🔴 Critical:
- Security vulnerabilities (SQL injection, XSS)
- Implementation does not match the spec
- Critical logic errors
- Risk of data corruption

Fix requirement: must be fixed immediately; blocks progress

🟠 Major

Definition: Should be fixed, but can be discussed

Examples:

🟠 Major:
- Duplicate code
- Missing error handling
- Performance issues
- Insufficient test coverage

Fix requirement: recommended; if not fixed, record technical debt

🟡 Minor

Definition: Can be fixed, but does not affect functionality

Examples:

🟡 Minor:
- Unclear naming
- Missing comments
- Formatting issues
- Small optimization suggestions

Fix requirement: fix when time allows

📝 Note

Definition: Informational only; no action required

Examples:

📝 Note:
- Future improvement ideas
- Alternative implementation suggestions
- Documentation improvements

Fix requirement: no action required

Review report example

## Code Review Report

**Review task**: Task 5 - Tag API endpoint
**Reviewer**: AI Reviewer
**Review time**: 2026-02-28 15:30

---

### 🔴 Critical (1)

1. **SQL injection risk**
   
   **Location**: `TagController.php:45`
   
   **Issue**:
   ```php
   // ❌ Dangerous: direct SQL concatenation
   $query = "SELECT * FROM tags WHERE name = '$name'";
   ```
   
   **Suggestion**:
   ```php
   // ✅ Use parameterized queries
   $query = "SELECT * FROM tags WHERE name = ?";
   $stmt->execute([$name]);
   ```
   
   **Blocks commit**: Yes

---

### 🟠 Major (2)

1. **Missing input validation**
   
   **Location**: `TagController.php:32`
   
   **Issue**: Tag name length is not validated
   
   **Suggestion**: Add length validation
   
   **Blocks commit**: No

2. **Insufficient test coverage**
   
   **Location**: `tests/Feature/TagApiTest.php`
   
   **Issue**: Missing error scenario tests
   
   **Suggestion**: Add 400/500 error tests
   
   **Blocks commit**: No

---

### 🟡 Minor (1)

1. **Unclear variable naming**
   
   **Location**: `TagController.php:78`
   
   **Issue**: `$t` is not descriptive
   
   **Suggestion**: Rename to `$tag`
   
   **Blocks commit**: No

---

### 📝 Note (1)

1. **Future improvement**: Consider adding tag search

---

## Review Summary

- Critical: 1 (must fix)
- Major: 2 (should fix)
- Minor: 1 (optional)
- Note: 1 (informational)

**Review result**: ❌ Not approved (critical issue exists)

**Next step**: Fix the Critical issue and resubmit for review

receiving-code-review Skill

When it triggers

receiving-code-review triggers automatically in these situations:

1. After review feedback arrives
2. Before implementing suggested changes
3. When the feedback is unclear

Core principles

receiving-code-review follows these principles:

receiving_code_review:
  principles:
    - "Technical rigor over performative agreement"
    - "Validate instead of blindly applying changes"
    - "Question unreasonable suggestions"
    - "Seek technical justification"
  
  anti_patterns:
    - "Blindly agreeing to everything"
    - "Implementing without validation"
    - "Performative thanks with no action"
    - "Ignoring technically invalid feedback"

Feedback handling flow

flowchart TD
    A[Receive review feedback] --> B[receiving-code-review]
    B --> C[Analyze each issue]
    C --> D{Is the issue reasonable?}
    D -->|Yes| E[Implement the fix]
    D -->|No| F[Question the suggestion]
    F --> G{Reviewer responds}
    G --> H[Technical explanation]
    H --> E
    G --> I[Explanation remains unconvincing]
    I --> J[Record the disagreement]
    E --> K[Resubmit for review]
    
    style B fill:#e1f5ff
    style E fill:#e8f5e9
    style J fill:#fff3e0

Response strategies

Responding to reasonable feedback

**Review comment**: SQL injection risk

**Response**:
✅ Issue is valid, fix it immediately

**Fix plan**:
1. Replace all direct SQL string concatenation with parameterized queries
2. Add SQL injection protection tests
3. Use an ORM or query builder if appropriate

**After fixing**:
- Run security tests
- Resubmit for review

Responding to unreasonable feedback

**Review comment**: Change all functions to single-line expressions

**Response**:
⚠️ Suggestion is debatable

**Technical reasons**:
1. Single-line code would reduce readability
2. The current implementation has clear logical separation
3. There is no performance issue that requires this change

**Recommendation**:
- Keep the current implementation
- Record it as "reviewed, not adopted"
- Provide benchmarks if performance is a concern

**Awaiting response**: Yes

Responding to unclear feedback

**Review comment**: "This design has a problem"

**Response**:
❓ Need clarification

**Clarification request**:
1. Which part of the design is problematic?
2. What consequence does the issue cause?
3. Do you have a concrete improvement suggestion?

**Wait**: Do not act until the feedback is clarified

Best practices for applying fixes

1. Handle one issue at a time

# ❌ Bad: change everything at once
git commit -am "Fix all review comments"

# ✅ Good: one commit per issue
git commit -m "fix: address SQL injection vulnerability (review #1)"
git commit -m "feat: add input validation (review #2)"
git commit -m "refactor: improve variable naming (review #3)"

2. Preserve review context

Commit message format:
```
fix: [issue type] [review reference]

[Problem description]
[Fix plan]
[Verification method]

Review: #123
```

Example:
```
fix: security SQL injection vulnerability

- Use parameterized queries instead of string concatenation
- Add a prepared statement for tag name lookup
- Add an SQL injection test case

Review: #123, Critical #1
```

3. Verify the fix

# Run tests after the fix
pytest tests/ -v --tb=short

# Run review again
# requesting-code-review triggers automatically

# Confirm the issue is resolved
# Reviewer confirms

Practical Case Studies

Case 1: Handling a reasonable issue

Scenario: The API endpoint is missing error handling

Review comment:

🟠 Major: Missing error handling

Location: TagController.php:56

Issue: There is no error handling when the database query fails

Suggestion:
```php
try {
    $tags = $this->tagRepository->getAll();
    return response()->json($tags);
} catch (Exception $e) {
    return response()->json(['error' => 'Failed to fetch tags'], 500);
}
```

Response:

✅ Accept the suggestion

Fix:
- Add a try-catch block
- Log the error
- Return a friendly error message

Verification:
- Add a database failure test
- Manually test the error scenario

Case 2: Handling an unreasonable issue

Scenario: The reviewer asks to delete all comments

Review comment:

🟡 Minor: Too many comments

Issue: The code should be self-explanatory and does not need so many comments

Suggestion: Remove all comments

Response:

⚠️ Suggestion is debatable

Technical reasons:
1. Complex business logic needs comments to explain the "why"
2. Public APIs need documentation comments
3. Team standards require JSDoc

Current comment categories:
- Documentation comments (required): keep
- Business logic explanations (required): keep
- Redundant comments (for example `i++ // increment i`): remove

Decision: Keep the necessary comments, remove redundant ones

Case 3: Clarifying an ambiguous issue

Review comment:

🟠 Major: This implementation is not good enough

Suggestion: Refactor

Response:

❓ Request clarification

Specific questions:
1. What does "not good enough" mean here?
2. What is the actual problem (performance/readability/maintainability)?
3. What refactoring direction do you expect?

Current implementation strengths:
- Full test coverage
- Matches the design spec
- Meets performance benchmarks

Wait for more specific review feedback before acting

Technical Rigor vs Performative Agreement

Anti-pattern: performative agreement

# ❌ Bad: performative agreement

"Thank you for the valuable feedback! You are absolutely right!
I will immediately implement your suggestions!
This is extremely helpful!"

Then:
- Do not validate whether the suggestion is reasonable
- Blindly apply potentially broken feedback
- Avoid technical discussion

Good: technical rigor

# ✅ Good: technical rigor

"Thanks for the review. About issue #1:

My analysis:
- This issue does create an SQL injection risk
- I tested it with malicious input ' OR '1'='1
- I confirmed the bypass works

Fix plan:
- Use parameterized queries (see the code above)
- Add SQL injection test cases
- Consider using an ORM to avoid this class of issue entirely

About issue #2:
- I think this suggestion is debatable
- The reasons are...
- I recommend keeping the current implementation

Please confirm whether my understanding is correct."

Comparison

Review Automation

Automatic review rules

code_review:
  auto_check:
    # Code style
    - lint_passed
    - format_correct
    
    # Tests
    - tests_passed
    - coverage_minimum: 85%
    
    # Security
    - no_sql_injection
    - no_xss_vulnerability
    - no_hardcoded_secrets
    
    # Quality
    - no_duplicate_code
    - function_length_max: 50
    - cyclomatic_complexity_max: 10

Review bot integration

code_review:
  integrations:
    - github_pr_review
    - gitlab_mr_review
    - phabricator
    - gerrit
  
  notifications:
    - slack
    - email
    - teams

Summary

Code review skills ensure technical rigor and high-quality code:

1. requesting-code-review - proactively ask for feedback
2. receiving-code-review - handle feedback rigorously
3. Issue classification - Critical/Major/Minor/Note
4. Verification first - do not agree blindly or apply changes blindly

Key points

• ✅ Ask for review proactively; do not avoid it
• ✅ Stay technically rigorous; do not performatively agree
• ✅ Handle issues by severity; Critical must be fixed
• ✅ Verify whether review feedback is actually valid

Review quotes

“Code review is not about nitpicking; it is a collective effort to improve code quality.”

“Performative agreement is a breeding ground for technical debt.”

Series navigation:

• ← Previous: Tutorial 6: using-git-worktrees - Isolated Development Workflow
• → Next: Tutorial 8: subagent-driven-development and Parallel Tasks
• Back: Series Index

References

• Google Code Review Guide
• Effective Code Review
• Code Review Best Practices