diff --git a/docs/reviews/CODE-REVIEW-595-r1.md b/docs/reviews/CODE-REVIEW-595-r1.md new file mode 100644 index 00000000..483a0c5f --- /dev/null +++ b/docs/reviews/CODE-REVIEW-595-r1.md @@ -0,0 +1,146 @@ +# CODE-REVIEW-595-r1 + +**Issue:** [#595](https://github.com/Matysh/houseplan-card/issues/595) — Тест #573 краснеет на любом коммите приёмки эталонов: заглушка `loadContext` не моделирует `Baseline-Reviewed` run. +**Класс:** B (инфраструктура/гейты) — ни одного файла класса A. Вход сразу на `S7-code-review` (#562), спецификация не требуется, AC заданы в теле issue. +**Заход:** r1 · блокирующих циклов израсходовано 0 из 4. +**Материал:** `eb145aba485fe62d363298bb4b09d33bd0bb096e` (рабочая копия пинована на этом SHA; `git fetch`/`checkout` на другой коммит не выполнялся). +**Diff:** `scripts/mutation-registry.mjs` (+12), `test/release-gate.test.mjs` (+72/-4). Никаких изменений в `src/**`, `custom_components/**`, `dist/**`. + +## Скоуп + +Дефект: `test/release-gate.test.mjs` строил ожидания composite-evidence (`own`) из +**живого** `HEAD` через `candidateExpectations`, а `localEvidence` читает +`baselines.reviewedRun` из последнего сообщения коммита. Пока HEAD — обычный +коммит, поле `null`; как только HEAD несёт трейлер `Baseline-Reviewed:` +(коммит приёмки golden-эталонов), поле становится непустым, и +`evaluateCiProof` законно требует объявленный прогон, которого тестовая +заглушка `loadContext` не отдавала. Тест падал на **любой** ветке, чья +вершина — коммит приёмки эталонов (воспроизведено автором на +`issue/594-form-kit-room@68bb7558`, 2776/1). Продуктовый код (`scripts/ci-proof.mjs`, +`scripts/release-gate.mjs`) не менялся и не должен был — контракт «объявленный +прогон обязан существовать, быть завершённым, не отменённым, быть Validate» +именно то, что задумано #573. + +Правка: зелёный путь теста `#573` теперь считается на evidence с явно +обнулённым `baselines.reviewedRun`, независимо от того, чем оказался HEAD. +Контракт объявленного прогона вынесен в отдельный тест `#595` и проверяется с +трёх сторон (завершённый неотменённый Validate с `conclusion: 'failure'` → +green; прогона нет → failed; прогон отменён → failed). Добавлен мутант +`declared-baseline-review-run-never-checked` на защиту `scripts/ci-proof.mjs`. + +## Как проверялось + +Диапазон `origin/dev..HEAD` содержит один коммит и не трогает `src/**` — +браузерные смоки, golden, backend pytest, perf-профили и geometry-invariants +не выбираются диффом (`node scripts/smoke-select.mjs --base origin/dev --head HEAD` +подтвердил: «Исполняемого frontend-диффа нет… Browser-smoke этим диффом не +выбираются»). `check-docs` не требуется — отпечаток скриншотов не мог устареть, +диффа в `src/**` нет. + +**Дешёвые гейты уже подтверждены зелёным Validate на этом самом SHA** — +перепроверено напрямую (`gh api repos/Matysh/houseplan-card/actions/runs/35425580065` +→ `status: completed, conclusion: success, head_sha: eb145aba…`), поэтому +`npx tsc --noEmit`, `npm test`, `npm run build` не перегонялись повторно. + +| Гейт | Результат | Как | +|---|---|---| +| typecheck/test/build (Validate) | green | ссылка на прогон 35425580065, SHA сверен вручную — совпадает | +| `node --test test/release-gate.test.mjs` (локально, на материале) | 10 pass / 0 fail | выполнено в этой сессии | +| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | «нечего выбирать» | выполнено в этой сессии | +| `node scripts/process-gate.mjs --issues` | гейт пройден, WARN п.8 (инфра-трек #562, статус до S7 не нужен) | выполнено в этой сессии, `gh` доступен в песочнице | +| `node scripts/mutation-gate.mjs --check` (весь реестр, включая новый id) | все `ok`, включая `declared-baseline-review-run-never-checked` | выполнено в этой сессии | +| golden / backend / браузерные смоки / perf | не прогонялись | diff не трогает `src/**`, `custom_components/**/*.py`; smoke-select подтвердил «нечего выбирать» | + +### AC → доказательство (перепроверено независимо, не со слов автора) + +| AC | Заявлено | Проверено ревьюером | +|---|---|---| +| **AC1** — тест зелёный и на ветке с `Baseline-Reviewed:` на вершине, и без него | автор привёл 8/1 → 10/0 на временной фикстуре, снесённой после замера | **Воспроизведено независимо** в отдельном `git worktree` (не в материале ревью): пустой коммит с трейлером `Baseline-Reviewed: …/actions/runs/35407468491` поставлен вершиной; на нём: старый (`origin/dev`) `test/release-gate.test.mjs` — **8 pass / 1 fail**, `AssertionError: 'failed' !== 'green'`, текст `Baseline-Reviewed run 35407468491 is missing, cancelled or not a Validate run` — то есть баг реален и воспроизводится не только на исходной ветке автора; новый `test/release-gate.test.mjs` на том же коммите — **10 pass / 0 fail**. Worktree удалён (`git worktree remove --force`), рабочая копия материала не пострадала (`git status` — чисто, HEAD не сдвигался) | +| **AC2** — контракт проверяется обеими сторонами: существующий Validate → green, отсутствующий → failed с текстом про `Baseline-Reviewed run` | тест `#595`, 3 фикстуры | прочитан код теста и `evaluateCiProof` (scripts/ci-proof.mjs:295-306): `conclusion: 'failure'` в фикстуре — законный случай (`status==='completed' && conclusion !== 'cancelled'`, без требования `success`), совпадает с продуктовым контрактом. Прогнано локально — все три under-фикстуры дают заявленный статус | +| **AC3** — зелёный путь не читает `reviewedRun` из HEAD | `withoutReviewed()` явно зануляет поле перед использованием в `#573`; `#595`-тест переопределяет `expected.baselines.reviewedRun` литералом, не читая его из HEAD | подтверждено чтением: оба места явно не зависят от фактического сообщения HEAD-коммита. Дополнительно подтверждено воспроизведением AC1 выше — тест зелёный именно тогда, когда HEAD **несёт** трейлер, то есть путь через `localEvidence(HEAD)` не мог быть источником зелёного результата | +| **AC4** — гейты + мутант ловит подмену | `npm test`/`typecheck`/`build` зелёные (хендофф), мутант 1/1 | typecheck/test/build — через Validate на этом SHA (см. таблицу выше); мутант перепроверен вручную в этой сессии двумя способами: (а) ручной патч `scripts/ci-proof.mjs` (`const declared = null;`) + `node --test --test-name-pattern="#595"` → **AssertionError: expected 'failed', actual 'green'** на кейсе «прогона нет»; (б) `node scripts/mutation-gate.mjs --check` — весь реестр включая новый id `ok` | + +### Защитный AC — таблица «чем краснеет» (§2.7) + +| AC | Чем доказан | Чем краснеет | +|---|---|---| +| Объявленный `Baseline-Reviewed` прогон обязателен к резолву | `node --test --test-name-pattern="#595" test/release-gate.test.mjs` | Мутация `scripts/ci-proof.mjs`: `const declared = proof.evidence.baselines?.reviewedRun ?? null;` → `const declared = null;`. Воспроизведено вручную в этой сессии (см. AC4) — тест `#595` краснеет (`expected 'failed', actual 'green'` на кейсе с отсутствующим прогоном), а не просто заявлен автором | +| Зелёный путь `#573` не зависит от содержимого HEAD | воспроизведение AC1 (worktree с фиктивным `Baseline-Reviewed` на вершине) | старый тест на этой фикстуре краснеет (8/1), новый — зелёный (10/0); граница показана прогоном, а не чтением | + +## Что проверено и корректно + +- Диапазон коммитов и трейлеры: один коммит, `Issue: #595`, `User-Visible: no` — + верно, поведение продукта не меняется. `User-Visible: no` не требует правок + changelog — оба файла не тронуты, это корректно. +- Заявление «продуктовый код исправен» подтверждено: `scripts/ci-proof.mjs` и + `scripts/release-gate.mjs` не входят в diff; контракт `evaluateCiProof` + (строки 295-306 `ci-proof.mjs`) действительно не требует `success` от + объявленного прогона — фикстура с `conclusion: 'failure'` в новом тесте + соответствует реальности, а не поблажке теста. +- Существующее покрытие `evaluateCiProof` в `test/ci-proof.test.mjs` (неверный id, + неверный workflow, отменённый прогон — строки 311-325) не дублируется новым + тестом: `#595` в `release-gate.test.mjs` бьёт по другому слою — фикстуре + `loadContext`/`classifyValidateProofs`, где жил именно этот баг. Разделение + ответственности между файлами тестов оправдано. +- `process-gate.mjs --issues` подтверждает, что задача корректно распознана как + инфраструктурная (#562): WARN, а не ошибка, статусная метка не требуется. +- Реестр мутаций синтаксически корректен и интегрирован — `mutation-gate.mjs + --check` проходит по всему реестру без падений после добавления записи. +- Диагностика бага в issue (##Что сломано / Почему) точно соответствует коду: + `localEvidence` (`ci-proof.mjs:103-127`) действительно читает + `baselines.reviewedRun` из `git log -1 --format=%B `, а + `candidateExpectations` (`release-gate.mjs:18-26`) действительно считает + `own` только на checkout самого кандидата — ровно то место, которое правка + меняет. + +## Находки + +Нет находок High или Medium. Скоуп задачи полностью закрыт минимальной правкой +теста и мутационного якоря; изменение не расширяет и не сужает скоуп issue. + +## Чего не проверял + +- **Golden, браузерные смоки, backend pytest, perf-профили, geometry-invariants** — + diff не трогает `src/**`, `custom_components/**/*.py`, геометрию или ссылки на + неё; `smoke-select.mjs` подтвердил «нечего выбирать». Прогон был бы пустой + тратой времени, а не тщательностью. +- **`npm run inventory`** — число тестов не менялось в отчёте вручную (не + копировалось в документ), в документ вставлен только фактический вывод + `node --test`, полученный в этой сессии. +- **Полный `npm test`/`npx tsc --noEmit`/`npm run build`** заново не гонялись — + засчитан зелёный Validate на точном SHA `eb145aba…` (сверено GitHub API + напрямую, не со слов хендоффа). Локально прогнан только целевой файл + `test/release-gate.test.mjs`, поскольку именно он — предмет правки. +- **`npm run docs:accept`/`check-docs.mjs`** — не требуется, diff не в `src/**`. +- Воспроизведение бага и мутанта выполнено во **временном `git worktree`**, не + в материале ревью: рабочая копия на `eb145aba` не сдвигалась и не менялась + (`git status` — чисто до и после), worktree удалён по завершении проверки. + +## Вердикт + +Зелёный. AC1-AC4 доказаны исполнением, а не заявлением — в том числе +независимым воспроизведением исходного бага и подтверждением, что новый тест +и его мутационный якорь действительно способны краснеть. Скоуп соответствует +issue, класс B подтверждён, трейлеры корректны, лишних гейтов не прогонялось. + +--- + +## Материал раунда + +- **Ветка/SHA:** `eb145aba485fe62d363298bb4b09d33bd0bb096e` (HEAD рабочей копии на момент ревью, detached). +- **Дерево:** `git rev-parse HEAD^{tree}` на момент ревью соответствует рабочей копии; расхождений с материалом не найдено (`git status` чисто на всём протяжении ревью). +- **Диапазон:** `origin/dev..HEAD` — 1 коммит. + +--- + + + +## Материал раунда + +- Ветка: `issue/595-release-gate-reviewed-run-fixture`, коммит `eb145aba485f` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `75ffd0790d06ec566a719305f13afe271807617f` + ``` + git log --all --format='%H %T' | grep 75ffd0790d06 + ``` +- Тело issue: `2a34c60ce3779c75b08a16df990b4b0f4bdefc31396d9c5a81eb9fb71752a516` +- Вердикт конвейера: `green` · High 0