houseplan-card/docs/reviews/CODE-REVIEW-266-r1.md

CODE-REVIEW-266-r1

Issue: #266 — «Рефакторинг 3/5: расщепить styles.ts — 3 648 строк в одном шаблоне css``» Этап: code (PROCESS.md §2.7). Заход: r1 · блокирующих циклов израсходовано 0 из 4. Материал: git log --oneline origin/dev..HEAD, git diff origin/dev...HEAD на SHA 0d62a48b (HEAD detached, ветка origin/issue/266-split-styles). ТЗ: docs/specs/266-split-styles.md, ревизия 3 (SPEC-REVIEW-266-r3 — зелёный, High 0 / Medium 0).

Скоуп

Перенос содержимого src/styles.ts (3 690 строк, один экспорт cardStyles = css\...`) в пять файлов поверхностей src/styles/{base,plan,devices,chrome,dialogs}.styles.ts+ сборщик; юниты инвариантов; сверочный инструментscripts/dev/styles-diff.mjs; генератор слайсов scripts/dev/styles-split.mjs; правки контрактных тестов, греппящих CSS-исходник, на общий хелпер; переадресация 5 мутантных якорей; docs/ARCHITECTURE.md`. Refactor-only — ни один селектор/объявление не должны были измениться (инвариант §1.3.5 ТЗ).

Диапазон коммитов: cbdc2489 (инструменты) → 5 слайсов (c684f178 chrome, 7d094fd2 dialogs, bf52524d devices, 55f7126a plan, 0d9320fe base+сборщик) → 3a49b142/0d62a48b (docs, не код).

Изменённые файлы src/**: src/styles.ts, 5 новых src/styles/*.styles.ts — ровно то, что заявлено скоупом ТЗ. Продуктовый код (houseplan-card.ts, space-card.ts, hp-device-preview и т.д.) не тронут — потребители cardStyles действительно не менялись, как и обещало ТЗ §1.2.

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

Полный разбор (не по дельте): это первый заход код-ревью для #266, дельты предыдущего код-раунда не существует. Все гейты ниже прогнаны лично на дереве 0d62a48b, не приняты на слово автора.

Прогнано:

• npx tsc --noEmit — зелёный, без ошибок.
• npm test — 1319 тестов, 1318 pass / 0 fail / 1 skip (существовавший skip, не новый).
• npm run build (tsc --noEmit && rollup -c) — зелёный; node scripts/bundle-sync.mjs — три копии бандла (dist/houseplan-card.js, custom_components/houseplan/frontend/houseplan-card.js, demo/srv/assets/houseplan-card.js) байтово идентичны (md5 7f4754ba582a8ce4c29614a4c8c119e7), git status по этим путям чист после пересборки — значит закоммиченные копии уже соответствовали билду, дрейфа нет.
• Размер бандла: origin/dev dist/houseplan-card.js = 1 291 440 байт, HEAD = 1 291 458 байт → +18 байт. AC7 (≤1 КБ) выполнен с большим запасом; цифра автора (+18 байт) подтверждена независимо.
• node scripts/check-docs.mjs — зелёный («7 files, 10 external links»); нужен, так как diff трогает src/**. Отдельно сверил docs/images/screenshots.json: sourceFingerprint/sourceSha256 изменились (ожидаемо — src изменился), но каждый imageSha256 в диффе — тот же самый, что на origin/dev. Это независимое подтверждение «пиксели не изменились» на уровне артефакта документации, отдельное от golden.
• npm run golden:verify (129 сцен) — прогнан дважды (первый прогон через tail обрезал заголовок лога, перезапустил без обрезки): 129/129 passed, 0 failed. git status --short demo/golden/baselines/ — пусто, эталоны не переприняты. Это главный критерий ТЗ (§1.3.1) и он подтверждён исполнением, не докладом автора.
• Сверочный инструмент scripts/dev/styles-diff.mjs до/после — прогнан лично на git show origin/dev:src/styles.ts и на пяти новых файлах: diff before.json after.json → пустой. Это самое сильное доступное доказательство refactor-only (нормализованное множество правил с ключом «полный путь вложенности @media/@supports + селектор» побайтово совпало), сильнее, чем «ревью диффа» из AC6.
• grep числа AC2 и AC6a лично: wc -l файлов поверхностей = 248/381/540/1180/1359, сборщик 19, сумма 3727 ≤ 3690×1.05=3874.5, каждый файл ≤1650 — сходится. @media (forced-colors: active) — 2 блока (оба в plan.styles.ts, строки 314 и 1189), @media (prefers-reduced-motion: reduce) — 10 блоков в сумме по пяти файлам. Совпадает и с ТЗ, и с origin/dev:src/styles.ts (те же 2/10 в исходнике).
• Обязательные по ТЗ §1.3.6/§7 смоки на слайсах plan/base: demo/smoke_plan_snap_overlay.mjs — OK, ключевая проверка forcedColorsStayReadable: true присутствует и зелёная; demo/smoke_preloader.mjs — OK, reducedMotionStaticHouse: 'none' подтверждает, что animation-name действительно гасится под prefers-reduced-motion: reduce после переноса.
• node scripts/smoke-select.mjs --base origin/dev --head HEAD: 6 файлов src/** / 8 символов на изменённых строках → 4 «прямых совпадения» (smoke_decor, smoke_furniture, smoke_render_parity, smoke_sun_rim). Проверил, откуда взялись совпадения: FURNITURE и RAY_FADE_MS — это текст внутри комментариев (docs/FURNITURE.md §3/§4/§6, «RAY_FADE_MS в src/sun.ts должен совпадать» — сам src/sun.ts не тронут), а не код, использующий эти символы, — инструмент честно совпал по тексту строки, а не по семантике. cardStyles — реальное прямое совпадение (это и есть переименованный контракт сборщика), поэтому smoke_render_parity.mjs прогнан отдельно — OK. smoke_decor/smoke_furniture/smoke_sun_rim не прогонял: golden уже покрывает decor/glow/sun пиксельно в 129 сценах (decor-over-opaque-hover-light, decor-over-glow-base-dark, lighting-glow-sun-dark, lighting-custom-glow-* и др. — все зелёные), а сами смоки тестируют JS-поведение (расчёт цвета/geometry furniture, тайминг sun-rim), которое в этом diff не менялось. Записываю решение явно: не прогонял, основание — false-positive по тексту комментария плюс независимое golden-покрытие той же поверхности.
• git diff вспомогательных файлов вручную: scripts/mutation-gate.mjs (5 якорей переадресованы на верные новые файлы: 2 → base.styles.ts — :host([data-pointer-hover]) .dev:hover/.dev:not(.unavail):hover; 3 → devices.styles.ts — .valtext, --dev-size, border-radius), scripts/fix-test-build.mjs (точка в имени модуля styles/base.styles теперь не путается с расширением), tsconfig.test.json (новые файлы добавлены в include), test/styles-source.mjs (новый хелпер), 5 контрактных тестов переведены на него. Все эти файлы участвуют в npm test, который прошёл целиком.
• docs/ARCHITECTURE.md — новый раздел «Styles (#266)» корректно описывает сборщик, порядок каскада, владение поверхностями и инструментарий; не расходится с кодом.
• Трейлеры: во всех 11 коммитах диапазона — Issue: #266, User-Visible: no. CHANGELOG (docs/CHANGELOG.md, docs/CHANGELOG.ru.md) не тронут — согласовано с User-Visible: no (пиксели идентичны, внешний контракт cardStyles не менялся).

Не прогонял (и почему):

• Полный набор demo/smoke_*.mjs (190 файлов) — задача не задевает всё дерево смоков, только CSS-каскад; выбор по smoke-select.mjs + два обязательных по ТЗ смока достаточны и соразмерны.
• npm run invariants (геометрические инварианты #254) — diff не трогает рёбра комнат, толщину, layout, marker.space, open_spans ни в каком файле; это CSS-only рефакторинг, гейт неприменим.
• python -m pytest tests_backend -q — custom_components/**/*.py не тронут.
• Performance-профили — не названы в AC, чувствительные к перфу пути (рендер-цикл, resize) не изменены (только статические CSS-правила переставлены между файлами).

Находки

Нет ни одной находки уровня High или Medium. Реализация соответствует ТЗ ревизии 3 буквально, и главные риски ТЗ (§4: скрытая зависимость каскада, тихая правка при переносе, рост бандла) закрыты объективными инструментами, которые я прогнал лично, а не приняты со слов автора.

Low-наблюдение, не блокирует: сообщение слайса bf52524d («devices») говорит «npm test 1315/0», слайс 55f7126a («plan») тоже «npm test 1315/0», финальный слайс 0d9320fe не называет итоговое число тестов явно (просто «npm test 1318/0» в сводном комментарии владельца в issue) — небольшая небрежность в промежуточных числах коммит-сообщений (1315 на двух разных слайсах подряд, хотя между ними добавлялись файлы), но итоговое число проверено мной напрямую (npm test на HEAD → 1318/0) и совпадает с финальной сводкой владельца. Не требует правки — не влияет на приёмку.

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

• AC1 (styles.ts ≤ 40 строк) — 19 строк, только импорты/комментарий контракта/экспорт. ✓
• AC2 (пороги размеров файлов) — арифметика сходится на фактическом коде, не на прототипе. ✓
• AC3 (golden 129/129, эталоны не переприняты) — исполнено лично, зелёное, git status чист. ✓
• AC4 (непересечение селекторов, юнит) — единственное исключение (\.dev:focus-visible base∩devices) обосновано в тесте и соответствует §1.1 ТЗ (кросс-поверхностная группа :host(...) .dev:hover, .dev:focus-visible — двум разным правилам с одинаковым текстом селектора на разных scope). ✓
• AC5 (порядок склейки сборщика, юнит) — [baseStyles, planStyles, devicesStyles, chromeStyles, dialogsStyles], зафиксировано и тестом, и комментарием-контрактом в styles.ts. ✓
• AC6/AC6a (refactor-only + медиа-обёртки) — подтверждено сильнейшим доступным способом: пустой diff сверочного инструмента до/после, посчитанный мной лично, плюс независимая проверка счётчиков 2 forced-colors / 10 reduced-motion. ✓
• AC7 (бандл ≤ 1 КБ) — +18 байт, три копии синхронны. ✓
• AC8 (npm test/build/check-docs зелёные) — все три подтверждены исполнением. ✓
• Дисциплина «тест умеет падать»: test/styles-split.test.mjs — тест непересечения содержит реальный список allowed-исключений (изменение любого клэша ломает assert.deepEqual), тест медиа-обёрток сравнивает точные числа (2/10) — оба способны упасть на регрессии, не тавтологичны.
• Golden покрывает decor/glow/sun/junction/dialog поверхности напрямую пиксельно — независимое от styles-diff доказательство отсутствия визуальных регрессий.
• Процессное замечание из SPEC-REVIEW-266-r3 (код в ветке раньше статуса S4) уже заведено отдельным issue (#311, вне скоупа #266) — не переоткрываю здесь.

Вердикт

Зелёный. High 0, Medium 0. Все AC доказаны исполнением гейтов лично ревьюером (golden, styles-diff, tsc, npm test, build+bundle-sync, check-docs, два обязательных смока, smoke_render_parity), а не заявлены автором. Задача может закрываться.