ia-simplifying-code

SKILL.md

---
name: ia-simplifying-code
class: discipline
description: >-
  Simplifies, polishes, and declutters code without changing behavior. Use when
  asked to simplify, clean up, refactor, declutter, remove dead code or AI slop,
  or improve readability. For analysis-only reports without code changes, use
  code-simplicity-reviewer agent.
---

# Simplifying Code

## Principles

| Principle | Rule |
|-----------|------|
| **Preserve behavior** | Output must do exactly what the input did -- no silent feature additions or removals. Specifically preserve: async/sync boundaries (do not convert sync to async or reverse), error propagation paths (do not alter strategy), logging/telemetry/guards/retries that encode operational intent, and domain-specific steps (do not collapse into generic helpers that hide intent) |
| **Explicit over clever** | Prefer explicit variables over nested expressions. Readable beats compact |
| **Simplicity over cleanliness** | Prefer straightforward code over pattern-heavy "clean" code. Three similar lines beat a premature abstraction |
| **Surgical changes** | Touch only what needs simplifying. Match existing style, naming conventions, and formatting of the surrounding code |
| **Surface assumptions** | Before changing a block, identify what imports it, what it imports, and what tests cover it. Edit dependents in the same pass |

## Process

1. **Read first** -- understand the full file and its dependents before changing anything. Apply Chesterton's Fence: if you see code that looks unnecessary but don't understand why it's there, check `git blame` before removing it. First understand the reason, then decide if the reason still applies.
2. **Identify invariants** -- what must stay the same? Public API, return types, side effects, error behavior
3. **Identify targets** -- find the highest-impact simplification opportunities. Impact = readability and maintainability; prioritize: control flow -> naming -> duplication -> types (see Smell -> Fix table)
4. **Apply in order** -- control flow → naming → duplication → data shaping → types. Structural changes first, cosmetic last
5. **Verify** -- confirm no behavior change: tests pass, types check, imports resolve
6. **Pre-submit scope audit** -- walk every changed line and ask "does the requested task explicitly require this line?" If no, revert it and list it as a follow-up under Residual Risks. Drive-by edits belong in a separate change, not the current patch. For the pre-edit complement on ambiguous-scope requests ("simplify my project"), see `ia-verification-before-completion`'s Scope Confirmation gate.

## Smell → Fix

| Smell | Fix |
|-------|-----|
| Deep nesting (>2 levels) | Guard clauses with early returns |
| Long function (>20 lines) | Extract into named functions by responsibility |
| Too many parameters (>3) | Group into an options/config object |
| Duplicated block (**3+** occurrences) | Extract shared function. Two copies = leave inline; wait for the third |
| Magic numbers/strings | Named constants |
| Complex conditional | Extract to descriptively-named boolean or function |
| Boolean-returning `if/else` (each branch returns a literal `True`/`False`) | Collapse to the boolean expression itself: `return a and b`, not a branch per literal |
| Dense transform chain (3+ chained methods) | Break into named intermediates for debuggability |
| Dead code / unreachable branches | Delete entirely -- no commented-out code |
| Unnecessary `else` after return | Remove `else`, dedent |

## AI Slop Removal

When simplifying AI-generated code, specifically target:

- **Redundant comments** that restate the code (`// increment counter` above `counter++`) -- delete them
- **Unnecessary defensive checks** for conditions that cannot occur in context -- remove the guard
- **Gratuitous type casts** (`as any`, `as unknown as T`) -- fix the actual type or use a proper generic
- **Over-abstraction** (factory for 2 objects, wrapper around a single call, util file with 1 function) -- inline the code
- **Inconsistent style** that drifts from the file's existing conventions -- match the file
- **Placeholder stubs** (`// ...`, `// rest of code`, `// similar to above`, `// continue pattern`, `// add more as needed`) -- leave unsimplified code as-is rather than replacing it with stubs
- **Redundant error wrapping** (`catch(e) { throw e; }`, `catch(e) { throw new Error(e.message); }`) that strips the original stack for no reason -- remove the try/catch entirely and let errors propagate
- **Verbose stdlib reimplementations** (hand-rolled loops that replicate `array_filter`, `Array.from`, `Collection::pluck()`, `itertools`) -- replace with the stdlib/framework one-liner, but verify edge-case parity first: empty input, null/None guard, no-match default, zero-value path. The one-liner can silently differ from the loop (an empty-input crash, a missing no-match default, lost ordering) -- a structurally cleaner version that changes behavior on an edge case is not a simplification

## Stop Conditions

Stop and ask before proceeding when:
- Simplification requires changing a public API (function signatures, return types, exports)
- Behavior parity cannot be verified (no tests exist and behavior is non-obvious)
- Code is intentionally complex for domain reasons (performance-critical, protocol compliance)
- Scope implies a redesign rather than a simplification

## Constraints

- Only simplify what was requested -- do not add features, expand scope, or introduce new dependencies
- Leave unchanged code untouched -- do not add comments, docstrings, or type annotations to lines that were not simplified
- Do not bundle unrelated cleanups into one patch -- each simplification should be a coherent, reviewable unit
- Do not introduce framework-wide patterns while simplifying a small local change
- Do not replace understandable duplication with opaque utility layers -- three similar lines are better than a premature abstraction
- Keep comments that explain intent, invariants, or non-obvious constraints. Remove comments that restate obvious code behavior.
- If a simplification would make the code harder to understand, skip it
- Watch for over-simplification: inlining too aggressively removes names that gave concepts meaning; combining unrelated logic into one function hides distinct responsibilities; removing abstractions that exist for testability breaks the test suite
- When unsure whether a block is dead code, ask instead of deleting

## Verify

- Tests pass and types check after changes
- No behavior change (same inputs produce same outputs)
- Scope limited to requested files -- no drive-by cleanups
- Match test scope to the importer count surfaced in step 1 (Surface assumptions). Zero external importers: scoped tests on the changed paths. One or more external importers, or shared/utility code edited: run tests covering each importer. Run the full suite when the test runner has no path-scoping mechanism.

## Orchestrator Mode (When Chained With Other Skills)

When this skill is invoked by an orchestrator that also runs `ia-code-review`, `ia-writing-tests`, or `ia-verification-before-completion` on the same scope, each sub-skill re-resolving scope independently wastes tokens and risks drift. Avoid this by resolving scope exactly once and passing a canonical block to every sub-skill.

**Resolved scope format** — the orchestrator builds this once, before dispatching any sub-skill:

```
## Resolved scope
Files:
- path/to/file-a.ts
- path/to/file-b.ts

Commit range: HEAD~3..HEAD (or "uncommitted")

Intent: [one-sentence description pulled from the user request or PR description]

Constraints:
- Preserve public API
- No behavior change
- [other constraints specific to this run]
```

Every chained sub-skill receives this block verbatim in its prompt and uses it as the source of truth — no re-running `git diff --name-only`, no re-parsing the user request, no independent scope resolution. Sub-skills accept `--no-verify --no-report` flags when chained so verification and reporting happen once at the end of the chain, not per-skill. The last sub-skill in the chain runs verification; the orchestrator trusts that result rather than re-verifying.

This prevents two failure modes: scope drift (sub-skill A simplifies one set of files, sub-skill B reviews a different set) and double work (every sub-skill rediscovers the same facts).

## Integration

- `ia-code-simplicity-reviewer` agent -- analysis-only pass producing a simplification report (no code changes). Use before refactoring to identify targets.

## Output

After simplifying, report:
- **Scope touched**: files and functions modified
- **Key simplifications**: what changed and why (one line each)
- **Verification**: tests pass, types check, no behavior change
- **Residual risks**: assumptions made, areas not touched that may need attention