mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-02 04:38:55 +00:00
@@ -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), а не заявлены автором. Задача может закрываться.
|
||||
Reference in New Issue
Block a user