review-order
Review Order
Arguments
Raw arguments: $ARGUMENTS
Parse the arguments as the base branch to diff against. If empty, default to main.
Goal
Produce a scannable map of the changes between the base branch and HEAD, grouped by feature and following the four-pass review order (Types → Data Flow → Business Logic → Edge Cases) within each feature.
The user is going to read this map, then jump into the code themselves. So every bullet needs a file:line so they can click straight in. Keep prose minimal — one short clause per bullet. Think index, not essay.
Do not perform the review. Do not make suggestions. Do not flag things to verify, confirm, double-check, or consider. Do not propose tests, fixes, or improvements. Do not ask "is this intended?" or "is this safe?".
Your job is purely descriptive: name the types, point at where data flows, state what the logic does, and point at where edge-case surfaces live. The user reads the map and decides for themselves what to check.
The review order (why this structure)
Most people read diffs top to bottom. That misses things. The user reads in this order within each feature:
- Types and interfaces first. Describe the data shapes the code expects and produces.
- Data flow second. Trace where data comes in, gets transformed, and goes out.
- Business logic third. State what the code does — methods, guards, branches.
- Edge cases last. Point at the locations where edge-case surfaces exist (branches, nullable values, external input, concurrent paths, queue-context assumptions). Just the location and what makes it an edge-case surface. No advice.
Your job is to make this order easy to follow per feature, not to mash everything into four global buckets.
Instructions
1. Get the diff
Run git diff <base>...HEAD (three-dot — i.e. changes on this branch since it diverged) to see what's changed. Also run git diff --name-status <base>...HEAD for a quick file-level overview. If there are no changes, tell the user and stop.
2. Read the changed files
For every changed file, read enough of the file (and surrounding files only when needed to understand a data flow) to categorise it.
Skip auto-generated files unless they contain something unexpected:
- Lockfiles (
composer.lock,package-lock.json,pnpm-lock.yaml, etc.) - Compiled assets (
public/build/,dist/) - Generated route/type definitions (e.g. Wayfinder action/route files, generated TS types)
3. Identify the feature changes
Group the diff into the distinct user-facing or behavioural changes the branch introduces. A feature is a coherent unit of behaviour, not a file or a layer. Examples of how to split:
- "Unsubscribe via signed link" — controller, route, blade unsubscribe-CTA, page, tests
- "Auto-subscribe on idea creation" — observer hook, migration relationship usage, test
- "Notify subscribers on status change" — observer, notification class, mail template, tests
Heuristics for identifying features:
- A new model/migration usually anchors at least one feature (the thing it represents).
- A new route + controller + page is almost always one feature.
- A new notification class + its trigger (observer/event) + its mail template is one feature.
- Side-effects added to existing flows (e.g. "voting now also subscribes you") are their own small feature.
- Pure refactors with no behavioural change can be grouped as a single "Refactor" feature at the end.
If the branch is genuinely a single feature, emit one feature heading. Don't force a split.
4. Output the map
Return only the markdown document. No preamble, no summary at the end. Order features roughly by importance / blast radius (auth/security first, refactors last).
Voice. Bullets, not paragraphs. Each bullet is path/to/file.ext:LINE — short clause. The clause names what's there in plain words; the file:line is what the user clicks. One line per bullet. No multi-sentence explanations. Think git grep output with a human-readable suffix, not a code review.
Use this structure:
## Feature: [short name of the change]
One sentence describing what behaviour this introduces or changes.
### 1. Types & Interfaces
- `path/file.php:12` — what's there (e.g. "new `subscribers` BelongsToMany on `Idea`")
- `path/migration.php:14` — `idea_subscribers` table, unique on `(idea_id, user_id)`
### 2. Data Flow
- `path/controller.php:23` — entry: `POST /x` → invokable
- `path/observer.php:18` — fan-out: queries subscribers, dispatches notification
- `path/notification.php:30` — exit: queued mail via `mail.foo` template
### 3. Business Logic
- `path/observer.php:22` — short-circuit on `is_internal`
- `path/observer.php:28` — actor-exclusion gate
- `path/controller.php:14` — `detach` runs unconditionally on bound pair
### 4. Edge Cases
- `path/observer.php:25` — `Auth::id()` is null in queue/console contexts
- `path/controller.php:14` — public route, no auth check, signed URL has no expiry
---
## Feature: [next change]
...
Omit a section if there's nothing meaningful in it. A small feature might be just Logic + Edges.
Rules
- Every bullet has a
file:line. That's the whole point — the user clicks it to jump. No bullet without a citation. If you genuinely need to say something that has no specific line (a feature-level note), put it in the one-sentence intro under the feature heading, not in the bullets. - One short clause per bullet. Aim for under ~15 words after the em-dash. If you need more, you're writing a review, not a map. Split into two bullets at different lines, or cut.
- No suggestions, no advice, no questions. Banned phrasings: "verify that…", "confirm…", "make sure…", "consider…", "check whether…", "is this intended?", "is this safe?", "should we…", "could…", "might want to…". State facts only.
- No emojis. If something is security-sensitive, say so in words in the feature intro — don't decorate.
- Description over prescription. "
detachruns unconditionally on the bound pair" is fine. "Confirm it's safe thatdetachruns unconditionally" is not. - Edge Cases name the surface, not the fix. Nullable input, unguarded branch, external state, queue-context assumption, concurrent path. Don't tell the reviewer what to do about it.
- Group by feature, not by review pass. The four passes go inside each feature, in order, every time.
- Omit a pass within a feature if nothing fits it. Don't pad with filler.
- Skip auto-generated files (lockfiles, compiled assets, generated route/type definitions) unless they contain something unexpected.
- Order features by blast radius: security/auth-sensitive first, user-facing behaviour next, internal/refactor last. Flag the sensitive surface in the feature intro sentence.
- A file can appear under multiple features (e.g. a controller that gained two unrelated endpoints). Within one feature, a given line should only appear once — pick the most useful pass for it.
More from unlearndev/skills
spec-generator
Generate a detailed product specification document from a vague idea, rough description, or supporting materials (UI sketches, screenshots, notes, existing docs). Use this skill whenever the user wants to turn a rough concept into a structured spec — even if they say things like "write up a spec for...", "turn this idea into a requirements doc", "I have this rough idea for a product", "help me spec this out", "create a PRD for...", or uploads images/files and wants a spec written from them. Covers all product types including internal tools. Trigger this skill even if the request seems simple — a vague one-liner idea deserves a proper spec just as much as a detailed brief.
43checklist
Convert the current plan, code review, or structured content in context into a persistent markdown checklist file under .claude/plans/
42code-review
Code review staged changes or a specific area of the codebase, optionally delegating to a chosen agent. Use when the user wants a code review.
40feature-generator
Generate a detailed features.md document from a spec.md, or sync changes between spec.md and features.md when either file is updated. Use this skill whenever the user wants to expand a product spec into a full feature list, asks to "generate features", "create features.md", "expand the spec", "update features from spec", or "sync spec and features". Also trigger when the user has modified either spec.md or features.md and wants to keep them in sync. Always use this skill when both files are in play together.
39first-five
Scan a branch or diff against the First Five checklist (Error Handling, Input Boundaries, External Calls, State Mutations, Assumed Dependencies) and report only genuine concerns. Use when the user asks to "run the first five" on a branch or diff.
8triage
Triage a branch or diff by grouping changed files into feature areas, assigning each a risk tier (High/Medium/Low), and producing a scannable summary that helps decide where to spend review time. Use when the user asks to "triage" a branch, PR, or diff.
2