From 424e613c6fbb9f8640c2a4f188c5ecce8ed2431e Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Wed, 19 Aug 2026 19:05:01 +0000 Subject: [PATCH] docs: review document for #179 Issue: #179 User-Visible: no --- docs/reviews/SPEC-REVIEW-179-r1.md | 271 +++++++++++++++++++++++++++++ 1 file changed, 271 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-179-r1.md diff --git a/docs/reviews/SPEC-REVIEW-179-r1.md b/docs/reviews/SPEC-REVIEW-179-r1.md new file mode 100644 index 00000000..88b60687 --- /dev/null +++ b/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`, ``, + `` координаты 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 — `` `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.