docs: review document for #510

Issue: #510
User-Visible: no

docs/reviews/SPEC-REVIEW-510-r2.md

@@ -0,0 +1,95 @@
+# SPEC-REVIEW-510-r2
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/510
+- **ТЗ:** `docs/specs/510-mutants-on-candidate-and-review-waits-validate.md`
+- **Этап:** spec (S4-spec-review) · заход r2 · блокирующих циклов израсходовано 1/4 до этого раунда
+- **Материал:** ТЗ на HEAD рабочей копии (`5fc235c0`), дельта против материала r1 (`34bf81a1`)
+- **Предыдущий раунд:** SPEC-REVIEW-510-r1, вердикт жёлтый, SHA материала `34bf81a1ed51f041fb3b5b967de84961ce566720`, High 0 · Medium 2 · Low 1
+
+## Скоуп ревью
+
+Разбор — по дельте (PROCESS.md §2.9/§2.10, issue #214): дельта локальна — правка того же ТЗ, той же ветки, без ребейза и без смены подсистемы, объём (5 хунков в одном файле) заметно меньше исходной задачи. Проверял: закрытие всех трёх находок r1 построчно против нового текста и фактического кода, и — поскольку правка r1→r2 сама переписывает межшаговую проводку `process.yml` (id, условия, порядок) — согласованность именно этого фрагмента внутри самого документа, включая соседние предложения, которые дельта не трогала текстуально, но которые описывают тот же контракт AC2. Остальные разделы (§1–§3, §6, §7, §9, §11) дельта не задевает и не меняет их доказательную базу — унаследованы без повторной проверки (см. ниже).
+
+## Как проверялось
+
+- `git diff 34bf81a1..5fc235c0 -- docs/specs/510-mutants-on-candidate-and-review-waits-validate.md` — вся дельта, 5 хунков (§4, §5.1 п.2, §5.2 первое предложение, §8 два пункта, новый §10.0).
+- Каждую находку r1 сверял с новым текстом ТЗ построчно (таблица ниже) и с фактическим порядком шагов в `.github/workflows/process.yml` (`grep -n "id: material|id: reuse|Конфликт с dev|id: validated"`) — реальный порядок в файле: `material`(414) → `reuse`(430) → «Конфликт с dev»(445) → «Зелёные гейты на этом SHA»/`validated`(485). Новое положение `gate`, описанное в ТЗ («после `reuse` и после „Конфликт с dev“, перед `validated`»), соответствует этому порядку — `reuse.outputs` на месте `gate` уже вычислен.
+- Проверил `.github/workflows/validate.yml`, job `changed_mutants`: реальное текущее условие — `needs.changes.outputs.frontend == 'true' || needs.changes.outputs.backend == 'true' || needs.changes.outputs.mutants == 'true'`, а отбор мутантов внутри шага уже идёт через `--changed="$base..$HEAD_SHA"` на шард. Новая формулировка §4 («условие становится ровно `mutants_requested`, отбор по файлам — внутри job») меняет именно этот внешний `if:`, оставляя существующий механизм `--changed` нетронутым — согласуется с не-скоупом §3 («отбор по диффу — без изменений»), поскольку меняется только триггер запуска job, а не то, какие мутанты внутри неё выбираются.
+- Перечитал весь новый фрагмент §5.2 (строки 69–73) и новую строку §8 (строка 99) вместе, а не только изменённые хунки по отдельности — так нашлась находка ниже: три соседних предложения, которые вместе описывают проводку `gate` → `red-return` / `validated` / «все последующие шаги», используют для одного и того же класса условий три разных имени выхода (`result`, `proceed`) с не совпадающей семантикой на ветке `result=skipped`.
+- Продуктовых вопросов владельцу нет и в этом раунде: задача инфраструктурная, `src/**` не затронут (AC6, §10.0 нового раздела).
+
+## Закрытие раунда r1
+
+| Находка r1 | Чем закрыта | Где это видно |
+|---|---|---|
+| Medium 1 — порядок шага `gate` относительно `reuse` не зафиксирован, `reuse.outputs` мог читаться до вычисления | §5.2 переписан: «`gate` стоит после `reuse` (#499) и шага «Конфликт с dev»… порядок в файле — `material` → `reuse` → возврат при конфликте → `gate` → возврат при красном → `validated`» | `docs/specs/510-*.md:69`; порядок сверен с реальными id/строками в `process.yml` (414/430/445/485) — совпадает |
+| Medium 2 — критерий «подходящий прогон» не проверял, что `changed_mutants` реально выполнена и зелёная (принимал `skipped` как доказательство) | §5.1 п.2 переписан: «завершённый success — доказательство только если job «Мутанты по диффу» в нём исполнены и зелёные… зелёный dispatch со `skipped` мутантами… не доказательство — он игнорируется»; §4 меняет условие самой job на `mutants_requested`, чтобы «skipped» однозначно значило «не запрашивали», а не «диффа не было»; новый тест `provesMutants` в §8 (строка ~100) перечисляет ветки job/skipped/red | `docs/specs/510-*.md:60` (§5.1 п.2), `:47` (§4), `:100` (§8, `provesMutants`) |
+| Low 1 — нет явной строки «пользовательское поведение не меняется» и N/A по i18n/UX/модели данных | Новый раздел §10.0 «UX, модель данных, i18n»: «Не затрагиваются: пользовательского поведения нет (`User-Visible: no`), i18n, схема конфига, Store и сетевые API не меняются; release-артефактов нет» | `docs/specs/510-*.md:117-119` |
+
+Все три находки r1 закрыты текстом, а не заявлением: каждая проверена построчно против нового текста и (для Medium 1/2) против фактического кода `process.yml`/`validate.yml`.
+
+## Находки
+
+### Medium (в скоупе) — 1: проводка `gate` → красный-возврат / продолжение ревью использует в трёх соседних предложениях два несовпадающих условия (`result` и `proceed`), и на ветке `result=skipped` (reuse/spec) они дают разный ответ
+
+**Файл:** `docs/specs/510-mutants-on-candidate-and-review-waits-validate.md`, §5.2 (строки 69, 71–73) и §8 (строка 99).
+
+Правка r1→r2 сама вводит явную третью ветку исхода шага `gate` — не только «green»/«red»/«missing», но и «skipped» (reuse==true, stage!=code, либо нет ветки): «внутри: при `stage != code`, `reuse == true` или отсутствии ветки шаг пишет `proceed=true`, `result=skipped` и выходит» (строка 69). Это ровно то, чем закрывается Medium 1: шаг больше не выключается целиком через собственный `if:` (как в r1), а всегда выполняется при отсутствии конфликта и сам решает, пропускать ли реальную проверку. Но три соседних предложения, описывающие, что происходит с результатом этого решения дальше, не согласованы между собой:
+
+- Строка 71 (шаг «Validate красный — вернуть автору без ревью»): условие — `steps.gate.outputs.result != 'green'`. Взято буквально, `'skipped' != 'green'` — истина: этот шаг **сработает и на reuse/skip-ветке**, отправив задачу `S7 → S6` с комментарием «Validate red» даже там, где `gate` намеренно ничего не проверял (например, ровно на этом самом ТЗ-ревью — этап `spec`, где `gate` явно описан как «не проходит», строка 76). Это не гипотетический край: это прямой путь ломки самого механизма #499/#343, ради которого Medium 1 и правился в этом раунде — сценарий #437 r4, который весь этот issue призван устранить, потенциально повторяется, но уже как ложный красный возврат, а не как лишний прогон CI.
+- Строка 73 (шаг `validated`): условие явно названо через другую переменную — «при `proceed == 'true'`» — то есть тот же класс «продолжать ли» здесь завязан на `proceed`, который на skip-ветке тоже `true`.
+- Строка 72 (обобщающий пункт «все последующие шаги ревью… кроме `validated`, который разобран отдельно строкой ниже»): условие сформулировано как `steps.gate.outputs.result == 'green'` — то есть по буквальному тексту эта группа шагов (Chromium, Claude, Review, публикация, решение, слияние) **не выполнится** на skip-ветке (`result=skipped != green`), и это здесь как раз корректно (reuse обязан коротко замкнуть без реального ревью) — но в той же строке добавлена скобка «одно условие — одна переменная: ввести выход `steps.gate.outputs.proceed`», которая предлагает заменить это корректное `result == 'green'` на `proceed`, что для этой группы шагов было бы уже неверно (proceed=true и на skip-ветке тоже).
+- Новая строка теста, §8 (строка 99): «шаги ревью условны по `proceed`; возврат в `S6` при `proceed != true`» — описывает тестируемое поведение через `proceed` для **обеих** групп (и продолжения, и красного возврата), что для группы «продолжение» противоречит корректному `result == 'green'` из строки 72, а для «красного возврата» как раз описывает верное поведение (`proceed != true` исключает skip, в отличие от буквального `result != 'green'` в строке 71).
+
+Итого — документ одновременно называет **три разных** пары условий для двух разных потребителей (красный-возврат, продолжение-ревью), и минимум одна из буквально данных формулировок (строка 71, `result != 'green'` для красного возврата) при реализации по тексту ломает reuse fast-path и spec-этап: пример конкретного провала — этот самый раунд (`stage=spec`) при появлении реального `gate`-шага в конвейере получил бы паразитный `S7→S6`-возврат вместо намеренного пропуска.
+
+**Чем закрывается в этом же ТЗ:** одно место, а не три. Развести по смыслу и явно об этом написать: красный-возврат (строка 71) — `steps.gate.outputs.proceed != 'true'` (эквивалент «result — red или missing», исключает skip); группа «продолжение ревью» (строка 72) — оставить `result == 'green'` буквально и убрать скобку про `proceed` для этой группы (она подходит только `validated`/красному-возврату, не сюда); строка 99 (`провды теста`) — явно развести «условие продолжения — `result == 'green'`» и «условие возврата — `proceed != 'true'`», а не одно слово «proceed» на обе группы. Правка одной формулировкой и синхронизацией трёх мест, отдельный issue не заводится (Medium в скоупе, PROCESS.md §2.4).
+
+## Что проверено и корректно
+
+- Все три находки r1 закрыты по существу и построчно (таблица выше), включая техническую проверку против реального кода `process.yml`/`validate.yml`, а не только внутреннюю согласованность текста.
+- Новое условие job `changed_mutants` (§4) технически совместимо с существующим механизмом `--changed` внутри шага и не расширяет не-скоуп §3 («отбор по диффу — без изменений»): меняется триггер job, а не то, какие мутанты она выбирает. Мотивировка («иначе `skipped` неотличим от «не запрашивали»») прямая и верно нацелена на Medium 2.
+- Новый раздел §10.0 закрывает Low 1 в формате, который требует `docs/specs/README.md` для чистого рефакторинга: явная строка `User-Visible: no` плюс перечисление технических доказательств (i18n/конфиг/Store/сеть не трогаются).
+- Порядок `material → reuse → возврат при конфликте → gate → возврат при красном → validated`, заявленный в §5.2, совпадает со строками 414/430/445/485 в текущем `process.yml` — не домысел, проверено чтением файла.
+- AC1, AC4 не задеты дельтой по существу (текст условий и тестов, на которые они ссылаются, не изменился в способе, которым AC доказывается) — унаследованы.
+
+## Чего не проверял
+
+- Реальный код `scripts/validate-gate.mjs`, изменения `process.yml`/`validate.yml` — их нет, этап spec их не предполагает; `npx tsc --noEmit`/`npm test`/`npm run build` не запускал — дельта этого раунда чисто текстовая правка одного `.md`-файла вне `src/**`/`scripts/**`/`test/**`, эти гейты неприменимы (класс C, PROCESS.md §1). `node scripts/check-docs.mjs` не запускал по той же причине — `src/**` не тронут.
+- Golden/смоки/perf/backend/инварианты модели — задача не трогает геометрию, рендер, состояние комнат или бэкенд; это верно и для дельты, и унаследовано из r1 без повторной проверки.
+- Права токена `HP_PROCESS_TOKEN` (действие GitHub-конфигурации, не код репозитория) — риск назван в §10.1, не менялся дельтой, унаследован из r1.
+
+## Унаследовано из r1
+
+Без повторной проверки в этом раунде — дельта их не касается:
+
+- Обязательные разделы §7.1 присутствуют и на своём месте (сценарий, что человек увидит, скоуп/не-скоуп, контракт поведения, план автотестов, риски, откат) — из SPEC-REVIEW-510-r1, SHA `34bf81a1`.
+- Скоуп/не-скоуп (§2/§3) не расширяются на #479/#480/#499 — из SPEC-REVIEW-510-r1, SHA `34bf81a1`; в этом раунде дополнительно перепроверено, что новая формулировка §4 (условие job) не нарушает не-скоуп «отбор по диффу — без изменений» (см. «Как проверялось»), поэтому граница не чисто унаследована, а подтверждена заново именно в задетой дельтой части.
+- AC1…AC6 указывают способ доказательства, ни один не оставлен без свидетеля — из SPEC-REVIEW-510-r1, SHA `34bf81a1`; AC2 и AC4 частично пересмотрены заново в этом раунде (см. находку выше и таблицу закрытия), AC1/AC3/AC5/AC6 — унаследованы без изменений.
+- Откат (§9) и риски (§10.1) названы предметно — из SPEC-REVIEW-510-r1, SHA `34bf81a1`, текст этих разделов дельтой не тронут.
+- AC4 (протокольные мутанты) реалистичен, реестр `scripts/mutation-gate.mjs` уже содержит нужные мутанты — из SPEC-REVIEW-510-r1, SHA `34bf81a1`.
+
+## Вердикт
+
+High: 0 · Medium (в скоупе): 1 · Low: 0.
+
+Единственная находка — не новая ошибка в новом механизме, а рассогласование формулировок внутри собственной правки r1→r2: три соседних предложения одного и того же ТЗ по-разному называют условие, решающее судьбу reuse fast-path и spec-этапа, и минимум одна из буквальных формулировок (`result != 'green'` для красного возврата) при реализации по тексту как есть отправляла бы reuse- и spec-заходы в паразитный `S7→S6`. Конкретный, воспроизводимый по тексту документа и коду `process.yml`, закрывается в этом же ТЗ одной синхронизацией трёх формулировок.
+
+**Вердикт: жёлтый.** Возврат в «ТЗ в работе» для уточнения §5.2 (строки 71–73) и §8 (строка 99); далее — повторный цикл ревью по дельте (PROCESS.md §2.9/§2.10).
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `issue/510-mutants-on-candidate-and-review-gate`, коммит `5fc235c0e8de` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `36597a771026ea7ffcc6f6558b0011f17df52ed9`
+  ```
+  git log --all --format='%H %T' | grep 36597a771026
+  ```
+- ТЗ `docs/specs/510-mutants-on-candidate-and-review-waits-validate.md`, блоб `819b325105f1828ccfb18355e8a3239f6c71b636`
+  ```
+  git log --all --find-object=819b325105f1828ccfb18355e8a3239f6c71b636 -- docs/specs/510-mutants-on-candidate-and-review-waits-validate.md
+  ```
+- Вердикт конвейера: `yellow` · High 0