goberomsu/craft-skills/refactor

refactor

Restructures code so it is easier to work with without changing what it runs — the moment behavior changes, the work is a feature or a fix, not a refactor. Done well: verification stays green at checkpoints proportional to the change's blast radius, each move is mechanical, and the diff never mixes structure with behavior.

Phase 0 — is refactoring warranted?

Before touching structure: a test suite exists for the path and is green right now (run it, don't trust memory), and the working tree isn't mixing this with in-flight feature work. No coverage for the path → the legacy-code protocol below runs first; restructuring un-pinned behavior is a guess.

Legacy-code protocol — characterization tests first

1. Run coverage against the target and read the gaps: uv run pytest --cov=<pkg> --cov-report=term-missing <path> (Python) or npx vitest run --coverage (TypeScript).
2. Write characterization tests against representative inputs, including the odd already-in-production ones, asserting the actual observed output — not the output assumed correct. assert compute(weird_input) == 47 is right if 47 is genuinely what today's code returns, even if it looks wrong.
3. Get them green against current behavior. They pin what the code does now, bugs included — not yet a fix.
4. Only then refactor, keeping every characterization test green after each step below.
5. A real bug surfacing mid-characterization gets flagged (a note, a follow-up issue), never fixed in this pass — unless the task at hand is that fix, which runs as its own red-green-refactor cycle via principle-programming, before or after the restructuring, never blended into it.

Safety protocol

1. Tests green before starting (from Phase 0, or from the characterization protocol).
2. One move at a time from references/catalog.md — never combine two into one step.
3. Verify proportionally to blast radius: run a focused test after a local move, a broader suite after a cohesive checkpoint, and the final relevant suite when the refactor is complete. A red result reverts the move or cohesive group that caused it rather than debugging forward on a refactor that just broke something.
4. Refactor commits stay separate from behavior commits. A planned change with declared breakage defers to the principle-architecture skill's outcome-oriented-execution reference instead. Detect a commit mixing the two before it lands:

``bash git diff --staged --diff-filter=M -- '*test*' '*spec*' '*_test.*' '*.test.*' | grep -E '^[+-][^+-].*\b(assert|expect)\b' ``

--diff-filter=M exempts brand-new test files (characterization tests, added coverage) by construction — no output means clean. An existing assertion's expected value flipping (a -/+ pair changing assert x == 47 to == 48, or a deletion) is the behavior-change signal — split the commit. A +-only line adding a new assertion beside untouched ones is just added coverage, not proof of a mix.

Scope guard

• Split when the change no longer forms one cohesive, independently reviewable and revertible structural unit. File count is a reviewability signal, not a fixed ceiling.
• Boy-scout boundary, restated: spotting an unrelated smell mid-task is never license to fix it inline — flag it (a follow-up note or a message proposing a follow-up) and keep the current diff scoped to the stated task.

Routing

• Sequencing an addition, refactor, or rewrite → Subtract Before You Add: remove dead code, redundant validators, and stub references first, then build on the simpler base.
• Which smell, which detect command, which threshold → references/code-smells.md.
• Which mechanical move fixes which smell, with worked Python/TypeScript examples → references/catalog.md.
• Shrinking a whole package rather than one function — measure candidate linter rule sets, enable what pays, autofix, then hand-simplify what lint cannot express → references/lint-first.md. Prefer linter configuration to a bespoke script, and keep the mechanical and judgment commits separate.
• A one-shot terminal scan across a whole directory → scripts/detect-smells.sh <dir> when the stated refactor concerns a smell class the script can detect (a reporter, always exits 0 — review relevant findings; false-positive profile documented per rule). Route it by the skill package's own directory, not the target project's cwd, since the agent's cwd at invocation time is the project being scanned: bash <skill-dir>/scripts/detect-smells.sh <target-dir>.
• When symbol safety matters, prefer available language-server definitions, references, rename, and diagnostics; confirm support in the source project, not from another workspace's server status.
• Use an available AST tool when syntax-aware matching or transformation helps; consult an existing code graph only for a concrete dependency or impact question, checking its coverage and freshness.
• Without semantic support, use bounded text search and source inspection, disclose coverage limits, and validate with project checks; text matches are not semantic proof or complete reference coverage. Stop when that fallback cannot establish the move's safety rather than assuming a server, hook, or skill wrapper must be installed.
• File-size ceiling (250 pure LOC) and its escape hatches → principle-programming; this skill owns function-level size, not file-level.
• Turning any rule here into an enforced lint/hook/pre-commit check → use the target repository's existing enforcement tooling.

Mutable tool and LSP facts

For mutable language-server, test, runtime, and tool behavior, consult official primary documentation for the installed version and platform first. Disclose conflicts rather than silently choosing a source. A more-specific repository-local contract or reproducible evidence for the matching version and platform may override general or stale documentation. Unresolved support stays unknown: do not invent a command or semantic operation; stop or use the repository's verified fallback.

Requirements

• git — the commit-separation check and churn-based smell detection. Consult Git documentation for the installed version, and safely record git --version before relying on mutable Git behavior.
• grep, awk, find (POSIX) — used directly and inside scripts/detect-smells.sh.
• A test runner: pytest (+ pytest-cov) for Python, vitest (--coverage) for TypeScript. Consult each incumbent runner's official documentation for its installed version and run its documented version probe.
• When language-server support is used, consult the incumbent server's official documentation for its installed version and run its documented version probe before relying on semantic operations.
• Optional: vulture (Python) / ts-prune (TypeScript) for the Dead Code entry in references/code-smells.md.
• The repository's incumbent linter when references/lint-first.md applies. Consult its official documentation for the installed version before enabling a rule set, run its documented version probe, and support only rule behavior verified for that version; a linter release that changes a rule's fix behavior re-triggers the re-evaluation below.
• When Git, the incumbent language server, or the incumbent test runner changes version or capability, review the applicable official documentation, rerun affected detection and characterization evals, update this recipe if needed, bump the package version, and record the reason in the commit message before trusting prior results.

Common rationalizations

Verification

• [ ] Phase 0 passed: trigger identified, tests confirmed green (or the characterization protocol completed first).
• [ ] Verification matched blast radius: focused test after each local move, broader suite at cohesive checkpoints, and final relevant suite at completion.
• [ ] No commit mixes a structural change with a changed test assertion (checked with the detection command above); a planned change with declared breakage follows the principle-architecture skill's outcome-oriented-execution reference instead.
• [ ] Any applicable smell fixed cites its references/code-smells.md entry and the detect command's result.
• [ ] Scope stayed within the stated task; anything spotted-but-out-of-scope was flagged, not fixed inline.
• [ ] For a lint-first pass: candidate rule sets were counted before being enabled, autofixes landed in their own commit ahead of the hand edits, and every reverted autofix is recorded as a scoped ignore with its reason.