mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-28 19:01:34 +00:00
@@ -0,0 +1,207 @@
|
||||
# CODE-REVIEW-597-r2
|
||||
|
||||
Issue: #597 · Этап: код-ревью · Заход: r2 · блокирующих циклов израсходовано 1/2 (лёгкий трек, лимит 2)
|
||||
Материал: `57817b3c91c87c15afa62b9ee2602db2c3b244fc` (рабочая копия, `git rev-parse HEAD`), диапазон `git diff origin/dev...HEAD`
|
||||
|
||||
Ветка приведена конвейером к актуальному `dev` **второй раз**: r1 разбирал материал
|
||||
`2b89f699618a` (сам приведённый к `dev` от `45dac1c0`, +2 коммита `dev`); после
|
||||
r1 автор закрыл M1 в коммите, который при повторном приведении к `dev` получил
|
||||
новый SHA `57817b3c` (пайплайн переставил его поверх ещё двух коммитов dev —
|
||||
`48c198ff`/`ac1d25a0`, оба документные ревью #598, к файлам #597 отношения не
|
||||
имеющие). `2b89f699`/`94639a89f4617d…` не резолвятся ни как объект, ни через
|
||||
`git log --all --format='%H %T'` — это ожидаемо после ребейза (§2.10, «SHA не
|
||||
резолвится — не находка»), не находка.
|
||||
|
||||
**Разбор проведён ПОЛНОСТЬЮ, не по дельте.** Основание — прямое указание §7.2:
|
||||
после ребейза на ушедший вперёд `dev` это другой код. Все восемь AC и К1–К5
|
||||
перепроверены мной лично, независимо от хендоффа и от r1 (см. таблицу ниже) —
|
||||
не переиспользовано ни одно измерение из прежних документов без собственного
|
||||
прогона.
|
||||
|
||||
## Скоуп
|
||||
|
||||
Шаг 2 эпика #591 (шаг 1 — #594, в `dev`). Генератор общего набора контролов
|
||||
(`src/styles/form-kit.styles.ts`) разрезан на пять именованных фрагментов;
|
||||
редактор боковой панели (`src/summary-panel-editor-style.ts`) подставляет
|
||||
каждый на своё место вместо собственной копии тех же правил. Контракт —
|
||||
«ноль видимых изменений» (К1–К5 ТЗ, ревью ТЗ зелёное на r2,
|
||||
`SPEC-REVIEW-597-r2`).
|
||||
|
||||
Диапазон диффа (класс A/B, без сгенерированного класса D):
|
||||
`src/styles/form-kit.styles.ts`, `src/summary-panel-editor-style.ts`,
|
||||
`scripts/mutation-registry.mjs`, `test/form-kit.test.mjs`, три новые фикстуры в
|
||||
`test/fixtures/`. Плюс класс C: `docs/reviews/CODE-REVIEW-597-r1.md` (документ
|
||||
r1, уже закоммичен) и `docs/images/screenshots.json` (закрытие M1). Проверено
|
||||
`git diff origin/dev...HEAD --name-only` без фильтра — список выше исчерпывающий,
|
||||
за вычетом сгенерированных `dist/**` и `custom_components/houseplan/frontend/**`.
|
||||
|
||||
Три коммита в диапазоне (`node scripts/process-gate.mjs`: «коммитов 3», гейт
|
||||
пройден): `b28907f6` (реализация), `27c1c891` (докладной документ r1, докоммичен
|
||||
раньше самим конвейером ревью), `57817b3c` (закрытие M1). Трейлеры `Issue: #597`
|
||||
и `User-Visible: no` на месте во всех трёх; `User-Visible: no` согласуется с ТЗ
|
||||
(«changelog не требуется: вид не меняется»), правок в changelog в диффе нет —
|
||||
корректно.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Дешёвые гейты подтверждены зелёным Validate на этом SHA
|
||||
(https://github.com/Matysh/houseplan-card/actions/runs/35448407081) — этого
|
||||
достаточно по условию задачи, но поскольку разбор полный, я всё равно прогнал
|
||||
их лично для независимого подтверждения (без специального доверия к прогону
|
||||
CI на не-дельта раунде).
|
||||
|
||||
| Гейт | Прогнан | Результат |
|
||||
|---|---|---|
|
||||
| `npx tsc --noEmit` | да, лично | exit 0, без ошибок |
|
||||
| `npm test` | да, лично | 2789 тестов, 2788 pass, 0 fail, 1 skipped — совпало цифра в цифру с хендоффом |
|
||||
| `node --test --test-name-pattern="#597" test/form-kit.test.mjs` | да, лично | 2/2 зелёные |
|
||||
| `npm run build` + `npm run bundle:sync` + сверка 3 копий бандла | да, лично | `cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js` — побайтовое совпадение; `git status` после `bundle:sync` пуст |
|
||||
| Мутант `form-kit-fragment-drifts-from-panel` (`padding: 14px→13px`) | да, лично, дважды (с восстановлением) | оба теста `#597` падают; после отката — снова зелёные |
|
||||
| Проба AC2 (литерал фрагмента карточки-группы TS-комментарием рядом с листом, вне шаблонной строки) | да, лично | тест 1 краснеет: `AssertionError`, `expected: true, actual: false` на строке проверки `PANEL_FRAGMENTS`; после отката — зелёный |
|
||||
| Проба AC3 (лишний `\n` на стыке `formKitCardsCss`/`formKitFocusCss` в `sharedCss`) | да, лично | тест 2 (`#597 лист диалогов карточки не изменился ни на байт`) краснеет; после отката — зелёный |
|
||||
| `node demo/smoke_summary_panel.mjs` (AC4) | да, лично (после `bundle:sync`) | `OK` |
|
||||
| `node demo/smoke_summary_panel_polish.mjs` (AC4) | да, лично | `OK` |
|
||||
| `node demo/smoke_room_settings.mjs` (AC5) | да, лично | `OK` |
|
||||
| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | да | 1 зарегистрированная связь — `smoke_summary_panel_polish.mjs` ← `summaryPanelEditorCss`; диф — 2 файла, 9 символов проекта на изменённых строках. Полный набор (250) не оправдан: узкий диф, ноль изменений собранного вывода |
|
||||
| `npm run golden:verify` (AC6) | да, лично | 80/80 сцен `passed`, exit 0, расхождений ноль (Chromium в песочнице — `151.0.7922.34`, тот же, что закреплён в `screenshots.json`) |
|
||||
| `node scripts/check-docs.mjs` | да, лично | `Documentation checks passed (7 files, 12 external links)` — зелёный **после** закрытия M1 в `57817b3c` |
|
||||
| `npm run bundle:budget` (AC7) | да, лично | initial View 290843/291400 (±2000), lazy editor 222359/222900, lazy furniture art 16942/17900 — совпало цифра в цифру с хендоффом и r1 |
|
||||
| `node scripts/no-new-any.mjs --base origin/dev --head HEAD` | да | «Новых any нет» (71 добавленная строка в 2 файлах, 9 дословно перенесены) |
|
||||
| `node scripts/process-gate.mjs` | да | «гейт пройден, предупреждений 0»; проверка статуса issue пропущена (нет `--issues`/`gh` в этой сессии — не влияет на решение ревьюера, статус issue виден напрямую) |
|
||||
| `demo/golden/matrix.mjs` — сцена `dialog: 'summary'` | да, чтением | подтверждено: `grep -c "dialog: 'summary'" demo/golden/matrix.mjs` = 0. Опора ТЗ («у панели нет golden-кадра, замена — побайтовое сравнение») не является догадкой |
|
||||
| `python -m pytest tests_backend -q` | нет | диф не касается `custom_components/**/*.py` |
|
||||
| Perf-профили | нет | не названы в AC, диф не в чувствительных путях (`src/iso-*`, `src/live-*`, `src/render-*`) |
|
||||
| Полный HA-харнесс | нет | не относится к CSS-рефакторингу |
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка r1 | Чем закрыта | Где это видно |
|
||||
|---|---|---|
|
||||
| **M1** (Medium, в скоупе) — `node scripts/check-docs.mjs` красил: отпечаток скриншотов документации устарел, диф трогает `src/**` | Коммит `57817b3c`: `npm run docs:accept -- --identical` пересчитал `sourceFingerprint`/`sourceSha256` в `docs/images/screenshots.json`, PNG не тронуты | Я лично прогнал `node scripts/check-docs.mjs` на материале r2 — зелёный (см. таблицу выше). Сверил `git diff origin/dev...HEAD -- docs/images/screenshots.json`: меняются только `sourceFingerprint`/`sourceSha256` (`047e917c…` → `f561ec99…`), все `imageSha256` — без изменений, то есть пиксели действительно не сдвинулись, как и обещало К1 |
|
||||
|
||||
Других находок в r1 не было (High: 0, Medium: 1, Low: 0).
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Формально — ничего: этот раунд разобран полностью (§7.2, ребейз на ушедший
|
||||
вперёд `dev`), поэтому все AC и оба К перепроверены заново моими собственными
|
||||
прогонами и пробами, а не приняты по документу r1 или по хендоффу автора. Там,
|
||||
где мой результат совпал с числами r1/хендоффа (14585 байт листа панели,
|
||||
1885/2399 байт листа диалогов, 290843/222359/16942 в бюджете бандла), это
|
||||
указано как независимое совпадение, а не как унаследованный вывод — цифры
|
||||
пересчитаны мной на этом SHA лично, а не скопированы.
|
||||
|
||||
Единственное, что не переоткрывается по существу, — сам факт, что r1 нашёл
|
||||
только M1 и что вопрос был закрыт ровно предложенным способом (раздел выше);
|
||||
техническое обоснование самого плана (разрез на пять фрагментов, причина, по
|
||||
которой прямой вызов `formKitCss` из панели невозможен) я перечитал и проверил
|
||||
чтением заново, а не принял на веру.
|
||||
|
||||
## Находки
|
||||
|
||||
Нет. High: 0, Medium: 0, Low: 0.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
**Защитные AC — таблица «чем краснеет» (§2.7):**
|
||||
|
||||
| AC/К | Чем доказан | Чем краснеет (лично воспроизведено) |
|
||||
|---|---|---|
|
||||
| К1/AC1 — лист панели совпадает побайтово | `test/form-kit.test.mjs`: `#597 собранный лист панели совпадает с замороженным побайтово` против `test/fixtures/summary-panel-editor.css` | Мутант `form-kit-fragment-drifts-from-panel` (`padding: 14px→13px` в `formKitCardsCss`) — тест падает `AssertionError` на сравнении длины/текста; после отката — зелёный |
|
||||
| К2/AC2 — ни один из пяти фрагментов не остаётся литералом в исходнике панели | тот же тест: цикл `for (const [name, fragment] of Object.entries(PANEL_FRAGMENTS))` по `src/summary-panel-editor-style.ts` | Проба: текст фрагмента карточки-группы, вставленный TS-комментарием рядом с листом (вне шаблонной строки, эмитируемый CSS не меняется) — тест падает на цикле именно с этим фрагментом; без изменения — зелёный. Старая проверка (`min-height: 54px`) такую пробу пропустила бы — подтверждено в r1 и переподтверждено мной на этом SHA |
|
||||
| К3/AC3 — лист диалогов карточки не меняется ни на байт, в обеих формах | `test/form-kit.test.mjs`: `#597 лист диалогов карточки не изменился ни на байт` против `form-kit-card-dialog.css` / `…-with-switch.css` | Проба: лишний `\n` на стыке `formKitCardsCss`/`formKitFocusCss` внутри `sharedCss` — тест падает; после отката — зелёный |
|
||||
|
||||
AC4–AC8 доказаны прямыми прогонами (не мутационные защиты в терминах §2.7, а
|
||||
регрессионные гейты, где свидетель — сам гейт): смоки `smoke_summary_panel.mjs`
|
||||
и `smoke_summary_panel_polish.mjs` (AC4), `smoke_room_settings.mjs` (AC5) — все
|
||||
`OK` на пересобранном после `bundle:sync` дереве; `golden:verify` — 80/80,
|
||||
расхождений ноль (AC6); `bundle:budget` — все три числа внутри потолков #593
|
||||
(AC7); `npm test`/`process-gate` — зелёные (AC8).
|
||||
|
||||
**К4 (граф загрузки).** `src/summary-panel-editor-style.ts` теперь импортирует
|
||||
`form-kit.styles.ts`, который уже был частью ленивого редакторского графа
|
||||
(`src/editors/form-kit.ts` — диалоги карточки). Новых точек входа не добавлено:
|
||||
`git diff --name-only -- src/` содержит только эти два файла. `bundle:budget`
|
||||
подтверждает эмпирически — initial View не выросла (290843, тот же потолок, что
|
||||
и до задачи).
|
||||
|
||||
**К5 (разметка панели не тронута).** `src/summary-panel-editor.ts` вне диффа —
|
||||
проверено `git diff origin/dev...HEAD --name-only -- 'src/*'`, список = ровно
|
||||
`src/styles/form-kit.styles.ts` и `src/summary-panel-editor-style.ts`.
|
||||
|
||||
**Провенанс и трейлеры.** 3 коммита, у каждого `Issue: #597` и
|
||||
`User-Visible: no`; `process-gate.mjs` подтверждает 0 предупреждений. Changelog
|
||||
не тронут — согласуется с `User-Visible: no` и разделом ТЗ «Release-артефакты».
|
||||
|
||||
**Классы риска §2.6.** async/данные и права/геометрия — не применимо (чистый
|
||||
CSS-текст без сети и геометрии, совпадает с самооценкой ТЗ). Визуал — главный
|
||||
риск, закрыт двойным свидетелем: побайтовым текстовым сравнением (сильнее
|
||||
кадра — ловит перестановку правил, которую пиксель не отличит) и личным
|
||||
прогоном `golden:verify` на полной матрице (побеждает риск того, что где-то
|
||||
всё-таки есть кадр с диалогом на общем наборе, который я мог пропустить
|
||||
чтением). Объём/perf — AC7 подтверждён измерением. Host/input — разметка и
|
||||
обработчики не менялись (К5), поведение контролов то же.
|
||||
|
||||
**Одно число — один источник.** Диф не вводит ни одной новой пользователем
|
||||
видимой величины: собранный CSS побайтово идентичен прежнему (К1/К3), в
|
||||
разметке (К5) и в i18n изменений нет. Правило неприменимо — проверять
|
||||
дублирование нечего.
|
||||
|
||||
**Риск ТЗ №1 (фикстуры-долг).** В `test/form-kit.test.mjs` рядом с фикстурами
|
||||
записан явный комментарий о том, что они удаляются на шаге эпика, который
|
||||
законно меняет вид панели — не «имеется в виду», а текст в файле, прочитано и
|
||||
подтверждено.
|
||||
|
||||
**Опора ТЗ на факт кода.** «У настроек панели нет golden-кадра» — проверено
|
||||
чтением `demo/golden/matrix.mjs`, сцены с `dialog: 'summary'` действительно
|
||||
нет. Не догадка.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Полный набор из 250 браузерных смоков — `smoke-select` называет одну прямую
|
||||
связь, диф — 2 CSS-файла с нулевым изменением собранного вывода; полный
|
||||
прогон — предрелизная обязанность, не гейт ревью на такой узкой задаче.
|
||||
- `python -m pytest tests_backend` — диф не касается Python.
|
||||
- Performance-профили — не названы в AC, диф не в чувствительных путях.
|
||||
- Полный HA-харнесс (`fcntl`) — не относится к CSS-рефакторингу.
|
||||
- `process-gate.mjs --issues` (проверка статуса issue через API) — не имеет
|
||||
значения для решения ревьюера, статус issue виден напрямую.
|
||||
- Мутационная защита `scripts/mutation-gate.mjs` как отдельный прогон — не
|
||||
обязательна: защита живёт в чистых юнит-тестах (`node --test`), не за дорогим
|
||||
гейтом (смок/бэкенд/golden), и прогон со снятой защитой уже приведён в
|
||||
документе лично мной (три пробы выше).
|
||||
|
||||
## Вердикт
|
||||
|
||||
Зелёный. Единственная находка r1 (M1, Medium) закрыта ровно предложенным
|
||||
способом и подтверждена независимым прогоном гейта. Разбор проведён полностью
|
||||
(§7.2, ребейз на ушедший вперёд `dev`): все восемь AC и К1–К5 перепроверены
|
||||
лично — не по хендоффу, не по документу r1 — включая три мутационные/пробные
|
||||
прогонки для защитных AC1–AC3, независимую пересборку бандла с побайтовой
|
||||
сверкой трёх копий и полный прогон `golden:verify` на всей матрице. Новых
|
||||
находок нет.
|
||||
|
||||
---
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/597-panel-on-form-kit` (рабочая копия в состоянии detached HEAD
|
||||
на коммите ниже — ветка задачи по хендоффу автора).
|
||||
- Коммит: `57817b3c91c87c15afa62b9ee2602db2c3b244fc`.
|
||||
- Дерево материала: `1c87b7fa2dbae8634209e92485a697e35bd2094d` (`git rev-parse HEAD^{tree}`).
|
||||
- Диапазон: `git diff origin/dev...HEAD`, `origin/dev` = `48c198ff265050406f155c9064bbc49999ebdadb`.
|
||||
- Вердикт этого документа: `green` · High 0 · Medium 0.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/597-panel-on-form-kit`, коммит `57817b3c91c8` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `1c87b7fa2dbae8634209e92485a697e35bd2094d`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 1c87b7fa2dba
|
||||
```
|
||||
- Тело issue: `8a56815128285d56f2c2f9f63eb6d8ae211525202f38e8bdd75159aa7c55412c`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user