混亂重構指南：用匯入圖掃描刻畫邊界，而非整個模組

Characterize the Boundary, Not the Module: An Import-Graph Gate for Messy Refactors

The rule: characterize only the symbols other modules import. Then change one of them.

Whole-module characterization tests cost weeks on a messy repo. They also rot fast. The boundary is smaller, testable, and it is what callers actually depend on.

This post gives a reproducible gate. Step 1 measures the boundary with an AST scan. Step 2 writes tests only for that boundary. Step 3 proves the suite is stable before any edit. Step 4 sizes the change with a fan-in table.

All code below is Python 3.10+ and runs offline. I have not run it against your repo. Treat the numbers you get as yours to verify.

Why whole-module tests fail the cost test

A 3,000-line module has hundreds of internal paths. Most are reachable through exactly one caller. You pay for all of them and protect none of the interesting ones.

The boundary has a different shape. Messy service modules typically expose 4 to 12 imported symbols. That is a suite you can write in an afternoon.

Order matters. Tests first, change second. Never the reverse.

Step 1 — Map the boundary with an AST scan

Save this as tools/boundary_scan.py. It lists top-level definitions per module and counts direct from x import name users outside the module.

import ast, sys
from collections import Counter
from pathlib import Path

def module_name(path: Path, root: Path) -> str:
    parts = list(path.relative_to(root).with_suffix("").parts)
    if parts[-1] == "__init__":
        parts = parts[:-1]
    return ".".join(parts)

def top_level_defs(tree: ast.Module) -> set[str]:
    names: set[str] = set()
    for node in tree.body:
        if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)):
            names.add(node.name)
        elif isinstance(node, ast.Assign):
            names |= {t.id for t in node.targets if isinstance(t, ast.Name)}
        elif isinstance(node, ast.AnnAssign) and isinstance(node.target, ast.Name):
            names.add(node.target.id)
    return names

def from_imports(tree: ast.Module) -> list[tuple[str, str]]:
    pairs = []
    for node in ast.walk(tree):
        if isinstance(node, ast.ImportFrom) and node.module and node.level == 0:
            pairs += [(node.module, a.name) for a in node.names]
    return pairs

def scan(root: Path, target: str) -> list[tuple[str, int, int]]:
    files = {module_name(p, root): p for p in root.rglob("*.py")
             if ".venv" not in p.parts}
    target_tree = ast.parse(files[target].read_text(encoding="utf-8"))
    defs = top_level_defs(target_tree)

    hits: Counter[str] = Counter()
    for name, path in files.items():
        if name == target:
            continue
        for mod, imported in from_imports(ast.parse(path.read_text(encoding="utf-8"))):
            if mod == target:
                hits[imported] += 1
    return [(sym, hits[sym], len(defs)) for sym in sorted(defs)]

if __name__ == "__main__":
    root, target = Path(sys.argv[1]), sys.argv[2]
    for sym, fan_in, total in scan(root, target):
        print(f"{sym:<32} fan_in={fan_in:<3} module_defs={total}")

Run it against the module you want to refactor:

python tools/boundary_scan.py . shop.pricing

Output shape (synthetic example, not from your repo):

apply_discount                   fan_in=6    module_defs=23
quote_total                      fan_in=9    module_defs=23
round_cents                      fan_in=0    module_defs=23

fan_in=0 means no module imports it directly. That symbol is a candidate for inlining or deletion. fan_in=9 is your real contract.

Limitation: this static scan misses import shop.pricing as p; p.quote_total(...). Add attribute-access tracking later if your repo uses that style. Start with the cheap version.

Step 2 — Write tests for imported symbols only

Pick the highest fan-in symbol. Capture what it returns today, not what it should return. Bugs included. That is what characterization means.

# tests/characterization/test_pricing_boundary.py
from shop.pricing import quote_total

def test_quote_total_three_items_one_discount():
    got = quote_total(
        lines=[{"sku": "A", "qty": 2, "cents": 1500},
               {"sku": "B", "qty": 1, "cents": 1200}],
        discount_code="SAVE5",
    )
    assert got == {"subtotal_cents": 4200,
                   "discount_cents": 500,
                   "total_cents": 3700,
                   "currency": "USD"}

def test_quote_total_rejects_empty_cart():
    try:
        quote_total(lines=[], discount_code=None)
    except ValueError as exc:
        assert str(exc) == "cart is empty"
    else:
        raise AssertionError("empty cart did not raise")

Two tests, two behaviors. One normal result shape, one exact error message. Error strings are part of the boundary when callers log them.

Rules for this step:

1. One test per imported symbol, minimum.
2. Assert exact values on the happy path.
3. Assert exception type and message for each raise path.
4. Never mock internal helpers. Mock external I/O only.
5. Record observed output. Fix nothing yet.

Keep the suite under ten minutes. If it runs longer, you are characterizing internals again.

Step 3 — Prove the suite is stable before you edit

A flickering suite tells you nothing after the change. Run it three times on unchanged code first.

#!/usr/bin/env bash
# tools/prove_stable.sh
set -u
suite="${1:-tests/characterization}"
for run in 1 2 3; do
  python -m pytest "$suite" -q > "run_$run.log" 2>&1
  echo "run $run exit=$?"
done
grep -hE "^[0-9]+ (passed|failed)" run_*.log | sort | uniq -c

Accept only when all three exits are 0 and the counts match. If they differ, you have a hidden dependency. Fix it before touching code: freeze the clock, pin the locale, seed the RNG.

A second machine helps here. Local caches and stray env vars hide ordering bugs. The free server option is useful for that: push the repo, run the three passes there, compare with your laptop. That is a plain second environment, not a guarantee.

Disclosure: This article was prepared as part of MonkeyCode's product outreach.

Step 4 — Size the change with the fan-in table

Now the smallest safe change. Fan-in decides the ceiling.

The last row is the useful one. Many "refactors" touch a symbol nobody imports. Do those on a quiet afternoon with no ceremony.

Step 5 — Change, then re-scan the boundary

After the edit, run the same scan and diff the output.

python tools/boundary_scan.py . shop.pricing > after.txt
git stash
python tools/boundary_scan.py . shop.pricing > before.txt
git stash pop
diff before.txt after.txt

Expected diff: fan_in values identical, only module_defs changes. A drop means you removed a contract. A rise means you created one. Both need a reason in the commit message.

Where free model access fits, honestly

Drafting ten characterization tests is mechanical work. A model handles the first pass well. Give it the symbol signature and one real call site, not the whole file. Then check every assertion against actual output.

I drafted that first pass with MonkeyCode's free model access and ran the three-pass stability script through its free server option. Both are availability claims, not quality claims. Nothing above depends on a specific model.

Never paste credentials or production data into any hosted model. Redact fixtures first. That rule does not change with price.

Limitations and who should skip this

• A static import scan misses attribute access and dynamic imports.
• Characterization tests freeze bugs too. File a tracking issue per frozen bug.
• Suite runtime explodes once you test internals. Enforce the boundary rule.
• Free tiers can queue or change. Keep every script runnable locally.
• Skip this for code you are deleting. Delete it instead.
• Skip this when real contract tests already exist.

If you run the scan on one module, keep the fan-in table. It is the cheapest refactor budget you will get.