From c2cf4a6996c8df526b6dc63368922f8ce8016e3d Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Fri, 28 Aug 2026 17:50:59 +0000 Subject: [PATCH] docs: review document for #357 Issue: #357 User-Visible: no --- docs/reviews/SPEC-REVIEW-357-r1.md | 158 +++++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-357-r1.md diff --git a/docs/reviews/SPEC-REVIEW-357-r1.md b/docs/reviews/SPEC-REVIEW-357-r1.md new file mode 100644 index 00000000..0e41ba8b --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-357-r1.md @@ -0,0 +1,158 @@ +# SPEC-REVIEW-357-r1 + +- Issue: https://github.com/Matysh/houseplan-card/issues/357 +- Этап: ТЗ на ревью (PROCESS.md §2.4), лёгкий трек (`small`) +- Заход: r1 · блокирующих циклов израсходовано 0 из 2 (лимит для лёгкого трека — 2, §5) +- Материал: тело issue #357 (полевой отчёт + раздел «ТЗ (small-трек, S3)»), + один комментарий владельца (перевод S2→S3). Кода ещё нет — ветка + `issue/357-*` не создана (проверено: `git ls-remote --heads origin` не + содержит её), поэтому дешёвые/тяжёлые гейты этого раунда не касаются. + +## Скоуп + +Регрессия #337 (ленивый вынос editor-runtime): четыре тонких обёртки на +`HouseplanCard` (`_toggleIntent`, `_toggleStateText`, +`_toggleConfirmationStateText`, `_toggleConfirmationLines`) остались +заглушками `return this._editorRuntimeOrThrow()....` — на холодной вкладке +(без единого захода в редактор) `_editorRuntime === null`, и первый же клик +по toggle-устройству в View синхронно бросает исключение до вызова HA/WS. +Задача — вернуть этим четырём методам собственную реализацию на карте (то же +тело, что сейчас лежит в `houseplan-editor-runtime.ts:11629-11700`), не +трогая ничего вокруг. Соответствует SCOPE.md J3 («tap-to-toggle for safe +domains») — это починка уже закрытого продуктового job, а не новая +функциональность. + +Small-трек заявлен корректно: сложность/риск ≤3 (механический перенос +готового тела метода без изменения логики), одна поверхность (toggle-путь +View-карточки), нет миграции конфига, нет нового UX-контракта (восстанавливает +уже описанное в USER-GUIDE поведение тапа), нет влияния на перф/touch +(данные ниже подтверждают). Ни один критерий §5 не нарушен — метка `small` +и трек выбраны верно. + +## Как проверялось + +Ревью ТЗ на этом этапе — не гейты, а проверка того, что диагноз и план не +являются недоказанной догадкой. Прочитал код на `dev` (SHA `399df907`, +Validate зелёный) построчно вдоль каждого утверждения ТЗ: + +1. **Диагноз воспроизведён по коду, не только заявлен.** + `src/houseplan-card.ts:12769-12773` — `_toggleIntent` карты сейчас + действительно `return this._editorRuntimeOrThrow()._toggleIntent(...)`; + аналогично `_toggleStateText` (:12782), `_toggleConfirmationStateText` + (:12787), `_toggleConfirmationLines` (:12792). `_editorRuntimeOrThrow` + (:871-874) бросает `Error('Houseplan editor runtime is not loaded')`, + когда `_editorRuntime` ещё `null`. `_clickDevice` (:5188, :5221) вызывает + `_toggleIntent`/`_toggleConfirmationLines` синхронно в обработчике клика + без try/catch вокруг них — исключение действительно долетает до + пользователя как мёртвый клик. Диагноз подтверждён чтением, а не принят + на слово. +2. **Утверждение «модуль и поля уже eager» проверено, не угадано.** + `resolveToggleIntent`, `formatToggleConfirmation`, `formatToggleIntent` и + компания импортированы в `houseplan-card.ts` статическим `import { ... } + from './device-toggle'` (строки 131-137) — не динамическим, значит уже в + initial-графе. `_planHass` (:4368), `_fullRegistryHass` (:4528), + `_virtualLights` (:959) — существующие поля/геттеры карты. Значит перенос + тела метода действительно не тянет новых зависимостей — заявление «ноль + байт к initial-бюджету» технически обосновано, а не голословно. +3. **Архитектурный паттерн делегирования host↔runtime не изобретён для этой + задачи.** `interface HouseplanEditorHostPort` (:805) уже существует, и в + `houseplan-editor-runtime.ts` уже 35 мест вида `return this.host._xxx(...)` + (например :1500, :1916, :2018) — предложенная в К1 схема «runtime-методы + делегируют в host» — не новый приём, а применение существующего. +4. **Все точки использования четырёх методов в View-пути перечислены полно.** + `_clickDevice` использует ровно `_toggleIntent` (:5188, :5231), + `_toggleConfirmationLines` (:5221) → внутри неё `_toggleConfirmationStateText` + → внутри неё `_toggleStateText`; прямой toggle без подтверждения — + `execute(initial)` (:5241) той же цепочкой. Других методов View-путь не + использует — К1 не занижает и не завышает объём переноса. + `_toggleIntentForDialog`/`_toggleHintLines` (диалог маркера, редакторская + поверхность) сознательно остаются заглушками на runtime — это не забытый + случай, а верно исключённая часть, потому что диалог редактора не может + быть открыт без загруженного runtime. +5. **`_tapConfirm` не требует редактора.** Диалог подтверждения — часть + собственного рендера карты (`houseplan-card.ts:11401-11415`, обычный + `` в шаблоне View), а не редакторский диалог — AC2 + (tap_confirm в холодной вкладке) технически достижим без загрузки + runtime. +6. **Инфраструктура для AC1/AC2/AC4 существует, ссылки не придуманы.** + `launchColdView` (`demo/serve.mjs:99`) реально существует и уже + используется для проверки «издатель runtime не запрошен по сети» в + `demo/smoke_lazy_editor_chunk.mjs:29-35` (тот же паттерн: слушать + `page.on('request')`, сверять с именем файла из + `dist/houseplan-assets.json`) — AC1(б)/AC2 не изобретают недоказанный + способ проверки. `scripts/smoke-links.mjs` — существующий реестр (AC4 + ссылается на реальный файл, не гипотетический). Файлы `smoke_controls.mjs`, + `smoke_card_controls.mjs`, `smoke_virtual_light_toggle.mjs`, + `smoke_linked_virtual_light.mjs` (AC3) существуют на диске; `demo/smoke_ + cold_view_toggle.mjs` (К2) — новый, ещё не создан, что ожидаемо на этапе + спецификации. +7. **Продуктовая рамка.** `docs/SCOPE.md` J3 и «lock invariant» абзац: логика + `resolveToggleIntent` не меняется (переносится тело как есть), поэтому + инвариант «замок не переключается тапом» не затрагивается переносом. + `docs/USER-GUIDE.ru.md` (раздел про Toggle/tap_confirm) не противоречит — + починка восстанавливает уже описанное поведение, нового UX-контракта нет. + +## Находки + +Не найдено ни одной High- или Medium-находки. Диагноз проверен чтением кода +и совпадает с фактическим состоянием `dev`; план переноса не оставляет +непроверенных мест (все вызовы четырёх методов в View-пути перечислены и +подтверждены); используемая тестовая инфраструктура (`launchColdView`, +паттерн проверки сетевых запросов, `smoke-links.mjs`) реальна, а не +гипотетична. Ни одного утверждения о поведении, которого нет в коде и не +помечено как предположение, не обнаружено — диагноз и план различимы как +факт, подтверждённый строкой кода, а не как догадка, выданная за решение. + +Low, снятые без правки (записываю здесь по правилу §2.4): + +- **AC1/AC2 формально ссылаются на подпункты К2**, а не формулируют + собственный проверяемый текст — при беглом чтении можно спутать. Но текст + К2, на который они ссылаются, однозначен и достаточен (а/б/в перечислены + явно), поэтому это стилистическое, не содержательное замечание — не + требует возврата. +- ТЗ не проговаривает явно, что после переноса `HouseplanEditorHostPort` + должен получить новые члены типов для четырёх методов (упомянуто одной + фразой вскользь: «порт... пополняется этими членами»). Это техническая + деталь реализации, которую §7.1 явно отдаёт автору («решение записывается… + либо согласовывается между собой») — не продуктовый вопрос и не повод для + возврата. + +## Что проверено и корректно + +- Диагноз регрессии — воспроизведён чтением кода построчно (не принят на + слово автора). +- Технические предпосылки «перенос бесплатен» (модуль и поля eager) — + подтверждены чтением импортов и полей. +- Полнота охвата: все вызовы четырёх переносимых методов в View-пути + найдены и совпадают с объёмом К1 — ничего не забыто, ничего лишнего не + включено. +- Архитектурный приём (host-порт, делегирование) — не новый для проекта. +- Ссылки на тестовую инфраструктуру (`launchColdView`, + `smoke_lazy_editor_chunk.mjs`, `smoke-links.mjs`, четыре существующих + смока AC3) — все существуют и делают то, что им приписано. +- Соответствие SCOPE.md (J3, lock invariant) и отсутствие противоречия с + USER-GUIDE.ru.md — рамка выдержана. +- Small-трек: все пять критериев §5 выполнены одновременно, нарушенных нет. +- Откат («один revert, публичные контракты и конфиг не меняются») — + реалистичен для механического переноса тела метода. + +## Чего не проверял + +- Гейты `typecheck`/`test`/`build`/golden/смоки — не прогонял: кода ещё нет, + ветка `issue/357-*` не существует. Это гейты код-ревью (§2.7), не + ревью ТЗ. +- Не оценивал производительность фактическим замером (bundle:budget) — + на этом этапе оцениваю только правдоподобность заявления «ноль байт к + initial-графу» по факту уже-eager зависимостей; фактическую цифру + проверит код-ревью после того, как появится диф. +- Не проверял качество будущего `demo/smoke_cold_view_toggle.mjs` — файла + ещё нет, это предмет код-ревью. + +## Вердикт + +Зелёный. AC1…AC5 однозначны, у каждого назван способ доказательства +(browser-smoke, конкретные файлы), диагноз подтверждён чтением кода, а не +принят на веру, план переноса покрывает весь фактический объём +использования (не больше и не меньше). Открытых продуктовых вопросов нет — +задача мала и решать в ней действительно нечего сверх уже описанного в +USER-GUIDE поведения. Готово к разработке.