cleanup-stale-code

name

cleanup-stale-code

description

Use when the user asks to clean up stale, outdated, low-value, or migration-era comments OR expired/deprecated/over-coupled test cases in a codebase. Triggers on phrases like "stale comments", "comment cleanup", "remove migration notes", "test cleanup", "delete test cases", "evaluate tests", "remove dead tests". Skip for adding new doc comments, improving test coverage, or auditing a single file manually.

Cleanup Stale Code

Overview

Two-phase workflow for cleaning two related kinds of codebase drift: comments (stale migration notes, low-value narration, expired bridge comments) and tests (over-coupled assertions, mock-path style debt, deprecated-API regression tests). Both share the same shape: produce a prioritized inventory, then batch-execute by priority. The key insight is that analysis and execution are separate steps with different failure modes — do not merge them.

When to Use

• "stale comments / comment cleanup / remove migration notes" — see Section A
• "test cleanup / delete test cases / evaluate tests / remove dead tests" — see Section B
• A codebase has accumulated phased implementation markers, migration notes, obvious-code comments, or failing tests, deprecated-API regression tests, and mock-path style debt over many iterations

When NOT to Use

• Adding new doc comments or improving comment style
• Adding new test coverage
• Auditing one specific file manually
• Change is ≤3 lines (do it inline)

General Workflow (Both Sections)

Phase 1: Inventory    →  prioritized table (do not edit files)
Phase 2: Execute       →  batched edits per priority, verify with tests/lint/typecheck

Do not skip Phase 1 even if "it looks obvious." The inventory forces a category decision per finding, which is where the work actually happens.

---

Section A: Cleaning Comments

Step A1 — Use the user's categories

If the user named specific categories (e.g. "expired, deprecated, inaccurate, low-value, migration-era"), use exactly those. Do not invent additional categories. If they didn't name any, default to: expired, deprecated, low-value, migration-era.

Step A2 — Search with anchors, not full reads

Anchor keywords that signal a stale comment (adapt to your project's language and idiom):

• Phased implementation markers: Phase [0-9], Step [0-9], Task [0-9]
• Deprecation signals: @deprecated, DEPRECATED, obsolete, removed
• In-progress markers: TODO, FIXME, HACK, XXX, stub, noop, not implemented, TBD, WIP
• Temporal hedging: previously, formerly, for now, temporarily, will be, coming soon
• Versioning notes: legacy, old, v1, v2 reserved
• Placeholders in any language: words like "placeholder", "TODO", "TBD", "for now", "coming soon", or whatever the project's placeholder vocabulary is

Use whatever text-search tool your environment provides (ripgrep, grep, IDE search). Read ±5 lines around each match. Do not full-read every file — it burns time with diminishing returns and finds off-scope issues.

Step A3 — Output a prioritized table

For each finding: file:line, comment text, category, one-line reasoning, action (remove / rewrite 1-line / keep).

Priority	Criterion	Examples
P0	Migration markers whose work is complete; expired bridge comments	All Phase 0: instances; "kept here for unmigrated callers" notes
P1	Low-value obvious-code comments, visual dividers, history section markers	// skip: any true; // --- migrated tags ---
P2	Verbose doc comments / multi-line explanations reducible to 1 line	(absent in legacy archives) notes; 5-line migration explanations
P3	Valid historical context — keep	Bug-fix rationale documenting current invariants

A finding is P0 only if removing it is unambiguously correct. If you need to think, it's P1 or lower.

Step A4 — Edit, don't rewrite

• Remove entirely if the comment adds nothing beyond the code
• Rewrite to 1 line if the underlying intent is still useful (e.g. // Phase 0: push to pluginTools → // cache tools registered via plugin API)
• Leave alone if uncertain

Match the existing style of the file. Never "improve" adjacent comments.

Step A5 — Verify

After edits, run whatever your project's standard checks are (formatter / linter / type checker / build). All must pass.

Step A6 — Check for dead code revealed by the cleanup

Phase markers often label code that was transitional. After removing the markers, search for whether the labeled function/variable still has callers:

<text-search> "functionName" <source-dirs>

If a previously-bridged function has zero callers, delete it. Cleanup often unlocks a second cleanup.

---

Section B: Cleaning Tests

Step B1 — Use the user's categories

If the user named specific categories (e.g. "expired, deprecated, low-value, useless"), use exactly those. Do not invent "over-coupled" or "mock-path debt" as new categories unless asked. If they didn't name any, use the default set:

• Expired: asserts behavior that no longer exists, or behavior the product never explicitly guaranteed
• Deprecated: tests an API marked @deprecated in source — usually a regression test for an already-removed feature
• Low-value: tautological assertions, or tests that exist only to test the mock itself
• Useless: skipped, commented-out, references non-existent files (e.g. mocking a module path that no source file matches)

Step B2 — Establish a baseline (Phase 1)

Always run the test suite first — this is a Phase 1 read-only step, not an edit. Use whatever runner your project uses. Record: total files, total tests, passing, failing, skipped, pass rate. The baseline tells you:

• If some tests are already failing, those are P0 candidates (the source has drifted)
• If some tests are skipped, those are P3 candidates (likely dead)
• The pass rate is the metric you report back to the user

Step B3 — Search with anchors

Adapt the patterns below to whatever mock / assertion / skip syntax your test framework uses:

• Tests of deprecated APIs: text search for @deprecated / DEPRECATED symbols inside test files
• Style debt in mocks: search for mock paths that use a non-canonical file extension (e.g. a reference using one extension when the project's source files use another)
• Skipped or commented-out tests: search for the framework's skip mechanism (skip decorators, commented-out test blocks, disabled test annotations, etc.)
• References to non-existent files: search for imports/mocks that point at paths with no matching source file

Step B4 — Output a prioritized table

Priority	Criterion	Examples
P0	Currently failing tests, or tests asserting behavior the product never explicitly guaranteed	A multi-turn test failing because it asserts session reuse, when the source never promised that
P1	Deprecated-API regression tests (tests for already-removed features)	Tests asserting that a @deprecated symbol throws when called
P1	Mock-path style debt (mechanical fixes, no behavior change)	Many mock paths using an extension that doesn't match the project's source extension convention
P2	Tests of low-value targets (test-the-mock patterns, tautological assertions)	expect(mock).toBe(mock) patterns
P3	Skipped tests that may be removed vs. moved to a separate suite	Performance benchmarks left in regular suite

Important: the user's stated starting point often is "I want to delete things, not mark them." If the user said the goal is to delete some cleanable test cases, treat P0-P1 as deletion candidates by default, not as "fix and keep" candidates. Confirm before rewriting tests for behavior the product never explicitly guaranteed.

Step B5 — Execute in batches

Within a priority batch, parallelize edits across files. For deletions, prefer removing the entire test block over editing its body.

For mock-path fixes (P1 style debt), a bulk find-and-replace is usually safe if the pattern is uniform. Verify with the project's linter/formatter after.

Step B6 — Verify

Re-run the test suite, linter, and type checker. The test count after cleanup must be ≤ the test count before. If it went up, you accidentally added tests.

Step B7 — Look for source-side dead code revealed

After deleting a test for a deprecated API, check whether the API itself is still in source:

<text-search> "deprecatedSymbol" <source-dirs>

If the deprecated API has zero callers in source AND zero tests, propose removing it from the source as a follow-up — but only as a proposal, not as part of this task's scope.

Step B8 — Common test-cleanup side task: exclude benchmarks

If benchmark files (e.g. files matching *.bench.* or a benchmarks/ directory) are skipped by default and pollute CI output, move them out of the regular test-runner include pattern. Don't delete the file — keep it as a manually-runnable benchmark.

---

Common Mistakes (Both Sections)

Mistake	Fix
Inventing extra categories the user didn't ask for	Stick to the user's set; ask if unclear
Full-reading every file to "be thorough"	Anchor greps + ±5 line context
Merging analysis and editing	Always: inventory first, then execute
Keeping Phase 0: while keeping the explanation text	Pick one: delete the marker line, or rewrite the whole thing to 1 line
Preserving obvious-code comments "they might help a junior"	They won't. Delete.
Doc comment that translates a function name (/** Get the platform */ above getCurrentPlatform) is a translation, not docs	Delete
"Fixing" a test instead of deleting it when the product never promised the behavior	Delete; the test was over-coupled to an internal detail
Adding new tests as part of a cleanup task	Cleanup is subtractive; adding tests is a separate task
Treating "skipped" tests as not-your-problem	Skipped tests pollute CI output; either move them or delete them

Anti-Patterns — Reject These

• "Let me also clean up the doc comment translations in another module" → out of scope unless user asked
• "I noticed <file>:<line> has a real bug" → surface as a separate finding; don't expand scope
• "I'll do a comprehensive sweep to be safe" → user asked for X, do X
• "Adding a P4 tier for human-judgment items" → if user said 3 tiers, give 3 tiers
• "Let me also fix the test assertion to match the new behavior" → if the product never promised the behavior, the test is wrong, not the source. Delete.
• "I'll move the skipped benchmark to a separate suite AND remove its skip wrapper" → that's two changes; do one, verify, then do the other

Common Workflow Endings

After Phase 2, report:

• Comments: file count, line count delta (always negative), lint + typecheck status
• Tests: test count delta (always negative), test pass rate, lint + typecheck status, follow-up source-side cleanup opportunities surfaced (e.g. "now zero callers + zero tests, suggest removing <deprecated-symbol> from source")

Then ask if the user wants to commit. Do not auto-commit — cleanup is a review-heavy task and the user often wants to adjust the diff before it lands.

See also

• find-duplication — for finding duplicated code (functions, blocks, fixtures) revealed after the comment/test cleanup, or that the cleanup made more obvious
• consolidate-imports — for auditing the import statements in the files you've just cleaned up; the report will surface any import-style drift that would otherwise pollute the next round of edits
• Typical sequence: cleanup-stale-code first (subtract drift), then consolidate-imports (audit imports before refactors), then find-duplication (extract shared helpers)