docs: review document for #179

Issue: #179
User-Visible: no

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

@@ -0,0 +1,271 @@
+# Ревью ТЗ — issue #179, цикл r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/179
+- **ТЗ:** `docs/specs/179-device-icons-redesign.md` (коммит `517a710`, ветка
+  `issue/179-device-icons-redesign`)
+- **Трек:** обычный (аналитика явно исключила `small`/`trivial`)
+- **Вердикт:** жёлтый · цикл r1/4 · High: 0 · Medium: 5 (все в скоупе задачи) · Low: 2
+
+## Скоуп ревью
+
+Проверялось только ТЗ как артефакт этапа `S4-spec-review`: обязательные разделы
+§7.1 PROCESS.md, однозначность и доказуемость AC1…AC14, соответствие
+`docs/SCOPE.md` (J1/J2/J7), `docs/USER-GUIDE.ru.md`, `docs/CONFIG-COMPATIBILITY.md`,
+`docs/TOUCH-SUPPORT.md`, а также фактическое соответствие ТЗ нормативному
+источнику — архиву дизайнера, приложенному к issue #179. Продуктовый код не
+существует (issue не дошёл до `S5-ready`), поэтому код не проверялся.
+
+## Как проверялось
+
+1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком.
+2. Прочитано тело issue #179 и все три комментария (`gh issue view 179 --json ...`).
+3. Прочитан файл ТЗ целиком (534 строки).
+4. **Архив дизайнера скачан и распакован** (`curl` по ссылке из тела issue,
+   `sha256sum` совпал с указанным в ТЗ `63670C73...`), чтобы проверить не только
+   внутреннюю согласованность ТЗ, но и его фактическое соответствие
+   нормативному источнику — это единственный способ отличить решение владельца
+   от догадки, выданной за факт:
+   - `SPECIFICATION.md`, `ACTIVE_ANIMATION_SPEC.md`, `DEVELOPER_HANDOFF.md`,
+     `README.md`, `PACKAGE_ANNOTATION.txt`, `manifest.json` — построчно;
+   - геометрия и цвета — вычислены из реальных SVG (`viewBox`, `<rect>`,
+     `<path>` координаты circle-shell, `fill`/`stroke`, `feColorMatrix`);
+   - тайминги анимаций — вычислены из `@keyframes`/`animation` в
+     `Animated/Light/*.svg` (текстовые документы архива тайминги короткого
+     события и alert не дают, только качественное описание).
+5. Сверено с текущей реализацией: `src/device-pulse.ts`,
+   `src/device-presentation.ts`, `src/styles.ts`, `src/types.ts`,
+   `src/houseplan-card.ts` — чтобы отличить утверждения ТЗ о «текущем
+   поведении» (раздел «До реализации») от факта, и проверить, действительно ли
+   новая раскладка цветов континуальной пульсации — новшество.
+6. Прочитаны релевантные фрагменты `docs/USER-GUIDE.ru.md` (терминология
+   состояний, приоритет статусов, пульсация, «Всегда статичный значок», LQI
+   комнаты) и `docs/CONFIG-COMPATIBILITY.md`, `docs/TOUCH-SUPPORT.md` целиком.
+7. Код не запускался, гейты не прогонялись — на этапе ревью ТЗ кода нет,
+   раздел «Гейты» PROCESS.md §8 к этому циклу неприменим.
+
+## Находки
+
+### M1 — цвет внешней тени искажён относительно нормативного источника
+
+**Файл:** `docs/specs/179-device-icons-redesign.md:122-123`
+
+ТЗ утверждает для light-shell на референсе 56 px: `0 1px 2px rgb(0 0 0 / 12%)`
+и `0 4px 8px -1.07px rgb(0 0 0 / 18%)` — чистый чёрный.
+
+Нормативный источник (`SPECIFICATION.md`, таблица §8) даёт другой цвет:
+`0 1px 2px 0 rgb(37 40 45 / 12%)` и `0 4px 8px -1.07px rgb(37 40 45 / 18%)`.
+Это подтверждается самим SVG: `feColorMatrix` в `Light/Icon Default.svg` задаёт
+`0.145098 / 0.156863 / 0.176471` — это ровно `37/255, 40/255, 45/255`, то есть
+тёплый тёмно-серый `#25282D`, а не `#000000`.
+
+По порядку приоритета самого ТЗ (§3: решения владельца → текстовые документы
+архива → SVG-примеры → текущая реализация) текстовый документ и SVG совпадают
+между собой и расходятся с ТЗ; решения владельца по этому пункту нет. Это не
+разночтение источников, а фактическая ошибка транскрибирования в самом ТЗ.
+
+**Почему это важно:** AC1 обещает golden-эталон 32/56/96 px по геометрии и
+токенам пакета. Реализация по тексту ТЗ буквально даст на глаз более резкую,
+холодную тень, чем в дизайне, и это will пройти собственный (неверный) golden,
+проверяющий не то, что задумано.
+
+**Требуется:** заменить `rgb(0 0 0 / …)` на `rgb(37 40 45 / …)` в §7.1 (обе
+тени, 56 px референс; проверить также, что при масштабировании на 32/96 px в
+реализации используется тот же цвет).
+
+### M2 — цвет continuous-пульсации для reason `presence` не определён
+
+**Файл:** `docs/specs/179-device-icons-redesign.md:222-230` (§10.1), `:129-144` (§7.2)
+
+Пакет содержит **три** статических и три анимированных варианта Continuous
+Working: `Default` (обводка `#0C82F0`), `Yellow` (`#F0A00C`), `Green`
+(`#1DC21D`). `SPECIFICATION.md` §4 перечисляет источники непрерывной работы:
+«свет, вентилятор, климат, уборка, **присутствие**» — но ни один текстовый
+документ архива (`SPECIFICATION.md`, `ACTIVE_ANIMATION_SPEC.md`,
+`DEVELOPER_HANDOFF.md`) не говорит, какому из трёх цветов соответствует какая
+причина. Green нигде не привязан к семантике — ни в архиве, ни в ТЗ.
+
+Это не абстрактная тонкость: в текущей реализации `resolveDevicePulse`
+(`src/device-pulse.ts:93-103`) уже различает три причины continuous-пульсации
+— `presence`, `transition`, `running` — но все три сегодня рисуются **одним**
+переданным цветом, без различия по причине. Пакет впервые вводит три разных
+цветовых варианта того же мотива, а ТЗ §7.2/§10.1 говорит только «цвет
+соответствует resolved ordinary activity», не решая: (а) вводится ли новое
+различение по `reason` (presence → зелёный, как логично предположить по тексту
+пакета) или (b) `Green`-ассет в этой итерации не используется вовсе.
+
+AC8 требует, чтобы «цвета соответствовали §10» — при текущем тексте §10 это
+непроверяемо для presence-пульсации, потому что нормативного значения нет ни в
+одном документе.
+
+**Требуется:** явное решение (не владельца — это техническое/дизайн-решение,
+которое ТЗ вправе принять само, см. PROCESS.md §7.1 "всё, чего пользователь не
+наблюдает, агенты решают сами"; здесь пользователь наблюдает результат, но
+выбор между двумя понятными вариантами не требует владельца) — либо
+зафиксировать маппинг `presence → Green`, либо явно исключить `Green` из
+поставки с пометкой «принято предположительно» в §20.
+
+### M3 — easing для Short и Alert не зафиксирован
+
+**Файл:** `docs/specs/179-device-icons-redesign.md:232-246` (§10.2, §10.3)
+
+§10.1 (Continuous) корректно и проверяемо квотирует
+`cubic-bezier(.45,.05,.55,.95)` — совпадает с `ACTIVE_ANIMATION_SPEC.md` и с
+`working-ring`/`working-core-blue` в `Animated/Light/Continuous Working Default
+Animated.svg`. §10.2 (Short) и §10.3 (Alert) дают duration, delay, scale,
+stroke, цикл — но **не** easing. В реальных SVG (`Animated/Light/Short
+Activity Animated.svg`, `Animated/Light/Alert Animated.svg`) обе анимации
+используют `cubic-bezier(.22,.61,.36,1)` — другую кривую, чем Continuous.
+`ACTIVE_ANIMATION_SPEC.md` этот параметр текстом тоже не называет, только
+качественно («цикл должен быть плавным»), то есть источник факта — только
+SVG-пример (третий приоритет по ТЗ), и раз он существует и однозначен, ТЗ
+должен был его перенести.
+
+AC8 обещает «timings ... и colors соответствуют §10» — без easing реализация
+по умолчанию получит линейное или произвольное ускорение, что даст другое
+ощущение движения и не пройдёт визуальную приёмку по духу задачи ("читаемое
+глазами" состояние).
+
+**Требуется:** добавить `easing: cubic-bezier(.22,.61,.36,1)` в §10.2 и §10.3.
+
+### M4 — стоимость и выбор backdrop-blur для Dark не решены
+
+**Файл:** `docs/specs/179-device-icons-redesign.md:125-127` (§7.1), §17 (риски), AC13
+
+`manifest.json` прямо объявляет `"backdropBlurPx": 20` для темы Dark, и
+`Dark/Icon Default.svg` реально содержит `foreignObject` с
+`backdrop-filter:blur(20px)` — а не просто SVG-тень. Пакет отдельно поставляет
+`Dark/Icon Default No Blur.svg`, который побайтно совпадает с `Icon
+Default.svg` за вычетом именно этого слоя (проверено diff'ом), и, в отличие от
+Light-алиаса, **не** помечен в `manifest.json` как `deprecated`/`aliasOf` — то
+есть это не забытый дубликат, а отдельный, специально сохранённый fallback.
+
+ТЗ §7.1 ограничивается «Dark использует неизменённую dark-ревизию пакета…
+эталонные эффекты» — не решая, войдёт ли `backdrop-filter: blur(20px)` **на
+каждый маркер** в продакшен, или используется No-Blur вариант. Это не мелочь:
+`backdrop-filter` — дорогая по композитингу операция; `docs/SCOPE.md` называет
+20–200 устройств на план и киоск-планшет как основную View-поверхность для двух
+из трёх персон (`docs/TOUCH-SUPPORT.md`: View/kiosk — блокирующие). В `src/`
+сегодня `backdrop-filter` не применяется ни к одному маркеру (только к
+editor-tray, `src/editor-secondary.styles.ts`), так что это действительно новая
+для маркеров нагрузка, а не продолжение существующей. AC13 («нет per-frame JS,
+ResizeObserver…») не упоминает composite-cost blur-слоёв, и §17 «Риски» этот
+риск не называет вовсе.
+
+**Требуется:** явное решение — либо применять blur (тогда назвать это в §17 и
+покрыть производительность в AC13/perf-профиле), либо взять готовый No-Blur
+Dark-ассет как основной рендер продакшена (быстрее, безопаснее для 200
+устройств), с записью решения и причины.
+
+### M5 — не назван обязательный тег `Touch editor:` из TOUCH-SUPPORT.md
+
+**Файл:** `docs/specs/179-device-icons-redesign.md` (документ целиком)
+
+`docs/TOUCH-SUPPORT.md`, раздел «Documentation rule»: «New editor feature
+specifications and code reviews must state one of: `Touch editor: supported`;
+`Touch editor: best effort / intentionally degraded`; `Touch editor: not
+exposed`.» ТЗ меняет интерактивный маркер (shell, hit area, focus/keyboard) на
+поверхности **Device editor** (§11: «Интерактивные маркеры в View/kiosk и
+Device editor получают `tabindex="0"`…»), то есть подпадает под правило, но
+нигде в документе буквально не заявляет ни один из трёх канонических тегов.
+Канонический документ подсистемы даёт готовый ответ («Device editor: Best
+effort» — из таблицы контракта), ТЗ просто не переносит его явным образом.
+
+**Требуется:** добавить одну строку вида `Touch editor: best effort /
+intentionally degraded` (или `supported`, если автор считает keyboard/hit-area
+изменения полностью бесплатными для touch — решение автора, не владельца) в
+раздел §11 или §17.
+
+### L1 — Light-only combo-эталоны (`Selected + Hover/Working/Green/Alert`,
+`Focus + Alert`) не отражены в golden-матрице
+
+`manifest.json` для Light содержит пять составных QA-эталонов
+(`Selected + Hover.svg` и т.д.), у Dark таких файлов нет вовсе (не только не
+хватает файлов — узлы Figma для них не заведены). §16.4 (golden matrix)
+перечисляет одиночные состояния, но не комбинации Selected/Focus с semantic
+state отдельно для Dark. Поскольку приоритет слоёв (§7.3) декларативный и
+реализуется одним рендерером, а не набором ассетов, это не блокирует
+разработку — но стоит явно зафиксировать в golden-матрице, что комбинации
+проверяются для обеих тем логически, а не по образцу (в Dark образца нет).
+Снимается автором с записью либо на одну строку в §16.4.
+
+### L2 — избыточная оговорка «unavailable hover запрещён» для `static_icon`
+
+§7.3 говорит про `static_icon`: «на интерактивной поверхности он сохраняет
+разрешённые hover/focus/click; unavailable hover запрещён». Действующее
+поведение `static_icon`, зафиксированное в `docs/USER-GUIDE.ru.md:811-816`,
+таково, что этот режим **вообще не входит** в ветку unavailable («не
+показывает… недоступность»). Формулировка ТЗ не противоречит этому, но звучит
+как отдельное правило для случая, который по документированному контракту не
+может произойти. Не блокирует; можно оставить как защитную формулировку или
+убрать как мёртвый кейс — на решение автора.
+
+## Проверено и признано корректным
+
+- **SHA-256 архива** (`63670C73E25D1E59DDAF1BE236F3D7F2FAC827B9B5D6DD4B77125EA9BC012025`)
+  совпадает с реально скачанным файлом по ссылке из issue.
+- **Геометрия shell/core**: круговой shell — путь-круг с центром `(63.5, 55.5)`
+  и радиусом `50.75` (диаметр `101.5`), core — `<rect>` `80×80` с тем же
+  центром. `101.5/127 ≈ 0.799`, `80/101.5 ≈ 0.788` — оба числа в ТЗ (§7.1)
+  подтверждены вычислением по реальному SVG, не переписаны на глаз.
+- **Цвета состояний** (§7.2): hover/focus `#0C82F0`, active/working/unlocked
+  `#F0A00C`, alert `#F0410C`, unavailable core `#B5BAC1`, high LQI `#1DC21D`,
+  lock `black` — все сверены построчным `grep` по `fill`/`stroke` в
+  соответствующих Light SVG и совпадают.
+- **Continuous/Short/Alert тайминги** (кроме easing Short/Alert — см. M3):
+  3.6 с / scale 1→1.5 / opacity .55→0 для Continuous и 1.1 с × 3 с задержками
+  0/1.1/2.2 с для Short — оба подтверждены и текстовым документом архива, и
+  реальными `@keyframes`.
+- **Обнаруженная и верно разрешённая ошибка пакета**: `Light/Zigbee LQI
+  Low.svg` действительно закрашен `#F0A00C` (тот же цвет, что Mid), а не
+  красным. Утверждение ТЗ §9/§20.2 о том, что это ошибка экспорта и что
+  production-эталон — red по текстовой спецификации и решению владельца,
+  подтверждено — это пример правильно задокументированного разрешения
+  конфликта источников, а не догадки.
+- **LQI-границы 40/180** согласуются с уже существующей документированной
+  границей комнатного LQI-градиента (`docs/USER-GUIDE.ru.md:873`: «градиент от
+  красного (≤40) до зелёного (≥180)») — решение владельца не произвольно, оно
+  продолжает существующий продуктовый факт.
+- **Секьюрность**: §11 явно исключает новый путь обхода подтверждения для
+  замков/клапанов через клавиатуру — Enter/Space вызывают тот же существующий
+  click handler. Инвариант `docs/SCOPE.md` («никогда не по тапу без
+  подтверждения») не нарушается ни в одном пункте ТЗ.
+- **Совместимость** (§12): поведение "Open → Save не материализует
+  `ripple_size`/`ripple_color` при смене UI-default 3→1.5" явно и правильно
+  учитывает существующий паттерн записи (`d.rippleSize !== 3 ? … : null` в
+  `src/houseplan-card.ts:18448`) и предотвращает тихий сброс уже явно
+  сохранённых значений.
+- **Обязательные разделы §7.1 PROCESS.md** — все присутствуют: сценарий,
+  что видит пользователь до/после, проблема, скоуп/не-скоуп, контракт
+  поведения, UX/доступность, данные/миграция, i18n, AC1…AC14 с доказательством,
+  план автотестов, риски, откат, release-артефакты.
+- **Трассируемость**: `docs/specs/README.md` обновлён в том же коммите с
+  двусторонней ссылкой issue↔ТЗ; коммит `517a710` несёт корректные трейлеры
+  `Issue: #179` и `User-Visible: no`.
+- **Открытых продуктовых вопросов владельцу нет** — все решения, требующие
+  продуктового выбора (unavailable hover, приоритет pulse-настроек, границы
+  LQI, клавиатура), приняты владельцем в комментарии аналитики и перенесены в
+  §4 ТЗ без искажений.
+
+## Чего не проверял
+
+- **Figma-фреймы** (`99:1290`, `104:1539`) напрямую не открывались — ревью
+  ограничено содержимым архива, доступного по ссылке из issue; это тот же
+  источник, которым пользовался автор ТЗ.
+- **`Index.html`/PNG-визуализации архива** не открывались в браузере —
+  ревью на этапе ТЗ смотрит на текст и код, не на артефакт для дизайнеров;
+  визуальная проверка релевантна golden-этапу code review.
+- **Полная построчная сверка размеров 32 px и 96 px** (только 56 px эталон
+  проверен вычислением; 32/96 в ТЗ не квотируются отдельно, они делегированы
+  таблице `SPECIFICATION.md` §8 без искажения).
+- **Производительность** не измерялась (кода нет); M4 — это находка на уровне
+  решения, а не измеренный факт деградации.
+- Гейты `typecheck`/`test`/`build`/smoke/golden не запускались — на этапе
+  ревью ТЗ продуктового кода не существует, раздел неприменим.
+
+## Итог
+
+Ни одна находка не блокирует по существу — все пять Medium устранимы точечными
+правками текста ТЗ (это ревью самого текста, а не архитектурного решения), но
+без них AC1, AC8 и AC13 не выполняют собственное обещание доказуемости
+(«соответствует §10», «соответствует пакету», «нет неизвестных perf-рисков»).
+Вердикт — жёлтый, возврат автору в рамках текущего issue.