code-police

# Code Police Review the current changes (scoped to the current branch/PR) against the rules below plus any project rules. The three passes — rule checklist, fact-check, elegance — run as sub-agents on fresh contexts: the implementer's main context just wrote the diff and is biased to rationalize it; sub-agents start cold. The orchestrator stitches their findings into one summary. ## Arguments `--no-elegance` — skip Pass 3 entirely (report `Elegance | – | Skipped (--no-elegance)`). Use when `/simplify` already ran over this same tree; without the flag Pass 3 would re-invoke it for a near-guaranteed no-op. Passes 1–2 still run. ## Project rules Before spawning the passes, read `.agency/code-police.md` if it exists — its rules (inline or via pointer) are additions to the built-in list, appearing as separate Pass 1 rows under the project's rule IDs. Missing file → built-in rules only. ## Reviewing principles Apply to every pass, and to the orchestrator (push back on a sub-agent that violates them rather than laundering its dismissal into the summary): - **NEVER talk yourself out of a finding.** No "However…", no "acceptable tradeoff", no "theoretically X but practically Y". - **NEVER issue "no action needed"** on a finding you just described, and never end with reassurance unless you genuinely found zero issues. - **Assume the code is wrong until proven right** — prosecutor, not defense. ## Rules ### dry-rule-of-three Two similar instances are fine; three is the threshold for extraction. But identical content that must stay in sync (same HTML, version string, port, path) is deduplicated immediately regardless of count. ### prefer-focused-library Before hand-rolling a utility (tokenizer, parser, date/semver/URL helper, arg parser, tree walker, path normalizer…), search for a focused library — and prefer it even as a new dependency when scope and bundle cost fit. "Zero deps" is an easiness judgment: code you don't own doesn't bitrot and its edge cases are someone else's problem. Hand-roll only when the library adds surface you actively don't want, or the hand-roll is genuinely a few branch-free lines. Neither "it's already in the tree" nor "only ~40 lines" gates this — judge scope fit and bundle cost, in both directions (left-pad exists). ### invalid-states-unrepresentable Discriminated unions, not booleans or stringly-typed fields. If two fields can't both be `undefined`, the type says so. ### no-dead-code Aggressively remove unused code — no commented-out blocks, no "just in case". ### no-silent-error-swallowing No empty `catch {}`, bare `catch: pass`, or `|| true`. At minimum log; an intentional best-effort catch carries a comment saying why ignoring is safe. ### no-unbounded-growth Collections, buffers, and listeners that grow with usage need a bound or cleanup path: cap or evict arrays pushed from handlers; debounce/throttle high-frequency sources (`fs.watch`, `resize`, `scroll`, `onmessage`) unless the handler is O(1) and allocation-free; stream instead of whole-file buffers when the source can grow; share watchers instead of per-caller installs. LLM-generated code defaults to the simplest correct implementation, which is often O(n) in session lifetime — fix at write time (cap, debounce, stream, share); it's rarely caught in review because the code is functionally correct. ### comment-the-non-obvious At every non-trivial declaration or block, ask: can a reader who didn't write this tell **what it does** or **why this shape**? Write the comment that supplies whichever is missing (design intent, control-flow semantics, a hidden constraint types don't carry). "Obvious to me because I just wrote it" is the failure mode. ## Running the passes Spawn Pass 1 and Pass 2 as **two parallel read-only sub-agents** (`subagent_type: "Explore"`; both `Agent` blocks in one response). Each prompt is self-contained and points at this file for the rules-of-record. Pass 3 runs **after** they return (it applies fixes and would race their reads); skip it under `--no-elegance`. Then stitch all outputs into the summary. **Pass 1 (rule checklist)** — sub-agent prompt must direct it to: read the "Reviewing principles" and "Rules" sections of `.apm/skills/code-police/SKILL.md` plus `.agency/code-police.md` if present; scope to `git diff origin/HEAD...HEAD` (or the appropriate base ref); and return one table covering **every** rule (built-in + project): | Rule ID | Violation found? | What was identified | Action taken | | ------- | ---------------- | ------------------- | ------------ | Every "No" requires a **`Checked by:`** field — the grep that confirmed absence (negative rules), or the enumeration of positive candidates ruled out (bidirectional rules like `comment-the-non-obvious`). A "No" without `Checked by:` is malformed. No skipped rows, no fixes applied (the orchestrator routes them). **Pass 2 (fact-check)** — sub-agent prompt must direct it to: read the "Reviewing principles" and apply them verbatim; scope as above; this is a **logic** review, not style — find where the code lies to itself: silent error swallowing, inaccurate fallbacks (defaults masking misconfiguration), wishful thinking (unvalidated boundary inputs, "can't fail" code that can, races papered over with comments), logic errors (always-true conditions, off-by-one, shadowing), and slow leaks (unbounded growth, undebounced hot handlers, per-caller watchers). Fail loud over fail silent; every fallback justified for the failure case; precision over coverage (3 real issues beat 20 maybes). Per finding: file, line, one-line risk, concrete fix. No fixes applied. **Pass 3 (elegance)** — skip under `--no-elegance`; skip when the diff is under 10 lines (`Elegance | 0 | Skipped (tiny diff)`) — the fan-out overhead is disproportionate there. Otherwise: under Claude Code, invoke the bundled `/simplify` via the Skill tool (three parallel lenses, applies fixes). On harnesses without it, run 3 iterations of understand → research → apply → verify, preferring fewer lines, clearer intent, idiomatic style — remove abstractions, don't add them. The reviewing principles bind here too. ## Output | Pass | Issues found | Details | | ---------- | ------------ | ------------------------ | | Rules | N | Brief summary or "Clean" | | Fact-check | N | Brief summary or "Clean" | | Elegance | N | Brief summary or "Clean" | Below the table, reproduce each pass's full findings verbatim (so a caller like `/do` can commit each violation individually). Any pass found issues → state **"Violations or issues found"**; all clean → **"All clear"**. ## Additional principles - **Simple, not easy**: modules do one thing; data flows through arguments and returns, not shared mutable state. No interfaces with one implementor, no "for future use" code, plain data over objects with behavior. - **Completeness**: implement the full spec; run tests and local CI before declaring done. - **Justfile**: every recipe carries a doc comment. - **Volatility-based module structure**: group by rate of change; each module owns one volatility zone; shared constants get their own file. - **Readability**: exported types and components get doc comments; extract deeply nested callbacks into named functions.