Files
2026-09-24 00:56:26 +00:00

17 KiB
Raw Permalink Blame History

SPEC-REVIEW — issue #615 · заход r1

Задача: «Плитки палитры после #605 всегда сплошные: «No lights» с прозрачностью 0 % выглядит насыщенным цветом» Этап: ТЗ на ревью (S4-spec-review) · трек small · лимит циклов ревью ТЗ — 2 Заход: r1 (первый разбор) · блокирующих циклов израсходовано 0/2 до этого вердикта

Скоуп разбора

Первый раунд — разбор полный. Проверялись: соответствие обязательным разделам §7.1 PROCESS.md, однозначность и проверяемость AC1…AC4, отсутствие догадок, выданных за факт, согласованность контракта поведения с реальным состоянием кода (src/), согласованность с каноном подсистемы (docs/USER-GUIDE.ru.md, docs/design/600-settings-dialogs/) и с фактическим состоянием issue #605, а также реалистичность заявленных в таблице AC свидетелей — включая колонку «чем краснеет», которая по §2.7 обязана называть настоящую, а не воображаемую защиту.

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

Ревью текста ТЗ дополнено чтением кода — не потому что это код-ревью, а потому что задача этапа spec прямо требует проверить, что контракт поведения и заявленные AC опираются на реальное состояние src/, а не на предположение автора:

  • src/editors/form-kit.ts (colorTile, isLightHex) — подтверждён механизм сплошной заливки плитки (background:${hex} непрозрачно, строка 316) и текущий расчёт цвета подписи только по hex без учёта α (строка 316).
  • src/hp-color-opacity.ts — подтверждён механизм opacity: this.flatSwatch ? 1 : … (строка 852) и что cover-swatch используется ровно в одном месте продукта (src/editors/general-settings-dialog.ts:68), остальные потребители (space-form.ts, room-settings-dialog.ts, marker-dialog.ts, декор) используют только flat-swatch без cover-swatch либо вовсе без обоих атрибутов.
  • src/logic.ts (DEFAULT_FILL_COLORS) — подтверждены значения по умолчанию light_none: a=0, light_on: a=0.18, ровно как в ТЗ.
  • docs/design/600-settings-dialogs/SPEC.md, ACCEPTANCE.md — подтверждена формулировка референса «Плитки цвета» (сетка 3 колонки, подпись поверх свотча, тёмный/белый текст по яркости) и число плиток (10: 3+3+2+2).
  • Тело issue #605 — подтверждён исходный контракт п.3 («одна сплошная цветная поверхность»), который данное ТЗ уточняет для плиток.
  • docs/USER-GUIDE.ru.md:401, docs/USER-GUIDE.md:455 — подтверждена текущая формулировка «Плашка цвета: сплошной цвет», которую не-скоуп ТЗ оставляет как есть, и отсутствие строки «Плитка цвета», которую ТЗ планирует добавить.
  • demo/smoke_dialog_polish_605.mjs — построчно сверены существующие проверки oneSurface, opensPicker, upperHex, названные в AC1/AC3 (строки 35–51): oneSurface действительно проверяет picker.flatSwatch && picker.coverSwatch и getComputedStyle(painted).opacity === '1' для всех .hpf-colortile — то есть ровно то, что ТЗ называет «меняется для плиток на AC1».
  • demo/smoke_color_picker_consumers.mjs, smoke_general_settings_form.mjs, smoke_general_settings.mjs, smoke_dialog_config_parity.mjs, smoke_room_settings_form.mjs, smoke_space_settings_form.mjs, smoke_bg_color.mjs, smoke_room_settings.mjs, smoke_device_settings_form.mjs — проверено наличие файлов, названных в плане автотестов, и выполнен сплошной поиск getComputedStyle(...).opacity / .opacity === по всему demo/*.mjs, чтобы проверить, существует ли сегодня guard, который AC3 называет «остаётся для плашек».
  • scripts/mutation-registry.mjs, docs/reviews/INDEX.md — подтверждено, что для #615 предыдущего раунда нет (это действительно r1), и что готового мутанта рядом с #605 в реестре ещё нет (создать его — часть задачи, не находка).

Гейты (typecheck/test/build) не прогонялись: на этапе ТЗ кода ещё нет, прогон неприменим.

Находки

Medium (в скоупе задачи — чинится в этом же ТЗ)

M1. AC3 называет несуществующую защиту от расползания на плашки цвета.

Файл: тело issue #615, раздел «Критерии приёмки», строка AC3, столбец «чем краснеет»:

«Смежная защита от расползания на плашки: если режим включить по flat-swatch, а не по cover-swatch, проверка плашек в смоке краснеет»

и столбец «чем доказан»:

«…Проверка opacity === '1' меняется для плиток на AC1 … и остаётся для плашек: для Wall fill и фона General вычисленная непрозрачность свотча 1»

Слово «остаётся» и общая формулировка «проверка… краснеет» утверждают, что такая проверка уже существует сегодня и продолжит действовать без изменений. Это проверено чтением, не исполнением: сплошной поиск getComputedStyle(...).opacity и .opacity === по всем demo/smoke_*.mjs (16 файлов с таким паттерном) и отдельно по всем местам, где смоки трогают .hpf-colorfield/.hpf-colorrow (smoke_general_settings_form.mjs:39,65-66, smoke_room_settings_form.mjs:72-78, smoke_space_settings_form.mjs:133-138, smoke_device_settings_form.mjs:98) — ни один не читает вычисленную непрозрачность закрашенного слоя (.swatch) для плашки. Единственная существующая проверка вычисленной непрозрачности свотча — oneSurface в smoke_dialog_polish_605.mjs:35-45 — итерирует по .hpf-colortile, то есть по плиткам, а не по плашкам; hpf-colorfield/hpf-colorrow там не участвуют. В test/ атрибуты flat-swatch/cover-swatch не упоминаются вовсе.

Воспроизведение: grep -rn "getComputedStyle.*opacity\|\.opacity ===" demo/*.mjs → 16 файлов; ни в одном нет ассерта на непрозрачность .hpf-colorfield или .hpf-colorrow. grep -rn "flatSwatch\|flat-swatch\|coverSwatch\|cover-swatch" test/ → пусто.

Почему это находка, а не придирка: ровно тот класс дефекта, о котором предупреждает PROCESS.md §2.7 («аудит v1.71.0-beta.1 нашёл пять контрактов, где тест оставался зелёным на снятой защите») и §7.1 («догадка, записанная как факт — худший вид дефекта: она проходит ревью, потому что выглядит решением»). Если разработчик реализует режим шире, чем через атрибут cover-swatch (например, через общее условие flatSwatch, что и произошло бы при неаккуратном рефакторинге CSS :host([flat-swatch]) .trigger { background: none }), утечка на Wall fill / фон General / Border & name color и т. д. не будет поймана ни одним из перечисленных в плане автотестов смоков — ни новым, ни существующим. Название «смежная защита… краснеет» в ТЗ создаёт ложное чувство, что регресс на плашках уже прикрыт, и код-ревьюер следующего этапа рискует принять это на веру, как разрешает делать сам процесс («документ ревью — не переисполнение, а полнота доказательств»).

Что нужно поправить в ТЗ: заменить «остаётся» на явное указание — либо (а) новая ассерция добавляется тем же PR: например, расширить oneSurface-подобную проверку на один representative-плашку (.hpf-card[data-card="plan"] .hpf-colorrow .hpf-colorfield hp-color-opacity) с проверкой flatSwatch && !coverSwatch и getComputedStyle(painted).opacity === '1' при произвольном α (например, после ввода 40 в .hpf-opacity input, как уже делает smoke_general_settings_form.mjs:65), либо (б) прямо признать, что защиты нет и её создание — часть Плана автотестов AC3, а не «уже существующий» гейт. Это техническое уточнение самой задачи, ответственность за него несёт AC3, поэтому Medium в скоупе: правится в этом же ТЗ без возврата к аналитике.

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

  • Обязательные разделы §7.1 присутствуют все: сценарий, что человек увидит, проблема, скоуп/не-скоуп, контракт поведения, UX, модель данных и миграция, i18n, AC1…AC4 с доказательством и таблицей «чем краснеет», план автотестов, риски, откат, release-артефакты.
  • Продуктовые первые два раздела (сценарий, что видит человек) отвечают на требуемые вопросы без терминов реализации; персона и поверхность по docs/SCOPE.md названы верно (администратор дома, десктоп, редактор General settings — не View, не касается других персон).
  • Описание проблемы фактически точное: код src/hp-color-opacity.ts:852 (opacity: this.flatSwatch ? 1 : …) и src/editors/form-kit.ts:316 (background:${hex} непрозрачно) подтверждают именно тот механизм, который ТЗ называет причиной. Значения по умолчанию light_none a=0, light_on a=0.18 (src/logic.ts:1436-1438) подтверждены буквально.
  • Не-скоуп точно перечисляет реальные места (colorField/colorRow в Wall fill, фоне General, Border & name color, Space/Room, Device) — все они действительно используют flat-swatch без cover-swatch (подтверждено построчным grep по src/editors/*.ts), и текущая формулировка docs/USER-GUIDE.ru.md/.md («Плашка цвета — сплошной цвет») этому не противоречит.
  • AC1 и AC2 — полноценные защитные AC с работоспособной таблицей «чем краснеет»: названы конкретные мутанты (вернуть opacity: 1, убрать шахматку; подпись без учёта α), ожидаемый результат прогона указан («поймано 1 из 1», конкретный входной случай #0d1b2a, a=0). Математика допущения (смешение с ~#d3d3d3 по α и isLightHex от результата) проверена вручную для граничных случаев AC2 — сходится с описанным поведением при α=0, α=1 и для #ffd45c на любом α. Технические решения корректно вынесены в блок «принято предположительно» и не требуют вопроса владельцу — это уже используемое в продукте обозначение (шахматка hp-color-opacity у потребителей без flat-swatch), а не новый UX-контракт.
  • Трек small: комментарий-оценка называет нарушенный критерий trivial («ожидаемое поведение уже зафиксировано» — не выполнен, есть выбор между шахматкой и полосой альфы) и подтверждает все критерии small по отдельности; расхождений с кодом не найдено.
  • Продуктовых вопросов владельцу нет, и по существу ТЗ их действительно не требует: всё разрешаемое — техническое (какой CSS-атрибут переключает режим, как считать цвет подписи), и явно помечено как «принято предположительно, поменять свободно».
  • AC4 (golden) корректно ссылается на предрелизный гейт, а не требует пересъёмки в рамках задачи — соответствует §8 PROCESS.md.

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

  • Реализация ещё не существует — не проверялись сами тесты (они появятся в коде), не прогонялись typecheck/test/build: неприменимо на этапе ТЗ.
  • Не проверялась визуальная читаемость подписи на «пёстрой» шахматке при средних α (40–60 %) — риск назван автором в самом ТЗ («Риски») и оставлен как допустимый, решение по существу не требуется на этом этапе.
  • Полный список золотых кадров, которые изменит правка (AC4, «все кадры General, где плитки попадут в diff»), не пересчитывался вручную по всем сценариям demo/golden/scenarios — это задача исполнителя при хендоффе, не спецификации.

Вердикт

Одна находка Medium в скоупе задачи, High нет. По §2.4/§4 PROCESS.md это жёлтый вердикт: ТЗ возвращается автору на правку AC3 (без нового цикла аналитики) и повторный проход спец-ревью.


Материал раунда

  • Issue: #615, репозиторий Matysh/houseplan-card.
  • Материал: тело issue (раздел ## ТЗ) на момент разбора; заход r1, предыдущих раундов ревью ТЗ для #615 не найдено (docs/reviews/INDEX.md не содержит строк по #615).

Материал раунда

  • Ветка: dev, коммит 9d1d27325d7f — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: 2b3f9bb13865aa81206dba3cd06f7b1cf151dd93
    git log --all --format='%H %T' | grep 2b3f9bb13865
    
  • Тело issue: 160aa133569bbc2145349668472db875bb0326975c789bd2b48b76511222101f
  • Вердикт конвейера: yellow · High 0