Code Refactor

Overview

This skill guides systematic refactoring of one or more codebase components through three phases:

• Research - Deep analysis of component behavior, usage patterns, and dependencies

• Proposal - Concrete change suggestions with code samples and impact analysis

• Test Plan - Test strategy to validate changes without regressions

Each phase produces a markdown document, creating a complete refactoring proposal that can be reviewed before implementation.

Output Structure

Ask the user for an output directory (e.g., ./docs/refactoring/ or ./proposals/ ).

Create a subfolder for the refactoring effort:

{output-directory}/ └── {component-name}-refactor/ ├── 01-research.md # Phase 1: Component Analysis ├── 02-proposal.md # Phase 2: Change Proposals └── 03-test-plan.md # Phase 3: Test Strategy

For multi-component refactoring, create subfolders:

{output-directory}/ └── {thread-name}-refactor/ ├── component-a/ │ ├── 01-research.md │ ├── 02-proposal.md │ └── 03-test-plan.md └── component-b/ ├── 01-research.md ├── 02-proposal.md └── 03-test-plan.md

Phase 1: Research

Goal: Comprehensive understanding of the component before proposing changes.

For multi-component refactoring, use parallel Explore agents to research each component simultaneously, then synthesize findings.

Step 1: Component Inventory (Use /codebase-librarian)

Start with the /codebase-librarian skill to establish baseline understanding:

• Project foundation (language, tooling)

• Entry points relevant to the component

• Services the component interacts with

• Infrastructure dependencies

• Domain model elements involved

This provides context for the deeper component analysis that follows.

Step 2: Component Deep Dive

After the inventory, analyze the specific component(s) being refactored.

Understand how it works:

• Core responsibilities and behavior

• Internal structure and control flow

• State management and data transformations

• Error handling patterns

• Configuration and initialization

Understand instantiation and usage:

• Where is it instantiated?

• How many instantiation patterns exist?

• What parameters/dependencies are injected?

• Is it a singleton, factory-created, or ad-hoc?

Map the surroundings:

• Dependents (what calls this component)

• Dependencies (what this component calls)

• Before: What happens before invocation?

• After: What happens after invocation?

Step 3: Call Graph

Create a visual representation of the component's call relationships:

┌─────────────────────────────────────────────────────────────┐ │ CALLERS │ ├─────────────────────────────────────────────────────────────┤ │ OrderController.create() ──┐ │ │ OrderController.update() ──┼──► PaymentService │ │ CheckoutWorker.process() ──┘ │ └─────────────────────────────────────────────────────────────┘ │ ▼ ┌─────────────────────────────────────────────────────────────┐ │ PaymentService │ ├─────────────────────────────────────────────────────────────┤ │ processPayment() │ │ refundPayment() │ │ validateCard() │ └─────────────────────────────────────────────────────────────┘ │ ▼ ┌─────────────────────────────────────────────────────────────┐ │ DEPENDENCIES │ ├─────────────────────────────────────────────────────────────┤ │ StripeClient │ PaymentRepository │ │ FraudDetectionService │ EventEmitter │ └─────────────────────────────────────────────────────────────┘

Research Output Format

Write to 01-research.md :

Research: [Component Name]

Generated: [Date] Scope: [Component path/module]

Executive Summary

[2-3 paragraphs covering:]

• Component's purpose and role in the system
• Key findings about current implementation
• Areas of concern or complexity discovered

Component Analysis

Core Behavior

[What the component does, its responsibilities]

Internal Structure

[Classes, functions, modules that compose it]

State & Data Flow

[How data moves through the component]

Instantiation Patterns

Dependency Analysis

Dependents (Who calls this)

Dependencies (What this calls)

Execution Context

Before invocation: [What typically happens before this component is called] After invocation: [What happens with the result]

Call Graph

[ASCII diagram as shown above]

Key Observations

• [Notable pattern or concern 1]
• [Notable pattern or concern 2]
• [Areas that may need attention during refactoring]

Phase 2: Proposal

Persona: Pragmatic Senior Engineer

Mindset: Balance ideal architecture with practical constraints. Quick wins build momentum and trust. Large changes need clear justification. Every change should have a clear "why".

Proposal Structure

Order changes from quick wins to complex restructuring:

• Quick Wins - Small, low-risk improvements (hours)

• Medium Changes - Meaningful refactoring (days)

• Large Restructuring - Significant architectural changes (weeks)

For Each Change

Provide:

• Problem Statement - What's wrong and why it matters

• Proposed Solution - Concrete approach

• Code Samples - Before and after comparisons

• Impact Analysis - Files affected, risks, dependencies

• Pros and Cons - Honest trade-off assessment

Proposal Output Format

Write to 02-proposal.md :

Refactoring Proposal: [Component Name]

Generated: [Date] Based on: 01-research.md

Executive Summary

[Overview of proposed changes, expected benefits, and effort distribution]

Quick Wins

1. [Change Title]

Problem [What's wrong - reference research findings]

Solution [What to do]

Before

// src/services/payment.ts:45-52
class PaymentService {
  constructor() {
    this.stripe = new Stripe(process.env.STRIPE_KEY);
  }
}

After

// src/services/payment.ts:45-55
class PaymentService {
  constructor(private readonly paymentGateway: PaymentGateway) {}
}

// src/ports/payment-gateway.ts (new)
interface PaymentGateway {
  charge(amount: number, currency: string): Promise<ChargeResult>;
}

Impact

- Files modified: src/services/payment.ts
, src/composition-root.ts

- Files created: src/ports/payment-gateway.ts

- Risk: Low - constructor signature change requires updating instantiation sites

Pros

- Enables unit testing without Stripe sandbox

- Prepares for potential payment provider switch

Cons

- Adds indirection layer

- Requires updating 3 instantiation sites

Medium Changes

2. [Change Title]

[Same structure as above]

Large Restructuring

3. [Change Title]

[Same structure - may be higher level for larger efforts]

Implementation Order

Recommended sequence considering dependencies:

- [Quick Win 1] - No dependencies

- [Quick Win 2] - No dependencies

- [Medium Change 1] - Depends on Quick Win 1

- [Large Restructuring] - Depends on Medium Change 1

Risk Assessment

Change
Risk Level
Mitigation

[Change 1]
Low
Existing tests cover behavior

[Change 2]
Medium
Add integration tests first

[Change 3]
High
Feature flag, gradual rollout

---

## Phase 3: Test Plan

**Goal**: Design tests that validate refactoring without regressions.

### Analysis Steps

1. **Audit existing tests** - What's already covered?
2. **Identify gaps** - What behavior lacks test coverage?
3. **Design new tests** - Focus on integration over unit tests
4. **Minimize mocks** - Prefer real dependencies when feasible

### Test Philosophy

- **Favor integration tests** - Test real interactions between components
- **Use mocks sparingly** - Only for external services, not internal dependencies
- **Test behavior, not implementation** - Tests should survive refactoring
- **Cover edge cases discovered in research** - Use Phase 1 findings

### Test Plan Output Format

Write to `03-test-plan.md`:

```markdown
# Test Plan: [Component Name] Refactoring

**Generated**: [Date]
**Based on**: 01-research.md, 02-proposal.md

## Executive Summary

[Overview of testing strategy and coverage goals]

## Current Test Coverage

### Existing Tests

| Test File | Type | Coverage | Notes |
|-----------|------|----------|-------|
| `payment.test.ts` | Unit | Core payment flow | Heavy mocking |
| `checkout.integration.ts` | Integration | Happy path only | No error cases |

### Coverage Gaps

- [ ] Error handling in `processPayment()`
- [ ] Concurrent payment attempts
- [ ] Retry behavior after transient failures

## Test Strategy

### Integration Tests (Preferred)

Tests that exercise real component interactions:

**Test: Payment processing end-to-end**
```typescript
// tests/integration/payment.test.ts
describe('PaymentService', () => {
  let service: PaymentService;
  let testDb: TestDatabase;

  beforeEach(async () => {
    testDb = await TestDatabase.create();
    service = new PaymentService(
      new StripeTestGateway(),  // Test mode, real API
      new PaymentRepository(testDb)
    );
  });

  it('processes payment and persists record', async () => {
    const result = await service.processPayment({
      amount: 1000,
      currency: 'usd',
      customerId: 'cust_123'
    });

    expect(result.status).toBe('succeeded');

    const record = await testDb.payments.findById(result.id);
    expect(record).toBeDefined();
    expect(record.amount).toBe(1000);
  });
});

Why integration over unit: Tests real database interactions, actual service coordination, and catches integration bugs that mocks would hide.

Unit Tests (When Necessary)

Use unit tests with mocks only when:

- External API calls would be slow/costly

- Testing pure business logic in isolation

- Simulating error conditions difficult to reproduce

Test: Fraud detection logic

// tests/unit/fraud-detection.test.ts
describe('FraudDetector', () => {
  it('flags high-risk transactions', () => {
    const detector = new FraudDetector();

    const result = detector.assess({
      amount: 10000,
      newCustomer: true,
      internationalCard: true
    });

    expect(result.riskLevel).toBe('high');
    expect(result.requiresReview).toBe(true);
  });
});

Why unit here: Pure logic, no external dependencies, fast execution.

Tests Per Proposal Change

For Change 1: [Extract PaymentGateway Interface]

Test
Type
Purpose

gateway-contract.test.ts

Contract
Verify adapter implements interface correctly

payment-with-mock-gateway.test.ts

Unit
Verify service works with any gateway

New test:

// tests/contract/payment-gateway.test.ts
describe('PaymentGateway contract', () => {
  const adapters = [
    ['Stripe', () => new StripePaymentAdapter(stripeTestClient)],
    ['Mock', () => new MockPaymentAdapter()],
  ];

  test.each(adapters)('%s adapter implements contract', (name, createAdapter) => {
    const adapter = createAdapter();

    expect(adapter.charge).toBeDefined();
    expect(adapter.refund).toBeDefined();
  });
});

For Change 2: [Change Title]

[Similar structure]

Mock Usage Guidelines

Acceptable mocks:

- External payment APIs (Stripe, PayPal)

- Email services

- SMS/notification services

- Third-party APIs with rate limits

Avoid mocking:

- Internal services (use real implementations)

- Repositories (use test database)

- Domain logic (test directly)

Verification Checklist

Before considering refactoring complete:

-  All existing tests pass

-  New integration tests cover changed behavior

-  Edge cases from research are tested

-  Error handling paths have coverage

-  No increase in mock usage

-  CI pipeline green

---

## References

For detailed guidance on each phase:

- **Phase 1 deep dive**: See [references/research-phase.md](references/research-phase.md) for advanced research techniques
- **Phase 2 patterns**: See [references/proposal-phase.md](references/proposal-phase.md) for common refactoring patterns
- **Phase 3 testing**: See [references/test-plan-phase.md](references/test-plan-phase.md) for test design patterns