review-code

# Review Code Changes

Perform a comprehensive code review of either the current branch or a specific GitHub pull request.

## Arguments

`$ARGUMENTS` determines the review mode:

**PR mode** — first argument is a number:
- `366` — review PR #366
- `366 focus on the API changes` — review PR #366 with a focus area

**Branch mode** — no number, or only instructions:
- *(empty)* — review current branch against `main`
- `compare against develop` — review against a different base
- `focus on the API changes` — review current branch with a focus area

Additional instructions work in both modes:
- `be strict about type annotations`
- `skip style nits`

## Step 1: Gather Changes

### If PR mode (argument starts with a number)

Run these commands in parallel using `gh`:

1. **PR details**: `gh pr view <number> --json title,body,author,baseRefName,headRefName,state,additions,deletions,changedFiles,commits,url`
2. **PR diff**: `gh pr diff <number>`
3. **PR files**: `gh pr diff <number> --name-only`
4. **PR commits**: `gh pr view <number> --json commits --jq '.commits[].messageHeadline'`
5. **Existing inline review comments**: `gh api repos/{owner}/{repo}/pulls/<number>/comments --paginate --jq '.[].body'`
5b. **Existing PR-level reviews** (top-level review bodies from "Review changes"): `gh api repos/{owner}/{repo}/pulls/<number>/reviews --paginate --jq '.[].body'`
6. **Repo info**: `gh repo view --json nameWithOwner -q '.nameWithOwner'`

Then get the PR branch locally for full file access. Prefer a **worktree** so your current branch and uncommitted work are untouched:

```bash
git fetch origin pull/<number>/head:pr-<number> --force
git worktree add /tmp/review-<number> pr-<number>
# Cleanup when done: git worktree remove /tmp/review-<number> && git branch -D pr-<number>
```

If worktrees aren't suitable, you can use `gh pr checkout <number>` (this switches your current branch — only if you have no uncommitted work). Run the rest of the review from `/tmp/review-<number>`.

If checkout isn't possible (e.g., external fork), use `gh api` to fetch file contents:

```bash
gh api repos/{owner}/{repo}/contents/{path}?ref={head-branch} --jq '.content' | base64 --decode
```

**Important checks:**
- If the PR number doesn't exist, inform the user
- If the PR is merged or closed, note the state but proceed (useful for post-merge audits)
- If the PR is a draft, note it — review may be on incomplete work
- For very large diffs (>3000 lines), fetch and read changed files individually instead of relying solely on the diff

### If Branch mode (no number)

First, fetch the base branch to ensure the remote ref is current:

0. **Fetch base**: `git fetch origin <base>`

Then run these commands in parallel:

1. **Current branch**: `git branch --show-current`
2. **Commits on branch**: `git log origin/<base>..HEAD --oneline`
3. **File changes summary**: `git diff --stat origin/<base>..HEAD`
4. **Full diff**: `git diff origin/<base>..HEAD`
5. **Uncommitted changes**: `git status --porcelain`
6. **Merge base**: `git merge-base origin/<base> HEAD`

Where `<base>` is `main` unless overridden in arguments.

**Important checks:**
- If no commits ahead of base, inform the user there's nothing to review
- If uncommitted changes exist, note them but review committed changes only
- For very large diffs (>3000 lines), read changed files individually instead of relying solely on the diff

## Step 2: Load Project Guidelines

Read the following files at the repository root to load the project's standards and conventions:

- **`AGENTS.md`** — architecture, layering, core design principles, structural invariants
- **`STYLEGUIDE.md`** — code style rules (formatting, naming, imports, type annotations), design principles (DRY, KISS, YAGNI, SOLID), common pitfalls, lazy loading and `TYPE_CHECKING` patterns
- **`DEVELOPMENT.md`** — testing patterns and expectations

**Documentation sources (load when the changeset touches matching areas):**

- **`architecture/*.md`** — subsystem maps aligned with `packages/` (e.g. `engine/mcp/` ↔ `architecture/mcp.md`). Use to verify the PR does not leave recorded architecture false relative to new behavior.
- **`docs/`** — published user-facing documentation. Cross-check when public API, CLI behavior, or config surface changes would affect what readers are told.

Use these guidelines as the baseline for the entire review. Project-specific rules take precedence over general best practices.

## Step 3: Understand the Scope

Before diving into details, build a mental model:

1. **Read the PR description** (PR mode) or commit messages to understand the stated intent
2. **Read each commit message** to understand the progression of changes
3. **Group changed files** by module/package to identify which areas are affected
4. **Identify the primary goal** (feature, refactor, bugfix, etc.)
5. **Note cross-cutting concerns** (e.g., a rename that touches many files vs. substantive logic changes)
6. **Check existing feedback** (PR mode): inspect both inline comments (Step 1, item 5) and PR-level review bodies (Step 1, item 5b) so you don't duplicate feedback already given

## Step 3.5: Structural Impact (if available)

Check for a pre-computed structural impact analysis at
`/tmp/structural-impact-<pr-or-branch>.md`. This file is produced by
`graphify` AST extraction and contains:

- **Risk level** (LOW/MEDIUM/HIGH) based on god nodes touched, import
  violations, and cluster spread
- **Core abstractions modified** - the most-connected entities in the
  codebase (high blast radius if changed)
- **Import direction violations** - cross-package edges that violate the
  layering rule (interface -> engine -> config)
- **High-connectivity changes** - entities with many dependents
- **Cross-package dependencies** - edges crossing package boundaries

If the file exists, read it and use it to calibrate your review:

- **HIGH risk**: apply extra scrutiny in Pass 2 (Design & Architecture).
  Verify backward compatibility for god