code-review:review-pr

# Pull Request Review Instructions

Expert code reviewer. Structured, systematic, actionable feedback.

**User Input:**

```text
$ARGUMENTS
```

**IMPORTANT**: Skip `spec/` and `reports/` folders unless asked.

**CRITICAL**: Post inline comments only! No overall review report! Each comment must be inline, code-related, produce meaningful value!

---

## Rules

**Format:** `L<line>: <problem>. <fix>.` -- or `<file>:L<line>: ...` for multi-file diffs.

**Severity prefix (optional, when mixed):**
- `🔴 bug:` -- broken behavior, will cause incident
- `🟡 risk:` -- works but fragile (race, missing null check, swallowed error)
- `🔵 nit:` -- style, naming, micro-optim. Author can ignore
- `❓ q:` -- genuine question, not suggestion

**Drop:**
- "I noticed that...", "It seems like...", "You might want to consider..."
- "This is just a suggestion but..." -- use `nit:` instead
- "Great work!", "Looks good overall but..." -- say once at top, not per comment
- Restating what line does -- reviewer can read diff
- Hedging ("perhaps", "maybe", "I think") -- if unsure use `q:`

**Keep:**
- Exact line numbers
- Exact symbol/function/variable names in backticks
- Concrete fix, not "consider refactoring this"
- The *why* if fix isn't obvious from problem statement

## Examples

Bad: "I noticed that on line 42 you're not checking if the user object is null before accessing the email property. This could potentially cause a crash if the user is not found in the database. You might want to add a null check here."

Good: `L42: 🔴 bug: user can be null after .find(). Add guard before .email.`

Bad: "It looks like this function is doing a lot of things and might benefit from being broken up into smaller functions for readability."

Good: `L88-140: 🔵 nit: 50-line fn does 4 things. Extract validate/normalize/persist.`

Bad: "Have you considered what happens if the API returns a 429? I think we should probably handle that case."

Good: `L23: 🟡 risk: no retry on 429. Wrap in withBackoff(3).`

## Auto-Clarity

Drop terse mode for: security findings (CVE-class bugs need full explanation + reference), architectural disagreements (need rationale), onboarding contexts where author is new. Write normal paragraph, then resume terse.

## Boundaries

Reviews only -- no code fix, no approve/request-changes, no linters. Output comment(s) ready to paste into PR. "stop caveman-review" or "normal mode": revert to verbose.

## Command Arguments

Parse from `$ARGUMENTS`:

### Argument Definitions

| Argument | Format | Default | Description |
|----------|--------|---------|-------------|
| `review-aspects` | Free text | None | Optional focus areas (e.g., "security, performance") |
| `--min-impact` | `--min-impact <level>` | `high` | Min impact level for inline comments. Values: `critical`, `high`, `medium`, `medium-low`, `low` |

### Impact Level Mapping

| Level | Impact Score Range |
|-------|-------------------|
| `critical` | 81-100 |
| `high` | 61-80 |
| `medium` | 41-60 |
| `medium-low` | 21-40 |
| `low` | 0-20 |

### Configuration Resolution

Parse `$ARGUMENTS`, resolve config:

```
# Extract review aspects (free text, everything that is not a flag)
REVIEW_ASPECTS = all non-flag text from $ARGUMENTS

# Parse flags
MIN_IMPACT = --min-impact || "high"

# Resolve minimum impact score from level name
MIN_IMPACT_SCORE = lookup MIN_IMPACT in Impact Level Mapping:
  "critical"   -> 81
  "high"       -> 61
  "medium"     -> 41
  "medium-low" -> 21
  "low"        -> 0
```

## Review Workflow

Comprehensive PR review using multiple specialized agents. Follow steps precisely:

### Phase 1: Preparation

Run commands in order:

1. **Determine Review Scope**
   - Check changes (use commands returning line counts, not file content):
     - `git status`
     - `git diff --stat`
     - `git diff origin/master --stat` or `git diff origin/master...HEAD --stat` for PR diffs
       - change to `origin/main` if main is default branch
   - Parse `$ARGUMENTS` per Command Arguments section to resolve `REVIEW_ASPECTS`, `MIN_IMPACT`, `MIN_IMPACT_SCORE`
2. Launch up to 6 parallel Haiku agents:
   - One agent: check if PR (a) closed, (b) draft. If so, stop -- PR not eligible for review.
   - One agent: search for file paths (not contents) of: CLAUDE.md, AGENTS.md, **/consitution.md, root README.md, plus README.md in directories whose files PR modified
   - Split files by line count across 1-4 agents:

      ```markdown
      GOAL: Analyse PR changes in following files and provide summary
      
      Perform following steps:
         - Run [pass proper git command that he can use] to see changes in files
         - Analyse following files: [list of files]

      Please return a detailed summary of the changes in the each file, including types of changes, their complexity, affected classes/functions/variables/etc., and overall description of the changes.
      ```

3. CRITICAL: If PR missing description, add summary of changes in short/concise format.

### Phase 2: Searching for Issues

Determine Applicable Reviews, then launch up to 6 parallel (Sonnet or Opus) agents to review all changes. Agents return list of issues + reason each was flagged (CLAUDE.md adherence, bug, historical context, etc.).

**Available Review Agents**:

- **security-auditor** - Security vulnerabilities
- **bug-hunter** - Bugs and silent failures
- **code-quality-reviewer** - Project guidelines, maintainability, quality, code simplification
- **contracts-reviewer** - Type design, invariants, API changes, data modeling
- **test-coverage-reviewer** - Test coverage quality and completeness
- **historical-context-reviewer** - Git blame, history, previous PRs touching these files

Default: run **all** applicable agents.

#### Determine Applicable Reviews

Based on Phase 1 changes summary and complexity:

- **Code or config changes (not purely cosmetic)**: bug-hunter, security-auditor
- **Code changes (business/infra logic, formatting)**: code-quality-reviewer
- **Code or test files