improvement(cleanup-skill): parallelize analysis, apply fixes sequentially

.agents/skills/babysit/SKILL.md

@@ -100,8 +100,9 @@ round. Always check both conditions freshly after every push.
    git fetch origin staging && git log --oneline --reverse origin/staging..HEAD
    gh pr view <n> --json commits -q '.commits[].messageHeadline'
    ```
-   `--reverse` matches `git log`'s newest-first default to the PR commit list's oldest-first
-   order — without it a positional comparison can spuriously fail on any multi-commit branch.
+   `--reverse` makes `git log` oldest-first, matching the PR commit list's order — plain
+   `git log` is newest-first, so without it a positional comparison can spuriously fail on any
+   multi-commit branch.
    These two lists must describe the same commits. A review loop runs many pushes across many
    rounds; checking sync only before the push (step 5) and never after is how a bad push or a
    PR whose commit history quietly went stale between rounds goes unnoticed.

.agents/skills/cleanup/SKILL.md

@@ -1,6 +1,6 @@
 ---
 name: cleanup
-description: Run all code quality skills in sequence — effects, memo, callbacks, state, React Query, and emcn design review
+description: Run all code quality skills — effects, memo, callbacks, state, React Query, emcn design review, url-state, and comments — analyzing in parallel, then applying fixes sequentially
 ---
 
 # Cleanup
@@ -11,18 +11,50 @@ Arguments:
 
 User arguments: $ARGUMENTS
 
-## Steps
+## Step 1 — Parallel analysis (read-only)
 
-Run each of these skills in order on the specified scope, passing through the scope and fix arguments. After each skill completes, move to the next. Do not skip any.
+First parse the user's `$ARGUMENTS` into `scope` and `fix`: extract the `fix=true|false` token wherever it appears in the string (start, middle, or end), and treat everything else — with that token removed — as `scope`. Defaults: `scope` = your current changes, `fix` = true. The `fix` value is consumed by Step 3 — it does NOT propagate to these passes, which always run `fix=false`.
 
-1. `/you-might-not-need-an-effect $ARGUMENTS`
-2. `/you-might-not-need-a-memo $ARGUMENTS`
-3. `/you-might-not-need-a-callback $ARGUMENTS`
-4. `/you-might-not-need-state $ARGUMENTS`
-5. `/react-query-best-practices $ARGUMENTS`
-6. `/emcn-design-review $ARGUMENTS`
+Spawn all eight passes concurrently as subagents in a **single message** (multiple Agent tool calls). Each runs its skill on the parsed `scope` with `fix=false` — analysis and proposals ONLY, no edits. Instruct each agent to return its findings as a structured list: for every proposed change, the file path, line range, a one-line description of the change, and the exact before/after so the orchestrator can apply it without re-deriving.
 
-After all skills have run, output a summary of what was found and fixed (or proposed) across all six passes.
+Run these eight in parallel, substituting the parsed `scope` for `<scope>` in each invocation (pass the real scope text, never the literal `<scope>`):
+
+1. `/you-might-not-need-an-effect <scope> fix=false`
+2. `/you-might-not-need-a-memo <scope> fix=false`
+3. `/you-might-not-need-a-callback <scope> fix=false`
+4. `/you-might-not-need-state <scope> fix=false`
+5. `/react-query-best-practices <scope> fix=false`
+6. `/emcn-design-review <scope> fix=false`
+7. `/you-might-not-need-url-state <scope> fix=false`
+8. `/you-might-not-need-a-comment <scope> fix=false`
+
+## Step 2 — Converge
+
+Collect all findings into one list, **keeping each proposal tagged with the pass that produced it** — do NOT collapse a file's proposals into a single unlabeled patch, because Step 3 applies in pass order and needs those labels. Detect overlaps where two passes touch the same region (common: a state pass and an effect pass on the same block, or a memo and callback pass on the same component). Reconcile only genuine same-region conflicts, and drop proposals a sibling pass has made moot; a reconciled change inherits the pass label of whichever of its passes comes first in the Step 3 dependency order (effects → state → memo → callback → React Query → url-state → emcn → comments), so it is applied at the earliest safe point. Non-overlapping proposals stay as-is with their own labels. The output is a per-pass list of surviving changes, not a per-file patch.
+
+## Step 3 — Sequential apply
+
+If `fix=false`, skip this step — just report the proposals from Step 2.
+
+Otherwise apply the surviving changes yourself (in the main context, not delegated), iterating **pass by pass** in this dependency order so earlier structural changes settle before later passes build on them:
+
+1. effects → 2. state → 3. memo → 4. callback → 5. React Query → 6. url-state → 7. emcn design → 8. comments
+
+For each pass in turn, apply all of that pass's changes, then move to the next pass. A file touched by several passes is therefore edited once per pass, in this order — not once as a merged patch. This is what makes the ordering real: a single merged-per-file patch would collapse all passes into one edit and lose it.
+
+Comments apply last, on purpose: that pass operates on whatever the earlier structural passes settled the code into, so it never edits lines a sibling pass is about to delete or rewrite.
+
+**Treat every Step 1 proposal as snapshot-relative, not authoritative.** All passes analyzed the *original* files in parallel, so a proposal's line ranges and before/after text describe the code as it was *before* any edits — once an earlier pass has run, a later pass's snippet may no longer match. So for each change, before applying:
+
+1. Re-read the file and locate the target by its **content** (the proposal's `old_string` snippet), not by its line number — line numbers from Step 1 are only a hint for where to look, since earlier edits shift them.
+2. If the `old_string` still matches verbatim, apply it — a content-anchored edit is safe even if its line moved.
+3. If it no longer matches (an earlier pass altered that region), do **not** force the stale patch. Re-derive the change from the current code by re-applying that pass's rule to the construct, or drop it if a prior pass already made it moot. Never apply a proposal against text it wasn't computed from.
+
+After all edits, run `bun run lint:check` on the touched files.
+
+## Step 4 — Summary
+
+Output a summary across all eight passes: what each found, what was applied vs. skipped-as-redundant, and any proposals that need a human decision.
 
 ## Boundary Audit Guidance
 

.agents/skills/you-might-not-need-a-comment/SKILL.md

@@ -0,0 +1,45 @@
+---
+name: you-might-not-need-a-comment
+description: Analyze and fix redundant or self-explanatory inline comments — remove noise, promote genuine documentation to TSDoc
+---
+
+# You Might Not Need a Comment
+
+Arguments:
+- scope: what to analyze (default: your current changes). Examples: "diff to main", "PR #123", "src/components/", "whole codebase"
+- fix: whether to apply fixes (default: true). Set to false to only propose changes.
+
+User arguments: $ARGUMENTS
+
+## The one rule that matters
+
+A comment must add information the code cannot express itself. Code says *what* and *how*; a comment earns its place only by explaining *why* — a non-obvious constraint, a workaround, a decision, a gotcha. If deleting the comment loses no information a competent reader wouldn't recover from the code in seconds, delete it.
+
+This codebase's convention: **TSDoc for documentation, no non-TSDoc comments, no `====` separators.** Genuine documentation belongs in a `/** ... */` block on the declaration; everything that survives as an inline `//` comment must be a real *why*, kept terse.
+
+## Anti-patterns to detect
+
+1. **Restates the code**: `// increment counter` above `counter++`, `// return the result` above `return result`, `// loop over items`. Delete.
+2. **Narrates the obvious from names**: the function is `fetchUserById`, the comment says `// fetches a user by id`. The identifier already said it. Delete.
+3. **Section-divider / banner comments**: `// ==== Helpers ====`, `// --- state ---`, `// #region`. Against convention. Delete (the code's structure is the structure).
+4. **Commented-out code**: dead code left as a comment. Delete — git is the history.
+5. **Redundant type/param echo in prose comments**: `// takes a string and returns a number` when the signature already says so. Delete.
+6. **Changelog / attribution noise**: `// added by X`, `// TODO(2021): ...` long-stale, `// fix for bug`. Delete unless it encodes a live, actionable constraint.
+7. **Genuine documentation written as a loose `//` block on a declaration**: a real explanation of what an exported function/type/const is for, but written as stacked `//` lines instead of TSDoc. Convert to a `/** ... */` TSDoc block on the declaration.
+
+## Patterns that ARE correct — do not flag
+
+- A `//` comment that explains a **non-obvious why**: a workaround for an upstream bug, an ordering constraint, a perf reason, a spec/edge-case the code can't self-document (`// first-match wins — matches the old find() semantics`).
+- Existing TSDoc `/** ... */` blocks on declarations — leave them (only tighten if verbose).
+- `// boundary-raw-fetch:`, `// double-cast-allowed:`, `// boundary-raw-json:`, `// untyped-response:`, `// migration-safe:` and other **machine-read annotations** — these are load-bearing, never touch them.
+- `// biome-ignore`, `// eslint-disable`, `// @ts-expect-error` and other tooling directives.
+- `// TODO` / `// FIXME` that point at real, still-open work.
+
+## Bias
+
+Prefer **deletion over rewriting**, and **no comment over a comment** when the code is already clear. When a comment is genuine documentation, prefer promoting it to terse TSDoc over leaving a loose `//` block. Never add new comments in this pass — this is a reduction pass. When unsure whether a comment encodes a real *why*, keep it.
+
+## Steps
+
+1. Analyze the specified scope for the anti-patterns listed above
+2. If fix=true, apply the fixes. If fix=false, propose the fixes without applying.

.agents/skills/you-might-not-need-url-state/SKILL.md

@@ -0,0 +1,45 @@
+---
+name: you-might-not-need-url-state
+description: Analyze and fix URL/query-param state anti-patterns — manual useSearchParams reads, hand-built query mutations, view-state trapped in useState, and objects in the URL
+---
+
+# You Might Not Need URL State
+
+Arguments:
+- scope: what to analyze (default: your current changes). Examples: "diff to main", "PR #123", "app/workspace/[workspaceId]/tables/", "whole codebase"
+- fix: whether to apply fixes (default: true). Set to false to only propose changes.
+
+User arguments: $ARGUMENTS
+
+## Context
+
+Shareable client view-state (active tab/panel, filters, search query, sort, pagination, selected-entity id, an open "view" modal/drawer that is a destination) lives in the URL via [`nuqs`](https://nuqs.dev) — driven by a co-located `search-params.ts`, never read via `useSearchParams().get(...)` and never mutated by hand-built query strings. Remote data stays in React Query; high-frequency / large / ephemeral / socket-synced state stays in Zustand; purely local UI stays in `useState`.
+
+`.claude/rules/sim-url-state.md` is the source of truth — read it first.
+
+## References
+
+Read these before analyzing:
+1. `.claude/rules/sim-url-state.md` — the decision framework, conventions, debounced-input pattern, sort convention, selected-entity deep-link pattern, and the workflow-editor carve-out
+2. https://nuqs.dev/docs/parsers — parsers (`parseAsString`/`parseAsInteger`/`parseAsBoolean`/`parseAsStringLiteral`/`parseAsArrayOf`/`createParser`)
+3. https://nuqs.dev/docs/options — `withDefault`, `history`, `shallow`, `clearOnDefault`
+4. https://nuqs.dev/docs/server-side — `createSearchParamsCache` for server reads
+
+## Anti-patterns to detect
+
+1. **Manual param reads for state**: `useSearchParams().get(...)` or `new URLSearchParams(window.location.search)` used to *read* view-state. Replace with `useQueryState`/`useQueryStates` bound to a `search-params.ts`. (Read-once auth/invite/redirect tokens — `token`, `callbackUrl`, `redirect`, `error`, `invite_flow`, `code` — are NOT view-state; leave them on `useSearchParams`.)
+2. **Hand-built query mutation**: constructing a query string + `router.replace`/`router.push` to change a param on the current path. Use a nuqs setter. (A `router.push` that changes the route *path* is fine; an outbound `new URLSearchParams` building an `href`/`window.open`/download/API URL is fine.)
+3. **`window.history.replaceState`/`pushState`** to mutate a param.
+4. **URL state duplicated into a store/useState + synced with an effect** (or a `popstate` listener). The URL is the single source of truth; derive from it, don't mirror it.
+5. **Objects in the URL**: serializing a `TableDefinition`/`SkillDefinition`/etc. Store the id and derive the object from the loaded list (`items.find(i => i.id === id)`).
+6. **High-frequency / large state in the URL**: cursor, pan/zoom, un-debounced keystrokes, big JSON blobs. Debounce text search (local `useState` mirror + reconcile effect); keep canvas/presence/resize state in Zustand.
+7. **Shareable view-state trapped in `useState`**: a tab/filter/sort/pagination/selected-entity that should be a link but lives in local state. Migrate it to the URL.
+8. **Missing Suspense boundary**: a component newly calling `useQueryState`/`useQueryStates` whose page entry has no `<Suspense>` wrapper (Next.js requires it for `useSearchParams`). Add one with a real-chrome fallback.
+9. **`import { z }` for param validation in client code**: use nuqs parsers instead.
+
+## Steps
+
+1. Read `.claude/rules/sim-url-state.md` and the nuqs docs above to understand the guidelines
+2. Analyze the specified scope for the anti-patterns listed above
+3. For each finding, decide the correct home using the decision table — do not force URL state onto ephemeral/high-frequency/socket-synced state
+4. If fix=true, apply the fixes (co-locate a `search-params.ts`, wire `useQueryState(s)`, add the Suspense boundary, delete the replaced state + sync effects). If fix=false, propose the fixes without applying.