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:
bash
# 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:
bash
# 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):
python
# 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)
python
# 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)
typescript
// 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
markdown
## 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:
bash
# 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