diff --git a/docs/reviews/CODE-REVIEW-266-r1.md b/docs/reviews/CODE-REVIEW-266-r1.md new file mode 100644 index 00000000..87542039 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-266-r1.md @@ -0,0 +1,63 @@ +# 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), а не заявлены автором. Задача может закрываться.