Cleanup stale code

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)