Messy kitchen-sink modules break under wholesale AI rewrites. Pin observable output with characterization tests before edits. Then extract one pure function and nothing else today.
This order keeps implicit contracts stable during review. It also keeps the resulting diff cheap to inspect.
The failure mode
Kitchen-sink files mix policy, mutation, and shared pools. Callers depend on side effects nobody documented. A full rewrite drops those implicit contracts without warning.
Fluent refactors often look cleaner in the diff. Tests then fail on order, caps, and in-place mutation. Engineering starts when those outputs are pinned in code.
Public debate still mixes fluent rewrites with verified refactors. Characterization tests are the cheaper gate for messy trees. They record behavior before structure changes at all.
Do not ask a model to “clean utils.py.” That prompt invites a taste rewrite. Taste rewrites reorder mutations that callers already ship.
What this tutorial pins
The sample is a credit allocator with residue. It mutates user dicts in list order. It spends a shared pool with a hidden per-user take.
It applies plan bonuses before the hard cap. Unknown plans receive zero bonus today. Inactive users may be skipped while keys appear.
Do not treat this module as a design target. Treat it as production residue you inherited. The tests below record what the function does now.
1. Snapshot the messy module
Save this file as credits.py without renaming symbols. Do not tidy the magic numbers yet.
DEFAULT_BONUS = {"free": 0, "pro": 50, "team": 120}
CAP = 1000
def allocate(users, pool, plan_overrides=None, skip_inactive=True):
overrides = plan_overrides or {}
leftover = pool
touched = []
for user in users:
if skip_inactive and not user.get("active", True):
user["credit"] = user.get("credit", 0)
continue
plan = user.get("plan") or "free"
bonus = overrides[plan] if plan in overrides else DEFAULT_BONUS.get(plan, 0)
current = user.get("credit", 0)
granted = bonus
if leftover > 0:
take = leftover if leftover < 25 else 25
granted += take
leftover -= take
next_credit = current + granted
if next_credit > CAP:
leftover += next_credit - CAP
next_credit = CAP
user["credit"] = next_credit
user["plan"] = plan
touched.append(user["id"])
return {"leftover": leftover, "touched": touched, "pool_in": pool}
Three hidden rules matter for later extracts. The pool grants at most twenty-five per active user. Cap overflow returns extra credit into leftover.
Inactive rows still receive a credit key. plan is written back only on the active path. Input order is the allocation order for the pool.
2. Write characterization tests first
Do not extract helpers before the suite is green. Pin return values and in-place mutations together.
Save this file as test_credits_char.py next to the module.
import copy
import unittest
from credits import allocate, CAP
def users(*rows):
return [dict(r) for r in rows]
class AllocateCharacterization(unittest.TestCase):
def test_empty_users_returns_full_pool(self):
out = allocate([], 100)
self.assertEqual(out["leftover"], 100)
self.assertEqual(out["touched"], [])
self.assertEqual(out["pool_in"], 100)
def test_inactive_skipped_but_credit_key_set(self):
rows = users({"id": "a", "active": False, "plan": "pro"})
original = copy.deepcopy(rows)
out = allocate(rows, 80)
self.assertEqual(out["touched"], [])
self.assertEqual(out["leftover"], 80)
self.assertEqual(rows[0]["credit"], 0)
self.assertEqual(rows[0]["plan"], original[0]["plan"])
def test_unknown_plan_gets_zero_bonus(self):
rows = users({"id": "b", "plan": "enterprise", "credit": 10})
out = allocate(rows, 0)
self.assertEqual(rows[0]["credit"], 10)
self.assertEqual(rows[0]["plan"], "enterprise")
self.assertEqual(out["leftover"], 0)
def test_pro_bonus_then_pool_take_of_25(self):
rows = users({"id": "c", "plan": "pro", "credit": 0})
out = allocate(rows, 40)
self.assertEqual(rows[0]["credit"], 75)
self.assertEqual(out["leftover"], 15)
self.assertEqual(out["touched"], ["c"])
def test_cap_overflow_returns_to_leftover(self):
rows = users({"id": "d", "plan": "team", "credit": 990})
out = allocate(rows, 50)
self.assertEqual(rows[0]["credit"], CAP)
self.assertEqual(out["leftover"], 160)
def test_override_replaces_default_bonus_only(self):
rows = users({"id": "e", "plan": "free", "credit": 0})
out = allocate(rows, 25, plan_overrides={"free": 7})
self.assertEqual(rows[0]["credit"], 32)
self.assertEqual(out["leftover"], 0)
def test_missing_plan_becomes_free_on_active_path(self):
rows = users({"id": "i", "credit": 0})
out = allocate(rows, 0)
self.assertEqual(rows[0]["plan"], "free")
self.assertEqual(rows[0]["credit"], 0)
self.assertEqual(out["touched"], ["i"])
def test_input_is_mutated_in_list_order(self):
rows = users(
{"id": "f", "plan": "pro", "credit": 0},
{"id": "g", "plan": "pro", "credit": 0},
)
out = allocate(rows, 30)
self.assertEqual(rows[0]["credit"], 75)
self.assertEqual(rows[1]["credit"], 55)
self.assertEqual(out["touched"], ["f", "g"])
self.assertEqual(out["leftover"], 0)
if __name__ == "__main__":
unittest.main()
Run the harness against the untouched module first.
python -m unittest test_credits_char.py -v
Every test must pass on the original file. That green run is the only spec you have. Proposed workflow: commit the tests before any extract.
git add credits.py test_credits_char.py
git commit -m "test: pin allocate mutation and leftover"
3. Record a decision table
Use a table instead of undocumented reviewer memory. Each row freezes one observed rule.
| Input signal | Observed rule | Safe to change now |
|---|---|---|
Empty users
|
Pool returned untouched | No |
| Inactive user | Credit key set, plan unchanged | No |
| Unknown plan | Bonus is 0 | No |
| Pool remainder | At most 25 per active user | No |
| Credit above cap | Overflow added to leftover | No |
plan_overrides |
Replaces default bonus only | No |
| List order | First user takes pool first | No |
Missing plan
|
Active path writes "free"
|
No |
Keep the last column at “No” until tests exist. After the suite stays green, one cell may flip. Flip only the cell you will extract.
Inactive users keep the original plan field unchanged. A tidy rewrite often writes plan before continue. The inactive test exists to catch that move.
4. Make the smallest safe change
Extract only bonus lookup on this pass. Leave mutation, cap math, and pool takes untouched.
Proposal: add _bonus_for as a pure function. Do not change call order around it.
def _bonus_for(plan, overrides):
if plan in overrides:
return overrides[plan]
return DEFAULT_BONUS.get(plan, 0)
Then replace the inline bonus line with one call.
bonus = _bonus_for(plan, overrides)
Re-run the same characterization file after that single edit. Do not add features and do not rename allocate.
python -m unittest test_credits_char.py -v
git diff -- credits.py
If any test fails, revert the extract immediately. The characterization suite remains the only merge gate. The expected diff is one helper plus one call site.
5. Keep model output inside that gate
Fluent coding assistants often skip this gate. Models propose whole-file cleanups that look tasteful. Whole-file cleanups break mutation contracts on the first run.
A tighter loop works better on messy repositories.
- Paste only
allocateplus failing test names. - Ask for one pure extract, never a redesign.
- Reject patches that alter leftover arithmetic at all.
- Re-run the characterization file after every patch.
Disclosure: This article was prepared as part of MonkeyCode's product outreach.
MonkeyCode offers free model access and a free server option. Those two pieces fit this characterization loop on messy repos. The model can draft _bonus_for from the pinned tests. The server can run unittest when the local tree is messy.
Do not send the whole repository on the first pass. Send the pinned tests and one function only. Compare the incoming diff against the decision table rows.
This article does not name models, quotas, or hardware. Treat those details as facts to verify in current docs. Availability can change without notice in product copy. Re-check the live docs before you plan a pipeline.
If the extract is trivial, skip the remote path. Local unittest already covers the seam.
6. A second extract only after green tests
Stop after _bonus_for lands and tests pass. The pool slice of twenty-five is still implicit. Cap overflow is still implicit in leftover math.
A later extract might isolate the pool take. Write new characterization rows before that extract.
def test_pool_take_never_exceeds_25(self):
rows = users({"id": "h", "plan": "free", "credit": 0})
out = allocate(rows, 80)
self.assertEqual(rows[0]["credit"], 25)
self.assertEqual(out["leftover"], 55)
Add that test on the still-unextracted pool logic. Then extract _take_from_pool(leftover) if the suite stays green.
One behavior per extract remains the whole method. Two extracts in one patch hide the failing seam.
Why these golden values are the artifact
The overflow row is easy to get wrong. Start with credit 990, team bonus 120, and pool 50. The function takes 25 from the pool, not 50.
Granted credit becomes 145 before the cap. 990 plus 145 is 1135, so 135 overflows. Leftover becomes 50 minus 25 plus 135, which is 160.
If a rewrite “simplifies” overflow, leftover drifts first. The characterization test fails before reviewers argue taste. That failure is the engineering signal, not the diff.
Mutation order is the second trap. The first pro user takes 25 pool units. The second pro user only receives the remaining five. A pure map over users would split the pool differently.
Copy the input with dict(r) before each case. Never reuse the same list across tests. Shared fixtures hide mutation bugs that production will show.
pool_in looks redundant until a wrapper appears. Wrappers often mutate the incoming pool name. Pinning the echo catches that rename during later extracts.
Limitations
Characterization tests freeze bugs as if they were law. If leftover overflow is wrong, tests will protect it.
They also miss uncalled branches in the same function. skip_inactive=False has no row in the suite above. Inactive users that already hold credit are untested here.
The suite uses in-process mutation of ordinary dicts. It will not catch file locking around this allocator. It will not catch thread races on the shared pool.
Golden values are literals copied from one run. They need review when the business policy actually changes. Do not confuse pinning current behavior with approving it.
Who should not use this approach
Do not use this flow for greenfield design work. You would pin accidents and call them contracts.
Do not use it when the module is wrong and security-sensitive. Fix the defect first, then pin the corrected output.
Do not use it as a substitute for explicit types. The dict shape is still implicit after _bonus_for.
Do not batch five extracts in one model pass. The point of the method is isolation of blame.
Skip remote model use if the code cannot leave the machine. Run the same tests locally in that case.
Checklist
- Copy the kitchen-sink function into place unchanged.
- Add tests that read return values and mutations.
- Fill a decision table from observed, failing knowledge.
- Extract one pure helper and no other seams.
- Re-run the same file, not a brand-new suite.
- Keep model patches inside that exact diff.
The core conclusion stays simple under pressure. Pin output first, then change one seam. Fluency is not the same artifact as a green characterization file.
Top comments (0)