diff --git a/docs/reviews/CODE-REVIEW-337-r4.md b/docs/reviews/CODE-REVIEW-337-r4.md new file mode 100644 index 00000000..f4a9b2a9 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-337-r4.md @@ -0,0 +1,216 @@ +# CODE-REVIEW-337-r4 + +- Issue: https://github.com/Matysh/houseplan-card/issues/337 +- Ветка: `issue/337-lazy-editor-chunk`, ревьюемый SHA: `0bd5570ae56b878dabe5c440840f5a3c90c523a0` +- Заход: r4 · блокирующих циклов израсходовано **3/4** до этого разбора +- **Вердикт: зелёный.** + +## Скоуп разбора + +Предыдущий вердикт — CODE-REVIEW-337-r3, SHA ревью `16c616be165604aca8eb9c7cdeab59024a828006` +(красный, High:1 — H7, Medium:0). Этот SHA в текущей истории отсутствует: ветка +была перебазирована. Проверил, на что именно: + +``` +git log --oneline 59ae6b1..ff6e53a4 +ff6e53a4 docs: review document for #333 +``` + +`dev` продвинулся ровно на **один** коммит между базой r3 (`59ae6b1`) и текущей +базой (`ff6e53a4` = текущий `origin/dev`) — класс C, docs-only ревью-документ +чужого issue (#333), не в `src/**`, не в `custom_components/**/*.py`. Это не +«ребейз на ушедший вперёд dev» в смысле §2.10/§7.2 (другая подсистема, смена +контракта), а техническая синхронизация без содержательного чужого кода. +Полный разбор с нуля не требуется; веду по дельте. + +**Материал дельты r3→r4:** + +``` +33a0110e fix: preserve warm editor camera during lazy adoption (тот же код, что 16c616be — рехеширован ребейзом; сверил построчно, см. ниже) +d30cc799 docs: review document for #337 (публикация документа r3, не продукт) +0bd5570a docs: accept lazy editor screenshot fingerprint (H7-фикс: docs/images/screenshots.json + 06-device-editor.png) +``` + +Проверил тождественность рехешированного `33a0110e` содержимому, разобранному в +r3, а не поверил ребейзу на слово: прочитал `src/houseplan-card.ts:803-925, +2549, 5880, 6662` на текущем HEAD и сверил построчно с описанием фикса в +CODE-REVIEW-337-r3.md — `_warmModeRequest`, гейт `_refitView`, снятие guard'а +после `updateComplete` + два `requestAnimationFrame`, снятие на всех путях +выхода — код идентичен тому, что r3 уже разобрал и принял. Единственный +содержательный новый коммит этого раунда — `0bd5570a` (закрытие H7). + +Единственная находка предыдущего раунда — **H7** (`check-docs` красный на +последнем коммите r3, AC13 не доказан). Разбор ограничен: (а) верификацией +закрытия H7, (б) подтверждением, что ребейз не изменил семантику уже принятого +кода, (в) самостоятельным перепрогоном дешёвых гейтов на итоговом SHA, раз +готового зелёного Validate-прогона с реально исполненными job'ами `frontend`/ +`smoke`/`golden` на этом конкретном SHA нет (см. ниже). + +## Как проверялось + +### Дешёвые гейты — прогнаны лично на `0bd5570a` + +| Гейт | Команда | Результат | +|---|---|---| +| typecheck | `npx tsc --noEmit` | pass, exit 0 | +| unit | `npm test` | 1440 total, **1439 pass, 1 skip, 0 fail** | +| build | `npm run build` | pass, `dist` пересобран за 13.8s | +| bundle sync | `npm run bundle:sync` | pass; после пересборки `git status` — 0 диффов в содержимом (только file-mode 755→644 в двух файлах от локальной пересборки, откатил `git checkout --`); три дерева (`dist`, `custom_components/houseplan/frontend`, `demo/srv/assets`) синхронны и побайтово совпадают с закоммиченными | +| bundle budget | `npm run bundle:budget` | initial View **255 910 B** ≤ 256 000 B (запас 90 B, то же число, что в r3/хендоффе); lazy editor 131 779 B | +| docs fingerprint (H7) | `node scripts/check-docs.mjs` | **pass**: «Documentation checks passed (7 files, 10 external links)» | + +Пересборка бандла на моей машине оказалась побайтово идентична закоммиченному +дереву — это независимое подтверждение, что `bundle:sync`/`bundle:budget` не +разошлись со временем автора, а не просто повтор его чисел. + +### Почему проверял `check-docs` и билд лично, а не принял чужой зелёный Validate как есть + +В задаче процитирован зелёный `Validate` на `0bd5570a` +(https://github.com/Matysh/houseplan-card/actions/runs/33144989734) как повод +не гонять `tsc`/`test`/`build` самому. Проверил сам прогон, потому что в r1 +этой же задачи уже был прецедент: зелёный Validate на docs-only коммите не +означает, что `frontend`/`smoke`/`golden` реально исполнялись (§1, находка r1). +Разобрал этот конкретный прогон: + +``` +gh run view 33144989734 --json jobs -q '.jobs[] | .name + " => " + .conclusion' +``` + +Job «Классификация изменённых файлов» вычислил инкрементальный diff между +`before`-SHA пуша (форс-пуш после ребейза, старый tip недостижим локально) и +`0bd5570a`: три файла — `docs/images/06-device-editor.png`, +`docs/images/screenshots.json`, `docs/reviews/SPEC-REVIEW-333-r1.md` (последний +— спутник ребейза на `ff6e53a4`, тоже docs). `frontend=false`, поэтому job +«Фронтенд: типы, юниты, мутанты, синхрон бандла» (где живут `tsc`/`test`/ +`build`) в этом конкретном прогоне **skipped**, а не green по содержанию — сам +Validate зелёный только потому, что docs-гейты (которые реально исполнялись) +прошли. Отдельно проверил job «Переиспользование: это дерево уже проверено» +(`scripts/gate-reuse.mjs`): `golden` и `backend` дали `Cache not found` (не +были waived кэшем), но всё равно оказались `skipped` в графе — их пропустила +та же path-классификация `frontend=false`, а не механизм переиспользования. + +Вывод не «Validate врёт»: срез diff между force-push before/after корректно +показал, что после ребейза содержимое `dist`/`src` не изменилось относительно +уже прошедшего полный `Validate` состояния (та же логика, что уже описана в +самом r1-документе про докс-коммиты). Но раз я не могу подтвердить это архивной +реконструкцией истории (старые SHA недостижимы после форс-пуша), надёжнее было +перепрогнать `tsc`/`test`/`build`/`bundle:*` самому на итоговом дереве — они +дешёвые (минуты) — чем полагаться на цепочку рассуждений о том, что должно было +быть проверено раньше. Результат подтвердил: числа и статусы совпадают с тем, +что репортили автор и r3. + +### Что не прогонял и почему + +- **Полный browser-smoke census (195 файлов) и `golden:verify`** — не + перезапускал. Дельта r3→r4 (`0bd5570a`) касается исключительно + `docs/images/**`; ни один файл `src/**`, `demo/**`, `test/**` не изменился. + Census и golden уже пройдены дважды на продуктовом дереве (r1: 195/195 и + 127/131 golden; r2: 195/195; targeted smokes зоны H6 — многократно в r2/r3) и + не могут быть чувствительны к правке `screenshots.json`/PNG документации — + они не читают эти файлы. `smoke-select` не гонял: применять его к диффу, не + содержащему ни одного `src`/`demo` файла, не даёт сигнала. +- **`node scripts/model-invariants.mjs`** — не запускал. Дельта не меняет + формат геометрической записи (`layout`, `marker.space`, `open_spans`, ключи + толщины); последний src-коммит, трогавший геометрию/камеру (`33a0110e`), + уже прогонялся через `npm test` (модельные инварианты входят в общий набор) + и мной, и r3 — без изменений с тех пор. +- **`node scripts/mutation-gate.mjs`** — не запускал повторно. Дельта не + трогает `src/**`; r3 уже прогнал `--changed` для единственного src-коммита + этой цепочки (30/30). +- **`python -m pytest tests_backend -q`** — не запускал. Диапазон дельты не + трогает `custom_components/**/*.py`. +- **Perf-профили** — не запускал, не названы в AC и не затронуты этой дельтой. +- **Ручное тестирование в реальном Home Assistant** — не выполнялось (фазы + ручного тестирования в процессе нет). + +## Находки + +Находок этого раунда нет — High/Medium/Low: 0/0/0. + +## Что проверено и корректно + +- **H7 закрыт.** `node scripts/check-docs.mjs` зелёный на `0bd5570a`: «pass (7 + files, 10 external links)», ровно та же форма, что r1 фиксировал как норму. + Причина закрытия видна в самом коммите: `sourceFingerprint` в + `docs/images/screenshots.json` обновлён с `d1ea5e9d…` на `5f56dab6…` — + значение, полученное реальной пересъёмкой (`sourceSha256` во всех 10 + сценариях синхронно обновлён на то же значение), а не скопировано вручную. + Единственный изменившийся `imageSha256` — `06-device-editor.png` + (`a01bad41…` → `36ed21b6…`), все остальные 9 `imageSha256` не тронуты — + соответствует утверждению автора и r3, что рендер изменился только на 1 байт + PNG-кодирования и только у одного кадра (r3 сам это независимо + воспроизвёл ещё до принятия). +- **Ребейз не изменил семантику.** `dev` продвинулся на один класс-C коммит + (docs-review чужого issue), не в `src/**`. Локальная пересборка (`npm run + build && npm run bundle:sync`) дала 0 диффов содержимого относительно + закоммиченного дерева — рехешированные ассеты `33a0110e` соответствуют + ровно той сборке, что уже стояла до ребейза. +- **Код фикса H6 (унаследован из r3, но перечитан для очистки от риска «ребейз + = другой код»)** — построчно совпадает с описанием r3: `_warmModeRequest` + выставляется только на `adopt`-пути (`src/houseplan-card.ts:894-901`), гейтит + `_refitView` (`:5880`), снимается после `updateComplete` + двух `rAF` + (`:918-924`) и на всех путях выхода (`:903, 906-907, 2549, 6662`). Изменений + в этой логике между `16c616be` и `33a0110e` нет. +- **Бюджет (AC1)** — пересчитан лично: 255 910 B ≤ 256 000 B, то же число, что + в r3 и в хендоффе — запас 90 B не изменился и не ухудшился этим раундом. +- **Трейлеры.** Оба коммита раунда (`33a0110e`, `0bd5570a`) несут `Issue: #337` + и `User-Visible: no` — корректно: ни один не добавляет нового наблюдаемого + пользователем поведения (H6 — внутреннее исправление тайминга камеры, + H7-фикс — служебный docs-артефакт), changelog для #337 уже закрыт в r1. +- **Одно число — один источник.** Дельта этого раунда не вводит и не дублирует + ни одной пользовательской величины: правка ограничена внутренним + fingerprint/хешами и байтами PNG документации, не выводимыми в UI как текст. + `test/single-source-numbers.test.mjs` проходит в общем `npm test`. + +## Закрытие раунда r3 + +| Находка r3 | Чем закрыта | Где это видно | +|---|---|---| +| **H7** (`check-docs` красный на последнем src-коммите r3, AC13 не доказан) | Канонический прогон `Docs screenshots` (workflow_dispatch) на ветке задачи, Linux-артефакт принят через `npm run docs:accept -- --reviewed --from=...`; `sourceFingerprint`/`sourceSha256` синхронно обновлены во всех 10 сценариях, изменился ровно 1 `imageSha256` | Коммит `0bd5570a` (`docs/images/screenshots.json`, `docs/images/06-device-editor.png`); `node scripts/check-docs.mjs` → pass лично на `0bd5570a` | + +## Унаследовано из r3 (и транзитивно из r1/r2) + +Принято без повторной проверки в этом раунде — дельта (`33a0110e`/рехеш + +`0bd5570a`, docs-only) их не касается: + +- **AC1 (initial budget)** — пересчитан лично в этом раунде (см. таблицу + гейтов), не просто унаследован. +- **AC2 (lazy boundary), AC5 (loader atomicity), AC6 (failure сохраняет + View)** — вне дельты; вывод CODE-REVIEW-337-r2.md (SHA `2beafc06`) остаётся в + силе. +- **AC3/AC4 (View и editor parity), включая всю зону H1–H4 из r1 (warm-remount, + kiosk, resize/optimize preflight, device inbox)** — закрыты и подтверждены + независимым прогоном в r2 (CODE-REVIEW-337-r2.md, SHA `2beafc06`); эта дельта + их зону не трогает. +- **AC7 (asset security), AC8 (полнота distribution), AC9 (CSS-минификатор)** — + не тронуты этой дельтой; вывод r1/r2 (CODE-REVIEW-337-r1.md, SHA `6d338b78`) + остаётся в силе. +- **AC10 (fingerprint mismatch), AC11 (onboarding/async config editor), AC12 + (no model drift)** — вне дельты, наследуются из r1/r2. +- **H6 (потеря бит-точности камеры warm-remount)** — закрыт и подтверждён в r3 + (CODE-REVIEW-337-r3.md, SHA `16c616be`, `smoke_warm_dialogs` 7/7 против 5/7 + fail); код идентичен на текущем SHA (см. «Что проверено и корректно» выше — + перечитан заново, не просто принят на слово). +- **H5/#346 (устаревшие golden-эталоны Device editor/mobile-ru)** — не + относится к #337, независимо воспроизведён на чистом `dev` в r2, заведён + отдельным issue #346 (проверил: issue открыт, `S1-new`, `bug`, `P3`). +- **Полный browser-smoke census (195/195) и `golden:verify` (127/131, 4 + «different» = #346)** — пройдены в r1/r2 на неизменной этим и предыдущим + (r3) раундами части дерева; не перезапускал, см. «Что не прогонял». + +Отдельно от AC-цепочки: **AC13 (docs) не наследуется** — на нём в r3 была +найдена находка H7, в этом раунде она закрыта заново проверенным гейтом, а не +принята со слов автора. + +## Итог + +Единственная блокирующая находка предыдущего раунда (H7 — устаревший +screenshot fingerprint) закрыта корректно: канонический Linux-прогон, +воспроизводимый дважды с одинаковым хешем, обновил fingerprint и ровно один +байтово отличающийся кадр, без замены остальных девяти. Ребейз между r3 и r4 +не изменил продуктовый код — на `dev` прилетел один нерелевантный docs-коммит +другого issue, и локальная пересборка дала 0 диффов с закоммиченным деревом. +Дешёвые гейты (`tsc`, `npm test`, `build+sync+budget`, `check-docs`) +перепрогнаны лично на итоговом SHA `0bd5570a`, а не приняты по цепочке +рассуждений о докс-коммитах — все зелёные, числа совпадают с отчётами автора и +r3. Новых находок нет. Вердикт: зелёный.