Cleanup

Cleanup

Shrink a branch's diff without changing what it does. Fewer lines, fewer overrides, fewer comments, one subject per file — every change here is behaviour-preserving unless it fixes a real bug found on the way.

Read first: .agents/rules/code-style.md, .agents/rules/ui.md, and the tamery-ui skill's hard rules. They define what "clean" means here; this skill is the pass that enforces them.

1. Get the diff

git fetch -q origin tamery main
BASE=$(for b in $(gh pr view --json baseRefName -q .baseRefName 2>/dev/null || echo tamery main); do m=$(git merge-base HEAD origin/$b) && echo "$(git rev-list --count $m..HEAD) $m"; done | sort -n | head -1 | cut -d' ' -f2)
git diff --stat $BASE           # keep for the report
git diff $BASE                  # committed + uncommitted work
git status --short              # untracked files are part of the review

The base is the PR's base, else whichever of tamery/main the branch is nearer to. Read every changed file whole, not only the hunks — a hunk hides the helper above it that is now unused.

2. Delete

In rough order of payoff:

• Dead code the branch created: unused exports, props never passed, state never read, helpers with one caller (inline them). grep each new symbol repo-wide before keeping it.
• Imagined states: a guard or try/catch the surrounding module would not write, protecting a state that cannot occur; compatibility shims, aliases, retries and fallbacks — the app is unreleased, so none are owed; casts that launder types (as any, as unknown as T, ! where narrowing works); intermediate variables that name nothing.
• Reinvention: a div stack that is a registry component, a hand-rolled effect where a library option exists, a loop where a built-in reads better. Kit search: pnpm dlx shadcn@latest search @shadcn -q <term>.
• Hand-written code for a well-known format or algorithm (CSV/TSV, diff, glob, semver, date math, …): check npm for a small, maintained, zero-dependency package. Adding a dependency is the user's call, so put it in the report under "left alone and why" with the package name; don't keep it silently.
• Speculative shape: an abstraction with one implementation, an option nobody sets, a wrapper that forwards. Collapse it.
• Duplicate logic added next to an existing helper — reuse the helper (a hand-rolled n > 1 ? 's' : '' is plural() from ~/lib/plural; a query key rebuilt by slicing another helper's queryOptions(...).queryKey is the exported *QueryKey builder). Two dialect branches of one createQuery that are character-for-character identical are one statement written twice. The same inline object type in two files is one named type exported by the file that owns it.
• No-op code: a prop set to the kit's default (side="top" on TooltipContent, variant="outline" on AlertDialogCancel — read the component's default before keeping it); an onClick re-checking the condition that already sets disabled; branches that all return the same value; a merge/spread over an object every caller passes whole; a caller spelling out undefined/false fields that are only read truthily; recomputing a value already in scope.
• Unused lint escapes: pnpm exec oxlint -c oxlint.config.ts --report-unused-disable-directives $(git diff --name-only --diff-filter=d $BASE -- '*.ts' '*.tsx') lists every oxlint-disable in the changed files that suppresses nothing — delete them; a moved or rewritten line often leaves its directive behind.
• Orphan files: a patch nothing lists in pnpm-workspace.yaml, an asset nothing imports, a fixture no test reads. Check the registry a file needs to be in, not just imports.

Do not delete a behaviour a rule file or the tamery-ui skill describes on purpose — an unused option is speculation, an unused behaviour someone wrote down is a decision. Speculation goes; a decision gets asked about. Either way, the doc describing what you deleted is updated in the same pass.

3. One subject per file

Deleting never finds a file that does too much, so ask it of every changed file separately, whatever its length: list its subjects. A subject is a component, a hook with its own refs or effects, one branch of a layout switch, or an adapter that unpacks one object into another's parameters. Anything past the first moves out:

• A layout branch whose sibling already has its own file (DocumentList beside an inline <Grid> setup) gets its own file, shaped like the sibling.
• A cluster of refs, an effect and an imperative handle serving one job is a use<Job> hook in its own file, together with the types it exports.
• An adapter that spreads an object into callbacks for a single callee means the callee should take the object; fold the adapter into it.
• After a split, grep what the old file returned or exported. A value only the moved code used (a lookup map returned beside the real result) is now dead.

Lines a split adds do not count against the shorter diff. Repoint importers at the new file; never re-export from the old one.

4. Match the house shape

New code should be indistinguishable from the code beside it — and read as written by a person, not compressed. Plain branches beat a clever one-liner (String(v ?? 'null') || 'empty', const [only] = cond ? list : [], a line opening with ;(); an if chain over one field's values is a lookup record; a class ternary repeating its shared classes is cn(shared, cond ? a : b); several useStates always set together are one object state.

Pick the majority spelling and use it everywhere rather than leaving two conventions in one folder: one hook for one job, one import path, one prop-type style across sibling files. Kit variants live in <component>.utils.ts as cva, not as a record inside the .tsx.

Look at what the repo actually does before "fixing" a file to a rule — useMemo/useCallback are banned, but react-hooks(exhaustive-deps) outranks that: if the linter demands a stable identity, the memo stays.