14 KiB
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 зелёный) построчно вдоль каждого утверждения ТЗ:
- Диагноз воспроизведён по коду, не только заявлен.
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 вокруг них — исключение действительно долетает до пользователя как мёртвый клик. Диагноз подтверждён чтением, а не принят на слово. - Утверждение «модуль и поля уже eager» проверено, не угадано.
resolveToggleIntent,formatToggleConfirmation,formatToggleIntentи компания импортированы вhouseplan-card.tsстатическимimport { ... } from './device-toggle'(строки 131-137) — не динамическим, значит уже в initial-графе._planHass(:4368),_fullRegistryHass(:4528),_virtualLights(:959) — существующие поля/геттеры карты. Значит перенос тела метода действительно не тянет новых зависимостей — заявление «ноль байт к initial-бюджету» технически обосновано, а не голословно. - Архитектурный паттерн делегирования host↔runtime не изобретён для этой
задачи.
interface HouseplanEditorHostPort(:805) уже существует, и вhouseplan-editor-runtime.tsуже 35 мест видаreturn this.host._xxx(...)(например :1500, :1916, :2018) — предложенная в К1 схема «runtime-методы делегируют в host» — не новый приём, а применение существующего. - Все точки использования четырёх методов в View-пути перечислены полно.
_clickDeviceиспользует ровно_toggleIntent(:5188, :5231),_toggleConfirmationLines(:5221) → внутри неё_toggleConfirmationStateText→ внутри неё_toggleStateText; прямой toggle без подтверждения —execute(initial)(:5241) той же цепочкой. Других методов View-путь не использует — К1 не занижает и не завышает объём переноса._toggleIntentForDialog/_toggleHintLines(диалог маркера, редакторская поверхность) сознательно остаются заглушками на runtime — это не забытый случай, а верно исключённая часть, потому что диалог редактора не может быть открыт без загруженного runtime. _tapConfirmне требует редактора. Диалог подтверждения — часть собственного рендера карты (houseplan-card.ts:11401-11415, обычный<hp-dialog>в шаблоне View), а не редакторский диалог — AC2 (tap_confirm в холодной вкладке) технически достижим без загрузки runtime.- Инфраструктура для 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) — новый, ещё не создан, что ожидаемо на этапе спецификации. - Продуктовая рамка.
docs/SCOPE.mdJ3 и «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 поведения. Готово к разработке.