pre-commit-review

name	pre-commit-review
description	ADVISORY validation of code against design principles, accessibility, and best practices that linters cannot fully enforce. Use after linter passes and tests pass to validate design quality. Categorizes findings as Design Debt, Readability Debt, or Polish Opportunities. Does NOT block commits.

Pre-Commit Design Review (React/TypeScript)

ADVISORY validation of code against design principles, accessibility, and practices that linters cannot fully enforce. Categorizes findings as Design Debt, Readability Debt, or Polish Opportunities.

When to Use

• Automatically invoked by @linter-driven-development (Phase 4)
• Manually before committing (to validate design quality)
• After linter passes and tests pass

What This Reviews

• NOT code correctness (tests verify that)
• NOT syntax/style (ESLint/Prettier enforce that)
• YES design principles (primitive obsession, composition, architecture)
• YES maintainability (readability, complexity, testability)
• YES accessibility (semantic HTML, ARIA, keyboard nav)

Review Scope

Primary Scope: Changed code in commit

• All modified lines
• All new components/hooks
• Specific focus on design principle adherence
• Accessibility compliance

Secondary Scope: Context around changes

• Entire files containing modifications
• Flag patterns/issues outside commit scope
• Suggest broader refactoring opportunities

Finding Categories (Debt-Based)

🔴 Design Debt

Will cause pain when extending/modifying code

Violations:

• Primitive obsession: string IDs, unvalidated inputs, no branded types
• Wrong architecture: Technical layers instead of feature-based
• Prop drilling: State passed through 3+ component levels
• Tight coupling: Components tightly coupled to specific implementations
• Missing error boundaries: No error handling for async operations
• No type validation: Runtime data not validated (no Zod schemas)

Impact: Future changes will require more work and introduce bugs

🟡 Readability Debt

Makes code harder to understand and work with

Violations:

• Mixed abstractions: Business logic mixed with UI in same component
• Complex conditions: Deeply nested or complex boolean expressions
• Inline styles/logic: Complex logic directly in JSX
• Poor naming: Generic names (data, handler, manager, utils)
• God components: Components doing too many things
• Missing extraction: Logic that should be custom hooks

Impact: Team members (and AI) will struggle to understand intent

🟢 Polish Opportunities

Minor improvements for consistency and quality

Violations:

• Missing JSDoc: Complex types/hooks without documentation
• Accessibility enhancements: Could be more accessible (but not broken)
• Type improvements: Could use more specific types (vs any/unknown)
• Better naming: Non-idiomatic or unclear names
• Performance: Unnecessary rerenders, missing memoization
• Bundle size: Unused dependencies, large imports

Impact: Low, but improves codebase quality

Review Workflow

0. Architecture Pattern Validation (FIRST CHECK)

Expected: Consistent architecture. Design Debt (ADVISORY) - never blocks commit.

Check file patterns:

• src/{components,hooks,contexts}/ → ✅ Layer-based (most common)
• src/features/[feature]/{components,hooks,context}/ → ✅ Feature-based
• Mixed patterns → ✅ Hybrid (if intentional)
• Inconsistent patterns → 🔴 Design Debt

Advisory Categories:

1. ✅ Consistent architecture → Acknowledge pattern, ensure new code follows it
2. 🟢 Hybrid with clear boundaries → Validate shared vs feature-specific distinction is clear
3. 🔴 Inconsistent patterns (advisory) → Suggest establishing clear conventions

Report Template:

🟢 Architecture Review: Layer-Based Pattern
- Current: Code organized by technical layer (components/, hooks/, contexts/)
- Status: Consistent with existing codebase ✅
- New code follows established pattern ✅

Or if inconsistent:

🔴 Design Debt (Advisory): Inconsistent Architecture
- Issue: Mixed patterns without clear conventions
- Examples: Some auth code in src/components/, other auth code scattered
- Suggestion: Document architecture decisions and apply consistently
- Alternative: Proceed as-is (address in future refactor)

Always acknowledge: Consistency with existing codebase is the priority.

---

1. Analyze Commit Scope

# Identify what changed
git diff --name-only

# See actual changes
git diff

2. Review Design Principles

Check for each principle in changed code:

Primitive Obsession

Look for:

• String types for domain concepts (email, userId, etc.)
• Numbers without validation (age, price, quantity)
• Booleans representing state (use discriminated unions)

Example violation:

// 🔴 Design Debt
interface User {
  id: string  // What if empty? Not UUID?
  email: string  // What if invalid?
}

// ✅ Better
type UserId = Brand<string, 'UserId'>
const EmailSchema = z.string().email()

Component Composition

Look for:

• Prop drilling (state passed through 3+ levels)
• Giant components (>200 lines)
• Mixed UI and business logic
• Inline complex logic in JSX

Example violation:

// 🔴 Design Debt: Prop drilling
<Parent>
  <Middle user={user} onUpdate={onUpdate}>
    <Deep user={user} onUpdate={onUpdate}>
      <VeryDeep user={user} onUpdate={onUpdate} />
    </Deep>
  </Middle>
</Parent>

// ✅ Better: Use context or composition
<UserProvider>
  <Parent>
    <Middle><Deep><VeryDeep /></Deep></Middle>
  </Parent>
</UserProvider>

Custom Hooks

Look for:

• Complex logic in components (should be in hooks)
• Duplicated logic across components
• useEffect with complex dependencies

Example violation:

// 🟡 Readability Debt: Logic in component
function UserProfile() {
  const [user, setUser] = useState(null)
  const [loading, setLoading] = useState(false)
  useEffect(() => {
    // 50 lines of fetch logic
  }, [])
  // More logic...
}

// ✅ Better: Extract to hook
function useUser(id) { /* fetch logic */ }
function UserProfile() {
  const { user, loading } = useUser(userId)
  return <UI user={user} loading={loading} />
}

3. Review Accessibility

Check for each component (jsx-a11y rules + manual review):

Semantic HTML

• Using correct HTML elements (, , , )
• Proper heading hierarchy (h1 → h2 → h3, no skipping)
• Lists for list content (
  ,
  )

  Example violations:

  // 🔴 Design Debt: Non-semantic
  <div onClick={handleClick}>Click me</div>  // Should be <button>
  
  // 🟡 Readability Debt: Wrong heading order
  <h1>Title</h1>
  <h3>Subtitle</h3>  // Skipped h2
  
  // ✅ Better
  <button onClick={handleClick}>Click me</button>
  <h1>Title</h1>
  <h2>Subtitle</h2>
  

  ARIA Attributes

  • Form inputs have labels
  • Interactive elements have accessible names
  • Images have alt text
  • Dialogs have proper roles and labels

  Example violations:

  // 🔴 Design Debt: Missing label
  <input type="text" placeholder="Email" />
  
  // 🟢 Polish: Could improve alt text
  <img src="avatar.jpg" alt="image" />  // Generic