Code Review Developer (Smart Router)

Purpose

Context-aware routing to code review guidelines. Helps you conduct thorough, actionable code reviews following project standards.

When Auto-Activated

•Reviewing pull requests or code changes
•Keywords: code review, PR review, review code, pull request, approve, issues
•Discussing review comments or feedback

🚨 CRITICAL RULES (NEVER VIOLATE)

•Be LEAN and ACTIONABLE - Only report actual issues, no noise
•NO praise sections - No "Strengths", no "no concerns" statements
•NO design suggestions - You cannot see visual design (padding, margins, colors)
•Reference file:line - Always include specific locations for issues
•If clean, just approve - "✅ Approved - No issues found" (nothing else!)
•Check CLAUDE.md - Review against project conventions

📋 Quick Review Workflow

1. Read the Changes

•Understand what the PR is trying to do
•Check file diffs thoroughly

2. Check Against CLAUDE.md

•Localization: Using Loc constants?
•Generated files: Not editing generated code?
•Code style: Following Swift best practices?
•Tests: Updated when refactoring?

3. Look for Real Issues

ONLY include sections if issues exist:

•Bugs/Issues - Logic errors, potential bugs
•Best Practices - Violations of CLAUDE.md guidelines
•Performance - Actual performance problems
•Security - Real security vulnerabilities

4. Format Your Review

If clean:

code

✅ **Approved** - No issues found

CRITICAL: When approving, output ONLY the line above. NO additional explanation, NO listing what the PR does, NO praise. Just the approval line.

If issues found:

code

## Bugs/Issues

**ChatView.swift:45**
Potential race condition when...

---

⚠️ **Minor Issues** - Fix race condition

⚠️ Common Mistakes to Avoid

Assuming Code is Unused After UI Removal

Scenario: PR removes a menu button but leaves the menu parameter

❌ WRONG:

code

"The menu parameter is now unused and should be removed"

✅ CORRECT:

code

Check if menu is used elsewhere:
- Long-press context menu?
- Dual UX pattern (button + long-press)?
- Multiple consumers?

Example:

swift

// menu() is used in BOTH places
.toolbar { Menu { menu() } }     // Visible button (removed)
.contextMenu { menu() }          // Long-press (still there!)

Before suggesting removal:

• Searched ALL usages in the file
• Checked for dual UX patterns
• Understood purpose of each flag
• Asked about design intent if unsure

Not Understanding Conditional Flags

Scenario: Component has allowMenuContent and allowContextMenuItems

❌ WRONG:

code

"These flags serve the same purpose, consolidate them"

✅ CORRECT:

code

They control DIFFERENT UI elements:
- allowMenuContent: Visible button
- allowContextMenuItems: Long-press menu
- Can be independently enabled/disabled

Flagging Properly Regenerated Files

Scenario: A PR includes changes to a generated file (e.g., Generated/FeatureFlags.swift).

❌ WRONG:

code

"Edited generated file instead of running code generation"
(Assuming any change to a generated file is a violation)

✅ CORRECT: Check if the corresponding SOURCE file is also in the PR diff:

Generated File	Source File
`Generated/FeatureFlags.swift`	`FeatureDescription+Flags.swift`
`Generated/Strings.swift`	`.xcstrings` files
`Generated/ImageAssets.swift`	`Assets.xcassets` folders
`Modules/*/Generated/`	Templates or annotated source files

Proper Workflow Pattern:

code

PR contains:
├── FeatureDescription+Flags.swift (source - CHANGED)
└── Generated/FeatureFlags.swift   (generated - ALSO CHANGED)
→ This is CORRECT! Developer edited source and ran `make generate`

Actual Violation Pattern:

code

PR contains:
└── Generated/FeatureFlags.swift   (generated - CHANGED)
    (No corresponding source file changes)
→ This is WRONG! Developer manually edited generated file

Before flagging generated file edits:

• Check if corresponding source file is in the diff
• If source file changed → regeneration was proper, NOT a violation
• If ONLY generated file changed → flag as violation

🎯 Review Sections (Include ONLY If Issues Exist)

Bugs/Issues

Logic errors, potential bugs that need fixing

Format:

code

**FileName.swift:123**
Description of the bug and why it's a problem.

Best Practices

Violations of Swift/SwiftUI conventions or CLAUDE.md guidelines (code quality only, not design)

Format:

code

**FileName.swift:45**
Using hardcoded strings instead of Loc constants.

Performance

Actual performance problems (not theoretical)

Format:

code

**ViewModel.swift:89**
N+1 query in loop - will cause performance issues with large datasets.

Security

Real security vulnerabilities

Format:

code

**AuthService.swift:34**
Storing credentials in UserDefaults - should use Keychain.

📊 Summary Format

End with ONE sentence with status emoji:

code

✅ **Approved** - Clean implementation following guidelines
⚠️ **Minor Issues** - Fix hardcoded strings and race condition
🚨 **Major Issues** - Critical security vulnerability in auth flow

🔍 Analysis Checklist

Before finalizing your review:

📚 Complete Documentation

Full Guide: .claude/CODE_REVIEW_GUIDE.md

For comprehensive coverage of:

•Core review rules
•Common analysis mistakes (with examples)
•Review sections and formats
•Complete checklist

CI/Automation: .github/workflows/pr-review-automation.md

For GitHub Actions integration:

•Context variables (REPO, PR_NUMBER, COMMIT_SHA)
•Valid runners and Xcode versions
•Review comment strategies
•How to post reviews via gh CLI

💡 Quick Reference

What to Check

From CLAUDE.md:

• No hardcoded strings (use Loc constants)
• No generated file edits (// Generated using...)
• Tests/mocks updated when refactoring
• Feature flags for new features
• No whitespace trimming
• Async/await over completion handlers

Code Quality:

• Swift best practices (guard, @MainActor)
• Proper error handling
• No force unwraps in production code
• Memory leaks (weak/unowned where needed)

What NOT to Comment On

•❌ Design/UI spacing (padding, margins)
•❌ Colors or visual appearance
•❌ Praise or "Strengths" sections
•❌ "No concerns" statements
•❌ Theoretical performance issues
•❌ Style preferences not in CLAUDE.md

🎓 Example Reviews

Example 1: Clean PR

code

✅ **Approved** - No issues found

That's it! Absolutely nothing else. Not even in comments posted to GitHub.

❌ WRONG (too verbose):

code

✅ **Approved** - No issues found

The PR correctly implements per-chat notification overrides:
- Added force list properties with proper subscription keys
- effectiveNotificationMode(for:) method correctly prioritizes...

✅ CORRECT:

code

✅ **Approved** - No issues found

Example 2: Minor Issues

code

## Best Practices

**ChatView.swift:34**
Using hardcoded string "Send Message" instead of localization constant.
Should be: `Text(Loc.sendMessage)`

**ChatViewModel.swift:89**
Tests not updated after renaming `sendMessage()` to `send()`.
Update `ChatViewModelTests.swift` to use new method name.

---

⚠️ **Minor Issues** - Fix hardcoded string and update tests

Example 3: Critical Issue

code

## Bugs/Issues

**AuthService.swift:45**
Storing password in UserDefaults (line 45). This is a security vulnerability.
Should use Keychain instead: `KeychainService.store(password, for: key)`

---

🚨 **Major Issues** - Fix password storage security vulnerability

🔗 Related Skills & Docs

•ios-dev-guidelines → IOS_DEVELOPMENT_GUIDE.md - Swift/iOS patterns to check against
•localization-developer → LOCALIZATION_GUIDE.md - Verify no hardcoded strings
•code-generation-developer → CODE_GENERATION_GUIDE.md - Verify no generated file edits

Navigation: This is a smart router. For detailed review standards and common mistakes, always refer to .claude/CODE_REVIEW_GUIDE.md.

For CI/automation: See .github/workflows/pr-review-automation.md for GitHub Actions integration.