docs: review document for #655

Issue: #655
User-Visible: no

docs/reviews/CODE-REVIEW-655-r2.md

@@ -0,0 +1,180 @@
+# CODE-REVIEW-655-r2
+
+Issue: #655 · Заход: r2 · Трек: light (`small`) · Блокирующих циклов израсходовано: 1/2
+
+Материал раунда: `git log --oneline origin/dev..HEAD` = три коммита
+(`075a2d3fb128` — реализация, `7acf5c74ada4` — документ ревью r1,
+`f340157f59b7` — закрытие находки r1), рабочая копия на
+`f340157f59b75cbef4dc20d6ac456a10f4053517`. Раунд r1 закончился жёлтым
+вердиктом с единственной находкой (Medium, в скоупе AC3); повод для r2 —
+дельта `git diff 075a2d3fb128..f340157f59b7`, а не задача целиком
+(PROCESS.md §2.10).
+
+## Дельта r1 → r2
+
+```
+git diff 075a2d3fb128..f340157f59b7 --stat
+ docs/reviews/CODE-REVIEW-655-r1.md   | 179 ++++++++++++++++++++++
+ scripts/mutation-registry.mjs        |  13 ++
+ tests_backend/test_virtual_lights.py |  35 +++++
+```
+
+Дельта строго локальна: продуктовый код (`custom_components/houseplan/*.py`)
+не тронут ни строкой — весь diff это один новый тест, один новый мутант и
+служебный документ ревью r1. Это ровно объём, заявленный автором в качестве
+исправления («один тест + мутант, без расширения скоупа»), и ровно то, что
+находка r1 просила. Ребейза на ушедший вперёд `dev` нет
+(`origin/dev..HEAD` по-прежнему три коммита, база не сдвинулась), новой
+подсистемы или смены контракта нет — разбор по дельте оправдан, полный
+повторный разбор всей задачи не требуется.
+
+## Закрытие раунда r1
+
+| Находка r1 | Чем закрыта | Где это видно |
+|---|---|---|
+| Medium — защитный `_flush_lock` в `VirtualLightController.async_flush()` не имел собственного теста/мутанта на конкурентный двойной flush (AC3, `virtual_lights.py:163`) | Новый тест `test_concurrent_flushes_after_failed_delayed_save_write_once` (`tests_backend/test_virtual_lights.py:129-163`): запускает `controller.async_flush()` дважды конкурентно (`asyncio.gather`) поверх состояния, оставленного неудачной отложенной записью (`_dirty=True`, `_save_task=None`); `BlockingStore.async_save` считает входы и блокируется на `asyncio.Event` до явного `release.set()`, так что до релиза видно, сколько задач реально вошло в `Store.async_save` конкурентно. Плюс новый мутант `virtual-light-concurrent-flushes-bypass-lock` (`scripts/mutation-registry.mjs`), убирающий `async with self._flush_lock:` → `if True:` | Тест: `tests_backend/test_virtual_lights.py:129` (`entered == 1`, `writes == [...]` — один физический вход и одна запись). Мутант: `scripts/mutation-registry.mjs` (id `virtual-light-concurrent-flushes-bypass-lock`, guard `backend-test-guard.mjs concurrent_flushes_after_failed_delayed_save_write_once tests_backend/test_virtual_lights.py`). Исполнение в CI: Validate `f340157f`, job «Мутанты по диффу (3/6)» (id `108623233714`), лог: `ok virtual-light-concurrent-flushes-bypass-lock: заявленный тест покраснел на мутанте` — реальный прогон с полным HA, не заявление автора |
+
+Третий столбец таблицы AC3 для `VirtualLightController`, пустой в r1, теперь
+заполнен тем же способом, каким уже была доказана идемпотентность
+`TrailRecorder` — паритет между двумя сторами восстановлен.
+
+## Независимая перепроверка (не на слово автору и не только на лог CI)
+
+Не ограничился чтением диффа и логом одного CI-job — воспроизвёл оба
+направления мутации локально в песочнице ревью:
+
+1. Прогнал все 5 тестов `tests_backend/test_virtual_lights.py` напрямую
+   (без `pytest` — недоступен в песочнице, файл нарочно грузится по пути,
+   так что достаточно вызвать `test_*` функции модуля): все 5 `OK`,
+   включая новый `test_concurrent_flushes_after_failed_delayed_save_write_once`.
+2. Применил ровно патч мутанта (`async with self._flush_lock:` →
+   `if True:  # mutant: ...`) к рабочей копии `virtual_lights.py`, повторно
+   вызвал только новый тест — тест упал:
+   `AssertionError: only one concurrent flush may reach durable storage`
+   (счётчик входов в `Store.async_save` стал 2 без лока, ровно как
+   предсказывает трасса конкуренции). Файл вернул в исходное состояние сразу
+   после проверки, `git status --porcelain` — пусто, рабочая копия чиста.
+
+Это подтверждает находку r1 закрытой доказательством, которое умеет падать
+(«тест умеет падать» — не только по логу CI, но и по независимому повторению
+здесь), а не только фактом присутствия нового теста в диффе.
+
+## Как проверялось (гейты)
+
+| Гейт | Результат | Как |
+|---|---|---|
+| Дешёвые гейты (`typecheck`, `test`, `build`/`bundle-policy --verify`) | подтверждены зелёным Validate на точном SHA | `run 36320469875`, `conclusion: success`, `head_sha: f340157f59b75cbef4dc20d6ac456a10f4053517` (ссылка из задания ревью) — не перегонял повторно |
+| `node scripts/smoke-select.mjs --base 075a2d3fb128 --head HEAD` | «Исполняемого frontend-диффа нет … Browser-smoke этим диффом не выбираются» | прогнал сам; diff не трогает `src/**` |
+| Новый тест `test_concurrent_flushes_after_failed_delayed_save_write_once` + весь модуль `test_virtual_lights.py` (пуре-python, без HA) | 5/5 «OK» | вызвал функции теста напрямую (см. раздел «Независимая перепроверка») — в песочнице нет `pytest`, обошёл прямым вызовом, это не хуже: код исполнялся, а не читался |
+| Мутант `virtual-light-concurrent-flushes-bypass-lock` (отрицательная проба) | тест краснеет | (а) лог реального CI-job'а Validate `f340157f`, шард 3/6 (`ok … покраснел на мутанте`); (б) независимо воспроизвёл тот же патч локально и получил тот же красный результат |
+| CI-мутанты диффа в целом (6 шардов) | все success | `gh api .../actions/runs/36320469875/jobs` — все шесть job'ов «Мутанты по диффу (N/6)» зелёные |
+
+Не прогонял: `npx tsc --noEmit`, `npm test`, `npm run build` (покрыты
+Validate на этом SHA — фронтенд не тронут, а гейт всё равно исполнялся в
+job'е «Фронтенд: типы, юниты, мутанты, синхрон бандла» с зелёным исходом);
+`python -m pytest tests_backend -q` с реальным HA (909/910 тестов) — снова
+недоступен в песочнице (нет `homeassistant`, нет WSL); закрыл это тем же
+способом, что и в r1 — не поверил заявлению, а нашёл в логах CI-шардов
+реальное исполнение целевого теста с полным `homeassistant` до и после
+мутации. Полный канонический backend-набор для этого SHA нигде не
+исполнялся, кроме advisory WSL-прогона автора (heavy-гейт, не гейт ревью —
+`AGENTS.md`, «Heavy CI gates…»). `golden:verify`, `check-docs.mjs`,
+`npm run invariants`, браузерные смоки, performance — не нужны: diff не
+меняет `src/**`, геометрию, рендер или пользовательские числа.
+
+## AC → доказательство (только то, что задевает дельта)
+
+AC1, AC2, AC4, AC5 не в дельте r1→r2 — их доказательства не менялись,
+наследуются из r1 без повторной проверки (см. раздел ниже). Единственный
+пересматриваемый пункт:
+
+| AC | Чем доказан | Чем краснеет |
+|---|---|---|
+| AC3 (unload/идемпотентность, часть «повторный/конкурентный `async_flush()` не пишет дважды», `VirtualLightController`) | `test_concurrent_flushes_after_failed_delayed_save_write_once` — конкурентный `asyncio.gather` двух `async_flush()` поверх pending-состояния после неудачной записи, счётчик реальных входов в `Store.async_save` | мутант `virtual-light-concurrent-flushes-bypass-lock` — убит и в CI (лог шарда 3/6), и локально при независимом воспроизведении в этом ревью |
+
+## Находки
+
+Новых находок нет. Единственная находка r1 (Medium, в скоупе AC3) закрыта
+доказательством, которое я перепроверил исполнением дважды (CI-лог +
+локальное воспроизведение), а не одним лишь фактом появления нового теста
+в диффе. High-находок не было и не появилось.
+
+## Что проверено и корректно
+
+- Коммит `f340157f`: трейлеры `Issue: #655`, `User-Visible: no` —
+  корректно для тестового/инфраструктурного изменения без видимого
+  поведения; changelog не тронут, что и требуется при `no`.
+- Изменение не вводит и не меняет ни одного числа, видимого пользователю —
+  правка тестовая/инфраструктурная, продуктовый код не тронут.
+- Новый тест использует уже существующий в файле паттерн (`FakeStore`,
+  `FakeHass`, приватный event loop `_run`) и приватную запись состояния
+  контроллера (`controller._state = …`) — это пуре-python backend-тест,
+  гейт `no-new-private-writes.mjs` на него не распространяется: его область
+  (`isGatedPath`) — только `demo/smoke_*.mjs` и `demo/helpers/**.mjs`
+  (браузерные смоки), не `tests_backend/**`; прямая работа с приватным
+  состоянием контроллера — устоявшийся паттерн уже существующих тестов
+  этого же файла.
+- Мутант зарегистрирован с корректным `guard` (`backend-test-guard.mjs`
+  с именем ровно нового теста и путём к файлу) и `because`, ссылающимся на
+  #655 AC3 / review r1 — соответствует формату соседних записей реестра.
+- Синтаксис обоих изменённых файлов проверен (`python3 -m py_compile`,
+  `node --check`) — ошибок нет.
+- Рабочая копия после локального воспроизведения мутанта возвращена в
+  чистое состояние (`git status --porcelain` пусто) — эксперимент не
+  оставил следов в дереве материала.
+
+## Унаследовано из r1
+
+Всё остальное содержимое диффа (`custom_components/houseplan/__init__.py`,
+`custom_components/houseplan/store.py`, `custom_components/houseplan/virtual_lights.py`
+кроме `_flush_lock`, `tests_backend/test_ha_virtual_lights.py`,
+`tests_backend/test_trail_recorder.py`, первый мутант
+`shutdown-skips-deferred-store-flush` и `virtual-light-save-bypasses-ha-task-tracking`,
+доказательства AC1/AC2/AC4/AC5, модель жизненного цикла HA STOP vs unload,
+обработка ошибок в `_async_flush_runtime`) принято без повторной проверки —
+дельта r1→r2 их не касается ни строкой. Источник:
+`docs/reviews/CODE-REVIEW-655-r1.md` (коммит `7acf5c74ada4714672dea585f7238e288493c066`),
+материал того раунда — коммит `075a2d3fb1284d58055160663adbfba9a1440202`,
+дерево `e04239085a2bdbe537f270df4bd9e6e4ef46a35f`, вердикт `жёлтый · High 0 · Medium 1`.
+
+## Чего не проверял
+
+- Полный `python -m pytest tests_backend -q` с реальным HA (канонический
+  набор) — недоступен в песочнице ревью (нет `homeassistant`, нет WSL).
+  Заменил точечным исполнением: лог CI-шарда 3/6 показывает реальный запуск
+  нового теста под полным HA до и после мутации, плюс независимое
+  локальное воспроизведение того же мутанта без HA (пуре-python путь того
+  же теста не зависит от HA-рантайма).
+- Полное повторное ревью AC1/AC2/AC4/AC5 и остального продуктового кода —
+  сознательно не повторял: дельта r1→r2 их не трогает (см. «Унаследовано из
+  r1»), а полный разбор был выполнен и опубликован в r1.
+- `golden:verify`, `npm run invariants`, браузерные смоки, performance —
+  не запускал: diff не меняет `src/**`, геометрию или рендер;
+  `smoke-select.mjs` подтвердил отсутствие кандидатов.
+- WSL-полный HA прогон автора (заявлен в предыдущей передаче) — принят как
+  advisory, не как канон, не переисполнял.
+
+## Вердикт
+
+Единственная находка r1 закрыта доказательством, проверенным исполнением
+дважды независимо от автора (лог CI-мутации + локальное воспроизведение в
+этой сессии). Новых находок нет, High нет. Дельта локальна, продуктовый код
+не тронут, трейлеры корректны.
+
+Вердикт: зелёный · заход r2 · блокирующих циклов 1/2 · High: 0 · Medium: 0 → в задаче
+
+---
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `issue/655-shutdown-flush`, коммит `f340157f59b7` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `f551607360b8f4ca652c065869a717735e2f2f5b`
+  ```
+  git log --all --format='%H %T' | grep f551607360b8
+  ```
+- Тело issue: `9592e83bda3f396fe43ee14eb6bc22b4860f8309d2c5e4e954dc1b90a293c348`
+- Вердикт конвейера: `green` · High 0