Pre Commit Review

Also in-context: a new //nolint directive or .golangci.yaml exclusion in the diff is itself a finding — the change must justify, with evidence, that the rule genuinely does not apply.

Also in-context — the when-in-Rome check: a diff must arrive in the host repo's existing style, not import a new one. Flag anything the diff introduces that the repo does not already use: a new test mechanism (golden files, snapshot testing, a new assertion library), a new dependency in go.mod, a new tool or config file, edits to repo-level convention files (CLAUDE.md, coding standards, lint config) bundled into a feature diff, or a directory layout unlike its siblings. Detection is comparative: for each candidate, grep the repo outside the diff for prior use — zero prior use is the finding. These are not style crimes; they are adoption decisions that belong to the repo owner. The fix is a discussion or a separate PR, never silent inclusion.

1. The rule file's FULL content, pasted — the hunter's entire rulebook and single obsession. Never a path reference alone; never more than one rule per hunter.
2. The diff scope — the changed-file list or git diff range.
3. That rule's pre-filter hits — as starting leads (the hunter re-runs the detection commands itself; leads are a starting point, not a limit).

If the rule cites a case file by plugin-relative path (e.g. ../examples/*.md), resolve it to an absolute path and include that path in the spawn prompt — the hunter runs in the reviewed project's cwd and cannot resolve plugin-relative paths on its own.

Each hunter returns one block per finding: rule | file:line | evidence (falsifying-question answers) | proposed fix pattern | effort (S/M/L) plus a final tally line (R<N>: <M> finding(s) or a hunted-clean line).

1. The extraction findings under review — the hunter blocks pasted verbatim.
2. Payload: the Juiciness scoring and The over-abstraction trap sections of ../../rules/R1-primitive-obsession.md, pasted.
3. Payload: the FULL content of ../../examples/overabstraction-cidr.md, pasted.

Verdicts per finding: CONFIRMED (score + verified evidence) or REFUTED (score 0–1 + reason) → cheaper alternative. A refuted proposal does not ship; when its cheaper alternative (better naming, private fields + accessors, or R11's Keep the Single Exhaustive Switch) is still worth doing, report the alternative as 🟢 Polish. When R11 dispatch proposals are under review, additionally paste the FULL content of both R11 case files — ../../examples/anti-if-dispatch.md (Move 3 is the juiciness rejection: the switch stays, goes exhaustive) and ../../examples/switch-to-polymorphism.md (the dependency-direction rejection: the move is unavailable when the consumer owns the output format; the switch shrinks to pure dispatch). Only findings the skeptic cannot kill ship as extraction findings. Non-extraction findings (R3, R5–R9, and R1/R2/R10/R11 findings that propose no new type) skip the skeptic and go straight to the report — R9 findings (orphans, broken edges, WHAT-comments, unwired root) propose no type extractions.

1. Payload: R9's Comment policy section (../../rules/R9-repo-brain.md, Design guidance) pasted verbatim — the Comment Value Toolbox kinds, the three-test standard, the tier table, budget accounting, and the visibility default.
2. Payload: the Comment Value Toolbox catalog section of ../documentation/reference.md (resolve to an absolute path) pasted verbatim.
3. The absolute path to ../../examples/private-comment-noise.md — the critic reads it when judging comments on unexported symbols.
4. The diff scope.

It judges every comment in the diff (godoc, in-body, test) against the three-test standard and returns per-comment verdicts (KEEP / TRIM / REWRITE / DELETE, or DELETE → route R3 for in-body extraction candidates) with evidence and proposed replacement text. Non-KEEP verdicts land in the report as 🟡 Readability Debt; DELETE → route R3 verdicts merge with any R3 hunter findings on the same lines (one finding, not two). The critic is advisory like everything else — accepted verdicts are fixed by @documentation (the rung-1 fixer), except R3 routes, which go to @refactoring.

Cluster pass (before categorizing): group surviving findings by shared anchor — the same type, field/discriminator, or function named in ≥2 findings from different rules. Each hunter is single-obsession and blind to the others, so independent convergence on one anchor is evidence that a domain concept is missing there — the cluster is a juiciness scorecard that filled itself in (R1 hunter sees the raw primitive, R11 the duplicated switch, R2 the ownerless validation: one disease, four jurisdictions). Render each cluster as a first-class entry above the categories:

🔗 CLUSTER: Alert.Channel (4 findings: R1, R11, R2, R7)
   Hypothesis: missing domain concept — a Channel type wants to exist
   Routing: design-first — @code-designing (cluster-scoped), then @refactoring
   implements; do NOT fix members independently (partial fixes undo each other)

Member findings still appear under their categories below, tagged [cluster: <anchor>]. Clustering is reporting — this skill still never edits and never invokes fix skills; the caller routes.

Category mapping:

• 🐛 Bugs — will fail at runtime regardless of rule (nil returned as a value, cancellation swallowed by context.Background(), R10 goroutine leaks and unguarded concurrent writes): fix immediately.
• 🟠 New Practice (when in Rome) — the when-in-Rome check's findings: a mechanism, dependency, framework, or convention the host repo does not already use, introduced without discussion. Advisory like everything else, but flag it loudly: reviewers reject these threads hardest, and the fix (owner buy-in or a separate PR) is cheap before pushing and expensive after.
• 🔴 Design Debt — R1, R2, R4, R6, R7, R8, R10's non-crash findings (production sleeps, fire-and-forget ownership, mutex placement), R11 (duplicated discriminators, boundary leaks), R12 (leaked mutable internals, unvalidated setters), and R5 (advisory — never blocks; the user may have valid reasons): fix before commit recommended.
• 🟡 Readability Debt — R3, R9, unclear naming, and the comment-critic's non-KEEP verdicts (trash or over-budget or hard-to-read comments): improves maintainability.
• 🟢 Polish — minor idiomatic improvements, the skeptic's cheaper alternatives.

Every finding carries evidence — file:line plus the falsifying-question answer or command output — never a bare verdict. Effort carries over from the hunter (S/M/L). Fix routing is each rule file's Fix pattern section; cite it, don't restate it. Issues noticed outside the diff scope go in a BROADER CONTEXT section, not as findings.

INCREMENTAL (re-run after fixes): diff scope = only files changed since the last review. Run steps 1–3 on that scope, compare against the previous findings, and report a delta: ✅ Fixed (previous finding no longer reproducible — re-run its detection command to confirm), ⚠️ Remaining (still evidenced), 🆕 New (introduced by the fixes). Use after @refactoring applies fixes or whenever the caller iterates.

🔴 DESIGN DEBT user/service.go:67 | session token travels as raw string; emptiness check inline (R1 Q1: yes; Q2: same predicate at user/auth.go:41 — two owners) | Replace Primitive with Domain Type: SessionToken — skeptic CONFIRMED (score 5) | M user/auth.go:34 | Authenticator.HashCost exported; methods re-check its range (R2 Q1: literal construction possible; Q2: re-check at auth.go:52) | validating constructor NewAuthenticator | S

🟡 READABILITY DEBT user/auth.go:89 | Authenticate() mixes auth flow with bcrypt byte handling (R3 Q1: two abstraction levels in one body) | Extract Step: comparePassword | S user/service.go:15 | godoc restates the name ("UserService provides user services") (critic: toolbox-value floor — no toolbox item delivered) | REWRITE → wider context: "Every user mutation flows through this service — auth, quota, and audit hooks attach here." | S

🟢 POLISH user/auth.go:12 | ComparePasswordWithHash → PasswordMatches — skeptic's cheaper alternative to REFUTED PasswordHash wrapper (score 1: only method unwraps) | S

📝 BROADER CONTEXT user/service.go:23 — email still a raw string (outside diff scope; same R1 pattern).

Caller decides: commit as-is · fix 🔴 first · fix all. Findings are advisory.

</report_example>

<constraints>
This skill MUST NOT:
- Edit code, fix findings, or invoke fix skills (@refactoring, @code-designing, @testing)
- Run the linter or tests — the caller does (see @linter-driven-development)
- Block commits — every finding is advisory; the caller decides what to fix
- Restate rule content — rules live once in `../../rules/`; paste them as spawn payload
  and cite them in findings
- Spawn anything other than `rule-hunter`, `overabstraction-skeptic`, and
  `comment-critic`
</constraints>

<who_invokes>
1. **@linter-driven-development** — Phase 4, pre-commit / per completed vertical slice
2. **@refactoring** — after applying patterns, to validate design quality (INCREMENTAL)
3. **User** — manual standalone review before commit
</who_invokes>