---

name: refactoring description: Use when user explicitly asks to refactor code, or when test coverage is requested for untested code with side effects. Enforces Functional Core Imperative Shell pattern extraction before any changes.

Refactoring with Functional Core, Imperative Shell

Extract pure functions before refactoring. Never refactor code that mixes logic with side effects.

When to Use

• User asks to "refactor", "consolidate", "extract", or "reduce duplication"
• Test coverage requested for hooks, components, or code with I/O
• Code review reveals logic buried in side-effect-heavy code

Core Pattern

Copy1. ANALYZE → Identify pure logic vs side effects
2. EXTRACT → Move pure logic to utils/ as pure functions
3. TEST → Write output-based tests for extracted functions
4. REFACTOR → Modify imperative shell (now thin wrapper)

Implementation

Step 1: Analyze Before Touching Code

Copy// IDENTIFY in existing code:
// - Pure logic (calculations, transformations, validations)
// - Side effects (API calls, state updates, localStorage, Date.now())

// Example: useCommentSuggestions.ts
// PURE: isCacheValid(entry, currentTime, ttl) → boolean
// IMPURE: localStorage.getItem(), Date.now(), queryClient.setQueryData()

Step 2: Extract Pure Functions

Copy// Before: Logic mixed with side effects
const loadFromCache = (key: string) => {
  const cached = localStorage.getItem(key);  // side effect
  if (!cached) return undefined;
  const entry = JSON.parse(cached);
  return Date.now() - entry.timestamp <= TTL ? entry.data : undefined;  // impure
};

// After: Pure function extracted
export const isCacheValid = (timestamp: number, currentTime: number, ttl: number): boolean =>
  currentTime - timestamp <= ttl;

// Imperative shell becomes thin
const loadFromCache = (key: string) => {
  const cached = localStorage.getItem(key);
  if (!cached) return undefined;
  const entry = JSON.parse(cached);
  return isCacheValid(entry.timestamp, Date.now(), TTL) ? entry.data : undefined;
};

Step 3: Test Only Pure Functions

Copydescribe('isCacheValid', () => {
  it('returns true when within TTL', () => {
    expect(isCacheValid(1000, 2000, 5000)).toBe(true);
  });

  it('returns false when TTL exceeded', () => {
    expect(isCacheValid(1000, 10000, 5000)).toBe(false);
  });
});

Common Mistakes

Red Flags

Stop and re-analyze if you find yourself:

• Writing vi.mock() for more than external APIs
• Testing a function that calls useState, useQuery, or Firebase
• Unable to test without renderHook() or QueryClientProvider