# Framework-Specific Review Checks

Reference document for expert reviewers. Contains common issues, anti-patterns, and best practices by framework.

---

## React / Next.js

### Hook Rules

```typescript
// BAD: Conditional hook call
function Component({ show }) {
  if (show) {
    const [value, setValue] = useState(0); // Hooks must be at top level
  }
}

// GOOD: Always call hooks unconditionally
function Component({ show }) {
  const [value, setValue] = useState(0);
  if (!show) return null;
}
```

### useEffect Dependencies

```typescript
// BAD: Missing dependency
useEffect(() => {
  fetchUser(userId);
}, []); // userId missing from deps

// GOOD: Include all dependencies
useEffect(() => {
  fetchUser(userId);
}, [userId]);

// GOOD: Use useCallback for stable references
const fetchUserData = useCallback(() => {
  fetchUser(userId);
}, [userId]);

useEffect(() => {
  fetchUserData();
}, [fetchUserData]);
```

### Key Props in Lists

```tsx
// BAD: Index as key (causes issues with reordering)
{items.map((item, index) => (
  <Item key={index} {...item} />
))}

// GOOD: Stable unique identifier
{items.map((item) => (
  <Item key={item.id} {...item} />
))}
```

### Server/Client Boundaries (Next.js App Router)

```tsx
// BAD: Using hooks in Server Component
// app/page.tsx (Server Component by default)
export default function Page() {
  const [count, setCount] = useState(0); // Error!
}

// GOOD: Mark as Client Component
'use client';
export default function Page() {
  const [count, setCount] = useState(0);
}

// BETTER: Keep Server Component, extract interactive part
// app/page.tsx
import Counter from './Counter';
export default function Page() {
  return <Counter />;
}

// app/Counter.tsx
'use client';
export default function Counter() {
  const [count, setCount] = useState(0);
  return <button onClick={() => setCount(c => c + 1)}>{count}</button>;
}
```

### Prop Drilling vs Context

```tsx
// BAD: Excessive prop drilling
<Parent user={user}>
  <Child user={user}>
    <GrandChild user={user}>
      <DeepChild user={user} />  // 4 levels deep
    </GrandChild>
  </Child>
</Parent>

// GOOD: Use Context for widely-shared state
const UserContext = createContext<User | null>(null);

function Parent({ user }) {
  return (
    <UserContext.Provider value={user}>
      <Child />
    </UserContext.Provider>
  );
}

function DeepChild() {
  const user = useContext(UserContext);
}
```

### Memo Optimization

```tsx
// BAD: Premature optimization
const MemoizedComponent = memo(({ onClick }) => {
  return <button onClick={onClick}>Click</button>;
}); // onClick is new every render anyway

// GOOD: Memoize the callback too
const Parent = () => {
  const handleClick = useCallback(() => {
    console.log('clicked');
  }, []);

  return <MemoizedComponent onClick={handleClick} />;
};
```

---

## TypeScript

### Avoid `any`

```typescript
// BAD: any defeats type safety
function process(data: any) {
  return data.foo.bar; // No type checking
}

// GOOD: Use unknown + type guards
function process(data: unknown) {
  if (isValidData(data)) {
    return data.foo.bar; // Type-safe
  }
  throw new Error('Invalid data');
}

function isValidData(data: unknown): data is { foo: { bar: string } } {
  return typeof data === 'object' && data !== null && 'foo' in data;
}
```

### Non-null Assertions

```typescript
// BAD: Non-null assertion hiding potential bugs
const user = users.find(u => u.id === id)!;
console.log(user.name); // Runtime error if not found

// GOOD: Handle the undefined case
const user = users.find(u => u.id === id);
if (!user) {
  throw new Error(`User ${id} not found`);
}
console.log(user.name);
```

### Generic Constraints

```typescript
// BAD: Overly permissive generic
function getProperty<T, K>(obj: T, key: K) {
  return obj[key]; // Error: Type 'K' cannot be used to index type 'T'
}

// GOOD: Constrain K to keys of T
function getProperty<T, K extends keyof T>(obj: T, key: K): T[K] {
  return obj[key];
}
```

### Type vs Interface

```typescript
// Prefer interface for object shapes (extendable)
interface User {
  id: string;
  name: string;
}

interface Admin extends User {
  permissions: string[];
}

// Use type for unions, intersections, primitives
type Status = 'pending' | 'active' | 'archived';
type UserWithMeta = User & { metadata: Record<string, unknown> };
```

### Discriminated Unions

```typescript
// BAD: Optional fields for variants
interface Result {
  success: boolean;
  data?: string;
  error?: Error;
}

// GOOD: Discriminated union
type Result =
  | { success: true; data: string }
  | { success: false; error: Error };

function handle(result: Result) {
  if (result.success) {
    console.log(result.data); // TypeScript knows data exists
  } else {
    console.error(result.error); // TypeScript knows error exists
  }
}
```

---

## Python

### Mutable Default Arguments

```python
# BAD: Mutable default persists across calls
def append_to(element, target=[]):
    target.append(element)
    return target

append_to(1)  # [1]
append_to(2)  # [1, 2] - Unexpected!

# GOOD: Use None and create inside function
def append_to(element, target=None):
    if target is None:
        target = []
    target.append(element)
    return target
```

### Bare Except Clauses

```python
# BAD: Catches everything including KeyboardInterrupt
try:
    risky_operation()
except:
    pass

# BAD: Too broad
try:
    risky_operation()
except Exception:
    pass

# GOOD: Catch specific exceptions
try:
    risky_operation()
except (ValueError, TypeError) as e:
    logger.error(f"Operation failed: {e}")
    raise
```

### Resource Management

```python
# BAD: Manual resource management
f = open('file.txt')
content = f.read()
f.close()  # May not run if exception occurs

# GOOD: Context manager
with open('file.txt') as f:
    content = f.read()

# GOOD: For database connections
with engine.connect() as conn:
    result = conn.execute(query)
```

### String Formatting for SQL

```python
# BAD: SQL injection vulnerability
query = f"SELECT * FROM users WHERE id = {user_id}"

# GOOD: Parameterized query
query = "SELECT * FROM users WHERE id = %s"
cursor.execute(query, (user_id,))

# GOOD: SQLAlchemy
query = select(User).where(User.id == user_id)
```

### Type Hints

```python
# BAD: No type information
def process(data):
    return data.items()

# GOOD: Full type annotations
from typing import Dict, List, Tuple

def process(data: Dict[str, int]) -> List[Tuple[str, int]]:
    return list(data.items())
```

### Async Patterns

```python
# BAD: Blocking call in async function
async def fetch_data():
    response = requests.get(url)  # Blocks event loop!
    return response.json()

# GOOD: Use async HTTP client
async def fetch_data():
    async with aiohttp.ClientSession() as session:
        async with session.get(url) as response:
            return await response.json()

# BAD: Sequential when could be concurrent
async def fetch_all(urls):
    results = []
    for url in urls:
        results.append(await fetch(url))  # Sequential!
    return results

# GOOD: Concurrent execution
async def fetch_all(urls):
    return await asyncio.gather(*[fetch(url) for url in urls])
```

---

## Go

### Error Handling

```go
// BAD: Ignoring errors
result, _ := someFunction()

// BAD: Just returning error without context
if err != nil {
    return err
}

// GOOD: Wrap errors with context
if err != nil {
    return fmt.Errorf("failed to process user %s: %w", userID, err)
}
```

### Goroutine Leaks

```go
// BAD: Goroutine blocked forever on channel
func process() {
    ch := make(chan int)
    go func() {
        result := expensiveComputation()
        ch <- result  // Blocks if no receiver
    }()
    // Function returns without receiving from ch
}

// GOOD: Use buffered channel or context
func process(ctx context.Context) error {
    ch := make(chan int, 1)  // Buffered
    go func() {
        ch <- expensiveComputation()
    }()

    select {
    case result := <-ch:
        return processResult(result)
    case <-ctx.Done():
        return ctx.Err()
    }
}
```

### Race Conditions

```go
// BAD: Concurrent map access
var cache = make(map[string]int)

func get(key string) int {
    return cache[key]  // Race!
}

func set(key string, value int) {
    cache[key] = value  // Race!
}

// GOOD: Use sync.Map or mutex
var cache sync.Map

func get(key string) (int, bool) {
    val, ok := cache.Load(key)
    if !ok {
        return 0, false
    }
    return val.(int), true
}

func set(key string, value int) {
    cache.Store(key, value)
}
```

### Context Propagation

```go
// BAD: Creating new context, breaking cancellation chain
func handler(ctx context.Context) {
    newCtx := context.Background()  // Loses parent's deadline/cancellation
    doWork(newCtx)
}

// GOOD: Propagate context
func handler(ctx context.Context) {
    doWork(ctx)
}

// GOOD: Add timeout while preserving parent
func handler(ctx context.Context) {
    childCtx, cancel := context.WithTimeout(ctx, 5*time.Second)
    defer cancel()
    doWork(childCtx)
}
```

### Defer in Loops

```go
// BAD: Defers accumulate, resources not released until function returns
func processFiles(files []string) error {
    for _, file := range files {
        f, err := os.Open(file)
        if err != nil {
            return err
        }
        defer f.Close()  // All closes happen at function end!
    }
    return nil
}

// GOOD: Use closure to scope defer
func processFiles(files []string) error {
    for _, file := range files {
        if err := processFile(file); err != nil {
            return err
        }
    }
    return nil
}

func processFile(path string) error {
    f, err := os.Open(path)
    if err != nil {
        return err
    }
    defer f.Close()  // Closes when this function returns
    return process(f)
}
```

### Interface Satisfaction

```go
// GOOD: Compile-time interface check
var _ io.Reader = (*MyReader)(nil)

type MyReader struct{}

func (r *MyReader) Read(p []byte) (n int, err error) {
    // Implementation
}
```

---

## Rust

### Ownership and Borrowing

```rust
// BAD: Trying to use moved value
let s1 = String::from("hello");
let s2 = s1;
println!("{}", s1);  // Error: value borrowed after move

// GOOD: Clone if you need both
let s1 = String::from("hello");
let s2 = s1.clone();
println!("{} {}", s1, s2);

// BETTER: Borrow instead of move
let s1 = String::from("hello");
let s2 = &s1;
println!("{} {}", s1, s2);
```

### Unwrap Abuse

```rust
// BAD: Panics on None/Err
let value = some_option.unwrap();
let result = some_result.unwrap();

// GOOD: Handle the error case
let value = some_option.ok_or_else(|| Error::new("value missing"))?;

// GOOD: Provide default
let value = some_option.unwrap_or_default();
let value = some_option.unwrap_or_else(|| compute_default());

// GOOD: Pattern matching
match some_option {
    Some(v) => process(v),
    None => handle_missing(),
}
```

### Lifetime Annotations

```rust
// BAD: Missing lifetime causes confusion
fn longest(x: &str, y: &str) -> &str {
    if x.len() > y.len() { x } else { y }
}

// GOOD: Explicit lifetime
fn longest<'a>(x: &'a str, y: &'a str) -> &'a str {
    if x.len() > y.len() { x } else { y }
}
```

### Unsafe Blocks

```rust
// BAD: Unnecessary unsafe
unsafe {
    let v = vec![1, 2, 3];  // Safe operation in unsafe block
}

// BAD: Unsafe without documentation
unsafe {
    ptr::copy_nonoverlapping(src, dst, len);
}

// GOOD: Documented safety invariants
// SAFETY: src and dst are valid for len bytes, non-overlapping,
// and properly aligned for T
unsafe {
    ptr::copy_nonoverlapping(src, dst, len);
}
```

### Error Handling

```rust
// BAD: String errors lose type information
fn process() -> Result<(), String> {
    Err("something went wrong".into())
}

// GOOD: Custom error types
#[derive(Debug, thiserror::Error)]
enum ProcessError {
    #[error("failed to read config: {0}")]
    ConfigRead(#[from] std::io::Error),
    #[error("invalid format: {0}")]
    InvalidFormat(String),
}

fn process() -> Result<(), ProcessError> {
    let config = std::fs::read_to_string("config.toml")?;
    // ...
}
```

### Clone vs Copy

```rust
// Know when to derive Copy
#[derive(Clone, Copy)]  // Small, stack-only types
struct Point {
    x: i32,
    y: i32,
}

// Don't derive Copy for heap-allocated types
#[derive(Clone)]  // Clone only, has String
struct User {
    name: String,
    age: u32,
}
```

---

## Vue.js

### Reactivity Gotchas

```vue
<script setup>
// BAD: Destructuring loses reactivity
const { name, email } = props;  // Not reactive!

// GOOD: Use toRefs
const { name, email } = toRefs(props);

// BAD: Replacing reactive object
let state = reactive({ count: 0 });
state = reactive({ count: 1 });  // Loses reactivity!

// GOOD: Mutate properties instead
const state = reactive({ count: 0 });
state.count = 1;
</script>
```

### v-for Key Requirement

```vue
<!-- BAD: Missing key -->
<li v-for="item in items">{{ item.name }}</li>

<!-- BAD: Index as key with mutable list -->
<li v-for="(item, index) in items" :key="index">{{ item.name }}</li>

<!-- GOOD: Unique identifier -->
<li v-for="item in items" :key="item.id">{{ item.name }}</li>
```

### Props Mutation

```vue
<script setup>
// BAD: Mutating props directly
const props = defineProps(['modelValue']);
props.modelValue = 'new value';  // Error!

// GOOD: Emit update event
const emit = defineEmits(['update:modelValue']);
emit('update:modelValue', 'new value');
</script>
```

### Computed vs Methods

```vue
<script setup>
// BAD: Method for derived data
const getFullName = () => `${firstName.value} ${lastName.value}`;

// GOOD: Computed for derived data (cached)
const fullName = computed(() => `${firstName.value} ${lastName.value}`);

// Methods are for actions/side effects
const submit = () => {
  api.save(fullName.value);
};
</script>
```

### Watch Cleanup

```vue
<script setup>
// GOOD: Clean up side effects
watch(searchQuery, async (newQuery, oldQuery, onCleanup) => {
  const controller = new AbortController();
  onCleanup(() => controller.abort());

  const results = await fetch(`/search?q=${newQuery}`, {
    signal: controller.signal
  });
});
</script>
```

---

## SQL / Database

### SQL Injection

```sql
-- BAD: String concatenation
query = "SELECT * FROM users WHERE id = " + userId

-- GOOD: Parameterized queries
query = "SELECT * FROM users WHERE id = $1"
```

### N+1 Query Problem

```python
# BAD: N+1 queries
users = User.query.all()
for user in users:
    print(user.posts)  # Separate query for each user!

# GOOD: Eager loading
users = User.query.options(joinedload(User.posts)).all()
for user in users:
    print(user.posts)  # Already loaded
```

### Missing Indexes

```sql
-- Check for missing indexes on frequently queried columns
-- BAD: Full table scan
SELECT * FROM orders WHERE customer_id = 123;

-- GOOD: Add index
CREATE INDEX idx_orders_customer_id ON orders(customer_id);

-- For compound queries
CREATE INDEX idx_orders_customer_date ON orders(customer_id, created_at);
```

### Transaction Boundaries

```python
# BAD: No transaction for related operations
user = create_user(data)
profile = create_profile(user.id)  # What if this fails?

# GOOD: Wrap in transaction
with db.transaction():
    user = create_user(data)
    profile = create_profile(user.id)
    # Both succeed or both rollback
```

### SELECT *

```sql
-- BAD: Fetching all columns
SELECT * FROM users WHERE id = 1;

-- GOOD: Fetch only needed columns
SELECT id, name, email FROM users WHERE id = 1;
```

### LIKE with Leading Wildcard

```sql
-- BAD: Cannot use index
SELECT * FROM products WHERE name LIKE '%widget%';

-- BETTER: Trailing wildcard can use index
SELECT * FROM products WHERE name LIKE 'widget%';

-- BEST: Full-text search for complex patterns
SELECT * FROM products WHERE to_tsvector('english', name) @@ to_tsquery('widget');
```

---

## Security Checks (Cross-Framework)

### Secrets in Code

```
# CRITICAL: Never commit secrets
BAD:  api_key = "sk-1234567890abcdef"
BAD:  password = "hunter2"
BAD:  AWS_SECRET_ACCESS_KEY=AKIAIOSFODNN7EXAMPLE

GOOD: Use environment variables
GOOD: Use secret management (AWS Secrets Manager, HashiCorp Vault)
GOOD: Use .env files (gitignored)
```

### Input Validation

```
# Always validate at system boundaries
- User input from forms
- URL parameters
- API request bodies
- File uploads (type, size, content)
- Headers and cookies
```

### Authentication/Authorization

```
# Common issues:
- Missing authentication on endpoints
- Authorization bypass (checking user ID client-side)
- Insecure session handling
- Missing rate limiting
- Weak password requirements
```

### Sensitive Data Exposure

```
# Check for:
- Passwords in logs
- PII in error messages
- Sensitive data in URLs
- Missing encryption at rest
- Missing HTTPS
```

---

## Performance Checks (Cross-Framework)

### Unnecessary Re-renders

```
# React: Check for
- Missing memo/useMemo/useCallback
- Creating objects/arrays in render
- Context value changing every render

# Vue: Check for
- Computed not used for derived state
- Missing v-once for static content
- Large reactive objects when ref would suffice
```

### Memory Leaks

```
# Check for:
- Event listeners not removed
- Timers not cleared
- Subscriptions not unsubscribed
- DOM references held after removal
- Closures capturing large objects
```

### Bundle Size

```
# Check for:
- Importing entire libraries (import _ from 'lodash')
- Missing tree-shaking
- Large dependencies for small features
- Duplicate dependencies
- Missing code splitting
```

---
name: review
description: "Code review with semantic diffs, expert routing, and auto-TaskCreate. Triggers on: code review, review changes, check code, review PR, security audit."
when_to_use: "Use when the user asks for a code review of staged changes, specific files, or a PR — e.g. 'review my changes', 'check this code before commit', 'security audit this diff', 'review PR 12'."
argument-hint: "[target|--all|--pr N] [--security|--perf|--types|--tests|--style] [--quick|--thorough] [--base <branch>] [--json]"
effort: high
license: MIT
allowed-tools: "Read Write Edit Bash Glob Grep Task TaskCreate TaskUpdate"
metadata:
  author: claude-mods
---

# Review Skill - AI Code Review

Perform comprehensive code reviews on staged changes, specific files, or pull requests. Dispatches general-purpose reviewers that preload the relevant `-ops` skill based on file types and automatically creates tasks for critical issues.

## Architecture

```
review [target] [--focus] [--depth]
    │
    ├─→ Step 1: Determine Scope
    │     ├─ No args → git diff --cached (staged)
    │     ├─ --all → git diff HEAD (all uncommitted)
    │     ├─ File path → specific file diff
    │     └─ --pr N → gh pr diff N
    │
    ├─→ Step 2: Analyze Changes (parallel)
    │     ├─ delta for syntax-highlighted diff
    │     ├─ difft for semantic diff (structural)
    │     ├─ Categorize: logic, style, test, docs, config
    │     └─ Identify touched modules/components
    │
    ├─→ Step 3: Load Project Standards
    │     ├─ AGENTS.md, CLAUDE.md conventions
    │     ├─ .eslintrc, .prettierrc, pyproject.toml
    │     ├─ Detect test framework
    │     └─ Check CI config for existing linting
    │
    ├─→ Step 4: Route to Reviewers (general-purpose + skill preload)
    │     ├─ TypeScript → general-purpose, preload typescript-ops
    │     ├─ React/JSX → general-purpose, preload react-ops
    │     ├─ Python → general-purpose, preload python-pytest-ops
    │     ├─ Go → general-purpose, preload go-ops
    │     ├─ Rust → general-purpose, preload rust-ops
    │     ├─ Vue → general-purpose, preload vue-ops
    │     ├─ SQL/migrations → general-purpose, preload postgres-ops
    │     ├─ Cypress/E2E → general-purpose, preload cypress-ops
    │     ├─ Cloudflare/Workers → general-purpose, preload cloudflare-ops
    │     ├─ Shell/bash → general-purpose, preload bash-ops
    │     ├─ Claude extensions → general-purpose, preload claude-code-ops
    │     ├─ Multi-domain → parallel general-purpose dispatch
    │     └─ All reviewers preload: security-ops + testing-ops context
    │
    ├─→ Step 5: Generate Review
    │     ├─ Severity: CRITICAL / WARNING / SUGGESTION / PRAISE
    │     ├─ Line-specific comments (file:line refs)
    │     ├─ Suggested fixes as diff blocks
    │     └─ Overall verdict: Ready to commit? Y/N
    │
    └─→ Step 6: Integration
          ├─ Auto-create tasks (TaskCreate) for CRITICAL issues
          ├─ Link to /save for tracking
          └─ Suggest follow-up: /testgen, /explain
```

## Execution Steps

### Step 1: Determine Scope

```bash
# Default: staged changes
git diff --cached --name-only

# Check if anything is staged
STAGED=$(git diff --cached --name-only | wc -l)
if [ "$STAGED" -eq 0 ]; then
    echo "No staged changes. Use --all for uncommitted or specify a file."
    git status --short
fi
```

**For PR review:**
```bash
gh pr diff $PR_NUMBER --patch
```

**For specific file:**
```bash
git diff HEAD -- "$FILE"
```

**For baseline comparison (--base):**
```bash
git diff $BASE_BRANCH...HEAD
```

### Step 2: Analyze Changes

Run semantic diff analysis (parallel where possible):

**With difft (semantic):**
```bash
command -v difft >/dev/null 2>&1 && git difftool --tool=difftastic --no-prompt HEAD~1 || git diff HEAD~1
```

**With delta (syntax highlighting):**
```bash
command -v delta >/dev/null 2>&1 && git diff --cached | delta || git diff --cached
```

**Categorize changes:**
```bash
git diff --cached --name-only | while read file; do
    case "$file" in
        *.test.* | *.spec.*) echo "TEST: $file" ;;
        *.md | docs/*) echo "DOCS: $file" ;;
        *.json | *.yaml | *.toml) echo "CONFIG: $file" ;;
        *) echo "CODE: $file" ;;
    esac
done
```

**Get diff statistics:**
```bash
git diff --cached --stat
```

### Step 3: Load Project Standards

```bash
# Claude Code conventions
cat AGENTS.md 2>/dev/null | head -50
cat CLAUDE.md 2>/dev/null | head -50

# Linting configs
cat .eslintrc* 2>/dev/null | head -30
cat .prettierrc* 2>/dev/null
cat pyproject.toml 2>/dev/null | head -30

# Test framework detection
cat package.json 2>/dev/null | jq '.devDependencies | keys | map(select(test("jest|vitest|mocha|cypress|playwright")))' 2>/dev/null
```

**Check CI for existing linting:**
```bash
cat .github/workflows/*.yml 2>/dev/null | grep -E "eslint|prettier|pylint|ruff" | head -10
```

### Step 4: Route to Reviewers

Dispatch is skills-first: domain knowledge lives in `-ops` skills, and the generic `general-purpose` subagent preloads the relevant SKILL.md before reviewing.

| File Pattern | Dispatch | Preload |
|--------------|----------|---------|
| `*.ts` | general-purpose | `skills/typescript-ops/SKILL.md` |
| `*.tsx` | general-purpose | `skills/react-ops/SKILL.md` + `skills/typescript-ops/SKILL.md` |
| `*.vue` | general-purpose | `skills/vue-ops/SKILL.md` + `skills/typescript-ops/SKILL.md` |
| `*.py` | general-purpose | `skills/python-pytest-ops/SKILL.md` (+ `skills/sql-ops/SKILL.md` if ORM) |
| `*.go` | general-purpose | `skills/go-ops/SKILL.md` |
| `*.rs` | general-purpose | `skills/rust-ops/SKILL.md` |
| `*.sql`, `migrations/*` | general-purpose | `skills/postgres-ops/SKILL.md` |
| `agents/*.md`, `skills/*`, `commands/*` | general-purpose | `skills/claude-code-ops/SKILL.md` |
| `*.test.*`, `*.spec.*` | general-purpose | (framework skill by file type) |
| `*.cy.ts`, `cypress/*` | general-purpose | `skills/cypress-ops/SKILL.md` + `skills/typescript-ops/SKILL.md` |
| `*.spec.ts` (Playwright) | general-purpose | `skills/playwright-ops/SKILL.md` + `skills/typescript-ops/SKILL.md` |
| `playwright/*`, `e2e/*` | general-purpose | `skills/playwright-ops/SKILL.md` + `skills/typescript-ops/SKILL.md` |
| `wrangler.toml`, `workers/*` | general-purpose | `skills/cloudflare-ops/SKILL.md` |
| `*.sh`, `*.bash` | general-purpose | `skills/bash-ops/SKILL.md` |

**Invoke via Task tool:**
```
Task tool with subagent_type: "general-purpose" (or surviving specialist from table)
model: "sonnet"
Prompt includes:
  - Skill preloading (domain knowledge):
    "First, read these files for review context:
     - Read: skills/security-ops/references/owasp-detailed.md
     - Read: skills/testing-ops/SKILL.md
     - Read: [Preload column for the matched file pattern]"
  - Diff content
  - Project conventions from AGENTS.md
  - Linting config summaries
  - Requested focus area
  - Request for structured review output
```

**Language-specific preloads** (append to the preloading section above):

| Language | Additional Preload | Why |
|----------|-------------------|-----|
| Python | `skills/python-pytest-ops/SKILL.md` | Python test patterns for coverage review |
| Go | `skills/go-ops/SKILL.md` | Go idioms, concurrency gotchas |
| Rust | `skills/rust-ops/SKILL.md` | Ownership patterns, unsafe review |
| TypeScript | `skills/typescript-ops/SKILL.md` | Type safety patterns |

### Step 5: Generate Review

The reviewer produces a structured review:

```markdown
# Code Review: [scope description]

## Summary

| Metric | Value |
|--------|-------|
| Files reviewed | N |
| Lines changed | +X / -Y |
| Issues found | N (X critical, Y warnings) |

## Verdict

**Ready to commit?** Yes / No

[1-2 sentence summary of overall quality]

---

## Critical Issues

### `src/auth/login.ts:42`

**Issue:** SQL injection vulnerability in user input handling

**Risk:** Attacker can execute arbitrary SQL queries

**Fix:**
```diff
- const query = `SELECT * FROM users WHERE id = ${userId}`;
+ const query = `SELECT * FROM users WHERE id = $1`;
+ const result = await db.query(query, [userId]);
```

---

## Warnings

### `src/components/Form.tsx:89`

**Issue:** Missing dependency in useEffect

**Suggestion:** Add `userId` to dependency array

```diff
- useEffect(() => { fetchUser(userId) }, []);
+ useEffect(() => { fetchUser(userId) }, [userId]);
```

---

## Suggestions

[Style improvements, optional enhancements]

---

## Praise

[Good patterns worth noting]

---

## Files Reviewed

| File | Changes | Issues |
|------|---------|--------|
| `src/auth/login.ts` | +42/-8 | 1 critical |
```

### Step 6: Integration

**Auto-create tasks for CRITICAL issues:**
```
TaskCreate:
  subject: "Fix: SQL injection in login.ts:42"
  description: "SQL injection vulnerability found in user input handling."
  activeForm: "Fixing SQL injection in login.ts:42"
```

**Link with dependencies for related issues:**
```
TaskCreate: #1 "Fix SQL injection in login.ts"
TaskCreate: #2 "Fix SQL injection in register.ts"
TaskUpdate: taskId: "2", addBlockedBy: ["1"]
```

**After fixing issues:**
```
TaskUpdate:
  taskId: "1"
  status: "completed"
```

---

## Severity System

| Level | Icon | Meaning | Action | Auto-Task? |
|-------|------|---------|--------|------------|
| CRITICAL | :red_circle: | Security bug, data loss risk, crashes | Must fix before merge | Yes |
| WARNING | :yellow_circle: | Logic issues, performance problems | Should address | No |
| SUGGESTION | :blue_circle: | Style, minor improvements | Optional | No |
| PRAISE | :star: | Good patterns worth noting | Recognition | No |

---

## Focus Modes

| Mode | What It Checks |
|------|----------------|
| `--security` | OWASP top 10, secrets in code, injection, auth issues |
| `--perf` | N+1 queries, unnecessary re-renders, complexity, memory |
| `--types` | Type safety, `any` usage, generics, null handling |
| `--tests` | Coverage gaps, test quality, mocking patterns |
| `--style` | Naming, organization, dead code, comments |
| (default) | All of the above |

---

## Depth Modes

| Mode | Behavior |
|------|----------|
| `--quick` | Surface-level scan, obvious issues only |
| `--normal` | Standard review, all severity levels (default) |
| `--thorough` | Deep analysis, traces data flow, checks edge cases |

---

## Advanced Flags

### `--base <branch>` - Baseline Comparison

Compare changes against a specific branch instead of HEAD:

```bash
/review --base main
/review src/ --base develop --thorough
```

### `--json` - CI/CD Integration

Output review results as JSON:

```json
{
  "summary": {
    "files_reviewed": 3,
    "lines_changed": { "added": 42, "removed": 8 },
    "issues": { "critical": 1, "warning": 2, "suggestion": 1 }
  },
  "verdict": {
    "ready_to_commit": false,
    "reason": "1 critical issue requires attention"
  },
  "issues": [...]
}
```

**CI/CD usage:**
```yaml
- name: Code Review
  run: |
    claude "/review --json" > review.json
    if jq -e '.issues[] | select(.severity == "critical")' review.json; then
      exit 1
    fi
```

### `--fix` - Auto-Apply Fixes

Automatically apply suggested fixes:

1. Performs standard review
2. For each fixable issue, prompts for confirmation
3. Uses Edit tool to apply approved fixes
4. Creates TaskUpdate for resolved issues

**Non-interactive mode:**
```bash
/review --fix --auto-approve
```

---

## CLI Tool Integration

| Tool | Purpose | Fallback |
|------|---------|----------|
| `delta` | Syntax-highlighted diffs | `git diff` |
| `difft` | Semantic/structural diffs | `git diff` |
| `gh` | GitHub PR operations | Manual diff |
| `rg` | Search for patterns | Grep tool |
| `jq` | Parse JSON configs | Read manually |

**Graceful degradation:**
```bash
command -v delta >/dev/null 2>&1 && git diff --cached | delta || git diff --cached
```

---

## Integration

| Command | Relationship |
|---------|--------------|
| `/explain` | Deep dive into flagged code |
| `/testgen` | Generate tests for issues found |
| `/save` | Persist review findings to session state |