dev-review

What this skill does

**Iteration topology:** parallel multi-reviewer fan-out (fresh read-only subagents; main chat reconciles)

### Context Check

Before starting this phase, check remaining context:

| Level | Remaining | Action |
|-------|-----------|--------|
| Normal | >35% | Proceed |
| Warning | 25-35% | Finish the current step, then invoke dev-handoff |
| Critical | ≤25% | Invoke dev-handoff immediately — resume fresh |

At Warning/Critical: Read `${CLAUDE_SKILL_DIR}/../../skills/dev-handoff/SKILL.md` and follow its instructions.

### The Iron Law of Topic Changes

**If the user sends a message NOT about the current review, announce the loop pause before responding — then resume.** dev-review runs a REVIEW_STATE.md fix-and-re-review loop; silently abandoning it (as dev-debug:121-139 documents) drops the structure the user invoked.

**Protocol:**
1. Announce: "Pausing the dev-review loop to address your request."
2. Handle the off-topic request (normal tools allowed — you're outside the loop).
3. Announce: "Resuming dev-review. Re-reading .planning/REVIEW_STATE.md for current state."
4. Re-read `.planning/REVIEW_STATE.md` and continue the review/fix iteration.

If the message could be EITHER a new topic OR part of the review, ask before assuming — do NOT silently abandon the loop.

## Contents

- [Prerequisites - Test Output Gate](#prerequisites---test-output-gate)
- [Review Strategy Choice](#review-strategy-choice)
- [Codex Adversarial Review](#codex-adversarial-review) (default when Codex is installed)
- [Parallel Review (Thorough)](#parallel-review-thorough) (Claude-only fallback)
- [The Iron Law of Review](#the-iron-law-of-review) (single Claude reviewer path)
- [Review Focus Areas](#review-focus-areas)
- [Confidence Scoring](#confidence-scoring)
- [Required Output Structure](#required-output-structure)
- [Agent Invocation](#agent-invocation)
- [Quality Standards](#quality-standards)

# Code Review

**Load shared enforcement:**

Auto-load all constraints matching `applies-to: dev-review`:

!`uv run python3 ${CLAUDE_SKILL_DIR}/../../scripts/load-constraints.py dev-review`

**You MUST have these constraints loaded before proceeding. No claiming you "remember" them.**

**Dynamic plan re-read:** Before starting review, re-read `.planning/SPEC.md` and `.planning/PLAN.md` to catch any requirements or tasks added during implementation. Do not rely on cached state from prior phases.

Single-pass code review combining spec compliance and quality checks. Uses confidence-based filtering to report only high-priority issues.

<EXTREMELY-IMPORTANT>
## Prerequisites - Test Output Gate

**Do NOT start review without test evidence.**

Before reviewing, verify these preconditions:
1. `.planning/LEARNINGS.md` contains **actual test output**
2. Tests were **run** (not just written)
3. Test output shows **PASS** (not SKIP, not assumed)

### What Counts as Test Evidence

| Valid Evidence | NOT Valid |
|----------------|-----------|
| `meson test` output with results | "Tests should pass" |
| `pytest` output showing PASS | "I wrote tests" |
| Screenshot of working UI | "It looks correct" |
| Playwright snapshot showing expected state | "User can verify" |
| D-Bus command output | "The feature works" |
| **E2E test output with user flow verified** | **"Unit tests pass" (for UI changes)** |

<EXTREMELY-IMPORTANT>
### The E2E Evidence Requirement

**FOR USER-FACING CHANGES: Unit test evidence is INSUFFICIENT.**

Before approving user-facing changes, verify:
1. Unit tests pass (necessary but not sufficient)
2. **E2E tests pass** (required for approval)
3. Visual evidence exists (screenshots/snapshots for UI)

| Change Type | Unit Evidence | E2E Evidence | Approval? |
|-------------|---------------|--------------|------------|
| Internal refactor | Yes | N/A | APPROVE |
| API change | Yes | Missing | BLOCKED |
| UI change | Yes | Missing | BLOCKED |
| User workflow | Yes | Missing | BLOCKED |

Return BLOCKED if E2E evidence is missing for user-facing changes.

"Unit tests pass" without E2E for UI changes is NOT approvable.
</EXTREMELY-IMPORTANT>

### Gate Check

Check LEARNINGS.md for test output:

```bash
rg -E "(PASS|OK|SUCCESS|\d+ passed)" .planning/LEARNINGS.md
```

If no test output is found, STOP and return to /dev-implement.

"It should work" is NOT evidence. Test output IS evidence.
</EXTREMELY-IMPORTANT>

## Review Strategy Choice

After verifying test output in LEARNINGS.md, choose review strategy.

**Skip this choice when:**
- Trivial changes (< 50 LOC, single file)
- Purely cosmetic changes (formatting, comments)
- Automated refactoring (rename, extract)
- Internal utility functions (not user-facing or security-sensitive)

### Step 1: Probe Codex availability (silent)

Codex provides an out-of-process adversarial reviewer that uses a different
model family than Claude — the diversity catches issues a Claude-reviewing-Claude
loop would miss. When installed and authenticated, it is the **default**
adversarial path. When unavailable, fall back to the existing Claude-based
flow without prompting the user about installation.

Read `${CLAUDE_SKILL_DIR}/../../references/codex-availability.md` for the full
probe and invocation contract. Execute the probe before asking the user:

```bash
CODEX_SCRIPT=$(find "$HOME/.claude/plugins/cache/openai-codex/codex" -maxdepth 3 -name codex-companion.mjs -type f 2>/dev/null | sort -rV | head -1)
if [ -n "$CODEX_SCRIPT" ]; then
  node "$CODEX_SCRIPT" setup --json 2>/dev/null | jq -r '.ready // false'
else
  echo "false"
fi
```

Set `CODEX_READY=true` only when the probe prints `true`. Otherwise
`CODEX_READY=false` and skip Codex entirely — do not announce its absence.

### Step 2: Ask the user

**If `CODEX_READY=true`:**

```python
AskUserQuestion(questions=[{
  "question": "How should we review this implementation?",
  "header": "Review Strategy",
  "options": [
    {"label": "Codex adversarial review (Recommended)", "description": "Out-of-process adversarial review via Codex. Different model family from Claude — catches issues a Claude-on-Claude loop misses. Default for adversarial review."},
    {"label": "Single Claude reviewer", "description": "Combined Claude review covering spec compliance and code quality. Faster, lower overhead. Use when Codex is overkill."},
    {"label": "Parallel Claude review (Thorough)", "description": "Spawn 3 specialized Claude reviewers (Security, Performance, Tests). Use when Codex is unavailable or you want multi-perspective Claude review."}
  ],
  "multiSelect": false
}])
```

**If `CODEX_READY=false`:**

```python
AskUserQuestion(questions=[{
  "question": "How should we review this implementation?",
  "header": "Review Strategy",
  "options": [
    {"label": "Single reviewer (Default)", "description": "Combined review covering spec compliance and code quality. Faster, lower overhead."},
    {"label": "Parallel review (Thorough)", "description": "Spawn 3 specialized reviewers (Security, Performance, Tests). Use for security-sensitive, performance-critical, or test-heavy PRs. Requires reconciliation."}
  ],
  "multiSelect": false
}])
```

**Routing:**

| Choice | Go to |
|--------|-------|
| Codex adversarial review | [Codex Adversarial Review](#codex-adversarial-review) |
| Single (Claude) reviewer | [The Iron Law of Review](#the-iron-law-of-review) |
| Parallel (Claude) review | [Parallel Review (Thorough)](#parallel-review-thorough) |

---

## Codex Adversarial Review

Use this section when the user chose **Codex adversarial review**.

> **Reference:** See `references/codex-availability.md` for the full invocation
> contract, JSON schema, and verdict mapping table.

### 1. Prerequisites Check

Before invoking Codex, verify (same as the other review paths):

1. **Test evidence exists** — LEARNINGS.md contains actual test output
2. **E2E evidence for UI changes** — user-facing changes have E2E test output
3. **SPEC.md exists** — for REQ-ID tagging of findings post-hoc
4. **Git repo present** — Codex adversar