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

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).

---

Материал раунда

• Ветка: 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