Code Reviewer
This skill routes to the code-reviewer agent for systematic review. The agent's 11-step protocol covers: context understanding, naming conventions, cyclomatic complexity, SOLID principles, error handling, test coverage, DRY violations, documentation, performance, architecture fit, and web-specific patterns.
This file adds dynamic context injection and supplementary checklists.
Dynamic Context (auto-loaded when available)
!git diff HEAD~1 --stat 2>/dev/null || echo "No git diff available"
!git log --oneline -5 2>/dev/null || echo "No git log available"
Supplementary: Accessibility Check (Web — Vite SPA + Next.js)
In addition to the agent's web-specific review (Step 11), verify:
- Semantic HTML elements (
<nav>, <main>, <section>, <article>, <button>, <a>)
- All interactive elements are keyboard accessible (Tab, Enter, Space, Escape)
- Images use
alt attributes. Decorative images use alt=""
- Color contrast meets WCAG AA (4.5:1 normal text, 3:1 large text)
- Forms have
<label> elements associated with inputs (via htmlFor)
- ARIA attributes used correctly — prefer semantic HTML over ARIA
- Focus management: modals trap focus, dialogs return focus on close
- Skip navigation link for keyboard users
- Next.js:
next/image with alt, next/link for navigation
See the std-accessibility skill for the full WCAG 2.2 AA standard.
Supplementary: Accessibility Check (React Native)
- Interactive elements have
accessibilityLabel and accessibilityRole props
- Touch targets are at least 44x44 points
- Color is not the sole means of conveying information
- Screen reader navigation order is logical
- Dynamic content updates announced via
AccessibilityInfo.announceForAccessibility
// Accessible touchable
<TouchableOpacity
accessibilityRole="button"
accessibilityLabel="Delete order"
accessibilityHint="Removes this order from your history"
style={{ minHeight: 44, minWidth: 44 }}
>
<TrashIcon />
</TouchableOpacity>
// Accessible form field
<View>
<Text nativeID="emailLabel">Email Address</Text>
<TextInput
accessibilityLabelledBy="emailLabel"
textContentType="emailAddress"
autoComplete="email"
/>
{error && <Text accessibilityRole="alert">{error}</Text>}
</View>
Supplementary: Stack-Specific Checks
Rails: Panko serializers used (not raw models)? Service objects for business logic? Redis cache TTLs set? Plus the five that fail silently — nothing raises, and the diff looks fine:
- Is the policy actually called?
authorize / policy_scope present, and index uses
policy_scope (authorizing a collection does not filter it). A policy that exists but is never
invoked returns 200 OK with another user's data.
- Does the migration set
lock_timeout? The default is 0 — wait forever — and a waiting
ALTER TABLE queues every query behind it.
remove_column? Then ignored_columns must have shipped in an earlier deploy, alone.
create!/save! inside a transaction, not the non-bang forms (they return false, so the
block completes and commits half the operation). after_commit, not after_save, for jobs.
- Money/irreversible job: explicit
sidekiq_options retry: — the default is 25 retries over
~20 days — and an idempotency key derived from the work, not per attempt.
React Native: Server data in TanStack Query (not Zustand)? Proper staleTime? FlatList for lists? useCallback on render functions? Centrifugo subscriptions cleaned up on unmount?
ReactJS (Vite SPA): Routes lazy-loaded? TanStack Query for server data? Tailwind CSS (no inline styles)? Forms use react-hook-form + zod? Bundle size checked?
Next.js (App Router): Server Components by default? Server actions validate with zod? next/image and next/link? Metadata exported? loading.tsx/error.tsx boundaries? revalidatePath/revalidateTag after mutations?
Migrations: Reversible? Foreign keys indexed? PostGIS columns have GiST index? No data + schema changes mixed?
Output Format
Present findings in this table format:
| Category |
Finding |
Severity |
File:Line |
Recommendation |
Severity Levels:
- Must-Fix: Security vulnerabilities, data loss, production-breaking — block merge
- Should-Fix: Design problems, maintainability — strongly recommend before merge
- Suggestion: Quality improvements — consider for this or follow-up PR
- Nit: Style preferences — optional, do not block merge
End each review with:
- Overall Assessment: Approve / Request Changes / Needs Discussion
- Strengths: What was done well (always include positive feedback)
- Key Issues: Top 3 items that must be addressed
- Key Takeaway: Single most important improvement for future code
Deep guides (read on demand, do not preload)
- Rails red flags, N+1 detection, migration-safety checks, PostGIS spatial checks, React Native red flags, Sidekiq job checks →
references/pr-review-guide.md
- The dimension-by-dimension pass: correctness, security, performance, maintainability, testing, documentation →
references/review-checklist.md
1---2name: code-reviewer3description: Review code and pull requests for quality, security, test coverage, and adherence to team conventions. Use this skill whenever someone asks to review a PR, check code quality, audit a diff, evaluate a changeset, or says things like "review my code", "check this PR", "look at my changes", "is this code good", "audit this module", or "what do you think of this diff". Also trigger when someone mentions code quality concerns, technical debt assessment, or asks for feedback on implementation approach.4---56# Code Reviewer78This skill routes to the **code-reviewer agent** for systematic review. The agent's 11-step protocol covers: context understanding, naming conventions, cyclomatic complexity, SOLID principles, error handling, test coverage, DRY violations, documentation, performance, architecture fit, and web-specific patterns.910This file adds dynamic context injection and supplementary checklists.1112## Dynamic Context (auto-loaded when available)1314!`git diff HEAD~1 --stat 2>/dev/null || echo "No git diff available"`1516!`git log --oneline -5 2>/dev/null || echo "No git log available"`1718## Supplementary: Accessibility Check (Web — Vite SPA + Next.js)1920In addition to the agent's web-specific review (Step 11), verify:21- Semantic HTML elements (`<nav>`, `<main>`, `<section>`, `<article>`, `<button>`, `<a>`)22- All interactive elements are keyboard accessible (Tab, Enter, Space, Escape)23- Images use `alt` attributes. Decorative images use `alt=""`24- Color contrast meets WCAG AA (4.5:1 normal text, 3:1 large text)25- Forms have `<label>` elements associated with inputs (via `htmlFor`)26- ARIA attributes used correctly — prefer semantic HTML over ARIA27- Focus management: modals trap focus, dialogs return focus on close28- Skip navigation link for keyboard users29- Next.js: `next/image` with `alt`, `next/link` for navigation3031See the `std-accessibility` skill for the full WCAG 2.2 AA standard.3233## Supplementary: Accessibility Check (React Native)3435- Interactive elements have `accessibilityLabel` and `accessibilityRole` props36- Touch targets are at least 44x44 points37- Color is not the sole means of conveying information38- Screen reader navigation order is logical39- Dynamic content updates announced via `AccessibilityInfo.announceForAccessibility`4041```tsx42// Accessible touchable43<TouchableOpacity44 accessibilityRole="button"45 accessibilityLabel="Delete order"46 accessibilityHint="Removes this order from your history"47 style={{ minHeight: 44, minWidth: 44 }}48 onPress={handleDelete}49>50 <TrashIcon />51</TouchableOpacity>5253// Accessible form field54<View>55 <Text nativeID="emailLabel">Email Address</Text>56 <TextInput57 accessibilityLabelledBy="emailLabel"58 textContentType="emailAddress"59 autoComplete="email"60 />61 {error && <Text accessibilityRole="alert">{error}</Text>}62</View>63```6465## Supplementary: Stack-Specific Checks6667**Rails**: Panko serializers used (not raw models)? Service objects for business logic? Redis cache TTLs set? Plus the five that fail **silently** — nothing raises, and the diff looks fine:68- **Is the policy actually *called*?** `authorize` / `policy_scope` present, and `index` uses69 `policy_scope` (authorizing a collection does not filter it). A policy that exists but is never70 invoked returns `200 OK` with another user's data.71- **Does the migration set `lock_timeout`?** The default is 0 — wait forever — and a waiting72 `ALTER TABLE` queues every query behind it.73- **`remove_column`?** Then `ignored_columns` must have shipped in an earlier deploy, alone.74- **`create!`/`save!` inside a transaction**, not the non-bang forms (they return `false`, so the75 block completes and commits half the operation). `after_commit`, not `after_save`, for jobs.76- **Money/irreversible job:** explicit `sidekiq_options retry:` — the default is 25 retries over77 ~20 days — and an idempotency key derived from the work, not per attempt.7879**React Native**: Server data in TanStack Query (not Zustand)? Proper `staleTime`? `FlatList` for lists? `useCallback` on render functions? Centrifugo subscriptions cleaned up on unmount?8081**ReactJS (Vite SPA)**: Routes lazy-loaded? TanStack Query for server data? Tailwind CSS (no inline styles)? Forms use react-hook-form + zod? Bundle size checked?8283**Next.js (App Router)**: Server Components by default? Server actions validate with zod? `next/image` and `next/link`? Metadata exported? `loading.tsx`/`error.tsx` boundaries? `revalidatePath`/`revalidateTag` after mutations?8485**Migrations**: Reversible? Foreign keys indexed? PostGIS columns have GiST index? No data + schema changes mixed?8687## Output Format8889Present findings in this table format:9091| Category | Finding | Severity | File:Line | Recommendation |92|---|---|---|---|---|9394**Severity Levels:**95- **Must-Fix**: Security vulnerabilities, data loss, production-breaking — block merge96- **Should-Fix**: Design problems, maintainability — strongly recommend before merge97- **Suggestion**: Quality improvements — consider for this or follow-up PR98- **Nit**: Style preferences — optional, do not block merge99100End each review with:1011. **Overall Assessment**: Approve / Request Changes / Needs Discussion1022. **Strengths**: What was done well (always include positive feedback)1033. **Key Issues**: Top 3 items that must be addressed1044. **Key Takeaway**: Single most important improvement for future code105106## Deep guides (read on demand, do not preload)107108- Rails red flags, N+1 detection, migration-safety checks, PostGIS spatial checks, React Native red flags, Sidekiq job checks → `references/pr-review-guide.md`109- The dimension-by-dimension pass: correctness, security, performance, maintainability, testing, documentation → `references/review-checklist.md`