# Pre-commit diff review sweep (BLOCKING — before the executor's final commit) This is where the preflight's conventions get enforced against the code you actually wrote. Run it once, after the last `Edit` / `Write` of the stage, before the final commit. Fix every finding in place (you are the executor — you may edit); this is a prevention pass, not a report you hand off. Do not scan holistically and stop when it "looks fine". Work the matrix exhaustively — the failure mode this gate exists to prevent is a real defect surviving because you eyeballed the diff instead of enumerating it. ## Method — enumerate the diff, then walk every (file × applicable-rule) cell 1. **List every changed file:** `git diff --name-only ..HEAD`. That list is your worklist — cover it to the end; no sampling. 2. **Classify each file and select its rule set** from the conventions the preflight already loaded (do NOT re-derive the rules here — read them from the routed pack): - any source file → `clean-code.md`: truthful + standalone names, one identifier one meaning per file (a key parameter must not take the name a sibling signature gives the entity), plain-English summary test, single-purpose ≤50-line functions, DRY (incl. scattered domain literals + documented forks), YAGNI, no magic numbers, shallow nesting, comments explain why. - The YAGNI cell also counts callers, not just their existence: an abstraction layer this diff *introduces* — helper module, strategy / factory / builder, indirection or wrapper layer, interface or abstract base — with exactly **one** caller after the change is an inline candidate under `overview.md` core principle 2 ("name the second caller now, or inline"). Collapse it into that call site unless one of four exits applies and you record which: the approved plan declares it as a test seam or variation-point extraction, the project-context preflight projection declares hexagonal architecture and it is a port at the domain boundary, it breaks a real import cycle, or its single caller is a published package's public API. A function extracted only to get a body under the 50-line cap is not this finding. - The YAGNI cell is a grep, not an impression: for every identifier this diff adds — and every one whose last caller it removes — search the whole repository and name the caller you found. A declaration-only, test-only, or commented-out hit is not a caller. Zero callers means delete it here (or fold an added parameter back into its single call site), unless the approved plan reserves it for a named later stage or something outside project code calls it (framework entrypoint, implemented interface method, migration hook, published-package API) — in which case record that justification in the audit note. Fixing it now is cheaper than the verifier's Static design gate, where the same finding is blocking and fails the stage (`_implementation-verifier.md` "Caller-less identifier"). - a file that wraps a third-party / library call → `clean-code.md` "Wrapping a third-party call": the wrapper adds behaviour the library does not already provide (read the installed library source before keeping a recovery branch — a `catch` repeating the library's own retry recovers nothing), no comment names a condition the call site does not establish, and a rethrow keeps the original error as `cause`. - a file that decides, mutates, or persists state → `clean-code.md` "Mutation and state boundaries": decide on the direct identifier rather than a status/flag proxy, capture before-state in one snapshot ahead of the mutating boundary, update only this work's owned fields on an existing row, re-read state before calling a zero-affected-rows write success or failure, put priority-between-inputs in a named domain function, and keep error messages to what was actually observed. Also check that no state union/enum was re-declared beside an authoritative one the domain or a dependency exports. - test file (`*.spec.*` / `*.test.*` / `test_*.py` / `*_test.go` …) → `clean-code.md` "Testing discipline": no self-mocking of the SUT, behavioral (outcome) assertions not interaction-only, no tautological delegation assertion, no effect claimed under its own mock, shared-fixture defaults left on the ordinary path, setup values that actually separate the scenarios, every new test helper/mock used by a test in this same diff, no positional mock-argument access (`rg 'mock\.calls'`), every branch this diff adds covered by a test that fails when the branch body is deleted, assertions on the last write to a record rather than an intermediate one, and test titles naming their unit plus the single condition each case isolates. - port / adapter / domain file, when the hexagonal overlay is loaded → `architectures/hexagonal.md`: no business logic in a port body, adapter methods are I/O only (no post-fetch filtering on domain state, no `findValid*`/`findActive*` names hiding a rule), domain objects declared under the domain boundary, no changed domain file importing an ORM / framework / adapter / service (read the import list — mechanical), and a service dependency you add or modify goes through a port rather than a concrete adapter (advisory only while the project has not declared `architecture.style = hexagonal` in `.okstra/project.json` — record it with the port sketch; blocking under that declaration, so fix it in place before the commit rather than record-and-pass, exactly as `_implementation-verifier.md` re-grades this same diff. Either way, the codebase already injecting concrete classes is the debt this pays down, not a reason to skip it). - a service file, when the hexagonal overlay is loaded → `architectures/hexagonal.md` Rule H6: a decision this diff puts in a service — an `if`/`else` chain over domain fields, a business formula, a private method named for a domain concept — belongs in a domain function. Orchestration control flow, DTO mapping and calling a domain predicate are not violations. - every changed source file → `clean-code.md` "Trace what this change can do wrong": walk the error, partial, concurrent and selection paths this diff creates to their end and fix wherever a wrong result comes out. Name the input that produces it — if you cannot name one, there is nothing to fix here and you move on rather than restructuring code that works. A file can hold several roles — apply every rule set that fits it. 3. **Decide clean-or-finding for each cell.** Read the full file when a rule needs context (never judge a port/adapter/domain or a naming rule from the hunk alone). 4. **Fix each finding in place** before the commit. When a readability finding is real, the fix is a named helper or named intermediate value — sketch the cleaner shape (a few lines) in your audit note, then apply it. When the fix is genuinely out of this stage's scope, record it as an `Out-of-plan` note instead of silently leaving it. ## Coverage footer (BLOCKING deliverable) End your audit-sidecar entry for this sweep with a one-line `Coverage:` footer naming each file you examined, the rule set(s) you applied to it, **and the outcome of each** — `clean` when you looked and found nothing, or `fixed ` when you fixed findings in place: `Coverage: users.service.ts [names: clean, plain-english: fixed 1], users.repository.adapter.ts [hexagonal: fixed 1, names: clean], users.service.spec.ts [testing-discipline: clean]` `clean` is a result you assert, not a blank you leave. Naming the rule set without its outcome reads identically whether you checked it or skipped it, which is exactly what this footer exists to distinguish — the completion self-check and any reviewer read it to confirm nothing in the diff was skipped. A sweep with no footer, or a footer whose entries carry no outcome, is an unfinished sweep. ## Graceful degradation When the routed coding-preflight pack is unreadable (codex / antigravity runtime, or the files are absent), do NOT skip the sweep — fall back to the always-binding principles the preflight enumerates plus the project's `CLAUDE.md` / `CONTRIBUTING` / lint config, and record `diff-review: resource-unavailable → applied ` with the Coverage footer.