| name | code-review-developer |
| description | Context-aware routing to code review guidelines. Use when reviewing pull requests, providing code feedback, or discussing review standards. |
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:
✅ **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:
## 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:
"The menu parameter is now unused and should be removed"
✅ CORRECT:
Check if menu is used elsewhere:
- Long-press context menu?
- Dual UX pattern (button + long-press)?
- Multiple consumers?
Example:
.toolbar { Menu { menu() } }
.contextMenu { menu() }
Before suggesting removal:
Not Understanding Conditional Flags
Scenario: Component has allowMenuContent and allowContextMenuItems
❌ WRONG:
"These flags serve the same purpose, consolidate them"
✅ CORRECT:
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:
"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:
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:
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:
🎯 Review Sections (Include ONLY If Issues Exist)
Bugs/Issues
Logic errors, potential bugs that need fixing
Format:
**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:
**FileName.swift:45**
Using hardcoded strings instead of Loc constants.
Performance
Actual performance problems (not theoretical)
Format:
**ViewModel.swift:89**
N+1 query in loop - will cause performance issues with large datasets.
Security
Real security vulnerabilities
Format:
**AuthService.swift:34**
Storing credentials in UserDefaults - should use Keychain.
📊 Summary Format
End with ONE sentence with status emoji:
✅ **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:
Code Quality:
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
✅ **Approved** - No issues found
That's it! Absolutely nothing else. Not even in comments posted to GitHub.
❌ WRONG (too verbose):
✅ **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:
✅ **Approved** - No issues found
Example 2: Minor Issues
## 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
## 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.