Python Code Review

This command invokes the python-reviewer agent for comprehensive Python-specific code review.

What This Command Does

• Identify Python Changes: Find modified .py files via git diff
• Run Static Analysis: Execute ruff, mypy, pylint, black --check
• Security Scan: Check for SQL injection, command injection, unsafe deserialization
• Type Safety Review: Analyze type hints and mypy errors
• Pythonic Code Check: Verify code follows PEP 8 and Python best practices
• Generate Report: Categorize issues by severity

When to Use

Use /python-review when:

• After writing or modifying Python code
• Before committing Python changes
• Reviewing pull requests with Python code
• Onboarding to a new Python codebase
• Learning Pythonic patterns and idioms

Review Categories

CRITICAL (Must Fix)

• SQL/Command injection vulnerabilities
• Unsafe eval/exec usage
• Pickle unsafe deserialization
• Hardcoded credentials
• YAML unsafe load
• Bare except clauses hiding errors

HIGH (Should Fix)

• Missing type hints on public functions
• Mutable default arguments
• Swallowing exceptions silently
• Not using context managers for resources
• C-style looping instead of comprehensions
• Using type() instead of isinstance()
• Race conditions without locks

MEDIUM (Consider)

• PEP 8 formatting violations
• Missing docstrings on public functions
• Print statements instead of logging
• Inefficient string operations
• Magic numbers without named constants
• Not using f-strings for formatting
• Unnecessary list creation

Automated Checks Run

# Type checking
mypy .
Linting and formatting
ruff check .
black --check .
isort --check-only .
Security scanning
bandit -r .
Dependency audit
pip-audit
safety check
Testing
pytest --cov=app --cov-report=term-missing

Example Usage

User: /python-review
Agent:
Python Code Review Report
Files Reviewed
app/routes/user.py (modified)
app/services/auth.py (modified)

Static Analysis Results
✓ ruff: No issues
✓ mypy: No errors
WARNING: black: 2 files need reformatting
✓ bandit: No security issues
Issues Found
[CRITICAL] SQL Injection vulnerability
File: app/routes/user.py:42
Issue: User input directly interpolated into SQL query

Fix: Use parameterized query

[HIGH] Mutable default argument
File: app/services/auth.py:18
Issue: Mutable default argument causes shared state

Fix: Use None as default

[MEDIUM] Missing type hints
File: app/services/auth.py:25
Issue: Public function without type annotations

Fix: Add type hints

[MEDIUM] Not using context manager
File: app/routes/user.py:55
Issue: File not closed on exception

Fix: Use context manager

Summary
CRITICAL: 1
HIGH: 1
MEDIUM: 2

Recommendation: FAIL: Block merge until CRITICAL issue is fixed
Formatting Required
Run: black app/routes/user.py app/services/auth.py

Approval Criteria

| Status | Condition | |--------|-----------| | PASS: Approve | No CRITICAL or HIGH issues | | WARNING: Warning | Only MEDIUM issues (merge with caution) | | FAIL: Block | CRITICAL or HIGH issues found |

Integration with Other Commands

• Use /tdd first to ensure tests pass
• Use /code-review for non-Python specific concerns
• Use /python-review before committing
• Use /build-fix if static analysis tools fail

Framework-Specific Reviews

Django Projects

• N+1 query issues (use select_related and prefetch_related)
• Missing migrations for model changes
• Raw SQL usage when ORM could work
• Missing transaction.atomic() for multi-step operations

FastAPI Projects

• CORS misconfiguration
• Pydantic models for request validation
• Response models correctness
• Proper async/await usage
• Dependency injection patterns

Flask Projects

• Context management (app context, request context)
• Proper error handling
• Blueprint organization
• Configuration management

Related

• Agent: agents/python-reviewer.md
• Skills: skills/python-patterns/, skills/python-testing/

Common Fixes

Add Type Hints

# Before
def calculate(x, y):
    return x + y
After
from typing import Union
def calculate(x: Union[int, float], y: Union[int, float]) -> Union[int, float]:
    return x + y

Use Context Managers

# Before
f = open("file.txt")
data = f.read()
f.close()
After
with open("file.txt") as f:
    data = f.read()

Use List Comprehensions

# Before
result = []
for item in items:
    if item.active:
        result.append(item.name)
After
result = [item.name for item in items if item.active]

Fix Mutable Defaults

# Before
def append(value, items=[]):
    items.append(value)
    return items
After
def append(value, items=None):
    if items is None:
        items = []
    items.append(value)
    return items

Use f-strings (Python 3.6+)

# Before
name = "Alice"
greeting = "Hello, " + name + "!"
greeting2 = "Hello, {}".format(name)
After
greeting = f"Hello, {name}!"

Fix String Concatenation in Loops

# Before
result = ""
for item in items:
    result += str(item)
After
result = "".join(str(item) for item in items)

Python Version Compatibility

The reviewer notes when code uses features from newer Python versions:

| Feature | Minimum Python | |---------|----------------| | Type hints | 3.5+ | | f-strings | 3.6+ | | Walrus operator (:=) | 3.8+ | | Position-only parameters | 3.8+ | | Match statements | 3.10+ | | Type unions (`x | None`) | 3.10+ |

Ensure your project's pyproject.toml or setup.py specifies the correct minimum Python version.