docs: review document for #510

Issue: #510
User-Visible: no

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

@@ -0,0 +1,102 @@
+# SPEC-REVIEW-510-r3
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/510
+- **ТЗ:** `docs/specs/510-mutants-on-candidate-and-review-waits-validate.md`
+- **Этап:** spec (S4-spec-review) · заход r3 · блокирующих циклов израсходовано 2/4 до этого раунда
+- **Материал:** ТЗ на HEAD рабочей копии (`3854fe72ac59b1e37a80b25faea0f8468356597a`), дельта против материала r2 (`5fc235c0e8de`)
+- **Предыдущий раунд:** SPEC-REVIEW-510-r2, вердикт жёлтый, SHA материала `5fc235c0e8de`, High 0 · Medium 1 · Low 0
+
+## Скоуп ревью
+
+Разбор — по дельте (PROCESS.md §2.9/§2.10, issue #214): дельта локальна — правка того же ТЗ, той же ветки, без ребейза и без смены подсистемы, объём (3 предложения в одном разделе, `git diff --stat` — 4 вставки/4 удаления в одном файле) на порядок меньше исходной задачи. Но, как и в r2, дельта сама переписывает межшаговую проводку `process.yml` (§5.2) — контракт, по которому ветвятся все шаги ревью после `gate`. Поэтому разбор по дельте здесь не сводится к чтению трёх изменённых строк текста: нужно (1) закрыть находку r2 построчно, как и предписано форматом, и (2) проверить новую формулировку контракта против реального `process.yml` и существующих тестов — именно так r2 нашла свою находку не в новом тексте, а в столкновении нового текста со старым. Разделы, которых дельта не касается текстуально и чей контракт не меняется этой правкой (§1–§4, §5.1, §6, §7, §8 кроме связки со строкой 72, §9, §10.0, §10.1, §11, §12), не перепроверялись — унаследованы (см. ниже).
+
+## Как проверялось
+
+- `git diff 5fc235c0..3854fe72 -- docs/specs/510-mutants-on-candidate-and-review-waits-validate.md` — вся дельта, один хунк из 3 изменённых предложений в §5.2 (строки 69, 71, 72; строка 73 — только добавление «на этапе code», не меняющее её смысл).
+- `git show 3854fe72` — сообщение коммита прочитано целиком, а не только дифф: «All step conditions now branch on `proceed` only (true = green or skipped, false = red/missing)» — это явная формулировка проектного решения, а не побочный эффект правки формулировок, и именно она проверялась.
+- Находка r2 (единственная) сверена построчно с новым текстом (таблица ниже).
+- Реальный `process.yml` (`grep -n "steps.reuse.outputs.reuse\|id: reuse\|id: gate\|Установить зависимости\|name: Review"`) — нынешние условия шагов «Установить зависимости», кэш/установка Chromium (пять мест) и шага `Review` (вызывающего модель) — везде `steps.rebase.outputs.conflict != 'true' && steps.reuse.outputs.reuse != 'true'`, не только `rebase.conflict != 'true'`, как упрощённо называет это ТЗ.
+- `test/review-doc-guard.test.mjs:592-603` — существующий (не из этой задачи) тест «конвейер: зелёный вердикт применяется повторно без модели» жёстко требует `assert.match(reviewStep, /steps\.reuse\.outputs\.reuse != 'true'/, 'модель не вызывается при повторном применении')`. Этот тест в §11 ТЗ №510 назван как затрагиваемый файл, но ни разу не упомянут в §8 (плане тестов) как изменяемый или заменяемый.
+- Перечитан весь фрагмент §5.2 целиком (строки 69–76), а не только изменённые предложения по отдельности — так же, как это делала r2 при поиске своей находки.
+
+## Закрытие раунда r2
+
+| Находка r2 | Чем закрыта | Где это видно |
+|---|---|---|
+| Medium 1 — три соседних предложения называли условие продолжения/возврата тремя разными именами (`result`/`proceed`), и буквальное `result != 'green'` для красного возврата (строка 71 r2) давало бы паразитный `S7→S6` на skip-ветке (spec/reuse) | **Частично.** Красный-возврат (строка 71) переписан на `steps.rebase.outputs.conflict != 'true' && steps.gate.outputs.proceed != 'true'` — на skip-ветке `proceed == 'true'`, шаг не срабатывает, паразитный возврат устранён. Формально устранена и текстовая нестыковка трёх формулировок: строка 69, строка 71 и строка 72 теперь одинаково используют `proceed`, а не смесь `result`/`proceed`. | `docs/specs/510-*.md:71` (красный-возврат — исправлено верно); `:69,72` (унификация терминов — сделана) |
+| — та же находка, часть про строку 72 («все последующие шаги ревью») | **Не закрыта, закрыта неверно.** r2 явно предлагала для этой группы **оставить** `result == 'green'` буквально, потому что она (в отличие от красного-возврата) обязана оставаться заблокированной на skip-ветке — reuse обязан коротко замкнуть без реального ревью. r3 вместо этого заменила условие всей группы (`validated`, зависимости, Chromium, Claude, Review, публикация, решение, слияние, перестановка метки) на `proceed == 'true'`, а `proceed == 'true'` истинно и на skip-ветке (reuse) по определению той же строки 69 («`true` = green **или** skipped»). Это не formальность: см. находку ниже — это открывает шаг `Review` (вызов модели) и установку зависимостей/Chromium на reuse-ветке, где сейчас они не выполняются вовсе. | `docs/specs/510-*.md:72` (новая формулировка); противоречит реальному `process.yml` (условия шагов «Установить зависимости», кэш/установка Chromium, `Review`) и `test/review-doc-guard.test.mjs:596` |
+
+Находка r2 закрыта только в своей «текстовой» части (три формулировки сведены к одному слову) — но одна из трёх сведена к неверному значению. Синтаксическая цель ревью r2 («одна переменная — одно условие») достигнута; содержательная («группа „продолжение ревью“ не должна срабатывать на skip-ветке») — нарушена этим же изменением.
+
+## Находки
+
+### Medium (в скоупе) — 1: унификация «всё через `proceed`» ломает reuse-fast-path (#499) — деплой-зависимости, Chromium и вызов модели получили бы условие, истинное и на ветке, где их сейчас не выполняют вовсе
+
+**Файл:** `docs/specs/510-mutants-on-candidate-and-review-waits-validate.md`, §5.2, строки 69 и 72.
+
+Строка 69 вводит абсолютное утверждение: «Единственная переменная, по которой ветвятся последующие шаги, — `proceed`». Строка 72 применяет это к группе «все последующие шаги ревью (`validated`, зависимости, Chromium, Claude, Review, публикация, решение, слияние, перестановка метки)» — их условие меняется на `steps.gate.outputs.proceed == 'true'` **вместо** нынешнего `steps.rebase.outputs.conflict != 'true'`.
+
+Но нынешнее условие этих конкретных шагов в реальном `process.yml` — не `rebase.conflict != 'true'` в одиночку, а `steps.rebase.outputs.conflict != 'true' && steps.reuse.outputs.reuse != 'true'` (проверено чтением файла: «Установить зависимости», пять мест кэша/установки Chromium, шаг `Review`, вызывающий модель, — везде оба условия). ТЗ называет только первый конъюнкт как заменяемый и ни словом не упоминает второй — а коммит r3 («All step conditions now branch on `proceed` only») прямо формулирует замену как тотальную, не частичную.
+
+Раз `proceed` по той же строке 69 истинен и на skip-ветке (`stage != code`, `reuse == true` или нет ветки — «`true` = green **или** skipped»), буквальная реализация строки 72 даёт: на reuse fast-path (`reuse == true`) условие `proceed == 'true'` истинно → шаги «Установить зависимости», кэш/установка Chromium и `Review` (вызов модели) **выполнятся**, хотя сегодня они не выполняются вовсе — в этом и была точка #499: «вердикт прошлого захода применяется повторно: модель не вызывается» (комментарий в `process.yml` перед шагом `reuse`). Конкретное воспроизведение: `test/review-doc-guard.test.mjs:592-603`, тест «конвейер: зелёный вердикт применяется повторно без модели», жёстко требует `steps.reuse.outputs.reuse != 'true'` именно в условии шага `Review` — при реализации §5.2 буквально по тексту r3 этот существующий тест либо ломается, либо реализация должна тихо отступить от буквального текста ТЗ и сохранить `&& steps.reuse.outputs.reuse != 'true'` вопреки заявленному «единственная переменная — `proceed`». Ни то, ни другое ТЗ не называет явно; `test/review-doc-guard.test.mjs` при этом перечислен в §11 как затрагиваемый файл, но конкретно эта проверка не упомянута в §8 ни как сохраняемая, ни как заменяемая.
+
+Здесь же собственное противоречие внутри абзаца: строка 73 (не изменена по смыслу в этом раунде) описывает шаг `validated` как срабатывающий «при `proceed == 'true'` **на этапе code**» — то есть его поведение зависит не только от `proceed`, но и от `stage`, отдельной переменной. Заявление строки 69 «единственная переменная — `proceed`» опровергается уже следующим абзацем того же документа.
+
+Воспроизведение (конкретный сценарий, не гипотетический): round задачи применяет reuse (зелёный вердикт прошлого захода переносится без вызова модели, ровно сценарий #437/#499, ради которого и заведён этот механизм) → `gate` пишет `proceed=true`, `result=skipped` (строка 69) → по буквальному тексту строки 72 шаг `Review` получает условие `proceed == 'true'` → модель вызывается повторно на том же дереве, за которое уже заплачен один заход, что и есть ровно тот перерасход, который #499 закрыл, только на новом месте.
+
+**Чем закрывается в этом же ТЗ:** явно развести, как и предлагала r2, но с учётом факта, найденного в этом раунде — существующего `reuse` конъюнкта, о котором ТЗ не сказало ни слова:
+- красный-возврат (строка 71) — `proceed != 'true'` (эта часть уже верна, менять не нужно);
+- группа «продолжение ревью» (строка 72) — либо явно сохранить существующий `&& steps.reuse.outputs.reuse != 'true'` в дополнение к `proceed == 'true'` для тех конкретных шагов, что сегодня его несут (зависимости, Chromium, `Review`), либо (эквивалентно и проще для «одной переменной») зафиксировать, что `gate` пишет `proceed=false`, а не `true`, на reuse-ветке — но тогда нужно менять и красный-возврат (строка 71), который на этой ветке не должен сработать, то есть развести «reuse» как третье, отдельно поименованное значение, а не смешивать его с `skipped`-для-spec;
+- строку 69 («Единственная переменная — `proceed`») — либо снять как неточную (строка 73 её же опровергает), либо явно ограничить область действия: «единственная переменная, добавляемая этим ТЗ» — не единственная вообще.
+Правка — уточнение слов в тех же трёх местах плюс явное упоминание существующего `reuse`-конъюнкта, отдельный issue не заводится (Medium в скоупе, PROCESS.md §2.4).
+
+## Что проверено и корректно
+
+- Красный-возврат (строка 71) исправлен верно и именно так, как предлагала r2: `proceed != 'true'` не срабатывает на skip-ветке — паразитный `S7 → S6` для spec/reuse-заходов устранён.
+- Формальная (лексическая) нестыковка трёх формулировок из r2 устранена: строки 69/71/72 больше не называют одно и то же разными именами.
+- Остальной текст §5.2 (порядок шага `gate` относительно `reuse`/«Конфликт с dev», алгоритм `validate-gate.mjs`, §5.1, §5.3) дельтой не тронут и остаётся тем, что уже проверено в r1/r2 построчно против кода.
+- AC1, AC3, AC4, AC5, AC6 дельтой этого раунда не задеты по существу — унаследованы.
+
+## Чего не проверял
+
+- `npx tsc --noEmit`, `npm test`, `npm run build`, `node scripts/check-docs.mjs` — не запускал: дельта r2→r3 — 4 вставки/4 удаления в одном `.md`-файле вне `src/**`/`scripts/**`/`test/**`, класс C (PROCESS.md §1), гейты неприменимы; то же самое было верно для r1 и r2, поведение не изменилось.
+- Golden/смоки/perf/backend/инварианты модели — задача не трогает геометрию, рендер, состояние или бэкенд; не изменилось с r2.
+- Права токена `HP_PROCESS_TOKEN`, риски §10.1 — не менялись дельтой, унаследованы из r1/r2.
+- Реальная реализация `process.yml`/`validate-gate.mjs` — их нет, этап spec их не предполагает; находка выше проверена **чтением существующего** `process.yml` и существующего теста, не запуском.
+
+## Унаследовано из r2
+
+Без повторной проверки в этом раунде — дельта их не касается:
+
+- Обязательные разделы §7.1 присутствуют и на своём месте — из SPEC-REVIEW-510-r1 (SHA `34bf81a1`) и подтверждено в SPEC-REVIEW-510-r2 (SHA `5fc235c0`).
+- Скоуп/не-скоуп (§2/§3), включая границу с #479/#480/#499, и совместимость условия job `changed_mutants` (§4) с существующим механизмом `--changed` — из SPEC-REVIEW-510-r2, SHA `5fc235c0`; дельта этого раунда §4 не касается.
+- Раздел §10.0 (UX/i18n/модель данных не затрагиваются) — из SPEC-REVIEW-510-r2, SHA `5fc235c0`; текст не менялся.
+- Порядок шага `gate` относительно `material`/`reuse`/«Конфликт с dev» (§5.2, первое предложение) — сверен в r2 с реальными строками `process.yml` (414/430/445/485), дельтой r3 не тронут — из SPEC-REVIEW-510-r2, SHA `5fc235c0`.
+- Алгоритм `scripts/validate-gate.mjs` (§5.1), включая закрытие Medium 2 из r1 («доказательство только при исполненных и зелёных job мутантов») — из SPEC-REVIEW-510-r1 (SHA `34bf81a1`) и SPEC-REVIEW-510-r2 (SHA `5fc235c0`), текст §5.1 дельтой r3 не тронут.
+- §6 (слияние кандидата), §7 (правила хендоффа), §9 (откат), §10.1 (риски), §11 (затронутые файлы), §12 (предположения) — из SPEC-REVIEW-510-r1/r2, текст не менялся ни в r2, ни в r3.
+- AC1, AC3, AC4, AC5, AC6 — из SPEC-REVIEW-510-r1/r2, способ доказательства не изменился.
+
+## Вердикт
+
+High: 0 · Medium (в скоупе): 1 · Low: 0.
+
+Находка r2 закрыта только частично: красный-возврат исправлен верно, но формулировка «единственная переменная — `proceed`», применённая ко всей группе «продолжение ревью» (включая установку зависимостей, Chromium и вызов модели), конфликтует с реальным кодом `process.yml` (условия этих шагов сегодня требуют ещё и `steps.reuse.outputs.reuse != 'true'`) и с существующим тестом `test/review-doc-guard.test.mjs:596`, который жёстко фиксирует, что модель не вызывается на reuse-ветке (#499). Буквальная реализация текста r3 открывает ровно тот перерасход (повторный вызов модели на уже принятом дереве), ради устранения которого заведён #499. Это не гипотетика — сценарий воспроизведён по коду и существующему тесту, а не по документу самому по себе. Закрывается в этом же ТЗ уточнением трёх формулировок §5.2 с явным упоминанием `reuse`-конъюнкта.
+
+**Вердикт: жёлтый.** Возврат в «ТЗ в работе» для уточнения §5.2 (строки 69, 72); далее — повторный цикл ревью по дельте (PROCESS.md §2.9/§2.10).
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `issue/510-mutants-on-candidate-and-review-gate`, коммит `3854fe72ac59` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `45ab656b9462fc4f8a3a6f5eaad139d4b14d9f99`
+  ```
+  git log --all --format='%H %T' | grep 45ab656b9462
+  ```
+- ТЗ `docs/specs/510-mutants-on-candidate-and-review-waits-validate.md`, блоб `66bd41844726bd574d4fe59b506cd1f4389c34f8`
+  ```
+  git log --all --find-object=66bd41844726bd574d4fe59b506cd1f4389c34f8 -- docs/specs/510-mutants-on-candidate-and-review-waits-validate.md
+  ```
+- Вердикт конвейера: `yellow` · High 0