/review-python

description: Comprehensive Python/FastAPI backend code review with optional parallel agents
name: review-python
disable-model-invocation: true

Backend Code Review

Hard gates (sequence)

Advance only when each **pass condition** is objectively satisfied (prevents linter-owned false positives and ungrounded findings):

| Gate | Pass condition | |------|----------------| | **G1 — Diff scope** | Step 1 command has been run; the changed `.py` paths are enumerated in writing (list may be empty — if empty, state that explicitly and do not invent Python findings). | | **G2 — Linters before manual style/type** | For `ruff` and `mypy`: either no project config exists for that tool, **or** it was run on the changed files and you captured pass/fail (exit code or clear tool output). **Do not** add manual style or type findings for rules those tools already enforce when configured. | | **G3 — Protocol and base skills** | The [review-verification-protocol](../review-verification-protocol/SKILL.md), [python-code-review](../python-code-review/SKILL.md), and [fastapi-code-review](../fastapi-code-review/SKILL.md) skills are loaded before Step 6 substantive review. | | **G4 — Evidence per issue** | Step 7 checks are satisfied for each reported issue before it appears in the final list (re-read source, search references for “unused”, confirm framework handling for “missing”, verify syntax against current docs). | | **G5 — Output contract** | Findings use sequential numbering, every issue has `FILE:LINE`, and the **Verdict** follows Step 8 (Critical/Major only block; Minor/Informational do not). |

Arguments

• `--parallel`: If the agent supports subagents, run a specialized subagent per technology area (otherwise reviews run sequentially with identical output)
• Path: Target directory (default: current working directory)

Step 1: Identify Changed Files

**Pass (G1):** Capture the command output (or equivalent) as your authoritative changed-`.py` set before Steps 2–3.

git diff --name-only $(git merge-base HEAD main)..HEAD | grep -E '\.py$'

Step 2: Verify Linter Status

**CRITICAL**: Run project linters BEFORE flagging any style or type issues. **Pass (G2):** You may only proceed to Step 3 after each configured linter has been run on the changed files or you have recorded why it was skipped (missing config).

# Check if ruff config exists and run it
if [ -f "pyproject.toml" ] || [ -f "ruff.toml" ]; then
    ruff check <changed_files>
fi

# Check if mypy config exists and run it
if [ -f "pyproject.toml" ] || [ -f "mypy.ini" ]; then
    mypy <changed_files>
fi

**Rules:**

• If a linter passes for a specific rule (e.g., line length), DO NOT flag that issue manually
• Linter configuration is authoritative for style rules
• Only flag issues that linters cannot detect (semantic issues, architectural problems)

**Why:** Analysis of 24 review outcomes showed 4 false positives (17%) where reviewers flagged line-length violations that `ruff check` confirmed don't exist. The linter's configuration reflects intentional project decisions.

Step 3: Detect Technologies

# Detect Pydantic-AI
grep -r "pydantic_ai\|@agent\.tool\|RunContext" --include="*.py" -l | head -3

# Detect SQLAlchemy
grep -r "from sqlalchemy\|Session\|relationship" --include="*.py" -l | head -3

# Detect Postgres-specific
grep -r "psycopg\|asyncpg\|JSONB\|GIN" --include="*.py" -l | head -3

# Check for test files
git diff --name-only $(git merge-base HEAD main)..HEAD | grep -E 'test.*\.py$'

Step 4: Load Verification Protocol

Load the [review-verification-protocol](../review-verification-protocol/SKILL.md) skill and keep its checklist in mind throughout the review.

Step 5: Load Skills

Load each applicable skill (read its `SKILL.md`) before reviewing its domain.

**Always load:**

• [python-code-review](../python-code-review/SKILL.md)
• [fastapi-code-review](../fastapi-code-review/SKILL.md)

**Conditionally load based on detection:**

| Condition | Skill | |-----------|-------| | Test files changed | [pytest-code-review](../pytest-code-review/SKILL.md) | | Pydantic-AI detected | [pydantic-ai-common-pitfalls](../../../beagle-ai/skills/pydantic-ai-common-pitfalls/SKILL.md) | | SQLAlchemy detected | [sqlalchemy-code-review](../sqlalchemy-code-review/SKILL.md) | | Postgres detected | [postgres-code-review](../postgres-code-review/SKILL.md) |

Step 6: Review

**If the agent supports subagents**, dispatch one per technology area in parallel; **otherwise** run the same areas sequentially, producing identical output.

**Sequential (default, or when subagents are unavailable):** 1. Load applicable skills 2. Review Python quality issues first 3. Review FastAPI patterns 4. Review detected technology areas 5. Consolidate findings

**Parallel (--parallel flag, when the agent supports subagents):** 1. Detect all technologies upfront 2. Dispatch one subagent per technology area 3. Each subagent loads its skill and reviews its domain 4. Wait for all subagents 5. Consolidate findings

Before Flagging Optimization or Pattern Issues

1. **Check project conventions** (e.g. AGENTS.md or CLAUDE.md) for documented intentional patterns 2. **Check code comments** around the flagged area for "intentional", "optimization", or "NOTE:" 3. **Trace the code path** before claiming missing coverage or inconsistent handling 4. **Consider framework idioms** - what looks wrong generically may be correct for the framework

**Why:** Analysis showed rejections where reviewers flagged "inconsistent error handling" that was intentional optimization, and "missing test coverage" for code paths that don't exist.

Step 7: Verify Findings

**Pass (G4):** No issue ships until all bullets below are true for that issue.

Before reporting any issue: 1. Re-read the actual code (not just diff context) 2. For "unused" claims - did you search all references? 3. For "missing" claims - did you check framework/parent handling? 4.