PR Reviewer

<mission_control> Review pull requests with systematic analysis before generating feedback <success_criteria>All findings categorized by severity with specific recommendations</success_criteria> <standards_gate> MANDATORY: Load pr-reviewer references BEFORE starting review:

• Security patterns → references/security.md
• Quality criteria → references/quality.md
• Performance guidelines → references/performance.md </standards_gate> </mission_control>

Review pull request changes with comprehensive analysis including security, performance, code quality, and architecture.

<interaction_schema> inject_context → thinking_analysis → stage1_spec_compliance → stage2_quality_review → categorize_findings → output </interaction_schema>

Thinking Protocol: Analyze Before Speaking

<thinking_protocol> <mandatory_trigger> Before generating ANY PR review feedback </mandatory_trigger>

Thinking Template

<thinking>
<diff_analysis>
- Files changed: [count and list]
- Lines added/removed: [approximate]
- Scope: [feature/bug/refactor/other]
- Risk level: [high/medium/low]
</diff_analysis>

<requirement_check>
- PR description states: [requirements]
- Implementation provides: [what was actually done]
- Gaps identified: [missing features, extra features]
- Spec compliance: [COMPLIANT/NON_COMPLIANT]
</requirement_check>

<issue_scan>
<security>
- Injection vulnerabilities: [findings]
- Auth/authz issues: [findings]
- Secrets exposure: [findings]
- Input validation: [findings]
</security>

<performance>
- N+1 queries: [findings]
- Algorithmic issues: [findings]
- Missing indexes: [findings]
- Caching opportunities: [findings]
</performance>

<quality>
- Code duplication: [findings]
- Naming issues: [findings]
- Test coverage: [findings]
- Complexity issues: [findings]
</quality>

<architecture>
- Layer violations: [findings]
- Dependency issues: [findings]
- Error handling: [findings]
- API design: [findings]
</architecture>
</issue_scan>

<severity_assessment>
- Blockers: [must-fix before merge]
- Important: [should-fix before merge]
- Nits: [nice-to-have improvements]
</severity_assessment>
</thinking>

This skill uses dynamic context injection with !command"` syntax to gather live PR data before review:

## Current Context

- **PR diff**: !`git diff HEAD~1`
- **PR title**: !`git log -1 --pretty=format:%s`
- **PR description**: !`git log -1 --pretty=format:%b`
- **Changed files**: !`git diff --name-only HEAD~1`
- **Commits**: !`git log --oneline HEAD~1..HEAD`

Two-Stage Review Architecture

MANDATORY: PR review must follow two-stage process:

1. Stage 1: Spec Compliance Review - Verify implementation matches requirements
2. Stage 2: Code Quality Review - Assess security, performance, quality, architecture

NEVER skip stages or review out of order.

Stage 1: Spec Compliance Review

First, verify implementation matches requirements:

Spec Compliance Checklist

• All requirements from PR description implemented
• Nothing extra added (YAGNI violations)
• Acceptance criteria met
• Edge cases addressed
• No missing functionality

Spec Compliance Output

## Stage 1: Spec Compliance Review

### Requirements Verification

✅ All required features implemented
✅ No extra features (YAGNI compliant)
✅ Acceptance criteria met

### Gap Analysis

- Missing: [List any gaps]
- Extra: [List any over-implementation]
- Ambiguous: [List unclear requirements]

**Result**: COMPLIANT | NON_COMPLIANT

Stage 2: Code Quality Review

Only after Stage 1 passes:

Execute comprehensive review across four dimensions:

1. Security Review

• Check for injection vulnerabilities (SQL, XSS, command injection)
• Verify authentication/authorization implementation
• Look for secrets exposure in code
• Validate input sanitization
• Check for OWASP Top 10 vulnerabilities

2. Performance Review

• Identify N+1 query problems
• Check for inefficient algorithms
• Look for missing database indexes
• Validate caching strategies
• Review API response times

3. Code Quality Review

• Review naming conventions
• Check for code duplication
• Verify test coverage
• Assess maintainability
• Check for code complexity issues

4. Architecture Review

• Check layer separation (MVC, clean architecture)
• Verify dependency injection
• Assess error handling patterns
• Review API design
• Check for proper abstractions

Output Format

<finding_categorization> Force findings into XML buckets for clear categorization and severity assessment

<security_issues> [What's wrong] <file_path>[path/to/file]</file_path> <line_number>[number]</line_number> [Specific fix with code example if applicable] <cve_reference>[If applicable CVE or OWASP reference]</cve_reference> </security_issues>

<performance_issues> [What's wrong] <file_path>[path/to/file]</file_path> <line_number>[number]</line_number> [Specific fix with optimization strategy] [Performance impact: +Xms, -Y queries/sec] </performance_issues>

<code_quality_issues> [duplication | naming | complexity | coverage | style] [What's wrong] <file_path>[path/to/file]</file_path> <line_number>[number]</line_number> [Specific improvement] </code_quality_issues>

<architecture_issues> [layer_violation | dependency | error_handling | api_design] [What's wrong] <file_path>[path/to/file]</file_path> <line_number>[number]</line_number> [Specific architectural improvement] </architecture_issues> </finding_categorization>

Markdown Output Template

# PR Review: [PR Title]

## Stage 1: Spec Compliance

**Status**: ✅ PASS / ❌ FAIL

### Requirements Verification

[Summary of spec compliance check]

### Gap Analysis

- Missing: [list]
- Extra: [list]
- Ambiguous: [list]

---

## Stage 2: Code Quality Review

### Security Issues

<security_issues count="0">

<!-- No security issues found -->

</security_issues>

### Performance Issues

<performance_issues count="0">

<!-- No performance issues found -->

</performance_issues>

### Code Quality Issues

<code_quality_issues count="2">
<issue severity="NIT">
<type>naming</type>
<description>Variable name 'x' not descriptive</description>
<file_path>src/auth.py</file_path>
<line_number>42</line_number>
<recommendation>Use descriptive name like 'user_id'</recommendation>
</issue>
</code_quality_issues>

### Architecture Issues

<architecture_issues count="0">

<!-- No architecture issues found -->

</architecture_issues>

---

## Overall Assessment

**Result**: APPROVE | REQUEST_CHANGES

**Summary**: [1-2 sentence summary]

**Blockers**: [count] must be fixed before merge
**Important**: [count] should be fixed before merge
**Nits**: [count] optional improvements

Review Loop Enforcement

CRITICAL: If issues found, fixes must be verified before approval.

Review Loop Protocol

1. Reviewer finds issues → Report all issues clearly
2. Developer fixes issues → Resubmit for review
3. Reviewer re-reviews → Verify fixes actually work
4. Repeat until approved → No shortcuts, no exceptions

NEVER approve with open issues.

Review Loop Template

## Review Results

### Stage 1: Spec Compliance

**Status**: PASS | FAIL
[If FAIL: List specific gaps]

### Stage 2: Code Quality

**Status**: PASS | FAIL

#### Issues Found:

1. **[Severity]** - [Issue]
   - File: [path]
   - Line: [number]
   - Fix: [specific recommendation]

#### Required Changes:

- [ ] Fix issue 1
- [ ] Fix issue 2
- [ ] Re-review after fixes

**Review will continue until all issues resolved.**

Review Checklist

Security

• No SQL injection vulnerabilities
• XSS protection implemented
• Authentication properly checked
• Authorization enforced
• No secrets in code
• Input validation present
• Rate limiting configured

Performance

• No N+1 queries
• Efficient algorithms used
• Proper indexing strategy
• Caching implemented where appropriate
• API responses optimized

Code Quality

• Consistent naming conventions
• No code duplication
• Adequate test coverage
• Clear, readable code
• Proper error handling
• No commented-out code

Architecture

• Proper layer separation
• Dependencies properly injected
• Errors handled appropriately
• API follows REST conventions
• Appropriate design patterns used

Integration

This skill integrates with:

• security - Security vulnerability detection
• coding-standards - Code quality standards
• backend-patterns - Architecture best practices
• engineering-lifecycle - Testing requirements

Genetic Code

This component carries essential Seed System principles for context: fork isolation:

<critical_constraint> MANDATORY: All components MUST be self-contained (zero .claude/rules dependency) MANDATORY: Achieve 80-95% autonomy (0-5 AskUserQuestion rounds per session) MANDATORY: Description MUST use What-When-Not format in third person MANDATORY: No component references another component by name in description MANDATORY: Progressive disclosure - references/ for detailed content MANDATORY: Use XML for control (mission_control, critical_constraint), Markdown for data No exceptions. Portability invariant must be maintained. </critical_constraint>

Delta Standard: Good Component = Expert Knowledge − What Claude Already Knows

Recognition Questions:

• "Would Claude know this without being told?" → Delete (zero delta)
• "Can this work standalone?" → Fix if no (non-self-sufficient)
• "Did I read the actual file, or just see it in grep?" → Verify before claiming

<critical_constraint> MANDATORY: Complete <thinking> analysis BEFORE generating review output

• Analyze diff, requirements, and impact
• Check security, performance, quality, architecture dimensions
• Weigh severity before generating output
• NEVER skip thinking phase, even for "simple" PRs

MANDATORY: Categorize findings into XML buckets

• Security issues → <security_issues>
• Performance issues → <performance_issues>
• Code quality issues → <code_quality_issues>
• Architecture issues → <architecture_issues>
• Each finding must have severity, description, location, recommendation

MANDATORY: Two-stage review process

• Stage 1: Spec compliance (requirements met?)
• Stage 2: Code quality (security, performance, quality, architecture)
• NEVER approve if Stage 1 fails (requirements not met)
• NEVER approve with BLOCKER or HIGH severity issues open

MANDATORY: Verify fixes before approval

• If issues found, developer must fix
• Re-review to verify fixes actually work
• Repeat until all issues resolved
• NEVER approve with open issues

MANDATORY: Never LGTM without analysis

• "Looks good to me" is forbidden without <thinking> analysis
• Every PR deserves thorough review
• Small PRs can have big bugs
• No short-cuts, no exceptions

No exceptions. PR review is a gate, not a formality. </critical_constraint>