shield_lock
JinDaGe
← Back to skills
extension
Category: Development & EngineeringNo API key required

pr-reviewer

Review pull requests for spec compliance, security, performance, and code quality. Use when analyzing PRs. Not for writing new code or initial development.

personAuthor: jakexiaohubgithub
open_in_newRepositorydownloadDownload package

PR Reviewer

<mission_control> <objective>Review pull requests with systematic analysis before generating feedback</objective> <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>

<process> 1. **Open `<thinking>`** - Analysis sandbox 2. **Analyze the diff** - What changed, why, impact 3. **Check requirements** - Does implementation match spec? 4. **Identify issues** - Security, performance, quality, architecture 5. **Weigh severity** - Blocker vs nitpick 6. **Close `</thinking>`** - Hard stop before output 7. **Generate review** - Categorized findings with recommendations </process> </thinking_protocol>

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

<router> flowchart TD Start([Start Review]) --> Stage1{Stage 1:\nSpec Compliance} Stage1 -- PASS --> Stage2{Stage 2:\nCode Quality} Stage1 -- FAIL --> Return[Return to User\n(Fix Spec)] Stage2 -- PASS --> Approve[Approve PR] Stage2 -- FAIL --> RequestChanges[Request Changes\n(Fix Code)] RequestChanges --> Return </router>

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> <purpose>Force findings into XML buckets for clear categorization and severity assessment</purpose>

<security_issues> <issue severity="BLOCKER | HIGH | MEDIUM | LOW"> <description>[What's wrong]</description> <file_path>[path/to/file]</file_path> <line_number>[number]</line_number> <recommendation>[Specific fix with code example if applicable]</recommendation> <cve_reference>[If applicable CVE or OWASP reference]</cve_reference> </issue> </security_issues>

<performance_issues> <issue severity="BLOCKER | HIGH | MEDIUM | LOW"> <description>[What's wrong]</description> <file_path>[path/to/file]</file_path> <line_number>[number]</line_number> <recommendation>[Specific fix with optimization strategy]</recommendation> <impact>[Performance impact: +Xms, -Y queries/sec]</impact> </issue> </performance_issues>

<code_quality_issues> <issue severity="NIT | LOW | MEDIUM"> <type>[duplication | naming | complexity | coverage | style]</type> <description>[What's wrong]</description> <file_path>[path/to/file]</file_path> <line_number>[number]</line_number> <recommendation>[Specific improvement]</recommendation> </issue> </code_quality_issues>

<architecture_issues> <issue severity="BLOCKER | HIGH | MEDIUM"> <type>[layer_violation | dependency | error_handling | api_design]</type> <description>[What's wrong]</description> <file_path>[path/to/file]</file_path> <line_number>[number]</line_number> <recommendation>[Specific architectural improvement]</recommendation> </issue> </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>