mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-05 22:29:05 +00:00
@@ -0,0 +1,202 @@
|
||||
# CODE-REVIEW-597-r1
|
||||
|
||||
Issue: #597 · Этап: код-ревью · Заход: r1 · блокирующих циклов израсходовано 0/2 (лёгкий трек, лимит 2)
|
||||
Материал: `2b89f699618a3fa3e9bafc07dfa71e07a8e19960` (рабочая копия), диапазон `git diff origin/dev...HEAD`
|
||||
Ветка приведена конвейером к актуальному `dev` до ревью (`45dac1c0` → `2b89f699`, +2 коммита `dev` сверху) —
|
||||
разбор проведён полностью (§7.2), не по дельте.
|
||||
|
||||
## Скоуп
|
||||
|
||||
Шаг 2 эпика #591 (шаг 1 — #594, в `dev`). Генератор общего набора контролов
|
||||
(`src/styles/form-kit.styles.ts`) разрезан на пять именованных фрагментов
|
||||
(`formKitCardsCss`, `formKitFocusCss`, `formKitDisabledCss`, `formKitSwitchRowCss`,
|
||||
`formKitSwitchCaptionCss`); редактор боковой панели (`src/summary-panel-editor-style.ts`)
|
||||
подставляет каждый на своё место вместо собственной копии тех же правил.
|
||||
Ни одна строка `src/summary-panel-editor.ts` (разметка) не меняется. Изменение
|
||||
задумано как «ноль видимых изменений» — К1–К5 ТЗ.
|
||||
|
||||
Диапазон диффа (без сгенерированного класса D): `src/styles/form-kit.styles.ts`,
|
||||
`src/summary-panel-editor-style.ts`, `scripts/mutation-registry.mjs`,
|
||||
`test/form-kit.test.mjs`, три новые фикстуры в `test/fixtures/`. Класс A задет —
|
||||
полный флоу обязателен и пройден (ТЗ в теле issue, ревью ТЗ зелёное на r2).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Дешёвые гейты (`typecheck`, `npm test`, `npm run build` со сверкой бандла) на этом
|
||||
SHA уже подтверждены зелёным Validate
|
||||
(https://github.com/Matysh/houseplan-card/actions/runs/35446978510) — не
|
||||
перегонялись. Вместо этого бюджет раунда потрачен на чтение кода и на гейты,
|
||||
которые Validate в этом прогоне **не покрывает**: в нём `smoke`, `golden` и
|
||||
`performance_smoke` стоят `skipped` (обычный push в задачную ветку, не кандидат
|
||||
беты и не PR — эти джобы включаются только на `heavy`).
|
||||
|
||||
| Гейт | Прогнан | Результат |
|
||||
|---|---|---|
|
||||
| `npx tsc --noEmit`, `npm test`, `npm run build` + сверка 3 копий бандла | нет — подтверждено зелёным Validate на этом SHA | success (ссылка выше) |
|
||||
| `npm run bundle:sync` (пересборка + сверка дерева) | да, лично | `dist/houseplan-card.js` побайтово совпал с закоммиченным после чистой пересборки — `git status` пуст |
|
||||
| `npm run bundle:budget` (AC7) | да, лично | initial View 290843/291400, lazy editor 222359/222900, lazy furniture art 16942/17900 — совпало цифра в цифру с хендоффом |
|
||||
| `node scripts/no-new-any.mjs --base origin/dev --head HEAD` | да | «Новых any нет» |
|
||||
| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | да | одна зарегистрированная связь — `smoke_summary_panel_polish.mjs` ← `summaryPanelEditorCss`; остальных совпадений нет (диф — 2 файла, 9 символов на изменённых строках) |
|
||||
| `node demo/smoke_summary_panel.mjs` (AC4) | да, лично | `OK` |
|
||||
| `node demo/smoke_summary_panel_polish.mjs` (AC4) | да, лично | `OK` |
|
||||
| `node demo/smoke_room_settings.mjs` (AC5) | да, лично | `OK` |
|
||||
| `npm run golden:verify` (AC6) | да, лично (в песочнице есть Chromium — у автора не было) | 80/80 сцен `passed`, exit 0 — расхождений ноль |
|
||||
| `node scripts/check-docs.mjs` (обязателен — diff трогает `src/**`) | да, лично | **ERROR: отпечаток скриншотов документации устарел** — см. находку M1 |
|
||||
| `npx tsc -p tsconfig.test.json && node scripts/fix-test-build.mjs && node --test test/form-kit.test.mjs` | да, лично | 5/5 тестов зелёные |
|
||||
| Мутант `form-kit-fragment-drifts-from-panel` (padding 14px→13px) | да, лично, дважды (с восстановлением) | оба теста `#597` краснеют: `AssertionError` на побайтовом сравнении |
|
||||
| Проба M1 ревью ТЗ (литерал фрагмента карточки-группы в блочном комментарии рядом с листом панели) | да, лично | тест 1 краснеет ровно заявленным текстом: `карточка-группа: правило осталось литералом...`; после снятия — снова зелёный |
|
||||
| Проба на стык фрагментов диалогов (лишний `\n` в `sharedCss`) | да, лично | тест 2 (`#597 лист диалогов карточки не изменился ни на байт`) краснеет |
|
||||
| Независимая пересборка фикстур с кода `origin/dev` (не доверяя хендоффу) | да, лично | панель: 14585 байт, побайтово совпало с `test/fixtures/summary-panel-editor.css`; диалоги без переключателя — 1885 байт, с ним — 2399 байт, оба побайтово совпали со своими фикстурами |
|
||||
| `python -m pytest tests_backend -q` | нет | диф не касается `custom_components/**/*.py` |
|
||||
| Perf-профили | нет | не названы в AC, диф не трогает чувствительные пути (`src/iso-*`, `src/live-*`, `src/render-*`) |
|
||||
| `npm run docs:accept -- --identical` (диагностика находки M1) | да, лично (изменения отменены, не коммитились) | «Все 11 кадров попиксельно совпали с закоммиченными: принят только отпечаток исходников» |
|
||||
|
||||
Не гонял полный набор смоков (250 в матрице) — задача узкая (2 файла CSS, один
|
||||
потребитель), `smoke-select` называет одну связь, остальные не относятся к
|
||||
диффу; прогон всей матрицы — предрелизная обязанность, не гейт ревью.
|
||||
|
||||
## Находки
|
||||
|
||||
### M1 (Medium, в скоупе задачи) — устаревший обязательный гейт `check-docs` не закрыт
|
||||
|
||||
`node scripts/check-docs.mjs` на материале ревью красит:
|
||||
```
|
||||
ERROR screenshot source fingerprint is stale; run npm run docs:capture and accept before the beta candidate (#479)
|
||||
```
|
||||
PROCESS.md §8 называет этот гейт безусловно обязательным при любом диффе по
|
||||
`src/**`: «отпечаток скриншотов документации считается по всему `src/**`,
|
||||
поэтому любая правка фронтенда делает его устаревшим... выборка «по diff и AC»
|
||||
здесь не работает — diff всегда попадает, и решать нечего». Диф задачи меняет
|
||||
`src/styles/form-kit.styles.ts` и `src/summary-panel-editor-style.ts` — значит
|
||||
гейт применим и обязан быть закрыт в рамках задачи, а не оставлен следующей.
|
||||
|
||||
Это не гипотетический риск: ровно тот же паттерн уже стоил продукту инцидента —
|
||||
«скриншоты не пересняли в #230 и #234, и `dev` стоял с красным job `docs`, пока
|
||||
это не нашли при следующей задаче (#237)» (PROCESS.md §8). Непосредственный
|
||||
предшественник этой ветки — коммит `fc4d55c7 docs: отпечаток скриншотов после
|
||||
правки комментария рантайма (#593)` — обновил отпечаток сразу после последней
|
||||
правки `src/**` перед этой задачей (`0565c7ad`, #593); других правок `src/**`
|
||||
между `fc4d55c7` и материалом ревью нет, кроме самой задачи #597. То есть
|
||||
именно этот коммит впервые за долгое время сделал отпечаток снова устаревшим
|
||||
и не закрыл это следующим шагом, хотя предыдущая задача сделала это ровно так.
|
||||
|
||||
Проверил, что правка дешёвая и не открывает новый спор с ТЗ: прогнал
|
||||
`npm run docs:accept -- --identical` — «Все 11 кадров попиксельно совпали с
|
||||
закоммиченными: принят только отпечаток исходников» (0 расхождений, как и
|
||||
обещает К1/AC1 задачи — вывод CSS не изменился ни на пиксель). Изменения не
|
||||
коммитил (ревьюер не правит материал); откатил рабочую копию.
|
||||
|
||||
Раздел ТЗ «Release-артефакты» («Документация не меняется») отвечает только за
|
||||
прозу (`docs/*.md`, `USER-GUIDE.ru.md`) и не называет отпечаток скриншотов
|
||||
отдельно — в этом смысле находка и к ТЗ (пропущенный release-артефакт), и к
|
||||
реализации (гейт не прогнан и не закрыт до хендоффа). Ни AC1–AC8, ни хендофф
|
||||
не упоминают `check-docs`, хотя PROCESS.md называет его обязательным именно по
|
||||
факту диффа `src/**`, независимо от AC.
|
||||
|
||||
**Чем краснеет:** сам гейт — `node scripts/check-docs.mjs` без флагов (умолчание
|
||||
`--screenshots=strict`) на материале ревью. В обычном пуше в задачную ветку CI
|
||||
джоб «Предполёт» использует `--screenshots=warn` и поэтому зелёный на этом SHA —
|
||||
это маскирует находку в Validate, но не отменяет требование PROCESS.md §8 и не
|
||||
меняет исход на кандидате беты, где режим строгий.
|
||||
|
||||
**Правка:** `npm run docs:accept -- --identical` и коммит обновлённого
|
||||
`docs/images/screenshots.json` (только отпечаток, без изменения PNG) в этой же
|
||||
задаче — ровно то, что сделал непосредственный предшественник (`fc4d55c7`).
|
||||
|
||||
Находка в скоупе задачи (диф сам её и вызвал) — по §2.7/§7.2 чинится в текущем
|
||||
issue, отдельный issue не заводится.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **К1/AC1 (ноль видимых изменений в листе панели).** Независимо от хендоффа
|
||||
пересобрал `formKitCss`-composition на коде `origin/dev` (через временный
|
||||
откат `src/styles/form-kit.styles.ts` к версии `dev` и компиляцию) и сравнил
|
||||
результат с закоммиченной фикстурой: 14585 байт, побайтовое совпадение.
|
||||
Тест `test/form-kit.test.mjs` подтверждает это же на актуальном коде.
|
||||
- **К2/AC2 (одна копия).** Прочитал `src/summary-panel-editor-style.ts` — ни
|
||||
один из пяти литералов `PANEL_FRAGMENTS` не встречается в файле, только вызовы
|
||||
`formKit*Css(SUMMARY_PANEL_FORM_KIT)`. Мутационная проба (литерал
|
||||
карточки-группы в блочном комментарии рядом с листом) краснит тест ровно
|
||||
словами хендоффа; без правки M1 прежняя проверка (`min-height: 54px`) её бы
|
||||
пропустила — это подтверждено чтением истории обеих находок ревью ТЗ r1/r2.
|
||||
- **К3/AC3 (генератор не меняет смысла листа диалогов).** Пересобрал
|
||||
`formKitCss(CARD_DIALOG_FORM_KIT)` в обеих формах на коде `origin/dev` и
|
||||
сравнил с новыми фикстурами: без переключателя 1885 байт, с ним 2399 —
|
||||
побайтовое совпадение в обоих случаях, независимо от заявленного в хендоффе.
|
||||
Проба с лишним переносом строки на стыке фрагментов (`sharedCss`) краснит
|
||||
именно этот тест — свидетель реален, а не декларативен.
|
||||
- **К4 (граф загрузки).** Ни `formKitCardsCss`/`formKitFocusCss`/... не
|
||||
экспортированы вовне лениво загружаемых модулей за пределы прежних; `git diff
|
||||
--stat` подтверждает изменения только в тех же двух файлах, что и раньше несли
|
||||
этот CSS. `npm run bundle:budget` — initial View / lazy editor / lazy furniture
|
||||
art в пределах прежних потолков, цифры совпали с хендоффом.
|
||||
- **К5 (разметка панели не тронута).** `src/summary-panel-editor.ts` вне диффа
|
||||
(`git diff --name-only -- src/`).
|
||||
- **AC4/AC5 (смоки панели и диалога комнаты).** Прогнал все три смока лично:
|
||||
`smoke_summary_panel.mjs`, `smoke_summary_panel_polish.mjs`,
|
||||
`smoke_room_settings.mjs` — все `OK`.
|
||||
- **AC6 (эталоны golden).** Автор не прогонял (в песочнице нет Chromium) — в
|
||||
этой среде Chromium доступен, прогнал сам: 80/80 сцен `passed`, exit 0.
|
||||
Расхождений нет ни в одной сцене, включая те, что рендерят диалоги на общем
|
||||
наборе контролов (диалог комнаты и т.п.).
|
||||
- **AC7 (бюджет бандла).** Числа совпали цифра в цифру с хендоффом.
|
||||
- **AC8 (полный набор).** `npm test` и `npm run gate:small` подтверждены зелёным
|
||||
Validate на этом SHA; отдельно прогнал целевой файл `test/form-kit.test.mjs`
|
||||
(5/5) и `no-new-any` (чисто).
|
||||
- **Мутант `form-kit-fragment-drifts-from-panel`.** Зарегистрирован в
|
||||
`scripts/mutation-registry.mjs`, гард — сам целевой тест. Применил мутацию
|
||||
(`padding: 14px` → `13px`) дважды независимо — оба раза оба теста `#597`
|
||||
падают с `AssertionError`; без мутации — зелёные. Мутант реален, не
|
||||
декоративен.
|
||||
- **Провенанс коммита.** Один коммит, трейлеры `Issue: #597` и
|
||||
`User-Visible: no` на месте; `User-Visible: no` не требует changelog, диф не
|
||||
трогает пользовательский текст ни в одном из двух changelog-файлов.
|
||||
- **i18n.** Диф не касается i18n-файлов — согласуется с ТЗ («i18n: нет»).
|
||||
- **Классы риска §2.6.** async/данные и права/геометрия — неприменимо (чистый
|
||||
CSS-рефакторинг без сети, без геометрии). Визуал — главный риск, закрыт
|
||||
двойным свидетелем: побайтовым текстовым сравнением (сильнее кадра, ловит
|
||||
перестановку правил, которую пиксель не отличит) и личным прогоном
|
||||
`golden:verify` на полной матрице. Объём/perf — AC7 подтверждён. Host/input —
|
||||
разметка и обработчики не менялись (К5), поведение то же.
|
||||
- **Факт из ТЗ, использованный как опора доказательства («у панели нет
|
||||
golden-кадра»).** Проверил `demo/golden/matrix.mjs` — сцены с `dialog:
|
||||
'summary'` действительно нет. Утверждение не является догадкой.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Полный набор из 250 браузерных смоков — не оправдано диффом (`smoke-select`
|
||||
называет одну прямую связь, диф — 2 CSS-файла с нулевым изменением
|
||||
собранного вывода для панели и диалогов карточки).
|
||||
- `python -m pytest tests_backend` — диф не касается Python.
|
||||
- Performance-профили — не названы в AC, диф не в чувствительных путях
|
||||
(`src/iso-*`, `src/live-*`, `src/render-*`, `houseplan-card.ts`).
|
||||
- Полный HA-харнесс (`fcntl`, недоступен вне Linux+HA) — не относится к CSS.
|
||||
- `process-gate.mjs --issues` — не имеет значения для решения ревьюера, статус
|
||||
и лейблы issue уже видны напрямую через `gh`.
|
||||
- `npm run docs:accept -- --identical` — прогнал только для диагностики находки
|
||||
M1 (подтвердить, что правка дешёвая), результат не коммитил: правку делает
|
||||
автор в задаче.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Жёлтый. Единственная находка (M1, Medium) — в скоупе задачи, вызвана самим
|
||||
диффом и чинится одной командой плюс коммитом отпечатка; отдельный issue не
|
||||
заводится. Все восемь AC доказаны свидетелями, которые я лично проверил на
|
||||
умение краснеть (мутант + две пробы совпали с заявленными в хендоффе текстами
|
||||
ошибок); AC6 закрыт мной вместо автора, у которого не было среды. Технический
|
||||
план задачи (разрезание генератора на фрагменты, подстановка вместо
|
||||
редизайна) корректен и полностью соответствует контракту К1–К5.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/597-panel-on-form-kit`, коммит `2b89f699618a` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `94639a89f4617d5498d0a2c63027db0fa2dab139`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 94639a89f461
|
||||
```
|
||||
- Тело issue: `8a56815128285d56f2c2f9f63eb6d8ae211525202f38e8bdd75159aa7c55412c`
|
||||
- Вердикт конвейера: `yellow` · High 0
|
||||
Reference in New Issue
Block a user