SKILL.md
---
name: branch-based-review
description: Use this skill whenever the user asks for a code review — of a pull request, a branch, a single file, a folder, or the whole codebase. Triggers include phrases like "review this PR", "review my code", "check this branch", "audit this", "look at this file", or any request for feedback on code quality, bugs, security, performance, accessibility, or best practices. Works for React (frontend) and Node.js (backend) code.
---
# Code Review Skill
Use this skill to review code thoroughly and consistently. Adapt the depth to the scope: a single file gets a focused review, a PR gets a diff-based review, a whole codebase gets a multi-pass audit.
## Step 1 — Establish scope
Before reviewing anything, figure out what you're reviewing:
- **PR / branch:** get the diff. `git diff <base>...HEAD` or `gh pr diff <number>`. Confirm the correct base branch — don't assume `main`.
- **File or folder:** read the full file(s), plus the immediate callers of any exported symbol.
- **Whole codebase:** map the structure first (package.json, configs, folder layout, entry points), then plan passes by area.
Read full files, not just hunks. Read callers of changed functions. Context prevents wrong feedback.
If scope is unclear, ask before reviewing.
## Step 2 — State assumed intent
Read the PR description, commit messages, or ask the user what the change is for. State your understanding of the intent at the top of the review. Review against that intent.
## Step 3 — Run the checks
Go through every category below. Skip a category only if it genuinely doesn't apply (e.g., no backend touched), and say so explicitly.
### Correctness
- Logic bugs, off-by-one errors, inverted conditionals
- Edge cases: empty arrays, `null`/`undefined`, network failures, concurrent requests, very large inputs
- Error handling: errors caught at the right layer, promises awaited, rejections propagated not swallowed
- Race conditions, stale closures, mutation of props or shared objects
### Security
- Hardcoded secrets, API keys, tokens
- User input flowing into DB queries, file paths, shell commands, `eval`, `dangerouslySetInnerHTML`, redirects — without validation/sanitization
- Auth and authorization on new endpoints (not just authentication)
- CORS, CSRF, cookie flags (`HttpOnly`, `Secure`, `SameSite`)
- New dependencies: pinned, no known CVEs, supply-chain risk considered
- External links use `rel="noopener noreferrer"` with `target="_blank"`
### React / frontend
- Hook rules: called unconditionally at the top level, dependency arrays correct
- Re-render hygiene: memoization where it matters, not where it doesn't; stable context values
- `key` props stable and unique (not array index for dynamic lists)
- State that should be derived isn't being stored (and vice versa)
- Effects have cleanup: subscriptions, timers, event listeners, abort controllers
- Loading, empty, and error states handled — not just the happy path
- Forms: controlled vs uncontrolled used correctly, validation feedback clear
- Responsive across mobile / tablet / desktop; no fixed widths breaking layout
- Hover / focus / active / disabled states all styled
- Design tokens used, not magic values for colors / spacing / typography
### Accessibility
- Semantic HTML: `<button>` for actions, `<a>` for navigation, one `<h1>`, no skipped heading levels
- Landmarks: `<header>`, `<nav>`, `<main>`, `<footer>`
- Keyboard reachable; visible focus rings (don't remove outline without replacement)
- Images: meaningful `alt` or `alt=""` for decorative
- Forms: every input has a `<label>`, errors linked via `aria-describedby`
- Color contrast meets WCAG AA (4.5:1 body, 3:1 large)
- ARIA only where semantic HTML can't do the job, used correctly
- `prefers-reduced-motion` respected for animations
- Page `<title>` and `lang` attribute set
### Performance
- Bundle: new deps justified, imports tree-shakeable, route-level code splitting
- Images: modern formats (webp/avif), `width`/`height` set to prevent CLS, `loading="lazy"` below the fold, priority for LCP image
- Fonts: preloaded for critical text, `font-display: swap`
- No expensive work in render; memoization where it actually helps
- No request waterfalls; sensible caching; payloads not bloated
- Third-party scripts deferred / loaded after interaction
- Core Web Vitals mindset: LCP, CLS, INP
### SEO (for public/marketing pages)
- `<title>`, meta description, canonical URL
- Open Graph + Twitter card tags
- Structured data (JSON-LD) where relevant
- Crawler-critical content rendered server-side (SSR/SSG) when needed
- Internal links use real `<a href>` with descriptive text
### Node.js / backend
- `async`/`await` consistent, no unhandled promises
- Input validation on every endpoint (zod, joi, valibot, or equivalent)
- Database: no N+1 queries, indexes for new query patterns, transactions where atomicity matters, parameterized queries only
- Errors don't leak stack traces, SQL, or internal paths to clients
- Logging: structured, appropriate levels, no PII / tokens / full request bodies
- Timeouts on outbound calls, sensible connection pool sizing
### API contracts
- Breaking changes to request/response shapes — clients updated in same PR or via deprecation path
- New fields optional; removed fields deprecated before removal
- Versioning when a change can't be backward-compatible
### Tests
- New code has tests covering happy path + at least one edge/failure case
- Tests assert behavior, not implementation details
- No `.only`, no skipped tests, no commented-out assertions left in
### Code quality
- Names express intent
- Functions do one thing; complexity not creeping up
- No dead code, commented-out blocks, debug `console.log`s
- Comments explain *why*, not *what*
- DRY where it matters; no premature abstraction
## Step 4 — Output the review
Use this format every time:
```
## Assumed intent
(one line — what you understood the change is for)
## Coverage
(what you reviewed, what you sampled, what you didn't get to — only for large reviews)
## Findings
### 🔴 Blocking
Must fix before merge: bugs, security holes, accessibility violations, broken responsive.
### 🟡 Should fix
Real problems but not merge-blocking. Author's call.
### 🟢 Nits
Style, minor improvements. Prefix each with "nit:".
### ✅ What's good
Call out 1–2 things done well. Useful for the author and for calibrating the rest.
## Verdict
One line: Approve / Request changes / Comment.
```
Cite `file:line` for **every** finding. No vague "somewhere in the auth code" comments.
Group repeated patterns. If twelve components have the same issue, list two examples and say "and 10 others — same pattern."
## Tone
- Direct and specific. No throat-clearing, no "this is mostly great but…" preambles.
- Treat the author as a peer who wants real feedback.
- If you're sure something is wrong, say so plainly and propose a fix.
- If you're not sure, ask: "I think this is wrong because X — can you confirm?" Don't assert.
- Don't repeat the same point across multiple findings; consolidate.
- Don't fabricate findings to pad the review. A short honest review beats a long padded one.
## When the review is large
- Checkpoint findings to `/tmp/review-findings.md` as you go so progress isn't lost if context runs out.
- If you hit limits, stop, summarize what you covered and what you didn't, and ask how to proceed. Don't pretend to have reviewed what you skipped.