From b53cca01d3885fdbcdc0f0e05a03bd3a524c3a13 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Fri, 18 Sep 2026 21:37:12 +0000 Subject: [PATCH] docs: review document for #594 Issue: #594 User-Visible: no --- docs/reviews/CODE-REVIEW-594-r1.md | 126 +++++++++++++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-594-r1.md diff --git a/docs/reviews/CODE-REVIEW-594-r1.md b/docs/reviews/CODE-REVIEW-594-r1.md new file mode 100644 index 00000000..c0c44cba --- /dev/null +++ b/docs/reviews/CODE-REVIEW-594-r1.md @@ -0,0 +1,126 @@ +# CODE-REVIEW-594-r1 + +Issue: #594 (шаг 1 эпика #591) · Ветка `issue/594-form-kit-room` · Материал: `c54a7770d92469916dedecac7d4595c55bea0cbe` (родитель `073c45b0`) · Заход r1 · блокирующих циклов до этого раунда: 0/4 + +## Скоуп + +Диапазон `origin/dev...HEAD` (`073c45b0..c54a7770`), 2 коммита: + +- `bd0cf829` feat: общий набор контролов формы и диалог комнаты по нему — `User-Visible: yes` +- `c54a7770` refactor: типизировать ключи заливки вместо `any` — `User-Visible: no` + +Файлы вне генерируемого (класс D) и трейлеры проверены командой `git diff origin/dev...HEAD` и `git log --format=full`. Классы: A — `src/editors/form-kit.ts` (новый), `src/styles/form-kit.styles.ts` (новый), `src/editors/room-settings-dialog.ts`, `src/houseplan-editor-runtime.ts`, `src/houseplan-card.ts`, `src/i18n/{ru,en,fr,de}.json`; B — `demo/smoke_room_settings.mjs`, `scripts/mutation-registry.mjs`, `test/*.test.mjs`; C — `docs/CHANGELOG*.md`, `docs/USER-GUIDE*.md`. Всё укладывается в скоуп ТЗ (`## ТЗ` → «Скоуп / не-скоуп»); `src/houseplan-card.ts` теряет только тонкую обёртку `_renderRoomSource`, прямое следствие переноса, а не расширение скоупа. Оба коммита несут `Issue: #594` и по одному `User-Visible:`; `bd0cf829` (yes) правит `docs/CHANGELOG.md` и `docs/CHANGELOG.ru.md` в том же коммите — трейлеры и §11 PROCESS соблюдены. + +ТЗ прошло ревью зелёным на r3 (`SPEC-REVIEW-594-r3.md`, High: 0, Medium: 0), обе Low-находки закрыты, i18n-таблица полна. Материал кода не редактировал тело issue после этого — вопросов к материалу ТЗ нет. + +## Как проверялось + +Инструкция разрешила не перегонять `typecheck`/`test`/`build` — Validate на `c54a7770` зелёный (https://github.com/Matysh/houseplan-card/actions/runs/35394685119). Проверено отдельно, что это действительно материал ревью: `headSha` прогона == `c54a7770d92469916dedecac7d4595c55bea0cbe`. Тем не менее часть работы требовала свежего локального дерева (браузерные смоки, golden, docs), поэтому `npm run build`/`npm run bundle:sync` выполнялись как побочный эффект — без отклонений от закоммиченного бандла (`git status` после сборки — чисто, дерево дистрибутива побайтово совпало). + +| Гейт | Результат | Почему | +|---|---|---| +| `npx tsc --noEmit`, `npm run build` | не гонял отдельно — уже в Validate `c54a7770`; но `npm run build`/`bundle:sync` выполнялись для смоков/golden и прошли чисто (`tsc --noEmit && rollup`) | дешёвый гейт подтверждён CI | +| `npm test` (весь набор) | не гонял целиком — подтверждён Validate (`Фронтенд: типы, юниты, мутанты, синхрон бандла` = success); прогнал вручную целевой поднабор | см. ниже | +| `node --test test/i18n.test.mjs test/i18n-dead-keys.test.mjs test/form-kit.test.mjs test/styles-split.test.mjs` | 43/43 pass | ключевые тесты этой задачи, включая замороженную фикстуру AC11 | +| `node scripts/no-new-any.mjs --base origin/dev --head HEAD` | «новых any нет» | подтверждает решение коммита `c54a7770` | +| `npm run bundle:budget` | initial View 291872 Б (потолок 292400±2000), запас 528 Б; сверено с `origin/dev` (291356 Б, запас 1044 Б) — рост ровно +516 Б, как заявлено | AC8 | +| `node scripts/mutation-gate.mjs --id=form-kit-writes-to-a-neighbour-key` | `поймано 1 из 1` | AC2 защитный столбец «чем краснеет» | +| `node scripts/mutation-gate.mjs --id=form-kit-segment-drops-radio-semantics` | `поймано 1 из 1` | AC5/К7 защитный столбец | +| `node demo/smoke_room_settings.mjs` | все ключи `out` truthy, `OK` | AC1/AC2/AC5/AC6, включая новые `nameWritesOnlyItsOwnKey`, `scaleWritesOnlyItsOwnKey`, `humiditySourceWritesOnlyItsOwnKey`, `everyGroupHasHelp`, `sourceSegment*` | +| `node demo/smoke_color_picker_consumers.mjs`, `smoke_room_temperature_thresholds.mjs`, `smoke_help_affordance.mjs`, `smoke_summary_panel.mjs`, `smoke_summary_panel_polish.mjs`, `smoke_font_scales.mjs`, `smoke_editor_tabs.mjs` | все `OK` | AC3, AC4, AC6 (доп.), AC9, регресс на соседей из «прямого совпадения» `smoke-select` | +| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | 19 «прямых совпадений» (все выше плюс 12 геометрических/декоровых смоков, не относящихся к форме — не гонял: символы `_curSpaceCfg`/`_nameSel` совпадают случайно, диалог настроек комнаты их не трогает по логике письма) + 31 «слабых связей» по `_curSpaceCfg` (не гонял — общий контекстный символ, форма не меняет геометрию/decor) | выбор смоков соразмерно диффу | +| `npm run golden:verify` (полный линуксовый набор, Chromium 151.0.7922.34 — совпадает с индексом эталонов) | **два расхождения сверх объявленных** — см. находку M1 | AC7 | +| `npm run docs:accept -- --identical` (диагностика, изменения отменены) | все 11 кадров документации побайтово совпали; принимать нечего, кроме отпечатка исходников | AC/Release-артефакты, см. находку M2 | +| `python -m pytest tests_backend` | не гонял — диф не касается `custom_components/**/*.py` | не применимо | +| перф-профили | не гонял — диф не касается `src/iso-*`, `src/live-*`, `src/render-*`, `houseplan-render-lifecycle.ts`, `houseplan-card.ts` рендер-путей (только удаление тонкой обёртки); лист набора — в ленивом графе, новых проходов по данным нет (AC8 подтверждает размер, не время) | вне AC, вне названных путей | +| `git worktree` на `origin/dev` (073c45b0) — контрольный `bundle:budget` и `golden:verify` | воспроизвёл то же самое: заголовок headroom-предупреждения и «different» `device-icon-state-table-{light,dark}` уже были на `dev` до этой задачи | атрибуция находок ниже | + +Не гонял: полную матрицу смоков (250 файлов) — задача не задевает всё; `pytest`; perf-профили (не применимо по путям). Обоснование выбора приведено в таблице. + +## Находки + +### Medium (в скоупе задачи) — M1: AC7 не закрыт до конца — эталоны не приняты + +`npm run golden:verify` на полном линуксовом артефакте (Chromium 151.0.7922.34, совпадает с `baselineManifest.chromium` в эталонах — то самое несоответствие 152/151, которое автор не смог обойти в песочнице, здесь не воспроизводится) даёт: + +``` +different device-icon-state-table-light +different device-icon-state-table-dark +different room-temperature-dialog-desktop-en +different room-temperature-dialog-mobile-ru +``` + +Две последние — ровно объявленные AC7 сцены, и визуально они корректны (см. `artifacts/golden/actual/room-temperature-dialog-{desktop-en,mobile-ru}.png` — карточки «Basics/Fill/Sensor sources», «Основное/Заливка», сегмент источника, значения и порядок полей совпадают с ТЗ). Две первые (`device-icon-state-table-*`) я атрибутировал отдельно: контрольный прогон `golden:verify` на чистом `origin/dev` (`073c45b0`, до этой задачи) даёт **то же самое** расхождение по этим двум сценам — это уже существующий, не связанный с #594 дефект (сцена добавлена под #588; регрессия чужая, трогать её в этой задаче нельзя, а заводить отдельный issue — не моя роль корректировать чужой скоуп находкой этого ревью, если только автор сам не поднимет вопрос; это pre-existing и не входит в блокирующий счёт). + +Проблема в другом: **AC7 требует не только «диф ограничен объявленными сценами» (это так), но и того, что эталоны для этих двух сцен пересняты и приняты** («Release-артефакты»: «эталоны двух объявленных сцен»). В диффе `git diff origin/dev...HEAD --stat` **нет ни одного изменения `demo/golden/baselines/**`** — новые кадры не приняты, коммит не несёт `Release:`/`Baseline-Reviewed:`. Если эту ветку слить как есть, `room-temperature-dialog-{desktop-en,mobile-ru}` останутся красными на первом же тяжёлом прогоне (кандидат беты, nightly) — притом что расхождение полностью предсказано и ожидаемо, не «дефект, который не мог проявиться раньше» (§11.4 сюда не подходит). + +**Чем краснеет:** `npm run golden:verify` на полном Linux-артефакте — уже красное на этом материале. +**Правка:** принять новые эталоны `npm run golden:accept -- --reviewed <ссылка на зелёный тяжёлый Validate/Docs-прогон>` с коммитом, несущим `Release:`/`Baseline-Reviewed:` (§10.1, правило 13/PROCESS.md), и запушить перед повторным заходом на код-ревью. + +### Medium (в скоупе задачи) — M2: отпечаток скриншотов документации не обновлён + +`node scripts/check-docs.mjs` красит: `ERROR screenshot source fingerprint is stale`. Диф трогает `src/**`, поэтому по правилу «выбирать тут нечего» (PROCESS.md §8, прецеденты #230/#234/#237) это ожидаемо и обязательно к починке **до** слияния — иначе `dev` получает красный job `docs` до следующей задачи, как уже дважды случалось. Проверил, что дело именно в отпечатке, а не в реальном визуальном расхождении: `npm run docs:accept -- --identical` (прогнал как диагностику, изменения в рабочей копии затем отменил) — «Все 11 кадров попиксельно совпали с закоммиченными: принят только отпечаток исходников». Диалог настроек комнаты действительно не открывает ни один из 10 сценариев `demo/docs` (ТЗ это верно называет), поэтому фикс тривиален и не требует пересъёмки на CI. + +**Чем краснеет:** `node scripts/check-docs.mjs` — красное сейчас. +**Правка:** `npm run docs:accept -- --identical` и закоммитить обновлённый `docs/images/screenshots.json` (класс C, не требует отдельного issue — часть DoD этой задачи). + +### Medium (в скоупе задачи) — M3: AC1 доказан у́же, чем заявлено + +ТЗ обещает для AC1 «новый unit `test/form-kit.test.mjs` — таблица «контрол → ключ → записанное значение» на фейковом хосте» для восьми полей: имя, область, режим заливки, цвет, пороги, два источника, два масштаба. По факту `test/form-kit.test.mjs` содержит три теста другого назначения (дословность CSS-фикстуры панели — AC11, параметризация имён генератора, радиосемантика сегмента — AC5/К7); таблицы «控трол → ключ» там нет вовсе. Контракт «пишет только своё поле» вместо этого проверяет расширенный `demo/smoke_room_settings.mjs`, но только для 3 из 8 полей: `name`, `nameScale`, `humSrc` (`nameWritesOnlyItsOwnKey`, `scaleWritesOnlyItsOwnKey`, `humiditySourceWritesOnlyItsOwnKey`). Цвет и пороги закрыты отдельно неизменными `smoke_color_picker_consumers.mjs`/`smoke_room_temperature_thresholds.mjs` (AC3/AC4 — это честно). Но **область (`_areaSel`) и режим заливки (`_roomFill`, включая только что перетипизированный `FILL_CHOICES`) не имеют исполнимого oracle вообще** — ни unit, ни smoke не кликает по ``; `test/form-kit.test.mjs` статически проверяет разметку, `demo/smoke_room_settings.mjs` — реальные размеры цели (≥44px) и фокус в браузере; мутация `type="radio"→"checkbox"` ловится (`поймано 1 из 1`). +- **AC8 (бюджет).** Число совпадает с заявленным (+516 Б до 291872, потолок 292400±2000); лист набора действительно не входит в `cardStyles` (`test/styles-split.test.mjs`, «issue 266 the aggregator is exactly the five surface files» — не тронут) и вносится в теневой корень отдельно (`ensureFormKitStyles`, `adoptedStyleSheets`/фолбэк на `