Refactoring Safely

Core cycle: Change → Test → Commit (repeat until complete)

Don't use for:

• Changing functionality (use feature development)
• Fixing bugs (use hyperpowers:fixing-bugs)
• Adding features while restructuring (do separately)
• Code without tests (write tests first using hyperpowers:test-driven-development)

BEFORE any refactoring:

# Use test-runner agent to keep context clean
Dispatch hyperpowers:test-runner agent: "Run: cargo test"

Verify: ALL tests pass. If any fail, fix them FIRST, then refactor.

Why: Failing tests mean you can't detect if refactoring breaks things.

2. Create bd Task for Refactoring

Track the refactoring work:

bd create "Refactor: Extract user validation logic" \
  --type task \
  --priority P2

bd edit bd-456 --design "
## Goal
Extract user validation logic from UserService into separate Validator class.

## Why
- Validation duplicated across 3 services
- Makes testing individual validations difficult
- Violates single responsibility

## Approach
1. Create UserValidator class
2. Extract email validation
3. Extract name validation
4. Extract age validation
5. Update UserService to use validator
6. Remove duplication from other services

## Success Criteria
- All existing tests still pass
- No behavior changes
- Validator has 100% test coverage
"

bd update bd-456 --status in_progress

3. Make ONE Small Change

The smallest transformation that compiles.

Examples of "small":

• Extract one method
• Rename one variable
• Move one function to different file
• Inline one constant
• Extract one interface

NOT small:

• Extracting multiple methods at once
• Renaming + moving + restructuring
• "While I'm here" improvements

Example:

// Before
fn create_user(name: &str, email: &str) -> Result<User> {
    if email.is_empty() {
        return Err(Error::InvalidEmail);
    }
    if !email.contains('@') {
        return Err(Error::InvalidEmail);
    }

    let user = User { name, email };
    Ok(user)
}

// After - ONE small change (extract email validation)
fn create_user(name: &str, email: &str) -> Result<User> {
    validate_email(email)?;

    let user = User { name, email };
    Ok(user)
}

fn validate_email(email: &str) -> Result<()> {
    if email.is_empty() {
        return Err(Error::InvalidEmail);
    }
    if !email.contains('@') {
        return Err(Error::InvalidEmail);
    }
    Ok(())
}

4. Run Tests Immediately

After EVERY small change:

Dispatch hyperpowers:test-runner agent: "Run: cargo test"

Verify: ALL tests still pass.

If tests fail:

1. STOP
2. Undo the change: git restore src/file.rs
3. Understand why it broke
4. Make smaller change
5. Try again

Never proceed with failing tests.

5. Commit the Small Change

Commit each safe transformation:

Dispatch hyperpowers:test-runner agent: "Run: git add src/user_service.rs && git commit -m 'refactor(bd-456): extract email validation to function

No behavior change. All tests pass.

Part of bd-456'"

Why commit so often:

• Easy to undo if next step breaks
• Clear history of transformations
• Can review each step independently
• Proves tests passed at each point

6. Repeat Until Complete

Repeat steps 3-5 for each small transformation:

1. Extract validate_email() ✓ (committed)
2. Extract validate_name() ✓ (committed)
3. Extract validate_age() ✓ (committed)
4. Create UserValidator struct ✓ (committed)
5. Move validations into UserValidator ✓ (committed)
6. Update UserService to use validator ✓ (committed)
7. Remove validation from OrderService ✓ (committed)
8. Remove validation from AccountService ✓ (committed)

Pattern: change → test → commit (repeat)

7. Final Verification

After all transformations complete:

# Full test suite
Dispatch hyperpowers:test-runner agent: "Run: cargo test"

# Linter
Dispatch hyperpowers:test-runner agent: "Run: cargo clippy"

Review the changes:

# See all refactoring commits
git log --oneline | grep "bd-456"

# Review full diff
git diff main...HEAD

Checklist:

• All tests pass
• No new warnings
• No behavior changes
• Code is cleaner/simpler
• Each commit is small and safe

Close bd task:

bd edit bd-456 --design "
... (append to existing design)

## Completed
- Created UserValidator class with email, name, age validation
- Removed duplicated validation from 3 services
- All tests pass (verified)
- No behavior changes
- 8 small transformations, each tested
"

bd close bd-456

// "Refactored" version fn validate_email(email: &str) -> Result<()> { if email.is_empty() { return Err(Error::InvalidEmail); } if !email.contains('@') { return Err(Error::InvalidEmail); } // NEW: Added extra validation if !email.contains('.') { // BEHAVIOR CHANGE return Err(Error::InvalidEmail); } Ok(()) }

1. Extract validation (pure refactoring, no behavior change)
2. Commit with tests passing
3. THEN add new validation as separate feature with new tests
4. Two clear commits: refactoring vs. feature addition

What you gain:

• Clear history of what changed when
• Easy to revert feature without losing refactoring
• Tests document exact behavior changes
• No surprises in production

Then runs tests

$ cargo test ... 23 test failures ...

Now what? Which change broke what?

1. Rename ONE function → test → commit
2. Extract ONE class → test → commit
3. Move ONE file → test → commit
4. Continue one change at a time

If test fails:

• Know exactly which change broke it
• Revert ONE commit, not all work
• Fix or make smaller change
• Continue from known-good state

What you gain:

• Tests break → immediately know why
• Each commit is reviewable independently
• Can stop halfway with useful progress
• Confidence from continuous green tests
• Clear history for future developers

// Developer refactors without tests: // - Extracts 5 methods // - Renames variables // - Simplifies conditionals // - "Looks good to me!"

// Deploys to production // 💥 Payments fail for amounts over $1000 // Edge case handling was accidentally changed

1. Write tests FIRST (using hyperpowers:test-driven-development)

   • Test happy path
   • Test all edge cases (amounts over $1000, etc.)
   • Test error conditions
   • Run tests → all pass (documenting current behavior)

2. Then refactor with tests as safety net

   • Extract method → run tests → commit
   • Rename → run tests → commit
   • Simplify → run tests → commit

3. Tests catch any behavior changes immediately

What you gain:

• Confidence behavior is preserved
• Edge cases documented in tests
• Catches subtle changes before production
• Future refactoring is also safe
• Tests serve as documentation

• Tests exist and pass
• Changes are incremental
• Business logic stays same
• Can transform in small, safe steps
• Each step independently valuable

When to Rewrite

• No tests exist (write tests first, then refactor)
• Fundamental architecture change needed
• Easier to rebuild than modify
• Requirements changed significantly
• After 3+ failed refactoring attempts

Rule: If you need to change test assertions (not just add tests), you're rewriting, not refactoring.

Strangler Fig Pattern (Hybrid)

When to use:

• Need to replace legacy system but can't tolerate downtime
• Want incremental migration with continuous monitoring
• System too large to refactor in one go

How it works:

1. Transform: Create modernized components alongside legacy
2. Coexist: Both systems run in parallel (façade routes requests)
3. Eliminate: Retire old functionality piece by piece

Example:

Legacy: Monolithic user service (50K LOC)
Goal: Microservices architecture

Step 1 (Transform):
- Create new UserService microservice
- Implement user creation endpoint
- Tests pass in isolation

Step 2 (Coexist):
- Add routing layer (façade)
- Route POST /users to new service
- Route GET /users to legacy service (for now)
- Monitor both, compare results

Step 3 (Eliminate):
- Once confident, migrate GET /users to new service
- Remove user creation from legacy
- Repeat for remaining endpoints

Benefits:

• Incremental replacement reduces risk
• Legacy continues operating during transition
• Can pause/rollback at any point
• Each migration step is independently valuable

Use refactoring within components, Strangler Fig for replacing systems.

1. Tests must stay green throughout refactoring → If they fail, you changed behavior (stop and undo)
2. Commit after each small change → Large commits hide which change broke what
3. One transformation at a time → Multiple changes = impossible to debug failures
4. Run tests after EVERY change → Delayed testing doesn't tell you which change broke it
5. If tests fail 3+ times, question approach → Might need to rewrite instead, or add tests first

Common Excuses

All of these mean: Stop and return to the change→test→commit cycle

• "Small refactoring, don't need tests between steps"
• "I'll test at the end"
• "Tests are slow, I'll run once at the end"
• "Just fixing bugs while refactoring" (bug fixes = behavior changes = not refactoring)
• "Easier to do all at once"
• "I know it works without tests"
• "While I'm here, I'll also..." (scope creep during refactoring)
• "Tests will fail temporarily but I'll fix them" (tests must stay green)

• All tests pass (verified with hyperpowers:test-runner agent)
• No new linter warnings
• No behavior changes introduced
• Code is cleaner/simpler than before
• Each commit in history is small and safe
• bd task documents what was done and why
• Can explain what each transformation did

Can't check all boxes? Return to process and fix before closing bd task.

This skill is called by:

• General development workflows when improving code structure
• After features are complete and working
• When preparing code for new features

Agents used:

• test-runner (runs tests/commits without polluting main context)

When stuck:

• Tests fail after change → Undo (git restore), make smaller change
• 3+ failures → Question if refactoring is right approach, consider rewrite
• No tests exist → Use hyperpowers:test-driven-development to write tests first
• Unsure how small → If it touches more than one function/file, it's too big