From d1c279ff5738e08c7733bdaddd442f8ec130aa94 Mon Sep 17 00:00:00 2001 From: Oscar Wallberg Date: Tue, 26 May 2026 21:47:43 +0200 Subject: [PATCH] feat(claude): add simplify skill --- .claude/skills/simplify/SKILL.md | 165 +++++++++++++++++++++++++++++++ 1 file changed, 165 insertions(+) create mode 100644 .claude/skills/simplify/SKILL.md diff --git a/.claude/skills/simplify/SKILL.md b/.claude/skills/simplify/SKILL.md new file mode 100644 index 0000000..0c53c91 --- /dev/null +++ b/.claude/skills/simplify/SKILL.md @@ -0,0 +1,165 @@ +--- +name: simplify +description: Daily-driver review of in-flight changes (or specified files) for reuse, simplification, efficiency, and high-confidence bugs. Flags code that duplicates existing helpers, overengineering, dead code, hot-path bloat, premature abstraction, real correctness issues, and unnecessary comments or tests. Default mode diffs the working tree and staged changes against the main branch. Pass paths (`/simplify src/foo.rs`) to review file state instead. Use as the fast pre-PR review pass. For a deeper whole-tree retrospective, use /audit. +--- + +# /simplify + +Simplification review of work-in-progress. Default runs against the current diff. Explicit paths switch to file-state review. + +## Invocation + +``` +/simplify # diff working tree + staged vs main/master +/simplify --staged # only staged changes +/simplify --branch develop # diff against a different base +/simplify src/foo.rs # review one file as-is (ignores diff) +/simplify src/foo/ src/bar.rs # review multiple paths as-is +``` + +Paths win: if any are given, ignore the diff and review file state. + +## Gathering context + +Diff mode: + +1. Resolve the base branch. Try `git symbolic-ref refs/remotes/origin/HEAD`, then `main`, then `master`. Ask the user if neither exists. +2. With `--branch `, use that base instead. +3. With `--staged`, use `git diff --cached `. Otherwise include working tree (`git diff ...HEAD` plus uncommitted). +4. Read each touched file fully, not just the hunks. Context outside the diff is needed to spot duplication and existing helpers. + +File mode: read each passed path (recurse into directories, skip generated and vendored trees). + +Always: read `CLAUDE.md` (root and any in touched dirs) and `TODO.md` / `BACKLOG.md` if present. Honor local conventions and do not re-raise deferred items. + +## What to flag + +### Reuse (look here first) + +- New helpers or inline logic that duplicates an existing utility in this codebase. Scan the file's directory and shared locations (`utils/`, `lib/`, `helpers/`, modules adjacent to the changed ones) before concluding nothing exists. +- Reinvented standard library or framework primitives (manual path joining, ad-hoc env var parsing, custom type guards, hand-rolled string manipulation, bespoke deep-equals, etc.) where the language or an in-repo helper already covers it. +- Inconsistent import paths for the same thing (some sites use a top-level re-export, others reach into deep paths). Pick the path the rest of the codebase uses. +- For each duplication finding: name the existing helper with `file:line` so the fix is concrete, not abstract. + +### Overengineering + +- New helpers, traits, generics, or indirection introduced for one caller. Inline. +- Flags, parameters, or fields with only one realistic value or use site. +- Layers added speculatively without a current second caller. + +### Dead code in the same change + +- Functions added but uncalled elsewhere in the diff. +- Branches that cannot be reached given the rest of the change. +- New types, fields, or variants that nothing constructs. + +### Bugs (high-confidence only) + +Flag only when the issue is clearly wrong on inspection. "This might fail under some inputs" is not a finding. + +- Logic errors: off-by-one, wrong condition in `if`, inverted comparison, incorrect loop termination, swapped arguments. +- Missing bounds checks where the input is not otherwise validated. +- Type confusion that compiles but is wrong (mixing units, conflating IDs, using one enum where another was meant, comparing values of different shapes). +- Null or undefined access in JS/TS where the type allowed it but the path is reachable. +- Async lifetime mistakes: missing `await`, awaiting the wrong promise, dropping a future, fire-and-forget where the result was meant to be used. +- Rust borrow or lifetime patterns that compile but violate the intended invariant (lifetimes too permissive, `Send`/`Sync` boundaries crossed by accident). +- Resource leaks: opened but never closed, listeners attached but never removed on the failure path. + +### Efficiency + +Focus on hot paths (per-render, per-request, per-frame, per-record) and on patterns that get worse with scale. + +- Redundant work: duplicate computations, repeated file reads, redundant API or network calls, N-plus-one query patterns (one query per item where one batch query would do). +- Independent operations run sequentially that could run in parallel. +- New blocking work added to startup or per-render/per-request paths that did not have it before. +- State updates inside polling loops or event handlers that fire even when nothing changed. Add change-detection so downstream consumers are not woken for no-ops. If a setter takes an updater callback, verify same-value returns are honored so callers' own no-op guards are not silently defeated. +- Pre-checking a file or resource for existence before operating on it. This creates a race where the resource can change between the check and the use. Operate directly and handle the error from the operation instead. +- Unbounded data structures, missing cleanup, leaked event listeners or timers. +- Operations broader than they need to be (reading entire files when only a portion is needed, loading all rows when filtering for one, scanning a whole collection to find a single item where an index already exists). + +### Premature flexibility + +- `Option` where the value is always `Some`. +- Trait bounds or generic parameters wider than the actual use. +- Enum variants for cases that do not yet exist. +- `impl Trait` where the concrete type would read just as clearly. + +### Comments to delete + +- Narration of what the code does (`// increments counter`). +- Section dividers (`// ---`, `// === FOO ===`). +- References to the task or commit that prompted the change ("added for X", "used by Y", "fixes #123"). +- Stale docs where the code has moved past them. + +### Tests not pulling weight + +- Tests that assert the language or framework's behavior rather than the new logic. +- Tests duplicated across files with no extra coverage. +- Fixtures wrapping a one-liner for a single test. +- Mocked tests where an integration test already covers the same path. + +### Scope creep + +- Unrelated formatting changes mixed in with functional changes. +- Drive-by refactors in files outside the stated goal. +- "While I'm here" cleanup that belongs in a separate commit. + +### Style-rule violations (from the user's global CLAUDE.md) + +Flag these whenever they appear in the diff or files, no preamble required: + +- Em-dashes, en-dashes, `--` as em-dash substitutes, and semicolons in prose (comments, docs, commit messages, PR descriptions). +- Programming-culture acronyms (YAGNI, DRY, KISS, SOLID, TDD, BDD, DDD, MVP, NBO, TOCTOU) in any prose. Replace with what they actually mean. +- Section divider comments. +- Milestone references in code docs ("for M2", "added in M1"). +- `Co-Authored-By: Claude` trailers in commit messages. + +Fold in any project-local `CLAUDE.md` conventions on top of these. + +## What NOT to flag + +- Low-confidence speculation. "This might break under some inputs" is not a bug finding. "This could be faster in theory" is not an efficiency finding. If you cannot point at a concrete failure mode or a concrete cost, leave it out. +- Issues a linter or formatter already catches. +- Missing tests or coverage gaps (separate concern, not a simplification). +- "You could rewrite this with library X" or other broad architectural rewrites. +- Items already in `TODO.md` / `BACKLOG.md`. +- Performance theatre: micro-optimizations outside hot paths. + +## Reporting + +Single pass, all findings together. No tiered ceremony. + +``` +## Simplification pass + + findings on . + +1. path/to/file.rs:42-58 - . + Fix: . +2. path/to/file.rs:88 - . + Fix: . + +Apply all? "Go ahead", or tell me which to skip. +``` + +If a finding touches public API or changes behavior beyond cleanup, surface it as **Needs discussion** with two options rather than dropping it in the auto-apply list. + +If nothing is worth flagging, say "Nothing flagged" in one line and stop. Do not pad. + +## Applying + +After approval: + +1. Apply only approved findings. +2. Diff mode: fold edits into the working tree. Do not commit unless the user asks, since they are mid-PR. +3. File mode (no in-flight diff): default to one commit per theme using Conventional Commits. +4. Run the project gate (`scripts/prepare.sh`, `pnpm check`, `cargo test`, equivalent) when applying multiple substantive changes. + +## Do not + +- Rewrite history. The user is preparing a PR. Do not squash or amend existing commits. +- Auto-apply changes to public API surface. Lift those into **Needs discussion**. +- Touch files outside the diff (diff mode) or outside the passed paths (file mode). +- Reference the simplify pass in commit messages ("from /simplify", "simplification cleanup"). Describe the change, not how it surfaced. +- Surface a "bug" or "perf" finding you are not confident about. The signal value of this skill comes from the user trusting flagged bugs are real. False alarms erode that fast. +- Pad. If the diff is clean, "Nothing flagged" is the right answer.