diff --git a/docs/reviews/CODE-REVIEW-475-r2.md b/docs/reviews/CODE-REVIEW-475-r2.md new file mode 100644 index 00000000..df2cdc8a --- /dev/null +++ b/docs/reviews/CODE-REVIEW-475-r2.md @@ -0,0 +1,201 @@ +# CODE-REVIEW-475-r2 + +Issue: #475 · «mutation-gate --check не отличает живого свидетеля от мёртвого» +Трек: `small` (лёгкий) · Этап: код-ревью · Заход: r2 · блокирующих циклов 0/2 + +**Разбор — полный, не по дельте.** Ветка приведена конвейером к `dev` до +ревью: поверх позиции r1 (`3ed8c920`) легло 5 коммитов `dev`, итог — +`0540901c`. После ребейза это другой код (§7.2): полный разбор обязателен, а +не сокращение по дельте §2.10, хотя раунд формально второй. Проверено: сами +5 коммитов `dev` (`isometric overlay plates`, ревью #471/#472, обновление +скриншотов) не пересекаются ни по одному файлу с диапазоном задачи +(`git diff origin/dev...HEAD` — 5 файлов, все из диапазона `origin/dev..HEAD` +для этой ветки, см. «Скоуп») — риск скрытого конфликта, ради которого +предписан полный разбор, не материализовался, но раздел «Унаследовано» +всё равно фиксирует это явно, а не тихо. + +## Скоуп + +`git diff origin/dev...HEAD` — 5 файлов, класс B (issue переиспользован, +класса A нет ни одного файла): + +- `.github/workflows/validate.yml` — выход `mutants` у job `changes`, третий + дизъюнкт в триггере `changed_mutants`, сама job (уже существовала с r1, + фикс только про триггер); +- `scripts/mutation-gate.mjs` — `guardFiles()`, расширенный + `selectChangedMutants()`, 2 мутанта (без изменений с r1); +- `test/mutation-gate.test.mjs` — AC1–AC3, AC5–AC7; AC7 переписан (фикс r1); +- `test/validate-workflow.test.mjs` — контракт на YAML (AC4), дополнен + проверкой третьего дизъюнкта и классификатора `mutants` (фикс r1); +- `docs/reviews/CODE-REVIEW-475-r1.md` — артефакт предыдущего раунда, + контенту не рецензируется повторно. + +## Как проверялось + +Зелёного Validate на `0540901c` нет — прогон не найден, гейты прогнаны +самостоятельно. Объём соразмерен классу B без правок в `src/**` (issue #127, +PROCESS.md §8): + +| Гейт | Команда | Результат | +|---|---|---| +| Typecheck | `npx tsc --noEmit` | зелёный | +| Юниты | `npm test` | **2083 pass / 0 fail / 1 skip** | +| Юниты по затронутым файлам отдельно | `node --test test/mutation-gate.test.mjs` · `node --test test/validate-workflow.test.mjs` | 17/17 и 11/11 pass, все `#475 AC*`-тесты и новый `AC4`-тест зелёные | +| Сборка + синхрон бандла | `npm run build` → `cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js` | зелёный, байт-в-байт; `git status` после сборки чист — правка не задевает `src/**`, бандл не изменился | +| Новый код не добавляет `any` | `node scripts/no-new-any.mjs --base origin/dev --head HEAD` | «добавленных строк в `src/**/*.ts`: 0» — diff не касается `src/**` | +| Отбор браузерных смоков | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | «Browser-smoke этим диффом не выбираются… src/**/*.ts не тронут» | +| Реестр мутантов применим | `node scripts/mutation-gate.mjs --check` | **509/509 `ok`**, включая оба новых мутанта, дублей якоря нет | +| Мутант `changed-selection-ignores-guard-files` | `node scripts/mutation-gate.mjs --id=changed-selection-ignores-guard-files` | чистый прогон ok → патч красит тест: «поймано 1 из 1» | +| Мутант `changed-selection-matches-any-token` | `node scripts/mutation-gate.mjs --id=changed-selection-matches-any-token` | то же, «поймано 1 из 1» | +| Самоприменение отбора к своему же диффу | `node scripts/mutation-gate.mjs --changed=origin/dev..HEAD` | «файлов в диффе 5, мутантов затронуто 4 из 509»: оба новых #475-мутанта плюс два не связанных с задачей (`workflow-scan-hardcodes-the-list`, `typing-gate-stops-running` — патчат тот же `validate.yml`); механизм отбирает сам себя корректно | +| AC7 напрямую по реальному реестру | `node -e "...MUTANTS.filter(patch.file==='.../frontend_registration.py')..."` | ровно 4 id, включая `frontend-reload-notice-forgets-persisted-flag` — тот самый пропущенный в Low r1 | +| Процессный гейт | `node scripts/process-gate.mjs --range fc5973c2..HEAD` | «гейт пройден, предупреждений 0»; ветка `issue/475-changed-mutants-on-push`, трейлеры `Issue:`/`User-Visible: no` на всех 3 коммитах диапазона | + +**Не прогонялось и почему:** + +- `python -m pytest tests_backend -q` — окружения ревью `.venv-backend`/`pytest` + не содержит (`pip show pytest` → not found), задокументированное ограничение + (AGENTS.md «Environments», PROCESS.md «Backend»), не следствие задачи. + Диф не трогает ни один `.py`-файл продукта; самоприменение отбора наткнулось + на ту же нехватку pytest на **чужом** мутанте (`typing-gate-stops-running`, + не из #475) — это подтверждает ограничение среды, а не пробел задачи. + Оба мутанта #475 проверены индивидуально через `--id=`, их гварды — `node --test`, + pytest не требуют. +- `check-docs.mjs`, `golden:verify`, `model-invariants`, performance-профили, + `single-source-numbers` — diff не трогает `src/**`, визуал, геометрию или + пользовательские числа; неприменимо, не пропуск. + +## Находки + +Блокирующих находок нет. + +**Low, снят без правки (новый, не из r1):** `AGENTS.md:398-400` перечисляет +«Gate jobs, matching the actual `validate.yml`» поимённо (`docs`, `provenance`, +`process-gate`, `hacs`, `hassfest`, `frontend`, `smoke`, `golden`, +`performance_smoke`, `backend`) и не включает новую `changed_mutants`, хотя +она блокирующая (без `continue-on-error`, проверено `test/validate-workflow.test.mjs`). +Проверено чтением `scripts/release-gate.mjs:9` (`classifyValidateRuns`) — +критерий готовности релиза читает **общий вывод workflow-прогона** Validate +через GitHub Actions API, а не имена отдельных job; красная `changed_mutants` +красит весь прогон и блокирует релиз тем же путём, что и остальные job из +списка. AC4 («job… входит в итоговый check») выполнен по факту — список в +AGENTS.md чисто описательный для человека и не читается никаким скриптом +(проверено: `grep -rn "Gate jobs, matching"` вне `AGENTS.md` пусто, синхронности +с кодом никто не проверяет). Обновление списка не входило в DoD ТЗ (документация +класса C не предмет этой задачи), и его отсутствие не создаёт функционального +дефекта — только неполноту одного предложения в другом документе. Снимаю без +правки. + +## Проверка AC — таблица «чем доказан / чем краснеет» + +| AC | Доказано | Чем | Чем краснеет | +|---|---|---|---| +| AC1 | unit | `test/mutation-gate.test.mjs` `#475 AC1` | не защитный AC (сравнение множества) | +| AC2 | unit + мутант | `#475 AC2` | `changed-selection-ignores-guard-files`: «поймано 1 из 1» (прогнано лично) | +| AC3 | unit + мутант | `#475 AC3` | `changed-selection-matches-any-token`: «поймано 1 из 1» (прогнано лично) | +| AC4 | unit (YAML-контракт) + чтение | `test/validate-workflow.test.mjs` (новый тест `#475 AC4`) + чтение `validate.yml:411-465` | не защитный AC (расположение/текст job) — обычное сравнение ожидаемого текста с фактическим; тест прогнан лично, зелёный | +| AC5 | unit + чтение | `#475 AC5` + `guardNeedsBundle` вызывается только для отобранных (чтение, не исполнение полного CI) | не защитный AC — расположение раннего выхода | +| AC6 | unit | `#475 AC6`, реальный реестр | не защитный AC — сравнение множества id | +| AC7 | unit (полный, фикс r1) + независимая проверка | `#475 AC7` (теперь фильтр по `patch.file`, не по префиксу id) + прямой запрос к `MUTANTS` (см. таблицу гейтов) | не защитный AC — сравнение множества id, но полнота множества (4, не 3) проверена мной по факту, а не на слово | + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где видно | +|---|---|---| +| **Medium** — триггер `changed_mutants` не покрывал изменение `scripts/mutation-gate.mjs` (третий дизъюнкт контракта §2 отсутствовал и в `if:`, и в контрактном тесте) | Классификатор получил выход `mutants` по точному пути `^scripts/mutation-gate\.mjs$`; триггер job — `frontend \|\| backend \|\| mutants`; все 3 fallback-ветки полного прогона выставляют `mutants=true` | `validate.yml:185,241,265,275,287,414` (прочитано построчно) + `test/validate-workflow.test.mjs` тест `#475 AC4` (прогнан лично, зелёный, проверяет все три места) | +| **Low** — тест AC7 фильтровал бэкенд-мутанты по префиксу id (`frontend-registration-*`) и пропускал 4-й (`frontend-reload-notice-forgets-persisted-flag`), хотя код отбирал все 4 | Тест переписан на фильтр по `patch.file === 'custom_components/houseplan/frontend_registration.py'`, что автоматически включает все 4 | `test/mutation-gate.test.mjs` тест `#475 AC7` (прогнан лично) + независимый запрос к `MUTANTS` в этом раунде подтвердил ровно 4 id, включая четвёртый | + +Обе находки закрыты кодом, а не заявлением автора — обе перепроверены +самостоятельным прогоном/чтением в этом раунде. + +## Унаследовано из r1 + +Формально не применимо в смысле «сокращения объёма» — разбор в этом раунде +полный (см. причину в шапке документа), каждый AC1–AC7 и обе находки r1 +перепроверены заново на `0540901c`, а не приняты на слово. Из документа +`docs/reviews/CODE-REVIEW-475-r1.md` (SHA материала `c0207af4`) взяты без +повторного вывода только два факта, не подлежащие пересмотру кодом: перечень +файлов диапазона (сверен заново и совпал) и формулировки самих находок +(процитированы дословно в разделе «Закрытие раунда r1» — их текст не +переизобретается, а их **закрытие** доказано отдельно). Спор о продуктовом +контракте (что должен покрывать триггер) на этапе ТЗ решён в +`SPEC-REVIEW-475-r1`/`r2` (жёлтый → зелёный) — это другой этап (§2.4, а не +§2.7) и не переоткрывается здесь; код-ревью проверяло только соответствие +уже принятому контракту, а не сам контракт. + +## Что проверено и корректно + +- `guardFiles()` — только токены `[\w./-]+\.(mjs|py)`, не начинающиеся с `-`, + существующие в репозитории; проверено на `--test-name-pattern="magnet + presses|x.mjs"` (юнит `#475 AC3`, прогнан лично) — фрагмент шаблона + `presses|x.mjs"` корректно не считается файлом гарда (несуществующий + символ `|` вне допустимого набора). +- `selectChangedMutants(mutants, changedFiles, exists)` обратно совместима: + вызов без третьего параметра (как в CLI, `mutation-gate.mjs:6891`) + использует дефолтный `existsSync` внутри `guardFiles`; старый тест `#332` + (шардирование) по-прежнему проходит без правок сигнатуры. +- Мутанты #475 верно устроены: якорь `changed-selection-ignores-guard-files` + собран из двух конкатенированных строк ровно затем, чтобы `--check` не + находил его дважды (в коде и в определении) — подтверждено: `--check` даёт + `ok`, дублей нет, реестр 507 → 509. +- База диапазона в `changed_mutants` (`PROVEN_BASE`/`BEFORE_SHA`/ + `merge-base`-фолбэк) побайтово идентична уже работающему блоку в job + `frontend` (гейт «новый код не добавляет any», #387/#388) — не новая + логика, а переиспользованная; сверено построчно чтением обоих мест. +- Установка Python и Chromium в `changed_mutants` дословно повторяет шаги + `mutation-gate.yml` (checkout → setup-node → npm ci → setup-python 3.14 → + `pip install -r tests_backend/requirements.txt` → кэш Playwright → + установка при промахе) — сверено построчно. +- Триггер `mutants` — точный путь (`^scripts/mutation-gate\.mjs$`), не + расширяет отбор ложно на соседние файлы; регресса для уже принятых + `frontend`/`backend`-веток нет — они лишь дополнены третьим условием + через `||`, не заменены. +- Трейлеры всех 3 коммитов диапазона (`Issue: #475`, `User-Visible: no`) — + корректны для инфраструктурной правки без пользовательского эффекта; + changelog не тронут, что верно при `User-Visible: no`; `process-gate.mjs` + подтверждает нулём предупреждений. +- Ни один другой файл репозитория не вызывает `selectChangedMutants`/ + `guardFiles` — изменение сигнатуры не создаёт риска регрессии на стороне + вызывающего кода (проверено `grep` по `*.mjs`/`*.ts`). + +## Чего не проверял + +- Полный `pytest tests_backend` (среда без `.venv-backend`) — см. «Как + проверялось». Ограничение среды, не задачи. +- Поведение `changed_mutants` job живьём в GitHub Actions (реальный запуск + runner, кэш Playwright, реальный `github.event.before` на push-событии) — + логика веток `EVENT_NAME`/`REF`/`PROVEN_BASE` проверена чтением и сверкой + с идентичным уже работающим кодом job `frontend`, не отдельным прогоном + workflow (это доступно только через реальный CI прогон на push, вне + ревью). +- Полноту списка «Gate jobs» в `AGENTS.md` как автоматизированного контракта + — там нет читающего его скрипта; см. Low. + +## Вердикт + +Зелёный. Обе находки r1 (Medium и Low) закрыты и проверены самостоятельно, а +не на слово автора. Новых High/Medium находок нет. Единственная новая +находка — Low вне блокирующего порога, снята без правки с объяснением, почему +функционального дефекта нет. Полный разбор всех AC подтверждает, что задача +решает заявленный сценарий: разрыв «неделя–релиз» для отбора мутантов по +диффу закрыт, включая триггер по правке самого реестра и по бэкенд-мутантам. + +--- + +## Материал раунда + +- Ветка: `issue/475-changed-mutants-on-push`, вершина `0540901cda486724a65ebcd9c4cb4f6d36bde906`. +- Диапазон разбора: `origin/dev..HEAD` (`fc5973c2..0540901c`), 3 коммита задачи. +- Предыдущий заход: `docs/reviews/CODE-REVIEW-475-r1.md`, SHA материала `c0207af4` (осиротевший после ребейза — ожидаемо, §2.10). + +--- + + + +## Материал раунда + +- Ветка: `issue/475-changed-mutants-on-push`, коммит `3ed8c9204473` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `12506be1e214ea1080a510d34089b505127c22bb` + ``` + git log --all --format='%H %T' | grep 12506be1e214 + ```