code-refactor（代码重构）

Code Refactor

Overview

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

1. Research - Deep analysis of component behavior, usage patterns, and dependencies
2. Proposal - Concrete change suggestions with code samples and impact analysis
3. 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

| Location | Pattern | Parameters | Frequency |
|----------|---------|------------|-----------|
| `src/api/orders.ts:45` | Direct new | config, logger | 3 places |
| `src/workers/checkout.ts:12` | Factory | config | 1 place |

## Dependency Analysis

### Dependents (Who calls this)

| Caller | Method | Context |
|--------|--------|---------|
| `OrderController` | `processPayment()` | HTTP request handling |
| `CheckoutWorker` | `processPayment()` | Background job |

### Dependencies (What this calls)

| Dependency | Usage | Abstracted? |
|------------|-------|-------------|
| `StripeClient` | Payment processing | No - direct SDK |
| `PaymentRepository` | Persistence | Yes - interface |

### 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:

1. Quick Wins - Small, low-risk improvements (hours)
2. Medium Changes - Meaningful refactoring (days)
3. Large Restructuring - Significant architectural changes (weeks)

For Each Change

Provide:

1. Problem Statement - What's wrong and why it matters
2. Proposed Solution - Concrete approach
3. Code Samples - Before and after comparisons
4. Impact Analysis - Files affected, risks, dependencies
5. 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:

1. [Quick Win 1] - No dependencies
2. [Quick Win 2] - No dependencies
3. [Medium Change 1] - Depends on Quick Win 1
4. [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:

# 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**

// 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 for advanced research techniques
• Phase 2 patterns: See references/proposal-phase.md for common refactoring patterns
• Phase 3 testing: See references/test-plan-phase.md for test design patterns