docs: review document for #333

Issue: #333
User-Visible: no

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

@@ -0,0 +1,191 @@
+# CODE-REVIEW-333-r2
+
+Issue: #333 · Заход: r2 · Блокирующих циклов израсходовано 1 из 2
+Материал: `git log --oneline origin/dev..HEAD` = три коммита на текущем
+`HEAD` = `4ded9c0b` (`git rev-parse HEAD` сверен непосредственно перед
+подведением итогов):
+
+```
+4ded9c0b test: the #248 roundtrip fixture is seeded, not first-written (#333 r1-H1)
+783cc68b docs: review document for #333            (публикация CODE-REVIEW-333-r1.md)
+5a2dd333 fix: plan/optimize passes the junction gate; import stays free by design (#333)
+```
+
+Родитель `5a2dd333` — `b37a5987` (= `origin/dev` на момент проверки).
+
+## 0. Важное расхождение с материалом прошлого раунда — рассмотрено и снято
+
+r1-документ (`docs/reviews/CODE-REVIEW-333-r1.md`) называет проверенным
+коммитом `c29df29b` с родителем `73406225`. Такого SHA в текущей истории
+нет — между r1-ревью и фиксом r1-H1 ветка была перебазирована на ушедший
+вперёд `dev`: между `73406225` и текущим родителем `b37a5987` легло 17
+чужих коммитов issue #337 (лениво загружаемый рантайм редактора, полная
+перестройка бандлинга — 114 файлов, ~34 тыс. строк).
+
+Это ровно сценарий «ребейз на ушедший вперёд dev» из брифа, который по
+умолчанию требует полного разбора, а не разбора по дельте. Проверил, что
+разбор действительно должен остаться полным или можно сузить:
+
+- **Пересечения файлов нет.** `git diff --stat 73406225 5a2dd333^`
+  (весь чужой рывок dev) не касается ни одного файла, который трогает
+  #333 (`custom_components/houseplan/{junction_limits,websocket_api}.py`,
+  `docs/specs/329-junction-limits.md`, `scripts/mutation-gate.mjs`,
+  `tests_backend/test_ha_websocket.py`) — только `src/**`, бандлинг и
+  фронтенд-доки.
+- **Номера строк не сдвинулись.** `websocket_api.py:1626`
+  (`ws_plan_optimize`), `:1783` (`rt.junction_baseline = …`), `:1396`
+  (`config/set`) — те же номера, что называет r1-документ. Файл ребейзом
+  физически не задет.
+- Поэтому содержательно к делу можно подойти по дельте (закрытие r1-H1),
+  но **гейты прогнал заново и полностью**, а не унаследовал: у ребейза
+  такого масштаба легко мог быть скрытый эффект на сборку/реестр
+  мутаций/бэкенд-набор, а зелёного Validate на этом SHA нет (см. §2). Это
+  тот случай, где полнота разбора обеспечена не «на всякий случай», а
+  проверкой конкретного факта (непересечение файлов), которая и позволяет
+  не заводить AC1–AC4 заново, ограничившись закрытием H1.
+
+## 1. Скоуп
+
+Не изменился с r1: ТЗ (тело issue, small-трек, ревизия 2, оба раунда
+spec-ревью зелёные) требует (1) `validate_junction_limits` в
+`ws_plan_optimize` после миграции кандидата, наследование по правилу;
+(2) обновление `rt.junction_baseline` после успешного optimize; (3)
+import/restore вне гейта, докстринг и спека §5 — честно. Продуктовая
+рамка (`docs/SCOPE.md`) и корректность `User-Visible: no` — без изменений
+с r1, задача только про то, доказан ли контракт и не сломано ли что-то
+существующее.
+
+Предмет r2 — единственная правка после r1: тестовая фикстура
+`test_plan_optimize_persists_exact_storage_roundtrip_target`
+(`tests_backend/test_ha_websocket.py`) теперь сеет исходный конфиг в
+хранилище перед вызовом `plan/optimize`, вместо пустого `previous`.
+Продуктовый код не тронут ни строкой — весь коммит `4ded9c0b`
+ограничен тестовым файлом.
+
+## 2. Как проверялось
+
+Зелёного Validate на `4ded9c0b` нет (сообщение брифа) — прогнал сам,
+полностью (обоснование §0):
+
+| Гейт | Команда | Результат |
+|---|---|---|
+| Типы | `npx tsc --noEmit` | чисто |
+| Frontend-юниты | `npm test` | 1439 passed, 0 failed, 1 skipped |
+| Сборка + синхрон бандла | `npm run build`; `cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js`; сверка списков файлов `dist/houseplan-assets/` и `custom_components/houseplan/frontend/houseplan-assets/` | все три копии идентичны |
+| Реестр мутаций | `node scripts/mutation-gate.mjs --check` | зелёный, `junction-limit-optimize-unguarded` зарегистрирован, patch-строка совпадает с реальным кодом (см. ниже) |
+| Мутант убивает тест | ручной патч `return validate_junction_limits(...)` → `return {}, None` в `websocket_api.py:1706-1708`, `pytest -k test_333_optimize_refuses_a_crafted_violation` | тест красный (`assert not True`), дерево возвращено (`git status` чист) — тест умеет падать |
+| Backend, полный набор | `python -m pytest tests_backend -q` (харнес `pytest-homeassistant-custom-component`+`home-assistant-frontend` установлены в сессии) | **432 passed, 1 skipped, 1 error** (error — известный teardown-артефакт Python 3.12/харнеса, см. §5, не по телу теста) |
+| Backend, целевые тесты | `pytest -k "test_plan_optimize_persists_exact_storage_roundtrip_target or test_333_optimize_refuses_a_crafted_violation_and_keeps_the_plan or test_333_optimize_inherits_stored_violations or test_333_optimize_refreshes_the_junction_baseline_cache"` | все 4 PASSED |
+| Смок-отбор | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | «исполняемого frontend-диффа нет» — браузерные смоки не выбираются, diff не трогает `src/**` |
+| check-docs | не гонял | diff #333 (все 3 коммита) не трогает `src/**` ни строкой — критерий запуска не выполнен |
+| `npm run invariants` | не гонял | diff не меняет геометрию/рёбра/записи толщины/`layout`/`marker.space`/`open_spans` — только добавляет вызов уже существующего Python-валидатора и правит тест-фикстуру |
+| performance-профили | не гонял | не названы в AC, дорогая цепочка уже в executor (#330), диффом не тронута |
+| «Одно число — один источник» | рассмотрено | diff не вводит и не меняет ни одной пользовательски видимой величины — `User-Visible: no` во всех трёх коммитах корректен |
+
+Дополнительно проверил рабочее дерево после ручного патча мутанта:
+`git status` — чисто, никаких файлов не осталось изменёнными
+(`dist/houseplan-card.js` случайно поменял режим/mtime после
+`npm run build` — вернул `git checkout -- dist/houseplan-card.js`).
+
+## 3. Закрытие раунда r1
+
+| Находка r1 | Чем закрыта | Где видно |
+|---|---|---|
+| **H1 (High).** Optimize с пустым `previous` (`expected_config_rev: 0`) на фикстуре #248 засчитывал легитимную стену 6 см как НОВОЕ нарушение П3 (правило «первая запись не может прийти уже сломанной») — существующий зелёный тест `test_plan_optimize_persists_exact_storage_roundtrip_target` падал на `assert response["success"]`. | Коммит `4ded9c0b`: фикстура теперь СЕЕТСЯ в `runtime.config_store` как хранимый документ (`rev: 1`) ПЕРЕД вызовом `plan/optimize` (`expected_config_rev: 1` вместо `0`), поэтому optimize видит непустой `previous` и 6-см стена читается как унаследованное нарушение — ремонтный поток, как в AC2. Ревизии в ассертах сдвинуты на 1 (`config_rev == 2`), storage-утверждения #248 (intent/pending/final/каноническая сериализация) не тронуты. | `tests_backend/test_ha_websocket.py:626-646` (комментарий + новый `async_save`), запуск: `pytest -k test_plan_optimize_persists_exact_storage_roundtrip_target` → **PASSED** (был **FAILED** на `c29df29b`/эквивалентном `5a2dd333`). Полный `pytest tests_backend -q`: 432 passed / 0 failed (было 1 failed) |
+| **Medium, вне скоупа.** Бриф ревью ссылался на Validate-run `c29df29b` (33145399107) как на «зелёные дешёвые гейты», но job'ы фронтенда и бэкенда в нём были `skipped`, не `success`, из-за потери диапазона коммитов классификатором после force-push. | Не в этой ветке — заведён отдельно, как и требует правило (Medium вне скоупа → отдельный issue, не «оставлено в тексте»). | #347 (`bug, infra, P1, S1-new`, ссылка на #333) — существует, открыт, метки на месте (проверено `gh issue view 347`) |
+
+Оба пункта r1 закрыты доказательно, не декларативно: H1 — перезапуском
+ранее красного теста и полного бэкенд-набора; Medium — проверкой, что
+issue реально существует с нужными метками.
+
+## 4. Унаследовано из r1 (без повторной проверки содержания)
+
+Из `docs/reviews/CODE-REVIEW-333-r1.md` (материал: коммит `c29df29b`,
+родитель `73406225`) принимаю без повторного разбора — с довеском:
+файлы, к которым относятся эти выводы, не менялись НИ РАЗУ между
+`c29df29b`/`73406225` и текущим `HEAD`/`origin/dev` (см. §0, номера строк
+совпадают), поэтому дельта их не задевает:
+
+- **Техническая точность контракта п.1–3** — вызов
+  `validate_junction_limits` в `_validate_optimize_cpu` после миграции
+  кандидата, то же место конвейера, что в `config/set`; `JunctionLimitError`
+  в except-списке перестал быть мёртвым.
+- **AC1 (крафтованный payload отклонён, `config`/`rev` побайтово
+  неизменны)** и структурная гарантия неизменности layout (отказ до
+  первого `async_save_*`).
+- **AC2 (наследование легаси-нарушения в optimize)** — тест
+  `test_333_optimize_inherits_stored_violations`.
+- **AC3 (обновление `rt.junction_baseline`)** — симметрия с
+  `config/set` (`websocket_api.py:1396` / `:1783`).
+- **AC4 (докстринг + спека §5 честны)** — текст `junction_limits.py:1-15`
+  и `docs/specs/329-junction-limits.md` §5 (абзац о периметре, строки
+  127-134) — сверил построчно ещё раз (§2, «Как проверялось» этого
+  документа не потребовалось трогать код, только перечитать текст) и
+  подтверждаю: абзац идентичен тому, что видел r1 — diff между
+  `origin/dev...HEAD` для этого файла ограничен ровно этим одним
+  абзацем, второй раз не добавлялся и не менялся.
+- **Реестр мутаций** — `junction-limit-optimize-unguarded` описан честно;
+  я самостоятельно перепрогнал и `--check`, и ручное убийство мутанта
+  (не чисто унаследовал — но результат совпадает с r1).
+- **Паритет П1–П4** — `test_parity_with_the_frontend_checks` не тронут,
+  прошёл в составе полного набора.
+- **Трейлеры** — `Issue: #333`, `User-Visible: no` верны во всех трёх
+  коммитах этого раунда (проверено заново, т.к. коммиты новые).
+- **Соответствие малому треку** — не переоценивал; объём (закрытие
+  одного High одним тестовым коммитом) трек не нарушает.
+
+## 5. Находки
+
+Новых находок нет. High: 0. Medium: 0.
+
+## 6. Что проверено и корректно
+
+- r1-H1 закрыт по существу, а не косметически: подтверждено, что
+  предмет теста (#248, байт-точность storage) не пострадал — все
+  storage-ассерты (`intent_write`, `pending`, `final_config`,
+  `final_layout`, каноническая сериализация JSON) остались в тесте
+  дословно, изменились только сид и ожидаемые номера ревизий.
+- Семантика фикса согласована с контрактом: посеянный `previous`
+  идентичен `candidate` (`source["config"]` без изменений), то есть это
+  echo-оптимизация того же класса, что AC2 — 6-см стена наследуется, а
+  не легализуется заново; ровно то поведение, которое и должен доказывать
+  тест про репутацию хранилища.
+- Полный бэкенд-набор (432/0/1 error-teardown) и целевые 4 теста
+  зелёные лично прогнанными командами, не по докладу автора.
+- Мутант `junction-limit-optimize-unguarded` реально убивает целевой
+  тест — проверено вручную в этом раунде.
+- Рабочее дерево после всех ручных экспериментов чистое
+  (`git status` пуст).
+- Ребейз на ушедший вперёд `dev` (#337) не затронул ни один файл этой
+  задачи — подтверждено диффом файлов, а не предположением.
+
+## 7. Чего не проверял и почему
+
+- **AC1–AC3 заново «с нуля»** — не пересобирал заново обоснование
+  каждого, ограничился перезапуском тестов и сверкой, что r1-текст
+  описывает код без расхождений (см. §4) — файлы, которых касаются эти
+  AC, дельтой не тронуты.
+- **check-docs, golden:verify, browser-смоки** — diff (все три коммита
+  #333) не трогает `src/**` ни разу; `smoke-select.mjs` подтвердил
+  инструментом.
+- **`npm run invariants`** — diff не меняет геометрию/ссылки на неё.
+- **Полный `node scripts/mutation-gate.mjs`** — избыточен, диффу
+  соответствует один мутант, проверен точечно.
+- **performance-профили** — не названы в AC, чувствительный путь
+  (#330 executor) не тронут.
+- **Спор о line-level точности спеки/докстринга AC4** — не перечитывал
+  файл целиком заново построчно посимвольно, положился на факт «diff
+  `origin/dev...HEAD` для этого файла = один абзац, тот же, что видел
+  r1» — это сильнее, чем повторное чтение вслепую, потому что исключает
+  случайный второй правки в этом раунде.
+
+## 8. Вердикт
+
+r1-H1 закрыт доказательно (ранее красный тест теперь зелёный, полный
+бэкенд-набор чист), r1-Medium корректно вынесен в #347 вне этой ветки.
+Ребейз на ушедший вперёд `dev` между раундами рассмотрен явно и не
+потребовал полного повторного разбора AC, так как ни один файл #333 им
+не затронут — но все дешёвые гейты и полный бэкенд-набор прогнаны заново
+(зелёного Validate на `4ded9c0b` нет). Новых находок нет.
+
+Вердикт: зелёный · заход r2 · блокирующих циклов 1/2 · High: 0 · Medium: 0