diff --git a/docs/reviews/CODE-REVIEW-600-r2.md b/docs/reviews/CODE-REVIEW-600-r2.md new file mode 100644 index 00000000..810d711b --- /dev/null +++ b/docs/reviews/CODE-REVIEW-600-r2.md @@ -0,0 +1,191 @@ +# CODE-REVIEW-600-r2 + +Issue: #600 · Этап: code · Заход: r2 · Материал: `f342ccce2c6f5c6d33ba7fef39caa1db464d893a` (`origin/dev..HEAD`, `origin/dev` = `6cee9504`, не сдвинулся с r1 — ребейза нет) + +Вердикт: **зелёный** · заход r2 · блокирующих циклов 1/4 · High: 0 · Medium: 0 + +## Скоуп раунда и почему разбор идёт по дельте + +r1 (материал `fa120452`) вернул задачу жёлтым: 4 находки Medium (M1–M4), все в +скоупе, плюс одна Low (L1), без High. Дельта этого раунда — +`git diff fa120452..HEAD`, 5 коммитов (`32791fb3` документ r1, `8dc60aec`, +`ec97fd95`, `fb9a2459`, `0ccc503b`, `f342ccce`), 78 файлов, из них по существу +5 файлов `src/**` (5-21 строка каждый), 4 словаря i18n, 2 новых смока, один +мутант, документация задачи, changelog/USER-GUIDE и приёмка двух golden-сцен. + +Дельта строго локальна условиям §2.9/2.10: `origin/dev` не сдвинулся +(`6cee9504` — тот же, что был материалом аналитики), контракт поведения не +расширяется (M2 как раз **убирает** незамеченное расширение), новая подсистема +не затронута, объём дельты (≈80 строк продуктового кода + тесты) на порядок +меньше исходной задачи (3554+/2191- по `src/**`). Полный разбор не требуется — +разбираю дельту и то, до чего она дотягивается. + +## Закрытие раунда r1 + +| # | Находка r1 | Чем закрыта | Где это видно | +|---|---|---|---| +| M1 | «Temperature» в 5‑сегменте Room переносится посреди слова (`docs/design/600-settings-dialogs/pairs/room-*.png`, `form-kit.styles.ts:295-312` `overflow-wrap: anywhere`) | `.hpf-seg label`: `min-width: fit-content` (колонка не сжимается ниже слова) + `overflow-wrap: normal` (перенос только по пробелам) + `.hpf-seg { flex-wrap: wrap }` (не помещается — вторая строка целыми кнопками). Подписи заливки унифицированы в общие ключи `fill.seg_{none,lqi,light,temp,custom}` для Room и Space | `src/styles/form-kit.styles.ts:147-153,295-321`; новый смок `demo/smoke_dialog_segments_i18n.mjs` — **лично прогнан, все 4 диалога × en/ru/de/fr зелёные**; мутант `form-kit-segment-breaks-words` — **лично воспроизведён живьём** (см. «Мутанты» ниже), красит `en/ru/fr room noMidWordBreak`; кадры `docs/design/600-settings-dialogs/pairs/room-{light,dark}.png` пересняты — «Temperature» в одну строку (проверено просмотром PNG) | +| M2 | Glow radius получил не согласованную владельцем блокировку Save (`marker-form-state.ts:41-58`, `glowRadiusValid`) | `glowRadiusValid`/проблема `marker-glow-radius` удалены целиком; `marker-dialog.ts` больше не ставит `error`/`invalid` на поле радиуса; поведение снова как на `dev` — неположительное/нечисловое значение молча становится общим радиусом при сохранении (`houseplan-editor-runtime.ts:7991`, не тронут) | `git diff` `src/editors/marker-form-state.ts`, `src/editors/marker-dialog.ts` — построчно сверено, `strictNumber`‑валидация исчезла без следа; `demo/smoke_device_settings_form.mjs`: `badRadiusDoesNotBlockSave`, `zeroRadiusKeepsSaveEnabled` — **лично прогнаны, зелёные**; `USER-GUIDE.md/.ru.md` и оба changelog радиус из списка ошибок Save убрали | +| M3 | Дефект №5 (поповер «?» режется границей диалога комнаты) без свидетеля закрытия | Свидетель добавлен: новый смок доказывает исполнением, что подсказка живёт вне скроллера `.content` (top layer Popover либо `.overlay-portal`, сиблинг ``/``, а не потомок) | новый `demo/smoke_dialog_help_clipping.mjs` — **лично прогнан, зелёный**; **лично воспроизведена живая мутация**: перенос `
` внутрь `` (в light DOM, так что он слотится в `.surface` с `transform`+`overflow:hidden` заглушки mwc) красит `haDialogPortalOutsideSurface`, `ha_fallback_sizes_insideViewport`, `ha_fallback_sizes_notClipped` — смок действительно способен упасть на реалистичной регрессии, а не только на бумаге (два более ранних мутационных захода — `position:absolute` на `.tooltip` и перенос портала внутрь `.content` нативного `` — **не** красили ничего, потому что `position: fixed` не клипуется обычным `overflow:auto` без трансформированного предка; красная мутация нашлась только там, где предок реально трансформирован — то есть смок ловит именно ту ловушку mwc, ради которой заведён) | +| M4 | `ACCEPTANCE.md` заявляет 11 сцен, принято 13 | Абзац переписан: явно «13 сцен» = 11 из ТЗ + 2 `room-temperature-dialog-*`, с объяснением почему; добавлена таблица «AC8: девять дефектов — свидетель на каждый», дефект №5 → `smoke_dialog_help_clipping` | `docs/design/600-settings-dialogs/ACCEPTANCE.md` — прочитан целиком, текст и таблица на месте | +| L1 | `USER-GUIDE.ru.md:529` — испорченная строка таблицы (слипшийся хвост, лишний `\|`) | Строка восстановлена в свою ячейку (слои), лишний `\|` убран, соседняя строка про лучи актуализирована под новую формулировку слоя | `docs/USER-GUIDE.ru.md` — прочитан диф, таблица рендерится корректно; `node scripts/check-docs.mjs` — **лично прогнан, зелёный** | + +Все пять — правки в скоупе задачи, без нового issue, как и требует жёлтый +вердикт r1 (#202). + +## Унаследовано из r1 + +Без повторной проверки приняты выводы `docs/reviews/CODE-REVIEW-600-r1.md` +(материал `fa120452271f47bac64db153b60737bc7bb4e5336`, дерево +`437733cefa9f7febee3f9edfed4e5290760d517a`), так как дельта их не задевает: + +- **AC0** — референс/докс/PNG в `docs/design/600-settings-dialogs/`, вне бандла. +- **AC1** (кроме сегмента Room, закрытого M1 выше) — попарное визуальное + сравнение по остальным трём диалогам и структуре карточек. +- **AC3** — побайтовое сравнение конфига UI‑путь / state‑путь, поконтрольные + «свой ключ / соседний не тронут». Дельта не касается `dialog-baseline.ts` + или путей записи конфига. +- **AC4** — все ветки К4 диалога устройства сверены построчно с `dev`; дельта + трогает только `error`/`invalid` атрибуты одного поля (закрыто как M2), не + ветвление видимости. +- **AC5** — `hp-color-opacity` не переписан, `nativeColors() === 0`. +- **AC6** — dirty‑tracking/confirm‑on‑close переиспользуют `_confirmDanger`; + для диалога устройства re‑verified через `badRadiusDoesNotBlockSave` / + `reviewLinkFocusesBinding` (дельта M2 туда попадает), для Space/General/Room + наследуется без изменений. +- **AC7** — `rhint` отсутствует в четырёх диалогах. +- **AC9** — ARIA/geometry ≥44px в реальном DOM; дельта меняет только внутренний + перенос текста сегмента, не геометрию цели (перепроверено попутно через + `w320…w1920_segmentsFit` в целевых смоках). +- **AC10** — 173/173 golden на материале r1; в r2 golden **перепрогнан заново** + (см. «Гейты»), поэтому это не чистое наследование, а подтверждение. +- **AC11** — независимая фикстура `test/fixtures/summary-panel-editor.css` не + тронута; фикстуры диалогов (`form-kit-card-dialog*.css`) переигрались вместе + с CSS‑правкой M1 и байтово совпадают (`npm test` зелёный). +- **AC12** — потолок ленивого графа поднят в r1 с запасом >500 Б; дельта r2 + правки CSS/TS минимальны, фактический запас в r2 не сузился (см. «Гейты»). +- Трейлеры/changelog всех коммитов r1 — в порядке. + +## Гейты — что прогнано и чем (r2) + +Дешёвые гейты на этом SHA не были подтверждены отдельной ссылкой на «полный» +CI-прогон именно `f342ccce` (push‑Validate на `ec97fd95` — success; полный +`workflow_dispatch` на `ec97fd95` — две ожидаемые красноты, golden и preflight, +обе закрыты `f342ccce`, мутанты 2/3/5 из 6 подтверждены отдельным dispatch на +`f342ccce`), поэтому дешёвые гейты и обязательный по diff `src/**` docs‑гейт +прогнаны лично на `f342ccce`, рабочая копия к исходу приведена в чистое +состояние (`git status` — clean): + +| Гейт | Результат | +|---|---| +| `npx tsc --noEmit` | чисто | +| `npm run build && npm run bundle:sync` + `cmp` трёх копий бандла (`dist`, `custom_components/houseplan/frontend`, демо-стенд) | побайтово равны | +| `npm test` (полный) | **2794 pass, 0 fail**, 1 skipped (не связан с #600) | +| `node scripts/no-new-any.mjs --base fa120452 --head HEAD` | 22 добавленные строки в 5 файлах, новых `any` нет | +| `node scripts/check-docs.mjs` (`src/**` в диффе) | зелёный, отпечаток скриншотов свежий (обновлён `fb9a2459`) | +| `node scripts/process-gate.mjs` | пройден, 0 предупреждений (21 коммит `origin/dev..HEAD`) | +| `npm run golden:verify` (полный, 173 сцены) | **173/173 passed** — подтверждает AC10 на самом материале ревью после приёмки двух эталонов `f342ccce` | +| `npm run bundle:budget` | initial 289 826 / потолок 290 900±2000; lazy editor 243 061 / потолок 243 900±2000 — оба внутри допуска, дельта r2 не сузила запас относительно r1 (только предупреждение про общий низкий запас проекта, не связанное с #600) | +| `node demo/smoke_dialog_segments_i18n.mjs` (новый, M1) | лично, зелёный — 4 диалога × en/ru/de/fr | +| `node demo/smoke_dialog_help_clipping.mjs` (новый, M3) | лично, зелёный | +| `node demo/smoke_device_settings_form.mjs` (диф затронут, M2) | лично, зелёный, включая полосы ширин 320…1920 | +| `node demo/smoke_room_settings_form.mjs`, `smoke_space_settings_form.mjs`, `smoke_general_settings_form.mjs` | лично, зелёные (сегменты и полосы ширин не пострадали от CSS‑правки M1) | +| `node demo/smoke_ux_fixes.mjs` (диф затронут — подписи `fillLabels`) | лично, зелёный | +| `node demo/smoke_dialog_config_parity.mjs` (AC3, зона риска — форм-стейт устройства менялся) | лично, зелёный | + +`smoke-select.mjs --base fa120452 --head HEAD` вернул НЕОПРЕДЕЛЁННОСТЬ +(символы `ROOM_FILL_MODES`, `SPACE_FILL_UI_MODES`, `strictNumber` не связаны +ни с одним смоком доказуемо) — решение по каждому: `ROOM_FILL_MODES`/ +`SPACE_FILL_UI_MODES` использованы только для смены источника подписи +(`t()` → `st()`), покрыты `smoke_ux_fixes`/`smoke_dialog_segments_i18n` выше; +`strictNumber` — это **удалённый** импорт (M2 убрал валидатор), не новый +контракт, отдельного смока не требует. + +### Мутанты — воспроизведены живьём, не только статически + +- **`form-kit-segment-breaks-words`** (новый, для M1): применил оба патча + реестра (`min-width: fit-content` убран, `overflow-wrap: anywhere` + возвращён) → `smoke_dialog_segments_i18n` покраснел на `en/ru/fr room + noMidWordBreak` («Temperature»/«Температура»/«Température» снова в 2 + строках) → откатил, дерево восстановлено (`cmp` трёх копий бандла после + отката — совпадают). +- **Свидетель M3 без отдельного мутанта в реестре** — проверил сам: + (а) `position: absolute` на `.tooltip` в `hp-help.ts` — смок не покраснел + (`position: fixed` не клипуется обычным `overflow: auto` без трансформированного + предка — ожидаемо, ложная тревога исключена); (б) перенос + `.overlay-portal` внутрь скроллера нативного `` — тоже не + покраснел (нативный `` не транформирует `.surface`); (в) перенос + `.overlay-portal` внутрь `` (слотится в `.surface` заглушки с + `transform`+`overflow:hidden`, воспроизводящей реальную ловушку mwc) — + **покраснел** (`haDialogPortalOutsideSurface`, + `ha_fallback_sizes_insideViewport`, `ha_fallback_sizes_notClipped`). Все три + попытки отменены, дерево восстановлено. Вывод: смок — не ритуальная + проверка, он ловит именно ту регрессию (портал внутри трансформированной + поверхности mwc), ради которой заведён. + +### Чего не проверял и почему + +- **Полный `scripts/mutation-gate.mjs` (весь реестр, живой прогон)** — запущен + и остановлен: несоразмерно дельте (проверяет сотни несвязанных мутантов по + всему проекту); прогнаны только два релевантных этой дельте, вручную и + адресно (выше). +- **Настоящий `ha-dialog` (реальный Home Assistant)** — не поднимался; как и + в r1, это отдельная диагностическая приёмка владельца (#505), записанная в + `ACCEPTANCE.md`. Заглушка в новом смоке умышленно воспроизводит именно + ловушку transform+overflow:hidden реальной mwc‑поверхности, а не берётся на + веру — см. живую мутацию выше. +- **`python -m pytest tests_backend`** — диапазон не трогает + `custom_components/**/*.py` по существу. +- **`node scripts/model-invariants.mjs`** — диапазон не касается геометрии, + толщины стен, `layout`, `marker.space` или `open_spans`. +- **Полная smoke‑матрица (258 файлов)** — не прогонялась целиком; выбраны + смоки, которые дифф трогает напрямую (device/room/space/general settings + form, ux_fixes, config_parity) плюс два новых свидетеля M1/M3. Остальные 250+ + уже дважды подтверждены CI на близких по дереву коммитах (`fa120452`, + `ec97fd95`), а дельта после этого не переигрывает ничего вне пяти + перечисленных файлов `src/**`. + +## Одно число — один источник + +Дельта не вводит новых видимых пользователю величин (радиус свечения как был +числом с единицей, так и остался; подписи сегментов — текст, не число). +Единственная закрытая находка с числом — «11 vs 13 сцен» в `ACCEPTANCE.md» +(M4) — теперь одно число с объяснением, второй копии этого счётчика в +репозитории нет. + +## Трейлеры и changelog + +Все 6 коммитов дельты несут `Issue: #600`. Два `User-Visible: yes` +(`8dc60aec`, `ec97fd95`) правят `docs/CHANGELOG.md` и `docs/CHANGELOG.ru.md` в +том же коммите — проверено чтением диффа каждого коммита. Коммит `f342ccce` +(класс D, только `demo/golden/baselines/**`) несёт `Release: v1.77.0-beta.3` и +`Baseline-Reviewed: <ссылка на прогон CI>` — соответствует правилу приёмки +golden (§13); проверено `git show -s --format=full`. + +## Итог + +Ни одной новой High‑ или Medium‑находки. Все четыре Medium и одна Low из r1 +закрыты доказательствами, которые я лично воспроизвёл исполнением (включая +живые мутации там, где заявлялась защита) — не приняты на слово. Дешёвые +гейты и golden зелёные лично на материале ревью; рабочая копия по окончании +разбора чистая (`git status`), SHA не менялся. + +Возврат: **зелёный**. Следующий статус — очередь на пре-релиз. + +--- + + + +--- + + + +## Материал раунда + +- Ветка: `issue/600-settings-dialogs`, коммит `f342ccce2c6f` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `2fdbe72238842442fdd5d7f0a56108624fbe87f0` + ``` + git log --all --format='%H %T' | grep 2fdbe7223884 + ``` +- Тело issue: `2871bca5cd192e74cd862e2a4db109f96d559aecf77c8c2032369475e76aa1c6` +- Вердикт конвейера: `green` · High 0