Pr Review

---
description: 'Systematic PR review for code quality assurance. Trigger with "review this PR", "check this pull request", "code review", "review please".'
allowed-tools:
  - Bash(gh *)
  - Read
  - Grep
  - Glob
---

# Systematic PR review for code quality assurance

Ensure code quality and architectural soundness through systematic Pull Request reviews.

## Usage

```bash
# Comprehensive PR review
gh pr view 123 --comments
"Systematically review this PR and provide feedback from code quality, security, and architecture perspectives"

# Security-focused review
gh pr diff 123
"Focus on reviewing security risks and vulnerabilities"

# Architecture perspective review
gh pr checkout 123 && find . -name "*.js" | head -10
"Evaluate the architecture from the perspectives of layer separation, dependencies, and SOLID principles"
```

## Basic Examples

```bash
# Quantitative code quality assessment
find . -name "*.js" -exec wc -l {} + | sort -rn | head -5
"Evaluate code complexity, function size, and duplication, and point out improvements"

# Security vulnerability check
grep -r "password\|secret\|token" . --include="*.js" | head -10
"Check for risks of sensitive information leakage, hardcoding, and authentication bypass"

# Architecture violation detection
grep -r "import.*from.*\.\./\.\." . --include="*.js"
"Evaluate layer violations, circular dependencies, and coupling issues"
```

## Comment Classification System

```text
🔴 critical.must: Critical issues
├─ Security vulnerabilities
├─ Data integrity problems
└─ System failure risks

🟡 high.imo: High-priority improvements
├─ Risk of malfunction
├─ Performance issues
└─ Significant decrease in maintainability

🟢 medium.imo: Medium-priority improvements
├─ Readability enhancement
├─ Code structure improvement
└─ Test quality improvement

🟢 low.nits: Minor points
├─ Style unification
├─ Typo fixes
└─ Comment additions

🔵 info.q: Questions/information
├─ Implementation intent confirmation
├─ Design decision background
└─ Best practices sharing
```

## Review Perspectives

### 1. Code Correctness

- **Logic errors**: Boundary values, null checks, exception handling
- **Data integrity**: Type safety, validation
- **Error handling**: Completeness, appropriate processing

### 2. Security

- **Authentication/authorization**: Appropriate checks, permission management
- **Input validation**: SQL injection, XSS countermeasures
- **Sensitive information**: Logging restrictions, encryption

### 3. Performance

- **Algorithms**: Time complexity, memory efficiency
- **Database**: N+1 queries, index optimization
- **Resources**: Memory leaks, cache utilization

### 4. Architecture

- **Layer separation**: Dependency direction, appropriate separation
- **Coupling**: Tight coupling, interface utilization
- **SOLID principles**: Single responsibility, open-closed, dependency inversion

## Review Flow

1. **Pre-check**: PR information, change diff, related issues
2. **Systematic checks**: Security → Correctness → Performance → Architecture
3. **Constructive feedback**: Specific improvement suggestions and code examples
4. **Follow-up**: Fix confirmation, CI status, final approval

## Comment Templates

### Security Issues Template

**Format:**

- Priority: `critical.must.`
- Issue: Clear description of the problem
- Code example: Proposed fix
- Rationale: Why this is necessary

**Example:**

```text
critical.must. Password is stored in plaintext

Proposed fix:
const bcrypt = require('bcrypt');
const hashedPassword = await bcrypt.hash(password, 12);

Hashing is required to prevent security risks.
```

### Performance Improvement Template

**Format:**

- Priority: `high.imo.`
- Issue: Explain performance impact
- Code example: Proposed improvement
- Effect: Describe expected improvement

**Example:**

```text
high.imo. N+1 query problem occurs

Improvement: Eager Loading
const users = await User.findAll({ include: [Post] });

This can significantly reduce the number of queries.
```

### Architecture Violation Template

**Format:**

- Priority: `high.must.`
- Issue: Point out architectural principle violation
- Recommendation: Specific improvement method

**Example:**

```text
high.must. Layer violation occurred

The domain layer directly depends on the infrastructure layer.
Please introduce an interface following the dependency inversion principle.
```

## Notes

- **Constructive tone**: Collaborative rather than aggressive communication
- **Specific suggestions**: Provide solutions along with pointing out problems
- **Prioritization**: Address in order of Critical → High → Medium → Low
- **Continuous improvement**: Document review results in a knowledge base