Code Critique

---
name: code-critique
description: Identifies, categorizes, and articulates code issues with brutal honesty and quantifiable specificity. Use when reviewing code, providing feedback, performing code audits, or when user needs direct assessment of code quality.
---

# Code Critique

Systematic methodology for identifying code problems and communicating them with precision, specificity, and quantifiable evidence. No hedging, no softening, no vague suggestions.

## Quick Start

**Full code critique:**
```
Perform a brutally honest code critique of this file. Identify all issues with specific line numbers, categories, and severity scores.
```

**Targeted review:**
```
Review this function for violations. Name each code smell, cite the line, and score severity 1-10.
```

**Pre-PR audit:**
```
Audit this code before PR submission. List every issue that would fail review, with exact locations.
```

## Core Principles

### The Three Commandments

1. **Be Direct** - State the problem, not a suggestion
2. **Be Specific** - Line numbers, counts, exact violations
3. **Be Quantitative** - Numbers, metrics, comparisons

### Language Standards

| Never Say | Always Say |
|-----------|------------|
| "Perhaps consider..." | "This violates X because Y" |
| "It might be nice if..." | "Line 47 requires Z" |
| "Generally speaking..." | "Lines 23-89 contain 7 instances of X" |
| "Could be improved" | "Fails criterion X, scores 2/10" |
| "Seems a bit long" | "This is 200 lines - 4x the recommended maximum" |
| "Error handling could be better" | "Line 47 catches Error but ignores it" |
| "Changes require touching many files" | "This is Shotgun Surgery - 12 files need modification" |

---

## Critique Methodology

### Phase 1: Structural Scan

Identify violations in code organization and architecture.

```
STRUCTURAL CHECKLIST
[ ] File length (>300 lines = violation)
[ ] Function length (>50 lines = violation)
[ ] Class responsibility count (>1 = violation)
[ ] Nesting depth (>3 levels = violation)
[ ] Parameter count (>4 = violation)
[ ] Return points (>3 per function = violation)
```

**Output format:**
```
STRUCTURAL VIOLATIONS: 7

1. FILE LENGTH: 487 lines (limit: 300, 62% over)
   - Extract: lines 234-340 (user validation logic)
   - Extract: lines 400-487 (formatting utilities)

2. FUNCTION: processOrder() at line 45 - 89 lines (limit: 50, 78% over)
   - Cyclomatic complexity: 23 (limit: 10)
   - Nesting depth: 5 levels (limit: 3)
```

### Phase 2: Naming Audit

Evaluate identifier quality with specific violations.

```
NAMING VIOLATIONS: 12

1. Line 23: `d` - Single letter variable (data? date? delta?)
2. Line 45: `handleStuff()` - Vague action verb
3. Line 67: `temp` - Meaningless temporary
4. Line 89: `data2` - Numeric suffix instead of meaning
5. Line 112: `doIt()` - Completely opaque purpose
```

**Naming severity scale:**
- **Critical (9-10)**: Actively misleading (`isValid` returns void)
- **High (7-8)**: Completely opaque (`x`, `temp`, `data`)
- **Medium (4-6)**: Vague but guessable (`handle`, `process`, `doStuff`)
- **Low (1-3)**: Suboptimal but clear (`getUserData` vs `fetchUser`)

### Phase 3: Logic Review

Find bugs, edge cases, and logical flaws.

```
LOGIC ISSUES: 5

1. Line 78: NULL DEREFERENCE - `user.name` accessed without null check
   - user comes from getUserById() which returns null on miss
   - Will throw at runtime when user not found

2. Line 112: OFF-BY-ONE - Loop iterates i <= array.length
   - Last iteration accesses array[array.length] = undefined
   - Should be i < array.length

3. Line 156: RACE CONDITION - Async write without await
   - saveToDatabase() is async but not awaited
   - Subsequent read may see stale data
```

### Phase 4: Performance Scan

Identify inefficiencies with quantified impact.

```
PERFORMANCE ISSUES: 4

1. Line 45-67: N+1 QUERY - Database called in loop
   - 1 query per user, N users = N+1 total queries
   - Current: ~200ms per user * 100 users = 20 seconds
   - Fixed: 1 batch query = ~250ms total
   - Impact: 80x improvement

2. Line 89: REDUNDANT COMPUTATION - filter().map() chain
   - Array iterated twice, can be single reduce()
   - Memory: 2x array allocation vs 1x
```

### Phase 5: Security Audit

Flag vulnerabilities with severity ratings.

```
SECURITY VULNERABILITIES: 3

1. CRITICAL - Line 34: SQL INJECTION
   - Query built with string concatenation: `"SELECT * FROM users WHERE id = " + userId`
   - Exploitable: userId = "1; DROP TABLE users;--"
   - Fix: Use parameterized queries

2. HIGH - Line 78: XSS VULNERABILITY
   - User input rendered without escaping: innerHTML = userComment
   - Fix: Use textContent or sanitize HTML

3. MEDIUM - Line 112: HARDCODED SECRET
   - API key in source: `apiKey = "sk_live_abc123"`
   - Fix: Environment variable
```

### Phase 6: Style Violations

Consistency and convention issues.

```
STYLE VIOLATIONS: 18

1. Lines 23, 45, 67, 89: INCONSISTENT QUOTES
   - 4 single quotes, rest of file uses double quotes

2. Lines 34-56: INCONSISTENT INDENTATION
   - Mix of 2-space and 4-space indentation

3. Line 78: TRAILING WHITESPACE
   - 4 trailing spaces
```

---

## Issue Categorization

### Category Taxonomy

| Category | Description | Severity Weight |
|----------|-------------|-----------------|
| **Security** | Vulnerabilities, injection, exposure | x3 |
| **Logic** | Bugs, edge cases, incorrect behavior | x3 |
| **Performance** | Inefficiency, resource waste | x2 |
| **Structural** | Organization, complexity, coupling | x2 |
| **Naming** | Unclear, misleading identifiers | x1 |
| **Style** | Formatting, consistency | x1 |

### Severity Scoring

Each issue gets a raw severity (1-10) multiplied by category weight.

```
WEIGHTED SEVERITY CALCULATION

Issue: SQL Injection at line 34
- Raw severity: 10 (critical exploitable vulnerability)
- Category: Security (weight: x3)
- Weighted score: 30

Issue: Vague variable name at line 23
- Raw severity: 4 (guessable meaning)
- Category: Naming (weight: x1)
- Weighted score: 4
```

### Quality Score Calculation

```
QUALITY SCORE = 100 - (sum of weighted severities / lines of code * 10)

Example:
- File: 200 lines
- Total weighted severity: 85
- Score: 100 - (85/200 * 10) = 100 - 4.25 = 95.75/100

Interpretation:
- 90-100: Production ready, minor polish only
- 70-89: Acceptable with noted improvements
- 50-69: Significant issues, review required
- Below 50: Not production ready, major rework needed
```

---

## Communication Patterns

### The Critique Template

```markdown
## Code Critique: {filename}

**Quality Score: {X}/100** ({interpretation})

### Summary
- {N} critical issues requiring immediate fix
- {N} high-priority issues for this PR
- {N} medium issues to address soon
- {N} low-priority style/polish items

### Critical Issues
| # | Line | Category | Issue | Severity |
|---|------|----------|-------|----------|
| 1 | 34 | Security | SQL injection via string concatenation | 10 |
| 2 | 78 | Logic | Null dereference on user.name | 9 |

### High Priority
...

### Quantified Summary
- Lines of code: {N}
- Issues per 100 LOC: {N}
- Worst function: {name} at line {N} - {count} issues
- Most common violation: {type} - {count} occurrences
```

### Brutal But Professional Phrasing

See [references/brutal-phrasing.md](references/brutal-phrasing.md) for comprehensive phrasing examples.

**The Pattern:**
```
"{This is} [named problem]. {Evidence: specific location and count}. {Impact: what breaks}."
```

**Examples:**
```
"This is a God Class. UserManager has 47 methods across 1,200 lines handling auth, email, billing, and analytics. Changes to any feature risk breaking unrelated functionality."

"Line 67 has a resource leak. FileInputStream opened but never closed in finally block. Each call leaks one file descriptor; production will hit ulimit after ~1000 requests."

"This function violates SRP. processOrder() at line 45 does validation (lines 46-78), pricing (lines 79-112), inventory check (lines 113-145), and notification (lines 146-189). That's 4 distinct responsibilities in one 144-line function."
```

---

## Evidence Gathering

### Quantification Requirements

Every critique must include:

1. **Location** - File, line number(s), function name
2. **Count** - How many occurrences
3. **Measurement** - Size, complexity, percentage
4. **Comparison** - To standard, limit, or best practice
5. **Impact** - What breaks, degrades, or fails

See [references/evidence-gathering.md](references/evidence-gathering.md) for detailed measurement techniques.

### Standard Measurements

| Metric | Tool/Method | Threshold |
|--------|-------------|-----------|
| Cyclomatic complexity | escomplex, radon | >10 = problem |
| Cognitive complexity | SonarQube | >15 = problem |
| Lines per function | wc -l | >50 = problem |
| Lines per file | wc -l | >300 = problem |
| Nesting depth | manual/lint | >3 = problem |
| Parameters per function | manual/lint | >4 = problem |
| Dependencies per module | madge | >10 = problem |

---

## Common Issue Patterns

See [references/issue-patterns.md](references/issue-patterns.md) for the complete catalog.

### Quick Reference

| Pattern | Detection | One-liner |
|---------|-----------|-----------|
| God Class | >500 lines, >20 methods | "This class has {N} responsibilities; should have 1" |
| Shotgun Surgery | Change requires N files | "Modifying X requires touching {N} files" |
| Feature Envy | Method uses other class more | "This method accesses OtherClass {N} times, self {M} times" |
| Primitive Obsession | Strings for structured data | "Email/phone/money represented as raw strings {N} times" |
| Long Parameter List | >4 parameters | "Function takes {N} parameters; max recommended is 4" |
| Magic Numbers | Unexplained literals | "Lines {list} contain unexplained numeric literals" |
| Dead Code | Unreachable/unused | "Lines {N-M} are unreachable due to early return at line {X}" |
| Copy-Paste | Duplicated blocks | "Lines {A-B} duplicate lines {C-D} with {N}% similarity" |

---

## Critique Workflows

### Quick Review

```
1. File length check - over 300 lines?
2. Function length scan - any over 50 lines?
3. Complexity hotspots - deeply nested code?
4. Obvious violations - hardcoded values, console.logs?
5. Top 3 issues with line numbers
```

### Standard Review

```
1. STRUCTURE - Run full structural scan
2. NAMING - Audit all identifiers in modified code
3. LOGIC - Trace happy path and 2 edge cases
4. SECURITY - Check all inputs/outputs
5. STYLE - Verify consistency
6. Generate quantified report
```

### Deep Audit

```
1. Run static analysis tools
2. Complete all 6 phases with full documentation
3. Calculate quality score
4. Generate dependency analysis
5. Document technical debt items
6. Produce remediation roadmap with effort estimates
```

---

## Output Templates

### Issue Entry

```markdown
### [{CATEGORY}] {Issue Name}
**Location:** `{file}:{line}` in `{function}()`
**Severity:** {N}/10 (weighted: {N*weight})
**Count:** {N} occurrence(s)

**Evidence:**
{Specific code snippet or measurement}

**Impact:**
{What breaks, fails, or degrades}

**Fix:**
{Specific remediation with example}
```

### Critique Summary

```markdown
# Code Critique Report

## Metadata
- **File:** {path}
- **Lines:** {N}
- **Functions:** {N}
- **Review depth:** {Quick|Standard|Deep}

## Quality Score: {N}/100

## Issue Distribution
| Category | Count | Weighted Score |
|----------|-------|----------------|
| Security | {N} | {N} |
| Logic | {N} | {N} |
| Performance | {N} | {N} |
| Structural | {N} | {N} |
| Naming | {N} | {N} |
| Style | {N} | {N} |
| **Total** | **{N}** | **{N}** |

## Issues by Severity
- Critical (9-10): {N}
- High (7-8): {N}
- Medium (4-6): {N}
- Low (1-3): {N}

## Top 5 Issues
1. {Brief description with line number}
2. ...

## Detailed Findings
{Full issue entries}
```

---

## Reference Files

- [references/issue-patterns.md](references/issue-patterns.md) - Complete catalog of code smells and anti-patterns
- [references/brutal-phrasing.md](references/brutal-phrasing.md) - Direct language patterns for honest feedback
- [references/categorization.md](references/categorization.md) - Issue category taxonomy and severity weights
- [references/evidence-gathering.md](references/evidence-gathering.md) - Quantification techniques and measurement tools