Safe Refactoring skill for Claude Code

A refactor is safe when behavior is provably unchanged: tests that pinned the behavior before still pass after, every intermediate step was green, and the diff can be reviewed as a sequence of mechanical moves. If behavior changes, it is not a refactor; split it out.

Ground rules

• Never mix refactoring and behavior change in the same commit. Refactor first, commit, then change behavior.
• No refactor without a safety net. If the code is not covered, write characterization tests first (step 2).
• Take steps small enough that a failing test points at one change. Run tests after every step; if red, revert the step rather than debugging forward.
• Prefer tool-driven edits (IDE rename, ast-grep, codemods, compiler-guided changes) over hand edits for anything that touches more than a few call sites.
• Keep public interfaces stable, or change them with parallel change (expand, migrate, contract) so callers never break.
• Stop when the goal is met. A refactor serves a purpose (enable a feature, fix a smell that slows work); “cleaner” without a target becomes endless.

Workflow

- [ ] 1. State the goal and the boundary of the refactor
- [ ] 2. Build the safety net (existing tests + characterization tests)
- [ ] 3. Plan a sequence of named refactoring steps
- [ ] 4. Execute step by step: edit, run tests, commit
- [ ] 5. Verify behavior is unchanged
- [ ] 6. Report the sequence and any found bugs (not fixed inline)

1. Goal and boundary

Write one sentence: “Extract pricing rules from OrderService so the new subscription flow can reuse them.” List the files in scope and what is explicitly out of scope. Check how widely the code is used before touching it:

grep -rn "calculateTotal" --include=*.ts src | wc -l
git log --oneline -10 -- src/orders/OrderService.ts    # recent churn; coordinate if active

2. Safety net

1. Run the existing tests for the area and record the pass count.
2. Measure coverage on the files in scope:

npx vitest run --coverage --coverage.include='src/orders/**'
pytest --cov=app/orders --cov-report=term-missing tests/orders
go test ./orders -coverprofile=c.out && go tool cover -func=c.out

3. For uncovered behavior you are about to move, write characterization tests: call the code with representative inputs and assert whatever it returns today, even if it looks wrong. Name them characterizes ... and note suspected bugs instead of fixing.
4. For large outputs (reports, rendered HTML, serialized payloads) use a golden-master or snapshot test over a set of varied inputs.
5. Mutation check the net: temporarily break a line in the code to be moved and confirm at least one test fails.

If the code is too tangled to test at all, switch to legacy-code-rescue to create seams first.

3. Plan the steps

Name each step with a standard refactoring so it is mechanical and reviewable:

Smell	Refactoring sequence
Long function	Extract Function per paragraph; then Rename; then Move to the right module
Large class / god object	Extract Class around a cohesive field group; delegate; move callers
Duplicated logic	Make the copies identical (small edits), then Extract Function, then replace each copy
Long parameter list	Introduce Parameter Object; then move behavior onto it
Conditional on type codes	Replace Conditional with Polymorphism, or a lookup table for pure data
Feature envy	Move Function to the data it uses
Primitive obsession (money, IDs as strings)	Introduce Value Object at the boundary; migrate callers gradually
Deep nesting	Replace Nested Conditional with Guard Clauses
Public API rename	Parallel change: add new name, delegate old to new, migrate callers, deprecate, remove
Module reorganization	Move files with re-exports from the old path, migrate imports, remove re-exports

4. Execute

The loop for every step:

edit (one named refactoring) -> compile/typecheck -> run focused tests -> commit

npx tsc --noEmit && npx vitest run src/orders
git commit -am "refactor(orders): extract applyDiscounts from calculateTotal"

Mechanical tools, preferred over hand edits:

• IDE / language server: rename symbol, extract function/variable, move file with import updates. These are scope-aware; text replace is not.
• ast-grep for structural search and rewrite across many files:

ast-grep run --pattern 'moment($A).format($B)' --lang ts            # find
ast-grep run --pattern 'oldFn($$$ARGS)' --rewrite 'newFn($$$ARGS)' --lang ts --update-all

• Codemods: jscodeshift (JS/TS), LibCST (Python), gofmt -r / gopls rename (Go), OpenRewrite (Java). Commit the codemod script or command in the commit message.
• Compiler-guided: change a type or signature, then fix each compile error. Only safe in statically typed code with no any/reflection paths.

Parallel change for interfaces with external callers:

/** @deprecated use calculateOrderTotal; removal tracked in #812 */
export function calculateTotal(order: Order): Money {
  return calculateOrderTotal(order);
}

Ship expand and migrate; contract (remove the old name) only after all callers, including other repos and services, are migrated.

5. Verify unchanged behavior

• Full test suite, lint, and type check green; pass count equals the baseline plus the characterization tests you added.
• Review the diff with moved code de-emphasized:

git diff --color-moved=zebra --color-moved-ws=allow-indentation-change origin/main...HEAD
git diff --stat origin/main...HEAD

• For performance-sensitive code, compare a benchmark before and after; extracting functions or adding layers in a hot loop can regress.
• For serialized output (APIs, files, events), compare real outputs before/after on the same inputs (golden master).

6. Report

**Goal:** Pricing rules reusable outside OrderService.
**Safety net:** 18 existing tests + 9 characterization tests (pricing edge cases);
coverage of src/orders/pricing 94% lines.
**Steps (each its own green commit):**
1. Extract Function `applyDiscounts` from `calculateTotal`
2. Introduce Parameter Object `PricingContext`
3. Move Function `applyDiscounts` -> `src/pricing/discounts.ts`
4. Parallel change: `calculateTotal` delegates to `calculateOrderTotal` (deprecated)
**Behavior change:** none. Suspected bug pinned by characterization test
`characterizes negative quantity` (returns negative total) — logged as #904, not fixed.
**Checks:** 312 tests pass (baseline 303 + 9 new); tsc clean.

Gotchas

• Text search-and-replace renames hit comments, strings, unrelated symbols with the same name, and miss dynamic access (obj[name], reflection, templates, DI by string). Grep for the string form after any rename.
• Moving files breaks imports outside the compiler’s view: Jest/Vitest moduleNameMapper, tsconfig paths, dynamic import(), lazy routes, Storybook globs, Dockerfile COPY paths, CODEOWNERS. Grep for the old path.
• Extracting code changes evaluation order or this binding in JS; extracting a method into a standalone function can drop this.
• Python default arguments and closures evaluate at definition time; moving them changes when.
• Reformatting in the same commit as a refactor hides the real change; format in a separate commit or not at all.
• Characterization tests that assert on bugs must be labeled; otherwise the next person “fixes” the test.
• Long-lived refactor branches rot against main. Merge small steps continuously, behind parallel change where needed.

Verification

• Every commit in the series passes tests: git rebase -i --exec "<test cmd>" origin/main.
• A deliberately injected bug in moved code is caught by the safety net.
• Diff review shows only moves, renames, and extractions; no logic edits hidden inside.