All skills
mblode avatar

/tidy

@1c00344
by Matthew Blodemblode/agent-skills136 stars
12

Reviews a local diff, branch diff, or PR with file:line findings in confirmed and plausible tiers, gates any test the diff adds or changes, and in apply mode lands the fixes and diff-scoped simplifications. Use when asked to "review my changes", "code review", "tidy this", "simplify my diff", "deslop this", "structural review", "security audit", or "audit these tests in my PR". For the PR itself use pr-creator; for CI and review threads use pr-babysitter; for UI defects use ui-design; for repo architecture use codebase-architecture; for pruning the whole suite to a target use test-audit.

Use this Skill: https://skilld.dev/gh/mblode/agent-skills/tidy

This session only. Nothing lands on disk.

referencesai-slop-patterns.md

≈3.1k tokens on demand. Your agent reads this file only when SKILL.md points to it.

AI Slop Patterns

Detection catalog for AI-generated code that passes lint and tests but reads as machine-written. Load when the user asks to deslop, clean up AI code, remove slop, or when reviewing AI-assisted changes.

Focus on distinctively AI-generated patterns, not general code quality (that belongs in structural-quality-rubric.md).

Contents

  • Over-commenting
  • Stale comments
  • History-citing comments
  • Invented API
  • Unnecessary error handling
  • Type bypasses
  • Premature abstraction
  • Scaffolding without a consumer
  • Verbose naming
  • Structural bloat
  • Defensive excess
  • Template residue
  • Test padding
  • Applying fixes

Over-commenting

Comments that restate what the code already says.

Flag:

  • JSDoc on functions with self-documenting names and types
  • // Handle the error above a catch block
  • // Return the result above a return statement
  • // Initialize variables above const declarations
  • Block comments explaining a single obvious line
  • // Destructure the props above destructuring assignments

Fix: Delete it. If removal would confuse a reader, the code needs a better name, not a comment.

Stale comments

Comments describing code the diff has since changed. Distinct from over-commenting: the problem is not that the comment is redundant, it is that it is now false, and the next reader trusts it over the code.

Flag:

  • A comment naming a value, unit, limit, or branch the diff just changed
  • A doc block whose @param, @returns, or thrown-error list no longer matches the signature
  • A comment describing the approach that was intended rather than the one that shipped
  • A "why" comment justifying a constraint the diff removed

Fix: Correct the comment, or delete it where the code now says the same thing. Quote both the comment and the line that refutes it; a stale-comment finding citing only the comment cannot be checked. Only comments this diff falsified are in scope; a repo-wide hunt produces noise rather than findings.

History-citing comments

Comments that justify code by pointing at where the justification supposedly lives, rather than stating it. Distinct from stale comments: the problem is not that the comment has gone false, it is that the reason was never actually written down, only a pointer to a conversation the reader cannot reopen.

Flag:

  • // per review / // per PR feedback / // as discussed
  • // changed after PR #<n> or a bare issue/PR link standing in for the reason
  • // used to do X / // previously this was Y with no statement of why the change happened
  • // workaround because <reviewer> said / // <name> wanted it this way
  • A commit hash or ticket id cited as the explanation instead of the invariant or bug class it fixed

Fix: Delete the comment, or replace it with the actual reason in the code's own terms: the invariant it protects, the bug class it prevents, the constraint it satisfies. That version survives the PR being squashed, the branch being deleted, and the reviewer leaving; a name or a PR number does not. If the history itself is the only thing worth keeping (why an odd-looking line is deliberate), put it in the commit message, where git log and git blame preserve it, not in a comment nobody will go re-fetch it from.

Invented API

A call that reads as real and is not: a method, option, or import that exists in a similar library, an older or newer major version, or nowhere. The tell is fluency; the name is exactly what the API should have been called. It survives lint, survives type-check in JavaScript or behind any, and fails at the first runtime call, or worse, is an option the library silently ignores.

Flag:

  • An import from a package whose installed version (per the lockfile) does not export that name
  • A method on a library object that the package's .d.ts or docs do not list
  • An option key in a config object the library never reads (no error, no effect)
  • A CLI flag or environment variable the tool does not recognise
  • A test that mocks the invented method, so the suite passes without ever calling the real library

Fix: Name the real API from node_modules/<pkg> at the installed version, or the closest one and what differs. Cite the export list or docs you checked; a "this does not exist" finding without the source you searched is itself unverifiable. A symbol absent from the installed package's .d.ts is confirmed, not plausible: the types are the export list. A symbol TypeScript accepts does exist, so look instead at the calls the checker never saw: JavaScript files, any-typed values, and string-keyed option bags.

Unnecessary error handling

Wrapping infallible operations in try-catch or guarding impossible states.

Flag:

  • try-catch around pure computations (string manipulation, array mapping, object destructuring)
  • Null checks on values the type system guarantees are non-null
  • || [] on a value already typed as an array
  • ?? undefined (a no-op: undefined is already the default)
  • || '' on a string (not string | undefined) type
  • Catch blocks that just rethrow without modification
  • Error boundaries wrapping components that cannot throw

Fix: Remove the guard only after proving the value is internal or already validated. Keep validation at system boundaries (user input, external APIs, network responses, generated files, persisted data).

Type bypasses

Casting or suppressing types instead of fixing the underlying issue.

Flag:

  • as any: always a smell; fix the type or narrow with a type guard
  • as unknown as T: double-cast to force an incompatible type
  • @ts-ignore / @ts-expect-error without an explanation comment
  • Unnecessary type assertions on values that already match the target type
  • ! (non-null assertion) when the value could genuinely be null
  • Generics defaulting to any (useState<any>())

Fix: Fix the type at its source: fix a wrong upstream type upstream, or override a wrong third-party type with a targeted .d.ts.

Premature abstraction

Abstractions created before a shared invariant, owner, lifecycle, or failure mode justifies them.

Flag:

  • Helper functions called from exactly one site that name no domain concept, boundary, or repeated invariant
  • Wrapper classes with a single method that delegates to the wrapped object
  • Config objects consumed by one function
  • Factory functions that always return the same variant
  • Custom hooks that thinly wrap a single useState or useEffect
  • utils/ files with one export
  • Constants extracted for a value used once

Fix: Inline the abstraction unless it protects shared knowledge: an invariant, protocol, lifecycle, owner, or business rule. Do not extract coincidental shape.

Scaffolding without a consumer

Support structure built out in full while the thing it supports is absent or unreachable. Distinct from premature abstraction: the abstraction at least has a caller. Distinct from test padding: that covers tests for code that exists but assert nothing actionable. If the module under test does not exist, report it here only, once.

Flag:

  • Test files for a module, route, or export that does not exist, or that nothing reaches
  • A generated registry, schema, or type surface no source file imports
  • A CI job or check enforcing a contract nothing currently emits
  • Fixtures, mocks, or seed data for an unbuilt API
  • Docs or a README section describing a command the binary does not implement

Fix: Delete the artifact, or build its consumer in the same diff. Shipped alone it is complete, it passes review, and it delivers nothing while implying the work is done. A search returning nothing is not proof of no consumer: dynamic imports, framework file-name conventions, generated code, and other repos all reach code no static search finds.

Verbose naming

Names repeating info already in the type system or context.

Flag:

  • userArray, nameString, isLoadingBoolean (type is in the name)
  • handleOnClickButton (redundant event + element in handler name)
  • fetchDataFromAPIAndTransformResponse (implementation in the name)
  • getUserByIdFromDatabase (storage detail in the name)
  • IUserInterface, UserType (type-system prefix/suffix on types)
  • setIsLoadingToTrue (value in the setter name)

Fix: Use the simplest name that is unambiguous in context: users, name, loading, handleClick, fetchUser, getUser.

Structural bloat

Files, exports, and patterns that add surface area without value.

Flag:

  • Barrel files (index.ts) that re-export everything from a directory
  • Empty utility files with boilerplate but no logic
  • Files with only type re-exports (export type { Foo } from './foo')
  • Dead code behind if (false) or // @deprecated with no removal date
  • Old/new dual paths, legacy aliases, or re-exports kept "for compatibility" when nothing depends on the old path
  • Duplicate type definitions when a shared type exists
  • Separate files for a single small constant or type

Fix: Delete the file or inline it. Co-locate small types and constants with their consumer.

Defensive excess

Guarding against states the language or framework prevents.

Flag:

  • ?. optional chaining on values that cannot be null (non-optional props, required function parameters, const assignments from non-nullable sources)
  • Array.isArray() check on a value typed as T[]
  • typeof x === 'function' on a value typed as a function
  • if (x !== null && x !== undefined) when the type is not nullable
  • try { JSON.parse(knownValidJSON) } on a value that is always valid JSON
  • Fallback UI for error states that cannot occur in the component's data flow

Fix: Remove the guard only after proving the value is internal or already validated. If unsure, fix the type or boundary validation instead of adding local defensive noise.

Template residue

Placeholder content left behind from AI generation.

Flag:

  • // TODO: implement or // TODO: Add error handling with no implementation
  • // Add your logic here
  • Generic error messages: "An error occurred", "Something went wrong", "Failed to process request"
  • Debugging console.log statements: console.log('here'), console.log(data)
  • Commented-out code blocks with no explanation
  • Empty function bodies or stub returns (return null, return undefined, return {})

Fix: Implement the functionality or delete the placeholder. Replace generic error messages with specific, actionable ones.

Test padding

Tests generated because coverage looks like rigor. The tell: the test cannot fail for a reason anyone would act on.

Flag:

  • Render-only tests: presence assertions with no interaction or branch
  • Mock-echo assertions: asserting a mock was called, or returned the value it was mocked to return
  • Change-detector snapshots that fail on any edit rather than on wrong behavior
  • Framework re-tests: proving React renders, the router routes, or the ORM saves
  • Happy-path triplication: the same branch asserted through near-identical cases
  • Diff-mirror tests: the assertion repeats a literal copied from the diff (a config row, flag default, route entry, label, or copy string); a second ledger to maintain, not behavior coverage
  • Export-for-testability: a helper extracted or exported solely so a test can name it, proving nothing about the original behavior
  • Fake integration tests: hand-rolled in-memory emulators of behavior that lives in the real system (schema, validators, indexes, permissions), unable to catch the bug class they claim to cover

Fix: Delete the test. A bug fix keeps exactly one: a repro that fails on the pre-fix code, anchored at the seam that failed (route, API, browser flow, integration point). A config-adjacent test stays only if it proves a rule across a class of cases; if its name contains one item id and its assertion repeats the value just changed, it goes.

Applying fixes

When reviewing for slop:

  1. Read the diff with slop detection in mind; don't flag pre-existing patterns outside the diff
  2. Group findings by category, not by file
  3. Prioritize behavioral preservation: deslop changes should never alter runtime behavior
  4. Apply the codebase's existing conventions, not an ideal standard
  5. Use git blame only to separate pre-existing code from diff scope; judge slop from code patterns, not author or timing

Source: SKILL.md on GitHub

1 warning3d3 checks · Risk SAFE
  • Gen Agent Trust Hub3d

    This skill is a code review and automated refactoring tool that analyzes git diffs, pull requests, and codebase subsystems. It uses standard developer utilities like git and gh to retrieve context and can optionally invoke other installed AI CLIs for secondary analysis. While it executes repository-defined commands like tests and linters in its 'apply' mode, this behavior is consistent with its primary purpose as a developer productivity tool.

  • Socket3d

    No alerts

  • Snyk3d

    Risk: MEDIUM · 1 issue

Signed by skilld at 1c00344. This ties the file your Agent reads to that commit on GitHub. It does not review the instructions.

Last checked against GitHub 4 hours ago.

Activeupdated 5 days ago

README badge

README badge for mblode/agent-skills/tidy