qa-refactoring

어떤 스킬인가요?

Safe refactoring with behavior preservation. Use when reducing technical debt, planning codemods, applying strangler migrations, or tightening CI guardrails around risky changes.

설치 방법

프로젝트 폴더에서 아래 명령어를 실행하고, 설치할 에이전트를 선택하세요.

설치 명령을 직접 실행해야 적용됩니다. 지원 에이전트와 필요한 권한·라이선스는 제작자의 안내를 확인하세요.

스킬 원문 파일

interface:
  display_name: "Refactoring QA"
  short_description: "Safe refactoring with behavior preservation"
  default_prompt: "Use $qa-refactoring for Safe refactoring with behavior preservation. Use when reducing technical debt, planning codemods, applying strangler migrations, or tightening CI guardrails around risky changes."

# Code Review Quality Checklist

Copy-paste checklist for reviewing code quality, maintainability, and technical debt.

---

## Quick Review (5 minutes)

Fast pass for obvious issues:

- [ ] **Linter passes** - no warnings
- [ ] **Tests pass** - all green
- [ ] **Builds successfully** - no errors
- [ ] **No debug code** - console.log, debugger, print statements removed
- [ ] **No commented-out code**
- [ ] **No TODOs** for critical functionality
- [ ] **Formatting consistent** - follows project style

---

## Comprehensive Review (30 minutes)

### Code Smells

#### Bloaters

- [ ] **No long methods** (>20 lines)
  - If found: Request Extract Method refactoring
- [ ] **No large classes** (>300 lines)
  - If found: Request Extract Class refactoring
- [ ] **No long parameter lists** (>3 parameters)
  - If found: Suggest Introduce Parameter Object
- [ ] **No primitive obsession** (string/number for domain concepts)
  - If found: Suggest value objects (Email, Money, etc.)
- [ ] **No data clumps** (same group of variables repeated)
  - If found: Suggest Extract Class

#### Object-Orientation Abusers

- [ ] **No switch statements on type codes**
  - If found: Suggest Replace Conditional with Polymorphism
- [ ] **No temporary fields** (fields only used sometimes)
  - If found: Suggest Extract Class or Remove Field
- [ ] **No refused bequest** (subclass not using parent's methods)
  - If found: Suggest Replace Inheritance with Delegation

#### Change Preventers

- [ ] **No divergent change** (class changed for many reasons)
  - If found: Violates Single Responsibility Principle
- [ ] **No shotgun surgery** (change requires updates in many classes)
  - If found: Suggest Move Method or Inline Class

#### Dispensables

- [ ] **No unnecessary comments** (code should be self-explanatory)
- [ ] **No duplicate code**
  - If found: Suggest Extract Method or Extract Class
- [ ] **No lazy classes** (classes doing too little)
  - If found: Suggest Inline Class or Remove Class
- [ ] **No dead code** (unused variables, methods, classes)
  - If found: Request removal
- [ ] **No speculative generality** ("in case we need it")
  - If found: Suggest YAGNI, remove unused abstractions

#### Couplers

- [ ] **No feature envy** (method uses more of another class)
  - If found: Suggest Move Method
- [ ] **No inappropriate intimacy** (classes too dependent)
  - If found: Suggest Extract Class or Hide Delegate
- [ ] **No message chains** (a.getB().getC().getD())
  - If found: Violates Law of Demeter, suggest Hide Delegate
- [ ] **No middle man** (class just delegates to another)
  - If found: Suggest Remove Middle Man or Inline Class

---

### Code Quality Metrics

#### Complexity

- [ ] **Cyclomatic complexity** < 10 per method
  - Tool: ESLint `complexity` rule, SonarQube
  - If >10: Request simplification
- [ ] **Cognitive complexity** < 15 per method
  - Measures understandability
  - If >15: Hard to understand, request refactoring
- [ ] **Nesting depth** < 3 levels
  - If deeper: Use guard clauses or Extract Method

#### Size Metrics

- [ ] **Method length** < 20 lines
  - If longer: Suggest Extract Method
- [ ] **Class length** < 300 lines
  - If longer: Suggest Extract Class
- [ ] **File length** < 500 lines
  - If longer: Consider splitting into modules
- [ ] **Line length** < 120 characters
  - If longer: Break into multiple lines

#### Maintainability

- [ ] **No magic numbers** - all constants named
  ```javascript
  // BAD: Bad
  if (age > 18) { /* ... */ }

  // GOOD: Good
  const LEGAL_ADULT_AGE = 18;
  if (age > LEGAL_ADULT_AGE) { /* ... */ }
  ```
- [ ] **Meaningful names** - variables, methods, classes
  ```javascript
  // BAD: Bad
  const x = users.filter(u => u.a > 18);

  // GOOD: Good
  const adultUsers = users.filter(user => user.age > 18);
  ```
- [ ] **No abbreviations** unless well-known (e.g., URL, HTTP)
- [ ] **Consistent naming** across codebase

---

### Design Principles

#### SOLID Principles

- [ ] **Single Responsibility Principle**
  - Each class/method has one reason to change
  - If violated: Class does too much, suggest splitting

- [ ] **Open/Closed Principle**
  - Open for extension, closed for modification
  - If violated: Suggest strategy pattern or inheritance

- [ ] **Liskov Substitution Principle**
  - Subtypes should be substitutable for base types
  - If violated: Check subclass overrides

- [ ] **Interface Segregation Principle**
  - Many specific interfaces > one general interface
  - If violated: Clients forced to depend on unused methods

- [ ] **Dependency Inversion Principle**
  - Depend on abstractions, not concretions
  - If violated: Hard-coded dependencies, suggest injection

#### DRY (Don't Repeat Yourself)

- [ ] **No duplicate code** in same file
- [ ] **No duplicate code** across files
- [ ] **No duplicate logic** with slight variations
  - If found: Extract shared logic, parameterize differences

#### YAGNI (You Aren't Gonna Need It)

- [ ] **No premature abstractions**
- [ ] **No unused parameters** "for future use"
- [ ] **No overly complex designs** for simple problems
- [ ] **No feature flags** for features not coming

#### KISS (Keep It Simple, Stupid)

- [ ] **Simplest solution** that works
- [ ] **No unnecessary complexity**
- [ ] **No over-engineering**

---

### Testing

#### Test Coverage

- [ ] **New code has tests**
  - [ ] Unit tests for business logic
  - [ ] Integration tests for external dependencies
  - [ ] E2E tests for critical user flows

- [ ] **Test coverage** > 80% for new code
  - Critical paths: 100%
  - Business logic: 90%+
  - Overall: 80%+

#### Test Quality

- [ ] **Tests are independent** - can run in any order
- [ ] **Tests are deterministic** - same result every time
- [ ] **Tests are fast** - unit tests < 1s each
- [ ] **Tests have clear names** - describe what they test
  ```javascript
  // BAD: Bad
  it('test1', () => { /* ... */ });

  // GOOD: Good
  it('should return 400 when email is invalid', () => { /* ... */ });
  ```
- [ ] **Tests use AAA pattern** - Arrange, Act, Assert
- [ ] **No test duplication** - use helper functions
- [ ] **Mocks used appropriately** - only for external dependencies

---

### Security

#### Common Vulnerabilities

- [ ] **No SQL injection** - use parameterized queries
  ```javascript
  // BAD: Bad
  db.query(`SELECT * FROM users WHERE id = ${userId}`);

  // GOOD: Good
  db.query('SELECT * FROM users WHERE id = ?', [userId]);
  ```

- [ ] **No XSS vulnerabilities** - escape user input
  ```javascript
  // BAD: Bad
  element.innerHTML = userInput;

  // GOOD: Good
  element.textContent = userInput;
  // or use DOMPurify for HTML
  ```

- [ ] **No command injection** - validate inputs
- [ ] **No hardcoded secrets** - use environment variables
- [ ] **No sensitive data in logs**
- [ ] **No weak crypto** - use industry standards (AES-256, bcrypt)
- [ ] **Input validation** on all user inputs
- [ ] **Authentication/authorization** implemented correctly

---

### Performance

#### Potential Issues

- [ ] **No N+1 queries** - use eager loading
  ```javascript
  // BAD: Bad
  const users = await User.findAll();
  for (const user of users) {
    user.orders = await Order.findByUserId(user.id); // N queries
  }

  // GOOD: Good
  const users = await User.findAll({ include: [Order] }); // 1 query
  ```

- [ ] **No unnecessary database calls** - cache if appropriate
- [ ] **No memory leaks** - clean up listeners, intervals
- [ ] **Efficient algorithms** - not O(n²) when O(n) possible
- [ ] **No blocking operations** in async code
- [ ] **Proper indexing** for database queries

---

### Error Handling

- [ ] **Errors handled appropriately**
  - Don't swallow errors silently
  - Log errors with context
  - Return meaningful error messages

- [ ] **No catch-all handlers** without re-throwing
  ```javascript
  // BAD: Bad
  try {
    await doSomething();
  } catch (error) {
    console.log(error); // Swallowed!
  }

  // GOOD: Good
  try {
    await doSomething();
  } catch (error) {
    logger.error('Failed to do something', { error });
    throw new ApplicationError('Operation failed', error);
  }
  ```

- [ ] **Specific error types** - not generic Error
- [ ] **Error messages are user-friendly** (for user-facing errors)
- [ ] **Stack traces preserved** when re-throwing

---

### Documentation

- [ ] **Public APIs documented** - JSDoc, TSDoc, etc.
  ```typescript
  /**
   * Calculates user's total order value.
   *
   * @param userId - The user's unique identifier
   * @param startDate - Filter orders from this date
   * @param endDate - Filter orders until this date
   * @returns Total order value in cents
   * @throws {UserNotFoundError} If user doesn't exist
   */
  async function calculateTotalOrders(
    userId: string,
    startDate: Date,
    endDate: Date
  ): Promise<number> {
    // ...
  }
  ```

- [ ] **Complex logic explained** - why, not what
  ```javascript
  // BAD: Bad
  // Multiply by 1.1
  const price = basePrice * 1.1;

  // GOOD: Good
  // Apply 10% VAT as required by EU regulations
  const VAT_RATE = 1.1;
  const price = basePrice * VAT_RATE;
  ```

- [ ] **README updated** if architecture changed
- [ ] **Breaking changes documented**

---

## Automated Checks

Use these tools to automate quality checks:

### Linters

- [ ] **ESLint** (JavaScript/TypeScript)
  ```json
  {
    "extends": ["eslint:recommended"],
    "rules": {
      "complexity": ["error", 10],
      "max-lines": ["error", 300],
      "max-lines-per-function": ["error", 20],
      "max-params": ["error", 3],
      "max-depth": ["error", 3]
    }
  }
  ```

- [ ] **Pylint** (Python)
- [ ] **RuboCop** (Ruby)
- [ ] **Clippy** (Rust)

### Code Quality Tools

- [ ] **SonarQube** - comprehensive analysis
- [ ] **CodeClimate** - maintainability metrics
- [ ] **Embold** - anti-pattern detection

### Security Scanners

- [ ] **npm audit** / **yarn audit** (JavaScript)
- [ ] **Snyk** - dependency vulnerabilities
- [ ] **OWASP Dependency-Check**

---

## Review Comments Template

### For Code Smells

```
**Code Smell: Long Method**

This method is 50 lines long, which makes it hard to understand and test.

Suggestion: Extract the validation logic into a separate `validateInput()` method and the calculation logic into `calculateTotal()`.

References:
- [Refactoring: Extract Method](https://refactoring.guru/extract-method)
- See references/refactoring-catalog.md
```

### For Complexity Issues

```
**High Complexity: 15**

This method has cyclomatic complexity of 15, which is above our threshold of 10.

Suggestion: Break down the nested conditionals using guard clauses, or use the Strategy pattern if this is polymorphic behavior.

Tool output:
```
eslint: complexity: Method 'processOrder' has complexity 15 (max 10)
```
```

### For Missing Tests

```
**Missing Test Coverage**

The new `PaymentProcessor` class has 0% test coverage.

Suggestion: Add unit tests covering:
- [ ] Happy path (successful payment)
- [ ] Error handling (failed payment)
- [ ] Edge cases (zero amount, negative amount)

Target coverage: 80%+
```

---

## Approval Criteria

Code is approved when ALL of these are true:

- [ ] **No critical issues** (security, bugs)
- [ ] **All automated checks pass** (linter, tests, build)
- [ ] **No major code smells** (God objects, high complexity)
- [ ] **Test coverage sufficient** (>80% for new code)
- [ ] **Documentation adequate**
- [ ] **Performance acceptable**
- [ ] **Follows team conventions**

---

## Technical Debt Assessment

If code has quality issues but must be merged:

### Document Technical Debt

```
**Technical Debt Created**

Issue: UserService class is 450 lines (>300 line limit)

Reason: Time-sensitive feature needed for demo

Impact: High (difficult to maintain)

Effort to fix: 1 day

Plan: Refactor in sprint 23 (TD-042)

Priority: P1 (high impact, low effort)
```

### Track in Debt Register

Add to technical debt register:
- ID: TD-XXX
- Description: Issue summary
- Type: Reckless/Prudent, Deliberate/Inadvertent
- Impact: High/Medium/Low
- Effort: Days to fix
- Priority: P1/P2/P3/P4
- Owner: Who will fix it
- Target sprint: When it will be addressed

---

## Summary Template

```
## Code Review Summary

**Overall**: [Approve / Request Changes / Reject]

**Positives**:
- [check] Good test coverage (85%)
- [check] Clean separation of concerns
- [check] Clear naming conventions

**Issues Found**:
- [FAIL] 3 methods exceed complexity threshold
- [WARNING] 1 missing error handler
- [WARNING] 2 minor code smells

**Action Items**:
1. Reduce complexity in `processOrder()` (Priority: High)
2. Add error handling in `validateInput()` (Priority: High)
3. Extract duplicated validation logic (Priority: Low)

**Technical Debt**:
- None created [check]

**Estimated Fix Time**: 2 hours
```

# Refactor Safety Checklist (Characterization + Incremental Steps)

Copy-paste checklist for safe refactoring that preserves behavior and reduces regression risk.

## Core

## Pre-Refactoring Checklist

Before starting any refactoring:

- [ ] **Safety net exists** for code being refactored
  - [ ] If tests already exist: identify which tests guard the behavior
  - [ ] If tests are missing: add characterization tests for current behavior (see `references/characterization-testing.md`)
  - [ ] Flaky tests addressed (fix or quarantine with owner + expiry)
- [ ] **Version control** is up to date
  - [ ] All changes committed
  - [ ] Working on feature branch
  - [ ] Branch is up to date with main
- [ ] **Baseline metrics** recorded
  - [ ] Current lines of code
  - [ ] Cyclomatic complexity
  - [ ] Code coverage percentage
  - [ ] SonarQube debt ratio (if available)
- [ ] **Refactoring scope** is defined
  - [ ] Specific files/classes identified
  - [ ] Clear goal stated (e.g., "reduce complexity")
  - [ ] Time-boxed (e.g., "2 hours max")
- [ ] **PR plan** is safe
  - [ ] Refactor-only PR (no feature changes mixed in)
  - [ ] Rollback strategy defined (revertable commits)
- [ ] **Stakeholders informed**
  - [ ] Team aware of refactoring session
  - [ ] No conflicting work on same files

---

## During Refactoring Checklist

### Every 15-30 Minutes

- [ ] **Run tests** after each small change
- [ ] **Commit** working code with descriptive message
- [ ] **Verify** behavior hasn't changed
  - [ ] No new failing tests
  - [ ] No performance degradation
  - [ ] Same output for same input

### Code Quality Checks

#### Method-Level Refactoring

- [ ] **Method length** < 20 lines
- [ ] **Method complexity** < 10 (cyclomatic)
- [ ] **Single responsibility** - method does one thing
- [ ] **Meaningful name** - describes what it does
- [ ] **Parameters** < 4 (use parameter objects if more)
- [ ] **No side effects** unless clearly named (e.g., `saveAndNotify`)
- [ ] **Comments removed** if code is self-explanatory
- [ ] **Magic numbers** extracted to named constants

#### Class-Level Refactoring

- [ ] **Class length** < 300 lines
- [ ] **Single responsibility** - one reason to change
- [ ] **Low coupling** - minimal dependencies on other classes
- [ ] **High cohesion** - related functionality grouped
- [ ] **Meaningful name** - describes purpose
- [ ] **No God objects** - not doing too much
- [ ] **Fields encapsulated** - private with getters/setters if needed
- [ ] **No duplicate code** within class

#### Code Smell Removal

- [ ] **Duplicate code** extracted to shared method
- [ ] **Long parameter lists** replaced with parameter objects
- [ ] **Large classes** split into focused classes
- [ ] **Switch statements** replaced with polymorphism (if appropriate)
- [ ] **Primitive obsession** replaced with value objects
- [ ] **Feature envy** fixed by moving method to proper class
- [ ] **Temporary fields** eliminated
- [ ] **Dead code** removed

---

## Post-Refactoring Checklist

### Verification

- [ ] **All tests pass**
  - [ ] Unit tests: [check]
  - [ ] Integration tests: [check]
  - [ ] E2E tests: [check]
- [ ] **Code coverage maintained or improved**
  - [ ] Before: ____%
  - [ ] After: ____%
- [ ] **Performance unchanged or improved**
  - [ ] Run performance benchmarks
  - [ ] Check memory usage
  - [ ] Verify response times
- [ ] **Linter passes** with no new warnings
- [ ] **Type checker passes** (if applicable)

### Metrics Improvement

- [ ] **Complexity reduced**
  - [ ] Before: _____
  - [ ] After: _____
- [ ] **Lines of code** (should decrease or stay same)
  - [ ] Before: _____
  - [ ] After: _____
- [ ] **Code duplication** reduced
  - [ ] Before: ____%
  - [ ] After: ____%
- [ ] **Technical debt** reduced (SonarQube)
  - [ ] Before: _____
  - [ ] After: _____

### Documentation

- [ ] **Commit message** describes refactoring
  - Example: "Refactor UserService: extract validation logic, reduce complexity from 25 to 8"
- [ ] **PR description** explains changes
  - Why refactoring was needed
  - What changed
  - Metrics before/after
- [ ] **Code comments** updated if needed
- [ ] **README** updated if architecture changed
- [ ] **Technical debt register** updated

### Code Review Preparation

- [ ] **Self-review** completed
  - [ ] Check diff for unintended changes
  - [ ] Verify no debug code left
  - [ ] Ensure consistent formatting
- [ ] **Tests demonstrate** refactoring didn't break functionality
- [ ] **Screenshots/metrics** show improvement
- [ ] **Reviewers assigned**

---

## Specific Refactoring Patterns

### Extract Method

- [ ] Identified code block that can be grouped
- [ ] Created method with descriptive name
- [ ] Moved code to new method
- [ ] Replaced original code with method call
- [ ] Verified tests still pass
- [ ] No duplicate code created

### Rename Variable/Method/Class

- [ ] New name is more descriptive
- [ ] New name follows naming conventions
- [ ] All references updated
- [ ] Tests still pass
- [ ] Documentation updated

### Extract Class

- [ ] Identified cohesive group of methods/fields
- [ ] Created new class with clear responsibility
- [ ] Moved methods/fields to new class
- [ ] Updated original class to use new class
- [ ] Tests still pass
- [ ] Both classes have single responsibility

### Replace Conditional with Polymorphism

- [ ] Identified type-based conditional logic
- [ ] Created interface or abstract class
- [ ] Created concrete subclasses for each type
- [ ] Moved type-specific logic to subclasses
- [ ] Replaced conditional with polymorphic call
- [ ] Tests still pass

### Introduce Parameter Object

- [ ] Identified related parameters (3+)
- [ ] Created parameter object class
- [ ] Updated method signature
- [ ] Updated all call sites
- [ ] Tests still pass
- [ ] Code is more readable

---

## Emergency Rollback Checklist

If refactoring causes issues:

- [ ] **Stop immediately** - don't add more changes
- [ ] **Identify issue**
  - Which test is failing?
  - What behavior changed?
- [ ] **Options**:
  - [ ] Quick fix (< 15 minutes)
  - [ ] Revert last commit
  - [ ] Revert entire refactoring branch
- [ ] **After rollback**:
  - [ ] Understand what went wrong
  - [ ] Plan safer approach
  - [ ] Add more tests before trying again

---

## Boy Scout Rule Checklist

When touching existing code (not dedicated refactoring session):

- [ ] Leave code **better than you found it**
- [ ] Fix at least **one code smell**
- [ ] Add at least **one test** if missing
- [ ] Improve at least **one variable name**
- [ ] Extract at least **one magic number** to constant
- [ ] Remove at least **one comment** by making code self-explanatory
- [ ] Changes are **small and safe**
- [ ] Changes **don't delay** feature delivery

---

## Optional: AI / Automation

Do:
- Use AI to propose mechanical refactors (rename/extract/move) and lint fixes; verify behavior with tests and contracts.
- Use AI to summarize diffs and highlight risky areas; validate by running characterization and integration tests.

Avoid:
- Accepting AI refactors that change behavior without explicit requirements and regression tests.
- Letting AI "fix CI" by weakening assertions or deleting tests.

## Team Refactoring Session Checklist

For organized team refactoring days:

### Before Session

- [ ] **Goal identified** (e.g., "reduce UserService complexity")
- [ ] **Time allocated** (e.g., "4-hour session")
- [ ] **Team available** - no meetings scheduled
- [ ] **Branch created** for refactoring
- [ ] **Baseline metrics** captured
- [ ] **Areas prioritized** by impact

### During Session

- [ ] **Pair/mob programming** - not solo refactoring
- [ ] **Small commits** every 15-30 minutes
- [ ] **Tests run** after each commit
- [ ] **Progress tracked** on board
- [ ] **Breaks taken** every 90 minutes

### After Session

- [ ] **Metrics compared** to baseline
- [ ] **PR created** with before/after stats
- [ ] **Team demo** of improvements
- [ ] **Retrospective** - what worked, what didn't
- [ ] **Next session planned** if needed

---

## Refactoring Safety Levels

Use this to assess risk:

### Level 1: Safe (No Tests Required)

- Rename variable (IDE refactoring)
- Extract constant
- Reorder method parameters (with IDE)
- Format code

### Level 2: Low Risk (Basic Tests)

- Extract method
- Inline variable
- Rename method/class (with IDE)
- Add parameter

### Level 3: Medium Risk (Good Test Coverage)

- Move method to another class
- Extract class
- Split conditional
- Replace conditional with polymorphism

### Level 4: High Risk (Extensive Tests Required)

- Change class hierarchy
- Modify algorithm
- Change data structure
- Refactor across multiple files

**Rule**: Never attempt Level 3-4 refactoring without 80%+ test coverage.

---

## Quick Refactoring Wins Checklist

15-minute improvements anyone can do:

- [ ] Remove unused imports
- [ ] Remove commented-out code
- [ ] Fix spelling in variable names
- [ ] Extract magic numbers to constants
- [ ] Add missing braces to single-line conditionals
- [ ] Break long lines (>120 characters)
- [ ] Add whitespace for readability
- [ ] Remove unnecessary else after return
- [ ] Replace var with const/let (JavaScript)
- [ ] Add missing error handling

---

## Summary Checklist

Before marking refactoring as complete:

- [ ] All tests pass [check]
- [ ] Code coverage maintained/improved [check]
- [ ] Metrics improved [check]
- [ ] Code is more readable [check]
- [ ] No new bugs introduced [check]
- [ ] Team reviewed and approved [check]
- [ ] Documentation updated [check]
- [ ] Committed and pushed [check]

**Time spent**: _____ hours
**Value delivered**: [Improved maintainability / Reduced complexity / Enabled feature X]

---

## Template for Refactoring Commit Message

```
Refactor [Component]: [Brief description]

What changed:
- [Change 1]
- [Change 2]
- [Change 3]

Why:
- [Reason 1]
- [Reason 2]

Metrics:
- Complexity: [Before] → [After]
- Lines of code: [Before] → [After]
- Test coverage: [Before] → [After]

Tests: All passing [check]
```

Example:
```
Refactor UserService: Extract validation and reduce complexity

What changed:
- Extracted email validation to EmailValidator class
- Extracted password validation to PasswordValidator class
- Split UserService into UserService and UserRepository
- Reduced method lengths from 50+ to < 20 lines

Why:
- UserService was 800 lines (God object)
- Cyclomatic complexity was 35 (very high risk)
- Mixed concerns (validation, persistence, business logic)

Metrics:
- Complexity: 35 → 8
- Lines of code: 800 → 250
- Test coverage: 45% → 82%

Tests: All passing [check]
```

// ESLint flat-config starter for refactoring safety.
// Copy to your project root as eslint.config.js and tune thresholds per repo.

const js = require("@eslint/js");
const globals = require("globals");
const tsParser = require("@typescript-eslint/parser");
const tsPlugin = require("@typescript-eslint/eslint-plugin");

module.exports = [
  {
    ignores: [
      "**/dist/**",
      "**/build/**",
      "**/coverage/**",
      "**/node_modules/**",
    ],
  },
  js.configs.recommended,
  {
    files: ["**/*.{js,cjs,mjs,ts,tsx}"],
    languageOptions: {
      ecmaVersion: "latest",
      sourceType: "module",
      globals: {
        ...globals.node,
        ...globals.es2024,
      },
      parser: tsParser,
      parserOptions: {
        ecmaFeatures: {
          jsx: true,
        },
      },
    },
    plugins: {
      "@typescript-eslint": tsPlugin,
    },
    rules: {
      // Safety rules for behavior-preserving refactors.
      complexity: ["warn", 12],
      "max-depth": ["warn", 4],
      "max-lines-per-function": [
        "warn",
        {
          max: 80,
          skipBlankLines: true,
          skipComments: true,
        },
      ],
      "max-params": ["warn", 4],
      "no-duplicate-imports": "error",
      "no-else-return": ["warn", { allowElseIf: false }],
      "no-empty": ["error", { allowEmptyCatch: false }],
      "no-empty-function": "error",
      "no-implicit-coercion": "warn",
      "no-lonely-if": "warn",
      "no-magic-numbers": [
        "off",
        {
          ignore: [-1, 0, 1, 2],
          ignoreArrayIndexes: true,
          enforceConst: true,
        },
      ],
      "no-negated-condition": "off",
      "no-nested-ternary": "warn",
      "no-param-reassign": "error",
      "no-shadow": "off",
      "no-unused-vars": [
        "off",
        {
          argsIgnorePattern: "^_",
          varsIgnorePattern: "^_",
        },
      ],
      "no-useless-return": "warn",
      "prefer-const": "error",
      "prefer-template": "warn",
      "require-await": "off",
      eqeqeq: ["error", "always"],

      "@typescript-eslint/no-explicit-any": "warn",
      "@typescript-eslint/no-floating-promises": "error",
      "@typescript-eslint/no-shadow": "error",
      "@typescript-eslint/no-unused-vars": [
        "error",
        {
          argsIgnorePattern: "^_",
          varsIgnorePattern: "^_",
        },
      ],
      "@typescript-eslint/prefer-nullish-coalescing": "warn",
      "@typescript-eslint/prefer-optional-chain": "warn",
    },
  },
  {
    files: ["**/*.{test,spec}.{js,ts,tsx}"],
    rules: {
      complexity: "off",
      "max-lines-per-function": "off",
      "max-params": "off",
    },
  },
];

/*
Install:
  npm install --save-dev eslint @eslint/js globals @typescript-eslint/parser @typescript-eslint/eslint-plugin

Suggested package.json scripts:
  {
    "scripts": {
      "lint": "eslint .",
      "lint:fix": "eslint . --fix"
    }
  }

Notes:
  - Keep formatting in Prettier (or the formatter your repo already uses).
  - Treat thresholds as starting heuristics, not universal refactor laws.
  - Add type-aware rules only after wiring project-specific tsconfig settings.
*/

# SonarQube Setup Guide

Complete guide to setting up SonarQube for code quality and technical debt management.

**Updated**: June 2026
**Target Docs**: SonarQube Server 2026.1 LTA and SonarQube Cloud

---

## Overview

SonarQube is an open-source platform for continuous inspection of code quality. It performs automatic reviews with static analysis to detect:

- Bugs
- Code smells
- Security vulnerabilities
- Technical debt
- Code coverage gaps

Current highlights (2026.1 LTA):

- **Sonar way for AI Code**: Built-in quality gate qualified for AI Code Assurance
- **Zero New Issues**: Stricter enforcement option to ensure no new issues enter codebase
- **Fudge Factor**: Relaxed conditions for small changes (<20 lines) to avoid over-enforcement
- **Quality Gate Recommendations**: Automatic suggestions when better configurations exist
- **MCP Server Integration**: AI agents (Claude Code, Cursor, Windsurf) can query issues and quality status via MCP
- **50% faster analysis** for JavaScript, TypeScript, Python, and Kotlin vs. 2025.x
- **Rust and Swift 6.2 full support**; C#14 / .NET 10 / Java 24 / Python 3.14 language support

---

## Installation Options

### Option 1: SonarQube Cloud (Hosted - Easiest)

**Best for**: Small teams, fast setup, and managed hosting

**Setup**:
1. Go to https://sonarcloud.io
2. Sign in with GitHub/Bitbucket/Azure DevOps
3. Import your repository
4. Follow integration steps below

---

### Option 2: Docker (Local Development)

**Best for**: Local development, private projects

```bash
# Start SonarQube locally.
# Pin an explicit supported image tag for your edition/version.
docker run -d --name sonarqube \
  -p 9000:9000 \
  -e SONAR_ES_BOOTSTRAP_CHECKS_DISABLE=true \
  sonarqube:<supported-tag>

# Wait 2-3 minutes for startup
# Access at http://localhost:9000
# Default credentials: admin/admin (change immediately)
```

---

### Option 3: Server Installation

**Best for**: Enterprise, on-premise deployment

1. **Requirements**:
   - Java 17 or 21
   - PostgreSQL, Microsoft SQL Server, or Oracle
   - 2GB RAM minimum (4GB recommended)

2. **Download**:
   ```bash
   # Download the current SonarQube Server ZIP for your edition/version
   # from the official install docs, then unzip it.
   wget <current-sonarqube-server-zip-url>
   unzip <current-sonarqube-server-zip>
   cd <current-sonarqube-server-dir>
   ```

3. **Configure Database** (conf/sonar.properties):
   ```properties
   sonar.jdbc.username=sonarqube
   sonar.jdbc.password=mypassword
   sonar.jdbc.url=jdbc:postgresql://localhost/sonarqube
   ```

4. **Start Server**:
   ```bash
   bin/linux-x86-64/sonar.sh start
   ```

5. **Access**: http://localhost:9000

---

## Project Configuration

### Step 1: Create Project

**In SonarQube UI**:
1. Click "Create Project"
2. Enter project key (e.g., `my-company_my-app`)
3. Set project name and visibility
4. Generate token (save it!)

---

### Step 2: Configure Project Properties

Create `sonar-project.properties` in project root:

```properties
# ============================================
# PROJECT IDENTIFICATION
# ============================================

sonar.projectKey=my-company_my-app
sonar.projectName=My Application
sonar.projectVersion=1.0.0

# ============================================
# SOURCE CODE LOCATION
# ============================================

# Source directories (comma-separated)
sonar.sources=src

# Test directories
sonar.tests=src/**/*.test.js,src/**/*.spec.js

# Exclusions (files to ignore)
sonar.exclusions=**/node_modules/**,**/dist/**,**/build/**,**/*.test.js

# Test coverage exclusions
sonar.coverage.exclusions=**/*.test.js,**/*.spec.js,**/mocks/**

# ============================================
# LANGUAGE CONFIGURATION
# ============================================

# Source encoding
sonar.sourceEncoding=UTF-8

# JavaScript/TypeScript
sonar.javascript.file.suffixes=.js,.jsx
sonar.typescript.file.suffixes=.ts,.tsx

# Python
# sonar.python.version=3.9

# Java
# sonar.java.binaries=target/classes

# ============================================
# CODE COVERAGE
# ============================================

# JavaScript/TypeScript with Jest
sonar.javascript.lcov.reportPaths=coverage/lcov.info
sonar.testExecutionReportPaths=coverage/test-reporter.xml

# Python with pytest-cov
# sonar.python.coverage.reportPaths=coverage.xml

# Java with JaCoCo
# sonar.coverage.jacoco.xmlReportPaths=target/site/jacoco/jacoco.xml

# ============================================
# QUALITY GATE SETTINGS
# ============================================

# Wait for quality gate result
sonar.qualitygate.wait=true
sonar.qualitygate.timeout=300

# ============================================
# QUALITY THRESHOLDS
# ============================================

# Code coverage minimum
sonar.coverage.threshold=80

# Duplicate code maximum (%)
sonar.cpd.exclusions=**/test/**
sonar.duplications.exclusions=**/test/**

# ============================================
# ANALYSIS PARAMETERS
# ============================================

# Branch name
sonar.branch.name=main

# Pull request analysis (if applicable)
# sonar.pullrequest.key=123
# sonar.pullrequest.branch=feature-branch
# sonar.pullrequest.base=main
```

---

## CI/CD Integration

### GitHub Actions

Create `.github/workflows/sonarqube.yml`:

```yaml
name: SonarQube Analysis

on:
  push:
    branches: [main, develop]
  pull_request:
    branches: [main, develop]

jobs:
  sonarqube:
    name: SonarQube Scan
    runs-on: ubuntu-latest

    steps:
      - name: Checkout code
        uses: actions/checkout@v4
        with:
          fetch-depth: 0 # Full history for better analysis

      - name: Setup Node.js
        uses: actions/setup-node@v4
        with:
          node-version: '18'

      - name: Install dependencies
        run: npm ci

      - name: Run tests with coverage
        run: npm run test:coverage

      - name: SonarQube Scan
        uses: sonarsource/sonarqube-scan-action@<pinned-version>
        env:
          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
          SONAR_HOST_URL: ${{ secrets.SONAR_HOST_URL }}

      - name: SonarQube Quality Gate Check
        uses: sonarsource/sonarqube-quality-gate-action@<pinned-version>
        timeout-minutes: 5
        env:
          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
        with:
          scanMetadataReportFile: .scannerwork/report-task.txt

      - name: Fail if Quality Gate failed
        if: steps.sonarqube-quality-gate-check.outputs.quality-gate-status == 'FAILED'
        run: exit 1
```

**Setup Secrets**:
1. Go to repository Settings → Secrets
2. Add `SONAR_TOKEN` (from SonarQube)
3. Add `SONAR_HOST_URL` (for example `https://sonarcloud.io` or your self-hosted server URL)

---

### GitLab CI

Create `.gitlab-ci.yml`:

```yaml
stages:
  - test
  - sonarqube

test:
  stage: test
  script:
    - npm ci
    - npm run test:coverage
  artifacts:
    paths:
      - coverage/
    expire_in: 1 day

sonarqube:
  stage: sonarqube
  image: sonarsource/sonar-scanner-cli:latest
  variables:
    SONAR_USER_HOME: "${CI_PROJECT_DIR}/.sonar"
    GIT_DEPTH: "0"
  cache:
    key: "${CI_JOB_NAME}"
    paths:
      - .sonar/cache
  script:
    - sonar-scanner
      -Dsonar.qualitygate.wait=true
      -Dsonar.projectKey=$CI_PROJECT_PATH_SLUG
      -Dsonar.sources=src
      -Dsonar.host.url=$SONAR_HOST_URL
      -Dsonar.login=$SONAR_TOKEN
  allow_failure: false
  only:
    - main
    - merge_requests
```

---

### Jenkins

Install SonarQube Scanner plugin, then add to Jenkinsfile:

```groovy
pipeline {
  agent any

  stages {
    stage('Checkout') {
      steps {
        checkout scm
      }
    }

    stage('Test') {
      steps {
        sh 'npm ci'
        sh 'npm run test:coverage'
      }
    }

    stage('SonarQube Analysis') {
      steps {
        withSonarQubeEnv('SonarQube') {
          sh 'sonar-scanner'
        }
      }
    }

    stage('Quality Gate') {
      steps {
        timeout(time: 5, unit: 'MINUTES') {
          waitForQualityGate abortPipeline: true
        }
      }
    }
  }
}
```

---

## Quality Gates (2026.1 LTA)

Quality Gates define minimum quality standards for code to pass.

**Reference**: [SonarQube Quality Gates Documentation](https://docs.sonarsource.com/sonarqube-server/2026.1/instance-administration/analysis-functions/quality-gates)

SonarQube 2026.1 LTA adds MCP server integration so AI agents (Claude Code, Cursor, Windsurf) can query quality and security insights directly. Analysis is up to 50% faster for JavaScript, TypeScript, Python, and Kotlin versus 2025.x.

### Built-in Quality Gates

SonarQube 2026.1 LTA provides two useful built-in quality gates:

| Quality Gate | Use Case |
| ------------ | -------- |
| **Sonar way** | Default for all projects (recommended) |
| **Sonar way for AI Code** | Projects containing AI-generated code (stricter) |

### Sonar way (Default)

**Conditions** (fail if any breached on new code):

- Number of issues > 0 (enforces zero new issues)
- Reliability Rating worse than A
- Security Rating worse than A
- Maintainability Rating worse than A
- Security Hotspots Reviewed < 100%
- Coverage < 80% (configurable)
- Duplicated Lines (%) > 3% (configurable)

### Sonar way for AI Code

**Additional conditions** for AI-generated code:

- All conditions from Sonar way
- Stricter enforcement on security and reliability
- Enhanced detection of AI-generated anti-patterns
- Required for AI Code Assurance qualification

**When to use**: Projects using GitHub Copilot, Cursor, Claude Code, or other AI coding assistants should consider this quality gate.

### Zero New Issues Strategy

The most effective quality gate strategy is **zero new issues**:

```
Condition: Number of issues > 0 → FAIL

Why: Prevents ALL technical debt from entering new code.
Rating conditions (A) still allow some issues to slip through.
```

### Custom Quality Gate

**Create in SonarQube UI**:

1. Quality Gates → Create
2. Start from "Sonar way" template (auto-copied)
3. Add/modify conditions:

```text
Conditions on New Code:
- Number of issues > 0 (strictest, recommended)
- Coverage < 80%
- Duplicated Lines (%) > 3%
- Security Hotspots Reviewed < 100%

Conditions on Overall Code (legacy projects):
- Technical Debt Ratio > 5%
- Code Smells > 100
- Bugs > 0
- Vulnerabilities > 0
```

### Fudge Factor

For small changes (<20 lines), SonarQube relaxes coverage and duplication checks to avoid over-enforcement. This is enabled by default.

**Behavior**: Coverage and duplication conditions are ignored until new code reaches 20+ lines.

---

## Analyzing Results

### Metrics Explained

**Reliability**:
- **Bugs**: Code that will likely fail in production
- **Reliability Rating**: A (0 bugs) to E (many bugs)

**Security**:
- **Vulnerabilities**: Security flaws
- **Security Hotspots**: Security-sensitive code to review
- **Security Rating**: A (0 vulnerabilities) to E (many)

**Maintainability**:
- **Code Smells**: Maintainability issues
- **Technical Debt**: Time to fix all code smells
- **Maintainability Rating**: A (<5% debt ratio) to E (>50%)

**Coverage**:
- **Coverage**: % of code covered by tests
- **Line Coverage**: % of lines executed
- **Branch Coverage**: % of branches (if/else) covered

**Duplication**:
- **Duplicated Lines**: % of duplicated code
- **Duplicated Blocks**: Number of duplicate blocks

**Size**:
- **Lines of Code**: Total lines (excluding comments/blank lines)
- **Statements**: Number of statements
- **Functions**: Number of functions
- **Classes**: Number of classes

---

## Best Practices

### 1. Focus on New Code

**Why**: Can't fix everything at once

**How**:
- Set strict quality gates for new code
- Allow legacy code to have more issues
- Gradually improve old code with Boy Scout Rule

### 2. Fix Blocker/Critical Issues First

**Priority Order**:
1. Blocker bugs/vulnerabilities
2. Critical bugs/vulnerabilities
3. Major bugs
4. Code smells (by debt)

### 3. Aim for "Clean as You Code"

**Principle**: All new code meets quality standards

**Practice**:
- Quality gate on new code only
- Fix issues before merging
- Don't accumulate new debt

### 4. Regular Debt Reduction

**Schedule**:
- Review debt in normal planning cadence
- Time-box focused cleanup when hotspots materially slow delivery
- Treat debt work as capacity planning, not a universal fixed percentage

### 5. Monitor Trends

**Watch for**:
- Increasing debt ratio
- Decreasing coverage
- Rising bug count
- Growing duplications

---

## Troubleshooting

### Analysis Fails

**Issue**: Analysis doesn't complete

**Solutions**:
- Check SonarQube logs
- Verify token permissions
- Ensure correct project key
- Check network connectivity

### No Coverage Data

**Issue**: Coverage shows 0%

**Solutions**:
- Verify test script generates coverage
- Check `sonar.javascript.lcov.reportPaths` path
- Ensure coverage file exists before scan
- Run tests before SonarQube scan

### Quality Gate Always Passes

**Issue**: Even bad code passes

**Solutions**:
- Check quality gate configuration
- Verify conditions are set
- Ensure quality gate is assigned to project
- Check for exclusions hiding issues

---

## Advanced Configuration

### Multi-Module Projects

```properties
# Parent module
sonar.projectKey=my-company_my-monorepo
sonar.modules=module1,module2,module3

# Module 1
module1.sonar.projectName=Module 1
module1.sonar.sources=packages/module1/src
module1.sonar.tests=packages/module1/tests

# Module 2
module2.sonar.projectName=Module 2
module2.sonar.sources=packages/module2/src
module2.sonar.tests=packages/module2/tests
```

### Branch Analysis

```properties
# Long-lived branches
sonar.branch.name=develop
sonar.branch.target=main

# Pull request analysis
sonar.pullrequest.key=123
sonar.pullrequest.branch=feature-xyz
sonar.pullrequest.base=main
```

### Custom Rules

Create custom rules using SonarQube plugin API (Java):
1. Create Maven project
2. Implement `JavaCheck` interface
3. Build JAR
4. Upload to SonarQube

---

## Maintenance

### Regular Tasks

**Weekly**:
- Review new issues
- Check quality gate status
- Monitor coverage trends

**Monthly**:
- Update quality gates
- Review custom rules
- Clean up old branches

**Quarterly**:
- SonarQube version upgrade
- Plugin updates
- Performance tuning

---

## Resources

- **Official Docs (2026.1 LTA)**: [SonarQube Server 2026.1](https://docs.sonarsource.com/sonarqube-server/2026.1/)
- **Quality Gates Guide**: [Quality Gates Documentation](https://docs.sonarsource.com/sonarqube-server/2026.1/instance-administration/analysis-functions/quality-gates)
- **Installation Requirements**: [Server Host Requirements](https://docs.sonarsource.com/sonarqube-server/2026.1/setup-and-upgrade/installation-requirements/server-host)
- **Database Support**: [Installing the Database](https://docs.sonarsource.com/sonarqube-server/2026.1/server-installation/installing-the-database)
- **SonarQube Cloud**: [sonarcloud.io](https://sonarcloud.io/)
- **Rules Reference**: [rules.sonarsource.com](https://rules.sonarsource.com/)
- **Community**: [community.sonarsource.com](https://community.sonarsource.com/)
- **GitHub**: [SonarSource/sonarqube](https://github.com/SonarSource/sonarqube)

---

## Quick Reference

### Common Commands

```bash
# Local analysis
sonar-scanner

# With custom properties
sonar-scanner -Dsonar.projectKey=my-project

# With token
sonar-scanner -Dsonar.login=my-token

# Verbose output
sonar-scanner -X
```

### Docker Commands

```bash
# Start
docker run -d --name sonarqube -p 9000:9000 sonarqube:<supported-tag>

# Stop
docker stop sonarqube

# Restart
docker restart sonarqube

# Logs
docker logs -f sonarqube

# Remove
docker rm -f sonarqube
```

# Technical Debt Register

Track and prioritize technical debt items. Copy this template to your project.

---

## Active Technical Debt

| ID | Description | Type | Impact | Effort | Priority | Created | Owner | Target | Status |
|----|-------------|------|--------|--------|----------|---------|-------|--------|--------|
| TD-001 | Refactor UserService (600+ lines, complexity 25) | Prudent Deliberate | High | 2d | P1 | 2025-11-01 | Alice | Sprint 24 | In Progress |
| TD-002 | Add tests for PaymentProcessor (0% coverage) | Reckless Inadvertent | High | 3d | P1 | 2025-10-15 | Bob | Sprint 24 | Backlog |
| TD-003 | Extract shared validation logic (duplicated 5x) | Prudent Inadvertent | Medium | 1d | P2 | 2025-11-10 | Charlie | Sprint 25 | Backlog |
| TD-004 | Update deprecated API endpoints (3 remaining) | Prudent Deliberate | Medium | 2d | P2 | 2025-09-01 | Dave | Sprint 26 | Backlog |
| TD-005 | Reduce OrderProcessor complexity (18) | Reckless Inadvertent | Low | 4h | P3 | 2025-11-15 | Eve | Sprint 27 | Backlog |

---

## Debt Classification

### Type (Technical Debt Quadrant)

**Reckless Deliberate**: "We don't have time for design"
- Most dangerous, avoid creating
- Example: Copy-pasting code to ship fast

**Prudent Deliberate**: "We must ship now and deal with consequences"
- Acceptable short-term
- Example: Shipping MVP with known limitations

**Reckless Inadvertent**: "What's layering?"
- Due to lack of knowledge
- Example: Not using design patterns

**Prudent Inadvertent**: "Now we know how we should have done it"
- Normal learning process
- Example: Realizing better architecture after implementation

---

## Prioritization

### Priority Matrix

```
          High Impact
              │
    P1 = Do Now │ P2 = Plan
    ─────────────┼─────────────
    P3 = Maybe  │ P4 = Skip
              │
          Low Impact
        Low Effort → High Effort
```

**P1**: High impact, low effort → Address immediately
**P2**: High impact, high effort → Plan and schedule
**P3**: Low impact, low effort → Fix if time available
**P4**: Low impact, high effort → Defer or reconsider

### Impact Assessment

**High Impact**:
- Blocks new features
- Causes frequent bugs
- Slows team velocity
- Security risk

**Medium Impact**:
- Makes changes difficult
- Reduces code quality
- Increases maintenance time

**Low Impact**:
- Minor inconvenience
- Aesthetic issue
- Minimal effect on development

### Effort Estimation

**Days** or **hours** to fix:
- Include time for testing
- Include time for documentation
- Include time for review

---

## Template for New Debt Items

```markdown
## TD-XXX: [Brief Description]

**Created**: YYYY-MM-DD
**Owner**: [Name]
**Status**: Backlog

### Description

[Detailed explanation of the technical debt]

### Type

[Reckless/Prudent] [Deliberate/Inadvertent]

### Why It Was Created

[Context: Why did we take this shortcut?]

### Impact

**Level**: High / Medium / Low

**Effects**:
- [Effect 1]
- [Effect 2]

**Business Impact**:
- [How does this affect business? Slower features? More bugs?]

### Effort to Fix

**Estimated**: X days / hours

**Tasks**:
- [ ] Task 1
- [ ] Task 2
- [ ] Task 3

### Priority

**Priority**: P1 / P2 / P3 / P4

**Reasoning**:
[Why this priority? Impact vs. effort trade-off]

### Target

**Sprint**: Sprint XX
**Deadline**: YYYY-MM-DD (if applicable)

### Related Items

- Related to TD-XXX
- Blocks Feature-YYY
- Depends on TD-ZZZ

### Metrics

**Before**:
- Lines of code: XXX
- Complexity: YY
- Test coverage: ZZ%
- Debt ratio: AA%

**After (Expected)**:
- Lines of code: XXX
- Complexity: YY
- Test coverage: ZZ%
- Debt ratio: AA%

### Notes

[Additional context, links to discussions, etc.]
```

---

## Example: Complete Debt Item

```markdown
## TD-001: Refactor UserService

**Created**: 2025-11-01
**Owner**: Alice
**Status**: In Progress

### Description

UserService class has grown to 600+ lines with cyclomatic complexity of 25. It handles authentication, validation, database operations, and email notifications—violating Single Responsibility Principle.

### Type

Prudent Deliberate

### Why It Was Created

Started as simple user CRUD. Features added incrementally over 2 years without refactoring. Shipped quickly to meet deadlines.

### Impact

**Level**: High

**Effects**:
- 3 bugs in last month due to complexity
- New features take 2x longer (must understand entire class)
- Difficult to test (mocking 5+ dependencies)
- Onboarding new developers takes extra 2 days

**Business Impact**:
- Slower feature delivery (estimated 15% velocity reduction)
- Higher bug rate (3 production incidents in Q4)
- Increased hiring friction (candidates mention code quality in interviews)

### Effort to Fix

**Estimated**: 2 days

**Tasks**:
- [x] Extract AuthenticationService
- [ ] Extract ValidationService
- [ ] Extract UserRepository
- [ ] Extract EmailService
- [ ] Add unit tests (target: 85% coverage)
- [ ] Update integration tests
- [ ] Update documentation

### Priority

**Priority**: P1 (Do Now)

**Reasoning**:
- High impact (blocks features, causes bugs)
- Relatively low effort (2 days)
- Quick win for team morale

### Target

**Sprint**: Sprint 24
**Deadline**: 2025-11-30

### Related Items

- Related to TD-003 (validation logic duplication)
- Blocks Feature-045 (OAuth integration)
- Mentioned in SEC-12 (security audit finding)

### Metrics

**Before**:
- Lines of code: 600
- Complexity: 25
- Test coverage: 45%
- Debt ratio: 18%

**After (Expected)**:
- Lines of code: ~250
- Complexity: <10
- Test coverage: 85%
- Debt ratio: 12%

### Notes

- Discussed in retrospective 2025-10-28
- Stakeholders aware: Will delay Feature-046 by 2 days
- SonarQube report: [link]
- Refactoring plan: [link to design doc]
```

---

## Tracking Metrics

### Overall Debt Metrics

**Current State** (updated weekly):

```
Total Debt Items: 12
├── P1 (Critical): 2
├── P2 (High): 4
├── P3 (Medium): 5
└── P4 (Low): 1

By Status:
├── In Progress: 2
├── Backlog: 8
├── Blocked: 1
└── Completed this quarter: 6

Technical Debt Ratio: 14% (SonarQube)
Target: <10%

Estimated Total Effort: 24 days
Sprint Capacity for Debt: 2 days/sprint (20%)
```

### Trend Chart

```
Debt Ratio Over Time:

Q1 2025: 22% ████████████████████████ ↓
Q2 2025: 18% ████████████████████     ↓
Q3 2025: 14% ████████████████         ↓
Q4 2025: 12% █████████████           (target: <10%)
```

---

## Sprint Planning

### Debt Allocation

**Rule**: Allocate 20% of sprint capacity to debt reduction

**Example** (2-week sprint):
- Total capacity: 10 days
- Feature work: 8 days (80%)
- Debt reduction: 2 days (20%)

### Sprint N Planning

**Debt Items for Sprint N**:
- [ ] TD-001: UserService refactoring (2d) - P1
- [ ] TD-005: OrderProcessor complexity (4h) - P3

**Total Debt Work**: 2.5 days (25% of sprint)

**Rationale**: Catching up on P1 items, will return to 20% next sprint

---

## Completed Debt (Archive)

| ID | Description | Priority | Completed | Time Spent | Owner |
|----|-------------|----------|-----------|------------|-------|
| TD-010 | Remove deprecated API v1 | P2 | 2025-10-15 | 1d | Dave |
| TD-011 | Add tests for AuthService | P1 | 2025-10-10 | 2d | Bob |
| TD-012 | Extract payment logic | P2 | 2025-09-20 | 3d | Alice |

---

## Prevention Strategies

### Code Review Checklist

When reviewing PRs, check for new debt:
- [ ] No methods >20 lines
- [ ] No classes >300 lines
- [ ] No complexity >10
- [ ] Test coverage >80%
- [ ] No duplicate code

If debt found:
1. **Option 1**: Fix before merge (preferred)
2. **Option 2**: Create debt item, get approval, merge with plan

### Quality Gates (CI/CD)

Prevent debt with automated checks:
- Linter: Max complexity 10
- SonarQube: Quality gate must pass
- Test coverage: Must be >80%
- Build fails if debt exceeds thresholds

---

## Communication

### To Team

**Weekly Update** (standup):
- "We reduced debt ratio from 14% to 12% this week"
- "Completed TD-001, UserService is now maintainable"
- "3 P1 items remaining, focusing on those next sprint"

### To Stakeholders

**Monthly Report**:

```
Technical Debt Update - November 2025

Progress:
[check] Completed 3 debt items (TD-001, TD-005, TD-008)
[check] Debt ratio reduced: 18% → 14%
[check] Team velocity increased: 15 → 17 story points/sprint

Impact:
[check] Bug rate decreased: 8 bugs/month → 4 bugs/month
[check] Feature delivery faster: -20% time for new features
[check] Developer satisfaction improved: 3.2 → 4.1 (out of 5)

Investment:
[check] 6 days spent on debt this month (20% of capacity)
[check] ROI: 4 hours/week saved in maintenance

Next Month:
→ Target debt ratio: 12%
→ Focus on P1/P2 items (4 remaining)
→ Continue 20% allocation
```

---

## Retrospective Questions

Include in sprint retrospectives:

1. **What new debt did we create this sprint?**
   - Was it intentional (Prudent Deliberate)?
   - Did we document it?

2. **Did we meet our 20% debt reduction target?**
   - If not, why?
   - What blocked us?

3. **Which debt items caused problems this sprint?**
   - Did lack of tests slow us down?
   - Did complexity cause bugs?

4. **Are we preventing new debt effectively?**
   - Are quality gates working?
   - Are code reviews catching issues?

---

## ROI Calculator

Use this to justify debt reduction to stakeholders. Replace every number with your own measured or estimated cost data before presenting it — the figures below are a worked arithmetic example, not a benchmark to expect.

```
Cost of Debt (per week):
- Bug fixes: 8 hours × $100/hour = $800
- Slow feature delivery: 10 hours × $100/hour = $1000
- Developer frustration: -10% productivity = $500
Total weekly cost: $800 + $1000 + $500 = $2300

Investment to Fix:
- 2 days refactoring × $800/day = $1600

ROI (standard formula: (gain - cost) / cost):
- Gross annual savings (avoided weekly cost × 52): $2300 × 52 = $119,600
- Net annual gain: $119,600 - $1600 = $118,000
- ROI: $118,000 / $1600 = 73.75 → 7375%
- Payback period: $1600 / $2300 per week = 0.70 weeks

Break-even: Less than 1 week.
```

Sanity-check any ROI this large before presenting it: a 7000%+ return usually means the weekly "cost of debt" estimate is too aggressive (padded with soft costs like "developer frustration") rather than that the fix is actually free money. Prefer a conservative, defensible weekly cost figure — ideally backed by ticket/time-tracking data — over a round percentage that will not survive scrutiny from finance.

---

## Tools Integration

### Jira

Create "Technical Debt" issue type:
- Fields: ID, Type, Impact, Effort, Priority
- Labels: debt, P1, P2, P3, P4
- Dashboard: Debt burndown chart

### GitHub

Use labels for tracking:
- `technical-debt`
- `debt-p1`, `debt-p2`, `debt-p3`
- `debt-security`, `debt-performance`

### SonarQube

Link debt items to SonarQube issues:
- Export debt metrics
- Track debt ratio over time
- Set quality gates

---

## References

- Technical Debt Quadrant: https://martinfowler.com/bliki/TechnicalDebtQuadrant.html
- Managing Technical Debt: references/tech-debt-management.md

{
  "metadata": {
    "skill": "qa-refactoring",
    "updated": "2026-08-31",
    "version": "2.5",
    "total_sources": 26,
    "description": "Primary references for safe refactoring, codemod verification, contract testing, migration rollouts, and CI quality gates. Prefer official docs for volatile tooling guidance.",
    "title": "QA Refactoring - Sources",
    "last_updated": "2026-08-31"
  },
  "categories": {
    "refactoring_foundations": [
      {
        "name": "Martin Fowler - Refactoring (Book)",
        "url": "https://martinfowler.com/books/refactoring.html",
        "description": "Canonical refactoring reference and catalog entry point.",
        "add_as_web_search": false,
        "optional": false
      },
      {
        "name": "Refactoring.Guru - Refactoring Catalog",
        "url": "https://refactoring.guru/refactoring",
        "description": "Refactoring patterns with examples; useful for mapping smells to safe moves.",
        "add_as_web_search": false,
        "optional": false
      },
      {
        "name": "Refactoring.Guru - Code Smells",
        "url": "https://refactoring.guru/refactoring/smells",
        "description": "Code smell identification and remediation hints.",
        "add_as_web_search": false,
        "optional": false
      },
      {
        "name": "Tidy First? - Kent Beck (Book, O'Reilly, 2023)",
        "url": "https://www.oreilly.com/library/view/tidy-first/9781098151253/",
        "description": "Small, reversible \"tidyings\" done before a risky change, framed as an economic (coupling/cohesion, optionality) rather than moral decision. Verified via web search 2026-07-11.",
        "add_as_web_search": false,
        "optional": false,
        "last_verified": "2026-07-11"
      }
    ],
    "legacy_code_safety": [
      {
        "name": "Working Effectively with Legacy Code (Book)",
        "url": "https://www.oreilly.com/library/view/working-effectively-with/0131177052/",
        "description": "Seams, characterization tests, and incremental refactoring strategies.",
        "add_as_web_search": false,
        "optional": false
      },
      {
        "name": "ApprovalTests",
        "url": "https://approvaltests.com/",
        "description": "Approval and golden-master testing workflows across languages.",
        "add_as_web_search": true,
        "optional": false
      },
      {
        "name": "Martin Fowler - Strangler Fig Application",
        "url": "https://martinfowler.com/bliki/StranglerFigApplication.html",
        "description": "Incremental modernization pattern for legacy systems.",
        "add_as_web_search": false,
        "optional": false
      }
    ],
    "codemods_and_contracts": [
      {
        "name": "OpenRewrite - Recipe Testing",
        "url": "https://docs.openrewrite.org/authoring-recipes/recipe-testing",
        "description": "Testing guidance for large-scale semantic rewrites.",
        "add_as_web_search": true,
        "optional": false
      },
      {
        "name": "ast-grep - Rewrite Rule",
        "url": "https://ast-grep.github.io/guide/rewrite-rule.html",
        "description": "Pattern-based AST rewrites for codemods and safe mechanical edits.",
        "add_as_web_search": true,
        "optional": false
      },
      {
        "name": "Pact Documentation",
        "url": "https://docs.pact.io/",
        "description": "Consumer/provider contract testing for boundary-preserving refactors.",
        "add_as_web_search": true,
        "optional": false
      }
    ],
    "quality_gates": [
      {
        "name": "ESLint - Configuration Files",
        "url": "https://eslint.org/docs/latest/use/configure/configuration-files",
        "description": "Current flat-config guidance for JavaScript/TypeScript linting.",
        "add_as_web_search": true,
        "optional": false
      },
      {
        "name": "Prettier - Integrating with Linters",
        "url": "https://prettier.io/docs/next/integrating-with-linters",
        "description": "Current guidance on keeping formatting separate from linting.",
        "add_as_web_search": true,
        "optional": false
      },
      {
        "name": "SonarQube Server 2026.1 LTA - Quality Gates",
        "url": "https://docs.sonarsource.com/sonarqube-server/2026.1/instance-administration/analysis-functions/quality-gates",
        "description": "Quality gate configuration for SonarQube 2026.1 LTA, including Sonar way for AI Code and MCP server integration.",
        "add_as_web_search": true,
        "optional": false,
        "last_verified": "2026-06-09"
      },
      {
        "name": "SonarQube Server 2026.1 LTA - Installation Requirements",
        "url": "https://docs.sonarsource.com/sonarqube-server/2026.1/setup-and-upgrade/installation-requirements/server-host",
        "description": "Current supported runtime and host requirements for SonarQube Server 2026.1 LTA.",
        "add_as_web_search": true,
        "optional": false,
        "last_verified": "2026-06-09"
      }
    ],
    "rollout_and_ci": [
      {
        "name": "LaunchDarkly - Migration Flags",
        "url": "https://launchdarkly.com/docs/home/flags/migration",
        "description": "Stage-based rollout control for legacy migrations and branch-by-abstraction cutovers.",
        "add_as_web_search": true,
        "optional": false
      },
      {
        "name": "GitHub Actions - Automating Builds and Tests",
        "url": "https://docs.github.com/en/actions/automating-builds-and-tests",
        "description": "CI workflow building blocks for refactor safety.",
        "add_as_web_search": true,
        "optional": false
      },
      {
        "name": "GitHub Actions - Caching Dependencies",
        "url": "https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/caching-dependencies-to-speed-up-workflows",
        "description": "Keep refactor verification pipelines fast and repeatable.",
        "add_as_web_search": true,
        "optional": false
      }
    ],
    "optional_ai_automation": [
      {
        "name": "GitHub Copilot Coding Agent - Create Skills",
        "url": "https://docs.github.com/en/copilot/how-tos/use-copilot-agents/coding-agent/create-skills",
        "description": "Official guidance for agent skills and task-specific operating instructions.",
        "add_as_web_search": true,
        "optional": true
      },
      {
        "name": "OpenRewrite - AI-Powered Recipe Authoring (Moderne)",
        "url": "https://www.moderne.ai/blog/ai-powered-openrewrite-recipe-authoring-with-claude-skill",
        "description": "How the Moderne CLI MCP server exposes OpenRewrite recipes as deterministic tool calls for AI coding agents. Primary reference for OpenRewrite + AI agent workflows.",
        "add_as_web_search": true,
        "optional": true,
        "last_verified": "2026-06-09"
      },
      {
        "name": "jssg - JavaScript ast-grep (Codemod)",
        "url": "https://codemod.com/blog/jssg",
        "description": "JavaScript/TypeScript-authored ast-grep transforms via jssg, announced October 2025. Write polyglot codemods in TypeScript using ast-grep as the matching engine.",
        "add_as_web_search": true,
        "optional": true,
        "last_verified": "2026-06-09"
      },
      {
        "name": "SonarQube Server 2026.1 LTA - What's New",
        "url": "https://www.sonarsource.com/products/sonarqube/whats-new/2026-1/",
        "description": "SonarQube 2026.1 LTA: MCP server integration, 50% faster JS/TS/Python/Kotlin analysis, Rust and Swift 6.2 support, AI Code Assurance improvements.",
        "add_as_web_search": true,
        "optional": true,
        "last_verified": "2026-06-09"
      },
      {
        "name": "ponytail (DietrichGebert)",
        "url": "https://github.com/DietrichGebert/ponytail",
        "add_as_web_search": false,
        "optional": true,
        "last_verified": "2026-08-31",
        "description": "YAGNI-first agent coding contract; source of the reuse-before-write ladder in references/operational-patterns.md. Pinned commit 2ed6c52c, MIT, extracted 2026-08-09 (docs/research/2026-08-09-skill-ponytail-scan.md)."
      }
    ],
    "mutation_testing": [
      {
        "name": "Stryker Mutator",
        "url": "https://stryker-mutator.io",
        "description": "Mutation testing framework for JavaScript, TypeScript, and .NET. Primary reference for Stryker configuration, runners, and incremental mutation.",
        "add_as_web_search": true,
        "optional": false
      },
      {
        "name": "mutmut (Python mutation testing)",
        "url": "https://github.com/boxed/mutmut",
        "description": "Lightweight Python mutation tester; integrates with pytest. Primary reference for mutmut usage and CI integration.",
        "add_as_web_search": true,
        "optional": false
      },
      {
        "name": "PIT (Pitest) - Java/Kotlin mutation testing",
        "url": "https://pitest.org",
        "description": "Mature mutation testing system for JVM languages. Primary reference for PIT Maven/Gradle configuration and incremental history mode.",
        "add_as_web_search": true,
        "optional": false
      },
      {
        "name": "cosmic-ray (Python mutation testing)",
        "url": "https://github.com/sixty-north/cosmic-ray",
        "description": "Session-based Python mutation tester with distributed (Celery) execution support. Primary reference for large-scale Python mutation runs.",
        "add_as_web_search": true,
        "optional": false
      }
    ]
  }
}

# qa-refactoring — Consolidated Learnings

Curated, dated, committed memory for this skill. Pruned from raw `learnings.md` via `agents-skills-feedback-loop/scripts/consolidate.py`. Human-approved.

Cap: 60 entries. When exceeded, promote durable rules to `references/`.

## Filter Override

<!-- Add 2-4 bullets that sharpen what counts as a learning for this skill. Leave empty to use the default filter from agents-skills-feedback-loop/references/learnings-format.md. -->

## Patterns That Work

## Mistakes to Avoid

## Domain Knowledge

## Open Questions

## Consolidated Principles

# qa-refactoring — Learnings

## Patterns That Work

## Mistakes to Avoid

## Domain Knowledge

- [2026-07-11] 2026-07-11 audit: fixed fabricated case-study stats, a ROI math error, a stale GritQL claim, and an unsourced AI-refactor stat; added Tidy First? citation and new SKILL.md expert-judgment sections.
## Open Questions

## Consolidated Principles

# Automated Refactoring Tools

Codemods, AST transforms, and IDE refactoring automation for safe, large-scale code changes. Move beyond manual find-and-replace.

## Contents

- [Codemod Frameworks](#codemod-frameworks)
- [AST Manipulation Basics](#ast-manipulation-basics)
- [Writing Custom Codemods](#writing-custom-codemods)
- [IDE Refactoring Features](#ide-refactoring-features)
- [Large-Scale Refactoring](#large-scale-refactoring)
- [Safety Verification](#safety-verification)
- [Codemod Testing Strategies](#codemod-testing-strategies)
- [Migration Codemods for Framework Upgrades](#migration-codemods-for-framework-upgrades)
- [Codemod Composition Patterns](#codemod-composition-patterns)
- [Related Resources](#related-resources)

---

## Codemod Frameworks

### Framework Overview

| Framework | Language | AST Library | Maintained By | Best For |
|-----------|---------|-------------|---------------|----------|
| **jscodeshift** | JavaScript/TypeScript | recast + ast-types | Meta | React/JS migrations (v17.3 stable) |
| **ts-morph** | TypeScript | TypeScript compiler | Community | TS-specific transforms |
| **libCST** | Python | libCST (concrete syntax tree) | Meta/Instagram | Python code transforms |
| **Scalafix** | Scala | Scalameta | Community | Scala migrations |
| **Rector** | PHP | php-parser | Community | PHP framework upgrades |
| **GritQL** | Multi-language | Tree-sitter | Originated at Grit.io (acquired by Honeycomb, Apr 2025); the `getgrit` GitHub org was archived May 2025, but the GritQL query engine continues under `biomejs/gritql` (unverified whether this fork is a drop-in CLI replacement — verify before depending on it; ast-grep or Semgrep remain the safer default) | Pattern-based, polyglot |
| **ast-grep** | Multi-language | Tree-sitter | Community | YAML rewrite rules; jssg enables TypeScript-authored transforms |
| **Semgrep** | Multi-language | Tree-sitter | Semgrep Inc | Pattern matching + autofix |
| **OpenRewrite** | Java/Kotlin/more | Lossless Semantic Tree | Moderne | Java framework migrations; 5,000+ community recipes; MCP server integration |

### OpenRewrite in AI-Agent Workflows (2026)

OpenRewrite's Moderne CLI ships a local MCP server that exposes 5,000+ community recipes as deterministic tool calls. AI coding agents (Claude Code, Cursor, Windsurf) can invoke recipes directly via MCP rather than writing ad hoc codemods for common Java framework migrations. This is the recommended path for large-scale Java modernization in AI-assisted workflows — the agent selects the recipe by intent; the recipe executes deterministically.

```bash
# Run OpenRewrite recipe via Moderne CLI (MCP-connected)
mod run . --recipe UpgradeSpringBoot_3_4
mod run . --recipe UpgradeToJava21
```

See [OpenRewrite docs](https://docs.openrewrite.org/) for recipe catalog. For Java projects, prefer OpenRewrite recipes over hand-written codemods when a recipe exists.

---

### Installation

```bash
# jscodeshift (JavaScript/TypeScript)
npm install -g jscodeshift

# ts-morph (TypeScript)
npm install ts-morph

# libCST (Python)
pip install libcst

# Rector (PHP)
composer require rector/rector --dev

# OpenRewrite (Java - via Maven)
# Add to pom.xml as plugin

# GritQL (getgrit org archived May 2025; engine continues at biomejs/gritql — verify current package name)
# Prefer ast-grep or Semgrep as the maintained default until you've confirmed the fork's CLI packaging
# npm install -g @getgrit/cli

# ast-grep
npm install -g @ast-grep/cli
# jssg (JavaScript/TypeScript-authored ast-grep transforms, announced Oct 2025)
npm install -g @codemod/jssg

# Semgrep
pip install semgrep
```

---

## AST Manipulation Basics

Understanding ASTs (Abstract Syntax Trees) is the foundation for writing codemods.

### What Is an AST?

```javascript
// Source code:
const total = price * quantity;

// AST (simplified):
{
  "type": "VariableDeclaration",
  "kind": "const",
  "declarations": [{
    "type": "VariableDeclarator",
    "id": { "type": "Identifier", "name": "total" },
    "init": {
      "type": "BinaryExpression",
      "operator": "*",
      "left": { "type": "Identifier", "name": "price" },
      "right": { "type": "Identifier", "name": "quantity" }
    }
  }]
}
```

### AST Explorer

Use [astexplorer.net](https://astexplorer.net) to visualize ASTs interactively. Select the parser that matches your codemod framework:

| Codemod Tool | AST Explorer Parser |
|-------------|-------------------|
| jscodeshift | recast |
| ts-morph | TypeScript |
| libCST | Python (CST) |
| Babel | @babel/parser |

### CST vs AST

| Property | AST (Abstract Syntax Tree) | CST (Concrete Syntax Tree) |
|----------|---------------------------|---------------------------|
| Whitespace | Discarded | Preserved |
| Comments | Usually discarded | Preserved |
| Formatting | Lost | Preserved |
| Best for | Analysis, linting | Refactoring (preserves style) |
| Libraries | babel, typescript, tree-sitter | recast, libCST, ts-morph |

For refactoring, prefer CST-based tools (recast, libCST) to preserve formatting and comments.

---

## Writing Custom Codemods

### jscodeshift: Rename a Function

```javascript
// codemod: rename-function.js
// Renames all calls from `oldFunctionName` to `newFunctionName`

module.exports = function (fileInfo, api) {
  const j = api.jscodeshift;
  const root = j(fileInfo.source);

  // Find all function calls to `oldFunctionName`
  root
    .find(j.CallExpression, {
      callee: { type: "Identifier", name: "oldFunctionName" },
    })
    .forEach((path) => {
      path.node.callee.name = "newFunctionName";
    });

  // Also rename the import if it exists
  root
    .find(j.ImportSpecifier, {
      imported: { name: "oldFunctionName" },
    })
    .forEach((path) => {
      path.node.imported.name = "newFunctionName";
      // Update local binding too
      if (path.node.local && path.node.local.name === "oldFunctionName") {
        path.node.local.name = "newFunctionName";
      }
    });

  return root.toSource({ quote: "single" });
};

// Run:
// jscodeshift -t rename-function.js src/**/*.js
```

### jscodeshift: Migrate API Pattern

```javascript
// codemod: migrate-fetch-to-axios.js
// Transform: fetch(url, { method: 'POST', body: JSON.stringify(data) })
// Into:     axios.post(url, data)

module.exports = function (fileInfo, api) {
  const j = api.jscodeshift;
  const root = j(fileInfo.source);
  let needsAxiosImport = false;

  root
    .find(j.CallExpression, { callee: { name: "fetch" } })
    .forEach((path) => {
      const args = path.node.arguments;
      if (args.length < 2) return;

      const url = args[0];
      const options = args[1];

      if (options.type !== "ObjectExpression") return;

      const methodProp = options.properties.find(
        (p) => p.key.name === "method" || p.key.value === "method"
      );
      const bodyProp = options.properties.find(
        (p) => p.key.name === "body" || p.key.value === "body"
      );

      if (!methodProp) return;

      const method = methodProp.value.value?.toLowerCase();
      if (!method) return;

      needsAxiosImport = true;

      // Build axios call
      let axiosArgs = [url];

      if (bodyProp && bodyProp.value.type === "CallExpression") {
        // Extract data from JSON.stringify(data)
        if (
          bodyProp.value.callee.object?.name === "JSON" &&
          bodyProp.value.callee.property?.name === "stringify"
        ) {
          axiosArgs.push(bodyProp.value.arguments[0]);
        }
      }

      // Replace fetch() with axios.method()
      j(path).replaceWith(
        j.callExpression(
          j.memberExpression(j.identifier("axios"), j.identifier(method)),
          axiosArgs
        )
      );
    });

  // Add axios import if needed
  if (needsAxiosImport) {
    const axiosImport = j.importDeclaration(
      [j.importDefaultSpecifier(j.identifier("axios"))],
      j.literal("axios")
    );

    const body = root.find(j.Program).get("body");
    body.unshift(axiosImport);
  }

  return root.toSource();
};
```

### libCST: Python Codemod

```python
"""
Codemod: Migrate from unittest assertions to pytest assertions.
Transform: self.assertEqual(a, b) → assert a == b
"""
import libcst as cst
import libcst.matchers as m

class UnittestToPytestTransformer(cst.CSTTransformer):
    """Transform unittest-style assertions to pytest-style."""

    ASSERTION_MAP = {
        "assertEqual": "==",
        "assertNotEqual": "!=",
        "assertTrue": None,  # Special handling
        "assertFalse": None,
        "assertIs": "is",
        "assertIsNot": "is not",
        "assertIn": "in",
        "assertNotIn": "not in",
        "assertIsNone": None,
        "assertIsNotNone": None,
        "assertGreater": ">",
        "assertGreaterEqual": ">=",
        "assertLess": "<",
        "assertLessEqual": "<=",
    }

    def leave_Expr(
        self, original_node: cst.Expr, updated_node: cst.Expr
    ) -> cst.BaseStatement:
        # Match self.assertXxx(...) calls
        if not m.matches(
            updated_node.value,
            m.Call(func=m.Attribute(value=m.Name("self"))),
        ):
            return updated_node

        call = updated_node.value
        method_name = call.func.attr.value

        if method_name not in self.ASSERTION_MAP:
            return updated_node

        args = [arg.value for arg in call.args]
        operator = self.ASSERTION_MAP[method_name]

        # Binary comparison: assertEqual(a, b) → assert a == b
        if operator and len(args) >= 2:
            comparison = cst.Comparison(
                left=args[0],
                comparisons=[
                    cst.ComparisonTarget(
                        operator=self._get_cst_operator(operator),
                        comparator=args[1],
                    )
                ],
            )
            return updated_node.with_changes(
                value=cst.Assert(test=comparison)
            )

        # assertTrue(x) → assert x
        if method_name == "assertTrue" and len(args) >= 1:
            return updated_node.with_changes(
                value=cst.Assert(test=args[0])
            )

        # assertFalse(x) → assert not x
        if method_name == "assertFalse" and len(args) >= 1:
            return updated_node.with_changes(
                value=cst.Assert(test=cst.UnaryOperation(
                    operator=cst.Not(),
                    expression=args[0],
                ))
            )

        return updated_node

    def _get_cst_operator(self, op_str: str):
        ops = {
            "==": cst.Equal(),
            "!=": cst.NotEqual(),
            ">": cst.GreaterThan(),
            ">=": cst.GreaterThanEqual(),
            "<": cst.LessThan(),
            "<=": cst.LessThanEqual(),
            "in": cst.In(),
            "not in": cst.NotIn(),
            "is": cst.Is(),
            "is not": cst.IsNot(),
        }
        return ops[op_str]

# Run the codemod
def run_codemod(file_path: str):
    with open(file_path) as f:
        source = f.read()

    tree = cst.parse_module(source)
    modified = tree.visit(UnittestToPytestTransformer())

    with open(file_path, "w") as f:
        f.write(modified.code)
```

---

## IDE Refactoring Features

### VS Code Refactoring

| Refactoring | Keyboard Shortcut | Scope |
|------------|-------------------|-------|
| Rename symbol | F2 | Project-wide |
| Extract function | Ctrl+Shift+R | Selection |
| Extract variable | Ctrl+Shift+R | Selection |
| Move to new file | Quick Fix menu | Single symbol |
| Inline variable | Quick Fix menu | Single variable |
| Convert to template literal | Quick Fix menu | String concatenation |

### IntelliJ IDEA Refactoring

| Refactoring | Keyboard Shortcut | Scope |
|------------|-------------------|-------|
| Rename | Shift+F6 | Project-wide with usages |
| Extract method | Ctrl+Alt+M | Selection |
| Extract variable | Ctrl+Alt+V | Expression |
| Extract interface | Refactor menu | Class |
| Inline | Ctrl+Alt+N | Variable, method, or class |
| Move | F6 | Class, file, or package |
| Change signature | Ctrl+F6 | Method parameters |
| Pull members up | Refactor menu | Inheritance |
| Push members down | Refactor menu | Inheritance |
| Safe delete | Alt+Delete | Verify no usages before deleting |

### When IDE Refactoring Is Sufficient

```
Number of files to change?
├── 1-5 files → IDE refactoring (rename, extract, inline)
├── 5-50 files → IDE refactoring OR simple codemod
├── 50-500 files → Codemod required
└── 500+ files → Codemod + staged rollout
```

---

## Large-Scale Refactoring

### Meta's Approach to Large-Scale Codemods

Meta (Facebook) runs codemods across millions of files. Their approach:

1. **Write the codemod** -- Transform the code pattern
2. **Test on a sample** -- Run on 100 files, review manually
3. **Dry-run at scale** -- Generate diffs for all files without writing
4. **Human review** -- Sample review of generated diffs
5. **Apply in batches** -- Commit in groups of 100-500 files
6. **CI validation** -- Full test suite runs on each batch

### Batch Execution Pattern

```bash
#!/bin/bash
# run-codemod-batched.sh
# Run a codemod in batches with CI validation

CODEMOD="$1"
BATCH_SIZE=100
FILES=$(find src -name "*.ts" -type f)
TOTAL=$(echo "$FILES" | wc -l)
BATCH=0

echo "Running codemod: $CODEMOD"
echo "Total files: $TOTAL"
echo "Batch size: $BATCH_SIZE"

echo "$FILES" | while mapfile -t -n $BATCH_SIZE batch && [ ${#batch[@]} -gt 0 ]; do
    BATCH=$((BATCH + 1))
    echo ""
    echo "=== Batch $BATCH (${#batch[@]} files) ==="

    # Apply codemod to this batch
    jscodeshift -t "$CODEMOD" "${batch[@]}"

    # Run tests
    echo "Running tests..."
    if ! npm test 2>/dev/null; then
        echo "TESTS FAILED in batch $BATCH. Reverting..."
        git checkout -- "${batch[@]}"
        echo "Reverted. Investigate and fix codemod."
        exit 1
    fi

    # Commit batch
    git add "${batch[@]}"
    git commit -m "codemod: $(basename "$CODEMOD" .js) (batch $BATCH)"

    echo "Batch $BATCH committed successfully."
done

echo ""
echo "All batches complete."
```

### Dry-Run and Diff Generation

```bash
# jscodeshift: dry-run mode (prints to stdout, doesn't modify files)
jscodeshift -t my-codemod.js --dry --print src/

# Generate diff without applying
jscodeshift -t my-codemod.js --dry src/ 2>&1 | tee codemod-preview.diff

# Count affected files
jscodeshift -t my-codemod.js --dry src/ 2>&1 | grep "^Modified" | wc -l

# Semgrep: autofix with diff preview
semgrep --config my-rules.yaml --autofix --dryrun src/
```

---

## Safety Verification

### Post-Codemod Verification Checklist

- [ ] **TypeScript compiles** -- `tsc --noEmit` passes
- [ ] **Linter passes** -- `eslint .` or equivalent
- [ ] **Unit tests pass** -- Full test suite green
- [ ] **Integration tests pass** -- API contract tests green
- [ ] **No behavior change** -- Characterization tests green
- [ ] **No import cycles** -- `madge --circular src/`
- [ ] **No dead code introduced** -- `ts-prune` or `vulture`
- [ ] **Bundle size stable** -- Size diff within 5%
- [ ] **Manual review of sample** -- Spot-check 10 transformed files

### Automated Verification Pipeline

```yaml
# .github/workflows/codemod-verify.yaml
name: Codemod Verification

on:
  pull_request:
    branches: [main]

jobs:
  verify:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4

      - name: Type check
        run: npx tsc --noEmit

      - name: Lint
        run: npx eslint src/ --max-warnings=0

      - name: Unit tests
        run: npm test

      - name: Check for circular imports
        run: npx madge --circular src/

      - name: Bundle size check
        run: |
          npm run build
          CURRENT_SIZE=$(du -sb dist/ | cut -f1)
          echo "Bundle size: $CURRENT_SIZE bytes"
          # Compare against baseline
          BASELINE=$(cat .bundle-size-baseline 2>/dev/null || echo 0)
          DIFF=$((CURRENT_SIZE - BASELINE))
          if [ $DIFF -gt 50000 ]; then
            echo "::warning::Bundle size increased by $DIFF bytes"
          fi
```

---

## Codemod Testing Strategies

### Unit Testing Codemods

```javascript
// __tests__/rename-function-codemod.test.js
const { applyTransform } = require("jscodeshift/dist/testUtils");
const transform = require("../rename-function");

describe("rename-function codemod", () => {
  it("renames function calls", () => {
    const input = `
      import { oldFunctionName } from './utils';
      const result = oldFunctionName(arg1, arg2);
    `;

    const expected = `
      import { newFunctionName } from './utils';
      const result = newFunctionName(arg1, arg2);
    `;

    const output = applyTransform(transform, {}, { source: input });
    expect(output.trim()).toBe(expected.trim());
  });

  it("handles already-renamed code (idempotent)", () => {
    const input = `
      import { newFunctionName } from './utils';
      const result = newFunctionName(arg1);
    `;

    const output = applyTransform(transform, {}, { source: input });
    expect(output.trim()).toBe(input.trim());
  });

  it("does not rename unrelated functions", () => {
    const input = `
      const result = unrelatedFunction(arg);
    `;

    const output = applyTransform(transform, {}, { source: input });
    expect(output.trim()).toBe(input.trim());
  });

  it("handles no matches gracefully", () => {
    const input = `console.log("hello");`;
    const output = applyTransform(transform, {}, { source: input });
    expect(output.trim()).toBe(input.trim());
  });
});
```

### Testing Python Codemods

```python
"""
Test suite for Python codemods using libCST.
"""
import pytest
import libcst as cst
from codemods.unittest_to_pytest import UnittestToPytestTransformer

def apply_codemod(source: str) -> str:
    tree = cst.parse_module(source)
    modified = tree.visit(UnittestToPytestTransformer())
    return modified.code

def test_assertEqual_transforms():
    input_code = 'self.assertEqual(result, 42)'
    expected = 'assert result == 42'
    assert apply_codemod(input_code).strip() == expected

def test_assertTrue_transforms():
    input_code = 'self.assertTrue(is_valid)'
    expected = 'assert is_valid'
    assert apply_codemod(input_code).strip() == expected

def test_assertFalse_transforms():
    input_code = 'self.assertFalse(is_deleted)'
    expected = 'assert not is_deleted'
    assert apply_codemod(input_code).strip() == expected

def test_preserves_non_assertion_code():
    input_code = 'result = calculate(x, y)'
    assert apply_codemod(input_code).strip() == input_code

def test_idempotent_on_already_transformed():
    input_code = 'assert result == 42'
    assert apply_codemod(input_code).strip() == input_code

def test_preserves_comments():
    input_code = '# Check the result\nself.assertEqual(result, 42)'
    output = apply_codemod(input_code)
    assert '# Check the result' in output
    assert 'assert result == 42' in output
```

### Test Fixtures Pattern

```
codemods/
  __tests__/
    __fixtures__/
      rename-function/
        input.js       # Before codemod
        output.js      # Expected after codemod
      migrate-api/
        input.ts
        output.ts
    rename-function.test.js
    migrate-api.test.js
```

```javascript
// Generic fixture-based test runner
const fs = require("fs");
const path = require("path");
const { applyTransform } = require("jscodeshift/dist/testUtils");

function testFixture(codemodName, fixtureName) {
  const fixtureDir = path.join(__dirname, "__fixtures__", codemodName);
  const input = fs.readFileSync(path.join(fixtureDir, `${fixtureName}.input.js`), "utf8");
  const expected = fs.readFileSync(path.join(fixtureDir, `${fixtureName}.output.js`), "utf8");
  const transform = require(`../${codemodName}`);

  const output = applyTransform(transform, {}, { source: input });
  expect(output.trim()).toBe(expected.trim());
}
```

---

## Migration Codemods for Framework Upgrades

### React Class to Functional Components

```javascript
// codemod: class-to-functional.js (simplified)
module.exports = function (fileInfo, api) {
  const j = api.jscodeshift;
  const root = j(fileInfo.source);

  root
    .find(j.ClassDeclaration, {
      superClass: { name: "Component" },
    })
    .forEach((path) => {
      const className = path.node.id.name;
      const renderMethod = path.node.body.body.find(
        (m) => m.type === "ClassMethod" && m.key.name === "render"
      );

      if (!renderMethod) return;

      // Create functional component
      const funcComponent = j.variableDeclaration("const", [
        j.variableDeclarator(
          j.identifier(className),
          j.arrowFunctionExpression(
            [j.identifier("props")],
            renderMethod.body
          )
        ),
      ]);

      j(path).replaceWith(funcComponent);
    });

  return root.toSource();
};
```

### Common Framework Migration Codemods

| Migration | Tool | Notes |
|-----------|------|-------|
| React class → functional | jscodeshift | Meta provides official codemods |
| Vue 2 → Vue 3 | @vue/compat + codemods | Vue CLI migration helper |
| Angular upgrade | ng update | Built-in schematics |
| Express 4 → 5 | Custom jscodeshift | Middleware signature changes |
| Jest 28 → 29 | jest-codemods | Official migration toolkit |
| Python 2 → 3 | 2to3, futurize | Built into Python stdlib |
| jQuery → vanilla JS | jscodeshift | Community codemods available |
| Moment.js → date-fns | Custom codemod | API surface changes |
| Enzyme → Testing Library | codemod-missing-await | Community codemod |

### Finding Existing Codemods

```bash
# Search npm for codemods
npm search codemod react
npm search jscodeshift migration

# Search GitHub for codemods
gh search repos "codemod jscodeshift" --sort stars

# React codemods (official)
npx @codemod-com/cli react/19/replace-string-ref

# Semgrep registry
semgrep --config "p/react-best-practices" src/
```

---

## Codemod Composition Patterns

### Sequential Composition

Run codemods in order, where each builds on the previous one.

```bash
#!/bin/bash
# run-migration.sh: Compose multiple codemods sequentially

echo "Step 1: Rename imports"
jscodeshift -t codemods/01-rename-imports.js src/

echo "Step 2: Update function signatures"
jscodeshift -t codemods/02-update-signatures.js src/

echo "Step 3: Migrate API calls"
jscodeshift -t codemods/03-migrate-api.js src/

echo "Step 4: Clean up unused imports"
jscodeshift -t codemods/04-remove-unused-imports.js src/

echo "Step 5: Run formatter"
npx prettier --write src/

echo "Step 6: Verify"
npx tsc --noEmit && npm test
```

### Pipeline Composition (libCST)

```python
"""
Compose multiple libCST transformers into a single pass.
More efficient than running each transformer separately.
"""
import libcst as cst
from typing import Sequence

def compose_transformers(
    source: str,
    transformers: Sequence[cst.CSTTransformer],
) -> str:
    """Apply multiple transformers in a single parse-transform-print cycle."""
    tree = cst.parse_module(source)

    for transformer in transformers:
        tree = tree.visit(transformer)

    return tree.code

# Usage
from codemods.rename_imports import RenameImportsTransformer
from codemods.update_signatures import UpdateSignaturesTransformer
from codemods.migrate_api import MigrateAPITransformer

result = compose_transformers(
    source=open("src/service.py").read(),
    transformers=[
        RenameImportsTransformer(),
        UpdateSignaturesTransformer(),
        MigrateAPITransformer(),
    ],
)
```

### Conditional Composition

```javascript
// codemod-runner.js: Apply codemods conditionally based on file analysis

module.exports = function (fileInfo, api) {
  const j = api.jscodeshift;
  const root = j(fileInfo.source);

  // Only apply React migration if file uses React
  const hasReactImport = root.find(j.ImportDeclaration, {
    source: { value: "react" },
  }).length > 0;

  if (hasReactImport) {
    // Apply React-specific transforms
    applyReactMigration(root, j);
  }

  // Only apply API migration if file imports the old API client
  const hasOldClient = root.find(j.ImportDeclaration, {
    source: { value: "@company/old-api-client" },
  }).length > 0;

  if (hasOldClient) {
    applyAPIMigration(root, j);
  }

  return root.toSource();
};
```

---

## Related Resources

- [Characterization Testing](./characterization-testing.md) - Verify behavior after automated refactoring
- [Code Smells Guide](./code-smells-guide.md) - Identify patterns to target with codemods
- [Refactoring Catalog](./refactoring-catalog.md) - Manual refactoring techniques
- [Strangler Fig Migration](./strangler-fig-migration.md) - Incremental system replacement
- [Tech Debt Management](./tech-debt-management.md) - Prioritizing what to codemod
- [Operational Patterns](./operational-patterns.md) - CI/CD integration for refactoring
- [OpenRewrite Recipe Testing](https://docs.openrewrite.org/authoring-recipes/recipe-testing) - Testing semantic rewrite recipes before wide rollout
- [ast-grep Rewrite Rules](https://ast-grep.github.io/guide/rewrite-rule.html) - Rule-driven AST rewrites for targeted mechanical changes
- [SKILL.md](../SKILL.md) - Parent skill overview

# Brownfield Agent Loop

How to point a coding agent at a large legacy codebase without it drifting, and how to
make the loop's exit condition something other than "the agent says it's done."

This reference owns **setup and loop construction**. It does not re-derive:

- what characterization tests are, or how to write them → [characterization-testing.md](characterization-testing.md)
- seams, sprout/wrap, dependency breaking → [legacy-code-strategies.md](legacy-code-strategies.md#seams-and-breaking-dependencies)
- strangler routing and dual-write → [strangler-fig-migration.md](strangler-fig-migration.md)
- how an agent's "refactor" silently changes behavior, and the merge gate that catches it →
  [`../SKILL.md#llm-agents-and-subtle-behavior-changes-during-refactors`](../SKILL.md)

Read those first. This file is the connective layer: repo → seam → gate → scoped task → loop.

## Contents

- [Why greenfield agent workflows fail here](#why-greenfield-agent-workflows-fail-here)
- [Precondition: the acceptance gate must exist first](#precondition-the-acceptance-gate-must-exist-first)
- [Step 1 — Graph the repo before prompting](#step-1--graph-the-repo-before-prompting)
- [Step 2 — Pick the seam from graph structure](#step-2--pick-the-seam-from-graph-structure)
- [Step 3 — Build the gate around the seam](#step-3--build-the-gate-around-the-seam)
- [Step 4 — Scope one task to one seam](#step-4--scope-one-task-to-one-seam)
- [Step 5 — Run the loop against the gate](#step-5--run-the-loop-against-the-gate)
- [Stop conditions](#stop-conditions)
- [Known traps](#known-traps)
- [Anti-patterns](#anti-patterns)

## Why greenfield agent workflows fail here

The standard agentic-coding loop assumes three things that a legacy repo does not provide:

| Greenfield assumption | Brownfield reality | Consequence |
|---|---|---|
| A test suite exists and encodes intent | Coverage is partial, stale, or absent | "Tests pass" proves nothing; the loop has no true signal |
| The spec is the source of truth | The *running system* is the source of truth, spec is lost | Agent optimizes toward the spec and breaks undocumented behavior real users depend on |
| Context fits: the agent can read what it needs | 200k+ LOC, implicit coupling, no module boundaries | Agent reads a plausible subset, misses the caller that matters |

The failure is not that the agent writes bad code. It is that **the loop has no trustworthy
acceptance signal**, so iteration converges on "looks right" instead of "behaves the same."
Every step below exists to manufacture that signal before the agent starts.

Success rates on agent-driven multi-file legacy work are materially below marketing claims
(see the scope-creep note in the parent skill, flagged unverified). Plan for a scoped,
gated, human-reviewed loop — not an overnight autonomous rewrite.

## Precondition: the acceptance gate must exist first

**Do not start the loop until a behavior-preservation gate exists that the agent did not write.**

This is the single load-bearing rule in this file. An agent-authored test suite passing an
agent-authored refactor is a closed loop with no external reference — it will converge, and
it will converge on the agent's own misunderstanding.

Order matters:

1. Human (or agent under human review) writes characterization tests against **current**
   behavior, on the pre-change code.
2. Those tests are committed and green **before** any refactor prompt is issued.
3. The refactor loop may not modify them. A diff touching them is disqualifying until a
   human justifies it.

If you cannot build a gate for a region of code, that region is not yet eligible for an
agent loop. Shrink the scope until you can, or do it by hand.

## Step 1 — Graph the repo before prompting

Ad hoc file reads on a large legacy repo produce a plausible-but-partial mental model —
the "lost in the middle" failure. Build the graph artifact first and let the agent query it
instead of guessing.

Use [`../../dev-context-code-graph/SKILL.md`](../../dev-context-code-graph/SKILL.md).
Switch to graph-first context when the repo exceeds ~500 source files or ~5k symbol nodes.

```bash
# from dev-context-code-graph/scripts/
python3 scan_code_repo.py     # discover
python3 build_code_graph.py   # emit graphs/code-graph.json
python3 validate_code_graph.py
```

What you need out of it before choosing anything:

| Query | Why it matters in brownfield |
|---|---|
| `--articulation-points` | Nodes whose removal disconnects the graph — the highest-risk things to touch, and often exactly where the seam wants to go |
| `--bridges` | Single edges holding subsystems together; a natural strangler boundary |
| `--cycles` | Cyclic clusters cannot be extracted incrementally without breaking the cycle first — they change the plan |
| `--communities` | Empirical module boundaries in a codebase that has no declared ones |
| test-coverage cone | Where the safety net already exists vs where you must build it |

Recipes: [`../../dev-context-code-graph/references/query-recipes.md`](../../dev-context-code-graph/references/query-recipes.md)
— specifically the refactor-risk packet and test-coverage cone.

**Parse-gap caveat.** The graph's precision is bounded by parser coverage; heuristic parsers
lose precision on dynamic dispatch, reflection, string-keyed lookups, DI containers, and
config-driven wiring — all of which are *more* common in legacy code. Treat blast radius as
a lower bound on what is affected, never as proof that nothing else is.

## Step 2 — Pick the seam from graph structure

Do not let the agent choose what to refactor. Structure chooses; you confirm.

Rank candidate seams by:

1. **Isolability** — bridge or articulation point, few inbound edges, no cycle membership.
2. **Existing coverage** — inside an existing test-coverage cone beats greenfield gate work.
3. **Change pressure** — churn from git history; refactoring frozen code buys nothing.
4. **Blast radius** — smallest reachable set that still delivers the value.

The intersection of *high churn* and *low coverage* is the standard priority target: it is
where defects concentrate and where a gate pays for itself immediately.

Reject a seam if it sits inside a cycle. Break the cycle first as its own gated task
(extract interface / parameterize constructor —
[legacy-code-strategies.md](legacy-code-strategies.md#dependency-breaking-techniques)),
then re-graph. A cycle-spanning "refactor" prompt is how multi-file agent runs go
architecturally inconsistent.

## Step 3 — Build the gate around the seam

The gate is the loop's exit condition. It has three layers; the loop is only as trustworthy
as the weakest one.

| Layer | Purpose | Built by |
|---|---|---|
| Characterization tests on current behavior | Detects behavior change | Human-reviewed, pre-change, immutable during the loop |
| Contract/integration tests at the seam boundary | Detects interface breakage across the strangler edge | Human-reviewed |
| Mutation score on the touched boundary | Detects a gate that passes vacuously | Tooling — [mutation-testing.md](mutation-testing.md#mutation-score-as-the-ai-generated-test-validator) |

Golden-master capture is the fastest way to build layer 1 on code with wide output surface;
see [characterization-testing.md](characterization-testing.md#golden-master-pattern) and the
log-derived generation path
([characterization-testing.md](characterization-testing.md#generating-tests-from-logs))
when production logs can supply realistic inputs.

Pin nondeterminism (clock, RNG, network, ordering, locale) before capture, or the gate will
flake and the loop will "fix" the flake by weakening the assertion.

**Verify the gate can fail.** Before the loop runs, deliberately introduce a small behavior
change and confirm the gate goes red. A gate never observed failing is not known to be a gate.

## Step 4 — Scope one task to one seam

One seam, one task, one PR. The scope-creep failure mode is the dominant one in agent-driven
legacy work, and the countermeasure is task construction, not prompt politeness.

The task handed to the agent should carry:

- the seam boundary, named explicitly (files and symbols it may modify)
- the blast-radius list from the graph — the callers it must not break
- the gate command, verbatim, as the definition of done
- an explicit prohibition on modifying the gate or any test file
- the drift list from the parent skill as things to avoid, not things to fix
- an instruction to stop and report rather than expand if the change does not fit the seam

Anything outside the named seam is a separate task. "While I was in there" is the failure,
and it is cheaper to prevent in scoping than to catch in review.

Pair this with [`../../dev-workflow-planning/SKILL.md`](../../dev-workflow-planning/SKILL.md)
for the plan-document format and per-step verification checks, and with journaling
(same skill) once the work crosses context-compaction boundaries.

## Step 5 — Run the loop against the gate

The loop shape is ordinary; what makes it brownfield-safe is that the acceptance check is
external and immutable:

```text
scoped task + blast radius + gate command
  -> agent proposes change inside the named seam
  -> run the pre-existing gate (not agent-authored tests)
  -> red? feed the failure back, iterate
  -> green? mutation-check the touched boundary
  -> human reads the diff for drift patterns
  -> merge or split
```

Loop mechanics — stagnation detection, iteration caps, budget ceilings, circuit breakers —
are owned by [`../../ai-agents/references/autonomous-loop-patterns.md`](../../ai-agents/references/autonomous-loop-patterns.md).
Reuse them; do not re-implement. The brownfield-specific additions to that machinery:

- **Gate integrity check each iteration.** Assert the gate files are unmodified (hash them).
  This is the highest-value guard in the whole loop, because test-healing is the agent's
  default escape hatch when it cannot make the change work.
- **Re-graph after structural change.** Once symbols move, the graph is stale and blast
  radius is wrong. Regenerate before the next seam.
- **Cap iterations lower than greenfield.** Repeated failure against a *behavior* gate
  usually means the seam was wrong, not that the agent needs another attempt. Escalate to a
  human re-scope rather than spending the budget.

## Stop conditions

Stop the loop and return to a human when any of these fire:

| Condition | Why |
|---|---|
| Gate files modified | Test-healing; the loop's signal is compromised |
| Same test red 3 iterations running | The seam is wrong, not the attempt |
| Diff extends outside the named seam | Scope creep; split the task |
| Mutation score on touched boundary drops | Gate is now vacuous even if green |
| Agent proposes deleting or skipping a test | Disqualifying without human justification |
| Behavior change is *intended* | This is no longer a refactor; it needs its own spec and review |

## Known traps

- **Treating a green agent-authored suite as behavior preservation.** It is evidence about
  the agent's model of the code, not about the code.
- **Graphing once and trusting it all the way through a multi-seam migration.** Index
  staleness against a moved codebase is a documented cause of agent architectural drift.
- **Choosing the seam by reading code with the agent.** The agent will propose the region it
  understands best, which correlates with well-written code — the region that needed the
  work least.
- **Assuming blast radius is complete.** Parse gaps hide dynamic dispatch, reflection, and
  config-driven wiring. Grep the symbol name as a string before trusting the cone.
- **Running the loop on a cycle.** Extraction cannot be incremental inside a cycle; the agent
  will produce locally-sensible, globally-inconsistent edits.
- **Letting the loop run unattended on its first seam.** Calibrate on one supervised seam
  before trusting the gate to hold unattended.

## Anti-patterns

- pointing an autonomous loop at a legacy repo with no characterization gate and an
  overnight budget
- a single "modernize this module" task spanning many seams and many files
- allowing the same agent run to author both the change and its safety net
- accepting the agent's diff summary in place of reading the diff
- measuring progress in files touched or lines changed rather than seams closed behind a
  passing immutable gate
- re-running a failed seam with a bigger model instead of re-scoping it

## Related

- [`../SKILL.md`](../SKILL.md) — parent skill; drift patterns and the merge gate
- [characterization-testing.md](characterization-testing.md) — gate construction
- [legacy-code-strategies.md](legacy-code-strategies.md) — seams and dependency breaking
- [strangler-fig-migration.md](strangler-fig-migration.md) — incremental replacement routing
- [mutation-testing.md](mutation-testing.md) — validating the gate is not vacuous
- [mikado-method.md](mikado-method.md) — ordering prerequisite changes when a seam needs others first
- [`../../dev-context-code-graph/SKILL.md`](../../dev-context-code-graph/SKILL.md) — graph artifacts and queries
- [`../../ai-agents/references/autonomous-loop-patterns.md`](../../ai-agents/references/autonomous-loop-patterns.md) — loop drivers, budgets, circuit breakers
- [`../../dev-workflow-planning/SKILL.md`](../../dev-workflow-planning/SKILL.md) — task scoping and journaling

# Characterization Testing

Golden master and approval testing techniques for preserving behavior during refactoring. Based on Michael Feathers' *Working Effectively with Legacy Code*.

## Contents

- [Characterization Test Theory](#characterization-test-theory)
- [Golden Master Pattern](#golden-master-pattern)
- [Approval Testing Libraries](#approval-testing-libraries)
- [When to Use Characterization Tests](#when-to-use-characterization-tests)
- [Generating Tests from Logs](#generating-tests-from-logs)
- [Maintaining Golden Masters](#maintaining-golden-masters)
- [Transitioning to Unit Tests](#transitioning-to-unit-tests)
- [Workflow Integration](#workflow-integration)
- [Related Resources](#related-resources)

---

## Characterization Test Theory

A characterization test documents what code actually does, not what it should do. The goal is to capture current behavior so you can refactor with confidence, even when you do not fully understand the code.

### Key Principles (Feathers)

1. **The code is the specification** -- existing behavior IS the requirement until proven otherwise
2. **Test what IS, not what SHOULD BE** -- do not fix bugs while characterizing
3. **Cover the boundary you will change** -- test the API surface, module boundary, or function you plan to refactor
4. **Use the tests as a safety net** -- if a characterization test fails after refactoring, you changed behavior

### When Characterization Tests Are Necessary

```
Is the code under test well-understood?
├── Yes → Does it have adequate unit tests?
│   ├── Yes → Refactor directly, existing tests protect you
│   └── No → Write unit tests first if feasible
└── No → Is it risky to change without tests?
    ├── Yes → Write characterization tests
    └── No → Consider the risk tolerance
        ├── High risk (money, auth, data) → Write characterization tests
        └── Low risk → Acceptable to refactor with integration tests only
```

### Legacy Extension Points in Shared Infrastructure

When modifying shared infrastructure internals (e.g., consumer packages, messaging abstractions), add explicit regression coverage for legacy extension points — not just the new behavior. New failure-handling modes are not safe if they break older extension points that existing adopters rely on. Specifically:
- Shared-consumer composition and fan-out behavior
- Custom subscription registration (`IMessageSubscription` and similar)
- Any public interface where downstream services have built their own behavior on top of the shared package

### The Characterization Test Workflow

```
1. Pick the boundary you'll refactor
2. Write tests that call the code and record actual outputs
3. Assert that outputs match the recorded values
4. Run the full characterization suite → all green (by definition)
5. Refactor the internals
6. Run the suite again → if anything fails, you changed behavior
7. Investigate: was the behavior change intentional?
   - Yes → Update the golden master
   - No → Revert the refactoring step
```

---

## Golden Master Pattern

The golden master pattern captures a snapshot of current output and compares future runs against it.

### Basic Golden Master Implementation

```python
"""
Golden master testing: capture output, store as reference,
compare future runs against the reference.
"""
import json
import hashlib
from pathlib import Path
from typing import Any

GOLDEN_DIR = Path("tests/golden_masters")

def capture_golden_master(test_name: str, output: Any) -> Path:
    """Capture current output as the golden master."""
    GOLDEN_DIR.mkdir(parents=True, exist_ok=True)
    path = GOLDEN_DIR / f"{test_name}.golden.json"

    with open(path, "w") as f:
        json.dump(output, f, indent=2, sort_keys=True, default=str)

    print(f"Golden master captured: {path}")
    return path

def assert_matches_golden_master(test_name: str, actual: Any):
    """Compare current output against the golden master."""
    path = GOLDEN_DIR / f"{test_name}.golden.json"

    if not path.exists():
        # First run: capture the golden master
        capture_golden_master(test_name, actual)
        return

    with open(path) as f:
        expected = json.load(f)

    actual_normalized = json.loads(json.dumps(actual, sort_keys=True, default=str))

    assert actual_normalized == expected, (
        f"Output does not match golden master for {test_name}.\n"
        f"To update the golden master, delete {path} and re-run.\n"
        f"Diff:\n{_diff(expected, actual_normalized)}"
    )

def _diff(expected: Any, actual: Any, path: str = "$") -> str:
    """Generate a human-readable diff."""
    diffs = []
    if isinstance(expected, dict) and isinstance(actual, dict):
        all_keys = set(expected.keys()) | set(actual.keys())
        for key in sorted(all_keys):
            if key not in expected:
                diffs.append(f"  ADDED {path}.{key}: {actual[key]}")
            elif key not in actual:
                diffs.append(f"  REMOVED {path}.{key}: {expected[key]}")
            elif expected[key] != actual[key]:
                diffs.append(f"  CHANGED {path}.{key}: {expected[key]} → {actual[key]}")
    elif expected != actual:
        diffs.append(f"  CHANGED {path}: {expected} → {actual}")
    return "\n".join(diffs) if diffs else "  (no differences)"

# Usage in tests
import pytest

def test_order_total_calculation():
    """Characterization test for the legacy order calculator."""
    from legacy_app.orders import calculate_order_total

    order = {
        "items": [
            {"sku": "WIDGET-A", "qty": 3, "unit_price": 9.99},
            {"sku": "GADGET-B", "qty": 1, "unit_price": 24.50},
        ],
        "coupon": "SAVE10",
        "shipping": "standard",
    }

    result = calculate_order_total(order)
    assert_matches_golden_master("order_total_basic", result)

def test_order_total_edge_cases():
    """Characterization: empty cart, zero quantities, negative discounts."""
    from legacy_app.orders import calculate_order_total

    edge_cases = [
        {"items": [], "coupon": None, "shipping": "standard"},
        {"items": [{"sku": "X", "qty": 0, "unit_price": 10.0}], "coupon": None, "shipping": "express"},
        {"items": [{"sku": "X", "qty": 1, "unit_price": 0.0}], "coupon": "SAVE10", "shipping": "standard"},
    ]

    results = [calculate_order_total(case) for case in edge_cases]
    assert_matches_golden_master("order_total_edge_cases", results)
```

### Golden Master for Data Transformations

```python
"""
Characterize a data transformation pipeline.
Useful for ETL code, report generators, CSV processors.
"""
import csv
import io

def test_csv_export_golden_master():
    """Capture the exact CSV output of the legacy export."""
    from legacy_app.reports import generate_monthly_report

    report = generate_monthly_report(month=1, year=2026)

    # Capture the full output including headers, formatting, precision
    output = io.StringIO()
    writer = csv.writer(output)
    for row in report:
        writer.writerow(row)

    assert_matches_golden_master(
        "monthly_report_jan_2026",
        output.getvalue()
    )
```

---

## Approval Testing Libraries

Approval testing automates the golden master workflow with built-in diff tools and approval commands.

### Python: approvaltests

```python
"""
Using the approvaltests library for Python.
pip install approvaltests
"""
from approvaltests import verify, verify_all
from approvaltests.reporters import GenericDiffReporterFactory

def test_pricing_engine():
    """Approval test: captures output, shows diff on failure."""
    from legacy_app.pricing import calculate_price

    scenarios = [
        ("Basic item", calculate_price(item="widget", qty=1)),
        ("Bulk discount", calculate_price(item="widget", qty=100)),
        ("Premium item", calculate_price(item="premium-widget", qty=1)),
        ("Zero quantity", calculate_price(item="widget", qty=0)),
    ]

    # verify_all generates a formatted string and compares to approved file
    verify_all(
        "Pricing Scenarios",
        scenarios,
        lambda s: f"{s[0]}: ${s[1]:.2f}"
    )

# First run:
#   Creates test_pricing_engine.received.txt
#   Fails (no approved file yet)
#   Review the .received.txt file
#   Rename to test_pricing_engine.approved.txt to approve

# Subsequent runs:
#   Compares output against .approved.txt
#   If different: shows diff and fails
#   If same: passes silently
```

### Java: ApprovalTests.Java

```java
import org.approvaltests.Approvals;
import org.approvaltests.combinations.CombinationApprovals;
import org.junit.jupiter.api.Test;

class PricingEngineTest {

    @Test
    void testPricingCombinations() {
        // Test all combinations of inputs
        CombinationApprovals.verifyAllCombinations(
            this::calculatePrice,
            new String[]{"widget", "premium-widget", "service"},  // items
            new Integer[]{0, 1, 10, 100}                          // quantities
        );
    }

    private String calculatePrice(String item, Integer qty) {
        double price = LegacyPricingEngine.calculate(item, qty);
        return String.format("$%.2f", price);
    }
}
```

### JavaScript: Jest Snapshots

```javascript
// Jest has built-in snapshot testing (approval-style)

const { processOrder } = require("../legacy/orderProcessor");

describe("Order Processor (characterization)", () => {
  test("standard order output", () => {
    const order = {
      items: [
        { sku: "WIDGET-A", qty: 3, price: 9.99 },
        { sku: "GADGET-B", qty: 1, price: 24.50 },
      ],
      coupon: "SAVE10",
    };

    const result = processOrder(order);

    // First run: creates __snapshots__/orderProcessor.test.js.snap
    // Subsequent runs: compares against snapshot
    expect(result).toMatchSnapshot();
  });

  test("edge cases", () => {
    const cases = [
      { items: [], coupon: null },
      { items: [{ sku: "X", qty: 0, price: 10 }], coupon: null },
      { items: [{ sku: "X", qty: -1, price: 10 }], coupon: "INVALID" },
    ];

    cases.forEach((testCase, index) => {
      expect(processOrder(testCase)).toMatchSnapshot(`edge-case-${index}`);
    });
  });
});

// Update snapshots: npx jest --updateSnapshot
```

### Library Comparison

| Library | Language | Diff Tool | CI Support | Combination Testing |
|---------|----------|-----------|------------|-------------------|
| **approvaltests** | Python | System diff, custom | Yes | verify_all |
| **ApprovalTests.Java** | Java | IntelliJ, custom | Yes | CombinationApprovals |
| **Jest snapshots** | JavaScript | Built-in | Yes | Manual loops |
| **verify** (Rust) | Rust | insta crate | Yes | Manual |
| **ApprovalTests.Net** | C# | VS, Beyond Compare | Yes | CombinationApprovals |
| **SnapshotTesting** | Swift | Xcode | Yes | Manual |

---

## When to Use Characterization Tests

### Good Fit

| Scenario | Why Characterization Tests Work |
|----------|-------------------------------|
| **Untested legacy code** | No existing safety net; characterization creates one fast |
| **Complex algorithms** | Behavior is hard to specify; easier to capture than describe |
| **Data transformations** | Output format is the contract; golden master verifies it |
| **Before strangler migration** | Prove the new system matches the old one |
| **Regulatory/compliance code** | Must prove behavior did not change |
| **Third-party integration wrappers** | Capture expected responses for offline testing |

### Poor Fit

| Scenario | Better Alternative |
|----------|-------------------|
| **Nondeterministic output** (timestamps, random IDs) | Mock or normalize before comparing |
| **UI rendering** | Visual regression testing (Chromatic, Percy) |
| **Performance characteristics** | Benchmark tests |
| **Well-understood code with clear specs** | Write proper unit tests instead |
| **Code that is known to be buggy** | Fix bugs first, then characterize |

### Handling Nondeterminism

```python
"""
Normalize nondeterministic values before golden master comparison.
"""
import re
from datetime import datetime

def normalize_for_golden_master(output: dict) -> dict:
    """Remove or normalize nondeterministic fields."""
    normalized = json.loads(json.dumps(output))

    # Replace timestamps with placeholder
    if "created_at" in normalized:
        normalized["created_at"] = "<TIMESTAMP>"

    # Replace UUIDs with placeholder
    if "id" in normalized:
        normalized["id"] = "<UUID>"

    # Normalize floating point precision
    if "total" in normalized:
        normalized["total"] = round(normalized["total"], 2)

    return normalized

def normalize_string_output(text: str) -> str:
    """Normalize nondeterministic values in string output."""
    # Replace UUIDs
    text = re.sub(
        r'[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}',
        '<UUID>', text
    )
    # Replace ISO timestamps
    text = re.sub(
        r'\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(\.\d+)?Z?',
        '<TIMESTAMP>', text
    )
    return text
```

---

## Generating Tests from Logs

Use production or staging logs to generate realistic characterization test cases.

### Log-to-Test Generator

```python
"""
Generate characterization tests from request/response logs.
Input: structured logs with request + response pairs.
Output: pytest test file with golden master assertions.
"""
import json
from pathlib import Path

def generate_tests_from_logs(log_file: str, output_dir: str, max_tests: int = 50):
    """Parse request/response logs into test cases."""
    test_cases = []

    with open(log_file) as f:
        for line in f:
            entry = json.loads(line)
            if entry.get("type") != "http_request":
                continue

            test_cases.append({
                "method": entry["http"]["method"],
                "path": entry["http"]["path"],
                "request_body": entry.get("request_body"),
                "response_status": entry["http"]["status_code"],
                "response_body": entry.get("response_body"),
            })

            if len(test_cases) >= max_tests:
                break

    # Generate test file
    output = Path(output_dir) / "test_characterization_generated.py"
    with open(output, "w") as f:
        f.write('"""Auto-generated characterization tests from production logs."""\n')
        f.write("import pytest\n")
        f.write("import requests\n\n")
        f.write('BASE_URL = "http://localhost:8000"\n\n')

        for i, tc in enumerate(test_cases):
            f.write(f"def test_case_{i:04d}_{tc['method'].lower()}_{tc['path'].replace('/', '_').strip('_')}():\n")
            f.write(f'    """Characterized from production log entry."""\n')
            f.write(f'    response = requests.{tc["method"].lower()}(\n')
            f.write(f'        f"{{BASE_URL}}{tc["path"]}",\n')
            if tc["request_body"]:
                f.write(f"        json={json.dumps(tc['request_body'])},\n")
            f.write(f"    )\n")
            f.write(f"    assert response.status_code == {tc['response_status']}\n")
            if tc["response_body"]:
                f.write(f"    assert response.json() == {json.dumps(tc['response_body'])}\n")
            f.write("\n\n")

    print(f"Generated {len(test_cases)} test cases in {output}")

# Usage
generate_tests_from_logs(
    "logs/api-access-2026-01.jsonl",
    "tests/characterization/",
    max_tests=100
)
```

### Sampling Strategy for Log-Based Test Generation

- [ ] Include at least one example per endpoint
- [ ] Include examples for each HTTP status code returned
- [ ] Prioritize endpoints that will be refactored
- [ ] Include edge cases (empty bodies, large payloads, special characters)
- [ ] Normalize nondeterministic fields (timestamps, IDs) before storing as golden masters
- [ ] Limit to 50-200 tests to keep the suite fast

---

## Maintaining Golden Masters

### Golden Master Lifecycle

| Phase | Action | Who |
|-------|--------|-----|
| **Capture** | Run test for the first time, review and approve output | Developer starting refactoring |
| **Protect** | Golden masters committed to Git, fail CI on mismatch | CI/CD pipeline |
| **Update** | Intentional behavior change requires re-approval | Developer + reviewer |
| **Retire** | Replace with unit tests after refactoring complete | Developer |

### Update Workflow

```bash
#!/bin/bash
# update-golden-masters.sh
# Use when behavior change is intentional

echo "WARNING: This will overwrite golden masters with current output."
echo "Only run this after confirming the behavior change is intentional."
read -p "Continue? (y/N) " confirm

if [ "$confirm" != "y" ]; then
    echo "Aborted."
    exit 0
fi

# For pytest + custom golden master
find tests/golden_masters -name "*.golden.json" -delete
pytest tests/characterization/ -x

# For Jest snapshots
# npx jest --updateSnapshot

# For approvaltests
# mv tests/*.received.txt tests/*.approved.txt

echo "Golden masters updated. Review changes with: git diff tests/"
```

### CI Protection

```yaml
# .github/workflows/characterization-tests.yaml
name: Characterization Tests

on:
  pull_request:
    paths:
      - "src/**"
      - "tests/characterization/**"
      - "tests/golden_masters/**"

jobs:
  characterization:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4

      - name: Run characterization tests
        run: pytest tests/characterization/ -v

      - name: Check for unapproved golden master changes
        run: |
          if git diff --name-only | grep -q "golden_masters"; then
            echo "::error::Golden master files were modified during test run."
            echo "::error::This means behavior changed. Review carefully."
            git diff tests/golden_masters/
            exit 1
          fi
```

---

## Transitioning to Unit Tests

Characterization tests are temporary scaffolding. Replace them with proper unit tests as you understand the code better.

### Transition Process

```
Phase 1: Characterize
  - Write golden master tests around the boundary
  - Cover happy paths and edge cases
  - All tests pass (they capture current behavior)

Phase 2: Refactor
  - Extract methods, introduce seams, simplify
  - Characterization tests catch any behavior changes
  - Keep refactoring steps small

Phase 3: Understand
  - As you refactor, you learn what the code actually does
  - Document discovered behavior as comments or specs
  - Identify bugs vs features in the current behavior

Phase 4: Replace
  - Write unit tests for the refactored code
  - Each unit test replaces part of the characterization test
  - Unit tests test intent; characterization tests test behavior
  - Delete characterization tests when fully covered

Phase 5: Clean up
  - Remove golden master files
  - Remove characterization test infrastructure
  - Update CI to run only unit/integration tests
```

### Replacement Checklist

- [ ] Every characterization test has a corresponding unit test
- [ ] Unit tests cover the same edge cases
- [ ] Unit tests are faster than characterization tests
- [ ] Golden master files deleted from repository
- [ ] CI updated to exclude characterization test directory
- [ ] Documentation updated with discovered behavior notes

---

## Workflow Integration

### Refactoring with Characterization Tests: Step by Step

```bash
# 1. Create the characterization test branch
git checkout -b refactor/order-calculator

# 2. Write characterization tests
pytest tests/characterization/test_order_calculator.py -v
# All pass (capturing current behavior)

# 3. Commit the golden masters
git add tests/characterization/ tests/golden_masters/
git commit -m "Add characterization tests for order calculator"

# 4. Refactor in small steps
# ... make changes ...
pytest tests/characterization/test_order_calculator.py -v
# If any fail: you changed behavior. Investigate.

# 5. After refactoring is complete, write unit tests
pytest tests/unit/test_order_calculator.py -v

# 6. Verify unit tests cover characterization tests
pytest tests/ --cov=src/orders/calculator.py
# Coverage should be equal or better

# 7. Remove characterization tests
git rm tests/characterization/test_order_calculator.py
git rm tests/golden_masters/order_calculator_*.golden.json
git commit -m "Replace characterization tests with unit tests for order calculator"
```

---

## Related Resources

- [Legacy Code Strategies](./legacy-code-strategies.md) - Broader strategies for working with legacy code
- [Code Smells Guide](./code-smells-guide.md) - Identifying what to refactor
- [Refactoring Catalog](./refactoring-catalog.md) - Specific refactoring techniques
- [Strangler Fig Migration](./strangler-fig-migration.md) - Incremental migration using characterization tests
- [Automated Refactoring Tools](./automated-refactoring-tools.md) - Tool-assisted refactoring
- [Tech Debt Management](./tech-debt-management.md) - Prioritizing refactoring work
- [SKILL.md](../SKILL.md) - Parent skill overview

# Code Smells Guide

Comprehensive guide to identifying and fixing code smells based on Martin Fowler's catalog and modern best practices.

## Contents

- [What Are Code Smells?](#what-are-code-smells)
- [Bloaters](#bloaters)
- [Object-Orientation Abusers](#object-orientation-abusers)
- [Change Preventers](#change-preventers)
- [Dispensables](#dispensables)
- [Couplers](#couplers)
- [Modern Code Smells](#modern-code-smells)
- [Detection Tools](#detection-tools)
- [References](#references)

---

## What Are Code Smells?

**Definition**: Code smells are surface indications that usually correspond to deeper problems in the system. They're not bugs—they don't prevent the program from functioning. Instead, they indicate weaknesses in design that may slow down development or increase the risk of bugs in the future.

**Origin**: The term was popularized by Kent Beck and Martin Fowler in *Refactoring: Improving the Design of Existing Code*.

**Key Principle**: Code smells are subjective and context-dependent. What's a smell in one context might be acceptable in another.

---

## Bloaters

Code, methods, and classes that have increased to enormous proportions.

### Long Method

**Symptoms**:
- Method exceeds 20-30 lines
- Needs comments to explain sections
- Multiple levels of abstraction

**Why it's bad**:
- Hard to understand and maintain
- Difficult to reuse parts
- Higher chance of bugs

**Refactoring**:
- Extract Method
- Replace Temp with Query
- Decompose Conditional

```javascript
// Smell
function processOrder(order) {
  // Validate order (10 lines)
  if (!order.items) throw new Error('No items');
  // ... more validation

  // Calculate total (15 lines)
  let total = 0;
  for (const item of order.items) {
    total += item.price * item.quantity;
  }
  // ... more calculation

  // Apply discounts (20 lines)
  if (order.coupon) {
    // ... discount logic
  }

  // Save to database (10 lines)
  // ... database logic
}

// Fixed
function processOrder(order) {
  validateOrder(order);
  const total = calculateTotal(order);
  const discounted = applyDiscounts(total, order);
  saveOrder(order, discounted);
}
```

---

### Large Class

**Symptoms**:
- Class has 300+ lines
- Class has 10+ methods
- Class has many instance variables
- Class name includes "Manager", "Controller", "Handler" (God Object)

**Why it's bad**:
- Violates Single Responsibility Principle
- Hard to understand and maintain
- Difficult to test

**Refactoring**:
- Extract Class
- Extract Subclass
- Extract Interface

```typescript
// Smell
class UserManager {
  createUser() {}
  deleteUser() {}
  updateUser() {}
  authenticateUser() {}
  authorizeUser() {}
  sendWelcomeEmail() {}
  sendPasswordResetEmail() {}
  generateUserReport() {}
  exportUserData() {}
  importUserData() {}
  validateUserInput() {}
}

// Fixed
class UserService {
  createUser() {}
  deleteUser() {}
  updateUser() {}
}

class AuthenticationService {
  authenticateUser() {}
  authorizeUser() {}
}

class UserEmailService {
  sendWelcomeEmail() {}
  sendPasswordResetEmail() {}
}

class UserReportService {
  generateUserReport() {}
}

class UserDataService {
  exportUserData() {}
  importUserData() {}
}

class UserValidator {
  validateUserInput() {}
}
```

---

### Primitive Obsession

**Symptoms**:
- Using primitives instead of small objects for simple tasks
- Using constants for type codes
- Using string constants for field names

**Why it's bad**:
- Logic scattered across codebase
- No type safety
- Validation repeated everywhere

**Refactoring**:
- Replace Data Value with Object
- Introduce Parameter Object
- Replace Type Code with Class

```typescript
// Smell
function validateEmail(email: string): boolean {
  return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email);
}

function sendEmail(to: string, subject: string, body: string) {
  if (!validateEmail(to)) {
    throw new Error('Invalid email');
  }
  // send email
}

// Fixed
class Email {
  private constructor(private readonly value: string) {}

  static create(value: string): Email {
    if (!Email.isValid(value)) {
      throw new Error('Invalid email');
    }
    return new Email(value);
  }

  private static isValid(email: string): boolean {
    return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email);
  }

  toString(): string {
    return this.value;
  }
}

function sendEmail(to: Email, subject: string, body: string) {
  // No validation needed - Email is guaranteed valid
  // send email
}

// Usage
const email = Email.create('user@example.com');
sendEmail(email, 'Hello', 'World');
```

---

### Long Parameter List

**Symptoms**:
- Method has 4+ parameters
- Parameters have natural groupings
- Parameters always passed together

**Why it's bad**:
- Hard to remember parameter order
- Difficult to add new parameters
- Makes method calls verbose

**Refactoring**:
- Introduce Parameter Object
- Preserve Whole Object

```typescript
// Smell
function createUser(
  firstName: string,
  lastName: string,
  email: string,
  phone: string,
  street: string,
  city: string,
  state: string,
  zip: string
) {
  // ...
}

// Fixed
interface UserData {
  name: Name;
  contact: ContactInfo;
  address: Address;
}

interface Name {
  first: string;
  last: string;
}

interface ContactInfo {
  email: string;
  phone: string;
}

interface Address {
  street: string;
  city: string;
  state: string;
  zip: string;
}

function createUser(userData: UserData) {
  // ...
}
```

---

### Data Clumps

**Symptoms**:
- Same group of variables appears in multiple places
- Deleting one variable from group makes others meaningless

**Why it's bad**:
- Repeated data structures
- Missing abstraction
- Changes require updates in multiple places

**Refactoring**:
- Extract Class
- Introduce Parameter Object

```typescript
// Smell
function printInvoice(
  customerName: string,
  customerEmail: string,
  customerPhone: string,
  orderDate: Date,
  orderItems: Item[]
) {}

function sendReceipt(
  customerName: string,
  customerEmail: string,
  customerPhone: string,
  orderDate: Date
) {}

// Fixed
interface Customer {
  name: string;
  email: string;
  phone: string;
}

interface Order {
  customer: Customer;
  date: Date;
  items: Item[];
}

function printInvoice(order: Order) {}
function sendReceipt(order: Order) {}
```

---

## Object-Orientation Abusers

Incomplete or incorrect application of object-oriented principles.

### Switch Statements

**Symptoms**:
- Switch statement based on type code
- Same switch statement in multiple places
- Adding new type requires updating all switches

**Why it's bad**:
- Violates Open-Closed Principle
- Scattered logic
- Easy to forget updating all switches

**Refactoring**:
- Replace Conditional with Polymorphism
- Replace Type Code with State/Strategy

```typescript
// Smell
class Employee {
  type: string;

  getSalary(): number {
    switch (this.type) {
      case 'engineer':
        return 80000;
      case 'manager':
        return 100000;
      case 'salesman':
        return 60000 + this.getCommission();
      default:
        throw new Error('Unknown employee type');
    }
  }

  getBonus(): number {
    switch (this.type) {
      case 'engineer':
        return 5000;
      case 'manager':
        return 10000;
      case 'salesman':
        return this.getCommission() * 0.1;
      default:
        throw new Error('Unknown employee type');
    }
  }
}

// Fixed
abstract class Employee {
  abstract getSalary(): number;
  abstract getBonus(): number;
}

class Engineer extends Employee {
  getSalary(): number {
    return 80000;
  }

  getBonus(): number {
    return 5000;
  }
}

class Manager extends Employee {
  getSalary(): number {
    return 100000;
  }

  getBonus(): number {
    return 10000;
  }
}

class Salesman extends Employee {
  getSalary(): number {
    return 60000 + this.getCommission();
  }

  getBonus(): number {
    return this.getCommission() * 0.1;
  }

  private getCommission(): number {
    // calculate commission
    return 0;
  }
}
```

---

### Temporary Field

**Symptoms**:
- Field set only in certain circumstances
- Field is null or undefined most of the time
- Field used only by specific methods

**Why it's bad**:
- Confusing to understand object state
- Difficult to know when field is valid
- Hidden dependencies

**Refactoring**:
- Extract Class
- Replace Method with Method Object

```typescript
// Smell
class Order {
  items: Item[];
  discount?: number; // Only set during calculation

  calculateTotal(): number {
    const subtotal = this.items.reduce((sum, item) => sum + item.price, 0);
    this.discount = this.calculateDiscount(subtotal);
    return subtotal - this.discount;
  }

  private calculateDiscount(subtotal: number): number {
    // complex discount logic
    return 0;
  }
}

// Fixed
class Order {
  items: Item[];

  calculateTotal(): number {
    const calculator = new OrderCalculator(this.items);
    return calculator.calculateTotal();
  }
}

class OrderCalculator {
  private discount: number;

  constructor(private items: Item[]) {}

  calculateTotal(): number {
    const subtotal = this.calculateSubtotal();
    this.discount = this.calculateDiscount(subtotal);
    return subtotal - this.discount;
  }

  private calculateSubtotal(): number {
    return this.items.reduce((sum, item) => sum + item.price, 0);
  }

  private calculateDiscount(subtotal: number): number {
    // complex discount logic
    return 0;
  }
}
```

---

### Refused Bequest

**Symptoms**:
- Subclass uses only some methods/properties of superclass
- Subclass overrides methods to do nothing or throw errors
- Inheritance used just for code reuse

**Why it's bad**:
- Wrong inheritance hierarchy
- Violates Liskov Substitution Principle
- Misleading design

**Refactoring**:
- Replace Inheritance with Delegation
- Extract Superclass

```typescript
// Smell
class Rectangle {
  constructor(
    protected width: number,
    protected height: number
  ) {}

  setWidth(width: number) {
    this.width = width;
  }

  setHeight(height: number) {
    this.height = height;
  }

  getArea(): number {
    return this.width * this.height;
  }
}

class Square extends Rectangle {
  setWidth(width: number) {
    // Refused bequest - has to override to maintain square
    this.width = width;
    this.height = width;
  }

  setHeight(height: number) {
    // Refused bequest
    this.width = height;
    this.height = height;
  }
}

// Fixed
interface Shape {
  getArea(): number;
}

class Rectangle implements Shape {
  constructor(
    private width: number,
    private height: number
  ) {}

  setWidth(width: number) {
    this.width = width;
  }

  setHeight(height: number) {
    this.height = height;
  }

  getArea(): number {
    return this.width * this.height;
  }
}

class Square implements Shape {
  constructor(private size: number) {}

  setSize(size: number) {
    this.size = size;
  }

  getArea(): number {
    return this.size * this.size;
  }
}
```

---

### Alternative Classes with Different Interfaces

**Symptoms**:
- Two classes do similar things but have different method names
- Interfaces not matching when they should
- Duplicate functionality with different signatures

**Why it's bad**:
- Can't use polymorphism
- Duplicate code
- Harder to maintain

**Refactoring**:
- Rename Method
- Move Method
- Extract Superclass

```typescript
// Smell
class FileReader {
  readFromFile(path: string): string {
    // read file
    return '';
  }
}

class DatabaseReader {
  fetchFromDatabase(id: number): string {
    // read from database
    return '';
  }
}

// Fixed
interface DataReader {
  read(source: string): string;
}

class FileReader implements DataReader {
  read(path: string): string {
    // read file
    return '';
  }
}

class DatabaseReader implements DataReader {
  read(id: string): string {
    // read from database
    return '';
  }
}
```

---

## Change Preventers

Smells that make changes difficult and error-prone.

### Divergent Change

**Symptoms**:
- One class commonly changed in different ways for different reasons
- Class has many reasons to change (violates SRP)

**Why it's bad**:
- Changes affect unrelated functionality
- High chance of introducing bugs
- Difficult to understand impact

**Refactoring**:
- Extract Class
- Extract Superclass

```typescript
// Smell
class User {
  // Authentication concerns
  login() {}
  logout() {}
  changePassword() {}

  // Profile concerns
  updateProfile() {}
  uploadAvatar() {}

  // Notification concerns
  sendEmail() {}
  sendSMS() {}

  // Database concerns
  save() {}
  load() {}
}

// Fixed
class User {
  constructor(
    private auth: AuthenticationService,
    private profile: ProfileService,
    private notifications: NotificationService,
    private repository: UserRepository
  ) {}
}

class AuthenticationService {
  login() {}
  logout() {}
  changePassword() {}
}

class ProfileService {
  updateProfile() {}
  uploadAvatar() {}
}

class NotificationService {
  sendEmail() {}
  sendSMS() {}
}

class UserRepository {
  save() {}
  load() {}
}
```

---

### Shotgun Surgery

**Symptoms**:
- Single change requires many small changes in many classes
- Difficult to find all places that need changes
- Changes scattered across codebase

**Why it's bad**:
- Easy to miss necessary changes
- High chance of bugs
- Time-consuming

**Refactoring**:
- Move Method
- Move Field
- Inline Class

```typescript
// Smell
// Changing how we calculate price requires changes in all these classes
class Order {
  calculatePrice() {
    return this.items.reduce((sum, item) => sum + item.price * 1.1, 0);
  }
}

class Invoice {
  calculateTotal() {
    return this.items.reduce((sum, item) => sum + item.price * 1.1, 0);
  }
}

class ShoppingCart {
  getTotal() {
    return this.items.reduce((sum, item) => sum + item.price * 1.1, 0);
  }
}

// Fixed
class PriceCalculator {
  static calculateItemPrice(item: Item): number {
    return item.price * 1.1;
  }

  static calculateTotalPrice(items: Item[]): number {
    return items.reduce((sum, item) => sum + this.calculateItemPrice(item), 0);
  }
}

class Order {
  calculatePrice() {
    return PriceCalculator.calculateTotalPrice(this.items);
  }
}

class Invoice {
  calculateTotal() {
    return PriceCalculator.calculateTotalPrice(this.items);
  }
}

class ShoppingCart {
  getTotal() {
    return PriceCalculator.calculateTotalPrice(this.items);
  }
}
```

---

### Parallel Inheritance Hierarchies

**Symptoms**:
- Creating subclass requires creating subclass in another hierarchy
- Similar class names in different hierarchies

**Why it's bad**:
- Duplicate structure
- Changes require updates in parallel
- Easy to forget one hierarchy

**Refactoring**:
- Move Method
- Move Field
- Collapse Hierarchy

```typescript
// Smell
abstract class Employee {
  abstract getType(): string;
}

class Engineer extends Employee {
  getType() { return 'Engineer'; }
}

class Manager extends Employee {
  getType() { return 'Manager'; }
}

// Parallel hierarchy
abstract class EmployeeReport {
  abstract generate(): string;
}

class EngineerReport extends EmployeeReport {
  generate() { return 'Engineer report'; }
}

class ManagerReport extends EmployeeReport {
  generate() { return 'Manager report'; }
}

// Fixed
abstract class Employee {
  abstract getType(): string;
  abstract generateReport(): string; // Moved report generation here
}

class Engineer extends Employee {
  getType() { return 'Engineer'; }

  generateReport() {
    return 'Engineer report';
  }
}

class Manager extends Employee {
  getType() { return 'Manager'; }

  generateReport() {
    return 'Manager report';
  }
}
```

---

## Dispensables

Something pointless that should be removed.

### Comments

**Symptoms**:
- Method has long explanatory comment
- Comments explain what code does (not why)
- Commented-out code

**Why it's bad**:
- Code should be self-explanatory
- Comments become outdated
- Dead code clutters codebase

**Refactoring**:
- Extract Method
- Rename Method
- Introduce Assertion

```javascript
// Smell
function calculatePrice(order) {
  // Calculate the base price by multiplying quantity by item price
  let basePrice = order.quantity * order.itemPrice;

  // Apply discount if customer is premium
  // Discount is 10% for premium customers
  if (order.customer.isPremium) {
    basePrice = basePrice * 0.9;
  }

  // Add shipping cost based on weight
  // $5 per pound
  let shippingCost = order.weight * 5;

  return basePrice + shippingCost;
}

// Fixed
function calculatePrice(order) {
  const basePrice = calculateBasePrice(order);
  const discount = calculateDiscount(order, basePrice);
  const shippingCost = calculateShippingCost(order);
  return basePrice - discount + shippingCost;
}

function calculateBasePrice(order) {
  return order.quantity * order.itemPrice;
}

function calculateDiscount(order, basePrice) {
  return order.customer.isPremium ? basePrice * 0.1 : 0;
}

function calculateShippingCost(order) {
  const COST_PER_POUND = 5;
  return order.weight * COST_PER_POUND;
}
```

---

### Duplicate Code

**Symptoms**:
- Same code structure in multiple places
- Similar algorithms with minor differences
- Copy-pasted code blocks

**Why it's bad**:
- Changes must be made in multiple places
- Easy to miss one location
- Increases maintenance cost

**Refactoring**:
- Extract Method
- Pull Up Method
- Form Template Method

```typescript
// Smell
class Report {
  generatePDFReport() {
    // Validate input
    if (!this.data) throw new Error('No data');

    // Format for PDF
    const formatted = this.formatForPDF();

    // Generate report
    return this.generatePDF(formatted);
  }

  generateHTMLReport() {
    // Validate input (duplicate)
    if (!this.data) throw new Error('No data');

    // Format for HTML
    const formatted = this.formatForHTML();

    // Generate report
    return this.generateHTML(formatted);
  }
}

// Fixed
abstract class Report {
  generateReport(): string {
    this.validate();
    const formatted = this.format();
    return this.generate(formatted);
  }

  private validate() {
    if (!this.data) throw new Error('No data');
  }

  protected abstract format(): string;
  protected abstract generate(formatted: string): string;
}

class PDFReport extends Report {
  protected format(): string {
    return this.formatForPDF();
  }

  protected generate(formatted: string): string {
    return this.generatePDF(formatted);
  }
}

class HTMLReport extends Report {
  protected format(): string {
    return this.formatForHTML();
  }

  protected generate(formatted: string): string {
    return this.generateHTML(formatted);
  }
}
```

---

### Lazy Class

**Symptoms**:
- Class does too little to justify its existence
- Class has only a few methods
- Class is just a data holder

**Why it's bad**:
- Unnecessary complexity
- Extra files to maintain
- Cognitive overhead

**Refactoring**:
- Inline Class
- Collapse Hierarchy

```typescript
// Smell
class Address {
  street: string;
  city: string;
}

class Person {
  name: string;
  address: Address; // Just a data holder
}

// Fixed (if Address has no behavior)
class Person {
  name: string;
  street: string;
  city: string;
}
```

---

### Dead Code

**Symptoms**:
- Unused variables, parameters, methods, classes
- Unreachable code (after return)
- Code that's never called

**Why it's bad**:
- Clutters codebase
- Confuses developers
- Maintenance burden

**Refactoring**:
- Delete it!

```javascript
// Smell
function processOrder(order) {
  const oldCalculation = order.total * 1.1; // Never used
  return order.total * 1.05;

  console.log('Processing complete'); // Unreachable
}

function legacyFeature() {
  // Never called anywhere
}

// Fixed
function processOrder(order) {
  return order.total * 1.05;
}
```

---

### Speculative Generality

**Symptoms**:
- Abstract classes with only one subclass
- Unused parameters "for future use"
- Methods that aren't called
- Overcomplicated design "in case we need it"

**Why it's bad**:
- YAGNI (You Aren't Gonna Need It)
- Premature optimization
- Harder to understand

**Refactoring**:
- Collapse Hierarchy
- Inline Class
- Remove Parameter

```typescript
// Smell
abstract class PaymentProcessor {
  abstract process(amount: number, currency?: string, metadata?: any): Promise<void>;
}

class CreditCardProcessor extends PaymentProcessor {
  async process(amount: number, currency?: string, metadata?: any): Promise<void> {
    // Only uses amount, currency and metadata never used
    await this.chargeCreditCard(amount);
  }
}

// Fixed (only one implementation, remove abstraction)
class CreditCardProcessor {
  async process(amount: number): Promise<void> {
    await this.chargeCreditCard(amount);
  }

  private async chargeCreditCard(amount: number): Promise<void> {
    // implementation
  }
}
```

---

## Couplers

Smells that contribute to excessive coupling between classes.

### Feature Envy

**Symptoms**:
- Method uses more features of another class than its own
- Method repeatedly accesses other object's data
- Method seems to belong to another class

**Why it's bad**:
- Logic is in wrong place
- Violates encapsulation
- Hard to maintain

**Refactoring**:
- Move Method
- Extract Method

```typescript
// Smell
class Order {
  items: Item[];

  calculateTotal(): number {
    let total = 0;
    for (const item of this.items) {
      // Accessing product details directly
      total += item.product.getPrice() * item.getQuantity();
      total -= item.product.getDiscount();
    }
    return total;
  }
}

// Fixed
class Order {
  items: Item[];

  calculateTotal(): number {
    return this.items.reduce((sum, item) => sum + item.getTotal(), 0);
  }
}

class Item {
  product: Product;
  quantity: number;

  getTotal(): number {
    return this.product.getDiscountedPrice() * this.quantity;
  }

  getQuantity(): number {
    return this.quantity;
  }
}

class Product {
  price: number;
  discount: number;

  getDiscountedPrice(): number {
    return this.price - this.discount;
  }
}
```

---

### Inappropriate Intimacy

**Symptoms**:
- Classes know too much about each other's internal details
- Classes access each other's private fields
- Bidirectional dependencies

**Why it's bad**:
- Tight coupling
- Changes cascade
- Hard to reuse classes separately

**Refactoring**:
- Move Method/Field
- Extract Class
- Hide Delegate

```typescript
// Smell
class Order {
  customer: Customer;

  getDiscount(): number {
    // Accessing customer's private implementation details
    if (this.customer.loyaltyPoints > 100) {
      return this.total * 0.1;
    }
    return 0;
  }
}

class Customer {
  loyaltyPoints: number; // Exposed to Order
}

// Fixed
class Order {
  customer: Customer;

  getDiscount(): number {
    return this.customer.calculateDiscount(this.total);
  }
}

class Customer {
  private loyaltyPoints: number;

  calculateDiscount(orderTotal: number): number {
    if (this.isLoyalCustomer()) {
      return orderTotal * 0.1;
    }
    return 0;
  }

  private isLoyalCustomer(): boolean {
    return this.loyaltyPoints > 100;
  }
}
```

---

### Message Chains

**Symptoms**:
- Code like `a.getB().getC().getD().doSomething()`
- Long chains of method calls
- Breaking Law of Demeter

**Why it's bad**:
- Client depends on navigation structure
- Changes in chain break client
- Tight coupling

**Refactoring**:
- Hide Delegate
- Extract Method

```typescript
// Smell
class Customer {
  getManager(): Manager {
    return this.account.getDepartment().getManager();
  }
}

const manager = customer.getAccount().getDepartment().getManager();

// Fixed
class Customer {
  getManager(): Manager {
    return this.account.getManager();
  }
}

class Account {
  getManager(): Manager {
    return this.department.getManager();
  }
}

// Client code
const manager = customer.getManager();
```

---

### Middle Man

**Symptoms**:
- Class does nothing but delegate to another class
- Most methods are simple delegations
- Class adds no value

**Why it's bad**:
- Unnecessary indirection
- Extra maintenance
- Confusing design

**Refactoring**:
- Remove Middle Man
- Inline Method

```typescript
// Smell
class Person {
  department: Department;

  getManager(): Manager {
    return this.department.getManager();
  }

  getOffice(): Office {
    return this.department.getOffice();
  }

  getTeam(): Team {
    return this.department.getTeam();
  }
}

// Fixed
class Person {
  department: Department;
}

// Client accesses department directly
const manager = person.department.getManager();
```

---

## Modern Code Smells

### Callback Hell

**Symptoms**:
- Deeply nested callbacks
- Pyramid of doom
- Hard to read async code

**Refactoring**:
- Replace with Promises
- Use async/await

```javascript
// Smell
function getData(callback) {
  fetchUser((error, user) => {
    if (error) {
      callback(error);
    } else {
      fetchOrders(user.id, (error, orders) => {
        if (error) {
          callback(error);
        } else {
          processOrders(orders, (error, result) => {
            if (error) {
              callback(error);
            } else {
              callback(null, result);
            }
          });
        }
      });
    }
  });
}

// Fixed
async function getData() {
  const user = await fetchUser();
  const orders = await fetchOrders(user.id);
  const result = await processOrders(orders);
  return result;
}
```

---

### Prop Drilling (React)

**Symptoms**:
- Props passed through many components
- Intermediate components don't use props
- Deep component trees with props

**Refactoring**:
- Use Context API
- Use state management (Redux, Zustand)
- Component composition

```javascript
// Smell
function App() {
  const [user, setUser] = useState(null);
  return <Dashboard user={user} />;
}

function Dashboard({ user }) {
  return <Sidebar user={user} />;
}

function Sidebar({ user }) {
  return <UserMenu user={user} />;
}

function UserMenu({ user }) {
  return <div>{user.name}</div>;
}

// Fixed
const UserContext = createContext();

function App() {
  const [user, setUser] = useState(null);
  return (
    <UserContext.Provider value={user}>
      <Dashboard />
    </UserContext.Provider>
  );
}

function Dashboard() {
  return <Sidebar />;
}

function Sidebar() {
  return <UserMenu />;
}

function UserMenu() {
  const user = useContext(UserContext);
  return <div>{user.name}</div>;
}
```

---

### God Object (Anti-pattern)

**Symptoms**:
- Object knows or does too much
- Object has too many dependencies
- Object is hard to test

**Refactoring**:
- Extract Class
- Apply Single Responsibility Principle

---

## Detection Tools

### Static Analysis Tools

- **SonarQube** - Detects code smells, bugs, vulnerabilities
- **ESLint** - JavaScript/TypeScript code quality
- **Pylint** - Python code analysis
- **RuboCop** - Ruby static code analyzer
- **ReSharper** - .NET code quality
- **IntelliJ IDEA** - Built-in inspections

### AI-Assisted Detection

- **Editor/agent copilots** - Suggest local cleanups and summarize likely hotspots
- **Static analysis platforms** - Detect maintainability and security issues consistently
- **IDE inspections** - Highlight low-cohesion code, dead branches, and unused abstractions

---

## References

- **Refactoring: Improving the Design of Existing Code** - Martin Fowler
- **Code Smells Catalog** - https://luzkan.github.io/smells/
- **Refactoring.guru** - https://refactoring.guru/refactoring/smells
- **Clean Code** - Robert C. Martin

# Feature Flag Retirement

A step-by-step recipe for safely removing a feature flag after its rollout is complete. Leaving dead flags in the codebase adds cognitive load, creates stale branching, and risks accidental re-activation. Retire each flag within one release cycle of reaching 100 % rollout.

## Contents

- [When a Flag Is Ready to Retire](#when-a-flag-is-ready-to-retire)
- [Step-by-Step Recipe](#step-by-step-recipe)
- [Post-Mortem the Rollout](#post-mortem-the-rollout)
- [Common Pitfalls](#common-pitfalls)

---

## When a Flag Is Ready to Retire

A flag is a retirement candidate when all of the following are true:

- It has been at 100 % enabled (or 100 % disabled) for at least one full release cycle.
- No rollback has been triggered in that period.
- Monitoring shows no anomalies attributable to the flagged feature.
- The product owner or feature owner confirms permanent direction.

Automate candidate detection: query your flag management system (LaunchDarkly, Unleash, custom config) for flags with `percentage = 100` and `age > 30 days`.

---

## Step-by-Step Recipe

### 1. Identify All References

Find every call site before touching any code.

```bash
# Grep for the flag key across the codebase (exclude archives and build artifacts)
rg "MY_FEATURE_FLAG" --type-list   # verify file types
rg "MY_FEATURE_FLAG" -g '!**/node_modules/**' -g '!**/.archive/**' -l

# AST-level search for typed flag enums (TypeScript example)
npx ts-morph-grep "FeatureFlag.MY_FEATURE_FLAG"

# For Java: use your IDE's "Find Usages" or
grep -rn "MY_FEATURE_FLAG" src/ --include="*.java"
```

Produce a checklist:

- Source files (application code, configuration)
- Test files (unit, integration, E2E)
- Infrastructure-as-code (env vars, Terraform, Helm values)
- Analytics and monitoring dashboards
- Documentation and runbooks

### 2. Mark the Dead Branch

Before deleting anything, confirm which branch is "dead" (the branch that will never execute again):

- Flag was enabled → the `else` / `false` branch is dead.
- Flag was disabled → the `if` / `true` branch is dead.

Open a PR that adds a comment marking dead code. This creates a reviewable checkpoint and catches disagreements about direction before deletion begins.

### 3. Remove the Dead Branch First

Delete the dead code path and its tests. Keep the flag guard in place for now.

```diff
- if (featureEnabled(FeatureFlag.MY_FEATURE_FLAG)) {
-   return newImplementation(input);
- } else {
-   return legacyImplementation(input);   // dead branch
- }
+ if (featureEnabled(FeatureFlag.MY_FEATURE_FLAG)) {
+   return newImplementation(input);
+ }
```

Run full CI. Confirm no test covers the removed branch (a surviving test means the branch was not truly dead — stop and investigate).

### 4. Delete the Flag Guard

With the dead branch gone, remove the flag check itself, making the surviving code unconditional.

```diff
- if (featureEnabled(FeatureFlag.MY_FEATURE_FLAG)) {
-   return newImplementation(input);
- }
+ return newImplementation(input);
```

Run full CI again. The behavior is unchanged — only the conditional and flag lookup are removed.

### 5. Delete the Flag Definition Last

Only after the guard is gone, remove:

- The flag constant / enum value.
- The default value in configuration files.
- The flag registration call in your feature-flag SDK initializer.
- The flag entry in your flag management system (LaunchDarkly dashboard, Unleash DB, etc.).

Deleting the definition before removing all call sites causes compile errors or silent `false` fallback — always clean call sites first.

### 6. Update Analytics and Dashboards

Feature flags often gate instrumented events or dashboard segments. After removal:

- Remove or archive dashboard panels that segment by the flag.
- Update alert conditions that reference the flag state.
- Remove any A/B experiment tracking that used the flag as a variant key.
- Notify the data/analytics team so they do not query a non-existent dimension.

### 7. Remove from Tests

Clean up test scaffolding:

- Delete test helpers that override the flag value.
- Delete test cases that test the dead branch.
- Remove `FeatureFlag.MY_FEATURE_FLAG` from any test fixture or factory.

Leaving dead test helpers signals to future developers that the flag is still meaningful.

---

## Post-Mortem the Rollout

After retirement, run a lightweight post-mortem (15–30 min, async is fine):

| Question | Purpose |
|----------|---------|
| Did the flag enable safe rollback? Was rollback used? | Validate flag necessity |
| How long did the flag live from creation to retirement? | Identify if flags are being retired promptly |
| Were there any incidents linked to the flag state? | Feed back into flag hygiene policy |
| Was the retirement PR larger or smaller than expected? | Surface scope creep or missed call sites |

Store the summary in your team's decision log or post-mortem system. Patterns across multiple flag retirements reveal systemic issues (flags living too long, missed dashboard cleanup, etc.).

---

## Common Pitfalls

| Pitfall | Effect | Remedy |
|---------|--------|--------|
| Deleting the flag definition before removing call sites | Runtime `false` fallback silently activates dead branch | Always remove call sites first, definition last |
| Skipping the analytics/dashboard step | Stale segments cause misleading metrics | Add analytics to the retirement checklist |
| Retiring a flag before 100 % rollout confirmation | Removes the rollback path while risk is still live | Gate retirement on explicit product sign-off |
| Large retirement PRs that mix flag removal with unrelated refactors | Hard to review; blame history polluted | One PR per flag; no unrelated changes |

# Legacy Code Modernization Strategies

Comprehensive guide to safely refactoring, testing, and modernizing legacy codebases.

## Contents

- [What is Legacy Code?](#what-is-legacy-code)
- [The Legacy Code Dilemma](#the-legacy-code-dilemma)
- [Characterization Testing](#characterization-testing)
- [Strangler Fig Pattern](#strangler-fig-pattern)
- [Seams and Breaking Dependencies](#seams-and-breaking-dependencies)
- [Incremental Refactoring Strategies](#incremental-refactoring-strategies)
- [Dependency Breaking Techniques](#dependency-breaking-techniques)
- [Modernization Roadmap](#modernization-roadmap)
- [Tools for Legacy Code](#tools-for-legacy-code)
- [Common Pitfalls](#common-pitfalls)
- [Illustrative Patterns (Not Verified Case Studies)](#illustrative-patterns-not-verified-case-studies)
- [Best Practices Summary](#best-practices-summary)
- [References](#references)

---

## What is Legacy Code?

**Michael Feathers' Definition**: "Code without tests."

**Practical Definition**: Code that is:
- Difficult to understand
- Hard to change safely
- Lacking documentation
- Using outdated practices
- Missing automated tests
- Has unknown dependencies

---

## The Legacy Code Dilemma

**Catch-22**:
1. Can't refactor safely without tests
2. Can't add tests without refactoring
3. Can't understand code without changing it
4. Can't change code without understanding it

**Solution**: Break the cycle with characterization tests and incremental improvements.

---

## Characterization Testing

Tests that describe current behavior (even if buggy) before refactoring.

### Purpose

- Document current behavior
- Create safety net for refactoring
- Detect unintended changes
- Build confidence

### Process

**1. Identify Behavior**
```javascript
// Legacy code
function calculatePrice(order) {
  let price = order.quantity * order.unitPrice;
  if (order.customer.type == 'PREMIUM') {
    price = price * 0.9;
  }
  return price.toFixed(2); // Weird: returns string, not number
}
```

**2. Write Test for Current Behavior**
```javascript
describe('calculatePrice - characterization', () => {
  it('returns string (not number) - current behavior', () => {
    const order = {
      quantity: 10,
      unitPrice: 100,
      customer: { type: 'REGULAR' }
    };

    const result = calculatePrice(order);

    // Document current behavior (string, not number)
    expect(typeof result).toBe('string');
    expect(result).toBe('1000.00');
  });

  it('applies 10% discount for PREMIUM customers', () => {
    const order = {
      quantity: 10,
      unitPrice: 100,
      customer: { type: 'PREMIUM' }
    };

    const result = calculatePrice(order);
    expect(result).toBe('900.00');
  });

  it('uses == for comparison (loose equality)', () => {
    const order = {
      quantity: 10,
      unitPrice: 100,
      customer: { type: 'PREMIUM' } // String 'PREMIUM'
    };

    // Test passes even with loose equality
    expect(calculatePrice(order)).toBe('900.00');
  });
});
```

**3. Refactor with Confidence**
```javascript
function calculatePrice(order) {
  const price = order.quantity * order.unitPrice;
  const discount = order.customer.type === 'PREMIUM' ? 0.9 : 1.0;
  return (price * discount).toFixed(2);
}
```

**4. Update Tests to Reflect Correct Behavior**
```javascript
describe('calculatePrice - after refactoring', () => {
  it('returns formatted price string', () => {
    const order = {
      quantity: 10,
      unitPrice: 100,
      customer: { type: 'REGULAR' }
    };

    expect(calculatePrice(order)).toBe('1000.00');
  });

  it('applies 10% discount for premium customers', () => {
    const order = {
      quantity: 10,
      unitPrice: 100,
      customer: { type: 'PREMIUM' }
    };

    expect(calculatePrice(order)).toBe('900.00');
  });

  it('uses strict equality for type checking', () => {
    const order = {
      quantity: 10,
      unitPrice: 100,
      customer: { type: 'premium' } // lowercase
    };

    // Now uses strict equality, case-sensitive
    expect(calculatePrice(order)).toBe('1000.00'); // No discount
  });
});
```

---

## Strangler Fig Pattern

Incrementally replace legacy system by building new system alongside and gradually migrating.

**Origin**: Named after strangler fig trees that grow around host trees.

### Process

**Phase 1: Identify Seam**
```
Legacy Monolith
├── User Management ← Start here (seam)
├── Order Processing
├── Payment Processing
└── Reporting
```

**Phase 2: Build New Implementation**
```javascript
// Legacy UserService (keeping as-is)
class LegacyUserService {
  getUser(id) {
    // Old database query
    return db.query('SELECT * FROM users WHERE id = ?', [id]);
  }
}

// New UserService (modern implementation)
class UserService {
  async getUser(id) {
    // New ORM, validation, caching
    return await User.findById(id);
  }
}
```

**Phase 3: Proxy/Router**
```javascript
class UserServiceProxy {
  constructor() {
    this.legacyService = new LegacyUserService();
    this.newService = new UserService();
    this.migrationPercentage = 10; // Start with 10%
  }

  async getUser(id) {
    // Feature flag determines routing
    if (this.shouldUseLegacy(id)) {
      return this.legacyService.getUser(id);
    }
    return await this.newService.getUser(id);
  }

  shouldUseLegacy(id) {
    // Gradually increase percentage
    const hash = this.hash(id);
    return hash % 100 >= this.migrationPercentage;
  }

  setMigrationPercentage(percentage) {
    this.migrationPercentage = percentage;
  }
}
```

**Phase 4: Gradual Migration**
```
Week 1-2: 10% traffic → new service
Week 3-4: 25% traffic → new service
Week 5-6: 50% traffic → new service
Week 7-8: 75% traffic → new service
Week 9-10: 100% traffic → new service
Week 11: Remove legacy code
```

**Phase 5: Monitoring**
```javascript
class UserServiceProxy {
  async getUser(id) {
    const startTime = Date.now();

    try {
      const result = this.shouldUseLegacy(id)
        ? await this.legacyService.getUser(id)
        : await this.newService.getUser(id);

      this.logMetrics({
        service: this.shouldUseLegacy(id) ? 'legacy' : 'new',
        latency: Date.now() - startTime,
        success: true
      });

      return result;
    } catch (error) {
      this.logMetrics({
        service: this.shouldUseLegacy(id) ? 'legacy' : 'new',
        latency: Date.now() - startTime,
        success: false,
        error: error.message
      });
      throw error;
    }
  }
}
```

---

## Seams and Breaking Dependencies

**Seam**: Place where you can alter behavior without editing source code.

### Types of Seams

**1. Object Seam (Dependency Injection)**
```javascript
// Before: Hard-coded dependency
class OrderProcessor {
  processOrder(order) {
    const payment = new PaymentService(); // Hard-coded
    payment.charge(order.total);
  }
}

// After: Injected dependency (seam)
class OrderProcessor {
  constructor(paymentService) {
    this.paymentService = paymentService;
  }

  processOrder(order) {
    this.paymentService.charge(order.total);
  }
}

// Can inject mock for testing
const mockPayment = { charge: jest.fn() };
const processor = new OrderProcessor(mockPayment);
```

**2. Preprocessing Seam (Build-time)**
```javascript
// Use environment variables or build flags
const API_URL = process.env.API_URL || 'https://legacy-api.com';

// Can override in tests
process.env.API_URL = 'https://test-api.com';
```

**3. Link Seam (Module replacement)**
```javascript
// Legacy module
// user-service.js
export function getUser(id) {
  // Legacy implementation
}

// New module with same interface
// user-service-v2.js
export function getUser(id) {
  // New implementation
}

// Import based on feature flag
const { getUser } = require(
  USE_NEW_SERVICE ? './user-service-v2' : './user-service'
);
```

---

## Incremental Refactoring Strategies

### Strategy 1: Sprout Method

Add new functionality without changing existing code.

```javascript
// Legacy code (don't touch)
function processOrder(order) {
  // 200 lines of complex legacy logic
  validateOrder(order);
  calculateTotals(order);
  applyDiscounts(order);
  saveOrder(order);
}

// New requirement: Send confirmation email
// Don't modify processOrder, sprout new method
function processOrderWithEmail(order) {
  processOrder(order); // Call legacy
  sendConfirmationEmail(order); // New functionality
}

function sendConfirmationEmail(order) {
  // New, testable code
  emailService.send({
    to: order.customer.email,
    subject: 'Order Confirmation',
    body: generateEmailBody(order)
  });
}
```

### Strategy 2: Wrap Method

Wrap legacy method with new code.

```javascript
// Legacy code (risky to change)
function saveUser(user) {
  // 100 lines of complex database logic
  db.users.insert(user);
}

// New requirement: Log user creation
// Wrap legacy method
function saveUserWithLogging(user) {
  const startTime = Date.now();

  try {
    saveUser(user); // Legacy call

    logger.info('User created', {
      userId: user.id,
      duration: Date.now() - startTime
    });
  } catch (error) {
    logger.error('User creation failed', {
      userId: user.id,
      error: error.message
    });
    throw error;
  }
}
```

### Strategy 3: Extract and Override

Extract method and override in subclass for testing.

```typescript
// Legacy code
class LegacyOrderProcessor {
  process(order: Order) {
    // Can't test because of hard-coded dependency
    const payment = new PaymentGateway();
    payment.charge(order.total);
  }
}

// Extract method
class ExtractedOrderProcessor {
  process(order: Order) {
    const payment = this.getPaymentGateway();
    payment.charge(order.total);
  }

  protected getPaymentGateway(): PaymentGateway {
    return new PaymentGateway();
  }
}

// Test by overriding
class TestableOrderProcessor extends ExtractedOrderProcessor {
  constructor(private mockPayment: PaymentGateway) {
    super();
  }

  protected getPaymentGateway(): PaymentGateway {
    return this.mockPayment;
  }
}
```

---

## Dependency Breaking Techniques

### Technique 1: Extract Interface

```typescript
// Legacy class with hard dependency
class UserController {
  private emailService = new EmailService(); // Hard-coded

  createUser(data: UserData) {
    const user = this.saveUser(data);
    this.emailService.sendWelcome(user);
  }
}

// Extract interface
interface IEmailService {
  sendWelcome(user: User): void;
}

class UserController {
  constructor(private emailService: IEmailService) {}

  createUser(data: UserData) {
    const user = this.saveUser(data);
    this.emailService.sendWelcome(user);
  }
}

// Can inject mock
class MockEmailService implements IEmailService {
  sendWelcome(user: User) {
    // Mock implementation
  }
}
```

### Technique 2: Parameterize Constructor

```typescript
// Before: Hard-coded dependencies
class OrderService {
  private db = new Database();
  private cache = new RedisCache();

  getOrder(id: string) {
    // ...
  }
}

// After: Parameterized
class OrderService {
  constructor(
    private db: IDatabase,
    private cache: ICache
  ) {}

  getOrder(id: string) {
    // ...
  }
}
```

### Technique 3: Extract and Override Call

```typescript
// Legacy code with global dependency
class ReportGenerator {
  generate() {
    const date = getCurrentDate(); // Global function
    // ...
  }
}

// Extract to method
class ReportGenerator {
  generate() {
    const date = this.getCurrentDate();
    // ...
  }

  protected getCurrentDate(): Date {
    return getCurrentDate();
  }
}

// Override in test
class TestableReportGenerator extends ReportGenerator {
  constructor(private testDate: Date) {
    super();
  }

  protected getCurrentDate(): Date {
    return this.testDate;
  }
}
```

---

## Modernization Roadmap

### 8-Week Incremental Plan

**Week 1: Assessment**
- [ ] Map codebase structure
- [ ] Identify critical paths
- [ ] Measure current metrics
- [ ] Create characterization tests for critical features

**Week 2: Quick Wins**
- [ ] Remove dead code
- [ ] Fix obvious bugs
- [ ] Add missing documentation
- [ ] Update dependencies (if safe)

**Week 3: Test Infrastructure**
- [ ] Set up test framework
- [ ] Add characterization tests
- [ ] Achieve 50% coverage on critical paths
- [ ] Set up CI/CD

**Week 4: Extract Methods**
- [ ] Break long methods into smaller ones
- [ ] Improve naming
- [ ] Extract magic numbers to constants
- [ ] Add inline documentation

**Week 5: Break Dependencies**
- [ ] Identify hard dependencies
- [ ] Extract interfaces
- [ ] Introduce dependency injection
- [ ] Add unit tests

**Week 6: Split Large Classes**
- [ ] Identify God objects
- [ ] Extract classes
- [ ] Apply Single Responsibility Principle
- [ ] Refactor coupling

**Week 7: Modernize Patterns**
- [ ] Replace callbacks with async/await
- [ ] Update to modern syntax
- [ ] Apply design patterns
- [ ] Improve error handling

**Week 8: Documentation & Cleanup**
- [ ] Document architecture
- [ ] Create developer guide
- [ ] Remove temporary fixes
- [ ] Final cleanup

---

## Tools for Legacy Code

### Code Analysis Tools

| Tool | Purpose |
|------|---------|
| SonarQube | Detect code smells, complexity |
| Understand | Visualize dependencies |
| JArchitect | Analyze .NET architecture |
| NDepend | .NET code quality metrics |
| Sourcetrail | Code exploration and navigation |

### Refactoring Tools

| Tool | Language | Features |
|------|----------|----------|
| IntelliJ IDEA | Multi-language | Automated refactoring, AI assistance |
| ReSharper | .NET | Safe refactoring, code analysis |
| Visual Studio | .NET | Built-in refactoring tools |
| VS Code | Multi-language | Extensions for refactoring |
| Eclipse | Java | Java refactoring tools |

### Testing Tools

| Tool | Purpose |
|------|---------|
| Approval Tests | Characterization testing |
| Mutation Testing | Test quality verification |
| Coverage.py | Python coverage analysis |
| Istanbul | JavaScript coverage |
| JaCoCo | Java code coverage |

---

## Common Pitfalls

### 1. Big Bang Rewrite

**Problem**: "Let's rewrite everything from scratch."

**Why it fails**:
- Underestimating complexity
- Losing hidden business logic
- No incremental value delivery
- High risk

**Solution**: Incremental refactoring with Strangler Fig pattern.

### 2. Refactoring Without Tests

**Problem**: Changing code without safety net.

**Why it fails**:
- Breaking existing behavior
- No way to verify correctness
- Fear of making changes

**Solution**: Write characterization tests first.

### 3. Perfect Code Obsession

**Problem**: "We must make everything perfect now."

**Why it fails**:
- Never finishing
- Over-engineering
- Losing focus on business value

**Solution**: Prioritize by impact, accept "good enough."

### 4. Ignoring Business Context

**Problem**: Refactoring for purity, not value.

**Why it fails**:
- No business benefit
- Wasted time
- Stakeholder frustration

**Solution**: Connect refactoring to business outcomes.

---

## Illustrative Patterns (Not Verified Case Studies)

The two scenarios below are composites used to illustrate sequencing and scale, not documented outcomes from a named, citable organization. Do not quote the numbers as benchmarks or repeat them as if they were sourced research — no public source backs specific percentages for "typical" strangler-fig or characterization-testing ROI. Treat any real engagement's numbers as unique to that codebase, team, and baseline.

### Pattern 1: Strangler Fig on a Large Monolith

**Context**: Multi-year PHP monolith, several hundred thousand LOC.

**Approach**: Strangler Fig pattern over 12-18 months
- Extract the highest-value, most-isolated service first (commonly user management)
- Extract remaining services in descending order of coupling risk
- Decommission legacy modules only after a soak period with parity metrics green

**What to expect directionally** (not a guaranteed outcome): shorter deploy cycles per extracted service, fewer regressions in the extracted area once characterization tests are in place, and slower velocity than hoped during the first extraction (the first slice always costs more than later ones because the proxy/seam infrastructure has to be built once).

### Pattern 2: Characterization-Test-First Modernization

**Context**: Legacy application with no automated tests.

**Approach**: Characterization tests + incremental refactoring over 6-12 months
- Add characterization tests around the highest-churn, highest-risk files first
- Refactor only where tests exist; expand coverage as you go
- Track coverage and incident rate as leading indicators, not vanity metrics

**What to expect directionally**: coverage climbs fastest in the first few months on high-churn files (Pareto effect), then slows as you reach rarely-touched code — decide explicitly whether to keep chasing coverage there or accept the risk, since coverage on cold code has low ROI.

---

## Best Practices Summary

1. **Write characterization tests** before refactoring
2. **Use Strangler Fig** for large rewrites
3. **Identify seams** to break dependencies
4. **Refactor incrementally** - small, safe steps
5. **Prioritize by business value** - not perfection
6. **Monitor metrics** during migration
7. **Use feature flags** for gradual rollout
8. **Document decisions** and learnings
9. **Celebrate progress** - even small wins
10. **Avoid big bang rewrites** - always incremental

---

## References

- **Working Effectively with Legacy Code** - Michael Feathers
- **Refactoring: Improving the Design of Existing Code (2nd Edition, 2018)** - Martin Fowler
- **Tidy First?: A Personal Exercise in Empirical Software Design (2023)** - Kent Beck — small, reversible "tidyings" done *before* a risky change to lower its cost; frames tidying as an economic decision (coupling/cohesion, optionality), not a moral one
- **Strangler Fig Application** - https://martinfowler.com/bliki/StranglerFigApplication.html
- **Legacy Code Rocks Podcast** - https://www.legacycode.rocks/
- **Code Complete** - Steve McConnell

# Mikado Method

A disciplined technique for making large, entangled changes safely: attempt the goal, let the compiler and tests surface what breaks, record each prerequisite as a node in a dependency graph, revert, then work leaf-first until the goal is reachable without breaking anything.

Named after the Japanese game Mikado (pick-up sticks), where you must remove individual sticks without disturbing the pile.

## Contents

- [Core Concept](#core-concept)
- [When to Use](#when-to-use)
- [The Five-Step Loop](#the-five-step-loop)
- [Building the Prerequisite Tree](#building-the-prerequisite-tree)
- [Leaf-First Execution](#leaf-first-execution)
- [Worked Example](#worked-example)
- [Common Pitfalls](#common-pitfalls)
- [Related Patterns](#related-patterns)

---

## Core Concept

A Mikado graph is a directed acyclic graph (DAG) where:

- The **root node** is the goal change (e.g., "Replace HashMap with ConcurrentHashMap in OrderService").
- Each **child node** is a prerequisite discovered by attempting the parent.
- **Leaf nodes** are changes with no further prerequisites — safe to merge immediately.

The graph makes hidden coupling visible before any code is committed. Progress is always releasable: every merged leaf is a valid, green state of the codebase.

---

## When to Use

Use the Mikado method when:

- The target change cascades into 5+ files or modules.
- There are no existing tests covering the area (so "just refactor" is unsafe).
- Previous attempts at the change caused a "while I'm here" sprawl that never shipped.
- The team cannot afford a long-lived feature branch; incremental progress must stay on `main`.

Do **not** use when:

- The change is contained to a single module with adequate test coverage. A direct refactor with a safety net is faster.
- The goal itself is unclear. Clarify the design before mapping prerequisites.

---

## The Five-Step Loop

```text
1. SET GOAL      — Write the goal as a single node at the root of the graph.
2. ATTEMPT       — Make the smallest code change that moves toward the goal.
3. RECORD        — When a compile/test failure surfaces, add a new child node
                   labeled with the prerequisite that must be true first.
4. REVERT        — Undo all changes from this attempt (git checkout -- .).
5. REPEAT        — Recurse: pick any leaf node and run the loop for it.
```

After reverting, the codebase is always green. Each iteration either adds leaves to the graph or (when the goal compiles and tests pass) removes the root node.

---

## Building the Prerequisite Tree

Start with a blank graph. The first attempt produces the first set of children.

```
Goal: Migrate OrderService to use Repository<Order>
  └── OrderService uses List<Order> directly
        └── List<Order> returned by LegacyDao
              ├── LegacyDao has no interface
              └── OrderService tests call LegacyDao.findAll() directly
```

Key practices:

- One node = one atomic, reviewable change.
- Keep node descriptions as imperative statements: "Extract OrderRepository interface from LegacyDao."
- If a node's attempt surfaces more prerequisites, add children before reverting.
- Use a whiteboard, index cards, or a lightweight tool (e.g., a plain text file committed as `mikado.md`) to track the graph during the refactor.

---

## Leaf-First Execution

A leaf node has no children: it can be done right now without breaking anything else.

Process:

1. Pick any leaf.
2. Make the change, run tests, verify CI is green.
3. Commit and merge to `main`.
4. Remove the node from the graph.
5. Re-examine its parent — it may now be a new leaf.

This produces a steady stream of small, shippable PRs. The root node is the last thing merged.

Benefits over a long-lived feature branch:

- No merge conflicts accumulating over weeks.
- Each PR is reviewable in isolation.
- The team can stop at any leaf boundary if priorities change; the codebase remains coherent.

---

## Worked Example

**Goal:** Replace `synchronized` blocks with `java.util.concurrent.locks.ReentrantLock` in `InventoryService`.

### Initial Attempt

Edit `InventoryService`, replace first `synchronized` block, run tests.

**Failure:** `InventoryServiceTest` mocks `synchronized` timing and asserts on thread state.

### Graph after first attempt

```
[ROOT] Replace synchronized with ReentrantLock in InventoryService
  └── [A] InventoryServiceTest couples to synchronized timing
```

Revert. Graph now has one leaf: A.

### Work leaf A

Edit `InventoryServiceTest`: remove timing assertions, replace with deterministic state checks. Tests pass. Commit: "refactor(test): decouple InventoryServiceTest from synchronized timing". Merge.

Remove A from graph. Root is now a leaf.

### Work root

Edit `InventoryService`, replace `synchronized` blocks with `ReentrantLock`. Tests pass. Commit: "refactor: replace synchronized with ReentrantLock in InventoryService". Merge.

Graph is empty. Done.

---

## Common Pitfalls

| Pitfall | Consequence | Remedy |
|---------|-------------|--------|
| Not reverting after each attempt | Accumulates broken state; graph becomes unreliable | Always revert before picking the next leaf |
| Nodes that are too large | Leaf merges cause their own cascades | Split: one node = one type or one method |
| Skipping the graph for "small" goals | Sprawl creeps back in | If you catch yourself editing 3+ files, start the graph |
| Merging non-leaf nodes | Breaks `main`; blocks other work | Only merge when tests are green with no open children |

---

## Related Patterns

- **Strangler Fig**: Mikado handles prerequisite untangling within a system; Strangler Fig handles routing traffic away from a legacy system. Use both when the goal spans architectural boundaries.
- **Characterization Testing**: Write characterization tests as Mikado leaf nodes when the legacy code has no coverage.
- **Branch by Abstraction / Expand-Contract**: Once prerequisites are cleared, use expand-contract to swap implementations without a flag or a long branch.

---

**Reference:** Ola Ellnestam & Daniel Brolund, *The Mikado Method* (The Pragmatic Bookshelf, 2014). Pattern origin: https://mikadomethod.info

# Mutation Testing

Mutation testing measures the quality of your test suite by deliberately introducing small faults (mutants) into the source code and checking whether your tests detect them. A test suite that passes against broken code provides false confidence; mutation testing makes that gap visible.

## Contents

- [Core Concept](#core-concept)
- [Mutation Score](#mutation-score)
- [Tooling by Ecosystem](#tooling-by-ecosystem)
- [CI Integration and Thresholds](#ci-integration-and-thresholds)
- [Performance Budget](#performance-budget)
- [Interpreting Results](#interpreting-results)
- [Incremental Workflow](#incremental-workflow)
- [Common Pitfalls](#common-pitfalls)

---

## Core Concept

A **mutant** is a copy of the source code with a single small change applied by the tool (a mutation operator):

- Arithmetic operator replacement: `+` → `-`
- Conditional boundary shift: `>` → `>=`
- Boolean literal flip: `true` → `false`
- Statement deletion: remove a `return` or assignment
- Negated condition: `if (x)` → `if (!x)`

Each mutant is compiled and your test suite is executed against it.

- **Killed mutant**: at least one test fails — the tests detected the fault.
- **Survived mutant**: all tests pass — the tests did not detect the fault.
- **Timed-out mutant**: execution exceeded the timeout — treated as killed.
- **No-coverage mutant**: the mutant is on a line not executed by any test — trivially survived.

---

## Mutation Score

```
mutation score = killed mutants / total mutants × 100
```

Where "total mutants" excludes no-coverage mutants in most tools (coverage must exist before mutation score is meaningful).

A mutation score of 80 % means 20 % of the injected faults went undetected. The gap between line coverage and mutation score reveals tests that execute code without asserting meaningful outcomes.

---

## Tooling by Ecosystem

### JavaScript / TypeScript — Stryker

**Homepage:** https://stryker-mutator.io

```bash
npm install --save-dev @stryker-mutator/core @stryker-mutator/jest-runner
npx stryker run
```

Minimal `stryker.config.mjs`:

```js
export default {
  testRunner: 'jest',
  coverageAnalysis: 'perTest',   // enables incremental on changed files
  reporters: ['html', 'progress', 'dashboard'],
  thresholds: { high: 80, low: 60, break: 50 },
};
```

- `coverageAnalysis: 'perTest'` maps each test to the mutants it can kill, enabling selective re-runs on PRs.
- Stryker also supports Mocha, Vitest, Karma, and Jasmine runners.
- `.NET` support via `dotnet-stryker` (`dotnet tool install -g dotnet-stryker`). Latest: Stryker.NET 4.14 (May 2026), with Microsoft Testing Platform (MTP) support in preview.
- Official Stryker VS Code plugin released November 2025 — run mutation tests directly from the editor.

### Python — mutmut

**Homepage:** https://github.com/boxed/mutmut

```bash
pip install mutmut
mutmut run          # run all mutants
mutmut results      # show surviving mutants
mutmut show <id>    # diff of a specific surviving mutant
```

- Integrates with pytest by default.
- Cache stored in `.mutmut-cache`; re-runs are fast after the first pass.
- Export results: `mutmut junitxml > mutmut-results.xml` for CI artifact upload.

### Python — cosmic-ray

**Homepage:** https://github.com/sixty-north/cosmic-ray

```bash
pip install cosmic-ray
cosmic-ray init config.toml session.sqlite
cosmic-ray exec session.sqlite
cr-report session.sqlite
```

- Session-based: work is stored in SQLite, enabling resumable runs.
- Supports distributed execution (Celery workers) for large codebases.
- Preferred when you need fine-grained operator control or distributed runs.

### Java / Kotlin — PIT (Pitest)

**Homepage:** https://pitest.org

Maven plugin:

```xml
<plugin>
  <groupId>org.pitest</groupId>
  <artifactId>pitest-maven</artifactId>
  <version>1.19.1</version>
  <configuration>
    <targetClasses><param>com.example.*</param></targetClasses>
    <mutationThreshold>75</mutationThreshold>
    <coverageThreshold>80</coverageThreshold>
  </configuration>
</plugin>
```

```bash
mvn org.pitest:pitest-maven:mutationCoverage
```

- HTML report at `target/pit-reports/`.
- Kotlin support via `pitest-kotlin` plugin.
- Incremental mode (`withHistory`): stores previous run state, only re-mutates changed classes.

---

## CI Integration and Thresholds

### Recommended threshold bands

| Band | Mutation Score | Action |
|------|----------------|--------|
| High | ≥ 80 % | Green; no gate triggered |
| Low | 60–79 % | Warning; notify but do not block |
| Break | < 50 % | Fail the build |

These are defaults in Stryker; calibrate for your domain. Safety-critical paths (auth, payments, data migrations) warrant a break threshold of 70 % or higher.

### GitHub Actions example (Node.js)

```yaml
- name: Mutation tests (PR only)
  if: github.event_name == 'pull_request'
  run: npx stryker run --incremental --incrementalFile .stryker-incremental.json
- name: Upload Stryker report
  uses: actions/upload-artifact@v4
  with:
    name: stryker-report
    path: reports/mutation/
```

Run full mutation suites on a nightly schedule, not on every push — see Performance Budget below.

---

## Performance Budget

Mutation testing is inherently slow: N mutants × test suite duration. Typical ratios:

| Codebase size | Mutants | Full run time |
|---------------|---------|---------------|
| Small (< 5k LOC) | ~500 | 2–10 min |
| Medium (5–50k LOC) | ~5 000 | 30–90 min |
| Large (> 50k LOC) | ~50 000 | 4–12 hours |

**Rules:**

1. **PRs**: run incremental mutation only on lines changed in the diff (`--incremental` / `coverageAnalysis: 'perTest'` / PIT `withHistory`). Target: < 5 min gate time.
2. **Main / nightly**: run full mutation suite. Store the HTML report as a CI artifact.
3. **Never run full mutation on every push** to a shared branch — it blocks developers without proportional value.
4. Parallelize using test sharding or distributed runners (cosmic-ray + Celery, Stryker concurrency settings) when full runs exceed acceptable nightly windows.

---

## Interpreting Results

### Low mutation score (< 60 %)

Your tests pass for the wrong reasons. Common causes:

- Tests exercise code paths but assert only on side effects, not return values.
- Tests are written to pass the current implementation, not to specify behavior.
- Large blocks of code have no coverage at all (check no-coverage mutants first).

**Fix**: write behavior-specifying tests — given an input, assert the exact output. Do not assert on implementation details (internal calls, intermediate state).

### High survived count on a specific operator

| Surviving operator | Likely root cause |
|-------------------|-------------------|
| Conditional boundary (`>` vs `>=`) | Off-by-one tests missing |
| Boolean literal | Defensive defaults not tested |
| Statement deletion | Code path never called in tests |
| Return value | Return value not asserted |

### Equivalent mutants

Some surviving mutants are semantically equivalent to the original and cannot be killed by any test. Do not chase 100 % mutation score — flag these in your tool's ignore config and focus on meaningful gaps.

---

## Incremental Workflow

For teams adopting mutation testing on an existing codebase:

1. Run mutation on the module being actively refactored only. Do not gate the entire codebase.
2. Fix the most impactful survivors (high-traffic, high-risk code) first.
3. Raise thresholds incrementally: start at break = 40 %, raise 5 % per sprint until stable at 70–80 %.
4. Add full-codebase runs to the nightly pipeline before enforcing repo-wide thresholds.

---

## Mutation Score as the AI-Generated-Test Validator

By 2026 this is the converged-on use for mutation testing in AI-assisted codebases, and it
directly serves behavior-preserving refactors: when AI generates the characterization safety
net for a refactor, mutation testing verifies the net itself.

- **The failure mode it catches:** AI/agent-authored tests routinely reach high line coverage
  while passing trivially — hardcoded expected values, assertions on incidental output, oracles
  that describe *actual* behavior rather than *intended* behavior. Such tests pass before and
  after a behavior change, so they provide zero refactor-safety signal.
- **The gate:** line coverage measures execution; mutation score measures detection. For a
  refactor safety net, require the AI-generated characterization tests to clear a mutation-score
  threshold on the diff boundary before trusting them — not a coverage threshold.
- **Closed loop:** (1) AI drafts characterization tests targeted at the change boundary →
  (2) run mutation on the touched module → (3) any surviving mutant in refactor-critical code
  means the safety net has a hole; fix the test, not the threshold → (4) only then perform the
  refactor behind the verified net.
- **Do not** let an agent raise the score by weakening assertions to kill survivors — that is
  the test-healing anti-pattern inverted. Survivors are fixed by strengthening oracles.

---

## Common Pitfalls

| Pitfall | Effect | Remedy |
|---------|--------|--------|
| Running full mutation on every PR | Pipelines time out; developers bypass gate | Run incremental on PRs; full run nightly |
| Setting break threshold at 100 % | Equivalent mutants cause permanent failures | Cap break at 85 %; triage survivors before raising further |
| Ignoring no-coverage mutants | Score looks high but large gaps exist | Fix coverage first, then interpret mutation score |
| Mutation testing without unit tests | Nothing to kill mutants; score is 0 % | Write a characterization test baseline before enabling |

# Operational Patterns and Standards

## Contents

- [Pattern: Reuse-Before-Write Ladder](#pattern-reuse-before-write-ladder)
- [Pattern: Classic Refactoring Catalog](#pattern-classic-refactoring-catalog)
- [Pattern: Code Smells Detection](#pattern-code-smells-detection)
- [Pattern: Technical Debt Management](#pattern-technical-debt-management)
- [Pattern: Automated Quality Gates](#pattern-automated-quality-gates)
- [Pattern: Legacy Code Modernization](#pattern-legacy-code-modernization)
- [Enterprise-Grade AI Refactoring Platforms](#enterprise-grade-ai-refactoring-platforms)
- [Popular Developer Tools](#popular-developer-tools)
- [AI Capabilities in Modern Refactoring](#ai-capabilities-in-modern-refactoring)
- [Quality & Testing](#quality--testing)
- [Code Review & Security](#code-review--security)
- [Architecture & Design](#architecture--design)
- [Frontend & Backend Development](#frontend--backend-development)
- [DevOps & Data](#devops--data)
- [Common Workflows](#common-workflows)

## Pattern: Reuse-Before-Write Ladder

**Use when:** Before writing any new code — refactor target, helper function, or new abstraction — run this ordered checklist first. Stop at the first rung that holds; only fall through to the next rung if it doesn't.

1. **Does this need to exist at all?** If the need is speculative (no concrete caller today), skip it — do not build for a hypothetical future requirement.
2. **Does the codebase already have this?** Search for an existing implementation (function, class, module) before writing a new one. A duplicate you didn't find is worse than a slower search.
3. **Does the standard library cover it?** Prefer the language/runtime standard library over a hand-rolled equivalent.
4. **Does a native platform feature cover it?** Framework or platform primitives (built-in caching, built-in validation, built-in retry) usually outrank custom code in reliability and maintenance cost.
5. **Does an already-installed dependency cover it?** Check `package.json`/`requirements.txt`/equivalent for a library already in the tree before adding new surface area or a new dependency.
6. **Can it be one line?** If the minimal correct implementation is a one-liner, write the one-liner, not a wrapper class around it.
7. **Only then, write the minimum.** Write the smallest change that satisfies the requirement — no unrequested abstraction, no speculative extensibility.

This ladder operationalizes this skill's existing simplicity bias (see [Do / Avoid](../SKILL.md#do--avoid) in the main skill file and the general "boring over clever" framing throughout this skill) as a literal, ordered sequence of lookups rather than a general preference — closer to a runbook an agent can execute step-by-step than a principle to keep in mind. It is most useful during the "smallest safe step" phase of the Safe Refactor Loop, before introducing a new helper, utility, or abstraction as part of a refactor.

Attribution: ladder structure adapted from the core ruleset in [DietrichGebert/ponytail](https://github.com/DietrichGebert/ponytail), commit `2ed6c52c`, MIT license (2026-08-09).

---

## Pattern: Classic Refactoring Catalog

**Use when:** Improving code structure without changing behavior.

**Extract Method:**
```javascript
// Before: Long method with mixed concerns
function processOrder(order) {
  // Validate
  if (!order.items || order.items.length === 0) {
    throw new Error('Empty order');
  }

  // Calculate total
  let total = 0;
  for (const item of order.items) {
    total += item.price * item.quantity;
  }

  // Apply discount
  if (order.coupon) {
    const discount = total * order.coupon.percentage;
    total -= discount;
  }

  // Save
  db.orders.insert({ ...order, total });
}

// After: Extracted methods
function processOrder(order) {
  validateOrder(order);
  const total = calculateTotal(order);
  saveOrder(order, total);
}

function validateOrder(order) {
  if (!order.items || order.items.length === 0) {
    throw new Error('Empty order');
  }
}

function calculateTotal(order) {
  const subtotal = order.items.reduce(
    (sum, item) => sum + item.price * item.quantity,
    0
  );
  return applyDiscount(subtotal, order.coupon);
}

function applyDiscount(amount, coupon) {
  if (!coupon) return amount;
  return amount * (1 - coupon.percentage);
}

function saveOrder(order, total) {
  db.orders.insert({ ...order, total });
}
```

**Replace Conditional with Polymorphism:**
```typescript
// Before: Type-checking with conditionals
class Bird {
  type: string;

  getSpeed(): number {
    switch (this.type) {
      case 'european':
        return this.getBaseSpeed();
      case 'african':
        return this.getBaseSpeed() - this.getLoadFactor();
      case 'norwegian-blue':
        return this.isNailed ? 0 : this.getBaseSpeed();
      default:
        throw new Error('Unknown bird type');
    }
  }
}

// After: Polymorphic classes
abstract class Bird {
  abstract getSpeed(): number;
  protected abstract getBaseSpeed(): number;
}

class EuropeanBird extends Bird {
  getSpeed(): number {
    return this.getBaseSpeed();
  }

  protected getBaseSpeed(): number {
    return 35;
  }
}

class AfricanBird extends Bird {
  constructor(private numberOfCoconuts: number) {
    super();
  }

  getSpeed(): number {
    return this.getBaseSpeed() - this.getLoadFactor();
  }

  private getLoadFactor(): number {
    return this.numberOfCoconuts * 2;
  }

  protected getBaseSpeed(): number {
    return 40;
  }
}

class NorwegianBlueBird extends Bird {
  constructor(private isNailed: boolean) {
    super();
  }

  getSpeed(): number {
    return this.isNailed ? 0 : this.getBaseSpeed();
  }

  protected getBaseSpeed(): number {
    return 32;
  }
}
```

**Introduce Parameter Object:**
```typescript
// Before: Long parameter list
function createUser(
  firstName: string,
  lastName: string,
  email: string,
  phone: string,
  address: string,
  city: string,
  state: string,
  zip: string
) {
  // ...
}

// After: Parameter object
interface UserData {
  firstName: string;
  lastName: string;
  email: string;
  phone: string;
  address: Address;
}

interface Address {
  street: string;
  city: string;
  state: string;
  zip: string;
}

function createUser(userData: UserData) {
  // ...
}
```

---

## Pattern: Code Smells Detection

**Use when:** Identifying areas needing refactoring.

**Common Code Smells:**

**1. Duplicated Code:**
```javascript
// Smell: Same logic in multiple places
function calculateEmployeeBonus(employee) {
  if (employee.department === 'sales') {
    return employee.salary * 0.15;
  }
  return employee.salary * 0.10;
}

function calculateManagerBonus(manager) {
  if (manager.department === 'sales') {
    return manager.salary * 0.15 + 5000;
  }
  return manager.salary * 0.10 + 5000;
}

// Fix: Extract common logic
const BONUS_RATES = {
  sales: 0.15,
  default: 0.10,
};

function getBonusRate(department) {
  return BONUS_RATES[department] || BONUS_RATES.default;
}

function calculateEmployeeBonus(employee) {
  return employee.salary * getBonusRate(employee.department);
}

function calculateManagerBonus(manager) {
  return calculateEmployeeBonus(manager) + 5000;
}
```

**2. Long Method (>20 lines):**
- Extract smaller methods
- Apply Single Responsibility Principle
- Use Extract Method refactoring

**3. Large Class (>300 lines or >10 methods):**
```typescript
// Smell: God class doing too much
class UserManager {
  createUser() {}
  deleteUser() {}
  authenticateUser() {}
  sendEmail() {}
  generateReport() {}
  processPayment() {}
}

// Fix: Split into focused classes
class UserService {
  createUser() {}
  deleteUser() {}
}

class AuthenticationService {
  authenticateUser() {}
}

class EmailService {
  sendEmail() {}
}

class ReportingService {
  generateReport() {}
}

class PaymentService {
  processPayment() {}
}
```

**4. Long Parameter List (>3 parameters):**
- Use parameter objects
- Builder pattern for complex objects

**5. Feature Envy:**
```javascript
// Smell: Method uses more features of another class
class Order {
  getTotal() {
    let total = 0;
    for (const item of this.items) {
      total += item.product.price * item.quantity;
      total -= item.product.discount;
    }
    return total;
  }
}

// Fix: Move logic closer to data
class OrderItem {
  getPrice() {
    return this.product.getDiscountedPrice() * this.quantity;
  }
}

class Product {
  getDiscountedPrice() {
    return this.price - this.discount;
  }
}

class Order {
  getTotal() {
    return this.items.reduce((sum, item) => sum + item.getPrice(), 0);
  }
}
```

**6. Primitive Obsession:**
```typescript
// Smell: Using primitives instead of small objects
function validateEmail(email: string): boolean {
  return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email);
}

// Fix: Create value object
class Email {
  constructor(private value: string) {
    if (!Email.isValid(value)) {
      throw new Error('Invalid email');
    }
  }

  static isValid(email: string): boolean {
    return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email);
  }

  toString(): string {
    return this.value;
  }
}

// Usage
const userEmail = new Email('user@example.com'); // Validates automatically
```

**Checklist:**
- [ ] No duplicated code blocks (DRY principle)
- [ ] Methods < 20 lines (extract if longer)
- [ ] Classes < 300 lines (split if larger)
- [ ] Functions have < 4 parameters (use objects for more)
- [ ] No "God objects" doing too much
- [ ] Value objects for primitives with validation rules
- [ ] Logic lives close to the data it operates on

---

## Pattern: Technical Debt Management

**Use when:** Prioritizing and tracking code improvements.

**Technical Debt Quadrant:**

```
     High Impact
         │
 Reckless│Prudent
─────────┼─────────── Deliberate
 Reckless│Prudent
         │
     Low Impact

Inadvertent
```

**Debt Types:**
1. **Reckless Deliberate**: "We don't have time for design" (avoid)
2. **Prudent Deliberate**: "We must ship now, deal with consequences" (acceptable short-term)
3. **Reckless Inadvertent**: "What's layering?" (fix through training)
4. **Prudent Inadvertent**: "Now we know how we should have done it" (normal learning)

**Technical Debt Register:**

```markdown
| ID | Description | Type | Impact | Effort | Priority | Created | Owner |
|----|-------------|------|--------|--------|----------|---------|-------|
| TD-001 | Refactor UserService (600 lines) | Prudent Deliberate | High | 2 days | P1 | 2025-10-01 | Alice |
| TD-002 | Add tests for PaymentProcessor | Reckless Inadvertent | Medium | 3 days | P2 | 2025-09-15 | Bob |
| TD-003 | Extract shared validation logic | Prudent Inadvertent | Low | 1 day | P3 | 2025-11-01 | Charlie |
```

**Quantifying Technical Debt (SonarQube Metrics):**

```
Technical Debt Ratio = (Remediation Cost / Development Cost) * 100

Example:
Remediation Cost: 50 hours (to fix all issues)
Development Cost: 500 hours (total project time)
Debt Ratio: 10%

Thresholds:
< 5%: Excellent
5-10%: Good
10-20%: Needs attention
> 20%: Critical
```

**Boy Scout Rule:**
```
Leave the code better than you found it.

When touching a file:
- [ ] Fix at least one code smell
- [ ] Add missing tests
- [ ] Improve naming
- [ ] Extract duplicated code
- [ ] Add documentation
```

**Checklist:**
- [ ] Technical debt tracked in backlog
- [ ] Debt prioritized by impact and effort
- [ ] 20% of sprint capacity for debt reduction
- [ ] Code quality metrics monitored (SonarQube, CodeClimate)
- [ ] Debt discussed in retrospectives
- [ ] Boy Scout Rule enforced in code reviews

---

## Pattern: Automated Quality Gates

**Use when:** Preventing quality regression in CI/CD.

**ESLint Configuration (.eslintrc.js):**

```javascript
module.exports = {
  extends: ['eslint:recommended', 'plugin:@typescript-eslint/recommended'],
  rules: {
    'complexity': ['error', 10],  // Max cyclomatic complexity
    'max-lines': ['error', 300],   // Max lines per file
    'max-lines-per-function': ['error', 50],  // Max lines per function
    'max-params': ['error', 3],    // Max parameters
    'max-depth': ['error', 3],     // Max nesting depth
    'no-duplicate-code': 'error',  // Detect duplicates
    '@typescript-eslint/no-unused-vars': 'error',
    '@typescript-eslint/explicit-function-return-type': 'warn',
  },
};
```

**SonarQube Quality Gate:**

```yaml
# sonar-project.properties
sonar.projectKey=my-project
sonar.organization=my-org

# Quality Gate thresholds
sonar.qualitygate.wait=true
sonar.coverage.threshold=80
sonar.duplications.threshold=3
sonar.complexity.threshold=10
sonar.maintainability.rating=A
sonar.reliability.rating=A
sonar.security.rating=A
```

**Pre-commit Hooks (Husky + lint-staged):**

```json
{
  "husky": {
    "hooks": {
      "pre-commit": "lint-staged"
    }
  },
  "lint-staged": {
    "*.{js,ts}": [
      "eslint --fix",
      "prettier --write"
    ],
    "*.{js,ts,tsx}": [
      "jest --bail --findRelatedTests"
    ]
  }
}
```

**GitHub Actions Quality Check:**

```yaml
name: qa-refactoring

on: [pull_request]

jobs:
  quality:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4

      - name: Run ESLint
        run: npm run lint

      - name: Check code coverage
        run: npm run test:coverage

      - name: SonarQube Scan
        uses: sonarsource/sonarqube-scan-action@<pinned-version>
        env:
          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}

      - name: Quality Gate Check
        run: |
          # Fail if quality gate fails
          if [ $SONAR_QUALITY_GATE == "ERROR" ]; then
            exit 1
          fi
```

**Checklist:**
- [ ] Linter configured (ESLint, Pylint, RuboCop)
- [ ] Formatter enforced (Prettier, Black, gofmt)
- [ ] Complexity limits set (cyclomatic complexity < 10)
- [ ] File size limits enforced (< 300 lines)
- [ ] Function length limits (< 50 lines)
- [ ] Test coverage threshold (> 80%)
- [ ] Pre-commit hooks run linter + formatter
- [ ] CI pipeline fails on quality gate violations

---

## Pattern: Legacy Code Modernization

**Use when:** Refactoring old codebases without tests.

**Strangler Fig Pattern:**

```
1. Identify seam (boundary between old and new)
2. Build new implementation alongside old
3. Redirect traffic to new implementation
4. Remove old implementation when confident

Example:
Old: Monolithic UserService
New: Microservice UserAPI

Phase 1: Proxy pattern (route 10% to new)
Phase 2: Increase to 50%
Phase 3: Full migration (100% new)
Phase 4: Remove old code
```

**Characterization Tests (Before Refactoring):**

```javascript
// 1. Write tests that describe current behavior (even if buggy)
describe('LegacyUserService', () => {
  it('returns user with uppercased name (weird, but current behavior)', () => {
    const user = legacyService.getUser(123);
    expect(user.name).toBe('JOHN DOE'); // Captures current behavior
  });
});

// 2. Refactor with confidence
function getUser(id) {
  const user = db.users.findById(id);
  return { ...user, name: user.name }; // Fix: removed toUpperCase()
}

// 3. Update tests to reflect correct behavior
it('returns user with original name casing', () => {
  const user = userService.getUser(123);
  expect(user.name).toBe('John Doe');
});
```

**Incremental Refactoring Steps:**

```
Week 1: Add characterization tests (no code changes)
Week 2: Extract methods, improve naming
Week 3: Split large classes
Week 4: Add proper error handling
Week 5: Modernize dependencies
Week 6: Remove dead code
Week 7: Performance optimization
Week 8: Final cleanup + documentation
```

**Checklist:**
- [ ] Characterization tests written before refactoring
- [ ] Refactor in small, safe steps (1 pattern at a time)
- [ ] Run full test suite after each change
- [ ] Use Strangler Fig for large rewrites
- [ ] Avoid "big bang" refactors (incremental > rewrite)
- [ ] Monitor performance metrics during migration
- [ ] Keep old code running until new is proven

---

# Modern AI-Assisted Refactoring

## Enterprise-Grade AI Refactoring Platforms

Use AI tools as draft generators and triage helpers, not as proof of correctness.

Recommended categories:

- Agent/editor copilots for proposing narrow refactors and summarizing diff risk.
- IDE-native refactor tools for compiler-aware renames, signature changes, and moves.
- Codemod platforms for mechanical multi-file rewrites that can be tested and reviewed.

## Popular Developer Tools

**GitHub Copilot**:
- Real-time refactoring suggestions as you code
- Suggests Extract Method, Rename Variable, Simplify Conditional
- Works best when repo instructions, tests, and review gates are already in place

**ReSharper (JetBrains)**:
- Compiler-aware .NET refactoring automation
- Real-time code quality checks
- Automated method extraction and class decomposition

**IntelliJ IDEA AI Assistant**:
- Intelligent code insight for Java, Kotlin, and more
- Automates method extraction, variable renaming, class decomposition
- Flags architectural smells early
- Context-aware refactoring recommendations

## AI Capabilities in Modern Refactoring

**Semantic Understanding**:
- Parse code structure and understand variable scope
- Maintain consistency across function calls and imports
- Enhanced semantic analysis with larger context windows
- Understand relationships between distant code sections

**Strategic Approach**:
- Incremental implementation preferred over big-bang refactors
- Human oversight remains crucial for quality assurance
- Systematic measurement of code quality improvements
- Continuous refactoring as part of regular development

**Best Practices**:
- Treat refactoring as ongoing practice, not one-time project
- Continuously identify opportunities during feature development
- Apply AI-assisted techniques incrementally
- Measure impact: complexity reduction, maintainability improvement
- Require deterministic tests, representative diff review, and explicit rollout controls before merge

---

# Related Skills

This skill works together with other quality and development skills. Use cross-skill combinations for comprehensive code improvement workflows.

## Quality & Testing

- [qa-testing-strategy](../../qa-testing-strategy/SKILL.md) - Test strategies to support safe refactoring; essential before attempting Level 3-4 refactorings
- [qa-debugging](../../qa-debugging/SKILL.md) - Debugging techniques for complex refactoring; use when refactoring introduces unexpected behavior
- [qa-observability](../../qa-observability/SKILL.md) - Monitoring code quality improvements and performance impact of refactoring
- [qa-resilience](../../qa-resilience/SKILL.md) - Building resilient systems during refactoring; error handling and fault tolerance patterns

## Code Review & Security

- [software-code-review](../../software-code-review/SKILL.md) - Review practices for refactored code; use for post-refactoring quality checks
- [software-security-appsec](../../software-security-appsec/SKILL.md) - Security considerations during refactoring; prevent introducing vulnerabilities

## Architecture & Design

- [software-architecture-design](../../software-architecture-design/SKILL.md) - Architectural patterns for large-scale refactoring; use for Extract Class, Move Method patterns
- [software-ui-ux-design](../../software-ui-ux-design/SKILL.md) - UI/UX considerations during frontend refactoring; maintain user experience during UI code improvements

## Frontend & Backend Development

- [software-frontend](../../software-frontend/SKILL.md) - Frontend-specific refactoring patterns; React hooks, component extraction, state management
- [software-backend](../../software-backend/SKILL.md) - Backend refactoring patterns; service decomposition, API versioning, database schema changes

## DevOps & Data

- [ops-devops-platform](../../ops-devops-platform/SKILL.md) - CI/CD integration for quality gates; automate refactoring checks in pipelines
- [data-sql-optimization](../../data-sql-optimization/SKILL.md) - Database refactoring patterns; schema evolution, query optimization, migration strategies

## Common Workflows

**Refactoring Legacy Code (High Risk)**:
1. Use [qa-refactoring](../SKILL.md) for characterization tests strategy
2. Use [qa-testing-strategy](../../qa-testing-strategy/SKILL.md) to build test coverage
3. Use [qa-debugging](../../qa-debugging/SKILL.md) if issues arise
4. Use [software-code-review](../../software-code-review/SKILL.md) for post-refactoring validation

**Establishing Quality Standards (New Project)**:
1. Use [qa-refactoring](../SKILL.md) for linting and quality gate setup
2. Use [ops-devops-platform](../../ops-devops-platform/SKILL.md) for CI/CD integration
3. Use [software-code-review](../../software-code-review/SKILL.md) for review checklists
4. Use [qa-testing-strategy](../../qa-testing-strategy/SKILL.md) for test pyramid

**Performance Optimization Refactoring**:
1. Use [qa-observability](../../qa-observability/SKILL.md) to identify bottlenecks
2. Use [qa-refactoring](../SKILL.md) to apply refactoring patterns
3. Use [qa-testing-strategy](../../qa-testing-strategy/SKILL.md) to add performance tests
4. Use [data-sql-optimization](../../data-sql-optimization/SKILL.md) if database queries are involved

---

# External Resources

See [data/sources.json](../data/sources.json) for:
- Refactoring books (Martin Fowler, Michael Feathers)
- Code quality tools (SonarQube, CodeClimate, ESLint)
- Modern AI-assisted tools (GitHub Copilot, ReSharper, IntelliJ IDEA)
- Refactoring patterns and techniques
- Legacy code rescue strategies
- current tool-assisted refactoring practices

---

# Quick Decision Matrix

| Scenario | Recommendation |
|----------|----------------|
| Long or mixed-concern function | Extract Method refactoring |
| Large or low-cohesion class/module | Split into smaller focused units |
| Duplicated code | Extract to shared function/class |
| Complex conditionals | Replace Conditional with Polymorphism |
| Long parameter list | Introduce Parameter Object |
| Legacy code without tests | Write Characterization Tests first |
| Large rewrite needed | Strangler Fig Pattern (incremental) |
| Setting quality standards | Automated quality gates in CI/CD |

---

# Anti-Patterns to Avoid

- **Big bang refactors** - High risk, prefer incremental
- **Refactoring without tests** - Breaks things silently
- **Premature optimization** - Refactor for clarity first, performance second
- **Over-engineering** - Keep it simple (YAGNI)
- **Ignoring technical debt** - Compounds over time
- **No quality gates** - Quality degrades without enforcement
- **Rewriting from scratch** - Usually fails, prefer strangler fig

---

> **Success Criteria:** Code is maintainable, readable, testable, and follows established quality standards. Technical debt is tracked, prioritized, and actively reduced. Automated quality gates prevent regression.

# Refactoring Catalog

Comprehensive guide to refactoring patterns based on Martin Fowler's catalog (2nd Edition, 2018).

## Contents

- [Composing Methods](#composing-methods)
- [Moving Features Between Objects](#moving-features-between-objects)
- [Organizing Data](#organizing-data)
- [Simplifying Conditional Expressions](#simplifying-conditional-expressions)
- [Making Method Calls Simpler](#making-method-calls-simpler)
- [Dealing with Generalization](#dealing-with-generalization)
- [Big Refactorings](#big-refactorings)
- [Modern Refactorings](#modern-refactorings)
- [References](#references)

---

## Composing Methods

Refactorings that help make methods more readable and maintainable.

### Extract Method

**Problem:** Code fragment that can be grouped together.

**Solution:** Move fragment to separate method with descriptive name.

```javascript
// Before
function printOwing(invoice) {
  printBanner();
  let outstanding = 0;

  for (const order of invoice.orders) {
    outstanding += order.amount;
  }

  console.log(`name: ${invoice.customer}`);
  console.log(`amount: ${outstanding}`);
}

// After
function printOwing(invoice) {
  printBanner();
  const outstanding = calculateOutstanding(invoice);
  printDetails(invoice, outstanding);
}

function calculateOutstanding(invoice) {
  return invoice.orders.reduce((sum, order) => sum + order.amount, 0);
}

function printDetails(invoice, outstanding) {
  console.log(`name: ${invoice.customer}`);
  console.log(`amount: ${outstanding}`);
}
```

**When to use:**
- Method is too long (>20 lines)
- Code needs explanation (comment before it)
- Logic can be reused elsewhere

---

### Inline Method

**Problem:** Method body is as clear as its name.

**Solution:** Replace method calls with method body content.

```javascript
// Before
function getRating(driver) {
  return moreThanFiveLateDeliveries(driver) ? 2 : 1;
}

function moreThanFiveLateDeliveries(driver) {
  return driver.lateDeliveries > 5;
}

// After
function getRating(driver) {
  return driver.lateDeliveries > 5 ? 2 : 1;
}
```

**When to use:**
- Method body is self-explanatory
- Over-abstraction adds complexity
- Method is only used once

---

### Extract Variable

**Problem:** Complex expression is hard to understand.

**Solution:** Place result in self-explanatory variable.

```javascript
// Before
if (platform.toUpperCase().includes('MAC') &&
    browser.toUpperCase().includes('IE') &&
    wasInitialized() && resize > 0) {
  // do something
}

// After
const isMacOS = platform.toUpperCase().includes('MAC');
const isIEBrowser = browser.toUpperCase().includes('IE');
const wasResized = wasInitialized() && resize > 0;

if (isMacOS && isIEBrowser && wasResized) {
  // do something
}
```

**When to use:**
- Expression is complex
- Expression is used multiple times
- Variable name adds clarity

---

### Inline Variable

**Problem:** Variable name doesn't add clarity beyond expression itself.

**Solution:** Replace variable references with expression.

```javascript
// Before
const basePrice = order.basePrice;
return basePrice > 1000;

// After
return order.basePrice > 1000;
```

---

### Replace Temp with Query

**Problem:** Temporary variable holds result of expression.

**Solution:** Extract expression into method, use method instead of variable.

```typescript
// Before
class Order {
  quantity: number;
  itemPrice: number;

  getPrice(): number {
    const basePrice = this.quantity * this.itemPrice;
    const discountFactor = 0.98;
    return basePrice * discountFactor;
  }
}

// After
class Order {
  quantity: number;
  itemPrice: number;

  getPrice(): number {
    return this.basePrice() * this.discountFactor();
  }

  private basePrice(): number {
    return this.quantity * this.itemPrice;
  }

  private discountFactor(): number {
    return 0.98;
  }
}
```

---

### Split Temporary Variable

**Problem:** Local variable assigned multiple times (not loop variable or accumulator).

**Solution:** Create separate variable for each assignment.

```javascript
// Before
let temp = 2 * (height + width);
console.log(temp);
temp = height * width;
console.log(temp);

// After
const perimeter = 2 * (height + width);
console.log(perimeter);
const area = height * width;
console.log(area);
```

---

### Remove Assignments to Parameters

**Problem:** Code assigns value to parameter.

**Solution:** Use local variable instead.

```javascript
// Before
function discount(inputValue, quantity) {
  if (inputValue > 50) inputValue -= 2;
  if (quantity > 100) inputValue -= 1;
  return inputValue;
}

// After
function discount(inputValue, quantity) {
  let result = inputValue;
  if (inputValue > 50) result -= 2;
  if (quantity > 100) result -= 1;
  return result;
}
```

---

## Moving Features Between Objects

Refactorings that help move functionality between classes.

### Move Method

**Problem:** Method is used more by another class than by its own.

**Solution:** Move method to the class that uses it most.

```typescript
// Before
class Account {
  overdraftCharge(): number {
    return this.type.isPremium() ? 10 : 20;
  }
}

class AccountType {
  isPremium(): boolean {
    return this.name === 'Premium';
  }
}

// After
class Account {
  overdraftCharge(): number {
    return this.type.overdraftCharge();
  }
}

class AccountType {
  isPremium(): boolean {
    return this.name === 'Premium';
  }

  overdraftCharge(): number {
    return this.isPremium() ? 10 : 20;
  }
}
```

---

### Move Field

**Problem:** Field is used more by another class than by its own.

**Solution:** Move field to class that uses it most.

```typescript
// Before
class Customer {
  plan: Plan;
  discountRate: number;
}

class Plan {
  name: string;
}

// After
class Customer {
  plan: Plan;

  get discountRate(): number {
    return this.plan.discountRate;
  }
}

class Plan {
  name: string;
  discountRate: number;
}
```

---

### Extract Class

**Problem:** Class does work of two or more classes.

**Solution:** Create new class, move relevant fields and methods.

```typescript
// Before
class Person {
  name: string;
  officeAreaCode: string;
  officeNumber: string;

  getTelephoneNumber(): string {
    return `(${this.officeAreaCode}) ${this.officeNumber}`;
  }
}

// After
class Person {
  name: string;
  officeTelephone: TelephoneNumber;

  getTelephoneNumber(): string {
    return this.officeTelephone.toString();
  }
}

class TelephoneNumber {
  areaCode: string;
  number: string;

  toString(): string {
    return `(${this.areaCode}) ${this.number}`;
  }
}
```

---

### Inline Class

**Problem:** Class does too little to justify its existence.

**Solution:** Move all features to another class and delete it.

```typescript
// Before
class Person {
  name: string;
  telephone: TelephoneNumber;
}

class TelephoneNumber {
  areaCode: string;
  number: string;

  toString(): string {
    return `(${this.areaCode}) ${this.number}`;
  }
}

// After
class Person {
  name: string;
  areaCode: string;
  number: string;

  getTelephoneNumber(): string {
    return `(${this.areaCode}) ${this.number}`;
  }
}
```

---

## Organizing Data

Refactorings that help organize data structures.

### Encapsulate Field

**Problem:** Public field accessed directly.

**Solution:** Make field private, provide accessors.

```typescript
// Before
class Person {
  name: string;
}

const person = new Person();
person.name = 'John';

// After
class Person {
  private _name: string;

  get name(): string {
    return this._name;
  }

  set name(value: string) {
    this._name = value;
  }
}

const person = new Person();
person.name = 'John';
```

---

### Replace Data Value with Object

**Problem:** Data item needs additional data or behavior.

**Solution:** Turn data item into object.

```typescript
// Before
class Order {
  customer: string;
}

// After
class Order {
  customer: Customer;
}

class Customer {
  constructor(private name: string) {}

  getName(): string {
    return this.name;
  }
}
```

---

### Change Value to Reference

**Problem:** Many equal instances of a class should be replaced with single object.

**Solution:** Turn object into reference object.

```typescript
// Before
class Customer {
  constructor(private name: string) {}
}

// Multiple instances created
const customer1 = new Customer('John');
const customer2 = new Customer('John');

// After
class Customer {
  private static instances = new Map<string, Customer>();

  private constructor(private name: string) {}

  static get(name: string): Customer {
    if (!Customer.instances.has(name)) {
      Customer.instances.set(name, new Customer(name));
    }
    return Customer.instances.get(name)!;
  }
}

// Single instance reused
const customer1 = Customer.get('John');
const customer2 = Customer.get('John'); // Same instance
```

---

### Replace Array with Object

**Problem:** Array with elements representing different things.

**Solution:** Replace with object with meaningful field names.

```javascript
// Before
const row = [];
row[0] = 'Liverpool';
row[1] = 15;

// After
const performance = {
  name: 'Liverpool',
  wins: 15
};
```

---

## Simplifying Conditional Expressions

Refactorings that simplify complex conditionals.

### Decompose Conditional

**Problem:** Complex conditional (if-then-else).

**Solution:** Extract methods from condition, then, and else parts.

```javascript
// Before
if (date.before(SUMMER_START) || date.after(SUMMER_END)) {
  charge = quantity * winterRate + winterServiceCharge;
} else {
  charge = quantity * summerRate;
}

// After
if (isSummer(date)) {
  charge = summerCharge(quantity);
} else {
  charge = winterCharge(quantity);
}
```

---

### Consolidate Conditional Expression

**Problem:** Multiple conditionals with same result.

**Solution:** Combine into single conditional expression.

```javascript
// Before
function disabilityAmount(employee) {
  if (employee.seniority < 2) return 0;
  if (employee.monthsDisabled > 12) return 0;
  if (employee.isPartTime) return 0;
  // compute disability amount
}

// After
function disabilityAmount(employee) {
  if (isNotEligibleForDisability(employee)) return 0;
  // compute disability amount
}

function isNotEligibleForDisability(employee) {
  return employee.seniority < 2
    || employee.monthsDisabled > 12
    || employee.isPartTime;
}
```

---

### Replace Nested Conditional with Guard Clauses

**Problem:** Method has conditional behavior that doesn't make normal path clear.

**Solution:** Use guard clauses for special cases.

```javascript
// Before
function getPayAmount() {
  let result;
  if (isDead) {
    result = deadAmount();
  } else {
    if (isSeparated) {
      result = separatedAmount();
    } else {
      if (isRetired) {
        result = retiredAmount();
      } else {
        result = normalPayAmount();
      }
    }
  }
  return result;
}

// After
function getPayAmount() {
  if (isDead) return deadAmount();
  if (isSeparated) return separatedAmount();
  if (isRetired) return retiredAmount();
  return normalPayAmount();
}
```

---

### Replace Conditional with Polymorphism

**Problem:** Conditional based on object type.

**Solution:** Create subclasses matching conditional branches.

```typescript
// Before
class Bird {
  getSpeed(): number {
    switch (this.type) {
      case 'european':
        return this.getBaseSpeed();
      case 'african':
        return this.getBaseSpeed() - this.getLoadFactor();
      case 'norwegian-blue':
        return this.isNailed ? 0 : this.getBaseSpeed();
      default:
        throw new Error('Unknown bird');
    }
  }
}

// After
abstract class Bird {
  abstract getSpeed(): number;
}

class EuropeanBird extends Bird {
  getSpeed(): number {
    return this.getBaseSpeed();
  }
}

class AfricanBird extends Bird {
  getSpeed(): number {
    return this.getBaseSpeed() - this.getLoadFactor();
  }
}

class NorwegianBlueBird extends Bird {
  constructor(private isNailed: boolean) {
    super();
  }

  getSpeed(): number {
    return this.isNailed ? 0 : this.getBaseSpeed();
  }
}
```

---

### Introduce Null Object

**Problem:** Repeated checks for null values.

**Solution:** Replace null value with null object.

```typescript
// Before
class Customer {
  getName(): string {
    return this.name;
  }
}

const customer = getCustomer();
const name = customer === null ? 'occupant' : customer.getName();

// After
class Customer {
  getName(): string {
    return this.name;
  }

  static createNullCustomer(): Customer {
    return new NullCustomer();
  }
}

class NullCustomer extends Customer {
  getName(): string {
    return 'occupant';
  }
}

const customer = getCustomer() || Customer.createNullCustomer();
const name = customer.getName();
```

---

## Making Method Calls Simpler

Refactorings that simplify method interfaces.

### Rename Method

**Problem:** Method name doesn't reveal its purpose.

**Solution:** Rename method.

```javascript
// Before
function getsnm() {
  return this.name;
}

// After
function getSecondName() {
  return this.name;
}
```

---

### Add Parameter

**Problem:** Method needs more information from caller.

**Solution:** Add parameter.

```javascript
// Before
function getContact() {
  return this.name;
}

// After
function getContact(includeTitle) {
  return includeTitle ? `${this.title} ${this.name}` : this.name;
}
```

---

### Remove Parameter

**Problem:** Parameter no longer used by method body.

**Solution:** Remove it.

```javascript
// Before
function getContact(includeTitle) {
  return this.name; // includeTitle never used
}

// After
function getContact() {
  return this.name;
}
```

---

### Separate Query from Modifier

**Problem:** Method returns value and changes object state.

**Solution:** Split into two methods.

```javascript
// Before
function getTotalOutstandingAndSetReadyForSummaries() {
  const total = this.orders.reduce((sum, order) => sum + order.total, 0);
  this.readyForSummaries = true;
  return total;
}

// After
function getTotalOutstanding() {
  return this.orders.reduce((sum, order) => sum + order.total, 0);
}

function setReadyForSummaries() {
  this.readyForSummaries = true;
}
```

---

### Parameterize Method

**Problem:** Multiple methods do similar things with different values.

**Solution:** Create one method using parameter for different values.

```javascript
// Before
function fivePercentRaise() {
  this.salary *= 1.05;
}

function tenPercentRaise() {
  this.salary *= 1.10;
}

// After
function raise(percentage) {
  this.salary *= (1 + percentage / 100);
}
```

---

### Replace Parameter with Explicit Methods

**Problem:** Method runs different code based on parameter values.

**Solution:** Create separate method for each parameter value.

```javascript
// Before
function setValue(name, value) {
  if (name === 'height') this.height = value;
  if (name === 'width') this.width = value;
}

// After
function setHeight(value) {
  this.height = value;
}

function setWidth(value) {
  this.width = value;
}
```

---

### Preserve Whole Object

**Problem:** Getting several values from object and passing as parameters.

**Solution:** Pass whole object instead.

```javascript
// Before
const low = daysTempRange.getLow();
const high = daysTempRange.getHigh();
const withinPlan = plan.withinRange(low, high);

// After
const withinPlan = plan.withinRange(daysTempRange);
```

---

### Replace Parameter with Method Call

**Problem:** Calling method, passing result as parameter to another method.

**Solution:** Make second method call first method directly.

```javascript
// Before
const basePrice = quantity * itemPrice;
const discountLevel = getDiscountLevel();
const finalPrice = discountedPrice(basePrice, discountLevel);

// After
const basePrice = quantity * itemPrice;
const finalPrice = discountedPrice(basePrice);

function discountedPrice(basePrice) {
  const discountLevel = getDiscountLevel();
  // use discountLevel
}
```

---

### Introduce Parameter Object

**Problem:** Methods have long parameter list with natural grouping.

**Solution:** Replace parameters with object.

```typescript
// Before
function amountInvoiced(startDate: Date, endDate: Date) {}
function amountReceived(startDate: Date, endDate: Date) {}
function amountOverdue(startDate: Date, endDate: Date) {}

// After
class DateRange {
  constructor(
    public startDate: Date,
    public endDate: Date
  ) {}
}

function amountInvoiced(dateRange: DateRange) {}
function amountReceived(dateRange: DateRange) {}
function amountOverdue(dateRange: DateRange) {}
```

---

### Remove Setting Method

**Problem:** Field should be set at creation time and never altered.

**Solution:** Remove methods that set field.

```typescript
// Before
class Account {
  private id: string;

  setId(id: string) {
    this.id = id;
  }
}

// After
class Account {
  constructor(private readonly id: string) {}
}
```

---

## Dealing with Generalization

Refactorings that deal with inheritance hierarchies.

### Pull Up Method

**Problem:** Methods with identical results in subclasses.

**Solution:** Move method to superclass.

```typescript
// Before
class Employee {}

class Salesman extends Employee {
  getName(): string {
    return this.name;
  }
}

class Engineer extends Employee {
  getName(): string {
    return this.name;
  }
}

// After
class Employee {
  getName(): string {
    return this.name;
  }
}

class Salesman extends Employee {}
class Engineer extends Employee {}
```

---

### Pull Up Field

**Problem:** Subclasses have same field.

**Solution:** Move field to superclass.

```typescript
// Before
class Employee {}

class Salesman extends Employee {
  name: string;
}

class Engineer extends Employee {
  name: string;
}

// After
class Employee {
  name: string;
}

class Salesman extends Employee {}
class Engineer extends Employee {}
```

---

### Pull Up Constructor Body

**Problem:** Subclasses have constructors with mostly identical bodies.

**Solution:** Create superclass constructor, call from subclass.

```typescript
// Before
class Employee {
  name: string;
  id: string;
}

class Manager extends Employee {
  constructor(name: string, id: string, grade: number) {
    this.name = name;
    this.id = id;
    this.grade = grade;
  }
}

// After
class Employee {
  constructor(name: string, id: string) {
    this.name = name;
    this.id = id;
  }
}

class Manager extends Employee {
  constructor(name: string, id: string, grade: number) {
    super(name, id);
    this.grade = grade;
  }
}
```

---

### Push Down Method

**Problem:** Behavior on superclass relevant only for some subclasses.

**Solution:** Move to those subclasses.

```typescript
// Before
class Employee {
  getQuota(): number {
    return 0; // Only relevant for salesmen
  }
}

class Engineer extends Employee {}
class Salesman extends Employee {}

// After
class Employee {}
class Engineer extends Employee {}

class Salesman extends Employee {
  getQuota(): number {
    return 100;
  }
}
```

---

### Extract Subclass

**Problem:** Class has features used only in some instances.

**Solution:** Create subclass for that subset of features.

```typescript
// Before
class JobItem {
  constructor(
    private unitPrice: number,
    private quantity: number,
    private isLabor: boolean,
    private employee?: Employee
  ) {}

  getTotalPrice(): number {
    return this.unitPrice * this.quantity;
  }

  getUnitPrice(): number {
    return this.isLabor ? this.employee!.getRate() : this.unitPrice;
  }
}

// After
abstract class JobItem {
  constructor(
    protected unitPrice: number,
    protected quantity: number
  ) {}

  getTotalPrice(): number {
    return this.getUnitPrice() * this.quantity;
  }

  abstract getUnitPrice(): number;
}

class PartItem extends JobItem {
  getUnitPrice(): number {
    return this.unitPrice;
  }
}

class LaborItem extends JobItem {
  constructor(
    unitPrice: number,
    quantity: number,
    private employee: Employee
  ) {
    super(unitPrice, quantity);
  }

  getUnitPrice(): number {
    return this.employee.getRate();
  }
}
```

---

### Extract Superclass

**Problem:** Two classes have similar features.

**Solution:** Create superclass, move common features.

```typescript
// Before
class Employee {
  constructor(
    private name: string,
    private id: string
  ) {}

  getName(): string {
    return this.name;
  }
}

class Department {
  constructor(
    private name: string
  ) {}

  getName(): string {
    return this.name;
  }
}

// After
class Party {
  constructor(protected name: string) {}

  getName(): string {
    return this.name;
  }
}

class Employee extends Party {
  constructor(
    name: string,
    private id: string
  ) {
    super(name);
  }
}

class Department extends Party {}
```

---

### Extract Interface

**Problem:** Multiple clients use same subset of class interface.

**Solution:** Move subset to interface.

```typescript
// Before
class Employee {
  getRate(): number {
    return this.rate;
  }

  hasSpecialSkill(): boolean {
    return this.specialSkill !== null;
  }

  getName(): string {
    return this.name;
  }

  getDepartment(): string {
    return this.department;
  }
}

// After
interface Billable {
  getRate(): number;
  hasSpecialSkill(): boolean;
}

class Employee implements Billable {
  getRate(): number {
    return this.rate;
  }

  hasSpecialSkill(): boolean {
    return this.specialSkill !== null;
  }

  getName(): string {
    return this.name;
  }

  getDepartment(): string {
    return this.department;
  }
}
```

---

### Collapse Hierarchy

**Problem:** Superclass and subclass not very different.

**Solution:** Merge them.

```typescript
// Before
class Employee {
  getName(): string {
    return this.name;
  }
}

class Salesman extends Employee {
  getOffice(): string {
    return this.office;
  }
}

// After (if Salesman has minimal difference)
class Employee {
  getName(): string {
    return this.name;
  }

  getOffice(): string {
    return this.office;
  }
}
```

---

## Big Refactorings

Large-scale refactorings for major structural improvements.

### Tease Apart Inheritance

**Problem:** Inheritance hierarchy doing two jobs at once.

**Solution:** Create two hierarchies, use delegation.

---

### Convert Procedural Design to Objects

**Problem:** Code in procedural style.

**Solution:** Turn data records into objects, split behavior into methods.

---

### Separate Domain from Presentation

**Problem:** GUI classes contain domain logic.

**Solution:** Move domain logic to separate domain classes.

---

### Extract Hierarchy

**Problem:** Class doing too much work with many conditional statements.

**Solution:** Create hierarchy of classes, each subclass representing special case.

---

## Modern Refactorings

### Replace Callback with Promise/Async-Await

**Problem:** Callback hell makes code hard to read.

**Solution:** Use modern async patterns.

```javascript
// Before
function getData(callback) {
  fetchData((error, data) => {
    if (error) {
      callback(error, null);
    } else {
      processData(data, (error, result) => {
        if (error) {
          callback(error, null);
        } else {
          callback(null, result);
        }
      });
    }
  });
}

// After
async function getData() {
  const data = await fetchData();
  const result = await processData(data);
  return result;
}
```

---

### Extract Hook (React-specific)

**Problem:** Component has complex logic mixed with UI.

**Solution:** Extract logic into custom hook.

```javascript
// Before
function UserProfile() {
  const [user, setUser] = useState(null);
  const [loading, setLoading] = useState(true);

  useEffect(() => {
    fetchUser().then(data => {
      setUser(data);
      setLoading(false);
    });
  }, []);

  return loading ? <Spinner /> : <Profile user={user} />;
}

// After
function useUser() {
  const [user, setUser] = useState(null);
  const [loading, setLoading] = useState(true);

  useEffect(() => {
    fetchUser().then(data => {
      setUser(data);
      setLoading(false);
    });
  }, []);

  return { user, loading };
}

function UserProfile() {
  const { user, loading } = useUser();
  return loading ? <Spinner /> : <Profile user={user} />;
}
```

---

## References

- **Refactoring: Improving the Design of Existing Code (2nd Edition)** - Martin Fowler
- **Refactoring.guru** - https://refactoring.guru/refactoring
- **Working Effectively with Legacy Code** - Michael Feathers

# Strangler Fig Migration

Incremental migration patterns for replacing legacy systems without big-bang rewrites. Based on Martin Fowler's Strangler Fig Application pattern.

## Contents

- [Strangler Fig Theory](#strangler-fig-theory)
- [Implementation Strategies](#implementation-strategies)
- [Proxy Layer Patterns](#proxy-layer-patterns)
- [Dual-Write Strategies](#dual-write-strategies)
- [Data Migration Approaches](#data-migration-approaches)
- [Feature Toggle Integration](#feature-toggle-integration)
- [Rollback Strategies](#rollback-strategies)
- [Progress Tracking](#progress-tracking)
- [Risk Assessment per Phase](#risk-assessment-per-phase)
- [Timeline Expectations](#timeline-expectations)
- [Common Pitfalls](#common-pitfalls)
- [Related Resources](#related-resources)

---

## Strangler Fig Theory

The strangler fig is a tropical plant that grows around a host tree, eventually replacing it entirely. The metaphor applies directly to software migration: build the new system around the old one, gradually routing traffic to the new system, until the old system can be decommissioned.

### Why Strangler Fig Over Big-Bang Rewrite

| Approach | Risk | Delivery | Rollback | Team Impact |
|----------|------|----------|----------|-------------|
| **Big-bang rewrite** | Very high | Nothing until done | All or nothing | Full team blocked |
| **Strangler fig** | Low per step | Incremental value | Per-feature rollback | Parallel work possible |

### Core Principles

1. **Never stop delivering value** -- the old system stays live while the new one grows
2. **Route, don't rewrite everything** -- migrate one feature or route at a time
3. **Prove equivalence** -- characterization tests ensure the new system matches the old
4. **Rollback at any point** -- each step is independently reversible
5. **Measure progress** -- track what percentage of traffic hits old vs new

### Modern Rollout Controls

For 2026-era migrations, treat the strangler as a staged release problem, not just a routing problem.

- Use migration flags or equivalent staged rollout controls instead of ad hoc booleans.
- Keep legacy and new paths comparable during shadow or dual-run phases.
- Track parity metrics explicitly: consistency rate, error rate, and p95/p99 latency by target.
- Do not advance stages until representative traffic stays green for an agreed soak window.

### The Three Phases

```
Phase 1: WRAP
  Build a proxy layer in front of the legacy system.
  All traffic still goes to legacy, but through your proxy.

Phase 2: STRANGLE
  One by one, implement features in the new system.
  Route traffic to new system feature by feature.
  Old and new run in parallel.

Phase 3: DECOMMISSION
  When all traffic routes to the new system,
  remove the proxy, shut down the legacy system.
```

---

## Implementation Strategies

> **Expand-Contract / Parallel Change**
> When swapping an interface or data structure within the strangler boundary, use the expand-contract pattern: first *expand* the system to support both the old and new shapes simultaneously, migrate all callers to the new shape, then *contract* by removing the old shape. This is also called "parallel change" — the old and new code coexist briefly, with no flag required, and each step is independently deployable. Apply it at the method, class, or schema level whenever a direct cut-over would require coordinating multiple services or teams in a single release.

### Route-Based Strangling

Migrate one API endpoint (or URL path) at a time.

```
Before:
  Client → Legacy System (all routes)

During:
  Client → Proxy → /api/users → New System
                  → /api/orders → Legacy System
                  → /api/payments → Legacy System

After:
  Client → New System (all routes)
```

**Best for:** REST APIs, microservice extraction, web application rewrites.

### Feature-Based Strangling

Migrate one business feature at a time, regardless of how many endpoints it touches.

```
Feature: "Order Management"
  Endpoints: POST /orders, GET /orders/:id, PUT /orders/:id/status
  Database tables: orders, order_items, order_events
  Background jobs: order_confirmation_email, inventory_update

All migrated together as a single unit.
```

**Best for:** Complex features that span multiple endpoints and data stores.

### Data-Based Strangling

Migrate based on data segments (by customer, region, or cohort).

```
Phase 1: New customers → New System, Existing → Legacy
Phase 2: Small accounts → New System, Enterprise → Legacy
Phase 3: All customers → New System
```

**Best for:** Multi-tenant systems, when data isolation is feasible.

### Strategy Selection Guide

| Factor | Route-Based | Feature-Based | Data-Based |
|--------|:-----------:|:-------------:|:----------:|
| API-heavy system | Best | Good | Possible |
| Tightly coupled features | Poor | Best | Possible |
| Multi-tenant SaaS | Possible | Possible | Best |
| Independent endpoints | Best | Good | Possible |
| Shared database | Possible | Possible | Best |
| Speed of migration | Fastest per step | Medium | Slowest to start |

---

## Proxy Layer Patterns

The proxy layer is the critical infrastructure that enables gradual migration.

### NGINX Routing Proxy

```nginx
# nginx.conf: Route-based strangler proxy

upstream legacy {
    server legacy-app:8080;
}

upstream new_system {
    server new-app:8080;
}

server {
    listen 80;

    # Migrated routes → new system
    location /api/v2/users {
        proxy_pass http://new_system;
        proxy_set_header X-Migrated "true";
        proxy_set_header X-Original-Host $host;
    }

    location /api/v2/products {
        proxy_pass http://new_system;
        proxy_set_header X-Migrated "true";
    }

    # Everything else → legacy
    location / {
        proxy_pass http://legacy;
        proxy_set_header X-Migrated "false";
    }
}
```

### Application-Level Proxy (Node.js)

```javascript
const express = require("express");
const { createProxyMiddleware } = require("http-proxy-middleware");
const LaunchDarkly = require("launchdarkly-node-server-sdk");

const app = express();
const ldClient = LaunchDarkly.init(process.env.LD_SDK_KEY);

// Feature flag-driven routing
app.use("/api/orders", async (req, res, next) => {
  const user = { key: req.headers["x-user-id"] || "anonymous" };
  const useNewSystem = await ldClient.variation("orders-new-system", user, false);

  if (useNewSystem) {
    // Route to new system
    return createProxyMiddleware({
      target: "http://new-order-service:8080",
      changeOrigin: true,
      on: {
        proxyReq: (proxyReq) => {
          proxyReq.setHeader("X-Routed-By", "strangler-proxy");
        },
      },
    })(req, res, next);
  }

  // Route to legacy
  return createProxyMiddleware({
    target: "http://legacy-monolith:8080",
    changeOrigin: true,
  })(req, res, next);
});

app.listen(3000);
```

### Proxy Monitoring

```python
"""
Track routing decisions for migration progress monitoring.
Emit metrics for every request showing old vs new routing.
"""
from prometheus_client import Counter

route_counter = Counter(
    "strangler_proxy_requests_total",
    "Requests routed by the strangler proxy",
    ["path", "target", "result"]
)

# In proxy middleware:
def track_routing(path: str, target: str, status_code: int):
    result = "success" if status_code < 500 else "error"
    route_counter.labels(
        path=parameterize_path(path),
        target=target,  # "legacy" or "new"
        result=result
    ).inc()
```

---

## Dual-Write Strategies

During migration, both systems may need to stay in sync. Dual-write patterns handle this.

### Dual-Write Approaches

| Approach | How It Works | Consistency | Complexity |
|----------|-------------|-------------|------------|
| **Synchronous dual-write** | Write to both in same transaction | Strong | High (distributed TX) |
| **Async replication** | Write to primary, replicate to secondary | Eventual | Medium |
| **Change Data Capture (CDC)** | Stream DB changes to secondary | Eventual | Medium |
| **Event sourcing** | Publish events, both systems consume | Eventual | Low |

### CDC with Debezium

```yaml
# docker-compose.yml (Debezium CDC setup)
services:
  debezium:
    image: debezium/connect:2.5
    environment:
      BOOTSTRAP_SERVERS: kafka:9092
      GROUP_ID: strangler-cdc
      CONFIG_STORAGE_TOPIC: debezium-configs
      OFFSET_STORAGE_TOPIC: debezium-offsets

# Register CDC connector
# POST http://debezium:8083/connectors
{
  "name": "legacy-db-connector",
  "config": {
    "connector.class": "io.debezium.connector.postgresql.PostgresConnector",
    "database.hostname": "legacy-db",
    "database.port": "5432",
    "database.user": "cdc_user",
    "database.password": "${CDC_PASSWORD}",
    "database.dbname": "legacy",
    "table.include.list": "public.orders,public.order_items",
    "topic.prefix": "legacy",
    "slot.name": "strangler_cdc"
  }
}
```

### Verification: Comparing Dual-Write Outputs

```python
"""
Compare outputs from legacy and new system to verify equivalence.
Run as a background job during the dual-write phase.
"""
import json
import logging
from datetime import datetime, timedelta

logger = logging.getLogger("dual-write-verifier")

def verify_dual_write_consistency(
    legacy_db,
    new_db,
    table: str,
    time_window_minutes: int = 5,
    sample_size: int = 100,
):
    """Compare records between legacy and new databases."""
    cutoff = datetime.utcnow() - timedelta(minutes=time_window_minutes)

    # Sample recent records from both systems
    legacy_records = legacy_db.query(
        f"SELECT * FROM {table} WHERE updated_at > %s ORDER BY RANDOM() LIMIT %s",
        (cutoff, sample_size)
    )

    mismatches = []
    missing_in_new = []

    for legacy_record in legacy_records:
        new_record = new_db.query(
            f"SELECT * FROM {table} WHERE id = %s",
            (legacy_record["id"],)
        )

        if not new_record:
            missing_in_new.append(legacy_record["id"])
            continue

        # Compare field by field
        for field in legacy_record:
            if field in ("updated_at", "created_at"):
                continue  # Timestamps may differ slightly
            if legacy_record[field] != new_record[0].get(field):
                mismatches.append({
                    "id": legacy_record["id"],
                    "field": field,
                    "legacy": legacy_record[field],
                    "new": new_record[0].get(field),
                })

    # Report results
    total = len(legacy_records)
    match_rate = (total - len(mismatches) - len(missing_in_new)) / max(total, 1) * 100

    logger.info(
        "dual_write_verification",
        table=table,
        total_checked=total,
        mismatches=len(mismatches),
        missing_in_new=len(missing_in_new),
        match_rate=f"{match_rate:.1f}%",
    )

    return {
        "match_rate": match_rate,
        "mismatches": mismatches,
        "missing": missing_in_new,
    }
```

---

## Data Migration Approaches

### Online Migration (Zero Downtime)

```
Step 1: Dual-write new records to both old and new DB
Step 2: Backfill historical data from old DB to new DB
Step 3: Verify consistency (comparison job)
Step 4: Switch reads to new DB
Step 5: Stop writes to old DB
Step 6: Decommission old DB after verification period
```

### Backfill Script

```python
"""
Backfill historical data from legacy to new database.
Designed for zero-downtime migration with idempotent operations.
"""
import time
import logging

logger = logging.getLogger("backfill")

def backfill_table(
    legacy_db,
    new_db,
    table: str,
    batch_size: int = 1000,
    sleep_between_batches: float = 0.5,
):
    """Backfill data in batches with progress tracking."""
    total = legacy_db.query(f"SELECT COUNT(*) FROM {table}")[0][0]
    migrated = 0
    last_id = 0

    logger.info(f"Starting backfill of {table}: {total} records")

    while True:
        batch = legacy_db.query(
            f"SELECT * FROM {table} WHERE id > %s ORDER BY id LIMIT %s",
            (last_id, batch_size)
        )

        if not batch:
            break

        # Upsert to handle re-runs (idempotent)
        for record in batch:
            new_db.upsert(table, record)

        last_id = batch[-1]["id"]
        migrated += len(batch)

        logger.info(
            "backfill_progress",
            table=table,
            migrated=migrated,
            total=total,
            pct=f"{migrated / total * 100:.1f}%",
            last_id=last_id,
        )

        # Throttle to avoid overwhelming the databases
        time.sleep(sleep_between_batches)

    logger.info(f"Backfill complete: {table} ({migrated} records)")
    return migrated
```

---

## Feature Toggle Integration

Feature toggles are the control mechanism for strangler fig migrations.

### Toggle Configuration

```python
"""
Feature toggle-driven routing for strangler fig migration.
"""
import launchdarkly_server_sdk as ld

MIGRATION_FLAGS = {
    "orders-new-system": {
        "description": "Route order endpoints to new system",
        "phase": "canary",
        "rollout_pct": 10,
        "fallback": False,
    },
    "users-new-system": {
        "description": "Route user endpoints to new system",
        "phase": "complete",
        "rollout_pct": 100,
        "fallback": False,
    },
    "payments-new-system": {
        "description": "Route payment endpoints to new system",
        "phase": "not_started",
        "rollout_pct": 0,
        "fallback": False,
    },
}

class MigrationRouter:
    def __init__(self, ld_client):
        self.client = ld_client

    def should_use_new_system(self, feature: str, user_context: dict) -> bool:
        """Check if request should route to new system."""
        flag_key = f"{feature}-new-system"
        user = ld.Context.builder(user_context.get("user_id", "anonymous")).build()
        return self.client.variation(flag_key, user, False)

    def get_migration_status(self) -> dict:
        """Get current migration status for all features."""
        return {
            flag: {
                "phase": config["phase"],
                "rollout_pct": config["rollout_pct"],
            }
            for flag, config in MIGRATION_FLAGS.items()
        }
```

### Rollout Stages

| Stage | Rollout % | Duration | Validation |
|-------|-----------|----------|------------|
| **Off** | 0% | -- | Development and testing |
| **Shadow** | 0% (dual-run) | 1-2 weeks | Run both, compare outputs, route to legacy |
| **Canary** | 1-5% | 1 week | Monitor errors, latency, correctness |
| **Partial** | 10-50% | 1-2 weeks | Expand if metrics are green |
| **Majority** | 50-95% | 1 week | Final validation |
| **Complete** | 100% | 2 weeks | Monitoring period before decommission |
| **Decommission** | 100% + remove flag | -- | Remove legacy code and toggle |

---

## Rollback Strategies

Every migration step must be reversible.

### Rollback by Phase

| Phase | Rollback Action | Recovery Time | Data Impact |
|-------|----------------|---------------|-------------|
| **Proxy setup** | Remove proxy, point DNS to legacy | Minutes | None |
| **Canary routing** | Set feature flag to 0% | Seconds | None (stateless) |
| **Dual-write** | Stop writes to new DB, keep legacy as source of truth | Minutes | Discard new DB data |
| **Read migration** | Switch reads back to legacy DB | Seconds (flag) | None |
| **Write migration** | Switch writes back to legacy DB | Minutes | Reconcile any new-DB-only writes |
| **Decommission** | Restore legacy from backup (last resort) | Hours | Potential data loss |

### Rollback Automation

```bash
#!/bin/bash
# rollback-migration.sh: Emergency rollback for strangler fig migration

FEATURE="$1"
PHASE="$2"

if [ -z "$FEATURE" ]; then
    echo "Usage: rollback-migration.sh <feature> [phase]"
    echo "Features: orders, users, payments"
    exit 1
fi

echo "=== Rolling back migration for: $FEATURE ==="

# Step 1: Disable feature flag (route all traffic to legacy)
echo "Setting feature flag to 0%..."
curl -X PATCH "https://app.launchdarkly.com/api/v2/flags/default/${FEATURE}-new-system" \
  -H "Authorization: $LD_API_KEY" \
  -H "Content-Type: application/json" \
  -d '[{"op": "replace", "path": "/environments/production/on", "value": false}]'

# Step 2: Verify legacy is serving traffic
echo "Waiting 30 seconds for traffic to drain..."
sleep 30

echo "Checking legacy health..."
HEALTH=$(curl -sf "http://legacy-app:8080/health" | jq -r '.status')
if [ "$HEALTH" != "ok" ]; then
    echo "WARNING: Legacy health check failed!"
    exit 1
fi

# Step 3: Notify team
echo "Sending rollback notification..."
curl -X POST "$SLACK_WEBHOOK" \
  -H "Content-Type: application/json" \
  -d "{\"text\": \"Migration rollback: ${FEATURE} routed back to legacy. Investigating.\"}"

echo "Rollback complete. All traffic for $FEATURE now routes to legacy."
```

---

## Progress Tracking

### Migration Dashboard

```
+-----------------------------------------------------------+
| Strangler Fig Migration Progress                           |
+-----------------------------------------------------------+
| Feature      | Phase     | New % | Legacy % | Errors     |
|-------------|-----------|-------|----------|------------|
| Users        | Complete  | 100%  | 0%       | 0.01%      |
| Products     | Majority  | 80%   | 20%      | 0.02%      |
| Orders       | Canary    | 5%    | 95%      | 0.05%      |
| Payments     | Shadow    | 0%*   | 100%     | N/A        |
| Reports      | Not started| 0%   | 100%     | N/A        |
+-----------------------------------------------------------+
| Overall: 37% migrated | Estimated completion: Q3 2026     |
+-----------------------------------------------------------+
```

### Prometheus Metrics

```promql
# Migration progress: percentage of traffic to new system
sum(rate(strangler_proxy_requests_total{target="new"}[5m]))
/
sum(rate(strangler_proxy_requests_total[5m]))

# Error rate comparison: new vs legacy
sum(rate(strangler_proxy_requests_total{target="new", result="error"}[5m]))
/
sum(rate(strangler_proxy_requests_total{target="new"}[5m]))

# Latency comparison
histogram_quantile(0.99,
  sum(rate(http_request_duration_seconds_bucket{system="new"}[5m])) by (le)
)
```

### Minimum Parity Scorecard

Use a small scorecard before increasing rollout:

| Metric | Target Before Expanding Rollout |
|--------|---------------------------------|
| Consistency / parity | No unexplained mismatches on sampled comparable requests |
| Error rate | New path is no worse than legacy within agreed tolerance |
| p95 / p99 latency | No regression beyond agreed budget |
| Rollback readiness | Flag or routing control can send traffic back to legacy immediately |

---

## Risk Assessment per Phase

| Phase | Risk Level | Key Risks | Mitigations |
|-------|-----------|-----------|-------------|
| **Proxy setup** | Low | Latency overhead, proxy becomes SPOF | Load test proxy, deploy HA |
| **Shadow mode** | Low | Resource cost of dual-running | Monitor resource usage |
| **Canary** | Medium | New system bugs affect real users | Small %, immediate rollback |
| **Partial rollout** | Medium | Data consistency between systems | Dual-write verification |
| **Majority** | Medium-High | Edge cases at scale | Comprehensive monitoring |
| **Complete** | Low | Legacy decommission leaves orphans | Audit dependencies first |
| **Decommission** | Medium | Hidden dependencies on legacy | Keep legacy available for 30 days |

---

## Timeline Expectations

### Realistic Timelines

| System Complexity | Feature Count | Expected Duration | Team Size |
|-------------------|---------------|-------------------|-----------|
| Small monolith | 5-10 features | 3-6 months | 2-3 engineers |
| Medium monolith | 10-30 features | 6-12 months | 3-5 engineers |
| Large monolith | 30-100 features | 12-24 months | 5-10 engineers |
| Enterprise legacy | 100+ features | 2-5 years | Dedicated team |

### Phase Duration Guidelines

```
Proxy Setup:          1-2 weeks
Per Feature Migration:
  Shadow mode:        1-2 weeks
  Canary (1-5%):      1 week
  Partial (10-50%):   1-2 weeks
  Majority (50-95%):  1 week
  Complete (100%):    1 week
  Monitoring period:  2 weeks
  Decommission:       1 week
  ---
  Total per feature:  6-10 weeks

With 10 features in parallel (2-3 at a time): ~6-9 months
```

---

## Common Pitfalls

### Pitfall 1: Big Bang Temptation

**Problem:** Team decides "we've migrated 80%, let's just do the rest at once."

**Why it fails:** The remaining 20% is usually the hardest (legacy edge cases, complex integrations, undocumented behavior).

**Prevention:** Maintain the same per-feature cadence for every feature. No shortcuts.

### Pitfall 2: Incomplete State Migration

**Problem:** New system handles requests but doesn't have all the historical state. Users see missing data.

**Prevention:**
- Backfill all historical data before routing reads
- Verify data completeness with automated comparison jobs
- Test with real user accounts in staging

### Pitfall 3: Proxy Becoming a Monolith

**Problem:** Business logic creeps into the proxy layer (data transformation, validation, authorization).

**Prevention:**
- Proxy should ONLY route. No business logic.
- Code review proxy changes strictly
- If you need transformation, do it in the destination service

### Pitfall 4: Neglecting Legacy Maintenance

**Problem:** Team focuses only on new system. Legacy gets zero maintenance and starts breaking.

**Prevention:**
- Legacy still serves production traffic. Keep it stable.
- Budget 20% of time for legacy bug fixes during migration
- Monitor legacy as carefully as the new system

### Pitfall 5: No Equivalence Testing

**Problem:** New system subtly differs from legacy (rounding, encoding, null handling). Users notice.

**Prevention:**
- Shadow mode with output comparison
- Characterization tests from legacy behavior
- Diff every response field during canary

### Summary Checklist

- [ ] Proxy layer is routing-only (no business logic)
- [ ] Feature flags control routing (not code deploys)
- [ ] Dual-write verification running
- [ ] Characterization tests prove equivalence
- [ ] Rollback tested for each phase
- [ ] Progress dashboard visible to stakeholders
- [ ] Legacy system still maintained and monitored
- [ ] Data backfill verified for completeness
- [ ] Decommission plan documented per feature

---

## Related Resources

- [Characterization Testing](./characterization-testing.md) - Proving behavioral equivalence
- [Legacy Code Strategies](./legacy-code-strategies.md) - Working with legacy systems
- [Operational Patterns](./operational-patterns.md) - CI/CD patterns for safe refactoring
- [Tech Debt Management](./tech-debt-management.md) - Prioritizing migration work
- [Automated Refactoring Tools](./automated-refactoring-tools.md) - Tool-assisted code migration
- [SKILL.md](../SKILL.md) - Parent skill overview

# Technical Debt Management

Comprehensive guide to identifying, measuring, tracking, and managing technical debt.

## Contents

- [What is Technical Debt?](#what-is-technical-debt)
- [Technical Debt Quadrant](#technical-debt-quadrant)
- [Measuring Technical Debt](#measuring-technical-debt)
- [8 Key Metrics for Technical Debt](#8-key-metrics-for-technical-debt)
- [Technical Debt Register](#technical-debt-register)
- [Feeding the Register from Agent-Authored Shortcut Markers](#feeding-the-register-from-agent-authored-shortcut-markers)
- [Managing Technical Debt](#managing-technical-debt)
- [Debt Prevention](#debt-prevention)
- [Communicating Technical Debt](#communicating-technical-debt)
- [Technical Debt in Agile](#technical-debt-in-agile)
- [Tools and Platforms](#tools-and-platforms)
- [Illustrative Debt-Reduction Sequence](#illustrative-debt-reduction-sequence-not-a-verified-case-study)
- [Best Practices Summary](#best-practices-summary)
- [References](#references)

---

## What is Technical Debt?

**Definition**: Technical debt is the implied cost of future rework caused by choosing an easy (limited) solution now instead of a better approach that would take longer.

**Origin**: Coined by Ward Cunningham in 1992, comparing shortcuts in code to financial debt that accrues interest.

---

## Technical Debt Quadrant

Martin Fowler's classification framework:

```
                    Reckless  |  Prudent
                    ─────────────────────
Deliberate    │  "We don't have  │  "We must ship
              │   time for design"│   now and deal
              │                   │   with consequences"
              ├─────────────────────────
Inadvertent   │  "What's         │  "Now we know
              │   layering?"     │   how we should
              │                   │   have done it"
```

### Quadrant Details

**1. Reckless Deliberate** (Avoid)
- Knowingly taking shortcuts without plan to fix
- "We don't have time for design"
- Dangerous and accumulates quickly

**2. Prudent Deliberate** (Acceptable short-term)
- Strategic decision to ship fast
- "We must ship now and deal with consequences later"
- Document decision and plan to address

**3. Reckless Inadvertent** (Fix through training)
- Lack of knowledge or skills
- "What's layering? What's dependency injection?"
- Address through education and mentoring

**4. Prudent Inadvertent** (Normal learning)
- Learning from experience
- "Now we know how we should have done it"
- Part of normal development process

---

## Measuring Technical Debt

### SonarQube Metrics

**Technical Debt Ratio (TDR)**:
```
TDR = (Remediation Cost / Development Cost) × 100

Example:
Remediation Cost: 50 hours (to fix all issues)
Development Cost: 500 hours (total project time)
TDR: 10%

Thresholds:
< 5%:    Excellent
5-10%:   Good
10-20%:  Needs attention
> 20%:   Critical
```

**Key Metrics**:
- **Code Smells**: Maintainability issues
- **Bugs**: Reliability issues
- **Vulnerabilities**: Security issues
- **Coverage**: Test coverage percentage
- **Duplications**: Duplicate code blocks
- **Complexity**: Cyclomatic complexity

### SonarQube Quality Gates

```yaml
# sonar-project.properties
sonar.projectKey=my-project
sonar.organization=my-org

# Quality Gate thresholds
sonar.qualitygate.wait=true

# Code coverage
sonar.coverage.threshold=80

# Duplications
sonar.duplications.threshold=3

# Complexity
sonar.complexity.threshold=10

# Ratings (A-E scale)
sonar.maintainability.rating=A
sonar.reliability.rating=A
sonar.security.rating=A

# Technical debt
sonar.techdebt.threshold=5  # 5% maximum TDR
```

---

## 8 Key Metrics for Technical Debt

### 1. Technical Debt Ratio (TDR)
Percentage of development time spent fixing debt.

### 2. Code Churn
Rate of code changes over time. High churn indicates instability.

```
Code Churn = (Lines Added + Lines Deleted) / Total Lines
```

### 3. Cycle Time
Time from commit to deployment. Longer cycles suggest debt.

### 4. Defect Density
Number of bugs per lines of code.

```
Defect Density = Total Defects / KLOC (thousands of lines of code)
```

### 5. Code Duplication
Percentage of duplicated code blocks.

### 6. Cyclomatic Complexity
Number of independent paths through code. Higher = more complex.

**Thresholds**:
- 1-10: Simple, low risk
- 11-20: Moderate, medium risk
- 21-50: Complex, high risk
- 50+: Very high risk, untestable

### 7. Code Coverage
Percentage of code covered by tests.

**Targets**:
- Critical paths: 100%
- Business logic: 90%+
- Overall: 80%+

### 8. Failed Builds
Frequency of CI/CD failures indicates quality issues.

---

## Technical Debt Register

Track and prioritize technical debt items.

### Template

| ID | Description | Type | Impact | Effort | Priority | Created | Owner | Status |
|----|-------------|------|--------|--------|----------|---------|-------|--------|
| TD-001 | Refactor UserService (600 lines) | Prudent Deliberate | High | 2 days | P1 | 2025-10-01 | Alice | In Progress |
| TD-002 | Add tests for PaymentProcessor | Reckless Inadvertent | Medium | 3 days | P2 | 2025-09-15 | Bob | Backlog |
| TD-003 | Extract shared validation logic | Prudent Inadvertent | Low | 1 day | P3 | 2025-11-01 | Charlie | Backlog |
| TD-004 | Remove deprecated API endpoints | Deliberate | Medium | 1 day | P2 | 2025-08-20 | Dave | Completed |

### Prioritization Matrix

```
          High Impact
              │
    P1 = Do Now │ P2 = Plan
    ─────────────┼─────────────
    P3 = Maybe  │ P4 = Skip
              │
          Low Impact
        Low Effort → High Effort
```

**Priority Levels**:
- **P1**: High impact, low effort → Do immediately
- **P2**: High impact, high effort → Plan and schedule
- **P3**: Low impact, low effort → Maybe do if time
- **P4**: Low impact, high effort → Skip or reconsider

---

## Feeding the Register from Agent-Authored Shortcut Markers

**Use when:** An AI coding agent has left inline `agent-debt:` markers in a session (deliberate simplifications the agent flagged rather than silently shipping) and those markers need to become tracked, prioritized register entries rather than comments nobody revisits.

The marker convention itself — format, provenance separation from human `TODO`/`FIXME`, and the "no upgrade trigger = rot risk" flag — is defined once, as `CC-DOC-05` in [`software-clean-code-standard/references/clean-code-standard.md`](../../software-clean-code-standard/references/clean-code-standard.md). Do not restate the marker format here; this section covers only what happens after a grep sweep finds those markers.

- Grep for the marker (`rg -n '(#|//) ?agent-debt:' .`) and add one Technical Debt Register row per hit, not per file — two markers in the same file are two separate debt items with independent payback plans.
- Carry the marker's `ceiling` and `upgrade` text into the register row's Description and Priority fields verbatim; do not paraphrase the upgrade trigger, since paraphrasing is exactly how a trigger condition quietly drifts out of sync with the code.
- A marker CC-DOC-05 already flags as rot risk (no upgrade trigger) is a P1 register candidate by default, not P3 — an untriggered shortcut is debt with no scheduled repayment, which sits in the highest-risk quadrant cell (Reckless Deliberate) even when the original shortcut was reasonable at the time.
- Re-run the grep sweep on the same cadence used for the rest of this register (sprint boundary or debt-review meeting) so markers aging past their own upgrade trigger get promoted, not just markers with no trigger at all.

---

## Managing Technical Debt

### Boy Scout Rule

> "Leave the code better than you found it."

**When touching a file**:
- [ ] Fix at least one code smell
- [ ] Add missing tests
- [ ] Improve naming
- [ ] Extract duplicated code
- [ ] Add documentation

### 20% Time Rule

Allocate 20% of sprint capacity to debt reduction:
- **80%**: New features
- **20%**: Refactoring, testing, documentation

### Debt Reduction Strategies

**1. Incremental Refactoring**
- Small, safe changes
- One pattern at a time
- Run tests after each change

**2. Feature Freeze Sprints**
- Dedicate sprint to debt reduction
- No new features
- Focus on quality improvements

**3. Debt Day**
- One day per week for debt
- Rotate team members
- Track progress

**4. Opportunistic Refactoring**
- Refactor while working on features
- Make code better for new feature
- Don't break existing functionality

---

## Debt Prevention

### Code Review Checklist

- [ ] No duplicate code
- [ ] Methods < 20 lines
- [ ] Classes < 300 lines
- [ ] Functions have < 4 parameters
- [ ] No magic numbers
- [ ] Meaningful names
- [ ] Tests included
- [ ] Documentation added

### Automated Quality Gates

**Pre-commit Hooks**:
```json
{
  "husky": {
    "hooks": {
      "pre-commit": "lint-staged"
    }
  },
  "lint-staged": {
    "*.{js,ts}": [
      "eslint --fix",
      "prettier --write",
      "jest --bail --findRelatedTests"
    ]
  }
}
```

**CI/CD Gates**:
```yaml
name: Quality Check

on: [pull_request]

jobs:
  quality:
    runs-on: ubuntu-latest
    steps:
      - name: Lint
        run: npm run lint

      - name: Test
        run: npm run test:coverage

      - name: SonarQube Scan
        uses: sonarsource/sonarqube-scan-action@<pinned-version>

      - name: Quality Gate
        run: |
          if [ $SONAR_QUALITY_GATE == "ERROR" ]; then
            exit 1
          fi
```

---

## Communicating Technical Debt

### To Non-Technical Stakeholders

**Don't say**: "We have high cyclomatic complexity in the UserService class."

**Do say**: "Our user management code is becoming difficult to maintain, which will slow down new features and increase bug risk. We need 3 days to simplify it."

### Business Impact Framework

**Connect debt to business outcomes**:

| Technical Issue | Business Impact |
|----------------|-----------------|
| High complexity | Slower feature delivery |
| Low test coverage | More production bugs |
| Code duplication | Inconsistent behavior |
| Legacy code | Hard to hire developers |
| Security debt | Risk of data breach |

### ROI Calculation

```
Cost of debt:
- Bug fix time: $500/week
- Slow feature delivery: $2000/week
- Developer frustration: $1000/week
Total: $3500/week

Investment to fix:
- 2 weeks refactoring: $10,000

ROI:
Payback period: 3 weeks
Annual savings: $182,000
```

---

## Technical Debt in Agile

### Scrum Integration

**Product Backlog**:
- Technical debt items as user stories
- Prioritize with business features
- Estimate using story points

**Sprint Planning**:
- Include debt reduction tasks
- Balance features and debt
- Track velocity impact

**Retrospectives**:
- Discuss new debt created
- Review debt reduction progress
- Adjust 20% allocation

### Debt Story Template

```
As a developer
I want to refactor the UserService class
So that it's easier to maintain and extend

Acceptance Criteria:
- [ ] UserService < 300 lines
- [ ] Extract authentication logic
- [ ] Extract validation logic
- [ ] Test coverage > 80%
- [ ] Cyclomatic complexity < 10

Definition of Done:
- [ ] Code reviewed
- [ ] Tests pass
- [ ] Documentation updated
- [ ] SonarQube metrics improved
```

---

## Tools and Platforms

### Static Analysis Tools

| Tool | Language | Key Features |
|------|----------|-------------|
| SonarQube | Multi-language | Comprehensive analysis, quality gates |
| CodeClimate | Multi-language | Maintainability metrics, GitHub integration |
| ESLint | JavaScript/TS | Linting, custom rules |
| Pylint | Python | Code analysis, PEP 8 compliance |
| RuboCop | Ruby | Style enforcement, security checks |

### Debt Tracking Tools

| Tool | Key Features |
|------|-------------|
| Jira | Debt as issues, custom fields, roadmaps |
| Linear | Modern interface, issue tracking |
| Stepsize | Dedicated debt tracking, metrics |
| GitHub Projects | Simple, integrated with code |

### AI-Assisted Workflows

| Category | Best Use |
|----------|----------|
| Agent/editor copilots | Draft narrow refactors and summarize risk hotspots |
| IDE-native refactors | Compiler-aware renames, moves, and signature changes |
| Static analysis platforms | Track debt trends and identify hotspots worth prioritizing |

---

## Illustrative Debt-Reduction Sequence (Not a Verified Case Study)

The plan below is a composite illustrating a workable sequence and rough timeframes — it is not a documented result from a named, citable company, and the specific before/after numbers are placeholders, not sourced measurements. Do not cite them as expected outcomes; a real program's numbers depend entirely on codebase size, team size, and starting debt level.

**Illustrative starting point**: mid-size codebase, elevated debt ratio (per SonarQube TDR), low test coverage, slow build.

**Strategy** (6-month plan):

**Month 1-2: Measurement**
- Install SonarQube
- Create debt register
- Baseline metrics

**Month 3-4: Quick Wins**
- Remove dead code
- Fix obvious code smells
- Add missing tests

**Month 5-6: Strategic Refactoring**
- Refactor high-churn files
- Extract shared logic
- Improve architecture

**Directionally expected outcomes** (order of magnitude, not a guarantee): debt ratio and build time trend down, test coverage and deploy frequency trend up. Measure your own baseline before promising a number to stakeholders — see [ROI Calculation](#roi-calculation) for how to build a defensible estimate from your own cost data instead of an industry average.

---

## Best Practices Summary

1. **Measure and track** debt continuously
2. **Allocate 20%** of time to debt reduction
3. **Boy Scout Rule** - always improve code you touch
4. **Automate quality gates** to prevent new debt
5. **Communicate business impact** to stakeholders
6. **Prioritize by impact** and effort
7. **Incremental refactoring** over big bang rewrites
8. **Include debt in sprint planning**
9. **Use tools** for detection and tracking
10. **Make debt visible** to entire team

---

## References

- **Managing Technical Debt** - Philippe Kruchten
- **SonarSource - Measuring Technical Debt** - https://www.sonarsource.com/learn/
- **Martin Fowler - Technical Debt Quadrant** - https://martinfowler.com/bliki/TechnicalDebtQuadrant.html
- **Stepsize - Technical Debt Metrics** - https://www.stepsize.com/blog/
- **CircleCI - Technical Debt Management** - https://circleci.com/blog/

---
name: qa-refactoring
description: "Safe refactoring with behavior preservation. Use when reducing technical debt, planning codemods, applying strangler migrations, or tightening CI guardrails around risky changes."
compatibility: Portable core. Works on Claude Code and Codex.
version: "1.1"
last_validated: 2026-07-11
---

# QA Refactoring

Use this skill to refactor safely: preserve behavior, reduce risk, and keep CI green while improving maintainability and delivery speed.

Defaults: baseline first, smallest safe step next, and proof via tests/contracts/observability instead of intuition.

## Quick Start (10 Minutes)

- If key context is missing, ask for: what must not change (invariants), risk level (money/auth/migrations/concurrency), deployment constraints, and the smallest boundary that can be protected by tests.
- Confirm baseline: `main` green; reproduce the behavior you must preserve.
- Choose a boundary: API surface, module boundary, DB boundary, request handler, or codemod blast radius.
- Add a safety net: characterization/contract/integration tests at that boundary.
- Refactor in micro-steps: one behavior-preserving change per commit/PR chunk.
- Prove: run the smallest relevant suite locally, then full CI; keep failures deterministic and artifact-rich.

## Workflow

1. Establish the safety net, boundaries, and rollback shape.
2. Choose the refactoring strategy and smallest change slice.
3. Make the change, verify behavior, and stop if the safety bar drops below acceptable risk.
4. Capture the next slice instead of expanding scope mid-pass.

## Core QA (Default)

### Safe Refactor Loop (Behavior First)

- Establish baseline: get `main` green; reproduce the behavior you must preserve.
- Define invariants: inputs/outputs, error modes, permissions, data shape, performance budgets.
- Add a safety net: write characterization/contract/integration tests around the boundary you will touch.
- Create seams: introduce injection points/adapters to isolate side effects and external dependencies.
- Refactor in micro-steps: one behavior-preserving change at a time; keep diffs reviewable.
- Prove: run the smallest relevant suite locally, then full CI; keep failures debuggable and deterministic.
- Ship safely: use canary, shadow mode, migration flags, or branch-by-abstraction when refactors touch production-critical paths.

### Risk Levels (Choose Safety Net)

| Risk | Examples | Minimum required safety net |
|------|----------|-----------------------------|
| Low | rename, extract method, formatting-only | unit tests + lint/type checks |
| Medium | moving logic across modules, dependency inversion, codemods with narrow blast radius | unit + integration/contract tests at boundary |
| High | auth/permission paths, concurrency, migrations, money/data-loss paths, large-scale automated rewrites | integration + contract tests, observability checks, rollout + rollback plan |

### Test Strategy for Refactors

- Prefer contract and integration tests around boundaries to preserve behavior.
- Use snapshots/golden masters only when outputs are stable and reviewed (avoid "approve everything" loops).
- For invariants, consider property-based tests or table-driven cases (inputs, edge cases, error modes).
- Avoid making E2E/UI tests the primary safety net for refactors; keep most safety below the UI.
- For flaky areas: fix determinism first (seeds, time, ordering, network) before trusting results.
- For API and service boundaries, prefer explicit contract tests over ad hoc end-to-end coverage.
- For automated rewrites, sample diffs manually before scaling to the full repository.

### CI Economics and Debugging Ergonomics

- Keep refactor PRs small and reviewable; avoid refactor + feature in one PR.
- Require failure artifacts for tests guarding refactors (logs, trace IDs, deterministic seeds, repro steps).
- Reduce diff noise: isolate formatting-only changes (or apply formatting repo-wide once with buy-in).
- Keep `git bisect` viable: avoid mixed "mechanical + semantic" changes unless necessary.
- For codemods, use dry runs first, then batch execution with a stop-on-failure path.

### Do / Avoid

Do:

- Add missing tests before refactoring high-risk areas.
- Add guardrails (linters, type checks, contract checks, static analysis/security checks) so refactors don't silently break interfaces.
- Prefer "branch by abstraction" / adapters when you need to swap implementations safely.
- Prefer compiler-aware and AST-aware refactors over regex edits when touching many files.
- Treat "more than ~500 lines touched by hand" as a second, line-count trigger for the same automate-vs-manual decision the file-count table in [references/automated-refactoring-tools.md](references/automated-refactoring-tools.md#when-ide-refactoring-is-sufficient) already makes on file count — a refactor can cross one threshold without the other (a single 2,000-line file is one file; a 600-file mechanical rename can be a few lines each), so check both axes, not just files.

Avoid:

- Combining large structural refactors with behavior changes.
- Using flaky E2E as the primary safety net for refactors.
- Treating arbitrary size thresholds as rules of nature; use both the file-count and line-count thresholds above only as review heuristics that prompt "should this be a codemod," not as hard gates — the decision is whether automation is safer than hand-editing at this scale, not compliance with a number.

The line-count trigger is adapted from addyosmani/agent-skills (MIT), commit `7676817`, 2026-08-09.

## Expert Judgment: What a Checklist Misses

### When NOT to Refactor

Refactoring is an investment decision, not a moral obligation — weigh it like one.

- Ask what the refactor buys (velocity, defect reduction, unblocking a specific feature) against what it costs (time, review load, regression risk) before starting. "This code is ugly" is not a business case by itself.
- Do not refactor stable, rarely-touched code that is scheduled for retirement or replacement — cleaning up code you are about to delete is waste. Check the migration roadmap before investing.
- Prioritize by hotspot (churn × complexity), not raw LOC or aesthetic discomfort. A messy function nobody touches is lower priority than a merely-average function that changes every sprint.
- Apply Kent Beck's framing from *Tidy First?* (2023): tidy first only when the tidying pays for itself in the change you are about to make — it shrinks the diff, de-risks it, or reveals the real shape of the work. If a tidying does not unlock the change you actually need, defer it to its own reviewed, low-stakes commit rather than bundling it in "while I'm here."
- If a strangler-fig migration will replace this module within the current roadmap horizon, put the investment into seams and characterization tests for the migration, not into deep internal refactors of code that is going away.

### Characterization-Test-First Discipline

- "I understand this code" is not a safety net — write the test, not just the belief. If you cannot state what a function returns for its three trickiest inputs without running it, you do not understand it well enough to skip characterization tests.
- Characterization tests capture behavior *including bugs*. Do not silently fix bugs while characterizing — log discovered bugs separately and let the product owner decide fix timing and sequencing relative to the refactor.
- Chesterton's Fence applies to deletion specifically: before removing code that looks unused or wrong, `git blame`/`git log -p` the lines to find the commit and linked ticket/PR that added them, and check for a comment explaining the rationale. Only delete once you can state why the fence was put up, not just that you can't currently see a reason for it.

### Behavior-Preservation Verification Strategies

- Verify in layers, cheapest first: type check → lint → unit/characterization → contract/integration → mutation-score gate on the touched boundary → canary/shadow for production-critical paths. Stop widening scope the moment a cheaper layer would have caught the same class of regression.
- For a refactor with no intended behavior change, the review question is not "is this good code" but "can I prove nothing observable changed." Golden master, property tests, and contract tests are the proof; code review alone only checks that the change *looks* safe.
- Explicitly test concurrency ordering, floating-point rounding, and error-message text that another system parses — these are the most common sources of invisible behavior change in an otherwise-clean refactor.

### Refactoring Under Deadline Pressure (Triage)

- Under time pressure, shrink scope — do not drop the safety net. A 30-minute characterization test around the touched boundary is cheaper than the incident it prevents.
- If there truly is no time for tests, restrict yourself to the most mechanical change possible (rename, extract-without-changing-logic) and defer anything that changes control flow or data shape to a follow-up ticket. Do not combine "fast" with "risky."
- Record the shortcut in the technical debt register in the same PR — deadline debt that is not written down does not get paid down later.
- Escalate rather than silently absorb: if the deadline forces skipping the safety net on a high-risk path (money, auth, data, migrations), say so explicitly to the reviewer or product owner instead of quietly shipping it.

### Strangler Fig vs. Big-Bang Rewrite

- Default to strangler fig whenever the system has live users or traffic and halting feature delivery for months is unacceptable — the incremental path is usually cheaper than the rewrite ever gets credited for, once you count the risk of a multi-month all-or-nothing cutover.
- Big-bang rewrite is defensible only when: there is no live traffic yet (true greenfield replacement), the domain logic is small enough for one team to hold in their heads, or the legacy system is so broken (unsupported runtime, expired license, unpatchable security hole) that partial operation is not viable.
- Watch for the "we're 80% migrated, let's finish it in one push" trap. The remaining slice is disproportionately the undocumented edge cases; hold the same per-feature discipline (one seam at a time, parity metrics before widening rollout) on the last slice as on the first.
- If a rewrite is genuinely chosen, still slice it: ship the smallest end-to-end vertical slice first and route real (even low-value) traffic through it before building the rest, to get delivery feedback without full big-bang risk.

### LLM Agents and Subtle Behavior Changes During "Refactors"

Prompts framed as "refactor this" fail agents in a specific way: the agent notices a local improvement opportunity and takes it, silently expanding scope from "same behavior, better structure" to "same behavior, better structure, plus a few fixes I noticed along the way." Watch for, and gate against, these failure modes:

- **Error-handling drift** — narrowing/widening an `except`/`catch`, turning a silent failure into a raised exception (or the reverse), or changing a default on an error path. Nearly invisible in review because the "refactored" code reads as cleaner.
- **Boundary drift** — `>` becomes `>=` (or vice versa) while consolidating near-duplicate conditionals.
- **Rounding/precision drift** — a manual accumulation loop replaced by a library call with different floating-point behavior at the margins; high-risk for money and scientific code.
- **Ordering/concurrency drift** — reordering statements that looked independent, or narrowing a lock scope, changes observable ordering or introduces a race that only shows up under load.
- **Test-healing anti-pattern** — when characterization or existing tests fail after an agent's change, the agent's default move is often to loosen the assertion or delete the failing case to turn the suite green. Treat every test-file change inside a "pure refactor" PR as a flag requiring explicit human justification, not a housekeeping detail.
- **Scope creep on multi-file rewrites** — 2026 practitioner write-ups on agent-driven refactors (a single practitioner's analysis, not a peer-reviewed benchmark — treat the exact figures as **unverified as of 2026-07-11**) describe roughly 40% real-world success on enterprise multi-file refactors and roughly a third on legacy codebases, notably below marketing claims, attributed to "lost in the middle" context loss, architectural drift (locally sensible, globally inconsistent decisions), and index staleness against a codebase that moved on after the agent's context was built. Whatever the precise number, scope the blast radius and sample-review real diffs before trusting a multi-file agent pass at scale.

Gate before merging an agent-authored "refactor":

1. Diff the test files first, before the source diff — any weakened, deleted, or newly-skipped assertion is disqualifying until a human explains why.
2. Re-run the pre-existing (not agent-authored) characterization/contract suite; a green agent-authored test suite proves nothing about behavior preservation on its own.
3. Where the agent also generated the safety net, mutation-test the touched boundary before trusting those tests — see [references/mutation-testing.md](references/mutation-testing.md#mutation-score-as-the-ai-generated-test-validator). Line coverage alone is exactly the metric agents learn to game.
4. Sample-review the actual diff for the drift patterns above yourself; do not accept a diff summary from the same agent that wrote the diff as a substitute for reading it.
5. Never let an agent expand scope mid-task ("while I was in there, I also…"); split any such change into its own reviewed PR.

## Quick Reference

| Task | Tool/Pattern | Command/Approach | When to Use |
| ---- | ------------ | ---------------- | ----------- |
| Long or mixed-concern function | Extract Method | Split into smaller functions | Single function mixes validation, orchestration, and side effects |
| Large or low-cohesion class/module | Split Class / Extract Module | Create focused units with narrower responsibilities | One type owns unrelated workflows or too many dependencies |
| Duplicated code | Extract Function/Class | DRY principle | Same logic in multiple places |
| Complex conditionals | Replace Conditional with Polymorphism | Use inheritance/strategy pattern | Switch statements on type |
| Long parameter list | Introduce Parameter Object | Create DTO/config object | Functions with >3 parameters |
| Legacy code modernization | Characterization Tests + Strangler Fig | Write tests first, migrate incrementally | No tests, old codebase |
| Large mechanical rewrite | Codemod / AST transform | Dry-run, sample diff review, staged batch rollout | Renames, API migrations, repetitive edits across many files |
| Java framework migration (Spring Boot, Java version) | OpenRewrite recipe via Moderne CLI or MCP | `mod run . --recipe UpgradeSpringBoot_3_4` | AI agents can invoke 5,000+ OpenRewrite recipes as deterministic tool calls |
| Automated quality gates | Compiler + linter + contract checks | CI pipeline with fail-fast checks and artifacts | Prevent silent regression during refactors |
| Technical debt tracking | Debt register + static analysis | Track trends, hotspots, and owners | Prioritize refactoring work |

## Decision Tree: Refactoring Strategy

```text
Code issue: [Refactoring Scenario]
    ├─ Code Smells Detected?
    │   ├─ Duplicated code? → Extract method/function
    │   ├─ Mixed concerns in one function? → Extract smaller methods
    │   ├─ Low cohesion / too many dependencies? → Split into focused classes or modules
    │   ├─ Long parameter list? → Parameter object
    │   └─ Feature envy? → Move method closer to data
    │
    ├─ Legacy Code (No Tests)?
    │   ├─ High risk? → Write characterization tests first
    │   ├─ Large rewrite needed? → Strangler Fig (incremental migration)
    │   ├─ Unknown behavior? → Characterization tests + small refactors
    │   └─ Production system? → Canary/shadow rollout + monitoring
    │
    ├─ Repetitive Multi-File Edit?
    │   ├─ Compiler/IDE can prove rename? → Use native refactor tooling
    │   ├─ Pattern is syntactic/semantic? → Use codemod or AST rewrite
    │   └─ Blast radius is large? → Dry-run + sample review + batch rollout
    │
    ├─ Quality Standards?
    │   ├─ New project? → Setup compiler/linter/test gates
    │   ├─ Existing project? → Add pre-commit hooks + CI checks
    │   ├─ Complexity hotspots? → Add targeted guardrails and characterization tests
    │   └─ Technical debt? → Track in register with owners and review cadence
```

## ASCII Flow

```text
Refactoring request
  -> State behavior that must not change and rollback boundary
  -> Capture baseline with tests, contracts, metrics, or characterization output
  -> Choose the smallest behavior-preserving step
  -> Apply native refactor tooling, codemod, or manual edit as appropriate
  -> Run targeted verification before widening scope
  -> Retire debt, flags, dead code, or guardrails only with evidence
```

## Navigation

- `## Workflow`, `## Core QA (Default)`, and `## Decision Tree: Refactoring Strategy` for the baseline sequence
- `## Operational Deep Dives`, `## Templates`, and `## Resources` for deeper materials
- `## Related Skills` for testing, architecture, and code-review handoffs

## Related Skills

| Skill | Purpose |
|-------|---------|
| [qa-debugging](../qa-debugging/SKILL.md) | Debugging production issues and test flakes |
| [software-code-review](../software-code-review/SKILL.md) | Code review process and checklists |
| [software-architecture-design](../software-architecture-design/SKILL.md) | Architecture design and redesign decisions |
| [qa-testing-strategy](../qa-testing-strategy/SKILL.md) | Test strategy and coverage planning |
| [data-sql-optimization](../data-sql-optimization/SKILL.md) | Performance tuning, SQL, and query plans |

## Operational Deep Dives

### Shared Foundation

- [../software-clean-code-standard/references/clean-code-standard.md](../software-clean-code-standard/references/clean-code-standard.md) - Canonical clean code rules (`CC-*`) for citation
- Legacy playbook: [../software-clean-code-standard/references/code-quality-operational-playbook.md](../software-clean-code-standard/references/code-quality-operational-playbook.md) - `RULE-01`–`RULE-13`, decision trees, and operational procedures
- [../software-clean-code-standard/references/refactoring-operational-checklist.md](../software-clean-code-standard/references/refactoring-operational-checklist.md) - Refactoring smell-to-action mapping, safe refactoring guardrails
- [../software-clean-code-standard/references/working-effectively-with-legacy-code-operational-checklist.md](../software-clean-code-standard/references/working-effectively-with-legacy-code-operational-checklist.md) - Seams, characterization tests, incremental migration patterns

### Skill-Specific

See [references/operational-patterns.md](references/operational-patterns.md) for detailed refactoring catalogs, codemod rollout patterns, quality gates, technical debt playbooks, and legacy modernization steps.

## Templates

Use copy-paste templates in `assets/` for checklists and quality-gate configs:

- Refactoring: [assets/process/refactoring-checklist.md](assets/process/refactoring-checklist.md), [assets/process/code-review-quality.md](assets/process/code-review-quality.md)
- Technical debt: [assets/tracking/tech-debt-register.md](assets/tracking/tech-debt-register.md)
- Quality gates: [assets/quality-gates/javascript/eslint-config.js](assets/quality-gates/javascript/eslint-config.js), [assets/quality-gates/platform-agnostic/sonarqube-setup.md](assets/quality-gates/platform-agnostic/sonarqube-setup.md)

## Resources

Use deep-dive guides in `references/` (load only what you need):

- **Operational Patterns**: [references/operational-patterns.md](references/operational-patterns.md) - Core refactoring catalogs, quality gates, and legacy modernization
- **Refactoring Catalog**: [references/refactoring-catalog.md](references/refactoring-catalog.md)
- **Code Smells Guide**: [references/code-smells-guide.md](references/code-smells-guide.md)
- **Technical Debt Management**: [references/tech-debt-management.md](references/tech-debt-management.md)
- **Legacy Code Modernization**: [references/legacy-code-strategies.md](references/legacy-code-strategies.md)
- **Brownfield Agent Loop**: [references/brownfield-agent-loop.md](references/brownfield-agent-loop.md) - Driving a coding agent against a legacy repo: graph the repo, pick the seam, build an immutable acceptance gate, scope one task, run the loop
- **Characterization Testing**: [references/characterization-testing.md](references/characterization-testing.md) - Golden master and approval testing patterns
- **Strangler Fig Migration**: [references/strangler-fig-migration.md](references/strangler-fig-migration.md) - Incremental legacy migration strategies (includes expand-contract / parallel change callout)
- **Automated Refactoring Tools**: [references/automated-refactoring-tools.md](references/automated-refactoring-tools.md) - Codemods, AST transforms, recipe testing, and IDE refactoring
- **Mikado Method**: [references/mikado-method.md](references/mikado-method.md) - Prerequisite tree for entangled legacy changes; leaf-first execution
- **Mutation Testing**: [references/mutation-testing.md](references/mutation-testing.md) - Stryker/mutmut/cosmic-ray/PIT; mutation score, CI thresholds, incremental runs
- **Feature Flag Retirement**: [references/feature-flag-retirement.md](references/feature-flag-retirement.md) - Step-by-step recipe: identify references, remove dead branch, delete definition last

## Optional: AI / Automation

Do:

- Use AI to propose mechanical refactors (rename/extract/move) only when you can prove behavior preservation via tests and contracts.
- Use AI to summarize diffs and risk hotspots; verify by running targeted characterization tests.
- Prefer tool-assisted refactors (IDE/compiler-aware, codemods) over freeform text edits when available.
- Treat agent-generated refactors as draft patches until a human reviews representative diffs and the safety net is green.
- For Java projects, prefer OpenRewrite recipes (via `mod` CLI or MCP server) over hand-written codemods — recipes are deterministic and version-aware; AI agents can invoke them directly as tool calls.
- Validate AI-generated characterization tests with mutation testing before treating them as a refactor safety net; line coverage alone does not prove test quality. See [references/mutation-testing.md](references/mutation-testing.md#mutation-score-as-the-ai-generated-test-validator).
- Before pointing an agent at a legacy repo, build the acceptance gate first and never let the agent modify it; graph the repo to choose the seam instead of letting the agent pick what it understands best. See [references/brownfield-agent-loop.md](references/brownfield-agent-loop.md).
- For large multi-file agent refactors, scope the blast radius before execution: a single 2026 practitioner report (not a peer-reviewed benchmark — treat the exact figures as unverified as of 2026-07-11) put AI agent success at roughly 40% on enterprise multi-file refactors and roughly a third on legacy codebases; whatever the true number, scope and review discipline are essential. See "LLM Agents and Subtle Behavior Changes During 'Refactors'" above.

Avoid:

- Accepting refactors that change behavior without an explicit requirement and regression tests.
- Letting AI "fix tests" by weakening assertions to make CI green.
- Rolling out AI-generated multi-file edits repo-wide without a dry run and sample review.
- Letting agents expand scope autonomously mid-refactor; define the boundary and blast radius before the agent starts.

See [data/sources.json](data/sources.json) for curated external references.

## Fact-Checking

- Known bugs, regressions, framework/compiler/runtime footguns, and version-specific crash or workaround guidance must be verified against current primary web sources before being treated as current fact.
- Use web search or web fetch to verify current external facts, versions, pricing, deadlines, regulations, or platform behavior before final answers.
- Prefer primary sources; report source links and dates for volatile information.
- If web access is unavailable, state the limitation and mark guidance as unverified.

## Learnings Loop

Before applying this skill on a non-trivial task, read `learnings.consolidated.md` in this directory (and `learnings.md` if present).

After applying it, if you encountered a pattern worth remembering, a mistake worth preventing, or a domain fact that surprised you, append one dated bullet to `learnings.md` via `agents-skills-feedback-loop/scripts/append_learning.py`. Do not modify `SKILL.md` itself.