cleanup
This skill should be used when the user asks to "clean up the code", "remove unnecessary comments", "simplify docstrings", "parameterize tests", or wants to review and clean up code.
SKILL.md
| Name | cleanup |
| Description | This skill should be used when the user asks to "clean up the code", "remove unnecessary comments", "simplify docstrings", "parameterize tests", or wants to review and clean up code. |
name: cleanup description: This skill should be used when the user asks to "clean up the code", "remove unnecessary comments", "simplify docstrings", "parameterize tests", or wants to review and clean up code. user-invocable: true
Cleanup Code
Review and clean up code, removing unwanted patterns and improving test structure.
Scope
By default, only clean up code that was changed on the current branch (i.e., changes not yet on main). Use git diff main...HEAD to identify which files and lines were modified.
Do NOT clean up anti-patterns in code that was already on main unless the user explicitly requests a broader cleanup scope.
What This Skill Does
- Removes unnecessary comments
- Simplifies or removes verbose docstrings
- Consolidates tests using parameterization
- Removes other unwanted patterns
Crosscheck Against CLAUDE.md
Before applying the hardcoded patterns below, also check the changed code against rules from:
- Project CLAUDE.md: Read the repo's
CLAUDE.md(if it exists) for project-specific style rules, conventions, and patterns - Global CLAUDE.md: Read
~/.claude/CLAUDE.md(if it exists) for global coding style rules
Scan the changed code for violations of any rules in these files. Common things to catch:
- Import style violations (e.g., importing individual items instead of modules)
- Testing anti-patterns (e.g., using classes to group tests, defensive
.get()in assertions, mocking internals) - Python style violations (e.g., for-loop accumulators instead of comprehensions, if-else blocks instead of ternary for simple returns)
- Error handling violations (e.g., logging warnings instead of failing early)
- Any other project-specific or global conventions defined in these files
CLAUDE.md rules take equal priority to the hardcoded patterns below — violations of CLAUDE.md rules are just as important to fix.
Cleanup Tasks
1. Remove Unnecessary Comments
Remove explanatory comments that state the obvious or describe what code does line-by-line. Remove comments that:
- Restate what the code already clearly expresses
- Explain standard library functions or well-known patterns
- Were added "for clarity" but add no value
- Describe obvious variable assignments or function calls
Keep comments that:
- Explain why something is done (business logic, workarounds, edge cases)
- Document non-obvious behavior or gotchas
- Provide context that can't be inferred from the code
Example - Before:
# Get the user from the database
user = db.get_user(user_id)
# Check if user exists
if user is None:
# Raise an error if user not found
raise UserNotFoundError(user_id)
# Return the user's email address
return user.email
Example - After:
user = db.get_user(user_id)
if user is None:
raise UserNotFoundError(user_id)
return user.email
2. Simplify Docstrings
Remove verbose docstrings that repeat parameter names and types already visible in type annotations, or describe obvious behavior. Clean up docstrings by:
- Removing docstrings from simple, self-explanatory functions
- Removing Args/Returns sections when types are annotated and obvious
- Keeping only non-obvious information
- Removing filler phrases like "This function..." or "This method..."
Example - Before:
def get_user_email(user_id: int) -> str:
"""
Get the email address for a user.
This function retrieves the email address associated with the given user ID
from the database.
Args:
user_id: The unique identifier of the user whose email should be retrieved.
Returns:
The email address of the user as a string.
Raises:
UserNotFoundError: If no user exists with the given ID.
"""
user = db.get_user(user_id)
if user is None:
raise UserNotFoundError(user_id)
return user.email
Example - After:
def get_user_email(user_id: int) -> str:
"""Raises UserNotFoundError if user doesn't exist."""
user = db.get_user(user_id)
if user is None:
raise UserNotFoundError(user_id)
return user.email
Or if the function name and signature are completely self-explanatory:
def get_user_email(user_id: int) -> str:
user = db.get_user(user_id)
if user is None:
raise UserNotFoundError(user_id)
return user.email
3. Parameterize Tests
Consolidate separate test functions that should be parameterized. Look for:
- Multiple test functions with nearly identical structure
- Tests that differ only in input values and expected outputs
- Copy-paste test patterns with minor variations
Consolidate using @pytest.mark.parametrize:
Example - Before:
def test_validate_email_valid():
assert validate_email("user@example.com") is True
def test_validate_email_valid_with_subdomain():
assert validate_email("user@mail.example.com") is True
def test_validate_email_invalid_no_at():
assert validate_email("userexample.com") is False
def test_validate_email_invalid_no_domain():
assert validate_email("user@") is False
def test_validate_email_invalid_empty():
assert validate_email("") is False
Example - After:
@pytest.mark.parametrize(
("email", "expected"),
[
("user@example.com", True),
("user@mail.example.com", True),
("userexample.com", False),
("user@", False),
("", False),
],
)
def test_validate_email(email: str, expected: bool):
assert validate_email(email) is expected
When NOT to parameterize:
- Tests with significantly different setup/teardown
- Tests that check different aspects of behavior (not just input/output variations)
- Tests where parameterization would obscure the intent
4. Remove Defensive Code Patterns
HIGHEST PRIORITY: Remove unnecessary defensive programming that hides bugs instead of failing loudly.
Defensive .get() Calls
Replace .get() with default values on internal data structures with direct access. Let KeyError propagate.
Before:
user_id = data.get("user_id", None)
if user_id is None:
return
After:
user_id = data["user_id"] # Let KeyError propagate if missing
Unnecessary Try-Except Blocks
Remove try-except blocks that catch exceptions without specific handling logic.
Before:
try:
result = parse_data(input)
except Exception as e:
logger.warning(f"Parse failed: {e}")
result = None
After:
result = parse_data(input) # Let exceptions propagate
Unnecessary None Checks
Remove None checks on fields that should always exist in internal data structures.
Before:
if score.metadata is not None and "error" in score.metadata:
error = score.metadata.get("error", "unknown")
After:
if "error" in score.metadata:
error = score.metadata["error"]
Validating Internal Invariants
Remove validation that checks internal invariants. Trust internal code.
Before:
def process_user(user: User) -> str:
if not isinstance(user, User):
raise TypeError("Expected User instance")
if user.email is None:
raise ValueError("User email is required")
return user.email
After:
def process_user(user: User) -> str:
return user.email
5. Fix Type Checking Issues
Replace type: ignore comments with proper type narrowing.
Before:
result = some_function() # type: ignore
After (Option 1 - Assert isinstance):
result = some_function()
assert isinstance(result, ExpectedType)
After (Option 2 - Assert not None):
result = some_optional_function()
assert result is not None
Keep type: ignore only for legitimate library limitations:
# pandas groupby has incomplete type stubs
grouped = df.groupby("column") # pyright: ignore[reportUnknownMemberType]
6. Improve Error Handling
Make code fail early and loudly instead of continuing with invalid state.
Before:
try:
data = json.loads(raw_data)
except json.JSONDecodeError:
logger.warning("Invalid JSON, using empty dict")
data = {}
After:
data = json.loads(raw_data) # Let JSONDecodeError propagate
Add newlines after raise statements:
# Before
if value < 0:
raise ValueError("Value must be positive")
return value * 2
# After
if value < 0:
raise ValueError("Value must be positive")
return value * 2
7. Fix Import Style (Google Style Guide)
Import modules, not individual items:
# Before
from mypackage.module import FunctionA, FunctionB, ClassC
# After
from mypackage import module
# Use as: module.FunctionA(), module.ClassC()
Move imports to top level:
# Before
def some_function():
import expensive_module
return expensive_module.do_work()
# After
import expensive_module
def some_function():
return expensive_module.do_work()
Exception: TYPE_CHECKING guard for type-only imports:
from typing import TYPE_CHECKING
if TYPE_CHECKING:
from mypackage import SomeType
8. Improve Test Structure
Beyond parameterization, also look for:
Using real instances instead of MagicMock:
# Before
mock_target = MagicMock(spec=Target)
mock_target.name = "test"
# After
target = Target(name="test") # Use real instance of simple class
Direct assertions instead of defensive ones:
# Before (defensive)
result = some_function()
assert result.get("value") == expected
# After (direct)
result = some_function()
assert result["value"] == expected # Let KeyError fail the test
Mock only at boundaries:
# Mock external APIs, not internal code
mocker.patch("requests.get") # Good - external boundary
mocker.patch("myapp.internal_helper") # Bad - internal code
9. Other Patterns to Remove
- Overly verbose error messages that duplicate context
- Redundant validation that duplicates framework behavior
- Empty except blocks or overly broad exception handling
- Unused imports added "just in case"
- Unnecessary intermediate variables that don't improve clarity
Workflow
- Load rules: Read the project's
CLAUDE.mdand~/.claude/CLAUDE.md(if they exist) to understand all applicable style rules and conventions - Identify files to clean: Run
git diff main...HEAD --name-onlyto find files changed on this branch, then focus cleanup on only the changed portions of those files (not pre-existing code) - Review systematically: Check changed code against both CLAUDE.md rules and the hardcoded patterns above, in priority order:
- Priority 1 (CRITICAL): Defensive code patterns (sections 4-6) and CLAUDE.md "fail early" rules
- Priority 2 (HIGH): Test structure and parameterization (sections 3, 8) and CLAUDE.md testing rules
- Priority 3 (MEDIUM): Type checking issues (section 5) and CLAUDE.md type checking rules
- Priority 4 (LOW): Comments, docstrings, imports (sections 1, 2, 7) and CLAUDE.md import/style rules
- Make changes: Edit files to fix violations in the new/changed code
- Run tests: Ensure changes don't break anything
- Summarize: Tell the user what was cleaned up, organized by category, noting which changes came from CLAUDE.md rules vs hardcoded patterns
Notes
- When in doubt, less is more—remove rather than keep
- If a comment or docstring makes you think "obviously", remove it
- Parameterization should make tests more readable, not less—if a test matrix is confusing, keep separate tests
- Always run tests after cleanup to catch any accidental removals