Pr Reviewer

PR Reviewer

• Security patterns → references/security.md
• Quality criteria → references/quality.md
• Performance guidelines → references/performance.md

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

Thinking Protocol: Analyze Before Speaking

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

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:

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

• 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.