Pin Caller Imports and Side-Effect Order Before One God-Module Split

Split a messy module only after characterization tests pin behavior. Record import surfaces, exception types, and side-effect order first. Then extract the smallest function that still passes those pins.

Large refactors fail when they move too many responsibilities. Callers still import the old public function names. Disk writes still must occur before any log line.

The failure this workflow targets

God modules mix parsing, I/O, and policy. One file both reads JSON and writes artifacts. The same function logs, mutates globals, and returns nested dicts.

AI-assisted cleanup often proposes a full layered rewrite. That rewrite usually breaks stable import paths first. It then reorders file writes and log lines.

Tests that only check a happy-path dict miss those breaks. This article treats that failure class as a measurement problem. Freeze observable behavior, then change one extract.

What to pin before any extract

Pin four observable surfaces rather than implementation comments. The table below is the working artifact contract. Skip a row only with a written reason.

Surface	Record exactly	Typical break after a sloppy split
Import surface	Public function names importers use	Facade rename, missing re-export
Raised types	Exception class, args, cause type	ValueError becomes RuntimeError
Return shape	Type, dict keys, list lengths	Nested key renamed or dropped
Side-effect order	mkdir, file bytes, logger name, log text	Log before file, extra newline

Do not pin private helper names in this suite. Do not pin comment text or docstring wording. Do not pin wall-clock duration in this suite.

Proposed messy module under test

The next block is a proposed example, not production code. It is unlabeled as executed on any host. Treat the block as a teaching fixture only.

# messy_report.py — proposed fixture
from __future__ import annotations

import json
import logging
from pathlib import Path
from typing import Any

log = logging.getLogger("messy_report")
_CACHE: dict[str, Any] = {}


def build_report(raw: str, out_dir: Path) -> dict[str, Any]:
    if raw in _CACHE:
        return _CACHE[raw]
    try:
        payload = json.loads(raw)
    except json.JSONDecodeError as exc:
        raise ValueError("invalid report json") from exc
    if not isinstance(payload, dict):
        raise TypeError("report root must be object")
    name = payload.get("name")
    if not isinstance(name, str) or not name:
        raise ValueError("missing name")
    out_dir.mkdir(parents=True, exist_ok=True)
    dest = out_dir / "report.json"
    dest.write_text(json.dumps(payload, sort_keys=True), encoding="utf-8")
    log.info("wrote %s", dest)
    result = {"name": name, "path": str(dest), "keys": sorted(payload)}
    _CACHE[raw] = result
    return result

The listed fixture hides three easy characterization traps. Cache identity keys rest on the raw string. The mkdir call still runs before the write.

The logger name remains the string messy_report. Callers depend on that exact logger name. Cached hits also skip later directory writes.

Characterization tests that freeze those traps

The next tests are a proposed harness. They do not claim a measured runtime. They pin order and identity, not speed.

# test_messy_report_pins.py — proposed, unexecuted
from __future__ import annotations

import inspect
import json
import logging
from pathlib import Path

import pytest

import messy_report as mr


@pytest.fixture(autouse=True)
def _clear_cache():
    mr._CACHE.clear()
    yield
    mr._CACHE.clear()


def test_public_import_surface():
    exported = {
        n for n, obj in vars(mr).items()
        if inspect.isfunction(obj) and not n.startswith("_")
    }
    assert exported == {"build_report"}


def test_invalid_json_raises_value_error():
    with pytest.raises(ValueError) as caught:
        mr.build_report("{", Path("/tmp/unused"))
    assert caught.value.args == ("invalid report json",)
    assert isinstance(caught.value.__cause__, json.JSONDecodeError)


def test_non_object_root_raises_type_error(tmp_path: Path):
    with pytest.raises(TypeError) as caught:
        mr.build_report("[]", tmp_path)
    assert caught.value.args == ("report root must be object",)


def test_missing_name_raises_value_error(tmp_path: Path):
    with pytest.raises(ValueError) as caught:
        mr.build_report("{}", tmp_path)
    assert caught.value.args == ("missing name",)
    assert not (tmp_path / "report.json").exists()


def test_write_then_log_order(tmp_path: Path, caplog: pytest.LogCaptureFixture):
    caplog.set_level(logging.INFO, logger="messy_report")
    raw = json.dumps({"name": "alpha", "n": 1})
    result = mr.build_report(raw, tmp_path)
    dest = tmp_path / "report.json"
    assert dest.read_text(encoding="utf-8") == '{"n": 1, "name": "alpha"}'
    assert result["name"] == "alpha"
    assert result["path"] == str(dest)
    assert result["keys"] == ["n", "name"]
    assert [r.name for r in caplog.records] == ["messy_report"]
    assert [r.getMessage() for r in caplog.records] == [f"wrote {dest}"]


def test_cache_returns_same_dict_identity(tmp_path: Path):
    raw = json.dumps({"name": "beta"})
    first = mr.build_report(raw, tmp_path)
    second = mr.build_report(raw, Path("still-unused"))
    assert first is second
    assert not Path("still-unused").exists()

Reset the module cache at the start of each pin. Shared module state will otherwise couple tests. The autouse fixture above keeps those pins independent.

Run the pin file before any production edit. A red suite means the teaching fixture drifted.

python -m pytest test_messy_report_pins.py -q

Do not extract while the pin file is red. Restore the fixture, then rerun the pin file.

Record the public surface as a file

Store exported names beside the tests, not in chat history. The next script is proposed and unexecuted. Commit its output with the pin file.

# print_surface.py — proposed
import inspect
import messy_report as mr

funcs = sorted(
    n for n, obj in vars(mr).items()
    if inspect.isfunction(obj) and not n.startswith("_")
)
print("\n".join(funcs))

python print_surface.py > pins/public_surface.txt
diff -u pins/public_surface.txt <(python print_surface.py)

Run that script and store the printed names. Treat a diff as a failed characterization pin. Do not rename exports to make the diff quiet.

Numbered workflow: smallest safe change

Follow these steps in the listed order. Do not skip writing the pin file.

1. Inventory callers with a grep, not a model chat.
2. Write the import-surface and exception pins first.
3. Add the write-order and logger-name pins second.
4. Add the return-key and cache-identity pins third.
5. Extract one pure step, usually JSON parse validation.
6. Re-export the old name from the original module.
7. Re-run the same pin file and stop on failure.
8. Only then consider a second extract from the module.

Use this command for step one on a Unix shell. It searches Python files for the public name.

rg -n "build_report|from messy_report import|import messy_report" --type py

If rg is missing, use this portable fallback. It walks .py files with a recursive grep.

grep -RInE "build_report|from messy_report import|import messy_report" --include='*.py' .

Record the caller list beside the pin file. Missing callers remain the usual production break.

Decision table for the first extract

Choose the smallest extract that current pins still allow. Larger structural moves must wait for later pins.

Candidate extract	Allowed now?	Why
parse_report(raw) -> dict	Yes	Pure path; exceptions already pinned
write_report(payload, dir)	Not yet	Needs mkdir-then-write pin plus callers
New ReportService class	No	Changes import surface in one step
Drop _CACHE	No	Identity pin would fail
Rename logger to report	No	Log pin keys on messy_report

The allowed first extract looks like this proposed patch. Keep build_report as the stable public name. Do not move that name in the first patch.

def parse_report(raw: str) -> dict[str, Any]:
    try:
        payload = json.loads(raw)
    except json.JSONDecodeError as exc:
        raise ValueError("invalid report json") from exc
    if not isinstance(payload, dict):
        raise TypeError("report root must be object")
    name = payload.get("name")
    if not isinstance(name, str) or not name:
        raise ValueError("missing name")
    return payload


def build_report(raw: str, out_dir: Path) -> dict[str, Any]:
    if raw in _CACHE:
        return _CACHE[raw]
    payload = parse_report(raw)
    out_dir.mkdir(parents=True, exist_ok=True)
    dest = out_dir / "report.json"
    dest.write_text(json.dumps(payload, sort_keys=True), encoding="utf-8")
    log.info("wrote %s", dest)
    result = {
        "name": payload["name"],
        "path": str(dest),
        "keys": sorted(payload),
    }
    _CACHE[raw] = result
    return result

Callers stay green without extra source edits. The wrapper still preserves cache and write order. Exact file bytes still follow sort_keys=True.

Where a free coding model fits

A coding model is useful after pins exist. It is not a substitute for the pin file.

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

MonkeyCode offers free model access and a free server option. Use those only against the pin file plus one candidate extract. Paste the failing assertion, not the whole repository.

Reject any patch that renames exports or reorders writes. Keep the first extract to one pure function.

The next loop is proposed and unlabeled as a timed benchmark.

1. Keep the pin file green on your local machine.
2. Send parse_report and the exception tests only.
3. Apply the diff on an isolated branch.
4. Run the same pytest pin command again.
5. Discard the branch if the import surface changes.

Do not request a full module rewrite from the model. Do not accept new class hierarchies in step one. Do not treat a clean compile as a pin.

What this method does not prove

Green pins do not prove the behavior is correct. They only prove the recorded behavior did not drift. Wrong JSON error text stays wrong until a later explicit change.

Cache identity can hide needed invalidation bugs here. If raw strings collide across tenants, pin that collision. Do not fix it inside the first extract.

Concurrency stays out of scope for this harness. The module level cache is not locked. These characterization pins will not catch data races.

Logger handlers attached during import remain unpinned here. Add a separate import-hook test for that case. Do not fold that case into this extract.

Byte pins freeze UTF-8 text with sorted keys. They do not freeze every JSON serializer flag. Add another pin before changing separators or ensure_ascii.

Who should not use this approach

Skip this workflow for greenfield modules with no callers. Skip it when product already rejects current behavior. Skip it when public error types must change now.

Skip it if you cannot run pytest locally. A free server does not replace a missing pin file. Skip it for binary formats this harness never opens.

Security-sensitive parsers still need dedicated fuzzing tools. These characterization pins are not a fuzzer. They only freeze one teaching fixture path.

Merge checklist

Before merge, confirm these four recorded facts. Public imports still match the recorded name set. Exception classes and args still match the pins.

Output files still appear before matching log lines. Cache identity still holds for the same raw string.

If a later extract needs a class, add new pins first. Then move one method and keep a wrapper.

The smallest safe change is the extract the pin file already understands. Stop there until the next pin file exists.