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
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.
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:
- Touch function bodies only.
- Change no signatures and no return types.
- Add no new dependencies.
- 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))
Enforce the budget with git:
git diff --stat
git diff -U0 | grep -E '^[-+]' | grep -vE '^[-+]{3}' | wc -l
Block signature creep with a failing grep:
git diff | grep -E '^[-+](async )?def |^[-+]class ' && echo "STOP: API change"
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
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)