Playbook: Legacy Refactor
Objective
Change the structure of legacy code while preserving behavior you care about — proven by characterization tests written before the refactor — without letting the agent "improve" undocumented behavior into something else.
When to use
You need to reshape a module/package that lacks trustworthy tests or clear specs. Not for greenfield features (Add a Feature) or for "fix the bug" without structural change (Production Debugging). If the refactor spans many packages, decompose with Large Tasks.
Inputs required
- Target path(s) and the reason for the refactor (seams for a feature, perf, delete dead branch).
- Behaviors that must not change (list them; "everything" is not a list).
- Behaviors you are willing to change (explicit — or the agent will pick).
- A rollback plan (git revert granularity).
Step-by-step process
1. Inventory — no edits yet
Explore <path> for a refactor toward <goal>.
Deliver only:
1. Entry points and call graph summary (key files)
2. Hidden dependencies (globals, DB, clock, feature flags)
3. Suspected duplicated logic
4. Recommended seam(s) for incremental change
5. Open questions — especially undocumented behavior
Do not edit code. Do not propose a full rewrite unless seams are impossible;
if you recommend rewrite, justify with evidence.You decide rewrite vs incremental. Agents bias toward greenfield rewrites that drop edge cases.
2. Characterization tests first
Lock current behavior — including quirks you are keeping:
Add characterization tests for <module/path>.
Rules:
- Tests document CURRENT behavior, including oddities noted in <list>.
- Prefer exercising real boundaries over mock-only setups.
- No production code changes except trivial hooks required for testability
(list them; stop for approval if non-trivial).
- Name tests after observed behavior, not aspirational behavior.
When done: paste test command output. Summarize behaviors locked.Reject mock-only tests that assert the mock was called. See Reviewing AI Tests and Testing AI Code.
3. Human gate: which quirks survive
Edit a short specs/<nnn>-characterize-<module>.md or ADR note:
| Behavior | Keep | Change | Notes |
|---|---|---|---|
Empty list returns 200 [] | ✓ | Clients depend on it | |
Invalid id returns 500 | ✓ | Will become 404 in a later PR — out of scope now |
Out-of-scope behavior changes belong in a separate feature PR. Mixing "fix while refactoring" is how characterization suites get rewritten to match new bugs.
4. Plan the structural change
Plan refactor of <path> toward <goal>.
Constraints:
- Characterization suite must remain green every increment
- File allowlist: <paths>
- Do NOT change HTTP/JSON contracts except as listed in <keep/change table>
- No new dependencies without asking
- No parallel v2 package — transform in place or extract behind a seam
Plan mode. Increments ≤ half-day each. Verification = characterization command.5. Implement incrementally
Execute increment <k> only. Run <characterization command> after.
If red: fix or revert the increment — do not weaken tests to match new code.
Paste output. Stop.If the agent wants to edit characterization expectations, that is a behavior change — stop and treat it as a product decision.
6. Optional: migrate tests toward intent
Only after the structure is stable: replace some characterization cases with intent-based tests that match an updated spec. Never delete characterization coverage until the new tests actually lock the same risk.
Human review points
- Inventory conclusions — rewrite vs seam is yours.
- Characterization suite — you agree it locks the right behaviors.
- Keep/change table — explicit.
- Each increment — suite green; spot-check one critical path.
- Final PR — no contract drift; no
utils2/ parallel packages (Navigability).
Expected artifacts
Characterization tests (green before refactor) · keep/change table · incremental PRs or commits · final structure matching the goal · optional ADR if a lasting pattern changed (Decision Records).
Common failures
| Symptom | Root cause | Early detection | Fix |
|---|---|---|---|
| Refactor PR rewrites tests to pass | Behavior changed silently | Diff in *.test.* expectation values | Revert test changes; restore characterization; re-plan |
| Big-bang rewrite, edge cases missing | Agent avoided reading legacy paths | File delete count; missing branches | Abort; characterization first; incremental extract |
| Mock-only suite, prod breaks | False confidence | Review test style | Add boundary tests before more structure change |
module-v2/ beside module/ | Unconstrained plan | New top-level folder | Reject; in-place or strangler with explicit cutover |
| "Cleanup" across unrelated packages | No allowlist | Diff scope | Revert; tighten allowlist |
Recovery strategy
Red characterization after an increment: revert that increment, context reset, smaller seam. If tests were edited to match bad code: restore tests from main, then Recovery. If you discover the keep/change table was wrong: stop refactor, ship a deliberate behavior-change PR with spec, then resume.
Acceptance criteria
- [ ] Characterization suite existed and was green before structural edits
- [ ] Keep/change table agreed; no undeclared contract changes
- [ ] Each merged increment kept characterization green without weakening assertions
- [ ] No parallel v2 package; navigability preserved
- [ ] Critical paths human spot-checked
- [ ] Follow-up behavior changes tracked as separate specs/PRs
Related
- Testing AI Code — how to demand real tests from agents.
- Reviewing AI Tests — catching mock-only and tautological tests.
- Architecture Erosion — what unconstrained refactors do.
- Migration — when the change is a platform/stack move.
- Large Tasks — multi-package refactors.
- Recovery — when an increment goes badly.