Refactoring playbook

Refactoring Playbook

Purpose

A refactor is a change that must prove a negative: that nothing observable changed. This skill is the strategy for that proof — characterization tests before touching anything, a catalog of behavior-preserving transformations applied one at a time, strangler-fig migration for anything too big to swap atomically, and the discipline to know when NOT to refactor at all.

Metadata

• Prerequisites: change-control (governs every diff this skill produces); validation-and-testing (test mechanics for the characterization step); codebase-onboarding if the code is unfamiliar.
• Related Skills: architecture-analysis (whether the structure should change), engineering-minimalism (whether the code should exist at all — deletion beats refactoring), api-and-interface-design (when the refactor touches a public surface), campaign-planning (multi-week migrations).
• Owns: behavior-preserving transformation strategy; characterization-test usage; strangler-fig migration; the when-NOT-to-refactor decision.

When to Use / When NOT to Use

Use when:

• Restructuring code whose external behavior must stay identical: extract/inline/rename/move/split/merge.
• Migrating from one implementation to another (library swap, pattern change, module reorganization).
• A feature change requires reshaping the code first (refactor first, in its own commit; then the feature).
• Deciding whether a proposed "cleanup" is worth its risk.

Do NOT use (load the sibling instead):

• The behavior is WRONG and must change → that is a bug fix: debugging-playbook then change-control. Never mix a fix into a refactor.
• The question is whether the code should exist → engineering-minimalism; deleting is better than restructuring.
• The refactor changes a public API consumers depend on → that is API evolution, not refactoring: api-and-interface-design step 7.
• General diff hygiene, commits, irreversible commands → change-control (always co-loaded).

Definitions & Mental Model

• Refactoring: changing internal structure with zero observable behavior change. If any consumer could notice, it is not a refactor — it is a change, and different rules apply.
• Characterization test: a test that records what the code currently does (including its quirks), written before restructuring, so any behavior drift trips it. It asserts what IS, not what SHOULD be.
• Behavior-preserving transformation: a mechanical restructuring whose correctness is arguable in isolation — extract function, inline function, rename symbol, move declaration, replace duplication with a call.
• Strangler fig: migrating a large component by growing the replacement alongside the old one and shifting consumers over incrementally, until the old one is dead code and can be deleted.
• Seam: a place where behavior can be substituted without editing callers (concept owned by architecture-analysis).

Mental model: a refactor is a proof obligation, discharged in small steps. You cannot prove "nothing changed" for a 40-file rewrite; you can prove it for one rename, one extraction, one moved function — because each step is small enough to argue, and the characterization tests re-check the whole claim after every step. The professional instinct is therefore not bravery but cowardice with a plan: never be more than one small, reversible step away from green. And before any of it, the cheapest transformation is deletion — code that shouldn't exist doesn't need restructuring.

Core Methodology

1. Establish why, in one sentence with a beneficiary. "Extract the parser so the new format lands as one module" is a reason. "This code is ugly" is not — aesthetics without a beneficiary loses to the risk (engineering-minimalism). If the honest reason is "I would have written it differently," stop.
2. Run the when-NOT-to-refactor gate (decision tree):

Should this refactor happen now?
├─ Is the code about to be deleted or replaced? → NO. Don't polish
│    the condemned. (Check roadmap/owners first.)
├─ Is behavior currently WRONG? → NO. Fix the bug first (its own
│    commit), THEN refactor. Never both at once.
├─ Are there tests covering the affected behavior?
│    ├─ NO → Can you add characterization tests cheaply (step 3)?
│    │    ├─ YES → add them, then proceed.
│    │    └─ NO (untestable without restructuring, no time budget)
│    │         → NO, unless the refactor is trivial-and-mechanical
│    │           (pure rename via tooling). Record the debt instead.
├─ Is ownership unclear / is someone else mid-change in this area?
│    → NOT YET. Coordinate first; refactor collisions are expensive.
└─ Otherwise → YES. Proceed with steps 3-8.

3. Write characterization tests before touching anything. Cover the observable behavior of the code you will restructure: normal inputs, the awkward inputs, and the current error behavior — as it is, quirks included. If the current behavior is surprising, record the surprise in the test with a comment; changing it is a separate, later decision. (Test mechanics — monkeypatch, tmp_path, fakes: validation-and-testing.) Run them; they must be green before you start.
4. Transform in small, individually-arguable steps. The core catalog:
   • Extract function/module (name a concept that was inline)
   • Inline (remove an indirection that earns nothing)
   • Rename (make the name true — use tooling/IDE where available; verify with grep that ALL references moved, including strings, docs, and config: change-control's enumerate-every-caller rule)
   • Move (relocate to where the concept belongs)
   • Replace duplication with a call (after the 2nd–3rd real duplication — engineering-minimalism)
   • Introduce seam (parameter/interface so behavior can be substituted) After EACH step: run the characterization tests. Green → next step. Red → the step changed behavior; revert the step (not the session) and re-approach.
5. Keep refactor commits pure. One refactor (or a few same-kind mechanical steps) per commit, message prefixed refactor:, containing zero behavior changes. Reviewers verify a refactor commit by checking behavior-preservation; mixing in a fix or feature destroys that verification (change-control owns commit discipline; engineering-minimalism §2 sends deletions to their own commit for the same reason).
6. Use strangler fig for anything too big to swap in one sitting:
   1. Build the replacement alongside the old implementation, behind the same interface/seam.
   2. Route a small, low-risk slice of consumers/inputs to the new path; compare outputs (in tests, or shadow-run both and diff).
   3. Expand routing slice by slice, each slice verified, each leaving the system working.
   4. When nothing routes to the old path, delete it in the same campaign — a strangler fig that never strangles is permanent double maintenance. Plan the deletion date at step 1. (Multi-week executions: campaign-planning.)
7. Plan the retreat before the advance. Refactors are the easiest changes to abandon safely — insist on it: work on a branch for anything multi-file; know the revert point; if the refactor stalls half-done, revert rather than leaving the codebase in a two-idioms state (half-refactored is worse than unrefactored).
8. Finish by re-verifying the claim. All characterization tests green, the full suite green, and — for user-facing paths — one manual smoke of the real flow. Then, if the behavior should also change (the bug you noticed, the quirk you recorded), do that now as a separate, ordinary change with its own test.

Refactoring checklist

• One-sentence reason with a beneficiary; "I'd have written it differently" rejected
• When-NOT gate run (condemned code? live bug? no tests? ownership conflict?)
• Characterization tests written, quirks included, green BEFORE the first edit
• Each transformation small enough to argue in isolation; tests run after each
• Renames verified with grep across code, strings, docs, config
• Commits pure: refactor: prefix, zero behavior change, no smuggled fixes
• Strangler fig migrations have a routing plan AND a deletion date
• Retreat planned: branch, revert point, no half-done end state
• Full suite + smoke green at the end; behavior changes deferred to their own change

Discovery & Audit Commands

# Is the target covered by tests? (the gate's key question)
grep -rln "target_function\|TargetClass" tests/ test/ 2>/dev/null
pytest -k "target" --collect-only -q 2>/dev/null | head    # which tests touch it

# Enumerate every reference before extract/rename/move (nothing may be missed)
grep -rn "target_function" --include="*.py" --include="*.ts" . | grep -v node_modules
grep -rn "target_function" --include="*.md" --include="*.yaml" --include="*.json" . | grep -v node_modules   # docs/config references too

# Who else is mid-change here? (collision check)
git log --oneline -10 -- path/to/area
git branch -a --contains $(git rev-parse HEAD) 2>/dev/null | head   # requires verification per hosting setup

# Duplication worth consolidating? (evidence before abstraction)
grep -rn "the_duplicated_snippet" --include="*.py" . | grep -v node_modules | wc -l

# After each step: the proof
pytest tests/ -x            # or: npm test / cargo test — the repo's suite
git diff --stat             # the step stayed small

Failure Modes & Anti-patterns

Symptom	Mistake	Correction
"Refactor" PR that also fixes two bugs	Behavior change smuggled into restructuring	Fix first or after, in its own commit; refactor commits are pure (step 5)
Restructured code, subtle behavior drift shipped	No characterization tests	Record current behavior (quirks included) BEFORE editing (step 3)
Big-bang rewrite stalls at 60%, both idioms live forever	Too-large atomic swap, no retreat plan	Strangler fig with routing slices + deletion date; revert beats half-done (steps 6-7)
Rename missed a string reference; runtime breaks	Grep skipped; only code references updated	Enumerate ALL references: code, strings, docs, config (step 4)
Week spent beautifying code deleted next sprint	When-NOT gate skipped	Check the roadmap; don't polish the condemned (step 2)
Characterization test "fixed" to match new behavior	Test treated as wrong when it tripped	A tripped characterization test means behavior CHANGED — revert the step, decide separately (step 4)
Abstraction introduced for one caller	Refactoring toward speculative generality	Duplication count ≥2-3 before consolidating (engineering-minimalism)
Old implementation never deleted after migration	Strangler fig without the strangling	Deletion date planned at migration start; double maintenance is the cost meter (step 6)
Refactor collides with teammate's in-flight feature	Ownership/coordination skipped	Collision check in the gate; sequence with the other change (step 2)
40-file "cleanup" commit nobody can review	Steps not individually arguable	Small steps, tests between, one kind of change per commit (steps 4-5)

Worked Example

Task: a 300-line process_order() function mixes validation, pricing, and notification; a new pricing rule is coming and nobody can find where to put it.

1. Reason: "Extract pricing so next week's rule lands as one function change." Beneficiary: the feature author.
2. Gate: code is live and staying; behavior believed correct; tests exist for the happy path only; no one else mid-change. → Proceed, but characterize first.
3. Characterization: six tests written against process_order() as-is: normal order, zero-quantity line (currently silently dropped — a quirk, recorded with a comment), discount stacking (current output captured, even though it looks odd), invalid address error text, notification payload shape, empty order. All green.
4. Steps: (a) extract validate_order() → tests green; (b) extract price_order() → one test red — the extraction reordered a rounding step; revert (b), re-extract keeping the original operation order → green; (c) extract notify() → green; (d) rename ambiguous data → priced_order with grep over code and the one YAML template referencing the field → green.
5. Commits: four commits, each refactor: extract ..., zero behavior change; the diff of each is reviewable in one screen.
6. Aftermath: the discount-stacking quirk recorded in step 3 is raised as a separate question to the owner — if it is a bug, it gets fixed as a bug, with its own test, next.

The new pricing rule ships the following week as a 12-line change inside price_order().

Repository Examples

Repo facts below are point-in-time illustrations (as of 2026-07-04) — examples, never assumptions about your system.

• ragit (~/ragit) — replacement over patching at the dependency seam: commit fc3f8a43 "Switch to pypdf and fix README setup/details" swapped the PDF library behind the loader seam rather than accumulating workarounds — a small strangler-shaped move where the interface (load a PDF, get text) stayed fixed while the implementation changed.
• orphy (~/orphy) — seams that make future refactors cheap: phases joined by JSON contracts and a swappable DeliveryChannel mean implementations can be restructured behind the contract with zero downstream edits — the precondition this skill's step 6 relies on, designed in early.
• prism (~/prism) — refactor-enabling test placement: tests/test_security.py characterizes the highest-risk observable behaviors (signature rejection, size limits, repo-name validation), which is exactly the safety net a future restructuring of server.py/config.py would run against.

Validation Exercise (any repository): pick a 50+ line function with tests; write two characterization tests for its quirkiest observable behavior; perform one extract-function step; run the tests. If you cannot write the characterization tests, you have discovered the real blocker — route to validation-and-testing's bootstrap procedure.

Validation Criteria

You applied this skill correctly when:

1. Characterization tests existed and were green before the first structural edit — and they still pass, unmodified, at the end.
2. Every commit in the refactor is behavior-pure and individually arguable; a reviewer can verify each in one sitting.
3. git log shows any bug fix or behavior change as a separate commit before/after the refactor, never inside it.
4. For renames/moves, a grep for the old name across code, docs, and config returns zero live references.
5. Any strangler migration has either completed its deletion or has a dated plan to.
6. If the refactor was abandoned, the codebase shows no trace — the retreat was clean.

Provenance & Maintenance

• Sources: ~/ragit, ~/orphy, ~/prism — investigated 2026-07-04. Skill authored 2026-07-06; methodology is repo-independent (characterization testing and strangler fig are industry-canonical practice).
• Assumptions: the ragit pypdf swap is read as a seam-preserving replacement from its commit message and dependency diff — the internal mechanics were not line-verified (Hypothesis on the details, verified on the swap itself).
• Re-verification commands:

  git -C ~/ragit show --stat fc3f8a43
  ls ~/prism/tests
  

• Likely to drift: example repos' structures; the availability of rename tooling per ecosystem.
• Maintenance checklist:
  • Re-run re-verification; re-stamp Repository Examples.
  • If an owner repo performs a real strangler migration, capture it as the primary case study.
  • Confirm cross-referenced skills still exist under their directory names.