Daniel Rodriguez

Refactor legacy code behind a characterization test gate

Make an AI or coding agent pin current behavior with tests, refactor in small steps, and never edit a test to make it pass, with preserved bugs listed.

At a glance

Best for: Developers asking an agent to clean up tangled code where any behavior change, even an improvement, would be a bug.
Tested on: Claude · Opus 5.5
You fill in: targetgoalcode
You get: Characterization tests (pass on current code) Invariant Same return value for every case above, and a missing cart must still not throw for… (full result below)

Prompt

Refactor {{target}} behind a test gate.

Goal of the refactor: {{goal}}

Code:
{{code}}

Rules:

1. First write characterization tests that pass against the CURRENT code. Include every boundary and any input that looks odd. If existing tests already cover a behavior, name them instead.

1. State the invariant: the list of behaviors that must be identical before and after, including errors thrown.

1. Refactor in small steps. After each step, name the tests to re-run. Never change a test and the code in the same step.

1. If a test would fail after a step, revert that step and explain. Do not edit the test to make it pass.

1. Behavior changes are out of scope, even improvements. If you spot a bug, preserve it and list it at the end.

Finish with: the final code, a one-paragraph before/after summary, and the preserved bugs.

Fill in

{{target}}: The function, class or module to refactore.g. getDiscount() in src/pricing/discount.js
{{goal}}: What should be better afterwardse.g. Replace nested ifs with a lookup table and small pure functions
{{code}}: The current codee.g. The 17-line getDiscount function

Example input

target: getDiscount() in src/pricing/discount.js (Jest)
goal: Replace the nested ifs with a lookup table and small pure functions so new tiers are a one-line change.
code:
```js
function getDiscount(user, cart) {
  let d = 0
  if (user) {
    if (user.isEmployee) {
      d = 30
    } else {
      if (user.tier === "gold") {
        d = cart.total > 5000 ? 15 : 10
      } else if (user.tier === "silver") {
        d = cart.total > 5000 ? 8 : 5
      } else {
        if (cart.total > 5000) d = 3
      }
    }
    if (user.firstOrder && d < 10) d = 10
  }
  return d
}
```

2620

Generated Outputs (1)

3 hours ago

Claude

Opus 5.5

3 hours ago

Generated Output

1. Characterization tests (pass on current code)
`js
const cases = [
["no user", null, 9999, 0],
["employee", { isEmployee: true }, 100, 30],
["employee, first order", { isEmployee: true, firstOrder: true }, 100, 30],
["gold at 5000 (not above)", { tier: "gold" }, 5000, 10],
["gold above 5000", { tier: "gold" }, 5001, 15],
["silver above 5000", { tier: "silver" }, 6000, 8],
["silver, first order", { tier: "silver", firstOrder: true }, 100, 10],
["no tier above 5000", {}, 6000, 3],
["no tier, small cart", {}, 100, 0],
["unknown tier 'platinum'", { tier: "platinum" }, 6000, 3],
["tier 'constructor'", { tier: "constructor" }, 6000, 3],
]
test.each(cases)("%s", (_, user, total, expected) => {
expect(getDiscount(user, { total })).toBe(expected)
})
test("employee never reads cart", () => {
expect(getDiscount({ isEmployee: true }, undefined)).toBe(30)
})
`

2. Invariant
Same return value for every case above, and a missing cart must still not throw for employees or for user = null.

3. Steps

1. Early return for !user. Re-run all tests.

1. Extract baseDiscount(user, cart). Re-run all tests.

1. Replace the tier ifs with a lookup. My first draft used a plain object, TIER_RATES[user.tier]. The 'constructor' test would fail, because the lookup returns Object itself. Reverted, and used a Map.

1. Replace if (firstOrder && d < 10) with Math.max. Re-run all tests.

A draft that read cart.total before the employee check would break the "employee never reads cart" test, so the total is read inside baseDiscount, after that check.

Final code
`js
const BIG_CART = 5000
const TIER_RATES = new Map([
["gold", { small: 10, big: 15 }],
["silver", { small: 5, big: 8 }],
])
const NO_TIER = { small: 0, big: 3 }

function baseDiscount(user, cart) {
if (user.isEmployee) return 30
const rates = TIER_RATES.get(user.tier) ?? NO_TIER
return cart.total > BIG_CART ? rates.big : rates.small
}

function getDiscount(user, cart) {
if (!user) return 0
const d = baseDiscount(user, cart)
return user.firstOrder ? Math.max(d, 10) : d
}
`

Summary
Behavior is identical on all 12 tests. Adding a tier is now one Map entry, and the first-order rule is one line instead of being tangled with the tiers.

Preserved bugs, for a human to decide

• An unknown tier such as "platinum" silently gets the no-tier rate.

• A cart of exactly 5,000 doesn't count as big (> rather than >=).

• A non-employee with no cart throws a TypeError.

Why it works

"Never change a test and the code in the same step" and "do not edit the test to make it pass" close the most common way agents fake a green run. The instruction to include inputs that look odd is what produced the 'constructor' test, which caught a real prototype-lookup bug in the obvious refactor. Stating the invariant, including errors thrown, protected a behavior nobody would have thought to keep (employees never touch cart). Listing preserved bugs separates cleanup from fixes, so each can be reviewed on its own.

When not to use it

It's too slow for code with no users yet, or code you're about to rewrite anyway. It works best on pure logic. For code tangled with I/O, you'll need integration tests or recorded fixtures first, and an agent that can actually run them. In a chat window, "tests pass" is the model's reasoning. Run the suite yourself before merging.

Comments (0)

No comments yet. Be the first to share your thoughts!