docs: review document for #600

Issue: #600
User-Visible: no

docs/reviews/CODE-REVIEW-600-r1.md

@@ -0,0 +1,273 @@
+# CODE-REVIEW-600-r1
+
+Issue: #600 · Этап: code · Заход: r1 · Материал: `fa12045271f47bac64db153b60737bc7bb4e5336` (детач, диапазон `origin/dev..HEAD`, `origin/dev` = `6cee9504`)
+
+Вердикт: **жёлтый** · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 4 → в задаче
+
+## Скоуп и что проверялось
+
+Задача переписывает внутренности четырёх диалогов настроек (Space, General
+settings, Room settings, Device on the plan) под референс дизайнера,
+сохраняя запись состояния (К1–К10 ТЗ). Диапазон: 15 коммитов поверх `dev`,
+199 файлов, из них по существу — 35 файлов `src/**` (3554+/2191-), 64 файла
+тестов/смоков/докс/чейнджлога. Референс, `SPEC.md`, `IMPLEMENTATION-GUIDE.md`
+уже разложены в `docs/design/600-settings-dialogs/` (AC0, принят на спек-ревью).
+
+Спек-ревью r1 по этой же задаче — зелёный (комментарий issue от 20.09,
+06:18), одна Low-находка снята без последствий для кода. Это первый заход
+код-ревью, предыдущего кода-ревью раунда не было — раздела «Унаследовано из
+r0» нет, разбор ниже полный.
+
+### Методология
+
+1. Прочитан `git diff origin/dev...HEAD` целиком по файлам (частями для
+   `marker-dialog.ts`, 1247 строк диффа) и структура `docs/design/...`.
+2. Запущено четыре параллельных фоновых агента с точными предписаниями
+   (файлы, строки ТЗ, конкретные критерии) на AC1/AC4/AC8 (по диалогам),
+   AC3/AC5/AC6/AC7 и мутанты, AC9–AC13 + трейлеры/changelog. Их находки
+   перепроверены выборочно (see below), а не приняты на слово.
+3. Самостоятельно: визуальное попарное сравнение референс/продукт по всем
+   4 диалогам × 2 темы (`docs/design/600-settings-dialogs/pairs/*.png` против
+   `screenshots/*.png`, с покадровым zoom через Pillow) — так была найдена
+   находка №1 ниже, которую не поймал ни один смок и не назвал сам автор.
+4. Проверка полноты CI-подтверждения на точном материале: dispatch-прогон
+   `35506392847` на `fa120452` **не** тяжёлый (`full=true` не задан) и для
+   `smoke`/`golden` не нашёл кэша переиспользования («Cache not found for
+   input keys: reuse-smoke-…», «reuse-golden-…» — see `Переиспользование` job
+   лога), то есть **ни smoke-матрица, ни golden не подтверждены CI на самом
+   материале ревью** — только на промежуточном `504c1609` (там же — одна
+   красная браузерная проверка, исправленная позже в `4bd80e46` без
+   последующего CI-подтверждения). Это закрыто мной лично (см. «Гейты» ниже),
+   а не принято на слово автора.
+
+## Гейты — что прогнано и чем
+
+Дешёвые гейты (`tsc --noEmit`, `npm test`, `npm run build`) уже зелёные на
+`fa120452` по ссылке из задания (Validate `35506392847`, success) — не
+перегонялись.
+
+Прогнано лично или агентами, с результатом:
+
+| Гейт | Кто | Результат |
+|---|---|---|
+| `npm run build && npm run bundle:sync` | я | чисто, дерево не изменилось |
+| `npm run golden:verify` (полный, 173 сцены) | я | **173/173 passed** — закрывает пробел CI (см. выше); подтверждает AC10 на самом материале, включая 13 новых и все старые сцены побайтово |
+| `node demo/smoke_room_tooltip_toggle.mjs` | я | OK, все 25 ключей `true` — закрывает пробел CI: у фикса из `4bd80e46` не было ни одного зелёного CI-прогона на финальном дереве |
+| `node demo/smoke_dialog_config_parity.mjs` | агент A | OK (AC3, побайтовое сравнение конфига через UI-путь и state-путь) |
+| `node demo/smoke_{space,general,room,device}_settings_form.mjs` | агенты A/B/C | все OK; проверяют собственный ключ + нетронутость соседнего для каждого нового контрола, `role=radiogroup`+`aria-label` на сегментах, цели ≥44px |
+| `node demo/smoke_color_picker_consumers.mjs` | агент A | OK, `nativeColors()===0` держится |
+| `node demo/smoke_esc_dialogs.mjs` | агент A | OK, включая переспрос при закрытии с изменениями |
+| `node demo/smoke_help_affordance.mjs`, `node --test test/i18n-dead-keys.test.mjs` | агент A | OK |
+| `node --test test/form-kit.test.mjs` | агент D | 5/5 pass, включая байтовое сравнение CSS панели с замороженной `test/fixtures/summary-panel-editor.css` (файл не тронут диапазоном — AC11 не фиктивен) |
+| `npm run bundle:budget` | агент D | потолок `LAZY_EDITOR_GZIP_CEILING` 222900→243900 с построчным обоснованием по сериям; факт 243125, зазор 775/1225 Б (оба >500) |
+| `node scripts/check-docs.mjs` | агент D | зелёный на HEAD (скриншоты пересняты в `fa120452`) |
+| `npm test` (полный) | агенты A/D независимо | 2794/2793 pass, единственный флак — не связанный с #600 тест по времени сборки одного этажа (`#509`), не влияет на вердикт |
+
+### Чего не проверял и почему
+
+- **Мутанты из `scripts/mutation-registry.mjs` не исполнены живьём** (patch →
+  guard → RED → revert) — только статическая проверка, что anchor-строки
+  существуют один раз и обоснование соответствует коду. Полный mutation-run
+  дорог и не назван в AC как обязательный отдельно от диффовых мутантов,
+  которые уже зелёные в CI на этом SHA («Мутанты по диффу 1–6/6» — success).
+- **Не все 88 «прямых совпадений» из `smoke-select.mjs` перепрогнаны поштучно
+  моей рукой.** Полная матрица уже прогнана CI дважды за время задачи
+  (`workflow_dispatch full=true` на `504c1609`, шарды 1/3 и 3/3 — success
+  целиком, шард 2/3 — одна красная, исправленная в `4bd80e46` и лично
+  перепроверенная мной на финальном дереве, см. таблицу выше). Дельта между
+  `504c1609` и `fa120452` — сам смок-фикс, два golden-коммита и один
+  docs-тест, ни один из них не переиграл более широкий смок. Прогон всех 88
+  заново не пропорционален размеру этой дельты.
+- **`python -m pytest tests_backend`** не запускался — диапазон не трогает
+  `custom_components/houseplan/**/*.py` по существу (только собранный
+  `frontend/**`, класс D).
+- **Живые ha-dialog** (реальный Home Assistant) не поднимались — только
+  демо-харнесс с нативным `<dialog>`; это и есть причина, по которой находка
+  №3 ниже (поповер «?») осталась неподтверждённой в обе стороны, а не закрытой.
+
+## Находки (все — Medium, в скоупе задачи, к исправлению в этом же заходе)
+
+### 1. Room settings: подпись «Temperature» в 5-сегменте переносится посреди слова
+
+`docs/design/600-settings-dialogs/pairs/room-light.png` и `room-dark.png` —
+кадры, которые сам автор снял и приложил как доказательство AC1 — показывают
+сегмент «Fill mode» (`None | Zigbee | Lights | Temperature | Custom`), где
+активный пункт отображается как `Temperatur` / `e` двумя строками внутри
+кнопки. Референс (тот же `SPEC.md` §6.1, тот же список из пяти пунктов)
+рассчитан на одну строку на пункт.
+
+Причина — `src/styles/form-kit.styles.ts:295-312`: `.hpf-seg label` держит
+`flex: 1 1 0; min-width: 0; overflow-wrap: anywhere` — при пяти равных
+колонках слово `Temperature` (11 симв.) не помещается и рвётся в
+произвольном месте вместо аккуратного переноса или уменьшения кегля.
+`src/editors/room-settings-dialog.ts:197-200` документирует, что автор был в
+курсе тесноты пяти сегментов («Короткие подписи там, где полные не
+помещаются…») и сократил `lqi`→«Zigbee», `custom`→«Custom», но не тронул
+`fill.temp` — то самое слово, которое рвётся. RU-вариант («По температуре»,
+14 символов) длиннее английского и пострадает не меньше; DE/FR — сопоставимо.
+
+`docs/design/600-settings-dialogs/ACCEPTANCE.md:69` при этом утверждает
+«Подписи сегмента заливки … то же» и не называет это расхождение —
+единственная строка ACCEPTANCE.md про подписи segment на самом деле не
+покрывает пять из пяти пунктов честно. AC2 («без переноса, читаемо при
+длинных RU/DE/FR») формально проверяет только отсутствие горизонтального
+скролла контейнера (`demo/smoke_room_settings_form.mjs:192`,
+`content.scrollWidth <= content.clientWidth`), а перенос внутри кнопки
+scrollWidth контейнера не трогает — поэтому смок зелёный, а дефект есть.
+
+**Сценарий:** открыть Room settings любой комнаты с выключенным «As the
+space», посмотреть на сегмент Fill mode — на активном/любом широком языке
+пункт «Temperature» отображается разорванным словом. Это ровно тот класс
+дефекта («ложный паритет», «сырая раскладка»), ради которого заведён #600.
+
+### 2. Device dialog: у Glow radius появилась не согласованная владельцем блокировка Save
+
+`src/editors/marker-form-state.ts:41-58` вводит `glowRadiusValid()` и новый
+`MarkerProblem` `marker.error_glow_radius`, который через
+`marker-dialog.ts` (`canSave = ... && problems.length === 0 ...`) **блокирует
+кнопку Save**, если поле Glow radius непусто и не является положительным
+числом.
+
+На `dev` это поле никогда не блокировало Save: запись конфига
+(`houseplan-editor-runtime.ts:7991-7995`, код не изменился в этом диапазоне)
+молча приводит любое непустое невалидное или неположительное значение к
+`null` (= общий радиус) прямо на сохранении. К10 в теле issue перечисляет
+закрытым списком, какие условия остаются ошибками — «пустое имя, режим файла
+без изображения, невалидный диапазон температур, отсутствие привязки при
+выборе из HA» — Glow radius в списке нет. Ни `OPEN-POINTS.md`, ни решения
+владельца от 20.09, ни один комментарий issue не называют эту новую
+проверку. Это функциональное расширение контракта Save (AC6/К10) за пределы
+переразметки, не решённое владельцем.
+
+**Сценарий:** диалог устройства → «Light and glow» → Glow radius → ввести
+`0` или `-1` → кнопка Save становится неактивной без объяснения, почему это
+стало ошибкой, — на `dev` то же действие сохраняло маркер с общим радиусом.
+
+Это не потеря данных и не крах, но это несогласованное расширение
+поведения контракта Save, а не просто вёрстка. Fix: либо снять валидацию
+(вернуть прежнее молчаливое приведение к `null`, как и было), либо получить
+явное решение владельца о пятом условии ошибки Save и внести его в текст
+К10, если задача снова пойдёт на спек-ревью, либо (раз это в рамках
+code-review light track) явно задокументировать это расширение как
+осознанное отступление в `ACCEPTANCE.md`/`IMPLEMENTATION-GUIDE.md` — сейчас
+это выглядит как незамеченный побочный эффект.
+
+### 3. Дефект №5 (поповер «?» обрезается границей диалога комнаты) не имеет свидетеля закрытия
+
+AC8 требует «девять прямых дефектов… закрыты поимённо», «по свидетелю на
+каждый». Для дефекта №5 (Room settings) свидетеля нет:
+
+- в диапазоне нет ни одной строки, адресующей клиппинг поповера (нет правки
+  `overflow`, нет промоушена `.overlay-portal` в top layer, `hp-help.ts` не
+  тронут);
+- `ACCEPTANCE.md` не упоминает дефект №5 ни разу — только №1 (Zigbee links);
+- в демо-харнессе (нативный `<dialog>`) клиппинг не воспроизводится ни в
+  одну, ни в другую сторону — харнесс использует другой путь модальности,
+  чем реальный `ha-dialog` (`src/hp-dialog.ts:553-609`,
+  `_usesHaDialog()`), так что харнесс не может ни подтвердить, ни опровергнуть
+  исходный дефект;
+- ни `demo/smoke_room_settings_form.mjs`, ни `demo/smoke_help_affordance.mjs`
+  не проверяют клиппинг попапа границей диалога.
+
+Не факт, что дефект жив (структура вокруг `hp-help` не менялась этим PR), но
+AC8 требует именованного доказательства закрытия для каждого из девяти
+пунктов, а не «скорее всего не мой код» — сейчас его нет ни для одного, ни
+для другого исхода. Нужно либо воспроизвести на реальном `ha-dialog`
+(методика #505) и показать, что не режется, либо признать дефект
+непроверенным и решить его в этом же заходе.
+
+### 4. `ACCEPTANCE.md`: заявлено 11 эталонных сцен, фактически принято 13
+
+`docs/design/600-settings-dialogs/ACCEPTANCE.md:28` (правился в `c3e64dc6`,
+до принятия эталонов) утверждает «Эталоны одиннадцати сцен». Фактически
+`ed2af1b1` («эталоны 13 сцен диалогов настроек») меняет хэши 13 сцен в
+`demo/golden/baselines/baselines-index.json` — расхождение 11 vs 13 нигде не
+объяснено, документ не синхронизирован после приёмки эталонов. AC1/AC10
+прямо требуют, чтобы расхождения были «названы», а не «примерно посчитаны» —
+это тот самый принцип, ради которого заведён #600 (не доверять цифре на
+глаз). Фикс — одна строка в `ACCEPTANCE.md`.
+
+### 5. `docs/USER-GUIDE.ru.md:529` — испорченная строка таблицы
+
+```
+| Карточки комнат | Сигнал Zigbee рядом с устройствами | Переопределяет базовую настройку карточки для этого пространства |ожки они остаются видимыми |
+```
+
+Хвост «ожки они остаются видимыми» — остаток удалённой строки про
+декоративный слой, слипшийся с текущей ячейкой без переноса строки: лишняя
+`|` создаёт для парсера Markdown лишний столбец, и в отрендеренной таблице
+пользователь увидит обрывок чужого предложения. Английская версия
+(`docs/USER-GUIDE.md`) аналогичной порчи не содержит — расхождение только в
+RU. Раз `User-Visible: yes` требует правку `USER-GUIDE` как часть DoD (и она
+входит в явный скоуп задачи), это Medium-находка внутри скоупа, а не
+придирка к стилю — текст, который увидит пользователь, буквально сломан.
+
+## Что проверено и корректно (по AC)
+
+- **AC0** — референс/докс/PNG на месте, отпечаток архива сверен ранее на
+  спек-ревью; ничего из `docs/design/**` не попадает в бандл (`bundle-tree`
+  зелёный в CI).
+- **AC1** (кроме находки №1) — прямое попарное сравнение восьми кадров
+  (4 диалога × 2 темы) с референсом показывает подлинный, а не косметический
+  паритет: сегменты, строки-тумблеры, плитки значений/цвета, компактная
+  плашка цвета, футер со статусом, единый скролл — всё воспроизведено по
+  структуре, а не только по обёртке карточек. Дублирующийся заголовок Zigbee
+  links (дефект №1) закрыт и подтверждён смоком (`zigbeeHeadingOnce`).
+- **AC3** — доказано исполнением на двух независимых уровнях: побайтовое
+  сравнение сохранённого конфига (`smoke_dialog_config_parity.mjs`) и
+  поконтрольные unit/смок-проверки «свой ключ меняется — соседний нет» для
+  каждого нового примитива. Тесты способны падать (проверено по формулировке
+  ассертов, не только по названию).
+- **AC4** — все ветки К4 (Entity to toggle, What to run, Leading light
+  entity, Value source, температура климата, радар/пылесос, баннеры
+  привязки, `ha_registry_limited`) сверены построчно с `origin/dev` — условия
+  видимости не изменились, изменена только обёртка в новые примитивы.
+- **AC5** — `hp-color-opacity` не переписан по семантике (диф — 6 строк,
+  только `hideLabel`), `nativeColors() === 0` держится и подтверждено
+  смоком; вокруг него ровно новая раскладка (свотч/hex/Opacity/Reset).
+- **AC6** — dirty-tracking и confirm-on-close переиспользуют существующий
+  `_confirmDanger` (не новый механизм), Save заблокирован при `busy` и
+  ошибках, «Review N fields» реально переводит фокус на первое проблемное
+  поле (проверено `activeElement.id`, не только текстом).
+- **AC7** — `rhint` в четырёх целевых диалогах отсутствует (0 совпадений);
+  оставшиеся вхождения (`vacuum-maps-section.ts`, `import`-диалог онбординга)
+  — вне скоупа четырёх диалогов и не регрессия этого PR (были такими же на
+  `dev`). Сообщения-состояния (К5) остаются открытыми callout-блоками.
+- **AC9** — `role="radiogroup"`/`role="group"` с `aria-label`, `label[for]`,
+  `aria-invalid`, `aria-live`, reduced-motion/forced-colors — присутствуют в
+  коде и подтверждены геометрией/атрибутами в реальном DOM через Playwright
+  (не заглушка по названию класса).
+- **AC10** — 173/173 сцен golden проходят на материале ревью (прогнано
+  лично, см. «Гейты»); трейлеры `Release`/`Baseline-Reviewed` на коммите с
+  эталонами на месте.
+- **AC11** — независимый замороженный фикстур панели (`summary-panel-editor.css`)
+  не тронут диапазоном, тест сравнивает его побайтово с реальным собранным
+  CSS — доказательство не фиктивно, рост диалоговых фикстур это отдельный,
+  явно расширяемый набор.
+- **AC12** — потолок поднят с построчным обоснованием по сериям, факт держит
+  запас >500 Б с обеих сторон полосы.
+- **Трейлеры/changelog** — все 15 коммитов несут `Issue`/`User-Visible`,
+  единственный `User-Visible: yes` правит оба changelog в этом же коммите.
+
+## Итог
+
+Ни одной High-находки: данные не теряются, инвариантов геометрии/замков не
+касается, полный набор автотестов и golden зелёные на материале ревью.
+Четыре находки Medium — все внутри объявленного скоупа задачи (Room dialog,
+Device dialog, собственная документация задачи, USER-GUIDE) — по правилу
+#202 не заводят отдельный issue и чинятся в этом же заходе. Возврат автору:
+жёлтый.
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `issue/600-settings-dialogs`, коммит `fa12045271f4` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `437733cefa9f7febee3f9edfed4e5290760d517a`
+  ```
+  git log --all --format='%H %T' | grep 437733cefa9f
+  ```
+- Тело issue: `2871bca5cd192e74cd862e2a4db109f96d559aecf77c8c2032369475e76aa1c6`
+- Вердикт конвейера: `yellow` · High 0