docs: review document for #489

Issue: #489
User-Visible: no

docs/reviews/SPEC-REVIEW-489-r1.md

@@ -0,0 +1,236 @@
+# SPEC-REVIEW-489-r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/489
+- **Этап:** ТЗ на ревью (PROCESS.md §2.4)
+- **Материал:** `docs/specs/489-data-hp-contract.md` на SHA `9cf10bdde7696e13c049fe83b4dc284622386127`
+  (commit `9cf10bdd`, ветка `issue/489-data-hp-contract`), плюс тело issue #489 и
+  комментарии аналитики/автора.
+- **Трек:** полный (обоснование в комментарии аналитики: несколько поверхностей +
+  новый публичный DOM-контракт — корректно, критерий `small` не пройден).
+- **Заход:** r1 (первый).
+
+## Скоуп разбора
+
+Первый цикл — разбор полный: прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md`
+целиком, тело issue #489 и оба комментария, действующий `docs/STYLING-HOOKS.md`,
+и сам ТЗ-документ целиком. Каждый раздел §6 (контракт поведения) сверен построчно
+с текущим исходником: `src/houseplan-card.ts` (`_booting`, `_mode`, header
+`.hdr`/`.zoomctl`/`.tabs`/`tabadd`/`.tabedit`, пустые/fixed-floor ветки,
+`toast`), `src/houseplan-editor-runtime.ts` (`_renderMarkupBar`,
+`_renderDevicesBar`, `_renderDecorBar`, `.editbar`/`.barclose`), `src/hp-dialog.ts`
+(host, fallback `.close`, отсутствие текущего понятия «kind»), 33 call site
+`<hp-dialog` в 9 файлах (выборочно проверены `gs.align_title`, `backup.*`,
+`kiosk.title`, `rules.title`, `import.title`), `src/houseplan-panel.ts` (меню и
+заголовок), `src/editor-secondary.ts` (единая контекстная поверхность —
+кандидат на «tray»), `src/space-card.ts` (`houseplan-space-card`: свой `ha-card`,
+без `_booting`/`_mode`). Issue #486 закрыт и соответствует описанной панели.
+
+Продуктовая рамка (§7.1: сценарий, что видит пользователь) не разбиралась заново
+для каждой строки — это первый заход, но она короткая и внутренне
+непротиворечивая: видимого поведения нет, персонажи — Home admin и внешний
+E2E/card-mod автор, что подтверждено §SCOPE.md (View — продукт, редакторы —
+admin-only, никакого нового пользовательского функционала не заявлено).
+
+## Находки
+
+### M1 — Условие существования зум-кнопок противоречит текущему рендеру и не-скоупу (Medium, в скоупе)
+
+**Файл:** `docs/specs/489-data-hp-contract.md`, §6.3, строки 121–123 (`zoom-in` /
+`zoom-out` / `zoom-fit`, условие «когда отображается header»).
+
+**Проблема.** В текущем коде (`src/houseplan-card.ts:11400`,
+`src/styles/chrome.styles.ts:98`) кнопки zoom **не имеют** отдельного условия
+рендера: `.zoomctl` рендерится безусловно, а `.hdr` в kiosk получает класс
+`kioskhide` (`display:none` только через CSS). Сам блок `.hdr` и его дети,
+включая zoom-кнопки, остаются в DOM и в kiosk-режиме — скрывается только
+визуально. Явное исключение из DOM по kiosk сделано только для `tabadd`
+(`space-add`, строка 11435: `this._canEdit && !this._kiosk && !this._hasFixedFloor`),
+и комментарий в исходнике (11430–11434) прямо объясняет, что это осознанное
+дополнительное решение именно для этой кнопки — «the whole `.hdr` is
+display:none there, but the button is **also** not RENDERED».
+
+Формулировка §6.3 «когда отображается header» — единственная в таблице, где
+условие не привязано к «как сейчас» (в отличие от `settings`/`pdf`/`support`,
+где явно «as сейчас»). Прочитанная буквально, она требует **нового** условия
+рендера (kiosk ⇒ элемент отсутствует в DOM), которого сегодня нет и которое
+правило 6.1.4 связывает с «не существует по permission/mode/state-правилам» —
+то есть с реальным JS-условием, а не CSS-видимостью. Добавление такого условия
+— это правка логики видимости, прямо запрещённая §5 «Не входит: … изменение
+логики готовности, загрузки, редакторов **или разрешений**».
+
+Если же имелось в виду обратное («header отрисован всегда, значит хук
+присутствует всегда, в т.ч. в kiosk, только невидимо») — формулировка вводит в
+заблуждение и разойдётся с машиночитаемым `docs/data-hp-contract.json` и текстом
+`STYLING-HOOKS.md`, которые унаследуют ту же фразу.
+
+**Почему это находка, а не техническая мелочь.** AC2 требует «все доступные в
+текущем состоянии header actions находятся по селекторам», AC6 требует падения
+теста на любое незадекларированное/несовпадающее поведение, а
+`demo/smoke_styling_hooks.mjs` должен получить kiosk-фикстуру — без
+однозначного условия эти проверки нельзя написать правильно с первого раза.
+
+**Что нужно.** Явно зафиксировать одно из двух:
+(a) существование хука = существование DOM-элемента как сегодня (kiosk не
+меняет DOM, только CSS) — тогда строку привести к формулировке «как сейчас»,
+как у соседних строк; или
+(b) добавить `!this._kiosk`-условие к zoom-кнопкам как новую, но безвредную для
+пользователя правку — тогда явно назвать это в §12 (риски) как намеренное,
+минимальное расширение существующей практики (по аналогии с `tabadd`), а не
+«не входит».
+
+### M2 — Гейт свежести документации (`check-docs`) не назван обязательным при правке `src/**` (Medium, в скоупе)
+
+**Файл:** `docs/specs/489-data-hp-contract.md`, §10 AC7 (строка 312–313) и §14
+(строки 368–375).
+
+**Проблема.** Задача правит `src/houseplan-card.ts`,
+`src/houseplan-editor-runtime.ts`, `src/hp-dialog.ts`, `src/houseplan-panel.ts` и
+другие файлы под `src/**` — то есть задевает `src/**` практически целиком по
+охвату (шапка, все три редактора, диалоги, панель). `PROCESS.md` §8 говорит
+прямо: «`check-docs` стоит в обязательной части не по важности, а по механике:
+отпечаток скриншотов документации считается по **всему** `src/**`, поэтому
+**любая** правка фронтенда делает его устаревшим… Цена пропуска измерена:
+скриншоты не пересняли в #230 и #234, и `dev` стоял с красным job `docs`, пока
+это не нашли при следующей задаче (#237)».
+
+ТЗ этого не отражает: §14 «Release-артефакты» не содержит пункта про
+пересъёмку скриншотов документации (`npm run build && node demo/docs/capture.mjs`,
+приёмка `npm run docs:accept -- --reviewed --from=…`), а единственное упоминание
+в AC7 — «`npm run check:docs` (если такой script доступен в текущем package)» —
+условно и, судя по `package.json`, вообще не тот скрипт: команда называется
+`node scripts/check-docs.mjs` (см. `AGENTS.md`/`PROCESS.md` §8), в `package.json`
+нет `"check:docs"`. Формулировка «если доступен» превращает обязательный гейт в
+опциональный по формальному основанию, которого нет.
+
+Отсутствие пиксельной дельты (§7 UX, «визуальная дельта равна нулю») не
+освобождает от этого гейта: `check-docs` сравнивает контент-хеш `src/**`, а не
+изображение — он красный при любой правке фронтенда независимо от того,
+поменялась ли картинка.
+
+**Что нужно.** В AC8 (обязательные гейты) добавить точную команду
+`node scripts/check-docs.mjs` (и её ожидаемый результат — предупреждение о
+устаревшем отпечатке до пересъёмки, зелёный после), в §14 — пункт про пересъёмку
+скриншотов документации и её приёмку, либо явное решение не трогать
+экранные раскладки так, чтобы `docs:capture` не требовался (нужно
+аргументировать, почему в этом случае фингерпринт не считается устаревшим —
+на сегодняшний день `check-docs.mjs` считает его по всему `src/**` без
+исключений, поэтому такого пути нет).
+
+### M3 — `data-hp-mode="device"` расходится с уже опубликованным именем `mode-devices` (Medium, в скоупе)
+
+**Файл:** `docs/specs/489-data-hp-contract.md`, §6.2 (строка 109) и §15
+(строка 390).
+
+**Проблема.** `STYLING-HOOKS.md` §5 уже публично документирует и обещает
+стабильность класса `mode-devices` на `.stage` («the stage carries `mode-view` /
+`mode-plan` / `mode-devices` / `mode-decor`, and those four names are part of the
+contract for exactly this purpose») — это существующий публичный контракт для
+того же самого понятия «режим редактора устройств». Новый атрибут
+`data-hp-mode` вводит для того же понятия другое публичное имя — `device`, в
+единственном числе, обосновывая это только внутренним `_mode === 'devices'`
+(§15: «`device` — публичное имя режима при внутреннем `_mode === "devices"`»).
+Расхождение с уже существующим `mode-devices` нигде в ТЗ не упомянуто и не
+объяснено — можно предположить, что автор не сверился с §5 `STYLING-HOOKS.md`
+при выборе имени.
+
+Для внешнего потребителя (ровно та аудитория, ради которой задача существует —
+`houseplan-e2e` и card-mod-пользователи) это два разных публичных имени для
+одного и того же состояния: `.stage.mode-devices` и
+`ha-card[data-hp-mode="device"]`. Не факт, что это плохо (два независимых
+словаря могут быть осознанным решением: старый — CSS-класс на `.stage`, новый —
+атрибут состояния на `ha-card`, для разных целей), но ТЗ обязано либо явно
+согласовать имя с уже опубликованным, либо явно зафиксировать причину
+расхождения в блоке принятых предположений — сейчас это не сделано ни там, ни
+там, то есть это непомеченная догадка, а не решение.
+
+**Что нужно.** Одно явное предложение в §15: либо переименовать в
+`data-hp-mode="devices"` вслед за `mode-devices`, либо аргументировать
+намеренное расхождение (например: `data-hp-mode` — новый, целенаправленно
+единообразный словарь с `data-kind` тулбара, а `mode-devices` — унаследованное
+имя другого поколения контракта, которое при случае стоит выровнять отдельной
+задачей).
+
+## Что проверено и корректно
+
+- Структура ТЗ содержит все обязательные разделы §7.1 (сценарий · что увидит
+  человек · проблема · скоуп/не-скоуп · контракт поведения · UX · модель данных
+  и миграция · i18n · AC1…AC8 с доказательством · план автотестов · риски ·
+  откат · release-артефакты) плюс корректный §15 «Принятые предположения».
+- Продуктовая рамка (сценарий, персона, поверхность, «что увидит человек») —
+  без нового пользовательского поведения, соответствует джобе «инфраструктура
+  тестирования» — это не отдельная строка `docs/SCOPE.md`, но полностью
+  укладывается в J6 «Keep the plan true as the home evolves» через качество
+  тестового покрытия и не создаёт нового пользовательского функционала;
+  прямого конфликта со SCOPE.md нет.
+- Открытых продуктовых вопросов владельцу нет и не должно быть: все решения в
+  ТЗ либо повторяют существующее поведение («как сейчас»), либо являются
+  чисто техническими решениями (формат JSON, словарь `data-tool`, вынос имени
+  из внутреннего `_tool`), которые по PROCESS.md §7.1 агенты решают сами.
+- §6.3 «space-add» и «space-settings», §6.5 (диалоги) и §6.6 (панель) сверены
+  построчно с исходником и совпадают с реальным условием рендера — без
+  расхождений (см. цитаты кода выше): `tabadd` уже гейтится `!this._kiosk`,
+  `.tabedit` уже завязан на `this._norm && this._canEdit`, `houseplan-panel`
+  уже имеет ровно один `menu`-обработчик, отправляющий `hass-toggle-menu`
+  (`src/houseplan-panel.ts:186`), и ровно один заголовок.
+- §6.4 (редакторы): у каждого из трёх редакторов сегодня один корневой видимый
+  контейнер (`editbar planbar` / `editbar devbar` / decor-эквивалент) и одна
+  кнопка закрытия (`.barclose`) — заявленный «один toolbar, один
+  editor-close» реалистичен без изменения разметки. `EditorSecondaryController`
+  (`src/editor-secondary.ts`) — уже единственная контекстная поверхность
+  редакторов, подходящий кандидат под «tray» без новой абстракции.
+- §6.5 (диалоги): выборочная проверка 10 из 33 call site `<hp-dialog` не
+  выявила случая, который не ложился бы в предложенный закрытый словарь из 19
+  broad kind (включая менее очевидный `gs.align_title`, укладывающийся в
+  широкий `settings`, так как это подшаг воркфлоу настроек).
+- §9 i18n: в `docs/USER-GUIDE.ru.md` сегодня нет ссылки на styling hooks,
+  поэтому условие «сохраняет ссылку, если уже есть» ничего не требует —
+  условие корректно тривиально выполнено, это не пропуск.
+- Пустое состояние (§6.3 `empty`/`create-space`): три ветки в
+  `src/houseplan-card.ts` (`fixed-floor pending`, `fixed-floor invalid`,
+  `!model.length`) и условие CTA (`this._serverStorage && this._canManageConfiguration`,
+  строка 11306) совпадают с текстом ТЗ дословно — данных за
+  расхождение нет.
+
+## Чего не проверял
+
+- Полноту словаря `data-tool` (§6.4) по всем группам-launcher — сверены только
+  явные кнопки редактора планов (`select`/`draw`/`column`/`merge`/`split`/
+  `resize`/`wallthick`→`wall-thickness`/`delroom`→`delete-room`); группы,
+  генерируемые `_editorToolbarGroups`/`_renderEditorGroupLauncher`
+  (`src/houseplan-editor-runtime.ts:5171`), не разбирались поэлементно —
+  доверился формулировке «и id существующих групповых launcher-кнопок», это
+  открытая, но не блокирующая техническая деталь для этапа кода, не ТЗ.
+- Оставшиеся 23 из 33 call site `<hp-dialog` не проверены построчно на предмет
+  однозначности broad kind — выборка не выявила проблем, но это не полное
+  покрытие; это уместно оставить код-ревью, где будет виден фактический diff с
+  назначенными kind на каждый call site.
+- Реализуемость regex/AST-сканера AC6 (риск §12 «Regex gate пропускает
+  динамику») — решение вынесено в план автотестов и риски с осознанной
+  оговоркой, это техническое решение автора кода, не предмет ТЗ-ревью.
+- Гейты (typecheck/test/build) не гонялись: на этапе ТЗ нет кода для проверки,
+  разбор — по тексту документа.
+
+## Вердикт
+
+`Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 3 → в задаче · Документ: docs/reviews/SPEC-REVIEW-489-r1.md`
+
+Все три находки — Medium, в скоупе задачи (правки самого ТЗ, тех же файлов и
+разделов, что уже в работе): без High это жёлтый вердикт, отдельный issue не
+заводится (#202). Автор правит ТЗ по трём пунктам выше и отправляет на
+повторный цикл (лимит на полном треке — 4, использован 1).
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `issue/489-data-hp-contract`, коммит `9cf10bdde769` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `aa85b7b01e6fe7a4dce55220b63b6da33f725eb8`
+  ```
+  git log --all --format='%H %T' | grep aa85b7b01e6f
+  ```
+- ТЗ `docs/specs/489-data-hp-contract.md`, блоб `b90694718d571ad77abea4fc592a5e641a0c3d19`
+  ```
+  git log --all --find-object=b90694718d571ad77abea4fc592a5e641a0c3d19 -- docs/specs/489-data-hp-contract.md
+  ```