Refactor

Refactor

> Requires: the sibling protocols skill (shared protocol masters); uses skills.config.json when present. Missing protocols → tell the user to install the full supermodo package.

A systematic, phase-gated refactoring process that transforms accumulated code into small, pure, well-tested functional modules. The skill analyzes before touching code, plans before executing, tests before changing, and verifies after each step.

Invocation

/refactor           # file, directory, module, or "project"
/refactor src/queries/    # all files in a directory
/refactor .               # current working directory

When scope is large, analyze the full scope, build a dependency-ordered plan, and chunk work into verifiable steps.

Core Principles

These shape every decision. Understand the reasoning — they enable judgment calls in edge cases.

Never Assume

When uncertain about intent, impact, or correctness — ask the user. Never silently delete exports (they may exist for future use), never silently tighten external API types, never assume a function is "dead" just because you can't find callers in this repo. Use AskUserQuestion for any decision that could be wrong.

Pure Core, Impure Shell

Separate I/O from logic. Transform functions are pure (output depends only on input). I/O lives at the edges — in thin orchestrators wiring pure functions together. This makes code testable without mocks and composable with pipe/chain.

Small Units

Functions ≤25 lines. Files ≥300 lines signal mixed concerns (exception: files large because the domain is large — many enum values, schema fields — where each internal unit is small and focused). Analyze before splitting.

DRY with Codebase Awareness

Before creating a utility function, search the codebase for existing functions with similar purpose. If one exists in a package, propose reusing or merging it — show both side by side, ask the user. Generic utilities belong in the appropriate package with comprehensive unit tests, not in the consuming app.

Behavior Preservation

Refactoring never changes what the code does — only how it's structured. Capture every behavioral contract in tests before structural changes begin.

Runtime Performance First

Refactored code must be as efficient or more efficient. When choosing between patterns, always prefer the faster option:

• for-of is faster than .map()/.flatMap() for large datasets? Use for-of.
• Object mutation is faster than spread in a hot path? Note it in the plan with reasoning.
• Single-pass is better than multiple chained operations? Combine them.

Functional style is the goal, but never at the cost of runtime performance. When a functional pattern introduces measurable overhead (intermediate arrays, repeated allocations), document the tradeoff in the plan and choose the performant option.

---

Phase 1: Analysis

Read the target and build a complete picture before proposing changes.

What to Read

1. The target file(s)
2. Every file importing from the target (dependents) — grep for import paths
3. Every file the target imports from (dependencies)
4. Existing test files for the target
5. The package's mod.ts to understand the public API surface

Smell Detection

Scan for these patterns and quantify each:

| Smell | What to Count | |-------|---------------| | Large files | Lines per file (signal at ≥300) | | Large functions | Lines per function (threshold: >25) | | let declarations | Every let that could be const or eliminated via map/reduce | | try/catch in logic | try/catch outside I/O boundary wrappers | | Imperative loops | for/while that should be map/filter/reduce/forEach — but keep for-of when it's faster | | Mixed concerns | Functions combining I/O and transformation | | Impure functions | Side effects that could be eliminated | | Loose types | any, unknown, generic string for domain concepts | | Circular deps | Mutual imports between files | | _-prefixed variables | Unused variables disguised with underscore — delete them entirely | | Silent error swallowing | catch blocks that return empty arrays/null, hiding real failures | | Inconsistent error handling | Mix of Result returns, thrown exceptions, and silent swallowing in the same module | | Inconsistent logging | Mix of logger patterns, missing context, or logging baked into core logic instead of hooks/callbacks | | Repeated construction | Same object shape built N+ times with only a few fields varying (candidate for factory/declarative spec) |

Usage Tracing

For every exported function in the target, grep the entire codebase for actual callers. This reveals:

• Dead exports: functions with zero callers outside their own file. Do NOT silently delete — flag them and ask the user. They may exist for future use, external consumers, or CLI entry points not visible in the codebase.
• Test-only usage: functions imported only by test files — potential candidates for internalization.
• Single-caller functions: exported but called from exactly one place — may not need to be public.

Present the usage map in the analysis.

Bug & Anomaly Hunting

Beyond structural smells, actively look for semantic issues:

• Stale closures: variables captured at construction time that should be captured at invocation time (e.g., Date.now() in a factory vs returned closure)
• Stale accumulators: mutable state (counters, metrics, buffers) initialized at construction but never reset between invocations — data bleeds across calls
• Uncaught exceptions in wrappers: functions that promise to return Result but don't catch throws from the wrapped operation — exceptions bypass the Result contract
• Silent data loss: catch blocks that return [] or null, masking schema changes, connection failures, or corrupt data
• Scope confusion: module-level let with lazy initialization that could be stale or cause race conditions
• N+1 query patterns: same database query called redundantly in a loop, or called independently by multiple sibling functions when a single call could be shared
• Performance hotspots: O(n) lookups (.find(), .filter()) inside loops or called frequently over large datasets — should use Map/Set for O(1) access
• Off-by-one: window calculations, retry counters, array slicing
• Duplicated state machines: same state transition logic implemented in multiple places, prone to divergence

If you find a potential bug, flag it clearly in the analysis with evidence. Do not fix it silently — ask the user whether to address it during refactoring.

Error Handling & Logging Audit

Catalog every error handling and logging pattern in the target:

• Which functions return Result vs throw vs silently swallow?
• Which use the project's logger vs console vs nothing?
• Are error types consistent (discriminated unions) or ad-hoc (generic Error)?
• Is logging baked into core logic, or separated via hooks/callbacks?

The plan should propose a coherent, consistent pattern for the entire module.

Dependency Order (multi-file scope)

Build the import graph. Work bottom-up: leaf files (no internal dependents) first, root orchestrators last. When refactoring a file, its dependencies are already clean.

Skip Recommendation

If a file is large but well-structured — each function small, pure, and focused — recommend skipping. Explain why. The user can override.

---

Phase 2: Plan & Approval

Present the plan. Wait for explicit approval before touching code.

Plan Format

## Refactoring Plan: 

### Assessment
- Files to refactor: N (with line counts)
- Files to skip: N (with reasons)
- Estimated new files: N
- Dead exports found: N (list with caller counts — pending user decision)
- Potential bugs found: N (list with evidence)

### Execution Order (bottom-up)
1. `types.ts` (leaf) — extract domain types, add branded types
2. `validators.ts` (leaf) — extract validation, eliminate try/catch
3. `transforms.ts` (depends on types) — extract pure transforms
4. `orchestrator.ts` (root) — thin I/O wiring only

### Per-File Changes
For each file:
- Current state (size, smell count)
- Proposed extractions (what moves where, before/after for key changes)
- New files with proposed paths

### Repeated Patterns
- Construction patterns to replace with factories or declarative specs
- Identify the template, show the proposed factory/spec, count affected sites

### Error Handling Strategy
- Proposed consistent pattern for the module (Result-based, with specific error types)
- Which existing patterns change and how
- Logging approach (separated from core logic via hooks/callbacks where possible)

### Performance Considerations
For each structural change that could affect runtime:
- Note whether it's hot-path code (called frequently, large datasets)
- If proposing immutable patterns (object spreads), note allocation cost vs mutation
- If replacing for-of with map/flatMap, note intermediate array creation
- Recommend the faster option with reasoning

### Utilities to Extract
- Functions to move to packages (with target package)
- Existing similar utilities found (with file paths and line numbers)

When pragmatism vs strictness is ambiguous — especially around external API types, third-party library patterns, or shared utility changes — use AskUserQuestion rather than assuming.

---

Phase 3: Safety Net

Write tests BEFORE refactoring begins to capture existing behavior.

Strategy: Lightweight Before, Comprehensive During

The pre-refactoring tests are a safety net, not the final test suite. The heavy testing investment goes into the extracted functions during Phase 4.

Before Refactoring: Behavioral Contract Tests

• Test every exported function's input→output contract
• Happy path + key error cases for each
• Test observable behavior and return shapes, not internal implementation
• For Result-returning functions: verify Ok shape on success, Err on expected failures
• Keep tests fast — mock I/O at boundaries

These tests verify that refactoring doesn't break the public API contract. They don't need to cover every internal code path — that code is about to be restructured.

Process

1. Create test files following project conventions (*.test.ts)
2. Write behavioral contract tests for each exported function
3. Run tests — all must pass before Phase 4 begins
4. If a test reveals an existing bug: flag it to the user, ask whether to fix during refactoring or preserve current behavior

---

Phase 4: Incremental Execution

Work through the plan one file at a time, in dependency order.

4a. Extract & Transform

Move functions to new homes. Convert imperative patterns to functional:

• let → const: Eliminate mutation via map/reduce/accumulated values
• for/while → map/filter/reduce/forEach: Declarative iteration — but keep for-of when it's faster for the dataset size
• try/catch in business logic → Result chains: pipe/chain/map composition. try/catch stays only at I/O boundaries in packages — wrapping external APIs and returning Result
• Large functions → composed small functions: Break into named steps, compose with pipe
• Silent error swallowing → explicit Result handling: Replace catch { return [] } with proper Result returns that make failures visible
• Repeated construction → factory functions or declarative specs: When the same object shape is built 5+ times with only a few fields varying, extract a factory function or a declarative field spec that generates the objects

4b. Organize by Concern

When splitting a file, create a directory structured by layer:

feature/
  mod.ts              # re-composes and exports the public API
  types.ts            # domain types, branded types, error unions
  queries.ts          # I/O: database/API calls (impure shell)
  transforms.ts       # pure: data transformations
  validators.ts       # pure: validation logic

Adapt to actual concerns present. Not every split needs all layers.

4c. Component Library Reuse (frontend)

When refactoring .tsx files, list the project's component library directory recursively to discover available components. If inline JSX replicates what a library component already provides, replace it with an import. If a reusable UI element doesn't exist in the library yet, create it there following the existing conventions — never leave components inline in route/page files.

4d. DRY Check

Before creating any utility function:

1. Search the codebase for functions with similar purpose
2. If similar exists: show both implementations, propose merge or reuse, ask user via AskUserQuestion
3. If the function is generic, place it in the right package with unit tests

4e. Type Tightening

• Replace any/unknown with proper types
• Branded types for domain concepts (e.g. UserId, Email, DateString)
• Discriminated union error types for Result
• ReadonlyArray and Readonly everywhere
• External API types: be pragmatic. If unsure whether a field is reliably present, ask the user via AskUserQuestion rather than assuming strict types

4f. Comprehensive Unit Tests for Extracted Code

This is where the heavy testing investment goes. Every extracted function gets a thorough test suite:

• Pure functions: various inputs, edge cases, type boundaries, empty inputs, large inputs
• I/O wrappers: Result shape (Ok on success, specific Err variants on failure)
• Factory functions: verify all generated shapes match expected output
• Composed pipelines: end-to-end behavior through the composition

These tests are the permanent regression suite — they "seal the bottle."

4g. Update Consumers

• Delete the original file — no barrel re-exports
• Update every import site to new file paths
• Verify no broken imports via type-checker

4h. Verify After Each Step

Run the full test suite (contract tests + new unit tests) after each extraction. All tests pass before moving to the next file.

If a test breaks because it tested implementation details (mocked internals, asserted call order), rewrite it to test behavior instead — this is an improvement, not a regression.

Handling Circular Dependencies

When analysis reveals circular imports:

1. Identify the shared types/interfaces causing the cycle
2. Extract them into a types.ts both modules import from
3. Break the cycle — neither file should import from the other

---

Phase 5: Verification & Summary

After all files are refactored:

1. Run complete test suite
2. Run linter and type-checker
3. Present metrics summary:

## Refactor Summary: 

| Metric              | Before | After  |
|---------------------|--------|--------|
| Files               | N      | N      |
| Avg function size   | N ln   | N ln   |
| Max function size   | N ln   | N ln   |
| Pure functions      | N%     | N%     |
| Test coverage       | N%     | N%     |
| let declarations    | N      | 0      |
| try/catch (non-I/O) | N      | 0      |
| Dead exports        | N      | 0      |
| Silent catch blocks | N      | 0      |

All N tests passing. No behavior changes.

---

Flow integration

When invoked by the flow orchestrator, refactor is stage 6 (clean the working feature) running as a subagent with its own context, after the tests gate has passed:

• Scope is the flow's working feature, not the whole project. Refactor the

code the run touched (read the earlier stage reports in .skills/supermodo/runs// to scope it) — a flow refactor is not an invitation to restructure unrelated modules.

• Write the stage report per ../protocols/references/reports.md to

.skills/supermodo/runs//06-refactor.md — skill: refactor, status (ok | failed | needs-input | skipped), summary (the before/after metrics), drift_notes, decisions, questions (only on needs-input).

• Never mutate documentation

…

Source & license

This open-source skill is cataloged on AgentStack and links to its original source — we do not rehost the code.

• Author: supermodo
• Source: supermodo/skills
• License: MIT

Install and usage instructions live in the source repository linked above.