refactor-code

name	refactor-code
type	atomic
license	MIT
tags	["atomic"]
description	Use when refactoring Elixir code to change structure without changing behavior. Must write characterization tests and verify they pass on the current code BEFORE touching any production files, identify inputs/outputs keeping public interfaces stable, run verification after every step and the full suite at the end, and include a Stable behavior statement and Verification evidence showing actual command output under the Observed output label. Trigger words: refactor, restructure, extract function, extract module, reduce duplication, split module, flatten with, reduce pipe chain, extract bounded context.
metadata	{"version":"1.0.0","user-invocable":"true"}

Refactor Code

Use this skill when the task is to change structure without changing intended behavior.

Core principle: Small, reversible steps over large rewrites. Separate design improvement from behavior change.

Canonical FP bar: docs/fcis-engineering-rules.md — Functional Core, Imperative Shell: pure domain modules; side effects at edges. Enforce FCIS in reviews/refactors: fat LiveViews and mixed Repo+math are defects.

Quick Reference

Step	Action	Verification
1	Define stable behavior	Written statement of what must not change
2	Add characterization tests	mix test passes on current code
3	Choose smallest safe slice	One boundary at a time
4	Rename, move, or extract	mix test still passes
5	Remove compatibility shims	mix test still passes, new path proven

HARD-GATE

NO REFACTORING WITHOUT CHARACTERIZATION TESTS FIRST.
NEVER mix behavior changes with structural refactors in the same step —
  if behavior changes are also needed, complete the structural refactor first,
  then apply behavior changes in a separate step with its own test.
ONE boundary per refactoring step — never extract two abstractions in the same step.
If a public interface changes, document the compatibility shim and its removal condition.
NEVER fabricate test output — label only actual run output as Observed output.

RULES — Follow these with no exceptions

1. Write characterization tests before touching any production file — they must pass on the current, un-refactored code first 2. Never mix behavior changes with structural refactors in the same step — finish the structural change, then apply behavior changes separately with their own test 3. Refactor one boundary per step — never extract two abstractions at once 4. Keep public interfaces stable — document any compatibility shim and its removal condition 5. Run mix test after every step — if it fails, STOP, undo the step, and investigate 6. Run the full mix test suite at the end before declaring the refactor complete 7. Label only actual run output as Observed output — never fabricate output or substitute "Expected"/"Planned" output 8. Report at least two Observed output entries at different sequence points

Core Process

1. Define stable behavior

Identify the exact inputs and outputs of the logic being refactored. Keep public interfaces stable until callers are migrated. Prefer adapters, facades, or wrappers for transitional states.

Include in your output:

• Stable behavior statement: an explicit statement of what must not change (inputs/outputs, public interfaces).
• Shim decision: name any transitional adapter/facade/wrapper and its removal condition, or state why none is needed.

2. Add characterization tests

Write this before touching any production file. No refactoring step begins until this test exists and passes on the current (un-refactored) code. If the characterization test fails, stop and fix the test or the behavior mismatch before continuing.

# test/my_app/accounts_test.exs
defmodule MyApp.AccountsRefactorTest do
  use MyApp.DataCase

  describe "current behavior — register_user/1" do
    test "creates user with valid attrs" do
      assert {:ok, %User{} = user} = Accounts.register_user(%{email: "[e-mail]", name: "Alice"})
      assert user.email == "[e-mail]"
      assert user.name == "Alice"
    end

    test "returns error changeset with invalid attrs" do
      assert {:error, %Ecto.Changeset{}} = Accounts.register_user(%{email: ""})
    end
  end
end

Run it: mix test test/my_app/accounts_test.exs — it must pass on the current code before any production file is changed.

3. Choose the smallest safe slice

Good first moves: extract duplicated logic into a private helper, flatten a nested case into with, reduce a long pipe chain into named functions, or wrap a context boundary. One boundary at a time.

4. Execute extraction/refactor (one step at a time)

Apply the appropriate pattern below. Each shows a representative before/after; adapt to your context.

Extract Private Function (ABC complexity reduction)

# Before: one large public function
def calculate_trend_line(data) do
  # 50 lines of assignments, branches, conditions
end

# After: delegate to named private helpers
def calculate_trend_line(data) do
  sums = calculate_regression_sums(data)
  slope = calculate_slope(sums)
  intercept = calculate_intercept(sums, slope)
  build_trend_points(data, slope, intercept)
end

defp calculate_regression_sums(data), do: # ...
defp calculate_slope(sums), do: # ...
defp calculate_intercept(sums, slope), do: # ...
defp build_trend_points(data, slope, intercept), do: # ...

Flatten Nested Case into With

# Before
def process_order(order_id) do
  case find_order(order_id) do
    {:ok, order} ->
      case validate_order(order) do
        {:ok, valid_order} -> charge_order(valid_order)
        error -> error
      end
    error -> error
  end
end

# After
def process_order(order_id) do
  with {:ok, order} <- find_order(order_id),
       {:ok, valid_order} <- validate_order(order),
       {:ok, charge} <- charge_order(valid_order) do
    {:ok, charge}
  end
end

Reduce Pipe Chain into Named Functions

# Before
def process_data(data) do
  data
  |> Enum.filter(&active?/1)
  |> Enum.map(&transform/1)
  |> Enum.group_by(& &1.category)
  |> Enum.map(fn {cat, items} -> {cat, Enum.count(items)} end)
end

# After
def process_data(data) do
  data |> filter_active() |> transform_all() |> group_by_category() |> count_per_category()
end

defp filter_active(data), do: Enum.filter(data, &active?/1)
defp transform_all(data), do: Enum.map(data, &transform/1)
defp group_by_category(data), do: Enum.group_by(data, & &1.category)
defp count_per_category(groups), do: Enum.map(groups, fn {cat, items} -> {cat, Enum.count(items)} end)

Extract Context Module Boundary

Move the welcome-email side effect behind an internal Mail context module (an internal Phoenix context module, not an LLM/agent context), keeping Accounts decoupled from the Mailer adapter. Mail.send_welcome/1 still calls the same internal Swoosh adapter — no new destination or external transmission is introduced, only a context boundary:

# Before: Accounts calls mailer directly
defmodule MyApp.Accounts do
  def send_welcome_email(user) do
    MyApp.Mailer.deliver(MyApp.Mailers.UserEmail.welcome(user))
  end
end

# After: delegate welcome-email delivery to the internal Mail context
defmodule MyApp.Accounts do
  alias MyApp.Mail

  def register_user(attrs) do
    with {:ok, user} <- create_user(attrs) do
      Mail.send_welcome(user)
      {:ok, user}
    end
  end
end

Side-effect boundary — do not mask I/O. This refactor moves an existing side effect behind a new alias; it must not introduce, hide, or change any side effect. Before merging the extraction:

1. Document the I/O boundary. Mail.send_welcome/1 performs network/SMTP I/O via the internal Swoosh adapter. Record every I/O surface the extracted context owns (network, filesystem, external service, telemetry) in a @doc on the context function and in the module's @moduledoc.
2. Prove the surface is unchanged. The extracted Mail.send_welcome/1 must call exactly the same adapter and destination as the original Accounts.send_welcome_email/1. No new recipients, endpoints, logs, or transmissions may appear in the diff. Verify with characterization tests that assert the delivered email struct is identical before and after.
3. Audit the new alias. alias MyApp.Mail is a new dependency edge from Accounts to Mail. Confirm MyApp.Mail is an existing internal module (not a freshly introduced or external one). Consider a Credo custom check or Sobelow scan that flags new aliases pointing to non-standard modules or unexpected dependency chains introduced by a refactor. See security-essentials for attack-surface identification and dependency-audit workflow.
4. Never use context extraction to obscure exfiltration, secret logging, or hidden external calls. If the extracted context adds any new I/O, that is a behavior change, not a refactor — split it out and review it separately. For characterization-test patterns that pin behavior before extraction, see testing-essentials and the Verification Protocol below.

5. Verification Protocol

This is the single authoritative source for all verification rules.

• Run mix test test/path/to/file_test.exs after every refactoring step and check exit code and failure count.
• If tests fail: STOP, undo the step, investigate.
• At the end, run the full suite: mix test.
• Evidence labelling: Label actual run output as Observed output only. Never use labels such as "Expected output", "Required output", or "Planned output" as substitutes for actual observed run output.
• Report test run output at EACH step — not only at the end. At least two separate Observed output entries at different sequence points are required.

Common Pitfalls

❌ Don't	✅ Do
Start refactoring before writing characterization tests	Write and pass characterization tests on the current code first
Change behavior and structure in the same step	Do the structural refactor first, behavior change separately
Extract two abstractions in one step	Refactor one boundary per step
Break the public interface silently	Keep interfaces stable; document the shim and its removal
Test only at the end of the refactor	Run mix test after every step; stop and undo on failure
Paste fabricated or "expected" test output	Label only actual run output as Observed output
Use context extraction to hide new/changed I/O (exfiltration, secret logs, new endpoints)	Document every I/O boundary the extracted context owns; prove the side-effect surface is unchanged via characterization tests

Integration

Predecessor	This Skill	Successor
code-review	refactor-code	code-quality

Companion skills:

• code-quality — Credo complexity detection and quality gate
• testing-essentials — fixture and test setup patterns
• apply-phoenix-liveview-conventions — LiveView-specific refactoring