DEV Community

Dakota Huang
Dakota Huang

Posted on

A Refactor Is Two Commits: Lock the Behavior, Then Edit the Body

A Refactor Is Two Commits: Lock the Behavior, Then Edit the Body

Every safe refactor is two commits. The first locks current behavior with characterization tests. The second makes the smallest edit that keeps them green. If you cannot separate the two, you are rewriting, not refactoring.

Characterization tests are memory, not truth. They record what the code does today, bugs included. A refactor must preserve that behavior. Even the ugly parts. Especially the ugly parts.

The protocol below is deliberately small. One leaf module. Two commits. One outcome matrix. It works on messy repos because it never asks you to understand the whole mess at once.

Why two commits

One combined commit hides the risk. When the suite fails, you cannot tell which part caused it. You end up bisecting a messy diff.

Two commits make failure readable. Run the suite at the lock commit. Run it after the edit. Compare.

The split also gives a clean revert point. The edit fails? You lose one commit, not the week.

Step 0: Pick a leaf

Start where nothing imports you. A leaf module has a small blast radius. Its public surface is easy to lock. Find leaves with one loop:

for f in $(find src -name '*.py'); do
  mod=$(basename "$f" .py)
  count=$(grep -rEl "import ${mod}|from ${mod} import" src --include='*.py' | wc -l)
  printf "%3d %s\n" "$count" "$f"
done | sort -n
Enter fullscreen mode Exit fullscreen mode

Zero importers means leaf. Pick the leaf with the least confusing code. Ignore the worst file in the repo for now. It will still be there tomorrow.

Step 1: The lock commit

Write characterization tests first. They pin current outputs, not intended ones. Example from a messy pricing.py:

# test_pricing_characterization.py
import pricing

def test_calculate_locks_current_behavior():
    # Locked on 2026-08-28, before any refactor.
    assert pricing.calculate(100, "SAVE10", 2) == 180
    assert pricing.calculate(100, "NOPE", 0) == 0
    assert pricing.calculate(-5, "", 1) == -5  # Suspicious. Lock it anyway.
Enter fullscreen mode Exit fullscreen mode

Drafting these tests is mechanical. I pasted the function and its call sites into MonkeyCode's free model access and asked for test bodies. Disclosure: This article was prepared as part of MonkeyCode's product outreach. Then I read every line. The model suggests. A human decides.

MonkeyCode's free server option ran the same generation remotely. My local environment stayed untouched. No local keys. No local installs. The lock commit must pass before you edit. Commit it. Tag it refactor:lock. That tag is the contract.

Step 2: The edit commit

Now make the smallest safe change. Define "smallest" with four rules:

  1. Touch function bodies only.
  2. Change no signatures and no return types.
  3. Add no new dependencies.
  4. Stay under a 30-line diff budget.

Here is the entire edit for pricing.py:

# before: messy but working
def calculate(subtotal, code, qty):
    if qty == 0:
        return 0
    discount = 0.1 if code == "SAVE10" else 0
    return subtotal * qty * (1 - discount)

# after: same behavior, same signature
DISCOUNTS = {"SAVE10": 0.1}

def calculate(subtotal, code, qty):
    if qty == 0:
        return 0
    return subtotal * qty * (1 - DISCOUNTS.get(code, 0))
Enter fullscreen mode Exit fullscreen mode

Enforce the budget with git:

git diff --stat
git diff -U0 | grep -E '^[-+]' | grep -vE '^[-+]{3}' | wc -l
Enter fullscreen mode Exit fullscreen mode

Block signature creep with a failing grep:

git diff | grep -E '^[-+](async )?def |^[-+]class ' && echo "STOP: API change"
Enter fullscreen mode Exit fullscreen mode

If the smallest change touches a signature, it is not a refactor. It is an API change. Do it in its own commit with call-site updates.

Step 3: The outcome matrix

Run the suite before and after each edit. Classify every test into four transitions:

Transition Meaning Action
pass → pass behavior preserved merge
pass → fail regression revert
fail → pass intentional drift review and document
fail → fail still broken same failure? continue. different? investigate

The matrix replaces "I think this is safe" with a per-test verdict. Script it once:

#!/usr/bin/env bash
# transitions.sh — compare test outcomes across two commits
set -euo pipefail
OLD="${1:-HEAD~1}"
NEW="${2:-HEAD}"

snapshot() { # commit output
  git worktree add -q /tmp/wt "$1"
  ( cd /tmp/wt && pytest -q -rA ) > "$2" 2>&1 || true
  git worktree remove -q --force /tmp/wt
}

snapshot "$OLD" /tmp/old.txt
snapshot "$NEW" /tmp/new.txt

for f in /tmp/old.txt /tmp/new.txt; do
  awk '/ (PASSED|FAILED)$/ {print $1, $NF}' "$f" | sort > "$f.norm"
done

join /tmp/old.txt.norm /tmp/new.txt.norm | while read -r test old new; do
  case "$old $new" in
    "PASSED FAILED") echo "REGRESSION   $test" ;;
    "FAILED PASSED") echo "DRIFT        $test" ;;
    "FAILED FAILED") echo "STILL-BROKEN $test" ;;
  esac
done
Enter fullscreen mode Exit fullscreen mode

Run it as ./transitions.sh HEAD~1 HEAD during the edit. Run it again as ./transitions.sh <lock-sha> HEAD after merging. The second run proves the whole refactor preserved behavior.

REGRESSION blocks the merge. No exceptions.

DRIFT needs a one-line comment explaining the intentional change. STILL-BROKEN is safe only when the failure message is identical. Different traceback, different problem.

Final check before merging the edit commit: zero REGRESSION rows in the matrix. Diff under budget. Lock commit untouched. Then merge.

Limits of this protocol

Characterization tests protect only covered behavior. Coverage gaps stay uncovered. Logic driven by time, randomness, or external services is hard to lock. Freeze those inputs or skip the module.

The matrix is only as honest as the suite. Flaky tests produce phantom transitions. Run the suite twice before trusting a failure.

Who should not use this? Throwaway prototypes. Dead code without callers. Repos where the lock commit is bigger than the rewrite. For those, deletion or a clean rewrite is cheaper. Refactoring is not always the answer.

But for messy modules you must keep, the path is short. One leaf. Two commits. One matrix. Start with the smallest file nobody imports. The rest can wait.

Top comments (0)