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):

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

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.