diff --git a/docs/reviews/SPEC-REVIEW-475-r1.md b/docs/reviews/SPEC-REVIEW-475-r1.md new file mode 100644 index 00000000..1245950f --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-475-r1.md @@ -0,0 +1,165 @@ +# SPEC-REVIEW — issue #475 · заход r1 + +**Тема:** `mutation-gate --check` не отличает живого свидетеля от мёртвого — +добавить отбор мутантов по диффу («гард») и запуск на каждом пуше. +**Трек:** лёгкий (`small`). ТЗ живёт в теле issue (комментарий «ТЗ готово», +2026-09-06T10:54:23Z). Файла в `docs/specs/` нет и не должно быть. +**Класс изменения:** B (гейты и инструменты) — `scripts/mutation-gate.mjs`, +`test/mutation-gate.test.mjs`, `.github/workflows/validate.yml`. Ни одного +файла класса A не затронуто. + +## Скоуп + +Расширить `selectChangedMutants` так, чтобы мутант считался задетым диффом не +только по `patch.file`, но и по файлам своего гарда (тест/смок), и подключить +`--changed` как блокирующий job `validate.yml` на каждом пуше. Цель — сократить +разрыв между поломкой свидетеля и её обнаружением с «неделя/релиз» до «тот же +пуш» (мотивирующий инцидент — #466/#467). + +`docs/SCOPE.md` этот класс задач не ограничивает: это не пользовательская +функция ни для одной из персон, а инструмент инженерного качества (по типу +`042-backend-engineering-quality.md`); видимого поведения продукта нет, что и +делает трек лёгким и ставит его вне вопроса «какую строку Core user jobs +закрывает». `docs/USER-GUIDE.ru.md` неприменим — интерфейса нет. + +## Как проверялось + +Прочитано: `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком, тело issue #475 +и оба комментария (S2-анализ и ТЗ). Код читался, не исполнялся: + +- `scripts/mutation-gate.mjs` — существующие `selectChangedMutants`, + `shardMutants`, ветка `--changed` в `main()`, весь `MUTANT_DEFINITIONS` + (60+ мутантов, все варианты `guard:`); +- `.github/workflows/validate.yml` — job `changes` (классификация `frontend` / + `backend` / `integration`, вывод `base`/`range_base`), job `frontend` + (`if: needs.changes.outputs.frontend == 'true'`), job `backend` + (`if: needs.changes.outputs.backend == 'true' ...`); +- подтверждено, что `test/validate-workflow.test.mjs` уже существует — + «контрактный тест на YAML», на который ссылается доказательство AC4, не + изобретённая сущность, а существующий паттерн. + +Гейты не гонялись: диапазон материала — только тело issue, продуктового кода +нет, `typecheck`/`test`/`build` к тексту ТЗ неприменимы на этой стадии. + +## Находки + +### Medium (в скоупе задачи) — job и `guardFiles()` не покрывают гарды бэкенда, хотя они уже есть в реестре + +**Файл:** тело issue #475, разделы «1. Отбор по диффу…» и «2. `--changed` +гоняется на каждом пуше». + +Контракт заявлен без ограничения по подсистеме: «мутант считается затронутым, +если дифф содержит любой `patch.file` **или** любой файл, на который +ссылается его `guard`» — и мотивирующий сценарий («тест перестал ходить по +мутированной ветке») сформулирован в S2-разборе тоже без оговорки +«только фронтенд». Но два независимых сужения в самом же контракте молча +исключают уже существующие бэкенд-мутанты: + +1. **Извлечение файлов гарда** ограничено суффиксами `.mjs`/`.test.mjs`. В + реестре (`scripts/mutation-gate.mjs:121-159`) четыре мутанта + (`frontend-registration-skips-retry`, + `frontend-registration-retries-without-delay`, + `frontend-registration-is-not-unload-bound`, + `frontend-reload-notice-forgets-persisted-flag`) патчат + `custom_components/houseplan/frontend_registration.py` и охраняются + напрямую `python3 -m pytest tests_backend/test_ha_frontend_registration.py + -k "..."` — токен `tests_backend/test_ha_frontend_registration.py` + оканчивается на `.py` и по объявленному правилу файлом гарда не + считается вовсе. +2. **Триггер job** — `needs.changes.outputs.frontend == 'true'` (плюс правка + самого `mutation-gate.mjs`). Паттерн `frontend` в job `changes` + (`.github/workflows/validate.yml:281`) — `^(src/|demo/|test/|dist/| + custom_components/houseplan/frontend/|package(-lock)?\.json$| + rollup\.config\.mjs$|tsconfig)` — не включает ни `tests_backend/`, ни + `custom_components/houseplan/frontend_registration.py` (последний + попадает под отдельный паттерн `backend`, + `^(custom_components/.*\.py$|tests_backend/|...)`, строка 282). Диапазон + диффа, задевающий только `custom_components/houseplan/frontend_registration.py` + и/или `tests_backend/test_ha_frontend_registration.py`, выставляет + `backend=true`, но не `frontend=true` — и по объявленному условию job + «Мутанты по диффу» вообще не запустится. + +**Воспроизведение (по коду, без запуска):** диапазон, меняющий строку +`entry.async_on_unload(state.cancel)` на `pass` в +`custom_components/houseplan/frontend_registration.py` и одновременно +ослабляющий соответствующую проверку в +`tests_backend/test_ha_frontend_registration.py` (буквально сценарий +«гард перестал ходить», ради которого написан этот контракт) — задевает +только паттерн `backend`. `changes.outputs.frontend` остаётся `false`, job +«Мутанты по диффу» не запускается (условие в п.2 ТЗ), значит ни AC1, ни +предполагаемый AC2 для этого мутанта не проверяются ни на одном пуше — свежая +поломка живого свидетеля здесь снова обнаружится только полным еженедельным +прогоном, то есть именно тот разрыв, ради закрытия которого заведён #475, +для этой части реестра остаётся ровно тем же, что и до задачи. + +AC6 воспроизводит только фронтенд-случай (`src/wall-merge.ts`, +`multi-wall-*`), поэтому по документу проверить это не на чем — а +существующая часть реестра, для которой контракт молча не работает, реальна +и приведена выше, не гипотетична. + +Это не продуктовый вопрос — граница чисто техническая (какое условие триггера +job, какие суффиксы допускает `guardFiles`), значит по §7.1 её решает автор, а +не владелец. Но граница должна быть решена явно — либо расширить условие +триггера до `frontend == 'true' || backend == 'true' || mutation-gate.mjs +изменён` и расширить извлечение токенов гарда суффиксом `.py`, либо, если +бэкенд-гарды сознательно остаются вне этой задачи, написать это прямо рядом с +уже объявленной границей про фикстуры — сейчас в тексте объявлена только +одна граница (фикстуры), а фактическая шире. + +**Чем закрывается:** AC2 (или новый AC) должен явно называть исход для +бэкенд-патченных/бэкенд-охраняемых мутантов, и триггер job — соответствовать +этому исходу. Один из двух путей выше, зафиксированный в тексте issue. + +## Что проверено и корректно + +- Обязательные для лёгкого трека разделы (§5): проблема · контракт · AC1…AC6 + с доказательством · откат — все присутствуют, ни один не пропущен. +- Каждый AC однозначен и называет способ доказательства (unit / контрактный + тест на YAML), доказательство реалистично: `test/validate-workflow.test.mjs` + для проверки структуры `validate.yml` уже существует как паттерн (AC4). +- База диапазона `needs.changes.outputs.base` / `range_base` (#387/#388) — + существующая инфраструктура, а не изобретённая; подтверждена и в коде + (`.github/workflows/validate.yml:182-190`), и в S2-комментарии владельца. +- Отказ от «отметки последнего доказательства» из исходного текста issue — + явное техническое решение с приведённой причиной (кому обновлять отметку), + не догадка, выданная за факт. +- Граница по фикстурам гарда (`test/fixtures/*` не выводятся из команды) — + объявлена прямо и признана самим автором как известное ограничение; это не + находка, а корректно зафиксированное решение. +- Ранний зелёный выход при пустой выборке — уже реализован в + `main()`/`--changed` (`scripts/mutation-gate.mjs`, ветка `if (!selected.length)` + до вызова `runCleanGuards`), так что AC5 доказуем без новой логики. +- Критерии лёгкого трека (§5) не нарушены: сложность ≤3 по факту одной + логической поверхности (отбор мутантов + его подключение в CI), нет + миграции конфига, нет нового UX-контракта (контракта UX не существует — + инструмент внутренний), нет влияния на touch; влияние на длительность + прогона названо и посчитано приемлемым (S2-комментарий). +- Продуктовый код не затронут ни одним файлом (класс A отсутствует), значит + вопрос «какую строку Core user jobs закрывает» к этой задаче неприменим по + типу задачи, а не пропущен. + +## Чего не проверял + +- Не запускал `node scripts/mutation-gate.mjs --changed=...` и не писал сам + тест — на стадии ревью ТЗ кода ещё нет, это предмет код-ревью (§2.7). +- Не проверял полный список всех 60+ мутантов на предмет других суффиксов + гарда, отличных от `.mjs`/`.test.mjs`/`.py` (например, составные команды с + `tsc`/`fix-test-build.mjs`) — выборочно просмотрел реестр целиком через + `grep`, других расширений файлов-целей, кроме `.mjs`, `.test.mjs` и `.py`, + не встретил. +- Не оценивал производительность самого job на реальном CI (стоимость сборки + бандла при непустой выборке) — S2-комментарий называет её «минуты», и это + вне ревью ТЗ, оценка возможна только по факту в код-ревью. + +## Вердикт + +Вердикт: жёлтый · заход r1 · блокирующих циклов 1/2 · High: 0 · Medium: 1 → в задаче · Документ: (этот файл) + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Якоря снять не удалось: ветки задачи нет, материал читался по `dev`.