Messy-repo refactors usually fail on size, not on design.
A forty-file diff cannot be reviewed, bisected, or reverted cheaply. Reviewers skim it. Behavior changes hide inside moves. So declare three numbers before you write code, then let a script enforce them.
This post is about the second half of a messy-repo refactor: after the characterization tests exist, what is the smallest change that actually removes the pain? The artifact is a budget checker you can drop into CI today.
The failure mode is a mixed diff
A refactor commit does two jobs at once: it moves code and it changes behavior. That combination is unreviewable at scale. No reviewer can tell whether a deleted line was redundant or load-bearing.
Size compounds the problem. Revert cost grows with files touched, not with lines changed. A 3-file revert is a git revert. A 30-file revert is an incident.
So the budget is a review contract, not a style preference.
Step 1: Declare the budget before the first edit
Write the budget in the commit message or the PR description. Pick one tier and stay in it.
| Tier | Files touched | Net lines | Protected paths edited | Revert plan |
|---|---|---|---|---|
| Seam only | ≤ 4 | ≤ 80 | 0 |
git revert one commit |
| Move only | ≤ 8 | no cap on moves | 0 | revert two commits |
| Behavior change | ≤ 3 | ≤ 40 | 0 | feature flag off |
Three rules make this work.
- Protected paths are always zero. Test files, migrations, and generated API docs are read-only during a refactor. If the change requires editing a test, the test was wrong; that is a separate commit with its own budget.
- Moves are not free. A pure move shows up as large additions and large deletions. That is intentional: it forces the move into its own commit.
- One tier per commit. A seam commit never carries a behavior change, even a trivial one.
Step 2: Census the call sites before choosing a shape
The shape of the change should follow the call-site count, not your taste. Count first.
# every call site of the function you plan to move or wrap
git grep -n --heading -E '\bcompute_total\s*\(' -- '*.py' ':!tests/*'
Add an import census too, because import edits are the hidden cost of moving a function.
# who imports the module you want to split
git grep -n -E '^\s*(from|import)\s+billing\.legacy\b' -- '*.py'
If the second command returns files you did not expect, you are not doing a local refactor. You are doing a package move, and that is a different tier.
Step 3: Pick the smallest shape that removes the pain
The pain is usually one concrete thing: the function reads a global, the module has 900 lines, or the import graph has a cycle. Match the shape to the pain.
| Shape | Call sites edited | Public surface | Budget cost | Use when |
|---|---|---|---|---|
| Add a defaulted parameter | 0 | 0 | Low | You need to inject a source and keep every caller working |
| Wrapper + delegate | 0 | +1 symbol | Medium | You need a new name without moving logic |
| Move + re-export | 1 per import | 0 | Medium | The file size is the pain, not the logic |
| Split into a class | Many | High | Reject for now | The pain is genuinely structural |
Most "we must extract a service" refactors are the first row wearing a costume.
# before: reads two globals, untestable in isolation
def compute_total(order_id):
rate = TAX_RATE # module global
order = db.fetch(order_id) # module-level connection
return sum(i.price for i in order.items) * (1 + rate)
# after: one defaulted parameter, zero call-site edits
def compute_total(order_id, fetch=None, rate=TAX_RATE):
fetch = fetch or db.fetch
order = fetch(order_id)
return sum(i.price for i in order.items) * (1 + rate)
The diff is six lines. The behavior is identical for every existing caller. The seam now exists and the tests can use it.
Step 4: Treat the characterization test as a fence, not a proof
One test at the new seam is enough for a seam commit. It answers a narrower question than people assume: did this specific change alter observable output?
A green test proves nothing about paths you never captured. So keep the fence narrow and the commit small. Small commits mean the fence only has to cover the surface you just touched.
If the seam changes output on any captured case, stop. That is a behavior change, and behavior changes get their own tier.
Step 5: Enforce the budget mechanically
Budgets that live only in a PR template get ignored. Put the numbers in a file and fail the build.
#!/usr/bin/env python3
"""Fail if a refactor diff exceeds its declared budget."""
import fnmatch
import json
import subprocess
import sys
BUDGET = {"files": 4, "net_lines": 80}
PROTECTED = ["tests/*", "*/migrations/*", "docs/public_api/*"]
def numstat(base):
out = subprocess.run(
["git", "diff", "--numstat", base],
capture_output=True, text=True, check=True,
).stdout
rows = []
for line in out.splitlines():
added, removed, path = line.split("\t", 2)
if added == "-" or removed == "-": # binary file
rows.append((path, 0, 0))
continue
rows.append((path, int(added), int(removed)))
return rows
def main(base):
rows = numstat(base)
files = len(rows)
net = sum(a - r for _, a, r in rows)
protected = [
p for p, _, _ in rows
if any(fnmatch.fnmatch(p, pat) for pat in PROTECTED)
]
print(json.dumps(
{"base": base, "files": files, "net_lines": net,
"protected": protected}, indent=2))
fails = []
if files > BUDGET["files"]:
fails.append(f"files {files} > {BUDGET['files']}")
if net > BUDGET["net_lines"]:
fails.append(f"net lines {net} > {BUDGET['net_lines']}")
if protected:
fails.append(f"protected paths touched: {protected}")
if fails:
print("BUDGET EXCEEDED:\n " + "\n ".join(fails), file=sys.stderr)
sys.exit(1)
print("budget ok")
if __name__ == "__main__":
main(sys.argv[1] if len(sys.argv) > 1 else "origin/main")
Run it against your merge base. Locally, for staged work:
python3 diff_budget.py origin/main
And in CI, as a required check on refactor branches:
- name: refactor diff budget
run: python3 tools/diff_budget.py origin/main
if: contains(github.event.pull_request.labels.*.name, 'refactor')
Illustrative output shape when the budget is blown:
{
"base": "origin/main",
"files": 7,
"net_lines": 214,
"protected": ["tests/test_billing.py"]
}
BUDGET EXCEEDED:
files 7 > 4
net lines 214 > 80
protected paths touched: ['tests/test_billing.py']
The protected-path check is the one that earns its keep. It catches the common shortcut where a test is edited to make a refactor pass.
Where a free model and a free server genuinely help
Two steps above are mechanical and low-risk to draft with a free model: the call-site census and the seam patch itself. Ask for a patch that edits zero call sites, then verify with git grep. I would not accept a model's call-site list without that grep, because it cannot see your runtime imports.
Running the budget script and the test suite is also a reasonable fit for a free server option. That keeps the refactor tooling out of your local environment and out of production CI while you tune the numbers. MonkeyCode's free model access and free server option are operator-supplied availability claims; quote no quotas and assume nothing about permanence — treat both as scratch capacity you can lose tomorrow.
Disclosure: This article was prepared as part of MonkeyCode's product outreach.
If you want to try the budget script without installing anything locally, the free server option is a low-commitment place to run it once.
Limitations and who should not use this
- The script counts lines, not risk. A 3-file diff to an auth module can be riskier than a 20-file rename.
-
--numstatcollapses file modes and renames, so a rename-heavy commit looks smaller than it is. - The budget is a per-repo constant here. Mature repos with generated code need path-specific budgets.
- Binary files report
-and are counted as zero lines. That undercounts real diffs.
Skip this approach if your change is genuinely architectural. A migration off a shared global cannot be hidden in an 80-line diff. In that case, use the budget to force a sequence of small commits, not to force one small commit.
Also skip it if you have no characterization tests at all. Adding the checker without the fence just makes small, unverified changes faster to ship.
The rule is simple: pick the tier first, then write the smallest change that stays inside it.
Top comments (1)
This framing helps a lot — especially treating the budget as a review contract rather than a style preference. The part that resonated most was protected paths always being zero. In practice that's where messy refactors quietly become behavior changes, when a test gets "tidied" alongside the code it's supposed to guard. I also like the call-site census before choosing a shape. Counting first has saved me from splitting a module that only had two real callers. One question: do you adjust the tiers for a repo with a lot of generated code, or keep the same budget and carve out generated paths separately?