docs: review document for #266

Issue: #266
User-Visible: no

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

@@ -0,0 +1,214 @@
+# SPEC-REVIEW-266-r1
+
+Issue: [#266](https://github.com/Matysh/houseplan-card/issues/266) — «Рефакторинг 3/5: расщепить styles.ts»
+ТЗ: `docs/specs/266-split-styles.md` (коммит `4c93f28e`, ветка `issue/266-split-styles`)
+Этап: spec (PROCESS.md §2.4) · заход r1 · блокирующих циклов израсходовано 0/4
+Вердикт: **жёлтый**
+
+## Скоуп ревью
+
+Класс изменения — A (`src/**`), но задача помечена как чистый refactor
+(tech-debt, User-Visible: no): перенос правил из монолитного `src/styles.ts`
+(3690 строк, один экспорт `cardStyles`) в пять файлов `src/styles/*.styles.ts`
+по поверхностям + сборщик. Продуктового поведения нет и не должно появиться —
+главный заявленный инвариант ТЗ: «пиксели не меняются».
+
+Ревью велось состязательно: без переписки с автором, по тексту ТЗ, телу issue
+и фактическому состоянию кода/гейтов на `dev`.
+
+## Как проверялось
+
+- Прочитаны `docs/SCOPE.md`, `PROCESS.md`, `AGENTS.md` (полностью, включая §7.1,
+  §8, §10.2–10.4).
+- Прочитано тело issue #266 и комментарий аналитики/автора (единственные два
+  комментария, ни одного цикла ревью до этого не было — подтверждает r1).
+- Сверены с кодом фактические числа из ТЗ:
+  - `wc -l src/styles.ts` → 3690 строк — совпадает с текстом ТЗ (§0).
+  - `node -e "import('./demo/golden/matrix.mjs').then(m=>console.log(m.GOLDEN_SCENARIOS.length))"`
+    → 129 сцен — совпадает с AC3.
+  - Потребители `cardStyles`: `houseplan-card.ts`, `hp-device-preview.ts`,
+    `space-card.ts`; `editorSecondaryStyles` подключается вторым элементом
+    массива в `houseplan-card.ts:21973` — совпадает с §1.2.
+  - Грубая (regex-based) бакетизация правил по кластерам из §1.1 показала явный
+    перекос: `plan` ≈ 1300–1400 строк против заявленного лимита ≤ 1200 (AC2) —
+    см. находку L2 ниже, оставлено как наблюдение, не как блокирующее.
+  - Найдены и прочитаны все 18 `@media` блоков `src/styles.ts` (8×
+    `prefers-reduced-motion`, 2× `forced-colors`, 2× `prefers-color-scheme`,
+    4× `max-width`, плюс дубли) и 11 `@keyframes` — см. находку M1.
+  - Проверено покрытие golden этих состояний: `demo/golden/harness.mjs:49` и
+    `demo/golden/run.mjs:607` — golden всегда эмулирует
+    `reducedMotion:'reduce'` и не эмулирует `forced-colors` вовсе.
+  - Проверено покрытие smoke: `demo/smoke_plan_snap_overlay.mjs:354-367`
+    (`forcedColors:'active'`) и `demo/smoke_preloader.mjs:120-121`
+    (`reducedMotion:'reduce'`, фаза «house does not pulse»); `grep` по
+    `test/*.mjs` на `forced-colors`/`prefers-reduced-motion` — только
+    `test/color-picker.test.mjs` (не относится к затронутым блокам) и
+    `test/isometric-contract.test.mjs` (проверяет только наличие класса в
+    разметке, не CSS-блок `forced-colors` на `src/styles.ts:594-601`).
+  - Проверена классификация файла `scripts/dev/styles-diff.mjs` против таблицы
+    классов PROCESS.md §1 — см. находку L1.
+  - `docs/ARCHITECTURE.md:17` содержит дерево с `editor-secondary.styles.ts`,
+    новый каталог `src/styles/` туда естественно вписывается — скоуп «раздел
+    ARCHITECTURE.md о структуре стилей» реалистичен, конфликтов с существующим
+    `src/styles/` нет (каталога пока не существует).
+- Технических тяжёлых гейтов (typecheck/test/build/golden) не гонял: на этапе
+  ревью ТЗ кода изменения ещё нет (диф — один файл `docs/specs/266-*.md`),
+  гонять их не по чему. Это соответствует масштабу этапа spec, не пропуск.
+
+## Находки
+
+### M1 — Medium (в скоупе) — заявленный «объективный критерий» (golden) не видит два реальных CSS-состояния, которые ровно и переносятся этим рефакторингом
+
+`src/styles.ts` содержит 8 блоков `@media (prefers-reduced-motion: reduce)` и 2
+блока `@media (forced-colors: active)` (строки 594–601 — `.iso-*` тени
+изометрии, 1922–1930 — `.plan-snap-line`/`.hidden-wall-line`). §1.3.1 ТЗ
+называет `npm run golden:verify` «главным и объективным критерием» того, что
+пиксели не изменились ни на одном слайсе.
+
+Фактически:
+- `demo/golden/harness.mjs:49` и `demo/golden/run.mjs:607` жёстко фиксируют
+  `reducedMotion: 'reduce'` на **каждом** golden-снимке и никогда не эмулируют
+  `forced-colors`. Значит: (а) состояние «motion не ограничен» (обычный
+  пользователь без предпочтения) вообще никогда не рендерится golden'ом — если
+  слайс случайно утащит правило из-под `@media (prefers-reduced-motion: reduce)`
+  наружу или, наоборот, спрятает обычное правило внутрь этого блока, картинка
+  под форсированным `reduce` не изменится, и golden останется зелёным; (б)
+  `forced-colors: active` не проверяется golden'ом вообще, ни в одном из 129
+  сцен.
+- Единственный другой заявленный инструмент проверки — сверочный скрипт
+  §1.3.5/AC6 — сравнивает «нормализованное множество правил (селектор → текст
+  объявлений)» без упоминания контекста вложенности. Правило, переехавшее из
+  одного `@media`-блока в другой (или потерявшее обёртку `@media`/`@supports`
+  при копировании), даст тот же «селектор → текст объявлений», то есть
+  инструмент тоже не заметит подмену.
+- Существующие browser-smoke закрывают это лишь частично: `smoke_plan_snap_overlay.mjs`
+  проверяет именно `forced-colors` для `.plan-snap-line`/`.hidden-wall-line`
+  (тот самый блок на 1922 строке), `smoke_preloader.mjs` проверяет, что
+  `hp-boot-pulse` не проигрывается при `reducedMotion:'reduce'` — но не
+  проверяет обратное (что анимация ЕСТЬ без этого предпочтения), и не
+  затрагивает остальные 7 `prefers-reduced-motion`-блоков. Блок `forced-colors`
+  для `.iso-*` теней (594–601) не покрыт вообще ничем — ни golden, ни smoke, ни
+  unit (`test/isometric-contract.test.mjs:25` проверяет только наличие класса
+  в разметке, не содержимое CSS-блока).
+- План тестов ТЗ (§7) не называет ни один browser-smoke вообще, хотя AGENTS.md
+  («по необходимости... браузерные смоки») и PROCESS.md §8 требуют явно
+  перечислить относящиеся к диффу смоки и решение по каждому.
+
+**Сценарий отказа:** на слайсе `plan` (4-й по плану) автор переносит
+`.plan-snap-line`/`.hidden-wall-line` в `plan.styles.ts`, но обёртка
+`@media (forced-colors: active) { … }` теряется или переезжает в другой файл
+без содержимого. В Windows High Contrast снап-оверлей становится нечитаемым
+(регрессия ровно того класса, что уже случался — #234 про пиксельные
+рассинхроны, найденные не тем гейтом, который должен был их найти).
+`golden:verify` зелёный (forced-colors не эмулируется), сверочный инструмент
+зелёный (текст правила не изменился, только его обёртка), и только если
+кто-то вручную вспомнит прогнать `smoke_plan_snap_overlay.mjs`, дефект
+всплывёт — а по плану §7 прогонять его не обязаны.
+
+**Почему в скоупе, не блокер целиком:** сама конструкция рефакторинга не
+порочна, и исправление дёшево и целиком укладывается в уже запланированный
+инструментарий:
+1. Расширить ключ нормализации сверочного инструмента (§1.3.5/AC6) до полного
+   пути вложенности: селектор + весь стек охватывающих `@media`/`@supports`/
+   `@keyframes`-условий, а не только «селектор → объявления».
+2. Явно назвать в §7 два существующих смока
+   (`demo/smoke_plan_snap_overlay.mjs`, `demo/smoke_preloader.mjs`) как
+   обязательные к прогону на слайсах `plan`/`base`, раз они трогают именно
+   переносимые `forced-colors`/`prefers-reduced-motion` правила.
+3. Добавить недостающую проверку (unit или расширение существующего смока) для
+   блока `.iso-*` `forced-colors` (594–601) до его переноса, либо явно
+   зафиксировать «проверено чтением» с указанием ревьюеру, что делать при
+   переносе этого конкретного блока.
+
+Без этого исправления AC3 («golden 129/129 — доказательство пиксельной
+неизменности») фактически доказывает меньше, чем заявлено в §1.3.1, а именно
+для тех самых мест, где перенос селекторов между файлами наиболее рискован.
+
+### L1 — Low — неверная классификация вспомогательного скрипта
+
+§1.3.4 (примечание к AC6) называет `scripts/dev/styles-diff.mjs` «класс C»
+(документация). По таблице PROCESS.md §1 путь `scripts/**` целиком относится к
+**классу B** («Гейты и инструменты»), не к C. Практического следствия нет —
+файл уже привязан к issue #266 при любой классификации, — но формулировка в
+самом ТЗ противоречит канону. Правится одним словом при следующей правке ТЗ;
+не считаю нужным делать отдельный цикл только за это, фиксирую с записью.
+
+### L2 — Low (наблюдение, не требует правки ТЗ) — лимит «≤ 1200 строк» на файл (AC2) может не выполниться для `plan.styles.ts`
+
+Грубая эвристическая бакетизация правил `src/styles.ts` по кластерам §1.1
+(regex по началу правила, накопление до следующего распознанного селектора)
+дала: `plan` ≈ 1382, `devices` ≈ 980, `dialogs` ≈ 865, `chrome` ≈ 407, `base` —
+заведомо недооценён эвристикой. Это неточный инструмент (не учитывает все
+переключения кластеров внутри `@media`), но сигнал достаточно сильный:
+поверхность `plan` (сцена, стены, снап, декор-слой, компас, измерения,
+`.room*`, `.bdframe`/`.dtframe`) — самая большая и самая связная, и её слайс
+идёт четвёртым, то есть после того как более мелкие поверхности уже вычищены.
+Если по факту `plan.styles.ts` выйдет за 1200 строк, AC2 не докажется буквально
+— это будет находка код-ревью, а не крах архитектуры ТЗ. Оставляю как
+наблюдение: автор может заранее заложить одно из двух — либо явно
+предусмотреть в ТЗ право одного файла (`plan`) на больший потолок с
+обоснованием, либо принять как есть и решить вопрос на слайсе 4. Не
+блокирую ревью этим пунктом.
+
+## Что проверено и корректно
+
+- Все семь обязательных продуктовых/технических подразделов §7.1
+  присутствуют в той или иной форме: сценарий (§0), контракт (§1), скоуп/не-
+  скоуп (§2), UX/данные/i18n/touch — единой строкой (§3, адекватно для
+  чистого CSS-рефакторинга с нулевым видимым эффектом), риски (§4),
+  release-артефакты (§5), AC с доказательством (§6), план тестов (§7), откат
+  (§8). Формулировка «что человек увидит» — «рендер обязан быть пиксельно
+  идентичен» — честный ответ для задачи без пользовательского эффекта, не
+  замаскированная техническая работа под видом продуктовой; прецедент такого
+  же лаконичного оформления для чистого рефакторинга есть в
+  `docs/specs/034-frontend-decomposition.md` (родительский зонтик, этап 5
+  которого — этот issue).
+- Числа в ТЗ (3690 строк, 129 golden-сцен, состав потребителей `cardStyles`,
+  порядок подключения `editorSecondaryStyles`) сверены с кодом дерева на
+  момент ревью и совпадают буквально — никакой догадки, выданной за факт, не
+  обнаружено.
+- Инвариант «без дубликатов» (§1.3.2) и «фиксированный порядок склейки»
+  (§1.3.3) — оба однозначно проверяемые юнитом, доказательство названо.
+- AC1 (`styles.ts` ≤ 40 строк, только импорты+экспорт) реалистичен и легко
+  проверяем ревью кода.
+- Порядок слайсов (chrome → dialogs → devices → plan → base) обоснован в теле
+  issue и ТЗ («от изолированной поверхности к связной») и подтверждён
+  наблюдением: у тулбаров/чекбоксов кластеров существенно меньше пересечений
+  с остальными зонами, чем у сцены плана.
+- Риск «скрытая зависимость каскада» (§4.1) назван прямо и с механизмом
+  устранения (перенос конфликтующего правила в `base` на прежнюю
+  относительную позицию) — это корректно закрывает случай межзонного
+  конфликта specificity внутри одного и того же слайса; отдельно не закрывает
+  M1 (это про типы состояний, не про порядок).
+- Release-артефакты: `docs/ARCHITECTURE.md` — реальный кандидат на правку
+  (дерево модулей уже документирует `editor-secondary.styles.ts` по такому же
+  шаблону), CHANGELOG обоснованно не трогается (User-Visible: no, инвариант
+  «пиксели не меняются» делает это законным, а не обходом правила).
+- Технический вопрос (порядок промежуточной склейки на слайсах 1–4, до того
+  как все пять файлов существуют) не эскалирован владельцу и решён ревью:
+  относится к «всё, чего пользователь не наблюдает» (PROCESS.md §7.1), автор
+  и ревьюер вправе договориться сами. Договорённость: относительный порядок
+  внутри промежуточного массива на каждом слайсе сохраняет исходную позицию
+  ещё не вынесенных правил, что и так неявно требуется инвариантом 1 и
+  ловится golden на каждом шаге (кроме состояний из M1).
+
+## Чего не проверял
+
+- Тяжёлые гейты (`npm test`, `npm run build`, `golden:verify`,
+  `model-invariants`, browser-smoke целиком) — не гонял: на этапе ревью ТЗ
+  продуктового кода ещё нет, гонять их не по чему; это не пропуск, а
+  соразмерность этапу (PROCESS.md §8).
+- Точный построчный состав будущих пяти файлов — не пересчитывал вручную
+  построчно, кроме грубой эвристической оценки для L2; финальное распределение
+  проверяется код-ревью по факту переноса.
+- Мутационный гейт (`scripts/mutation-gate.mjs`) — существует в проекте и
+  запускается стабильным релизом, не гейтом ревью; ТЗ корректно ссылается на
+  него в §7 как на «якоря вне styles.ts», не требуя отдельного прогона здесь.
+
+## Унаследовано из предыдущих раундов
+
+Не применимо — это первый заход (r1), предыдущих раундов нет.
+
+---
+
+**Вердикт: жёлтый · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 1 → в задаче**