---
description: Use when reviewing implementation code before it ships to remove AI-ish comments, process residue, unnecessary complexity, and overbuilt code while preserving behavior and important context.
polytoken:
  tags: [code-review]
---
# Reviewing Code for Shipping Quality

Use this skill when code is functionally close but needs a final shipping-quality pass. The goal is not style nitpicking. The goal is to make the shipped code smaller, clearer, and less agent-shaped without deleting context that future maintainers need.

Announce: "using the reviewing-code-for-shipping-quality skill to strip code/comment slop before shipping"

## What this review catches

Look for:

- comments that narrate the implementation instead of explaining why it exists;
- comments that sound like an agent talking to itself or to the user;
- leftover planning, TODO, debug, or migration narration that does not belong in shipped code;
- code structured around the prompt rather than the domain;
- over-general abstractions created before there is a second use;
- defensive layers that do not protect a real boundary;
- verbose helper names or comments that restate the code;
- avoidable branching, wrapping, mapping, or type churn;
- tests that assert implementation trivia instead of behavior.

## Comment policy

Keep comments that explain:

- non-obvious invariants;
- protocol, API, or platform quirks;
- security-sensitive assumptions;
- concurrency or lifecycle constraints;
- performance tradeoffs that are not obvious from the code;
- why a tempting simpler approach is wrong.

Remove or rewrite comments that:

- narrate obvious code: `// increment counter`, `// return the result`;
- describe the edit process: `// added for the new workflow`, `// updated to handle...`;
- mention the agent/user/session: `// the user wanted`, `// Claude generated`, `// as requested`;
- use promotional or vague language: `robust`, `seamless`, `comprehensive`, `powerful`, `nuanced`;
- preserve uncertainty without action: `// maybe`, `// probably`, `// should be fine`;
- duplicate names: `// UserConfig stores user config`.

If a comment is necessary but bloated, rewrite it as the shortest precise statement of why the code is surprising.

## Code compactness policy

Prefer the least code that clearly does the job.

Do not compact by making code clever. Compact by removing unnecessary structure:

- inline one-use helpers when the helper name adds no meaning;
- remove configuration knobs that no caller uses;
- collapse wrappers that only forward arguments;
- replace generic abstractions with direct code until there is a real second use;
- delete dead branches and unused parameters;
- simplify names when types or context already carry the meaning;
- keep explicit error handling at real trust boundaries.

## Review process

1. Identify the changed or reviewed scope.
2. Read enough surrounding code to understand local conventions.
3. Separate behavior fixes from shipping-quality cleanup. Do not hide behavior changes inside cleanup.
4. Inspect comments first. Remove, shorten, or rewrite them according to the comment policy.
5. Inspect structure. Remove prompt-shaped scaffolding, one-use abstractions, and defensive code without a real boundary.
6. Inspect tests. Prefer behavior assertions over tests that encode incidental implementation structure.
7. Run or recommend focused validation for any changed code.

## Findings format

When reviewing, report findings like this:

```md
## Shipping-quality findings

### SQ1 — Remove process comment
Location: `path/to/file.ts:42`
Severity: cleanup
Issue: Comment describes why the agent added the branch, not why the branch exists.
Fix: Delete it, or replace with a short invariant if one exists.

### SQ2 — Collapse one-use abstraction
Location: `path/to/file.ts:80-110`
Severity: cleanup
Issue: Helper only forwards arguments and is not reused.
Fix: Inline it unless the name captures domain meaning not visible at call site.
```

Use severities:

- `must-fix`: misleading comments, dead code, debug leftovers, behavior-risking complexity;
- `cleanup`: unnecessary comments, one-use wrappers, verbose scaffolding;
- `consider`: subjective simplifications that need maintainer judgment.

## Fix policy

Fix directly when:

- the change is behavior-preserving;
- the local convention is clear;
- validation is available or the edit is obviously mechanical.

Ask or defer when:

- a comment may encode domain knowledge;
- simplification could change behavior;
- an abstraction may exist for future planned use;
- local style conflicts with this skill's preference.

## Anti-pattern examples

Bad comment:

```ts
// Added comprehensive handling for the new edge case requested by the user.
if (value == null) return defaultValue;
```

Better:

```ts
// The upstream API sends null when the field is omitted.
if (value == null) return defaultValue;
```

Bad abstraction:

```ts
function processUserConfigurationInput(config: UserConfig): UserConfig {
  return normalizeUserConfig(config);
}
```

Better:

```ts
const normalized = normalizeUserConfig(config);
```

Bad test smell:

```ts
it("calls normalizeUserConfig before validateUserConfig", () => { ... })
```

Better:

```ts
it("rejects configs with missing required fields after defaults are applied", () => { ... })
```

## Checklist

Before finishing, confirm:

- [ ] Comments explain why, not what or how the edit happened.
- [ ] No agent/user/session/process narration remains in shipped code.
- [ ] No debug leftovers, stale TODOs, or migration scaffolding remain unless intentionally tracked.
- [ ] One-use abstractions and wrappers are justified by domain meaning, not prompt structure.
- [ ] Cleanup did not hide behavior changes.
- [ ] Validation was run or the remaining validation gap is stated.
