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

SPEC-REVIEW-629-r1

Issue: #629 · Этап: spec (ТЗ на ревью, PROCESS.md §2.4) · Заход: r1 · блокирующих циклов израсходовано 0 из 4 Материал: тело issue #629, раздел ## ТЗ (плюс верхняя часть ## Факты/Правка/AC, которую ТЗ явно наследует и уточняет). Аналитика (S2) — комментарий https://github.com/Matysh/houseplan-card/issues/629#issuecomment-5805622166. Трек: полный (S5 «сложность ≤ 3» нарушена — гейт с разбором AST, фасад из 10 операций, перевод трёх смоков; названо явно в ТЗ и в аналитике). Проверено на дереве репозитория: adb1727c50b09a2fe37ec7e0342cfd45dfa4d491 (для сверки технических утверждений ТЗ с текущим кодом — не как материал code-review). Вердикт: зелёный.

Скоуп ревью

Задача — инструментальная (харнесс/гейт тестов), ни одна из трёх персон SCOPE.md ничего не видит; продукт получает единственный невидимый DOM-атрибут (data-hp="mode-tab") на вкладках режимов. Проверялось:

1. Обязательные разделы §7.1 присутствуют и в правильном порядке (сценарий → что человек увидит → проблема → скоуп/не-скоуп → контракт поведения → UX → модель данных и миграция → i18n → AC → план автотестов → перф/touch → риски → откат → release-артефакты) — раздел «Принято предположительно» сверх нормы, по правилу §7.1.
2. Каждый AC (AC1–AC11) — однозначен, у каждого указан способ доказательства и явно назван мутант/красный случай («чем краснеет»), кроме AC11 (документация, доказывается чтением — верно для документного AC) и AC10 (перевод смоков, доказывается прямым сравнением списков проверок до/после — по тексту AC явно написано, что «защиты нет, §2.7 не требуется», это осознанное и корректное исключение, не пропуск).
3. Продуктовая рамка (SCOPE.md): задача не создаёт нового продуктового поведения, только повышает надёжность тестов, защищающих J3/J4/J6-поверхности (редакторы). Вопросов, требующих продуктового решения владельца, в ТЗ корректно не возникло — видимых изменений нет, «что человек видит» отвечено явно: «пользователь карточки не видит ничего».
4. Раздел §15 «Принято предположительно» — проверено, что каждое допущение действительно техническое (не продуктовое) и обосновано, а не выдано за факт без пометки. Главное отступление — «фасад в харнессе, а не в продукте», вопреки формулировке в верхней (более старой) части issue «Фасад — продуктовый код»; это архитектурное решение о механизме инструмента, а не о видимом поведении, и потому по PROCESS.md §7.1 относится к зоне «агенты решают сами», не к зоне вопросов владельцу. Обоснование (три причины: приватные методы остались бы приватными; отсутствие бэкдора по флагу в публичном бандле; отсутствие новых членов карточки) — предметное, ревьюер имеет право оспорить и не оспаривает: аргументы состоятельны, а альтернатива (__HP_VERSION_OVERRIDE__-подобный продуктовый флаг) была бы избыточным риском ради задачи с нулевой пользовательской видимостью.

Как проверялось

Ревью ТЗ построено на сверке фактических технических утверждений ТЗ с текущим деревом — не потому что это код-ревью (это не он), а потому что «утверждение о поведении, которого нет ни в одном документе и которое не помечено как предположение, — замечание» (инструкция ревью), и единственный способ отличить проверенный факт от красиво написанной догадки — заглянуть в код, на который ТЗ ссылается. Расхождение здесь означало бы, что реализация в 4/5 случаев наткнётся на несуществующий хук, и задача вернётся на второй круг ещё до кода.

Проверенные утверждения (все подтвердились):

• .modetab существует (src/houseplan-card.ts:10864), data-hp="mode-tab" на нём — действительно отсутствует; data-editor-navigation и [data-hp="editor-close"] — на месте (не меняются, как и требует P1).
• Все 9 остальных селекторов фасада (F) уже существуют как контрактные хуки: data-hp="space-tab" (houseplan-card.ts:10825), data-hp="tool" data-tool=… и data-hp="toolbar" (houseplan-editor-runtime.ts, editor-secondary.ts), data-hp="room-settings" data-room=… (houseplan-editor-runtime.ts:10658), data-hp="space-settings" (houseplan-card.ts:10839), data-hp="space-add" / data-hp="create-space", data-hp="dialog-cancel", data-kind="room"|"marker"|"space" на hp-dialog (houseplan-editor-runtime.ts:12336, src/editors/marker-dialog.ts:858, src/space-copy-runtime.ts:86). Значит утверждение §3 «не хватает только хука вкладки режима» — не догадка, а проверенный факт.
• docs/data-hp-contract.json действительно уже несёт audience: ["test"] для чисто тестовых хуков (create-space и др.) — паттерн для нового mode-tab воспроизводим без нового прецедента.
• docs/STYLING-HOOKS.md §7.4 «Editors» существует и уже описывает data-hp="toolbar"/"tool"/"editor-close" — ссылка ТЗ на «STYLING-HOOKS §7.4» для новой записи корректна и не потребует создавать раздел с нуля.
• demo/srv/demo.html: CFG/LAYOUT сейчас действительно const (строки 52, 69), CFG_REV/LAYOUT_REV уже let; CONNECTION.subscribeEvents (строка 111) на неизвестном имени события возвращает ()=>{} немедленно — подтверждает утверждение §7 «подписка и раньше возвращала функцию отписки, просто пустую» дословно.
• scripts/no-new-any.mjs действительно экспортирует movedLinesByFile и MOVED_BLOCK_MIN, использует маркер any-ok с тем же форматом сообщения, который ТЗ переиспользует для private-ok (G3, G4) — «переиспользуется, не копируется» реалистично, а не пожелание.
• scripts/unused-locals-gate.mjs действительно оперирует терминами portPrivates/harnessPrivates — F3/риск «lint:unused» ссылается на существующий механизм, а не на выдуманный.
• scripts/mutation-registry.mjs: анкер hp-dialog-ignores-flex-content существует (строка 9922), новых коллизий с предложенными id (room-settings-click-does-not-open, hp-dialog-escape-does-not-close, config-updated-event-ignored, private-writes-*) нет — место вставки в §9 ТЗ выполнимо буквально.
• demo/smoke_area_relocation.mjs действительно оборачивает hass.callWS и копит calls (строка 13+) — риск §12 о журнале вызовов при добавлении setServerConfig обоснован, не гипотетичен.
• demo/helpers/ сейчас содержит ровно фикстуру ha-dialog #505 (README-ha-dialog-505.md, ha-dialog-assets.mjs, ha-dialog-fixture.mjs) — подтверждает и утверждение issue «demo/helpers — только фикстура ha-dialog», и что F4 (крестик HA — приватный shadow root) опирается на реальный существующий контекст, а не изобретён для этой задачи.
• scripts/check-inputs.mjs: BROWSER_PROTOCOL и frontend/smoke роуты существуют в описанном виде — AC5 (манифест) выполним без новой концепции.
• dist/ и custom_components/houseplan/frontend/ — оба реально существуют и оба содержат houseplan-card.js/houseplan-panel.js — команда AC9 (grep -rc __hpTest dist custom_components/houseplan/frontend) исполнима как написана.
• scripts/gate-small.mjs и .github/workflows/validate.yml действительно вызывают no-new-any.mjs в описанных местах (parallelSteps / шаг frontend).
• test/data-hp-contract.test.mjs существует — AC8 ссылается на реальный существующий валидатор, а не на будущий.
• scripts/monolith-metrics.mjs существует — ссылка в «не-скоупе» корректна.

Ни одно проверенное фактическое утверждение не разошлось с кодом. Это необычно высокая для полного трека степень технической проработки — автор явно провёл собственный разбор дерева перед написанием ТЗ, а не собрал правдоподобный текст.

Находки

Нет ни одной High- или Medium-находки. Мелочей, которые стоило бы фиксировать как Low, тоже не нашлось после проверки: формулировки однозначны, зачёты (G3), исключения (G4) и покрытие вызовов (G5) описаны с точными границами и контрпримерами прямо в тексте («c._tool = 'draw' → 'select' проходит, новая запись в новое поле — нет»), что снимает обычный для лёгких ревью риск «красиво звучит, но не проверяемо».

Единственное, что заслуживало разбора — уже разобрано в §15 «Принято предположительно» самим автором (отступление «фасад в харнессе, не в продукте» от формулировки верхней части issue) — и разбор корректен по существу (см. «Скоуп ревью», п. 4). Это не находка, а пример того, как раздел §15 должен работать: явное решение с обоснованием, которое ревьюер может оспорить и не находит оснований оспаривать.

Что проверено и корректно

• Структура ТЗ полностью соответствует §7.1 PROCESS.md, обе продуктовые секции (сценарий, что человек увидит) отвечены по существу и корректно constatируют отсутствие видимого изменения.
• Все AC1–AC9 несут мутант или отрицательную пробу («чем краснеет»); AC10/AC11 — документные/сравнительные, корректно освобождены от требования мутанта самим текстом §2.7/AC.
• Технические утверждения (см. «Как проверялось») подтверждены чтением актуального кода, а не приняты на слово.
• Скоуп/не-скоуп разграничены точно, включая явный вывод продуктовых дефектов, найденных при переводе смоков, в отдельные issue (§12, риск 1) — соответствует правилу «скоуп не расширяется» (PROCESS §2.6).
• Откат и release-артефакты покрыты корректно для задачи без пользовательской видимости (User-Visible: no везде, changelog не требуется).
• i18n, touch, перф — отвечены по существу («нет»/«не затрагивается» с кратким обоснованием), не формальной отпиской.

Чего не проверял

• Не проверял, что реализация действительно уложится в объём (гейт + фасад из 10 операций + фикстура + 3 перевода смоков + новый смок + документы) без выхода за скоуп — это предмет код-ревью, не спецификации.
• Не прогонял никакие гейты (typecheck/test/build) — это этап spec-review, материал которого есть текст ТЗ, а не диапазон коммитов; на этом этапе они не требуются и не относятся к предмету ревью.
• Не проверял глубину покрытия трёх переводимых смоков построчно (какие именно строки smoke_area_relocation/smoke_glow/smoke_grid_snap попадут под исключения) — это будет видно по факту перевода в коде, спецификация лишь обязывает к результату (AC10) и это корректно для стадии ТЗ.
• Не проверял названия i18n-ключей (их нет — задача не создаёт пользовательских строк).

Заключение

ТЗ полное, однозначное, каждый AC доказуем и способен покраснеть, продуктовая рамка выдержана (нулевая видимая поверхность корректно constatирована, а не скрыта), единственное нетривиальное архитектурное отступление явно помечено, обосновано и не требует эскалации владельцу. Материала для возврата автору нет.

Готово к разработке.

---

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

• Ветка: dev, коммит adb1727c50b0 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: c5f33ffb2d92b3b2675ee1f23f67ac61ef572365

  git log --all --format='%H %T' | grep c5f33ffb2d92
  

• Тело issue: 00fd8d5cfe7f3778ae654cefa41d7042e6f514b9a75b2db5eaa26c253a73deb9
• Вердикт конвейера: green · High 0