Code Review Helper Skill

Systematic code review workflows with utilities for thorough, consistent reviews.

Overview

This skill provides structured workflows and utilities for conducting high-quality code reviews. It emphasizes:

Systematic analysis across multiple dimensions
Automated pattern detection
Structured feedback format
Constructive, actionable suggestions

Review Process

1. Initial Context Gathering

Before reviewing, understand the change:

# View PR metadata
gh pr view <pr-number> --repo <owner/repo>

# Check file changes
gh pr diff <pr-number> --repo <owner/repo>

# Review CI status
gh pr checks <pr-number> --repo <owner/repo>

2. Systematic Analysis

Review across these dimensions:

Correctness

Does the code do what it claims?
Are edge cases handled?
Are error conditions addressed?

Clarity

Is the code easy to understand?
Are names descriptive?
Is complexity justified?

Testing

Are there adequate tests?
Do tests cover edge cases?
Are tests clear and maintainable?

Documentation

Is public API documented?
Are complex algorithms explained?
Are assumptions stated?

Performance

Are there obvious inefficiencies?
Are large operations appropriate?
Is caching used where beneficial?

Security

Are inputs validated?
Are sensitive data handled properly?
Are dependencies trustworthy?

3. Pattern Detection

Use bundled utilities to detect common issues:

# Import review helpers
from review_helpers import (
    check_naming_conventions,
    detect_code_smells,
    analyze_complexity,
    find_duplicate_code,
    check_test_coverage
)

# Analyze code
naming_issues = check_naming_conventions("path/to/file.py")
smells = detect_code_smells("path/to/file.py")
complexity = analyze_complexity("path/to/file.py")
duplicates = find_duplicate_code("src/")
coverage = check_test_coverage("tests/", "src/")

# Report findings
for issue in naming_issues + smells:
    print(f"⚠️ {issue}")

4. Structured Feedback Format

Provide feedback in this format:

## Summary
[Brief overview of the PR and general assessment]

## Strengths
- [Positive aspect 1]
- [Positive aspect 2]

## Issues Found

### Critical
- [ ] **[File:Line]**: [Description]
  - Why: [Explanation]
  - Fix: [Suggestion]

### Important
- [ ] **[File:Line]**: [Description]
  - Why: [Explanation]
  - Fix: [Suggestion]

### Minor
- [ ] **[File:Line]**: [Description]
  - Why: [Explanation]
  - Fix: [Suggestion]

## Suggestions
- [Non-blocking improvement 1]
- [Non-blocking improvement 2]

## Questions
- [Clarification needed 1]
- [Clarification needed 2]

## Overall Assessment
[Approve / Request Changes / Comment]

Bundled Utilities

review_helpers.py

Provides automated analysis functions:

check_naming_conventions(filepath)

Validates Python naming conventions
Checks for PEP 8 compliance
Returns list of naming issues

detect_code_smells(filepath)

Identifies common anti-patterns
Detects magic numbers
Finds long functions/classes
Reports deeply nested code

analyze_complexity(filepath)

Calculates cyclomatic complexity
Identifies complex functions
Suggests refactoring candidates

find_duplicate_code(directory)

Detects code duplication
Uses AST-based analysis
Reports similar code blocks

check_test_coverage(test_dir, source_dir)

Analyzes test-to-code ratio
Identifies untested code paths
Suggests missing test cases

Common Review Patterns

Pattern: Security-Sensitive Code

When reviewing authentication, authorization, or data handling:

**Security Checklist**:
- [ ] Input validation present
- [ ] SQL injection prevention
- [ ] XSS prevention (if web)
- [ ] Secrets not hardcoded
- [ ] Sensitive data encrypted
- [ ] Access control enforced
- [ ] Audit logging included

Pattern: Performance-Critical Code

When reviewing loops, database queries, or large data operations:

**Performance Checklist**:
- [ ] N+1 queries avoided
- [ ] Appropriate indexes used
- [ ] Caching considered
- [ ] Bulk operations used where possible
- [ ] Memory usage reasonable
- [ ] Algorithmic complexity acceptable

Pattern: API Changes

When reviewing public API modifications:

**API Checklist**:
- [ ] Backward compatibility maintained or migration path provided
- [ ] Documentation updated
- [ ] Examples provided
- [ ] Error handling clear
- [ ] Type hints complete
- [ ] Deprecation warnings added if needed

Best Practices

Be Constructive

Focus on improvement, not criticism:

# Unhelpful
This code is terrible.

# Helpful
This function is hard to test due to tight coupling. Consider using dependency injection to improve testability.

Provide Context

Explain why something matters:

# Incomplete
This variable name is bad.

# Complete
The variable name 'x' is unclear. Consider 'customer_count' which better expresses the domain concept and improves readability.

Suggest Alternatives

When requesting changes, show how:

# Vague
This needs to be refactored.

# Specific
Consider extracting this logic into a separate function for better testability and reuse.

Prioritize Issues

Distinguish between blocking and non-blocking feedback:

**Blocking**: Security vulnerability in password handling (line 42)
**Important**: Missing input validation (line 67)
**Nice-to-have**: Consider extracting helper function (line 123)
**Question**: Why use X instead of Y here? (line 89)

Integration with Other Tools

With Shell Tool

# Check for common issues
rg "TODO|FIXME|XXX" --type py src/
grep -r "import \*" src/
find src/ -name "*.py" -exec wc -l {} + | sort -rn | head -10

With Patch Tool

<<<<<<< ORIGINAL
def process_data(data):
    result = []
    for item in data:
        result.append(transform(item))
    return result
=======
def process_data(data):
    """Process data items with transformation.

    Args:
        data: Iterable of items to process

    Returns:
        List of transformed items
    """
    return [transform(item) for item in data]
>>>>>>> UPDATED

With ast-grep

# Find patterns
sg --pattern 'except: $$$' --lang python src/
sg --pattern 'print($MSG)' --lang python src/
sg --pattern 'def $FUNC($ARGS): pass' --lang python src/

Workflow Example

Complete review workflow:

#!/usr/bin/env python3
"""Automated code review workflow."""

from review_helpers import (
    check_naming_conventions,
    detect_code_smells,
    analyze_complexity,
    find_duplicate_code
)

def review_pr(pr_number: int, repo: str):
    """Conduct automated review of PR."""

    # Get changed files
    files = get_pr_files(pr_number, repo)

    # Analyze each file
    all_issues = []
    for filepath in files:
        if filepath.endswith('.py'):
            issues = []
            issues.extend(check_naming_conventions(filepath))
            issues.extend(detect_code_smells(filepath))

            complexity = analyze_complexity(filepath)
            if complexity > 10:
                issues.append(f"High complexity: {complexity}")

            all_issues.extend([(filepath, issue) for issue in issues])

    # Check for duplicates across files
    duplicates = find_duplicate_code("src/")

    # Generate review comment
    comment = format_review_comment(all_issues, duplicates)

    # Post review
    post_pr_review(pr_number, repo, comment)

if __name__ == "__main__":
    review_pr(123, "owner/repo")

Tips for Reviewers

Review small chunks: Don't try to review 1000+ line PRs in one sitting
Test the code: Pull the branch and run it locally when possible
Consider context: Understand the business requirements and constraints
Be timely: Review promptly to unblock authors
Distinguish preferences from problems: Not all feedback is blocking
Learn from reviews: Use reviews as learning opportunities

Tips for Authors

Keep PRs small: Aim for <300 lines when possible
Write descriptive PR descriptions: Explain what, why, and how
Self-review first: Review your own diff before requesting review
Respond to feedback: Address comments or explain why not
Ask questions: Clarify unclear feedback

GitHub PR Workflow - Git practices
gptme patch tool - Code modification

code-review-helper