docs: review document for #212

Issue: #212
User-Visible: no

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

@@ -0,0 +1,182 @@
+# SPEC-REVIEW-212-r1
+
+- **Issue:** [#212](https://github.com/Matysh/houseplan-card/issues/212) — «Правки по новым иконкам»
+- **ТЗ:** `docs/specs/212-device-icons-polish.md` (обычный трек, не `small`)
+- **Этап:** ревью ТЗ (PROCESS.md §2.4)
+- **Вердикт:** зелёный
+- **Цикл:** r1/4
+- **High:** 0 · **Medium (в скоупе):** 0 · **Medium (вне скоупа):** 0 · **Low:** 1 (снят с записью)
+
+## Скоуп ревью
+
+Issue #212 — консолидация трёх открытых задач (#22 отклик на нажатие, #154
+sticky touch-hover, #181 неверный адрес документации) плюс собственные пункты
+владельца: уменьшение маркера устройства на 10% и исправление формы капсулы
+`display: value`. ТЗ живёт файлом `docs/specs/212-device-icons-polish.md`
+(трек «обычный», не `small`) — формат файла корректен для этого трека.
+
+Проверялось: тело и комментарии #212, тела #22/#154/#181 (полностью, без
+усечения), #179 и #211 (нормативный дизайн-пакет и его принятое исправление),
+`docs/SCOPE.md`, `docs/TOUCH-SUPPORT.md`, `docs/UX-MODES.md`,
+`docs/CONFIG-COMPATIBILITY.md`, сам текст ТЗ целиком (§1–§22) и — построчно —
+код на `dev`, который ТЗ цитирует как «подтверждённую причину» в §3 и как
+основание архитектурных решений в §16.
+
+## Как проверялось
+
+Ревью на этапе `spec` не запускает гейты сборки/тестов — предмет проверки
+это ambiguity, доказуемость AC и отсутствие выданных за факт догадок. Вместо
+прогона тестов я построчно сверил каждое фактическое утверждение ТЗ о текущем
+коде с самим кодом на `dev` (`ec6f77b`), поскольку §3 ТЗ прямо заявляет
+«аудит текущего dev подтвердил четыре независимых дефекта» — такое заявление
+обязано быть проверяемым, а не риторическим.
+
+Сверено чтением (`grep`/`sed` по `src/styles.ts`, `src/houseplan-card.ts`,
+`src/space-card.ts`, `src/hp-dialog.ts`, `src/hp-help.ts`,
+`src/hp-color-opacity.ts`, `src/vacuum.ts`):
+
+| Утверждение ТЗ | Где | Результат сверки |
+|---|---|---|
+| §3.1: shell = `1.26875 × core`, нет отдельного коэффициента 0.9 | `styles.ts:1877` | подтверждено; `grep` на `* 0.9` / `--global-icon-factor` — ноль совпадений |
+| §3.2: `.device-core` держит `border-radius: 50%` глобально, `valonly` растит только ширину | `styles.ts:1812–1816, 1991–2001` | подтверждено — эллипс на широком value неизбежен |
+| §3.2: shell использует постоянный радиус `device-shell-size/2` | `styles.ts:1939, 1955` | подтверждено |
+| §3.3: `_clickDevice()` не создаёт transient feedback | `houseplan-card.ts` (нет отдельного feedback-owner рядом с dispatch) | подтверждено отсутствием |
+| §3.4: `_touchSeen` — статический (глобальный) флаг, `_notePointer()` закрывает только `_tip` | `houseplan-card.ts:5765,5776–5779` | подтверждено |
+| §3.4: `_hoverRoom` пишется из `mouseenter`/`mouseleave` | `houseplan-card.ts:15837–15864` | подтверждено |
+| §3.5/#181: hover-контракт View описан в `UX-MODES.md`, не в `CANVAS.md` | `UX-MODES.md` «View — display and device interaction only» | подтверждено дословно |
+| §7.3: Double-value pill уже имеет константу `0.39375 × core` | `styles.ts:2248` | подтверждено |
+| §7.2: vacuum puck зависит от `--dev-size` | `styles.ts:2556` (`--puck-size: calc(var(--dev-size)…)`) | подтверждено |
+| §16: перечисленные файлы (`device-face.ts`, `space-render.ts`, `hp-device-preview.ts`, `hp-dialog.ts`, `hp-help.ts`, `hp-color-opacity.ts`, `space-card.ts`) существуют; `pointer-modality.ts` — новый | `ls src/` | подтверждено |
+| §13: единственный `:hover` в `space-card.ts` — `.hp-static-btn:hover` | `space-card.ts:903` | подтверждено |
+| §13: по одному `:hover` в `hp-dialog.ts`/`hp-help.ts`/`hp-color-opacity.ts` (`.close`, `.trigger`) | построчный `grep` | подтверждено |
+| §19.1: названные smoke-файлы существуют, «device action feedback smoke» — новый, не переиспользование `smoke_feedback_v2.mjs` | `ls demo/`, чтение `smoke_feedback_v2.mjs` | подтверждено; `smoke_feedback_v2.mjs` вообще не про device-маркеры (комнатная gear-кнопка) — ТЗ верно не сослалось на него |
+| §4: версия пакета «1.1.1» | `docs/specs/179-…md`, `docs/specs/211-…md`, `docs/TESTING.md:342` | подтверждено предыдущими принятыми ТЗ, не изобретено |
+| 44×44 CSS px как уже существующий пол, а не новое требование | `styles.ts:1939` (`max(44px, var(--device-shell-size))`) | подтверждено |
+
+Все проверенные фактические утверждения ТЗ о текущем состоянии кода и
+документации оказались точными. Раздел §3 «Проблема и подтверждённая причина»
+не содержит ни одной догадки, выданной за факт, — каждый пункт воспроизводится
+в реальном коде по указанным строкам.
+
+Дополнительно сверены структурные требования PROCESS.md §7.1: сценарий,
+«что человек увидит», проблема, скоуп/не-скоуп, поведенческий контракт, UX,
+данные/миграция/i18n, AC1…AC16 с указанием способа доказательства, план
+тестов, риски, откат, release-артефакты — все разделы присутствуют, ни один
+не сведён к формальной заглушке. Блок §22 «принятые технические
+предположения» оформлен явно и содержит только технические (не продуктовые)
+решения — ни один из семи пунктов не подменяет продуктовое решение техническим
+языком.
+
+## Находки
+
+### Low-1 — межевой случай «plan-wide setting» из #22 закрыт молча, без явной пометки
+
+- **Файл:** `docs/specs/212-device-icons-polish.md`, §6 «Не входит в задачу»
+- **Что происходит:** оригинальный текст #22 (полностью поглощаемого этим ТЗ)
+  перечисляет ожидаемое поведение как «a subtle scale dip or flash,
+  **plan-wide setting**, only on devices that actually act, reduced-motion
+  fallback». Формулировка «plan-wide setting» синтаксически допускает два
+  прочтения: (a) эффект применяется единообразно по всему плану, без
+  per-marker переопределения — то, что фактически описывает ТЗ в §10; или
+  (b) запрос на новый пользовательский переключатель уровня плана
+  (вкл/выкл или интенсивность). ТЗ выбирает (a) молча и одновременно явно
+  исключает (b) в §6 («новый пользовательский toggle интенсивности/
+  длительности feedback»), не фиксируя, что здесь вообще была развилка.
+- **Почему не Medium.** Это ровно тот тип вопроса, который по PROCESS.md
+  §7.1 адресуется владельцу («какой объём видимых изменений входит в этот
+  issue»), но: (1) §4 ТЗ прямо и обоснованно ставит текст #212 выше текста
+  #22 по приоритету, а сам #212 (написанный позже, тем же владельцем)
+  перечисляет пять конкретных пунктов и ни словом не упоминает настройку;
+  (2) §15 ТЗ уже держит открытым предохранитель — если реализация всё же
+  потребует новую строку/переключатель, это автоматически трактуется как
+  расширение скоупа с отдельным решением владельца, то есть риск «тихо
+  потерять требование навсегда» отсутствует.
+- **Решение ревьюера:** снимается с записью. Автору стоит одной строкой в
+  §22 зафиксировать, что «plan-wide setting» прочитано как «единая политика
+  на весь план, не per-marker», а не как новый UI-переключатель — это
+  ничего не меняет по существу, но убирает тихое разночтение источника,
+  которое иначе выглядит как то самое «оставили в тексте, а не решили».
+
+Других находок — ни High, ни Medium, ни дополнительных Low — не выявлено.
+
+## Что проверено и корректно
+
+- **Формат трека.** Issue не помечен `small`/`trivial` (сложность и риск 8/10
+  по аналитике владельца) — файл в `docs/specs/` обязателен и создан
+  корректно, `small`-путь неприменим.
+- **Продуктовые разделы.** §1 «Сценарий» называет персону из `docs/SCOPE.md`
+  (домочадец на телефоне/панели/компьютере, админ в редакторе устройств) и
+  момент («нажимает на устройство»); §2 «Что человек увидит» — одной парой
+  фраз, без терминов реализации (не упоминает `--dev-size`, resolver,
+  pointermodality и т.п.).
+- **Обоснование скоупа по SCOPE.md.** Задача закрывает J1/J2/J7 (спойлер
+  плашки для длинного значения читается хуже — J1; отклик на нажатие — J3;
+  честный touch-контракт — обязательное требование `TOUCH-SUPPORT.md`, а не
+  отдельная строка SCOPE, но сам документ называет touch-only дефект View
+  продуктовым багом). Ничего из «Out of scope» SCOPE.md не задето: цвета,
+  lock-инвариант, glyph viewport, LQI threshold — везде явно исключены в §6.
+- **Lock-инвариант не тронут.** §6 явно исключает изменение lock security;
+  §9 явно исключает feedback для «secure/no-target/unavailable/missing/
+  unsupported no-op»; confirmation-путь (§9) требует повторной валидации
+  target при подтверждении — согласуется с параграфом «The lock invariant»
+  `docs/SCOPE.md` (единственная санкционированная поверхность — кнопка
+  Unlock/Lock внутри открытой карточки, а не тап по плану).
+- **Правильный канонический адрес документации.** §19.4 обновляет
+  `docs/UX-MODES.md` (не `docs/CANVAS.md`) для hover/pressed-владения —
+  именно то, что требует #181, с явной фразой «`docs/CANVAS.md` не
+  обновляется ради hover ownership».
+- **AC1…AC16.** Каждый критерий сформулирован как проверяемое числовое или
+  структурное утверждение (допуски `±0.005`, `±20 ms`, `≤0.5 CSS px`,
+  `44×44 CSS px`) и несёт способ доказательства (`unit`/`smoke`/`golden`/
+  `code review`/`performance profile`), как того требует DoR (PROCESS.md
+  §2.5). Ни один AC не сформулирован через реализацию так, что критерий
+  нельзя было бы фальсифицировать независимо от кода.
+- **Данные/миграция/i18n.** §15 корректно фиксирует отсутствие новых
+  config/localStorage/backend-полей и отсутствие миграции; совместимость с
+  `docs/CONFIG-COMPATIBILITY.md` не требует новой записи, так как round-trip
+  не меняется — проверено чтением раздела «Status meanings» в этом документе.
+- **Touch-контракт.** Трейлер `Touch editor: best effort / intentionally
+  degraded` относится к редакторам (Device/Plan/Background остаются
+  desktop-first, §14), тогда как View/kiosk-touch — обязательная,
+  release-blocking часть скоупа (AC8–AC12) в полном соответствии с
+  `docs/TOUCH-SUPPORT.md` («A touch-only failure in View is a product
+  defect»). Формат трейлера соответствует одному из трёх канонических
+  значений документа.
+- **Откат.** §20 указывает на одиночный revert реализации без миграции
+  данных и без storage rollback — согласуется с «Migration не нужна» (§15).
+- **Никаких искусственно вычисленных «Fresh»-цифр.** Числа 0.90/0.95/200 ms
+  трассируются либо к телу #212 (5%, 0.2s), либо к уже принятому ТЗ #22
+  (перенос контракта eligibility/confirmation/reduced-motion, §4), а не
+  придуманы автором ТЗ.
+- **Единственный открытый вопрос не требуется.** Явных продуктовых развилок,
+  которые нельзя было бы разрешить чтением #212/#22/#154/#181 и уже принятых
+  #179/#211, не найдено — комментарий аналитики владельца прямо закрывает
+  этот путь («Продуктовых вопросов не осталось»), а единственная найденная
+  двусмысленность (Low-1) не поднимается до уровня, требующего его участия.
+
+## Чего не проверял
+
+- Не запускал `npx tsc --noEmit`, `npm test`, `npm run build` — на этапе
+  `spec` кода ещё нет, эти гейты относятся к код-ревью (PROCESS.md §2.7) и
+  реализации (§2.6).
+- Не запускал browser-смоки/golden — они тоже принадлежат циклу реализации
+  и код-ревью, а не ревью ТЗ.
+- Не проверял `custom_components/**/*.py` — ТЗ прямо заявляет отсутствие
+  изменений бэкенда и новых config-полей (§15), backend-путей в
+  «Ожидаемые файлы» (§16) нет.
+- Не оценивал реализуемость 200 ms/0.95 анимации в конкретном браузерном
+  движке (WAAPI vs CSS transition) — это явно помечено в §22 как «принято
+  предположительно, ревьюер вправе изменить свободно» и будет предметом
+  код-ревью, а не ревью ТЗ.
+
+## Итог
+
+`docs/specs/212-device-icons-polish.md` — самодостаточное, добросовестно
+проверяемое ТЗ: каждое фактическое утверждение о текущем коде подтверждено
+построчной сверкой, каждый AC проверяем и несёт способ доказательства, скоуп и
+не-скоуп аккуратно очерчены и не задевают lock-инвариант или другие
+ограничители `docs/SCOPE.md`, канонический адрес документации (#181)
+исправлен верно. Единственная находка — Low, снимается с запиской автору, не
+блокирует переход в «Готово к разработке».
+
+**Вердикт: зелёный · цикл r1/4 · High: 0 · Medium: 0 в задаче**