review-elixir

# Elixir Code Review

## Arguments

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

## Hard gates

Complete in order before writing **Issues** in the output (empty scope is allowed; fabricated findings are not).

1. **Scope gate:** You have an explicit list of `.ex`/`.exs`/`.heex` paths under review (from Step 1 or user path). **Pass:** List printed or "No Elixir files in scope" — then stop with no Issues.
2. **Linter gate (style):** Step 2 commands ran for this Mix project; skipped tools are noted in one line (e.g. no `.credo.exs`). **Pass:** You do not report a style issue that already passes the project's formatter/linter for that line.
3. **Protocol gate:** [review-verification-protocol](../review-verification-protocol/SKILL.md) is loaded before Step 6. **Pass:** At least one reported finding was checked against that checklist (state which item in the Review Summary or first Critical/Major note).
4. **Evidence gate (Critical/Major):** For each Critical or Major item, you re-read the file at `FILE:LINE` (full surrounding context, not only the diff hunk). **Pass:** The Issue description matches observable code at that location.

## Step 1: Identify Changed Files

```bash
git diff --name-only $(git merge-base HEAD main)..HEAD | grep -E '\.ex$|\.exs$|\.heex$'
```

## Step 2: Verify Linter/Formatter Status

**CRITICAL**: Run project linters BEFORE flagging any style issues.

```bash
# Check formatting
mix format --check-formatted

# Check Credo if present
if [ -f ".credo.exs" ] || grep -q ":credo" mix.exs 2>/dev/null; then
    mix credo --strict
fi

# Check Dialyzer if configured
if grep -q ":dialyxir" mix.exs 2>/dev/null; then
    mix dialyzer --format short
fi
```

**Rules:**
- If a linter passes for a specific rule, DO NOT flag that issue manually
- Linter configuration is authoritative for style rules
- Only flag issues that linters cannot detect (semantic issues, architectural problems)

## Step 3: Detect Technologies

```bash
# Detect Phoenix
grep -r "use Phoenix\|Phoenix.Router\|Phoenix.Controller" --include="*.ex" -l | head -3

# Detect LiveView
grep -r "use Phoenix.LiveView\|Phoenix.LiveComponent\|~H" --include="*.ex" -l | head -3

# Detect Oban
grep -r "use Oban.Worker\|Oban.insert" --include="*.ex" -l | head -3

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

## 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 below (read its SKILL.md and apply its rules).

**Always load:**
- [elixir-code-review](../elixir-code-review/SKILL.md)

**Conditionally load based on detection:**

| Condition | Skill |
|-----------|-------|
| Phoenix detected | [phoenix-code-review](../phoenix-code-review/SKILL.md) |
| LiveView detected | [liveview-code-review](../liveview-code-review/SKILL.md) |
| Performance focus requested | [elixir-performance-review](../elixir-performance-review/SKILL.md) |
| Security focus requested | [elixir-security-review](../elixir-security-review/SKILL.md) |
| Test files changed | [exunit-code-review](../exunit-code-review/SKILL.md) |

## Step 6: Review

**If the agent supports subagents** (and `--parallel` is set), dispatch one subagent per technology area in parallel; **otherwise** run sequentially. Both paths produce identical output.

**Sequential (default):**
1. Load applicable skills
2. Review Elixir quality issues first
3. Review Phoenix patterns (if detected)
4. Review LiveView patterns (if detected)
5. Review detected technology areas
6. Consolidate findings

**Parallel (when subagents are available and `--parallel` is set):**
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 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
4. **Consider framework idioms** - what looks wrong generically may be correct for Elixir/Phoenix

## Step 7: Verify Findings

Satisfy **Hard gates** items 2–4 before finalizing Issues. 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. For syntax issues - did you verify against current version docs?
5. Remove any findings that are style preferences, not actual issues

## Step 8: Review Convergence

### Single-Pass Completeness

You MUST report ALL issues across ALL categories (style, logic, types, tests, security, performance) in a single review pass. Do not hold back issues for later rounds.

Before submitting findings, ask yourself:
- "If all my recommended fixes are applied, will I find NEW issues in the fixed code?"
- "Am I requesting new code (tests, types, modules) that will itself need review?"

If yes to either: include those anticipated downstream issues NOW, in this review, so the author can address everything at once.

### Scope Rules

- Review ONLY the code in the diff and directly related existing code
- Do NOT request new features, test infrastructure, or architectural changes that didn't exist before the diff
- If test coverage is missing, flag it as ONE Minor issue ("Missing test coverage for X, Y, Z") — do NOT specify implementation details like mock libraries, behaviour extraction, or dependency injection patterns that would introduce substantial new code
- Typespecs, documentation, and naming issues ar