27 KiB
SPEC-REVIEW-57-r1
- Issue: https://github.com/Matysh/houseplan-card/issues/57
- ТЗ под ревью:
docs/specs/057-color-opacity-picker.md(коммитb8d2613a18f53f0d1bae9e02d9966496d49ad22d, веткаissue/57-color-opacity-picker) - Роль: ревьюер ТЗ (не автор), этап
S4-spec-review - Трек: обычный (метки issue:
P3,polish,tech-debt,S4-spec-review— меткиsmall/trivialнет), файл вdocs/specs/создан корректно, а не ТЗ в теле issue. - Цикл: r1/4
Скоуп ревью
Проверялось соответствие ТЗ:
docs/SCOPE.md— легитимность задачи как editor usability polish, а не расширение продукта за пределы job'ов;PROCESS.md§2.4, §2.5 (DoR), §7.1 (обязательные разделы), §3/§12 (в т.ч. «догадка вместо решения», Medium → отдельный issue);AGENTS.md— классы файлов, ветка, трейлеры коммита ТЗ;docs/TOUCH-SUPPORT.md— контракт editors (best effort) и требование явной меткиTouch editor: …для новых editor-фич;- полному тексту issue #57 и всем трём комментариям (аналитика с Q1 и предложенным default, решение владельца по Q1, финальный хендофф автора);
- фактическому состоянию
src/hp-color-opacity.tsиsrc/floating-surface-controller.ts— на предмет того, что технические утверждения ТЗ о существующей инфраструктуре (#68) не выдуманы; - фактическим call sites
hp-color-opacityи связанных цветовых настроек вsrc/houseplan-card.ts— на предмет полноты списка «Existing call sites» (§12/§16 ТЗ) относительно текста issue («decor, room colors, ripple»).
Как проверялось
- Прочитан весь тред issue #57 (
gh issue view 57 --json body,comments): исходное описание (research task → recommendation → swap behindhp-color-opacityAPI, явно называет три категории call sites: "decor, room colors, ripple"), аналитика 2026-08-14 (ценность 4/10, сложность 4/10, риск 5/10, P3, обычный трек, один продуктовый вопрос Q1 с default), решение владельца 2026-08-15 по Q1 («убрать вложенность: один клик — одна поверхность с цветом и прозрачностью вместе, без второго системного dialog»), финальный комментарий автора со ссылкой на коммит и файл ТЗ. Вопрос задан корректно — единственный, продуктовый, с default; ни одного технического вопроса владельцу не эскалировано. - Сверены обязательные разделы ТЗ (§7.1 PROCESS.md) построчно — таблица ниже.
- Прочитан текущий
src/hp-color-opacity.tsцеликом: подтверждено, что проблема из §3 ТЗ реальна —_pickerTemplate()(строка 347) рендерит<input type="color">внутри popover-поверхности, то есть первый клик открывает House Plan popover, а сам цвет по-прежнему делегирован системному<input type=color>без alpha. Формулировка проблемы не голословна. - Прочитан
src/floating-surface-controller.tsиregisterOverlayвsrc/hp-dialog.ts:365— оба реальны, используются уже сегодня вhp-color-opacity.ts. Ссылки ТЗ §7/§9/§10 на «общий floating-surface/ overlay lifecycle из #68» и «exclusive transient surface, конкурирующая сhp-help» подтверждены существующим кодом и терминологиейdocs/specs/068-help-affordance.md:50(«одна exclusive transient- поверхность на диалог,hp-helpиhp-color-opacityиспользуют один контракт»), а не изобретены заново. - Проверены заявленные в §12 ТЗ «Existing call sites» построчным поиском
<hp-color-opacityвsrc/houseplan-card.ts: найдены decor color/fill (строки 9152, 9167, 9325, 9395, 9442), marker Glow override (17917,showOpacity=falseподтверждён на 17918), room/space custom fill (18365, 18549). Список ТЗ для этих сайтов точен. - Тем же поиском по всему файлу найдена «третья» линия цветовых контролов,
которая не проходит через
hp-color-opacityвовсе:_renderColorRow()(houseplan-card.ts:13477-13487) — рендерит голый<input type="color">рядом с отдельным<input type="range">, без единого swatch/popover. Используется 11 раз в диалоге "Общие настройки" (gs.*):light_on/off/none(14003-14005),temp_cold/ok/hot(14007-14009),lqi_low/high(14011-14012),glow_base/glow_light(14014-14015),wall_fill(14028) — тот же класс проблемы («второй клик по цвету — системный picker без прозрачности», здесь даже без единого swatch), но не упомянуто ни в §12 (call sites), ни в §16 (затронутые поверхности), ни в §5 (не входит в задачу) ТЗ. - Отдельно проверено значение слова «ripple» из текста issue: marker
activity color(rippleColor,houseplan-card.ts:18087-18090, типripple_colorвtypes.ts:1442) — тоже голый<input type="color">безhp-color-opacityи без alpha. Это ровно та категория, которую issue называет по имени («decor, room colors, ripple») — и она отсутствует в §12 ТЗ, который вместо неё называет «marker Glow override» (уже мигрировавший наhp-color-opacityкомпонент, не требующий работы). См. Medium-1 ниже. - Проверено наличие требуемой
docs/TOUCH-SUPPORT.mdбуквальной декларацииTouch editor: …— отсутствует; см. Low-1. - Проверены трейлеры и class-принадлежность:
git show b8d2613 --statпоказывает толькоdocs/specs/057-color-opacity-picker.mdиdocs/specs/README.md(класс C, ни одного файла класса A — правило №1 AGENTS.md соблюдено, продуктовый код не тронут на этапе ТЗ). Коммит несётIssue: #57иUser-Visible: no— корректно для документа класса C.docs/specs/README.mdобновлён тем же коммитом, ссылка issue↔ТЗ в обе стороны на месте. - Проверено существование release-артефактов, которые §18 ТЗ обещает
обновить:
docs/TESTING.md,docs/CHANGELOG.md,docs/CHANGELOG.ru.md,docs/USER-GUIDE.ru.md— все существуют, ссылки не на несуществующие файлы. - Проверены i18n-конвенции: явные ключи не перечислены буквально (только
человеко-читаемые названия controls в §13), но такой же уровень
детализации принят в прошедшем ревью
docs/specs/068-help-affordance.md(тоже описывает механизм именования, а не конкретные строки) — не считаю это дефектом при спек-ревью, конкретные ключи — свободное техническое решение реализации.
Обязательные разделы (§7.1 PROCESS.md)
| Раздел | Есть | Комментарий |
|---|---|---|
| Сценарий (персона/поверхность/момент) | ✅ | §1 — home admin, editor-диалоги, момент клика по образцу цвета |
| Что человек увидит до/после | ✅ | §2, одна фраза до/после, без глубоких implementation-терминов |
| Проблема (с подтверждённой причиной) | ✅ | §3, подтверждена чтением hp-color-opacity.ts (см. «Как проверялось» п.3) |
| Скоуп / не-скоуп | ⚠️ | §4/§5 — полны для перечисленного, но не покрывают все call sites из текста issue, см. Medium-1 |
| Контракт поведения | ✅ | §7 (interaction contract), §8 (значения/преобразования) |
| Модель данных и миграция | ✅ | §12 — API/schema неизменны, обоснованно (presentation-only компонент) |
| UX, i18n, accessibility, touch | ✅ | §9-11, §13; touch — см. Low-1 (нет буквальной метки) |
| AC1…ACn с доказательством | ✅ | §14, 9 штук, у каждого назван способ доказательства (unit/smoke/golden/build artifact) |
| План автотестов | ✅ | §15, unit/smoke/golden/performance по отдельности |
| Риски | ✅ | §17, таблица риск/мера, 5 строк |
| Откат | ✅ | §17 — public API и данные неизменны, откат = revert реализации |
| Release-артефакты | ✅ | §18, конкретные существующие документы, оба changelog в одном User-Visible: yes коммите |
Присутствует и явный блок «Принятые технические предположения» (§19) — разделение продуктового/технического, которого требует §7.1: 5 пунктов, всё базовая реализация/API-неизменность, не user-facing решения.
Находки
Medium-1 — ТЗ молча сужает названные в issue call sites: «ripple» пропущен, параллельная семья «Общие настройки» не упомянута вовсе
Файл: docs/specs/057-color-opacity-picker.md:179-181 (§12, «Existing
call sites»), :253-259 (§16, «Затронутые поверхности»), :46-54 (§5, «Не
входит в задачу») против src/houseplan-card.ts:13477-13487
(_renderColorRow), :14003-14028 (11 вызовов), :18087-18090
(rippleColor).
Issue #57 явно перечисляет три категории call sites, которые должны
обновиться «at once»: «decor, room colors, ripple». ТЗ §12 вместо этого
называет «decor stroke/fill/text/furniture, room/space custom fill и marker
Glow override» — «ripple» нигде не упомянут, ни в scope, ни в §5 «Не входит
в задачу» как явно отложенный пункт. Проверка кодом подтверждает: marker
ripple_color (houseplan-card.ts:18089, «Marker: activity color» в
диалоге) — это голый <input type="color">, вообще не использующий
hp-color-opacity, то есть требующий такой же работы, как и остальные call
sites, но не включённый в контракт, AC или тестовый план.
Дополнительно обнаружена целая параллельная реализация того же паттерна:
_renderColorRow() (houseplan-card.ts:13477-13487) рендерит <input type="color"> рядом с отдельным <input type="range"> — тот же класс
проблемы, который issue описывает как причину задачи («второй клик по цвету
— системный picker без прозрачности»), только даже без единого swatch. Она
используется 11 раз в диалоге "Общие настройки" для light_on/off/none,
temp_cold/ok/hot, lqi_low/high, glow_base/glow_light, wall_fill —
именно тех состояний, вокруг которых построены J1/J5/J7 SCOPE.md (живая
заливка комнат по свету/температуре/LQI). Она не упомянута ТЗ вовсе — ни как
included, ни как explicitly excluded.
Воспроизведение: после реализации ТЗ как написано, пользователь по-прежнему
встретит исходную проблему (второй клик по цвету открывает системный picker,
прозрачность теряется/недоступна одновременно) на: (1) marker "activity color"
(инструмент разметки, диалог устройства, display: icon_ripple) — явно
названный issue как «ripple»; (2) любом из 11 полей диалога "Общие настройки".
То есть заявленная в issue ценность «all call sites... upgrade at once»
достигается лишь частично, и в самом ТЗ этот пробел не зафиксирован как
осознанное решение — то есть выглядит как полное покрытие, не будучи им.
Это не делает текущий контракт невыполнимым или непроверяемым — AC1-9 самодостаточны и проверяемы для того набора call sites, который ТЗ действительно перечисляет. Поэтому уровень Medium, а не High: реализация по этому ТЗ работает и не содержит логической ошибки, дефект — в неполноте заявленного покрытия и в тишине вокруг этой неполноты.
Решение ревьюера: Medium, заведён отдельным issue
#180 со ссылкой на
#57, метки tech-debt/P3/S1-new. Не блокирует S5-ready. Рекомендация
автору — на выбор: (а) явно перечислить ripple в §12/§16 и включить в объём
этого issue, раз он и так использует тот же компонент с
showOpacity=false по образцу Glow (стоимость мала, паттерн уже есть); либо
(б) добавить одну строку в §5 «Не входит в задачу», явно называющую ripple
и "Общие настройки" state-color rows отложенными в #180 — чтобы ТЗ не
выглядело полным покрытием, будучи им лишь частично. Выбор (а)/(б) — не
продуктовый вопрос сам по себе (объём этого конкретного issue уже решён
владельцем неявно: он не разбирал этот список построчно), поэтому это может
решить сам автор технически; если он предпочтёт спросить владельца — это
корректный продуктовый вопрос («входит ли ripple в объём #57») с default
(б).
Low-1 — нет буквальной декларации Touch editor: … по docs/TOUCH-SUPPORT.md
Файл: docs/specs/057-color-opacity-picker.md:124-137 (§9)
docs/TOUCH-SUPPORT.md, раздел «Documentation rule», требует, чтобы новая
спецификация editor-фичи явно указывала одно из: Touch editor: supported /
best effort / intentionally degraded / not exposed. §9 ТЗ подробно
описывает, что сам picker получает «явную touch-поддержку» (pointer capture,
multi-touch safety, 40×40 touch targets), но не содержит буквальной строки
формата Touch editor: …. Тот же класс замечания последовательно
фиксировался как Low в прошлых спек-ревью этого репозитория
(SPEC-REVIEW-89-r1, SPEC-REVIEW-122-r1, SPEC-REVIEW-141-r1,
SPEC-REVIEW-146-r1), включая прецедент docs/specs/068-help-affordance.md:10
(«Touch editor: поддерживается для самого affordance» — компонент,
получающий явную touch-гарантию поверх общего best-effort правила
редакторов, ровно как здесь).
Решение ревьюера: Low, не блокирует. Рекомендация — добавить одну строку
вида «Touch editor: supported (сам picker; остальные операции родительских
редакторов остаются best effort по docs/TOUCH-SUPPORT.md)» рядом с §9.
Оставляю на усмотрение автора; фиксирую здесь как условие «низкое либо
правится, либо снимается с записью» (§3.8 PROCESS.md) — снимаю без правки,
так как содержательно требование уже выполнено прозой, отсутствует только
буквальный формат метки.
Что проверено и корректно
- Соответствие
docs/SCOPE.md: задача — editor usability polish, не создаёт нового продуктового job'а и не расширяется на профессиональный графический редактор (владелец явно закрыл этот вопрос в аналитике: «в scope как editor usability, но без необходимости создавать профессиональный графический редактор»); эксплуатирует существующие J1/J4/ J5/J6/J7 через editor usability, не создавая нового. - Легитимность полного трека: issue не помечен
small/trivial, файл ТЗ вdocs/specs/создан по правилу (не в теле issue) — соответствует сложности 4/10 и множеству поверхностей (decor/room/space/marker). - Продуктовый вопрос закрыт по процессу: единственный вопрос Q1
(«насколько сложным должен быть picker») задан батчем с default,
blockedбыл выставлен и снят владельцем при ответе; технических вопросов владельцу не эскалировано. - Главное решение владельца воспроизведено точно: §7 ТЗ («один клик — одна поверхность, где сразу доступны цвет и прозрачность, без вложенного системного picker») дословно соответствует формулировке владельца в комментарии от 2026-08-15.
- Технический диагноз §3 не голословен — подтверждён прямым чтением
src/hp-color-opacity.ts(см. «Как проверялось» п.3): текущий компонент действительно делегирует цвет<input type=color>внутри своего popover. - Инфраструктурные ссылки на #68 (
FloatingSurfaceController,registerOverlay, «exclusive transient surface») реальны и точны — подтверждены существующим кодомfloating-surface-controller.tsиhp-dialog.ts:365, а не изобретены. - Перечисленные (не все, см. Medium-1) существующие call sites точны —
все указанные в §12 использования
<hp-color-opacity>найдены построчно вhouseplan-card.tsс совпадающей семантикой (showOpacity=falseдля Glow подтверждён). - API/compatibility (§12) корректно не расширяет config/storage schema —
оправданно: изменяется только внутренняя реализация presentation-only
компонента, внешний контракт (
color,opacity, событие) не меняется. - Bundle-budget (§6, §14 AC8) сформулирован как измеримый критерий с точным способом доказательства (exact production build artifact, raw+gzip), включает условный путь для vendored-альтернативы с теми же ограничениями — не оставляет технический выбор недоказуемым.
- AC1-AC9 однозначны и снабжены допустимым по §2.5 PROCESS.md способом доказательства (unit/smoke/golden/review), ни один не оставлен неопределённым.
- Release-артефакты (§18) ссылаются на существующие файлы документации,
оба changelog в одном
User-Visible: yesкоммите — соответствует правилу 11 PROCESS.md. - Трассируемость:
docs/specs/README.mdобновлён тем же коммитом (раздел «P3», ссылка на ТЗ); коммитb8d2613несётIssue: #57,User-Visible: no, класс C — корректно для документа ТЗ.git show --statне содержит ни одного файла класса A — правило №1 AGENTS.md соблюдено. - «Принятые технические предположения» (§19) корректно отделяют свободные для автора реализации решения от продуктового контракта — соответствует требуемой в §7.1 PROCESS.md структуре.
Чего не проверял
- Не прогонял никаких гейтов (
typecheck/test/build/golden/смоки) — на этапе ТЗ продуктовый код не существует, гейты неприменимы; существование упомянутых механизмов (FloatingSurfaceController,registerOverlay,demo/golden,docs/TESTING.md) проверено чтением файловой системы, а не исполнением. - Не проверял реализуемость конкретной геометрии saturation/value field, round-trip toleranse ≤1 RGB channel или точный алгоритм hue-memory для achromatic цвета как кода — по тексту ТЗ (§19) это явно свободное техническое решение автора кода, подлежащее доказательству тестами на реализации/код-ревью, а не предмет ревью ТЗ.
- Не проверял точность числовых оценок аналитики (ценность 4/10, сложность 4/10, риск 5/10, P3) по существу — это поле владельца (PROCESS.md §2.2), уже принятое явным решением до написания ТЗ.
- Не проверял весь
src/houseplan-card.tsпострочно на предмет иных, ещё не найденных мест с голым<input type=color>за пределами уже идентифицированных_renderColorRow/ripple_color/bg_color— поиск был ограничен паттернамиhp-color-opacity,input type=.?color.?и_renderColorRowчерез grep, не исчерпывающим построчным чтением всего файла (~18500 строк).bg_color(фон, без alpha) сознательно не включён в Medium-1: у него нет отдельного opacity-компонента, то есть не тот класс проблемы, который описывает issue. - Не проверял, действительно ли
docs/USER-GUIDE.ru.mdтребует конкретной правки текста прямо сейчас (спека верно откладывает точную формулировку в release-артефакты §18, реализация ещё не начата).
Вердикт
Зелёный. High: 0, Medium: 1 (заведён отдельным issue
#180, не блокирует),
Low: 1 (косметика, снята с записью в этом документе, не требует правки перед
S5-ready). ТЗ корректно и проверяемо решает то, что само заявляет как
скоуп: устраняет вложенный system picker для decor/room/space/Glow call
sites, техническая база (#68) подтверждена чтением кода, продуктовый вопрос
владельца закрыт по процессу. Единственный содержательный пробел — молчаливое
сужение относительно явно названного в issue call site «ripple» и не
упомянутой параллельной семьи «Общие настройки» — не делает написанный
контракт невыполнимым, поэтому не блокирует, но обязан быть решён отдельно
(issue #180) до того, как задача #57 будет считаться полным ответом на
исходный запрос.