--- name: cleanup description: Use when the user asks to clean up, lint, or review uncommitted code for common code smells — duplicate logic, magic numbers, unclear naming, dead code, style inconsistencies. Fixes issues without changing any runtime behavior. --- # Cleanup Review and fix code quality issues in the current working tree without altering any logic or behavior. ## When To Use - The user asks to clean up, tidy, or lint uncommitted changes - The user wants a code smell review before releasing or committing - The user mentions magic numbers, duplicate logic, dead code, or naming issues Do not refactor architecture, add features, or change behavior. ## Scope If the user specifies a file or directory, check only that. Otherwise check all uncommitted changes (`git diff HEAD`). Only report issues present in **newly added or modified** lines of this diff — do not audit unchanged code. ## Checklist ### 1. Duplicate Logic - Identical or near-identical code blocks appearing in multiple places - A function/helper that already exists but is re-implemented elsewhere instead of being reused - Repeated DOM queries, regex literals, or template strings within the same file ### 2. Magic Numbers / Magic Strings - Bare numeric literals used in calculations (offsets, timeouts, sizes, thresholds) without a named constant - Hardcoded strings (IDs, status values, URL fragments) scattered through logic - Exceptions: `0`, `1`, `-1`, `100`, `""` and other idiomatically clear values are fine ### 3. Naming Issues - Cryptic abbreviations (`or_`, `tmp2`, `x2`) - Names that do not match actual behavior - The same concept referred to by different names in different places ### 4. Dead Code - Commented-out code blocks (3+ lines) - Variables, parameters, or imports declared but never used - Branches that can never execute ### 5. Style Inconsistencies - Trailing whitespace - Mixed quote styles or indentation within the same file - Inconsistent blank-line usage (multiple consecutive blank lines, etc.) ### 6. Other - Private helper functions that should be exported but are not, causing callers to duplicate the implementation - Overly verbose conditions that can be simplified without changing logic ## Steps ### Step 1 — Get the file list ```bash git diff HEAD --name-only ``` Filter to the user-specified path if one was provided. ### Step 2 — Read and analyze each file Read the full file (not just the diff) with the Read tool. For each file, record every issue found: filename, line number, category, and suggested fix. ### Step 3 — Report findings before touching anything Print a structured list: ``` Found N issues: [file] js/foo.js · L34, L78: Duplicate logic — same DOM query implemented twice; extract to getPanel() · L91: Magic number — bare 14 used as pixel offset; name it TOOLTIP_OFFSET [file] js/bar.js · L12: Naming — variable `or_` is unclear; rename to outerR, outerG, outerB ... ``` If no issues are found, output "No code smells detected. Code quality looks good." and stop. ### Step 4 — Fix each issue Use the Edit tool for **minimal, targeted changes**: - **Duplicate logic**: extract to a shared constant or function; update all call sites - **Magic number/string**: declare `const NAME = value` near the top of the relevant scope; replace all usages - **Naming**: rename the variable/function; update all references - **Dead code**: delete it - **Trailing whitespace / style**: fix in place - **Unexported helper**: add `export`; update callers to import instead of re-implementing Principles: - Only fix issues identified in the checklist — no extra improvements - Keep each Edit as small as possible - After fixing, verify the old bad pattern is gone with grep ### Step 5 — Summary ``` Cleanup complete: Fixed N issues: ✓ earth.js — extracted duplicate vertexShader into ATMOS_VERTEX_SHADER constant ✓ main.js — extracted TOOLTIP_CURSOR_OFFSET = 14 (4 references updated) ✓ controls.js — exported updateLayerButtonState; removed duplicate implementation in main.js ... Skipped (needs manual review): ! foo.js L45 — large commented-out block; confirm it is safe to delete ``` ## Constraints - **Do not** change function signatures, exported interfaces, or public APIs (unless the issue is a missing export) - **Do not** add new features, abstractions, or parameters - **Do not** rewrite comments (only delete commented-out dead code) - **Do not** touch test file logic - If a magic number's intent is uncertain, skip it and flag it in the summary