docs: spec review document for #152

Issue: #152
User-Visible: no

docs/reviews/SPEC-REVIEW-152-r1.md

@@ -0,0 +1,285 @@
+# Ревью ТЗ — issue #152, цикл r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/152
+- **ТЗ:** `docs/specs/152-room-click-fit.md`, ветка `issue/152-room-click-fit`,
+  коммит `7f8751b4dce84880edaab95600be7f028b9fab5f`
+- **Этап:** spec (PROCESS.md §2.4)
+- **Вердикт:** красный · цикл r1/4 · High: 2 · Medium: 2
+
+## Скоуп ревью
+
+Прочитаны в указанном порядке: `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (§1,
+§2, §3, §4, §7, §8, §12), тело issue #152 и все три комментария (уточнение и
+самоотзыв про double-tap на фоне, актуализация аналитики 2026-08-15, хендофф
+автора ТЗ), `docs/USER-GUIDE.ru.md` (терминология «Карточка комнаты», клик по
+комнате в Resize, пространство/комната), `docs/UX-MODES.md`, `docs/TOUCH-SUPPORT.md`,
+`docs/CANVAS.md` (§4–8: content frame, ZOOM_MIN/MAX, viewport/editor handoff),
+`docs/CONFIG-COMPATIBILITY.md`. Issue не помечен `small`/`trivial` (аналитика:
+сложность 4/10, риск 5/10, «обычный трек»), поэтому ТЗ корректно вынесено в
+отдельный файл `docs/specs/152-room-click-fit.md`, а не в тело issue. Формат
+ревью — полный документ.
+
+## Как проверялось
+
+ТЗ активно ссылается на «существующее» поведение как на данность (существующий
+resolver, существующий double-tap, существующая доступная подпись, существующие
+ZOOM_MIN/MAX). Каждое такое утверждение сверено с реальным кодом, а не принято
+на веру:
+
+- `src/houseplan-card.ts:4792-4793`, `src/space-geometry.ts:203` — `ZOOM_MIN`/
+  `ZOOM_MAX`/`MIN_ZOOM` существуют, значения `8` и `1/3` подтверждены.
+- `src/houseplan-card.ts:5023-5035` (`_resetZoom`) и `:5264-5300`
+  (`_stagePointerUp`) — double-tap-to-reset реализован, но целиком внутри
+  `if (this._kiosk) { … }`; вне этого блока на stage нет ни `@dblclick`, ни
+  иной обработки двойного тапа (проверено также по полному списку обработчиков
+  стейджа, `:14471-14480`).
+- `src/houseplan-card.ts:14619-14646` (room hover через нативные
+  `@mouseenter`/`@mouseleave` на каждой SVG-форме комнаты) против
+  `:6528, :9936, :11045, :11096` (`[...rooms].reverse().find(r => this._pointInRoom(raw, r))`,
+  используется инструментами Resize/Delete/Merge/Split) — это два разных
+  механизма, не один вызываемый resolver.
+- `src/houseplan-card.ts:16001-16065` (`_renderRoomLabel`) — `.roomlabel` не
+  имеет `tabindex`/`role`/`aria-label`/`@keydown`; единственные `tabindex` в
+  файле — `tabindex="-1"` на editor-тулбарах (`:9241, :16963, :17044`),
+  подтверждает `docs/SCOPE.md` («no ARIA labelling of the plan»).
+- `gh issue view 82` — открыт, всё ещё `S3-spec`: гипотеза «до реализации #82»
+  в ТЗ корректна, `ViewportAnimator` в `src/` действительно не найден (grep).
+- `gh api repos/Matysh/houseplan-card/issues/28` — `state: closed,
+  state_reason: not_planned`, закрыт 2026-08-14, то есть до публикации этого ТЗ.
+- `docs/specs/146-four-phase-sun-background.md` §15 и
+  `docs/specs/138-adjacent-room-autoclose.md` §13 — эталон формата AC с
+  инлайн-аннотацией способа доказательства, использован как мера сравнения.
+
+## Находки
+
+### High-1 — ни один AC не указывает способ доказательства
+
+**Где:** `docs/specs/152-room-click-fit.md` §11 (строки 172–187).
+
+Раздел перечисляет 14 пронумерованных критериев без пометки `unit`/`backend`/
+`smoke`/`golden`/«ревью кода» у каждого — в отличие от §12 «План тестирования»,
+который группирует проверки по категории, но не привязывает их к номерам AC.
+Например, AC5 («Flat и Isometric проверяются по итоговой экранной геометрии»)
+может быть закрыт unit-тестом проекции, browser smoke со скриншотом или
+golden-эталоном — все три упомянуты в §12, и по тексту ТЗ невозможно понять,
+какой из них действительно обязателен для приёмки, а какой — просто
+сопутствующий.
+
+PROCESS.md §2.5 (DoR) требует буквально: «AC1…ACn — пронумерованные проверяемые
+критерии приёмки; у каждого указано, чем он доказывается: unit / backend /
+smoke / golden / «ревью кода»». Соседние ТЗ этого же репозитория соблюдают это
+дословно (`docs/specs/146-four-phase-sun-background.md:417-465`: «**AC1
+(`unit`; разработчик):**» и так для всех 16 пунктов; аналогично
+`docs/specs/138-adjacent-room-autoclose.md:276-323`). ТЗ #152 — единственное
+из свежей серии, где это правило не выполнено ни разу.
+
+**Почему High, а не Low:** это не стилистическая придирка, а прямое условие
+готовности к разработке (§2.5) и то, что мне прямо поручено проверить в этом
+ревью («указание способа доказательства» для каждого AC). Без этой аннотации
+issue не может законно перейти в `S5-ready`.
+
+**Что нужно поправить:** проставить у каждого из AC1–AC14 способ доказательства
+и исполнителя, как в §15 `docs/specs/146-...md`.
+
+### High-2 — потерян отдельный проверяемый AC про синхронность hit targets во время перехода
+
+**Где:** `docs/specs/152-room-click-fit.md` §8 (строки 130–151) и §11 (172–187);
+ср. issue #152, пункт критериев приёмки «Viewport и все SVG/HTML hit targets
+остаются синхронны во время перехода» (14-й из 15 чекбоксов тела issue).
+
+Это требование в финальном ТЗ не стало отдельным AC — оно осталось прозой
+внутри §8: «Viewport, SVG и HTML overlays получают один и тот же camera state
+на каждом кадре; промежуточное рассогласование hit targets недопустимо.» Ни
+одного номера в §11, ни отдельного пункта browser smoke в §12 (там есть
+«Flat/Isometric screenshot bounds; ни один видимый слой или hit target не
+отстаёт» — но это один общий пункт про финальный кадр, а не про сам переход
+кадр за кадром) под это не выделено.
+
+**Сценарий, которому это открывает дорогу:** как только заработает `ViewportAnimator`
+(#82) и переход из клика по комнате в её fit станет анимированным tween'ом,
+визуальный viewBox обновляется на каждом кадре, а SVG/HTML hit-элементы
+(устройства, проёмы, room label) должны обновляться синхронно. Если реализация
+пересчитывает hit-геометрию на кадр позже видимой, пользователь тапает туда,
+где устройство видно **сейчас**, и промахивается — либо получает клик по
+случайно оказавшемуся там другому объекту. Именно этот класс дефекта
+(«hit target отстаёт от кадра во время перехода») уже упомянут как прецедент в
+`AGENTS.md` (#89 — «смок, который не умел падать» стоил целого цикла ревью), и
+без отдельного AC с проверяемым доказательством ничто не гарантирует, что тест
+на это вообще напишут, а не просто положатся на прозу §8.
+
+**Почему High:** это не новая функциональность сверх скоупа — она уже была
+явно сформулирована владельцем в теле issue как критерий приёмки и молча
+выпала при переносе в ТЗ. Восстановление контракта — не расширение задачи, а
+исправление ТЗ до его полноты.
+
+**Что нужно поправить:** вернуть требование как отдельный пронумерованный AC с
+указанным способом доказательства (browser smoke с промежуточными кадрами
+tween, не только конечным) и явной строкой в §12.
+
+### Medium-1 — accessibility-раздел выдаёт «существующую accessible-подпись» за факт, хотя такой доступности сегодня нет
+
+**Где:** `docs/specs/152-room-click-fit.md` §9 (строки 153–163), формулировка
+«Существующая доступная подпись либо room-card target получает action Enter и
+Space».
+
+`.roomlabel` (`src/houseplan-card.ts:16043-16056`, `_renderRoomLabel`) сегодня
+не имеет ни `tabindex`, ни `role`, ни `aria-label`, ни `@keydown` — только
+pointer-обработчики для drag в Plan-редакторе и клик на дочерней `ha-icon`
+(переход в HA area). Единственные `tabindex` во всём файле — три
+`tabindex="-1"` на editor-тулбарах (`:9241, :16963, :17044`), что снимает tab-
+stop, а не добавляет. Это согласуется с прямой формулировкой `docs/SCOPE.md`
+(«Accessibility: … no ARIA labelling of the plan») — сегодня в плане нет ни
+одного клавиатурно-доступного или ARIA-размеченного элемента вообще.
+
+Формулировка §9 читается как «добавить действие к уже доступному элементу», а
+по факту задача вводит **первый** в продукте клавиатурно-фокусируемый,
+ARIA-размеченный элемент плана, причём по одному на каждую комнату — со всеми
+последствиями (порядок табуляции по потенциально многим комнатам,
+обнаруживаемость, стили `:focus-visible`, отсутствие конфликта с уже занятыми
+`tabindex="-1"` зонами тулбаров). Раздел §9 сам по себе технически осторожен
+(явно запрещает невидимую полноэкранную сетку tab-stop), но опирающееся на него
+предложение 1 и допущение из AC12 создают впечатление меньшего объёма работы,
+чем есть на самом деле, и не фиксируют явно, что это новая a11y-поверхность, а
+не расширение существующей.
+
+**Почему Medium:** это не блокирует ни один AC — предложенное поведение
+реализуемо, задача была явно запрошена в теле issue (пункт про Enter/Space уже
+был в исходных критериях). Но формулировка занижает объём и скрывает
+продуктовый факт (первая ARIA-разметка плана), который стоит зафиксировать
+явно, а не как побочный эффект неточной фразы.
+
+**Заведён отдельный issue** https://github.com/Matysh/houseplan-card/issues/182.
+
+### Medium-2 — «существующий» double-click/tap fit-all на фоне на деле существует только в kiosk
+
+**Где:** `docs/specs/152-room-click-fit.md` §3 («не входит… изменение
+существующего double-click/tap fit всего плана на свободном фоне»), §7
+(«Double click/tap по свободному фону сохраняет существующий fit/reset всего
+плана без изменений»).
+
+`docs/UX-MODES.md` документирует double-tap-reset только в разделе «Kiosk mode
+(v1.41.0)»: «double tap resets zoom». В разделе «View» такого жеста нет вовсе.
+Чтение кода подтверждает документацию: логика `_lastTap`/`_resetZoom()` в
+`_stagePointerUp` (`src/houseplan-card.ts:5264-5277`) целиком находится внутри
+`if (this._kiosk) { … }`; на самом stage нет `@dblclick` (полный список
+обработчиков — `:14474-14480`). То есть на обычном (не kiosk) View — ни
+десктопный dblclick мышью, ни touch double-tap — сегодня **ничего** не делают
+на фоне; жест существует только в kiosk.
+
+ТЗ формулирует это как факт про View в целом, а не как допущение или явно
+ограниченное kiosk-наблюдение (в отличие от §16, где рядом стоящее допущение
+«до #82 переход атомарный» правильно вынесено в блок предположений). Первый
+комментарий владельца к issue («двойной клик/тап… – Поправка, это поведение
+кажется уже реализовано») показывает, что сама неоднозначность уже была на
+поверхности при заведении issue и не была закрыта явной проверкой.
+
+**Последствия:** (1) пункт browser smoke «double tap свободного фона сохраняет
+прежнее поведение» (§12) содержателен только для kiosk — для обычного View
+«прежнее поведение» это просто отсутствие эффекта, что тривиально пройдёт даже
+при ошибке в общей gesture-арбитраже; (2) для ownership-логики §6–7 (отличить
+одиночный tap от первой половины double-tap) потребуется **новая** общая
+(не только kiosk) логика распознавания double-tap — её сегодня не существует,
+а формулировка «новые независимые константы не вводятся» (§7) намекает, что
+общий тайминг уже есть и его достаточно переиспользовать, что неверно вне
+kiosk.
+
+**Почему Medium:** не блокирует ни один AC (инструкция «сохранить как есть»
+остаётся корректной независимо от истинного текущего охвата), но фактическая
+неточность способна привести к смоку, проверяющему не то окружение, и к
+недооценке объёма новой gesture-арбитражной логики.
+
+**Заведён отдельный issue** https://github.com/Matysh/houseplan-card/issues/183.
+
+## Заведённые issue по Medium-находкам
+
+- **Medium-1** → https://github.com/Matysh/houseplan-card/issues/182
+- **Medium-2** → https://github.com/Matysh/houseplan-card/issues/183
+
+## Что проверено и корректно
+
+- **Геометрический контракт (§4)** — однозначен и самосогласован: safe rect
+  `0.8W×0.8H`, формула `scale = min(0.8W/Rw, 0.8H/Rh)`, пример 2000×1000 →
+  800×800 с полями 100/100/600/600 px — арифметика проверена вручную и
+  совпадает с телом issue дословно.
+- **`ZOOM_MIN`/`ZOOM_MAX` действительно существуют** (`houseplan-card.ts:4792-4793`,
+  `space-geometry.ts:203`, значения 8 и 1/3) — ссылка на «существующие пределы
+  zoom» не выдумана.
+- **#82 корректно учтён как ещё не реализованный** — issue #82 открыт,
+  `S3-spec`; `ViewportAnimator` в `src/` не найден (grep). Условная
+  формулировка «до реализации #82 … атомарно» точна, а не догадка.
+- **Конфликт с #28 (§10)** — сам факт конфликта (обычный клик не может
+  одновременно принадлежать двум действиям) реален, а решение (primary click
+  закреплён за fit-to-room) — явное продуктовое решение, а не техническая
+  подмена; см. также Low-1 про устаревший статус #28 ниже.
+- **Модель данных и миграция** — по существу закрыто корректно: room fit не
+  меняет `room`/`space`/backend-схему, новых полей не появляется; сверено с
+  `docs/CONFIG-COMPATIBILITY.md` (нет ни одной точки пересечения с текущим
+  реестром совместимости). Разнесено по §8/§15 вместо отдельного раздела
+  (см. Low-3), но по содержанию пробелов нет.
+- **Non-scope (§3)** согласован с non-scope тела issue и не оставляет скрытого
+  расширения (открытие карточки, кнопка «назад», авто-смена пространства,
+  свободное вращение изометрии, второй animator — всё явно исключено).
+- **Блок предположений (§16)** используется по назначению: пункты про базис
+  измерения 10%, состав bounds, атомарность до #82 и передачу trigger карточки
+  в #28 — это действительно технические/уточняющие решения, а не спрятанные
+  продуктовые вопросы, которые следовало адресовать владельцу. Ревьюер
+  оспаривает формулировку пункта про «единственный resolver» (см. Low-4) в
+  рамках штатного технического спора автор/ревьюер, не как продуктовый вопрос.
+- **i18n** — конкретный текст на двух локалях дан («Вписать комнату Гостиная» /
+  «Fit room Living room»); имена ключей не выбраны, но это техническая деталь
+  именования, а не продуктовая неясность.
+
+## Low (снято с записью, не блокирует)
+
+- **Low-1.** §10 обсуждает #28 как открытую задачу, которой перед реализацией
+  «нужно изменить UX». По факту #28 закрыт `not_planned` 2026-08-14 — до
+  публикации этого ТЗ. Само продуктовое решение (primary click → fit-to-room)
+  от этого не меняется, но формулировку стоит поправить в следующей редакции,
+  чтобы не выглядела как отсылка к живой задаче.
+- **Low-2.** §1 не называет явно персону из таблицы `docs/SCOPE.md` (Home admin /
+  Household members / Guests) — сценарий описан через поверхность (View,
+  kiosk) и типичный триггер (большой/сложный план), персона угадывается, но не
+  названа. Не мешает пониманию контракта.
+- **Low-3.** В отличие от `docs/specs/146-...md`/`138-...md`, в ТЗ #152 нет
+  отдельного заголовка «Модель данных и миграция» — содержание корректно, но
+  разнесено по §8 и §15. Рекомендация на будущее — для единообразия со
+  смежными ТЗ этого репозитория.
+- **Low-4.** §16 полагается на «единственный existing room hover/hit resolver».
+  По коду это два разных механизма: hover в View — нативные
+  `@mouseenter`/`@mouseleave` на SVG-форме каждой комнаты (`:14619-14646`), а
+  инструменты Resize/Delete/Merge/Split используют собственный
+  `[...rooms].reverse().find(pointInRoom)`, продублированный в 4 местах
+  (`:6528, :9936, :11045, :11096`). Оба, вероятно, совпадают на практике
+  (одинаковый z-порядок), но структурно это не один переиспользуемый
+  resolver. Пункт уже явно оформлен как допущение в §16, поэтому не требует
+  отдельного issue — фиксирую как технический спор для реализации: имеет
+  смысл вынести один явный shared-хелпер, а не полагаться на совпадение двух
+  независимых путей.
+
+## Чего не проверял
+
+- Реализацию — её нет, это ревью ТЗ, а не кода.
+- Golden/визуальные артефакты и производительность — на этапе ТЗ не
+  требуются; §12 корректно откладывает их проверку до реализации/пре-релиза.
+- Backend/`custom_components/houseplan/**/*.py` — задача их не касается по
+  заявленному scope, отдельно построчно не сверялось, так как заявлено
+  «room fit не меняет model/backend», а `docs/CONFIG-COMPATIBILITY.md` не
+  показал точек пересечения.
+- Полное соответствие проектируемого «одного focus target на комнату»
+  практикам ARIA authoring patterns — это вопрос code review на этапе
+  реализации, не ТЗ.
+- Не выполнялись гейты (`typecheck`/`test`/`build`) — на этом этапе нет кода
+  для их запуска, только документ.
+
+## Вывод
+
+Два блокирующих дефекта: (1) ни один из 14 AC не указывает способ
+доказательства, что напрямую противоречит DoR (PROCESS.md §2.5) и практике
+соседних ТЗ этого репозитория; (2) явный критерий приёмки issue про
+синхронность SVG/HTML hit targets с viewport во время перехода потерялся при
+переносе в ТЗ и не имеет ни номера AC, ни отдельной smoke-проверки. Оба
+дефекта устраняются правкой текста ТЗ, а не пересмотром продуктового решения.
+Геометрический контракт, non-scope, обработка конфликта с #28/#82 и модель
+данных проверены по коду и корректны.
+
+Возврат в «ТЗ в работе» (`S3-spec`), цикл r1/4.