Code Refactoring

refactorer

When to use

Use this skill when renaming, moving, extracting, or restructuring code — any change that may have downstream effects on callers, interfaces, tests, documentation, or API contracts.

Do NOT use when:

• New feature development (use feature-planning skill)
• Bug fixes that don't involve restructuring (use bug-analyzer skill)

Before refactoring

1. Read the agent docs — check agents/reference/docs/ and agents/settings/contexts/ for the area you're refactoring. For modules, also read the project's module-docs directory (path varies by stack — Laravel: app/Modules/{Module}/agents/; Nx: apps/{app}/docs/; mono-repo: per-package docs/). See the project-docs skill for the mapping.
2. Understand the scope — what exactly needs to change and why?
3. Find ALL references — use codebase-retrieval and view with search_query_regex to find every caller, implementation, test, and configuration that references the code being changed. If the change is to a closed set — an enum, union, literal type, state machine, role or permission set — identifier search is not enough: the consumers that break are the ones that never name the type. Search by shape as well (switch / match / if-chains over it, lookup tables keyed by it, validators, serializers, schemas, fixtures, translation keys) and classify each as exhaustive, deliberate fallback, or missing case. Procedure: downstream-changes-mechanics.
4. Map the impact — create a list of all files that will need changes.
5. Decide the verdict, if the refactor is a de-duplication — see the safe-abstraction gate below. An extraction with no recorded verdict is a guess about whether two things are the same thing.
6. Present the plan — show the user what will be affected before starting.

Safe-abstraction gate — before merging two things into one

This skill performs extractions; it is therefore the place that decides whether one should happen. A wrong abstraction costs more than the duplication it replaced, because the duplication is visible and the wrong abstraction grows caller flags until nobody can remove it. Ask, before moving any code:

1. Is this the same knowledge, or two facts that currently look alike?
2. Would both copies change for the same reason?
3. Is there one honest domain name that fits every call site?
4. Can the common core stay free of caller-specific flags and modes?
5. Are public contracts preserved, and do tests prove behavioural equivalence?
6. If the two copies diverged tomorrow, would that be a defect or legitimate evolution?

A "legitimate evolution" answer to the last question is decisive: stop, and record the verdict keep-duplicated. That closes the refactor successfully — so does de-abstract, splitting a shared unit that has grown modes back apart. Neither is a failure to act, and neither needs an apology in the output. Textual similarity is not evidence of shared knowledge: copies that read identically can encode different assumptions, and merging them changes behaviour silently.

The class names and the full verdict list are in redundancy-taxonomy.

When the refactor goes the other way and creates a new unit, the reuse question comes first: does an existing class or component already cover this, and what does composing or extending it cost against a new one? Thresholds per artifact class: component-oriented-and-oop-development.

Procedure: Refactor code

Step 1: Make the core change

• Rename, extract, move, or restructure the target code.
• Keep the change minimal and focused.

Step 2: Update all downstream dependencies

For each affected file (from the impact analysis):

• Callers: Update method calls, constructor arguments, imports.
• Interfaces / abstract methods: Update all implementations to match new signatures.
• Subclasses: Update overridden methods.
• Type hints / annotations: Update type references (PHPDoc, TypeScript types, Python type hints, Go generics, Rust generics).
• Config / bindings: Update service container bindings, route references, etc.
• Imports: Add or update import statements (use for PHP, import for JS/TS/Python, import blocks for Go, use for Rust).

Step 3: Update API layer (if request handlers / endpoints are affected)

When refactoring touches handlers, route registrations, request validators, or response shapers, walk the stack-appropriate boundary. Use the carve-out skill for the project's framework if one exists; otherwise consult the table below for what to check on each stack.

Carve-out routing:

• Laravel: laravel-api-endpoint
• Next.js: nextjs-patterns
• Symfony: symfony-workflow

Step 4: Update tests — with user approval

Before changing any test, present a summary to the user:

1. List all affected tests — which test files and which test cases need changes.
2. Describe what changes — for each test, explain what assertion/setup changes and why.
3. Classify the change:
   • ✅ Non-breaking: Test adapts to internal refactoring (renamed class, moved method) — same behavior, different code path. No API contract change.
   • ⚠️ Potentially breaking: Test expectations change (new field in response, changed validation rule, different error code) — this MAY affect API consumers.
   • 🔴 Breaking: Test removes or changes existing API behavior (removed field, changed endpoint URL, different response structure) — this WILL affect API consumers.
4. Ask for confirmation (numbered options):

   > 1. Yes — apply test changes
   > 2. No — I'll adjust the tests myself
   

5. Only proceed after the user confirms.

Rules for test changes:

• Do NOT change test expectations to make failing tests pass unless the refactoring intentionally changes behavior. A failing test after refactoring usually means the refactoring has a bug.
• Do NOT delete tests — adapt them to the new code structure.
• Do NOT reduce test coverage — if you split a class, split the test too.

Step 5: Verify with the project's quality tools

Run the project's type-checker and linter after each significant step — do NOT batch everything to the end. The exact command set depends on the stack; resolve via quality-tools skill or the project's Taskfile.yml / package.json scripts / composer.json scripts / Makefile.

If auto-fixers can rewrite types (Rector for PHP, eslint --fix for TS), re-run the type-checker after them — auto-fixers can introduce new errors.

Step 6: Run tests

• Run tests related to the changed code first (php artisan test --filter=..., pnpm test -- <pattern>, pytest -k <pattern>, go test ./{path}/..., cargo test {pattern}).
• Then run the full test suite (php artisan test, pnpm test, pytest, go test ./..., cargo test).
• All tests must pass before the refactoring is considered complete.

Step 7: Update documentation

After the code changes are verified, update all affected documentation:

Rules for doc updates:

• If you rename a class/method referenced in docs → update the reference.
• If you move files → update all path references.
• If you change a pattern → update the pattern description.
• If you add a new convention → document it.
• Do NOT create new docs unless the refactoring introduces a genuinely new concept.

Common refactoring patterns

Rename (method, class, property)

1. Find all usages → update all usages → update docs → run the static analyser → run tests.

Extract method / class

0. Run the safe-abstraction gate above — a keep-duplicated verdict ends it here.
1. Create the new method/class → move logic → update caller → update docs → run the static analyser → run tests.

Move class to different namespace

1. Move file → update namespace → find all use statements → update imports → update docs → run the static analyser.

Change method signature

1. Update the method → find all callers → update each caller → present test changes → update docs → run the static analyser.

Change API endpoint

1. Update controller + request + resource + OpenAPI schemas + route → present test changes → update docs (agents/reference/docs/controller.md, agents/reference/docs/api-resources.md) → run the static analyser → run tests.

Replace implementation (e.g. switch service)

1. Create new implementation → update binding → find all direct references → update → present test changes → update docs → run the static analyser → run tests → remove old implementation.

Move/restructure module

1. Move files → update namespaces → update ModuleServiceProvider if needed → update module routes → update module agent docs → update project contexts → run the static analyser → run tests.

Safety rules

• Never skip the caller search — missing a caller is the #1 cause of broken refactorings.
• Never remove old code before verifying the new code works everywhere.
• Never change test expectations without user approval — explain what changes and why first.
• Run the static analyser after every step (PHPStan, tsc --noEmit, mypy — whichever the project runs), not just at the end.
• Run tests after every step, not just at the end.
• Update docs after code changes — stale docs are worse than no docs.
• Do NOT commit or push — only apply local changes.
• Do NOT refactor code outside the requested scope — no drive-by cleanups.
• If the refactoring reveals more work than expected, stop and discuss with the user.

Cross-References

Output format

1. Refactored code with clear separation of changes
2. Before/after comparison for key changes
3. Test verification confirming no behavior change

Gotcha

• Always find ALL callers before renaming — the model frequently misses usages in tests and config files.
• Don't refactor and add features in the same commit — separate concerns for reviewability.
• The model tends to "improve" code it's refactoring — stay scope-focused, refactor means same behavior.
• Run the full test suite after every refactoring step — don't batch multiple refactors.

Do NOT

• Do NOT refactor without running tests before and after.
• Do NOT change test expectations without explicit user approval.
• Do NOT refactor across module boundaries without checking downstream.

Auto-trigger keywords

• refactoring
• rename
• move class
• change signature