houseplan-card/docs/reviews/SPEC-REVIEW-180-r1.md

SPEC-REVIEW-180-r1

• Issue: #180 — hp-color-opacity: unified picker не покрывает ripple/activity-color и state-color settings (wall/light/temp/LQI/glow)
• ТЗ: docs/specs/180-all-color-call-sites.md (коммит 3540d24, ветка issue/180-unified-color-picker)
• Трек: обычный (метка small не установлена; лимит циклов ревью ТЗ — 4)
• Ревьюер: Claude, роль «ревьюер ТЗ» (сессия отдельна от анализа/автора)
• Цикл: r1/4

Вердикт

Зелёный. High: 0 · Medium: 0. Одна Low-находка, ниже — с записью «waived».

Скоуп проверки

Этап — ревью ТЗ (PROCESS.md §2.4). Материал: тело issue #180, оба комментария (аналитика владельца + хендофф автора ТЗ), файл docs/specs/180-all-color-call-sites.md, docs/specs/README.md, канонический docs/specs/057-color-opacity-picker.md (родительский контракт hp-color-opacity), docs/USER-GUIDE.ru.md, docs/TOUCH-SUPPORT.md, docs/CONFIG-COMPATIBILITY.md и фактическое состояние src/houseplan-card.ts, src/hp-color-opacity.ts, src/logic.ts, src/types.ts, src/i18n/{en,ru}.json на ветке issue/180-unified-color-picker (совпадает с origin/dev + два документационных коммита). Продуктовый код на этой ветке не менялся — проверено git diff --name-only origin/dev...HEAD: только docs/specs/180-all-color-call-sites.md и docs/specs/README.md.

Как проверялось

Ревью состязательное: каждое фактическое утверждение ТЗ (номера полей, названия свойств API, i18n-ключи, персистентная модель) сверено с реальным кодом на ветке, а не принято со слов автора.

1. Прочитаны docs/SCOPE.md, AGENTS.md, PROCESS.md (полностью, включая §7.1, §4, §5) — для критериев обязательных разделов, лимита циклов и трека.
2. Прочитано issue #180 целиком: исходная находка ревьюера #57, комментарий аналитики владельца (2026-08-19, сам зафиксировал продуктовый контракт — мигрировать все 15 полей), хендофф автора ТЗ.
3. Прочитан весь файл ТЗ docs/specs/180-all-color-call-sites.md (17 разделов).
4. Прочитан канонический docs/specs/057-color-opacity-picker.md — сверен заявленный публичный API hp-color-opacity (color, opacity, showOpacity, disabled, label, opacityLabel, pickerLabels, событие hp-color-opacity-change) с тем, что ТЗ #180 обещает переиспользовать.
5. Построчно сверены с исходным кодом (Read/Grep):
   • все пять native <input type="color"> в src/houseplan-card.ts (14503, 15085, 19332, 19529, 19548) — состав и группировка полей совпадают с таблицей §4 ТЗ (11 через _renderColorRow, плюс bgColor/ripple/roomColor/ space bgColor);
   • публичные свойства hp-color-opacity (src/hp-color-opacity.ts:34-62) — showOpacity, pickerLabels, opacityLabel, disabled реально существуют;
   • уже мигрированный Glow (houseplan-card.ts:19158-19168) как прецедент паттерна showOpacity=false + игнорирование opacity в draft — совпадает с §7.2 и техническим предположением №2 ТЗ;
   • модель данных: FillColors (src/types.ts:1287-1302, тип {c,a}), space.settings.room_color/room_opacity (src/logic.ts:1245-1246, houseplan-card.ts:13447-13448), marker.ripple_color (src/types.ts:131), settings.bg_color/space.settings.bg_color (houseplan-card.ts:13449, 14436-14437) — все совпадают с §9 ТЗ буквально;
   • i18n-ключи, на которые ссылается ТЗ (gs.wall_fill, gs.bg_default, gs.bg_theme, space.bg_inherit, space.bg_inherited, marker.activity_color, marker.ripple_size, space.room_color, space.opacity) — присутствуют в обоих en.json/ru.json парами.
6. Терминология сверена с docs/USER-GUIDE.ru.md: «Общие настройки», «Пространство», формулировка decor-picker («нажмите на образец цвета, чтобы открыть выбор цвета и прозрачности», строка 947) и существующее описание пульсации активности (строки 721-748) — ТЗ не придумывает новых терминов.
7. Формулировка Touch editor: supported (§11 ТЗ) сверена с каноническим списком трёх допустимых фраз в docs/TOUCH-SUPPORT.md:149-151 — совпадает буквально.
8. docs/CONFIG-COMPATIBILITY.md — подтверждено, что задача не вводит и не меняет персистентные поля, поэтому реестр совместимости не затрагивается; ТЗ §9 прямо констатирует это, а не молчит об этом.
9. Проверена трассируемость: docs/specs/README.md:113-114 содержит обе строки (issue ↔ ТЗ) для #57 и #180; коммит 3540d24 несёт корректные трейлеры Issue: #180 / User-Visible: no (git log -1 --format=... 3540d24).
10. Проверена феासибельность golden-плана: demo/golden/{harness,accept,matrix}.mjs построены вокруг произвольного scenario.id/viewport/theme — добавление трёх новых сценариев (general settings, marker mobile, space) механически возможно без архитектурных изменений инфраструктуры.
11. Проверено отсутствие незамеченных мест: полный grep -rn "type=[\"']color[\"']" src/*.ts — ровно 5 совпадений в houseplan-card.ts (совпадает с инвентаризацией ТЗ) плюс одно в src/styles.ts:2162 — CSS-селектор, разбор ниже.
12. Проверено, что day/night фон (bgMode: 'daynight') не хранит собственных цветов (процедурный градиент по docs/SUN.md), то есть инвентаризация ТЗ (пять шаблонов/15 полей) действительно исчерпывающая, а не пропускает шестое скрытое место.

Гейты для этапа спек-ревью не прогонялись — на этой стадии нет продуктового кода для typecheck/test/build; сам факт отсутствия правок в src/** подтверждён diff'ом (п.10 выше), а не декларацией автора.

Находки

Low-1 — CSS-селектор input[type='color'] не упомянут в плане очистки

Файл: src/styles.ts:2162 (.colorrow input[type='color'] { width: 42px; ... }).

После миграции ни один элемент в DOM не будет соответствовать этому селектору — правило станет мёртвым. ТЗ формулирует source-invariant AC1 как «в src/**/*.ts отсутствует input[type=color] в любой допустимой Lit-форме» — эта фраза точна для шаблонов, но явно не называет styles.ts и не включает очистку CSS в план тестов (§13, п.1: «Рекурсивный scan src/**/*.ts запрещает native input[type=color]»). Риск скорее методологический, чем поведенческий: если исполнитель реализует scan буквальным regex по всему src/**/*.ts без разбора Lit-шаблонов от строковых литералов CSS, тест либо ложно упадёт на не относящемся к делу файле, либо (если scan ограничат template literals) осиротевшее правило останется в бандле молча.

Воспроизведение: grep -rn "type=\"color\"\|type='color'" src/*.ts — 5 совпадений в houseplan-card.ts (реальные шаблоны) + 1 в styles.ts (CSS селектор), тогда как ТЗ и его AC1 описывают только первые пять.

Серьёзность: Low — не блокирует ни один AC, не меняет наблюдаемое поведение, исправляется одной строкой при реализации.

Решение ревьюера: waived, с записью. AC1 в применении к «допустимой Lit-форме» де-факто ограничивает scope шаблонами; удаление мёртвого CSS-правила — естественное следствие реализации АС2/АС3 (все пять consumers перестают рендерить native input) и не требует отдельного продуктового решения владельца. Не выношу отдельным issue: находка ниже порога Medium (PROCESS.md §3.8), достаточно фиксации здесь.

Что проверено и корректно

• Обязательные разделы §7.1 — все присутствуют и в правильном порядке: сценарий (§1), что человек увидит до/после (§2), проблема (§3), scope/не-scope (§4-5), контракт поведения (§6-7), UX/accessibility (§8), модель данных и совместимость (§9), i18n (§10), touch как часть UX-контракта (§11), AC1-AC8 с доказательством у каждого (§12), план автотестов (§13), риски (§14), откат (§15), release-артефакты (§16). Плюс два продуктовых обязательных пункта — персона/ поверхность/момент и однострочное «что увидит человек» — закрыты §1-2 без терминов реализации.
• Трек выбран верно. Сложность 4/10, пять разных dialog families, alpha/ nullable/inherit контракты — превышает порог «одна поверхность» и «сложность ≤3» из §5 PROCESS.md; small не применили обоснованно.
• Инвентаризация полная и точна. Все 15 полей и все 5 native-шаблонов из §4 ТЗ подтверждены построчно в текущем src/houseplan-card.ts; ни одно дополнительное место (day/night, decor, room custom fill, Glow — уже мигрированные #57 consumers) не пропущено и не задвоено.
• Технические утверждения не являются догадками. Каждое поведенческое обещание (nullable/default/inherit модель, showOpacity=false не пишет alpha, hp-color-opacity API) имеет прямой аналог в уже смерженном коде (#57, Glow) — это перенос существующего паттерна, а не изобретённое поведение.
• AC проверяемы и однозначны. Все 8 AC пронумерованы, у каждого — конкретный способ доказательства (unit/source-scan, DOM/browser smoke, golden, round-trip unit); ни один не сформулирован расплывчато настолько, чтобы допускать произвольную трактовку «выполнено».
• Продуктовая неопределённость закрыта на входе, а не додумана. Ключевой продуктовый вопрос («что входит в #180 — весь остаток native color inputs или уже нет») задан и решён самим владельцем в комментарии аналитики 2026-08-19, до написания ТЗ; спека честно ссылается на это решение (§4, преамбула) вместо того чтобы выдавать его за собственную догадку автора.
• Assumed-block корректен. §17 содержит 5 явно технических предположений (структура компонентов, обработка opacity=1 для color-only, локальность layout-классов, отсутствие reset callback у picker, golden thresholds на реализации) — ни одно из них не является продуктовым вопросом, который следовало бы задать владельцу.
• Терминология и touch-формулировка соответствуют канону — сверено с docs/USER-GUIDE.ru.md и docs/TOUCH-SUPPORT.md построчно (см. «Как проверялось», пп. 6-7).
• Совместимость. §9 явно и верно констатирует отсутствие изменений схемы/ миграции; сверка с docs/CONFIG-COMPATIBILITY.md подтверждает, что реестр не требует новой записи.
• Трейлеры и трассируемость корректны для документационного коммита на этом этапе (Issue: #180, User-Visible: no, ссылки issue ↔ ТЗ в docs/specs/README.md).

Чего не проверял

• Golden pixel-thresholds и точные scenario id — по ТЗ (§17, п.5) выбираются на реализации по полному Linux-артефакту; на этапе спек-ревью нет артефакта для проверки.
• Реальную работу браузерных smoke/golden сценариев — код ещё не написан (подтверждено diff'ом), поэтому запуск npm test/npm run build/ node demo/smoke_color_picker.mjs на этом этапе неприменим и не выполнялся.
• Backend/Python — задача не касается custom_components/**/*.py; pytest не запускался, это верно для class A/B TS-only изменения.
• Производительность на реальном железе — компонентная модель без новых observers/network подтверждена чтением, а не профилированием (профиль не требуется до S7-code-review при отсутствии заявленного perf-риска).

Итог

ТЗ #180 выполнимо, проверяемо и не содержит выданных за факт догадок: каждое поведенческое утверждение либо повторяет уже принятый контракт #57, либо сверено с текущим кодом. Единственная находка (Low, мёртвый CSS-селектор вне явного scope AC1) waived с запиской, доработки ТЗ не требует. Готово к S5-ready.