Pr Review

Pull Request Reviewer

Comprehensive PR review covering code quality, security, performance, and best practices.

Arguments

• $ARGUMENTS: PR number or branch name to review

PR Context

Fetch PR information first:

# Get PR details
gh pr view $ARGUMENTS --json title,body,author,additions,deletions,files,commits

# Get PR diff
gh pr diff $ARGUMENTS

# Get changed files list
gh pr diff $ARGUMENTS --name-only

# Get PR comments
gh pr view $ARGUMENTS --comments

Review Checklist

1. Code Quality

• Readability: Is the code easy to understand?
• Naming: Are variables, functions, and classes named clearly?
• DRY: Is there code duplication that should be refactored?
• Complexity: Are functions/methods reasonably sized?
• Comments: Are complex sections documented?
• Dead Code: Are there unused imports, variables, or functions?

2. Security Review

Check for:

• SQL/NoSQL injection vulnerabilities
• XSS vulnerabilities (unsanitized user input in HTML)
• Authentication/authorization bypasses
• Hardcoded secrets or API keys
• Insecure dependencies
• Improper error handling (exposing stack traces)
• Missing input validation

Patterns to search:

# SQL injection
grep -rn "execute.*\$\|format.*%s\|f\".*{.*}.*WHERE" --include="*.py"

# Hardcoded secrets
grep -rn "api_key\|secret\|password\|token" --include="*.py" --include="*.ts" --include="*.js"

# Unsafe eval
grep -rn "eval\|exec\|dangerouslySetInnerHTML"

# XSS patterns
grep -rn "innerHTML\|document.write\|\.html\("

3. Performance Review

• N+1 Queries: Are database queries efficient?
• Caching: Is appropriate caching used?
• Lazy Loading: Are large resources loaded lazily?
• Memory Leaks: Are resources properly cleaned up?
• Async Operations: Are I/O operations non-blocking?

Check MongoDB queries (Beanie):

# Good: Using select_related equivalent
await Model.find().limit(100).to_list()

# Bad: Loading all in memory
await Model.find().to_list()  # No limit!

4. Testing Review

• Coverage: Are new features covered by tests?
• Edge Cases: Are edge cases tested?
• Mocking: Are external dependencies properly mocked?
• Test Names: Do test names describe what they test?

5. Architecture Review

• Separation of Concerns: Is business logic separate from HTTP handling?
• Dependency Injection: Are dependencies properly injected?
• Error Handling: Are errors handled consistently?
• API Design: Are API endpoints RESTful and consistent?

6. Documentation Review

• README: Is README updated if needed?
• API Docs: Are new endpoints documented?
• Comments: Are complex algorithms explained?
• CHANGELOG: Is changelog updated?

Project-Specific Checks

FastAPI Projects (eruditiontx, mathmatterstx)

# Check route patterns
# Good
@router.get("/items/{item_id}")
async def get_item(item_id: str, current_user: User = Depends(get_current_user)):

# Check for proper validation
# Good: Using Pydantic models
async def create_item(item: ItemCreate):

# Check role-based access
# Good: Using JWTBearer dependency
@router.post("/admin/action", dependencies=[Depends(JWTBearer(access_levels=["admin"]))])

Next.js Projects (notaryo.ph, bocs-turbo)

// Check for proper server/client separation
// Good: 'use server' for server actions
'use server'
export async function createItem() {}

// Check for proper error boundaries
// Good: error.tsx in route folders

// Check for proper loading states
// Good: loading.tsx in route folders

Output Format

## PR Review: #[number] - [title]

### Summary
[Brief description of what this PR does]

### Changes Overview
- Files changed: X
- Additions: +Y
- Deletions: -Z

### Review Findings

#### Code Quality
- [Finding 1 with file:line reference]
- [Finding 2]

#### Security
- [Security finding or "No security issues found"]

#### Performance
- [Performance finding or "No performance concerns"]

#### Testing
- [Testing coverage assessment]

### Recommendations

**Required Changes:**
1. [Critical issue that must be fixed]

**Suggested Improvements:**
1. [Nice-to-have improvement]

### Verdict
- [ ] Approve
- [ ] Request Changes
- [ ] Comment

Automated Checks

Before review, run automated checks:

# Lint the changed files
/lint-fix $(gh pr diff $ARGUMENTS --name-only | tr '\n' ' ')

# Run tests
/test all

# Check for security vulnerabilities
npm audit  # or pip-audit for Python