diff --git a/docs/reviews/CODE-REVIEW-597-r2.md b/docs/reviews/CODE-REVIEW-597-r2.md new file mode 100644 index 00000000..72779f83 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-597-r2.md @@ -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. + +--- + + + +## Материал раунда + +- Ветка: `issue/597-panel-on-form-kit`, коммит `57817b3c91c8` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `1c87b7fa2dbae8634209e92485a697e35bd2094d` + ``` + git log --all --format='%H %T' | grep 1c87b7fa2dba + ``` +- Тело issue: `8a56815128285d56f2c2f9f63eb6d8ae211525202f38e8bdd75159aa7c55412c` +- Вердикт конвейера: `green` · High 0