/refactor-smeller — audit for refactoring debt

Surface technical debt that survives short-term review but compounds. The skill is adversarial — assume the reviewed code looks fine on first read but contains drift, fragility, or sibling defects that will fire later. Default to skepticism.

Usage

• /refactor-smeller — scan whole repo (warn if >10k files; ask user to scope)
• /refactor-smeller <path> — scan one file or directory
• /refactor-smeller --since <commit> — only code changed since <commit> (default: most recent merge to main)
• /refactor-smeller --category <name> — limit to one category (file-size, citations, siblings, tests, defensive, doc-drift, split-second-order, disclosure)

The discipline

Every finding must be actionable. No "consider refactoring" hedges. Each finding cites file:line, names the smell class, explains why it matters, and proposes ONE concrete remediation: split into N modules / extract helper X / delete dead code Y / add regression test for path Z / replace literal with grep-anchor.

If you can't propose a concrete remediation, don't emit the finding — it's noise.

Two failure modes to actively guard against:

1. Useless generic complaints: "this file is large" with no proposed seams. Always cite the seams (head-comments, // --- markers, pub/private boundaries, impl blocks) that justify a specific split.
2. The reviewer false-positive: just because a finding LOOKS plausible doesn't mean it's empirically true. If a finding rests on a code-path claim, grep to verify the claim before emitting. Reject your own finding if the grep shows otherwise.

Categories

1. File-size debt

• >1000 LoC = HIGH unless the file has a single coherent concern (e.g., a parser, a long enum impl, an event-list walker — file-size alone isn't the smell; lack-of-cohesion is).
• 800-1000 LoC = MEDIUM — monitor; flag only if multiple unrelated concerns share the file.
• Count significant LoC (exclude blank and pure comment lines). wc -l for the rough count, then a comment-stripping count for significant (adapt the comment marker: grep -cvE '^\s*(//|#|--|$)' <file>).
• Always cite seams when proposing a split. Look for:
  • file-level doc-comment paragraph breaks
  • banner/section dividers (// ---, # ===, /* --- */)
  • exported-symbol boundaries (the language's pub/export/capitalised/__all__ marker)
  • type-implementation blocks (impl, class, a receiver-method run)
  • inline sub-module or namespace markers

Example finding:

[FILE-SIZE-HIGH] src/backend/core.<ext>:1-1997
1997 LoC, 5 distinct concerns: public API (lines 1-134), plan emitter
(288-1499), encode helpers (1499-1564), relay machinery (1578-1947), file I/O
(1972-1997). Section seams already named in the head-comment.
REMEDIATION: split into plan/{events,relay,program} + encode + walkers along
the cited line boundaries.

2. Fragile citations

• Absolute line numbers in comments (see line 493) rot when the target file changes. Replace with symbol-name anchors (see parse_header()) or grep witnesses (grep -n "parse_header" <path>) that survive edits above them.
• Predictive claims (will be, pending cycle N, blocked on, TODO once X lands) age badly. After X lands, the comment doesn't auto-update. Reframe to past-tense (landed cycle N (commit abc1234)) or grep-witnessable invariants.
• Numeric drift (3 of 4 backends pass, the 88/70/0/18 baseline) — list-or-recipe instead. The number rots; the list+recipe doesn't.
• Stale cross-references — counterpart in <path> where that path no longer exists. Grep the cited path; if absent, the comment is a doc-lie.

Grep patterns (adapt the file globs to the project's languages; rg shown, plain grep -rn works too):

rg '(line [0-9]+|:[0-9]+\b)' --type-add 'src:*.{rs,py,go,ts,js,java,c,cc,h}' -t src
rg '\b(pending cycle|will be|TODO once|blocked on|in the next)\b' -t src
rg '\b[0-9]+ of [0-9]+ (backends|cells|files|cases)\b' -t src

3. Silent-sibling defects

When N sites encode the same logic separately, they drift. Detection:

• Parallel directories: sibling implementations of one interface (backends/<a>/ vs backends/<b>/, drivers/, adapters/) — functions with matching names should have matching shape. Diff the parallel files; any divergence is either documented-and-deliberate or a silent-sibling defect.
• Manual mirrors: pick the 3-5 function names that recur across the sibling implementations and search for the block via rg -A 10 '(<name1>|<name2>|<name3>)' — if every sibling open-codes the same logic, the helper extraction is overdue.
• Comment-vs-code mirrors: a doc-comment claiming foo() returns N where the signature returns something else.

The structural test: a silent-sibling defect is one where fixing one site WITHOUT fixing the others creates correctness drift. If fixing one suffices (e.g., independent implementations with genuinely different semantics), it's not a sibling defect.

Remediation always: extract shared helper, OR add a structural-check recipe to the project's task runner (just, make, npm run, a CI job) that fail-louds when the sites diverge.

4. Test discipline

• New code without nearby new tests: git log --name-only --since=... ∩ git log --name-only --since=... -- 'tests/' — gaps are suspect.
• Tests that don't bite: apply the BITE test — temporarily break the production code; the test should fail. If it doesn't, the test is worthless. Suggest bite-test targets; don't actually execute (would dirty the working tree).
• Single-caller functions: grep each definition's name and count call sites. Often inline candidates unless the name carries meaning.
• Zero-caller functions: dead code. Delete, not test.
• Happy-path-only tests: the function can fail (a Result, an exception, an error return, a nullable) but only the success branch is exercised. Flag.

5. Defensive-construct rot

Language-neutral classes, with the idiom each appears as:

• Unchecked assertions — an assertion whose message doesn't say WHY it cannot fail, leaving a useless failure context. Rust unwrap()/expect("") · Python bare assert · TS ! non-null · Go unchecked type assertion.
• Silently dropped errors — the error value is discarded with no comment justifying it. Rust let _ = result; · Go _ = err / _, _ = · JS empty catch {} · Python bare except: pass.
• Unjustified escape hatches — a construct that suspends the language's guarantees without a written justification next to it. Rust unsafe with no // SAFETY: · C casts away const · TS any / @ts-ignore · Python # type: ignore · a suppressed lint with no reason string.
• Type erasure at a boundary — a rich error or value collapsed into an opaque one at a module edge, where the caller could have acted on the distinction. Rust Box<dyn Error> / anyhow::Error · Java throws Exception · Go errors.New where a sentinel or typed error existed · Python raising bare Exception.
• Over-owned parameters — taking ownership or a full copy where a borrowed or read-only view would do. Rust Vec<u8> vs &[u8] · C++ by-value container vs const& · Python copying a list to read it. LOW priority.

6. Doc-vs-code drift

• Run the project's docgen and read its warnings (cargo doc, pydoc/Sphinx, godoc, typedoc, javadoc — whichever applies).
• Doc-link references to symbols (Rust [`foo`], Javadoc {@link foo}, Sphinx :func:) — grep each referenced name in the current tree. If absent, it's a stale ref.
• README claims vs implementation: spot-check 3 random claims; verify against code.
• Signature drift: a doc-comment saying "returns Foo" where the signature returns Bar.

7. Split-induced second-order drift (HIGH-VALUE category)

When a file gets split, two patterns recur:

Module-doc promotion: an inline comment becomes a file- or module-level docstring (Rust //!, Python module docstring, a package-level doc-comment). Its visibility jumps — it now appears in the generated docs — so its staleness becomes MORE consequential. Every split should:

• Grep the moved sub-files for predictive claims: rg '(will be|pending|TODO once|deferred)' <new-dir>/
• Reframe predictive claims to past-tense before commit.

Citations FROM other files going stale: when foo.<ext> becomes foo/{a,b,c}.<ext>, every other file in the tree that cites foo.<ext>:NNN is now stale.

• Grep: rg "foo\.<ext>:[0-9]+"
• Update each citation to reference the new location.

This category is HIGH-VALUE because it's invisible during the split itself — the splitter sees only the moved file. A post-split sweep is mandatory.

8. Enumerated-disclosure discipline

When reviewing or producing a refactor commit, reject "verbatim move" claims that don't enumerate every mechanical change:

• "moved foo() to new/file" — fine
• "moved foo() to new/file and aligned indentation while there" — also fine
• "verbatim move; no functional change" — RED FLAG when the diff is >100 lines. Verify by reading; undisclosed fixes are silent-sibling-defect carriers.

The smell: enumerate-positive (a)/(b)/(c)/... is the discipline. Anything that isn't enumerated probably wasn't intentional.

Output shape

## refactor-smeller findings (N items, prioritized)

Repo scope: <path>
Total LoC scanned: <n>; files: <m>; commits since `<since-ref>`: <k>

### HIGH (N)
<numbered findings, each with file:line, category tag, why-it-matters one-liner, concrete REMEDIATION>

### MEDIUM (N)
...

### LOW (N)
...

### Cross-cutting observations
<patterns spanning multiple findings — e.g., "5 of the 11 high-priority findings
sit under one directory, suggesting that module's interface should be tightened">

## Limitations
- Test-bite detection is heuristic. Final verification requires actually running the bite-test in a clean working tree.
- Doc-vs-code drift detection requires running the docgen tool, which the skill DOES NOT do automatically (output too noisy to read in a slash-command). Spot-check 3-5 doc claims; suggest running full docgen separately.
- File-size threshold is heuristic — don't flag cohesive long files (parsers, generated code) just for size.
- The skill does NOT replace a human code reviewer for semantic / domain-specific concerns.

Caveats and meta-discipline

The recipe meta-pattern-lock: this skill is itself a pattern-detector. It CAN BE pattern-locked. Specifically:

• If you find yourself only flagging the categories above without spotting new patterns, the skill has become a checklist rather than a critical eye. Always add an open-ended "other concerns" sweep to each invocation — scan for anything the categories don't cover.
• If a category's grep pattern is too narrow, it'll miss its own target. Re-evaluate patterns periodically. A real instance: a doc-lie linter shipped with an allow-annotated line that literally contained the forbidden word — the recipe missed its own ALLOW.

The freshly-fixed-class rule: right after fixing a defect class, the next refactor is the HIGHEST-RISK CYCLE for the same class (pattern fresh in the orchestrator's head + surface partially refactored). When invoked immediately after a hygiene cycle, scan extra-carefully for siblings of the just-fixed defect.

Reject your own false-positives: if a finding rests on a structural claim, grep to verify. If the grep contradicts, drop the finding. Better to emit N-1 strong findings than N with one wrong.

Don't auto-fix: this skill outputs findings, not commits. The user/reviewer evaluates which to act on. Auto-fix would compound the silent-sibling-defect risk (now the fix is silent too).

Hard-earned patterns from prior projects

The following patterns came from extended sessions on real refactor work. Each is a real recurring defect class — bake into the skill's adversarial mindset:

• Predictive-claim hostage: comments naming future cycles age badly once the future cycle lands. Always past-tense with citation.
• Effort-estimate overstated: refactors estimated as "small" routinely take 2-3x longer because review-gate findings compound. Don't trust the original framing of any refactor; size empirically.
• Shell-dialect assumption in build recipes: task-runner recipes (just, make, CI run: steps) usually execute under /bin/sh, not bash. Bash-only idioms — <(...) process substitution, [[ ... ]] tests, arrays — fail silently or at runtime only. Flag them wherever the runner's shell isn't pinned explicitly.
• Lint regressions that only fire after a split: moving code across file boundaries changes what the compiler and linter can infer, so a clean file can fail lint purely by being relocated. (Rust: methods that no longer reference a lifetime need Foo<'_> or clippy's needless_lifetime fires. Equivalents exist elsewhere — unused-import and cyclic-import errors surface the same way.) Re-run the linter after every split, not just the tests.
• Visibility over-widening: exporting a symbol "just in case" during a split is a smell — the split is the moment it happens, since a moved helper suddenly needs to be reachable. Default to the narrowest visibility the language offers and widen only on an actual call site, with a one-line justification.
• Deferred-not-cancelled: filed-but-deferred follow-ups should be marked with a re-evaluation trigger. Otherwise the same defect gets re-filed when re-encountered.
• Sub-task nesting depth: tracker entries deeper than 3 levels (TASK-NNNN.MM.PP) often signal the parent should have been a flat list of independent tasks with depends-on edges.
• Lesson-recorded-but-not-applied-to-own-neighborhood: writing a lesson in commit/tracker notes without applying it to the code in the SAME commit. Architect-class catch.