version: "1.0.0" name: refactor disable-model-invocation: true description: >- Clean up and improve already-written code — simplification, reuse/DRY extraction, efficiency, language modernization, and architecture/layer hygiene. QUALITY-ONLY: this skill does NOT hunt for correctness bugs — use /code-review for that. It applies edits then verifies via format + test. TRIGGER when: the user wants to tidy, simplify, modernize, de-duplicate, or make changed code more reusable or efficient. DO NOT TRIGGER when: the user wants to hunt correctness/security/crash bugs (use /code-review), scan the WHOLE codebase for rot/ball-of-mud hotspots (use /arch-health), or design a module's interface from scratch (use /codebase-design). allowed-tools: Read, Grep, Glob, Bash, Edit, Agent effort: high

Refactor

Improve the quality of changed code WITHOUT changing its behavior. Pure refactor: same inputs produce the same outputs. This skill APPLIES the cleanups, then runs the project's format and test commands to confirm nothing broke. It does NOT look for bugs — for correctness/security/crash analysis run /code-review.

Phase 0 — Load profile

Read these before touching any code. They define every project fact this skill needs:

1. .claude/PROJECT.md — Commands (format, lint, test), Architecture (layers,

boundaries, composition root), and any project-specific conventions.

1. .claude/rules/_generic/* — stack-agnostic rules (code quality, comments, naming). If

CONTEXT.md exists, align names toward its ubiquitous language (a rename to the canonical term is behaviour-preserving and in scope).

1. Stack rule packs in .claude/rules/* whose paths (front-matter glob) match the

target files. These carry the language- and framework-specific modernization and anti-pattern checks — do not hardcode any here; defer to them.

If PROJECT.md is missing or still TEMPLATE, fall back to the root CLAUDE.md (always in context) when it carries the commands/architecture above — proceed on it, noting you're running without a kit profile. Only if neither has those facts, tell the user to run /bootstrap first, then stop.

Phase 1 — Resolve target

$ARGUMENTS selects what to refactor:

• file path — that file.
• directory — all source files under it.
• `staged` — git diff --cached --name-only files.
• range (e.g. main..HEAD) — git diff --name-only <range> files.
• plan/card file — the files it scopes; the log goes back into it (see Persist to the artifact).
• (empty, default) — changed files: git diff --name-only plus

git diff --cached --name-only. If none, ask the user what to refactor.

Read every target file in full before judging it. Match each file against the rule packs' paths so you apply the right stack-specific checks per file.

Phase 2 — Refactor dimensions

Apply these in order. Each is QUALITY-only; never alter observable behavior. Give every candidate a verdict before it can become an edit: FIRST — it makes the change you are about to make cheaper; AFTER — that change is done and this area is due again soon; LATER — real, no payoff attached to today's work; NEVER — this behaviour is not changing again. Only FIRST and AFTER are applied; the rest go to Output § Deferred. A first-of-its-kind mess is never LATER.

D1 — Language modernization (per installed rules)

Apply the modern-idiom and anti-pattern checks defined in the matched stack rule packs (control flow, data containers, I/O, exception/error handling, etc.). Do not invent idioms — use only what the installed rules prescribe for this stack.

D2 — Architecture & layer hygiene (per PROJECT.md)

• Boundaries: imports point inward only; no sibling-to-sibling coupling; cross-boundary

wiring lives in the composition root named in PROJECT.md.

• SRP: each unit has one reason to change. If describing it needs "and" → split. First

answer aloud whether a second use site exists and whether an alternative implementation does; neither → the split buys an interface and no depth, so leave it and say why.

• Layer purity: domain/business logic stays free of I/O, rendering, and framework calls

per the layering in PROJECT.md; orchestration and presentation stay in their layers.

• Boundary conversion: external/raw responses are mapped to domain types at the adapter

edge, not passed raw into inner layers.

• Irremovable dependencies: one you cannot remove is exposed in a single place, not buried —

construction lifted into the constructor or one lazy accessor, a fragile external call wrapped.

D3 — Function quality (per code-quality rule)

• Size: a limit passed is a look, not a cut — extract only at a seam where meaning and

mechanics part ways and the split test holds (the piece reads without its caller, the caller without opening the piece); otherwise keep the function long and say why. Outputs include side effects: list the mutations before choosing the cut. Extract into the SAME unit first with an awkward name; moving it elsewhere is a separate step, and a branch moves with its condition.

• Naming: names state intent; rename the unclear.
• Parameters: 0–2 ideal; 3 acceptable; 4+ → group into a parameter object, and replacing two

or more fields still pays. A flag is any argument every caller passes as a literal and the body branches on — enums and strings too, not a computed value; split the function. For an Extract Class cut, remove one member and note what became nonsense: that list, not the count, is it.

• Guard clauses: flatten nesting with an early return only when one leg is the unusual case

AND nothing runs after the block — an audit call or a log after the if makes the flattening a behaviour change (Phase 3 rule 2 applies: confirm first).

• Command/Query separation: a unit either does something or returns something. The test is

observable side effects — caching, memoisation and lazy initialisation pass it and are not violations; do not flag them.

• Conditionals: one that picks which object handles a case is fine; one that inspects a value

and supplies behaviour on its behalf means a type is missing (a switch on the receiver's class, a respond_to? chain, an attribute named type/kind). Report what is missing, not "simplify".

D4 — Duplication extraction (DRY)

Same logic, computation, or construction in 2+ places → extract to one shared unit at the lowest layer both callers reach, per the architecture in PROJECT.md. Extract only what changes for the same reason: identical bodies encoding two rules are coincidence, and two cases do not show the axis of variation — name it, or wait for the third. Derive the abstraction, do not invent it: take the two most alike cases, remove the single smallest difference, run the suite, repeat. Gates — what does the merged version simplify (removing duplication is not itself a reason); which error can it no longer raise loudly; does it read harder than the copies, and then stop. Judge by result: one place to edit per behaviour, and a wrong abstraction gets dearer with every caller. An abstraction already proved wrong is undone by procedure: inline it back into EVERY caller, delete in each what its arguments show it never runs, then re-extract with all cases visible. Before a stand-in object replaces a repeated null check, say what the absence means — "there is none" and "there is one but unknown" differ; a site still asking "is this real?" guards instead.

D5 — Dead-code removal

Delete unused imports/symbols, unreachable branches, commented-out code, and stale TODO/FIXME/legacy/back-compat shims (MVP-stage projects keep no re-export stubs). Replace magic numbers with named constants. Delete comments that merely narrate the code; keep only non-obvious WHY. Confirm before deleting anything that could be a public/exported symbol or a back-compat shim callers may rely on — never remove user-visible behaviour silently (Phase 3 rule 2 applies). Before deleting an override or a field that may shadow a base one, text-search the tree for a base definition of the same name: found → the deletion is a behaviour change, not cleanup, and a green build proves nothing. When the complaint is "I can't follow this", subtract before explaining and comment only what survives removal.

D6 — Stack-specific dimensions

Apply every additional check defined in the installed stack rule packs matched in Phase 0 (framework patterns, data/ORM patterns, concurrency, domain invariants, etc.). These are the source of truth for stack specifics — reference them generically, never duplicate them.

Phase 3 — Apply & verify

1. Confirm before writing. Phases 1–2 produce a proposed cleanup list; this phase is the

first that touches files. Show the list — file, what changes, why — and get the user's go before the first Edit. A refactor is a multi-site, judgment-bearing edit whose diff the user is expected to review as a whole, so the gate is "apply these N?", not a per-edit prompt. Skip it only when the user's own request already named the exact change ("rename X to Y here").

1. Apply the cleanups with Edit, smallest safe steps first, running the tests after each

step, never once at the end and never from a red bar. A step that reddens the suite and is not obvious in seconds is reverted to the last green and redone smaller — never debugged forward; a reverted step is a result, not a failure. The red reads exactly two ways: behaviour changed (revert), or the test asserted an interaction the refactor dissolved (fix the test, say so). Merge two units by making them textually identical first, one micro-change at a time; replace a stored value by running new beside old and asserting they agree at every read before deleting the old. Name each move before making it and its mirror after; name its abort condition first (docs/decision-craft.md §1). Replacing working code wholesale runs as the reversible sequence in skills/tdd/refactoring.md §Changing structure safely.

1. Stay pure: if a change could alter behavior, is ambiguous, or needs a judgment call

(e.g. removing code that might be used by reflection, changing a public signature), STOP and ask the user instead of guessing. Defer rather than risk semantics.

1. Format: run the format command from PROJECT.md. If format is n/a (no

formatter), skip and note it in the output.

1. Test: run the test command from PROJECT.md (scope to affected tests when the

project supports it). If tests fail, the refactor changed behavior — revert the offending edit and report it. If a risky refactor lacks coverage, suggest /test first. If test is n/a (no suite at all), say so and lean harder on staying pure — recommend /test before any non-trivial refactor, since nothing else pins behavior. If the unit will not instantiate in a harness at all, read skills/refactor/LEGACY.md and follow it instead of widening this pass.

For large or multi-file targets, fan out reading/analysis with the Agent tool, then apply edits yourself so changes stay coordinated.

Output

Report concisely:

• Changes by dimension — what was cleaned up under each of D1–D6 (one line each).
• Files touched — absolute paths.
• Verification — format result and test result (pass/fail + summary).
• Deferred — anything skipped as risky/ambiguous or verdicted LATER/NEVER, with the reason.
• Price — what the pass cost as well as bought: more indirection, a worse metric, debts left.

More complex than the problem it removed → revert; an invisible concept made visible still wins.

Persist to the artifact

(Mandatory when `$ARGUMENTS` or the session brief names a plan, card or spec file.) Follow `../implement/reference/work-log.md`. Keep a ledger on disk from the first phase and record deviations the moment they are decided. Before the final reply, append ## Refactor Log — <date> — <verdict> to that file; its ### Result is the changes by dimension, each candidate's verdict (FIRST / AFTER / LATER / NEVER) with applied / left, and the price line. Anything you could not observe yourself goes under Re-check later, with its command and today's baseline: a post-deploy log grep, a soak count, a UI gesture. The chat output is a copy of the log, never the only record. No planning artifact → skip this section.

Hard rules

• Behaviour-preserving only. Same inputs → same outputs. If a change could alter

observable behaviour or is ambiguous, STOP and ask — defer rather than guess.

• Never hunt bugs here. Correctness/security/crash analysis is /code-review's job.
• Confirm destructive removals. Don't silently delete exported symbols or back-compat

shims callers may rely on.

• Never trade structure for a guessed cost. A cleanup that "looks slower" still ships —

finish it, then take the doubt to /perf with a baseline, not to a watered-down move.

• Verify, never publish. Run format + test; never git add/commit — suggest the

command, the user runs it.

Cross-reference

• `/code-review` — finds correctness, security, and crash bugs. This skill does NOT;

run it for bug hunting. The split mirrors the built-in taxonomy (/code-review = bugs, built-in /simplify = quality-only): this skill is the kit's PROJECT.md-aware counterpart of /simplify. When both passes run on the same diff, quality findings belong HERE and correctness findings there — never report the same issue from both.

• `/test` — add coverage BEFORE attempting a risky refactor so behavior is pinned.
• `/perf` — where a suspected performance cost goes, after the structure is clean.
• `/arch-health` — scans the WHOLE codebase for shallow modules & ball-of-mud

hotspots and routes the one you pick here; this skill cleans the current diff on demand.

• `/codebase-design` — the deep-module vocabulary D2's architecture hygiene draws on

when a boundary needs redesigning, not just tidying.

• `/clean-mvp` — whole-codebase cruft sweep (dead code, unused files, legacy shims);

this skill's D5 removes dead code only within the current target.

• `/distill` — mines the whole repo's implicit conventions into project rules; this

skill then applies those installed rules to the target diff (D1/D6 read them).