mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-05 14:19:04 +00:00
claude[bot]
@@ -0,0 +1,187 @@
|
||||
# CODE-REVIEW-475-r1
|
||||
|
||||
Issue: #475 · «mutation-gate --check не отличает живого свидетеля от мёртвого»
|
||||
Трек: `small` (лёгкий) · Этап: код-ревью · Заход: r1 · блокирующих циклов 0/2
|
||||
SHA материала: `c0207af40f25564eac9773d583cdcc7f5c56ea45` (`git rev-parse HEAD` перед выводом)
|
||||
Ветка приведена к `dev` конвейером до ревью: поверх легло 4 коммит(ов) dev
|
||||
(`9b41ac1c` → `c0207af4`). Диапазон `origin/dev..HEAD` после этого содержит
|
||||
ровно один коммит задачи — разбор полный (§7.2), а не по дельте, как и
|
||||
предписано при ребейзе на ушедший вперёд `dev`.
|
||||
|
||||
## Скоуп
|
||||
|
||||
Диапазон `git diff origin/dev...HEAD`, 4 файла, все класса B:
|
||||
|
||||
- `.github/workflows/validate.yml` — новая job `changed_mutants`;
|
||||
- `scripts/mutation-gate.mjs` — `guardFiles()`, расширенный `selectChangedMutants()`, 2 новых мутанта;
|
||||
- `test/mutation-gate.test.mjs` — AC1–AC3, AC5–AC7;
|
||||
- `test/validate-workflow.test.mjs` — контрактный тест на YAML (AC4).
|
||||
|
||||
Класса A ни один файл не задет — трек `small`, ТЗ в теле issue, ревью ТЗ
|
||||
прошло два захода (r1 жёлтый/Medium, r2 зелёный) и закрыто до кода.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Гейты, соразмерно классу B без изменений в `src/**` (issue #127, PROCESS.md §8):
|
||||
|
||||
| Гейт | Команда | Результат |
|
||||
|---|---|---|
|
||||
| Typecheck | `npx tsc --noEmit` | зелёный, 7.2s |
|
||||
| Юниты | `npm test` | **2083 pass / 0 fail / 1 skip**, 38.7s (совпадает с числом автора в хендоффе) |
|
||||
| Сборка + синхрон бандла | `npm run build` затем `cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js` | зелёный, байт-в-байт совпадение |
|
||||
| Новый код не добавляет `any` | `node scripts/no-new-any.mjs --base origin/dev --head HEAD` | «Проверено добавленных строк в src/\*\*/\*.ts: 0» — диф не касается `src/**` |
|
||||
| Отбор браузерных смоков | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | «Исполняемого frontend-диффа нет (src/\*\*/\*.ts не тронут)» — смоки не выбраны, выбирать нечего |
|
||||
| Реестр мутантов применим | `node scripts/mutation-gate.mjs --check` | 509 мутантов, все `ok`, включая 2 новых |
|
||||
| Мутант `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` | «файлов в диффе 4, мутантов затронуто 4 из 509» — независимо воспроизведено, состав описан ниже |
|
||||
| AC7 напрямую | `node -e "...selectChangedMutants(MUTANTS, [...])..."` | все 4 бэкенд-мутанта `frontend_registration.py`/`test_ha_frontend_registration.py` отобраны и по гарду, и по патчу |
|
||||
|
||||
**Не прогонялось и почему:**
|
||||
|
||||
- `check-docs.mjs` — правило #127/§8 требует его при диффе по `src/**`; diff файлов `src/**` не содержит. Пропуск честный, не молчаливый.
|
||||
- `python -m pytest tests_backend -q` — окружение ревью не содержит `.venv-backend` и `pytest` (`pip show pytest` → not found), это задокументированное ограничение (PROCESS.md «Backend» и AGENTS.md «Environments»), не результат этой задачи. При попытке независимо прогнать `--changed=origin/dev..HEAD` до конца скрипт сам упёрся в это же: мутант `typing-gate-stops-running` (не из #475, патчит `.github/workflows/validate.yml`) требует `pytest`, которого здесь нет. AC7 проверен напрямую через API функции (см. таблицу) вместо прогона pytest.
|
||||
- `golden:verify`, model-invariants, performance-профили, single-source-numbers — diff не меняет визуал, геометрию, ссылки на неё или пользовательские числа; неприменимо.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе) — триггер job не покрывает третье условие контракта
|
||||
|
||||
**Файл:** `.github/workflows/validate.yml:410`
|
||||
**Что не так:** Контракт §2 принятого ТЗ (тело issue #475, редакция после фикса Medium из SPEC-REVIEW r1) требует:
|
||||
|
||||
> job запускается при `frontend == 'true'` **или `backend == 'true'` или изменении `scripts/mutation-gate.mjs`**
|
||||
|
||||
Реализовано:
|
||||
|
||||
```yaml
|
||||
if: needs.changes.outputs.frontend == 'true' || needs.changes.outputs.backend == 'true'
|
||||
```
|
||||
|
||||
Третье условие отсутствует — ни в `if:`, ни в контрактном тесте
|
||||
`test/validate-workflow.test.mjs:290`, который проверяет ровно эти же два
|
||||
дизъюнкта и не более.
|
||||
|
||||
**Воспроизведение (не гипотеза — проверено чтением классификатора):**
|
||||
`changes` job классифицирует файлы двумя regex (`validate.yml:262-263`):
|
||||
|
||||
```
|
||||
frontend: ^(src/|demo/|test/|dist/|custom_components/houseplan/frontend/|package(-lock)?\.json$|rollup\.config\.mjs$|tsconfig)
|
||||
backend: ^(custom_components/.*\.py$|tests_backend/|scripts/support-relay/|pytest\.ini$)
|
||||
```
|
||||
|
||||
`scripts/mutation-gate.mjs` не начинается ни с одного из перечисленных
|
||||
префиксов и не совпадает ни с одним `$`-якорем — ни `frontend`, ни `backend`
|
||||
не станут `true` от диффа, ограниченного этим файлом. Значит правка
|
||||
`scripts/mutation-gate.mjs` (например, новый мутант, патчащий
|
||||
`src/`/`custom_components/`-файл, без сопутствующей правки в `test/**`)
|
||||
проходит без единого запуска `changed_mutants` — job попросту не
|
||||
запланируется на этом пуше.
|
||||
|
||||
Это ровно тот класс дефекта, ради которого заведён #475: рефакторинг файла,
|
||||
отвечающего за отбор мутантов, остаётся невидимым до недельного полного
|
||||
прогона (или до релиза) — тот же разрыв «неделя–релиз» из #466/#467, только
|
||||
теперь применительно к самому гейту, а не к продуктовому коду. Contract §2
|
||||
называет это условие явно и с тем же обоснованием, каким объяснено условие
|
||||
`backend` («бэкенд-мутанты патчат `.py`... дифф... даёт `backend=true` без
|
||||
`frontend=true`») — то есть это не пропущенная деталь ТЗ, а нереализованная
|
||||
часть уже принятого контракта.
|
||||
|
||||
**Почему не High:** основной сценарий (правка продуктового/тестового кода,
|
||||
меняющая `patch.file` или файл гарда любого мутанта) работает и подтверждён
|
||||
и юнитами, и самоприменением. Пробел узкий — правка ограничена буквально
|
||||
одним файлом `scripts/mutation-gate.mjs` без сопутствующих `test/**`/`src/**`/
|
||||
`custom_components/**.py` изменений — но контракт именно этот случай называет
|
||||
по имени, поэтому строка контракта не выполнена.
|
||||
|
||||
**Чем закрывается (в скоупе, без выхода за задачу):** добавить в `if:`
|
||||
третий дизъюнкт, обнаруживающий изменение `scripts/mutation-gate.mjs`
|
||||
(например, отдельным шагом `changes`/`classify` либо прямой проверкой пути
|
||||
диапазона в самой job), и покрыть его тем же контрактным тестом.
|
||||
|
||||
### Low — тест AC7 не покрывает четвёртый бэкенд-мутант, хотя код это делает
|
||||
|
||||
**Файл:** `test/mutation-gate.test.mjs:259–267`
|
||||
В реестре ровно 4 мутанта, патчащих `custom_components/houseplan/frontend_registration.py`
|
||||
с гардом `tests_backend/test_ha_frontend_registration.py` (`scripts/mutation-gate.mjs:120,132,146,158`).
|
||||
Тест AC7 фильтрует их через `id.startsWith('frontend-registration-')`, что
|
||||
даёт только 3 — четвёртый называется `frontend-reload-notice-forgets-persisted-flag`
|
||||
(другой префикс id, тот же патч-файл и гард) и в assert-цикл не попадает.
|
||||
|
||||
Проверено напрямую (не через тест): `selectChangedMutants` реально отбирает
|
||||
все 4 и по гарду, и по патчу — сама реализация не различает мутанты по имени,
|
||||
поэтому регрессии здесь нет, только тест не проверяет ровно то, что заявляет
|
||||
ТЗ («отбирает все четыре frontend-registration-\* мутанта»). Наименование в
|
||||
ТЗ тоже неточно (не все четыре носят префикс `frontend-registration-`).
|
||||
Снимаю без исправления: механизм общий, не завязан на имя мутанта, и
|
||||
пропущенный четвёртый защищён тем же кодовым путём, что и проверенные три —
|
||||
регрессия, ломающая только его и не ломающая остальные три, потребовала бы
|
||||
отдельного условия по конкретному id, которого в реализации нет.
|
||||
|
||||
## Проверка AC — таблица «чем доказан / чем краснеет»
|
||||
|
||||
| AC | Доказано | Чем | Чем краснеет |
|
||||
|---|---|---|---|
|
||||
| AC1 (патч-файл в диффе → отбор) | unit | `test/mutation-gate.test.mjs:228` (`#475 AC1`) + ранее существовавший `#332` тест | защитный AC без мутанта не заявлен (простое сравнение выборки) |
|
||||
| AC2 (файл гарда — тест/смок/pytest — в диффе → отбор) | unit + мутант | `test/mutation-gate.test.mjs:233` (`#475 AC2`) | мутант `changed-selection-ignores-guard-files`: гвард (`--test-name-pattern="#475 AC2"`) чист на исходном коде и красный на патче (проверено: `--id=changed-selection-ignores-guard-files` → «поймано 1 из 1») |
|
||||
| AC3 (токены-нефайлы не считаются) | unit + мутант | `test/mutation-gate.test.mjs:241` (`#475 AC3`) | мутант `changed-selection-matches-any-token`: проверено аналогично, «поймано 1 из 1» |
|
||||
| AC4 (job в validate.yml: триггер, база диапазона, python-зависимости, блокирующая) | частично unit, частично чтением | `test/validate-workflow.test.mjs:283` + чтение `validate.yml:407-459` | **не выполнено полностью** — триггер не покрывает третий дизъюнкт контракта (см. находку Medium); база диапазона, установка Python и отсутствие `continue-on-error` подтверждены и текстом job, и сверкой с `mutation-gate.yml`/`frontend`-job (byte-идентичный паттерн вычисления `base`) |
|
||||
| AC5 (пустой отбор → зелёный выход без сборки бандла) | unit + чтение | `test/mutation-gate.test.mjs:250` (`#475 AC5`) для отбора; `guardNeedsBundle`/`buildBundle` вызывается только для отобранных мутантов с браузерным гвардом (`scripts/mutation-gate.mjs:6779,6801`) — проверено чтением, не исполнением полного CI-прогона | защита не заявлена как guard/limit — расположение раннего выхода, обычное сравнение |
|
||||
| AC6 (репродукция #467: `src/wall-thickness.ts` → 3 мутанта) | unit | `test/mutation-gate.test.mjs:255` (`#475 AC6`), прогнан в составе `npm test` | простое сравнение множества id, мутант не требуется правилом (не защитный AC) |
|
||||
| AC7 (репродукция ревью: `.py`-патч/гард → 4 бэкенд-мутанта) | unit (неполный, см. находку Low) + независимая проверка | `test/mutation-gate.test.mjs:259`; дополнено прямым вызовом `selectChangedMutants` в этом ревью, подтвердившим все 4 | простое сравнение множества id |
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **`guardFiles()`** извлекает файлы без парсинга команды: только токены вида
|
||||
`[\w./-]+\.(mjs|py)`, не начинающиеся с `-`, существующие в репозитории.
|
||||
Проверено на реальных гардах (`--test-name-pattern="magnet presses|x.mjs"`
|
||||
корректно исключает фрагмент шаблона, оставляя только `test/furniture.test.mjs`)
|
||||
и на `bundle:sync && node demo/smoke_*.mjs` (составные команды с `&&`).
|
||||
- **`selectChangedMutants`** обратно совместима: старые вызовы без третьего
|
||||
параметра `exists` продолжают работать (дефолт — `existsSync`), старый тест
|
||||
`#332` по-прежнему проходит без изменений сигнатуры вызова.
|
||||
- **Мутанты #475 сгенерированы корректно**: якорь `changed-selection-ignores-guard-files`
|
||||
собран из двух конкатенированных строк ровно затем, чтобы `--check` не находил
|
||||
его дважды (в коде и в определении мутанта) — воспроизведено: `--check` даёт
|
||||
`ok` на обоих новых мутантах.
|
||||
- **Установка Python-зависимостей и Chromium** в `changed_mutants` дословно
|
||||
повторяет шаги `mutation-gate.yml` (checkout → setup-node → npm ci →
|
||||
setup-python 3.14 → `pip install -r tests_backend/requirements.txt` → кэш
|
||||
Playwright → установка Chromium при промахе кэша) — сверено построчно.
|
||||
- **База диапазона** (`PROVEN_BASE`/`BEFORE_SHA`/`merge-base` с фолбэком)
|
||||
побайтово идентична блоку, уже проверенному в job `frontend` для гейта
|
||||
«новый код не добавляет any» (#387/#388) — не новая, а переиспользованная
|
||||
логика.
|
||||
- **Трейлеры коммита**: `Issue: #475`, `User-Visible: no` — корректно для
|
||||
инфраструктурной правки без пользовательского эффекта; changelog не тронут,
|
||||
что верно при `User-Visible: no`.
|
||||
- **Класс файлов**: все правки — класс B (`test/**`, `scripts/**`,
|
||||
`.github/workflows/**`), issue переиспользован по правилу AGENTS.md.
|
||||
|
||||
## Унаследовано из предыдущих раундов
|
||||
|
||||
Не применимо — это первый заход код-ревью (`r1`). Ревью ТЗ (`SPEC-REVIEW-475-r1`
|
||||
жёлтый, `SPEC-REVIEW-475-r2` зелёный) относится к отдельному этапу и не
|
||||
экономит объём этого разбора; код проверен полностью, включая уже закрытую
|
||||
там находку (расширение `.py`/`backend`), которая была сверена по факту в коде
|
||||
(см. «Что проверено и корректно»), а не принята на слово.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Жёлтый. Единственная блокирующая находка — Medium в скоупе задачи
|
||||
(триггер `changed_mutants` не покрывает изменение `scripts/mutation-gate.mjs`,
|
||||
как того явно требует Contract §2 принятого ТЗ). High-находок нет. Правится
|
||||
в этом же issue без нового цикла ревью ТЗ; после фикса — обычный повторный
|
||||
заход код-ревью по дельте (§2.10).
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/475-changed-mutants-on-push`, коммит `9b41ac1c7041` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `5483eee97c02fa1a40b6d6199b30741353263bf4`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 5483eee97c02
|
||||
```
|
||||
Reference in New Issue
Block a user