diff --git a/docs/reviews/SPEC-REVIEW-358-r1.md b/docs/reviews/SPEC-REVIEW-358-r1.md new file mode 100644 index 00000000..15456bab --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-358-r1.md @@ -0,0 +1,156 @@ +# SPEC-REVIEW-358-r1 + +**Issue:** #358 — «Холодная вкладка + пылесос (реальная телеметрия): `_vacMapId` +бросает в `willUpdate` — карточка перестаёт обновляться» +**Этап:** spec (PROCESS.md §2.4) · **Трек:** small (§5, метка `small`, +подтверждено владельцем: комментарий S2→S3 от 2026-08-28) · **Заход:** r1 · +блокирующих циклов израсходовано 0 из 2. + +## Скоуп + +Лёгкий трек: ТЗ живёт в теле issue #358 (раздел «## ТЗ (small-трек, S3)»), +файла в `docs/specs/` нет — соответствует §5, проверено `ls docs/specs/`. + +ТЗ описывает три пункта контракта: + +- **К1** — перенос реализации `_vacMapId` с `houseplan-editor-runtime.ts` на + `houseplan-card.ts` (владелец полей — карта), runtime делегирует в host. Тот + же приём, что #357 применил к `_toggleIntent`. +- **К2** — два защитных гарда: `_decorShapeDown` (парный гард к + `_decorShapeDbl` из #337) и рендер диалога `_vacCalConfirm` за + `this._editorRuntime ?`. +- **К3** — новый холодный смок `demo/smoke_cold_view_vacuum.mjs`, закрывающий + слепое пятно демо-фикстуры (`vacuum.mower` без атрибутов позиции). + +AC1–AC4 повторяют issue-AC1…4 с указанием способа доказательства (smoke / +smoke + мутант / smoke + чтение кода). Откат — один revert, без миграций. + +## Как проверялось + +Ревью ТЗ этапа `spec` не гоняет гейты (§8 относится к код-ревью) — предмет +проверки этого этапа: выполнимость и однозначность ТЗ, и то, что автор не +выдал догадку за факт. Проверка велась чтением действующего исходника на +`61911b86` (текущий `HEAD`, тот же SHA, на котором зелёный CI Validate уже +подтверждён — #343) и сопоставлением каждого утверждения ТЗ с кодом. + +Прочитано и сверено: + +1. `docs/SCOPE.md` — сценарий issue закрывает J1 («живой обзор дома»): баг + останавливает весь цикл обновления Lit, то есть бьёт по ядру продукта + (кухня/дача с киоском), а не по периферийной функции. +2. `PROCESS.md` §2.4, §5, §7.1 — обязательные разделы лёгкого трека + (проблема · контракт · AC с доказательством · откат) присутствуют. +3. Тело issue #358 целиком + единственный комментарий (владелец, S2→S3). +4. `docs/VACUUM.md` — цепочка фолбэка `map_name → current_map → map_index → + selected_map → default` совпадает с описанием К1. +5. Код: + - `src/houseplan-card.ts:11707-11709` — сегодняшняя жёсткая заглушка + `_vacMapId`, вызываемая без гарда из `_captureRenderDeviceSnapshot` + (`:4471`, внутри `willUpdate`) и `_renderVacuums` (`:11807`) — подтверждён + ровно тот баг, который описывает issue; + - `src/houseplan-editor-runtime.ts:9979-9985` — реализация, которую К1 + предлагает перенести. Текст в ТЗ (`const sel = ve ? planHass?.states?.[ve] + ?.attributes?.selected_map : null; return vacMapIdWithFallback(tele.mapId, + sel);`) и комментарий HP-1541-01 процитированы буквально, без искажений; + - `src/vacuum.ts:252` — `vacMapIdWithFallback` действительно экспортируется + оттуда и реализует «не-nullish» правило, о котором говорит комментарий; + - `src/houseplan-editor-runtime.ts:11633-11648` — прецедент того же приёма + на `_toggleIntent` из #357: карта владеет реализацией, runtime держит + делегирующего двойника (`return this.host._toggleIntent(...)`), внутренние + вызовы runtime (`this._toggleIntent(...)` на `:11929`, `:11648`) продолжают + работать без изменений. К1 в точности повторяет эту схему — не догадка, а + проверенный шаблон; + - `src/houseplan-card.ts:7630-7632` — `_decorShapeDown` действительно жёсткая + заглушка без гарда; `:7639-7642` — `_decorShapeDbl` действительно уже + несёт `if (!this._editorRuntime) return;` (гард из #337, на который К2 + ссылается как на образец); + - `src/styles/plan.styles.ts:664` — `.decorlayer .dshape { pointer-events: + none; }` вне `mode-decor` подтверждает, что реальный указатель не достаёт + фигуру в View, а значит только синтетический `pointerdown` (как в плане + смока К3-г) может воспроизвести падение — соответствует помеченной в + issue пометке «проверено» (исполнением, не гипотеза); + - `src/houseplan-card.ts:11346-11361` — ряд редакторских диалогов + действительно рендерится по шаблону `${this._X ? this._editorRuntime ? + html\`…\` : nothing : nothing}`, а `_vacCalConfirm` (`:11351`) — единственный + из них без внешнего гарда `this._editorRuntime ?`. К2 описывает точное + целевое выражение, совпадающее с этим шаблоном; + - `src/houseplan-editor-runtime.ts:10038,10052` — `_vacCalConfirm` + присваивается truthy-значение только внутри runtime; в + `houseplan-card.ts` встречается лишь сброс в `null`. Значит утверждение + AC4 «кейс недостижим исполнением по построению» — не догадка, а факт, + подтверждённый полным grep всех присваиваний; + - `src/houseplan-card.ts:11730-11731` — `_vacApplyCalibrationProposal` + (кнопки диалога) — тоже жёсткая заглушка, как и указано в issue. +6. `demo/smoke_vacuum.mjs`, `demo/smoke_cold_view_toggle.mjs`, + `demo/serve.mjs` (`launchColdView`) — механика инъекции сущности с + телеметрией через `card.hass`/`card._serverCfg`/`card._layout` и трекинг + `page.on('request'/'pageerror')` уже существует и покрывает ровно то, что + требует план К3 (несколько кадров `hass`, отсутствие запроса + `houseplan-editor-runtime-*.js`, ноль pageerror, синтетический pointerdown). + Смок, описанный в К3, технически реализуем без новой инфраструктуры. +7. `scripts/mutation-gate.mjs` — реестр мутаций, шаблон записи + `cold-view-toggle-delegated-to-runtime` (добавлен в #357) — ревертит + `_toggleIntent` карты назад на делегацию в runtime и проверяется тем же + холодным смоком. AC3 просит зеркальную запись для `_vacMapId`; формулировка + ТЗ («вернуть делегацию `_vacMapId` в runtime») однозначно отображается на + эту механику — проверяемый, а не расплывчатый критерий. +8. `demo/smoke_static_icon.mjs`, `demo/smoke_vacuum_firstuse.mjs` — подтверждён + слепое пятно демо-фикстуры: `vacuum.mower` заведён без `vacuum_position`, + значит ни один существующий смок (включая холодные) не проходит через + ветку `telemetry ? this._vacMapId(...) : null`. + +## Находки + +Нет. High: 0, Medium: 0, Low: 0. + +Отдельно проверено на «догадку, выданную за факт» (главный риск по инструкции +ревью): каждое утверждение К1/К2/К3/AC1-4, сформулированное как факт о текущем +коде, сверено с исходником построчно (см. таблицу выше) и подтвердилось без +исключений. Технические решения, оставленные без явного обсуждения (имя файла +смока, точный id мутанта, порядок гардов), относятся к классу «агенты решают +сами» (§7.1) и не требуют пометки «принято предположительно» — они не +затрагивают видимое пользователю поведение. + +## Что проверено и корректно + +- Соответствие лёгкому треку (§5): ТЗ в теле issue, файла в `docs/specs/` нет. +- Обязательные разделы ТЗ лёгкого трека присутствуют и однозначны. +- Каждый AC называет способ доказательства (smoke / smoke+мутант / + smoke+чтение кода) и не содержит открытых вопросов. +- К1 текстуально и по логике совпадает с действующим кодом-источником + (`houseplan-editor-runtime.ts`) и с прецедентом #357. +- К2 описывает ровно два реальных гэпа (найдены построчным grep всех + присваиваний/рендеров) и предлагает решение по образцу уже принятого кода. +- К3 реализуем на существующей инфраструктуре демо-стенда без новых + примитивов. +- Продуктовая рамка (сценарий, кого касается, почему это выше по тяжести, чем + #357) присутствует в теле issue и совпадает с J1 из `docs/SCOPE.md`. +- Открытых продуктовых вопросов к владельцу нет — и по факту не должно быть: + ожидаемое поведение («карточка живёт на холодной вкладке») уже + зафиксировано как баг, а не как решение с вариантами. +- Откат описан («один revert, конфиг/контракты не меняются») и достаточен для + этого класса правки (без миграций, без нового UX-контракта). + +## Чего не проверял + +- Гейты `npx tsc --noEmit` / `npm test` / `npm run build` не гонялись: на + этапе `spec` кода ещё нет (issue не покидал `S3/S4`), гонять их не над чем. + Дешёвые гейты на `61911b86` (последнем коммите `dev`) уже зелёные — + Validate: https://github.com/Matysh/houseplan-card/actions/runs/33198366583. +- Исполнение самого смока `demo/smoke_cold_view_vacuum.mjs` — он ещё не + написан, это предмет реализации, не ТЗ. Оценивалась только его + осуществимость на существующей инфраструктуре (`launchColdView`, паттерны + инъекции сущности), а не его будущий текст. +- `scripts/smoke-links.mjs` / реестр обнаружения смоков — не проверял, + требует ли новый файл смока отдельной регистрации там; это техническая + деталь реализации, а не предмет ТЗ, и её увидит код-ревью. +- Golden/визуальные эталоны, backend-тесты (`tests_backend`), + performance-профили — вне скоупа: правка не трогает `custom_components/**`, + не меняет визуальный результат и не названа в AC как влияющая на + производительность. + +## Вердикт + +Зелёный. ТЗ выполнимо, каждый AC проверяем и снабжён способом доказательства, +фактические утверждения о коде подтверждены построчно, догадок, выданных за +решения, не найдено. Готово к разработке.