git:20260926.947eac0 to git:20260927.b24cc01

- model: sonnet

- Reply in the project conversation language (CLAUDE.md → Language); code, identifiers, paths and commit messages stay in English.

- Read-only plus running checks; fixes only on a separate request from the user.

- In the steps below, `<default>` is the default branch (`master` or `main`); `<base>` is `$(git merge-base origin/<default> HEAD)`, taken after `git fetch origin`; `<id>` is the story ID (`S-NNN`); without a story it is the scope of the branch's own fix commit when there is one (a `hotfix/<slug>` branch: the `<scope>` of its `fix(<scope>): …` commit), else the branch name; `<hooks>` is `.claude/hooks/` in copy mode and `${CLAUDE_PLUGIN_ROOT}/hooks/` in plugin mode; `$TMPDIR` falls back to `/tmp` when unset. A reviewer's `subagent_type` is `web-studio:<name>` in plugin mode and `<name>` in copy mode.

- 1. **Map files to reviewers** by extension/path: `*.go` → `go-engineer` (+ `backend-lead`); `*.php` → `php-engineer`; `*.graphql`/resolvers → `graphql-engineer`; Angular → `angular-engineer`; `*.vue` → `vue-engineer`; `*.css/scss` → `css-engineer`; `game/`, three.js → `threejs-engineer`/`web-game-engineer`; migrations/SQL → `database-engineer`; workflows/Docker/`Makefile` → `devops-engineer`; tests → `test-engineer`.

- 3. **Write the diff once**: `git diff <base> > "$TMPDIR/review-<id>.diff"`, plus `git diff -M --name-status <base>` for the file list. A reviewer that reads the diff through `sed -n` in pieces runs out of turns on a third of the diffs and returns a verdict about the part it reached.

- 4. **Print the routing table** before the reviewers run: extension or path in the diff → the reviewer this list requires → spawned yes/no. A required reviewer may be skipped, but only as a line saying so and why; chosen "by eye", routing quietly shrinks to one reviewer on a diff that touches three layers.

- 5. **Spawn the reviewers** in one parallel batch. Each gets the diff path, the file list, the ADRs, the story's acceptance criteria (when there is a story) and the rules, and returns findings as `severity | file:line | what | risk | fix`.

-

- **General**: `go vet`/`staticcheck`/`govulncheck`; PHP: `php -l`, the recorded analyser (`vendor/bin/phpstan analyse` or `vendor/bin/psalm`), the recorded coding-standard tool (`vendor/bin/ecs check` or `php-cs-fixer fix --dry-run`); `eslint`/`tsc --noEmit`/`vue-tsc`; `graphql-inspector diff`; tests of affected packages.

-

- **Go tests**, on every `_test.go` in the diff (files importing `testing/synctest` and comment lines excluded):

- - `grep -n 'time\.Sleep(' <files>` → WARNING `TEST-SLEEP | file:line | fixed wait in a test | flaky under load | poll with a deadline or testing/synctest`.

- - `grep -nE '\.Error\(\) *[!=]=' <files>` → WARNING `TEST-ERRSTR | file:line | error compared as a string | breaks on the first %w wrap | errors.Is/As against the sentinel`.

- - A Go test function with no `t.Run` and more than one scenario → INFO `TEST-TABLE`.

-

- **PHP tests**, on every `*Test.php` in the diff:

- - `grep -nE '(^|[^>[:alnum:]_])u?sleep\(' <files>` → WARNING `TEST-SLEEP`.

- - An `assertSame`/`assertEquals`/`assertStringContainsString` on `getMessage()` → WARNING `TEST-ERRSTR | file:line | exception asserted by message | breaks on a wording change | expectException(Class::class)`.

- - A `TestCase` with several `test*` methods that differ only in data → INFO `TEST-TABLE (data provider)`.

-

- **PHP layered** (`php_architecture: layered`). The numbers go into the report even when clean.

- - `composer arch-check` (deptrac): a violation is BLOCKING `LAYER | file:line | <layer> depends on <layer> | the rule the architecture rests on | move the code; a value library the domain genuinely needs goes into php_domain_allow through technical-preferences and /test-setup, never into deptrac.yaml by hand`.

- - `composer coverage-gate` on the clover report of the tests already run (`composer test:coverage` writes it), only when the diff touches `src/Domain/` or `src/Application/`: a layer below its threshold is BLOCKING `COVERAGE`.

- - A class changed under `src/Domain` or `src/Application` with no test change in the diff → WARNING `TEST-LAYER`.

- - A `*Test.php` under `tests/Unit/Domain` that creates a mock, or one under `tests/Unit/Application` that boots the framework or opens a connection → WARNING `TEST-LAYER`.

- - An entity with public writable state and its rules in a use case or action → WARNING `RICH-MODEL`.

- - An action or resolver that injects a repository instead of a use case → WARNING `LAYER`.

- - A framework namespace (the `FRAMEWORK_NAMESPACES` of the recorded `php_framework`) imported outside `src/Infrastructure` → BLOCKING `LAYER`.

-

- **Go layered** (`go_architecture: layered` in technical-preferences). The numbers (lint issues, gate lines) go into the report even when clean.

- - `golangci-lint run --new-from-rev=<base> ./...` — only the issues the diff introduces; the whole-module run belongs to `make ci` and the engineer's own result. A `depguard` line is BLOCKING `LAYER | file:line | <layer> imports <outer layer> | the rule the architecture rests on | move the code; a value library the domain genuinely needs goes into go_domain_allow through technical-preferences and /test-setup, never into the linter config by hand`.

- - `make coverage-gate` on the profile of the tests already run (`make test` writes it), only when the diff touches `internal/domain/` or `internal/usecase/`: a layer below its threshold is BLOCKING `COVERAGE | internal/<layer> | N % < M % | untested rules | tests in the same story`.

- - A file changed under `internal/domain` or `internal/usecase` with no `_test.go` change in the diff → WARNING `TEST-LAYER`.

- - A `_test.go` under `internal/domain` that declares a mock/fake type, or one under `internal/usecase` importing `pgx`, `testcontainers`, `net/http` or `database/sql` → WARNING `TEST-LAYER`.

- - An entity with exported mutable state and its rules in a use case or resolver → WARNING `RICH-MODEL`.

- - A resolver/handler that imports `internal/infrastructure/postgres` (or any repository) instead of a use case → WARNING `LAYER`.

-

- **Go layout** (`go.md` "Project layout"), whenever the diff touches `cmd/` or the project has one. The numbers go into the report even when clean; a code comment calling the code "wiring" changes nothing.

- - `wc -l cmd/*/*.go` and `grep -ln 'flag\.\|Fprint' cmd/*/*.go`: a second non-test file in `cmd/<app>`, a `main.go` over 50 lines or a `flag.`/`Fprint` hit there is a WARNING `LAYOUT | cmd/<app>/<file>:1 | application code in cmd/ | grows with every story, untestable without package main | move to internal/app/<app>`.

- - The same dependency-graph struct literal in two files is BLOCKING when a field is already missing in one of them (that difference is the bug), WARNING otherwise (fix: one constructor).

-

- **Scope** (CLAUDE.md principle 9), with a story. Every changed line should trace to one of the story's acceptance criteria or to a check the story must pass. The numbers go into the report even when clean.

- - The parent prints `git diff --stat <base>` next to the story's files (its Tasks, the criteria table's Test column): a file in the diff that the story does not name is listed, not yet judged.

- - The reviewers judge the hunks. A hunk that serves no criterion — a refactor of neighbouring code, a fixed pre-existing lint issue, a renamed identifier the story does not touch → WARNING `SCOPE | file:line | change outside the story's criteria | review cost, an unowned behaviour change, merge conflicts with other stories | revert it here; record it with /backlog add or in production/findings.md`.

- - An option, abstraction or error path no criterion asks for → WARNING `SCOPE-SPEC | file:line | speculative code | code with no test that pins it | remove it, or add the criterion through /impact`.

- - A behaviour the story's new code exposes — a new route, a new input reaching old code — is inside the criteria even when the lines that misbehave are old: SCOPE covers changes, not consequences. Such a finding keeps the severity its reviewer gave it and is never downgraded to INFO as "pre-existing".

- - Reformatting, reflowed comments or reordered imports in code the story does not otherwise change → INFO `SCOPE-STYLE`.

- - Not findings: removing what this change itself made unused, the tests for the criteria, files a tool regenerated (lockfiles, generated code), and the changes a `/code-review` fix round was asked for.

-

- Comments, on the added lines of the diff (`grep -nE '^\+' "$TMPDIR/review-<id>.diff"`), three checks; the counts go into the report even when zero, and an existing file's old comments are not a finding of this review (`rules/comments.md`: new code only):

- - a `TODO`/`FIXME`/`HACK` without `(S-NNN)`/`(I-NNN)` → WARNING `TODO-NOID | file:line | TODO without a story or idea id | never reaches /tech-debt, forgotten at merge | TODO(S-NNN): … after /backlog add`;

- - a comment line (test files excluded; the well-formed `TODO(S-NNN)`/`TODO(I-NNN)` lines of the first check skipped with `grep -vE '(TODO|FIXME|HACK)\((S|I)-[0-9]+\)'`) matching `\b(S|I|OPS|ARCH|SEC|GEO|TL)-[0-9]+\b|pre-S-|used to|previously|was .* until|/code-review|review (round|found|raised|asked)|out of (this|the) story` → INFO `COMMENT-HISTORY | file:line | comment tells the story's history | stale at merge, read as a requirement later | keep the contract or the reason, the rest is in the PR (rules/comments.md)`;

- - a doc comment on an exported Go symbol longer than 15 lines → INFO `COMMENT-LONG | file:line | doc comment is a design note | rendered by go doc as the API | contract above, reason in the body, the rest to an ADR`.

- 1. **Report**: the BLOCKING/WARNING/INFO summary; the routing table from Phase 2 with each reviewer's verdict next to it (a review whose reviewers are invisible cannot be audited later — `production/session-logs/agent-audit.log` names who actually ran); the findings table; the verdict `APPROVED` / `NEEDS CHANGES`.

- 3. **Fixes go through specialists**: the relevant engineer via `Task` for code and tests, `tech-writer` for documents. The answer covers the findings it names; a specialist that needs to go beyond them reports back and the parent asks. The parent writes no code in review rounds either (coordination-rules § Subagents), because "faster to fix it myself" is how review rounds drift. A fix the parent wrote anyway is named in the report ("written by the parent: <finding>").

- 5. **Commit gate**, one `AskUserQuestion` (`.claude/docs/git-workflow.md`, step "Review"), recorded first as `Gate "/code-review Phase 5: commit the fixes?"` in the session state (`<hooks>session-state.sh set Gate …`, rule 7: an open gate survives the next turn) and cleared after the answer (`set Gate "—"`): on "yes", `git commit -m "fix(<id>): apply /code-review findings"` (`fix(S-NNN): …` on a story branch, `fix(<scope>): …` on a hotfix branch) staging the fixed files and `.claude/agent-memory/` (the reviewers' and engineers' notes from this round, git-workflow § Agent memory) and `git push` on the current branch — only when it has an upstream (`git rev-parse --abbrev-ref @{u}`); without one, the commit stays local and the report says so instead of a failed push. No fixes → no commit. The review itself never commits or changes the branch.

- Next step — one `AskUserQuestion`, never a bare "run /story-done?": `/web-studio:story-done` (copy mode `/story-done`) (Recommended on APPROVED) · re-review after manual fixes — past 50 % context in a fresh session: `/clear`, then `/web-studio:code-review --diff` (copy mode `/code-review --diff`) (rule 13) · stop here.