docs: review document for #337

Issue: #337
User-Visible: no
This commit is contained in:
claude[bot]
2026-08-28 05:40:28 +00:00
parent 81420a0d94
commit c88af61be2
+264
View File
@@ -0,0 +1,264 @@
# CODE-REVIEW-337-r3
- Issue: https://github.com/Matysh/houseplan-card/issues/337
- Ветка: `issue/337-lazy-editor-chunk`, ревьюемый SHA: `16c616be165604aca8eb9c7cdeab59024a828006`
- Заход: r3 · блокирующих циклов израсходовано **2/4** до этого разбора
- **Вердикт: красный.**
## Скоуп разбора
Предыдущий вердикт — CODE-REVIEW-337-r2, SHA ревью `2beafc06398c16d1cad80fddbe77e6a40f111b9a`
(красный, High:1 — H6, Medium:0). Этот SHA и SHA r1 (`6d338b78`) в текущей истории
отсутствуют: ветка была перебазирована. Проверил, на что именно:
```
git log --oneline 59ae6b1 -3
59ae6b10 ci: полная история без блобов там, где содержимое старых файлов не нужно
80a664bc docs: рекомендовать blobless-клон вместо полного
508945c0 fix: a 0° pair is the shared-wall model, not a duplicate — field revert of #331 §2.2
```
`508945c0` — это тот же `dev`, на котором уже стояли и r1, и r2 (см. r2-документ:
«branch и так уже стоит на `508945c0` = `origin/dev`»). Между r2 и этим заходом `dev`
продвинулся всего на два коммита класса C/B (`docs`, `ci`, ни один не в `src/**` и не
в `custom_components/**/*.py`) — не «ребейз на ушедший вперёд dev» в смысле §2.10/§7.2
(другая подсистема, смена контракта), а техническая синхронизация без содержательного
чужого кода. Полный разбор с нуля не требуется; веду по дельте.
**Материал дельты r2→r3** — единственный содержательный коммит:
```
16c616be fix: preserve warm editor camera during lazy adoption (src/houseplan-card.ts +28/-4, плюс зеркальные dist/*)
```
(Коммиты `8afc8636` — документ r2, `6b482f4c`/`946f67ac`/`6f9df14d` предшествуют r2 и
уже разобраны в CODE-REVIEW-337-r2 — не входят в дельту этого раунда.)
Единственная находка предыдущего раунда — **H6** (потеря бит-точности камеры
warm-remount при lazy-adopt). Разбор ограничен: (а) верификацией фикса H6, (б) всеми
проверками, до которых дотягивается 15-файловый diff (`src/houseplan-card.ts` +
зеркальные `dist/`/`custom_components/houseplan/frontend/`), (в) обязательным
по AC13/§8 `check-docs` (диф трогает `src/**`).
## Как проверялось
Дешёвые гейты — прогнаны лично на `16c616be`, не приняты со слов автора:
| Гейт | Команда | Результат |
|---|---|---|
| typecheck | `npx tsc --noEmit` | pass, exit 0 |
| unit | `npm test` | 1440 total, **1439 pass, 1 skip, 0 fail** (у автора 1438/2 — расхождение на один conditional skip, см. ниже, не регрессия) |
| build+sync+budget | `npm run build && npm run bundle:sync && npm run bundle:budget` | initial View **255 910 B** ≤ 256 000 B (запас **90 B**, у автора — то же число); lazy editor 131 779 B; три дерева (`dist`, `custom_components/houseplan/frontend`, `demo/srv/assets`) синхронны |
| docs fingerprint (диф трогает `src/**`, AC13 прямо называет `check-docs` доказательством) | `node scripts/check-docs.mjs` | **FAIL, exit 1**: `ERROR screenshot source fingerprint is stale; run npm run build && node demo/docs/capture.mjs` — см. находку H7 |
| mutation, changed-range | `node scripts/mutation-gate.mjs --check --changed=16c616be^..16c616be` | 30/30 применимых guard'ов ловят подмену |
Разница в skip-счётчике: у меня единственный skip — `issue 281 private exact fixture
has no enabled zero-range handle # SKIP private #281 fixture is not present`, условный
пропуск из-за отсутствующей приватной фикстуры в песочнице, к #337 не относится. Не
исследовал, какой второй тест пропущен у автора на Windows — сумма (1440) совпадает,
расхождение не в счёте тестов #337.
Целевые browser smokes (по дельте — файл трогает ровно зону `_requestMode` /
`_refitView` / `_setMode` / disconnect-guard, разбор не «по названию», а по
затронутым символам):
| Smoke | Зачем выбран | Результат |
|---|---|---|
| `smoke_warm_dialogs.mjs` ×7 подряд, свежий `bundle:sync` | тот самый смок, поймавший H6 (5/7 fail в r2) | **7/7 OK**, `bViewBitExact` не подводит ни разу |
| `smoke_warm_owners.mjs` | та же warm-adopt зона (`_warmAdoptViewport`/`_requestMode(adopt)`) | OK |
| `smoke_preloader_lifecycle.mjs` | та же зона, ранее ловил H1 | OK |
| `smoke_kiosk.mjs` | ранее ловил H2, эта зона трогает общий `_requestMode` | OK |
| `smoke_mode_transition.mjs` | напрямую использует `_modeTransitionBusy`, которым теперь делит guard `_refitView` | OK |
| `smoke_resize_wall_thickness.mjs` | использует resize/refit во время editor-режима | OK |
| `smoke_warm_remount.mjs` | смежный warm-remount контракт, слабая связь по `smoke-select` | OK |
| `smoke_room_resize.mjs`, `smoke_optimize_geometry_preflight.mjs` | H3-зона (`this.host.` preflight), проверка на отсутствие регресса от нового `isConnected`/`_warmModeRequest` раннего return | OK |
`node scripts/smoke-select.mjs --base origin/dev --head HEAD` даёт 175/195 «прямое
совпадение» — бесполезно как фильтр для этого раунда: `origin/dev` не содержит всей
задачи #337 целиком, поэтому инструмент показывает список для ВСЕЙ ветки, а не для
дельты r2→r3. Использовал его для контекста (список выше уже включает `smoke_warm_dialogs`,
`smoke_mode_transition` и `smoke_warm_remount` из числа тех, что он называет), но
подбирал набор вручную по символам самого диффа, а не по выдаче инструмента.
Полный census 195 smoke-файлов и `golden:verify` **не перезапускал**: он уже
пройден дважды (195/195 в r1, 195/195 в r2) на неизменной по этому раунду части
дерева, а этот раунд — один файл в одной узкой зоне таймингов, для которой census
уже доказанно нечувствителен (сам census не поймал H6 ни разу, только точечный
7×-прогон). Полный набор — предрелизный гейт (§8), не гейт ревью при такой дельте.
`model-invariants` — не запускал: diff не меняет ни один формат геометрической
записи (`layout`, `marker.space`, `open_spans`, ключи толщины) — правка только
про `_view`/`_zoom` кэш камеры и таймер refit. Backend/pytest — не тронут этой
дельтой, не перепрогонял.
## Находки
### High (блокирует)
**H7. `check-docs` красный на ревьюемом SHA — стало устаревшим ровно из-за
этого коммита, AC13 не доказан.**
`node scripts/check-docs.mjs` → `ERROR screenshot source fingerprint is stale`
(exit 1). Причина механическая и не спорная: `scripts/source-fingerprint.mjs`
хэширует весь `src/**`; `16c616be` меняет `src/houseplan-card.ts` (единственный
src-файл в этом коммите) и не обновляет `sourceFingerprint` в
`docs/images/screenshots.json`, поэтому хэши расходятся. На SHA r2
(`2beafc06`) `check-docs` был зелёным (сам документ r2: «pass (7 файлов, 10
внешних ссылок)») — регрессия введена именно этим, последним коммитом
раунда, а не унаследована.
Это прямо предусмотренный в задаче случай, а не мелочь: AC13 спецификации
(`docs/specs/337-lazy-editor-chunk.md:346-348`) называет `check-docs` **прямым
доказательством** этого критерия приёмки, а §8 `PROCESS.md` называет его
«реальным блокером» именно за то, что отпечаток покрывает весь `src/**` — «любая
правка фронтенда делает его устаревшим… выбирать тут нечего». Цена пропуска
уже была измерена дважды на этой же кодовой базе: «скриншоты не пересняли в
#230 и #234, и `dev` стоял с красным job `docs`, пока это не нашли при
следующей задаче (#237)». Слияние `16c616be` в `dev` в текущем виде
воспроизводит ровно этот сценарий.
Автор в хендоффе смешал эту находку с H5/#346: «известный stale screenshot
fingerprint; H5 уже независимо подтверждён ревьюером и вынесен в #346» — но
H5/#346 про **golden-эталоны** (`demo/golden/baselines/**`, устаревший baseline
тулбара Device editor и mobile-ru диалогов, не связан с #337 и воспроизводится
на чистом `dev`), а это — про **fingerprint скриншотов документации**
(`docs/images/screenshots.json`, `docs-section` в `README`/`USER-GUIDE`),
совершенно другой гейт с другим владением. Ссылка на #346 не закрывает эту
находку: #346 не затрагивает `sourceFingerprint`, а причина здесь — собственный
код-коммит этой ветки, а не предсуществующее состояние `dev`.
Сам автор уже дважды в этой же задаче правильно закрывал этот же гейт без
замены единого PNG («fingerprint обновлён без замены PNG, потому что
наблюдаемого UI-изменения нет» — хендофф после H1–H4). Здесь этот шаг просто
пропущен для последнего коммита.
**Воспроизведение:** `git checkout 16c616be -- .` (или просто на HEAD), затем
`npm run build && node scripts/check-docs.mjs` → exit 1 с текстом выше.
**Исправление в скоупе задачи**: прогнать `Docs screenshots` (workflow_dispatch)
и принять новый fingerprint через `npm run docs:accept -- --reviewed
--from=<артефакт>` (замены PNG не требуется, если рендер не изменился —
ровно то, что уже делалось между r1 и r2 этой же задачи).
### Medium
Нет находок этого уровня в дельте r2→r3.
### Low
Нет.
## Что проверено и корректно
- **Сама механика фикса H6 логически закрывает найденную гонку.** Разобрал
правку построчно (`src/houseplan-card.ts:803-925, 2549, 5880, 6662`):
- новое поле `_warmModeRequest` выставляется только на `adopt`-пути
`_requestMode` (`:894-901`) и гейтит `_refitView` (`:5880`: `if
(this._modeTransitionBusy || this._warmModeRequest) return;`) — то есть
ResizeObserver-триггерный пересчёт `x`/`w` камеры (источник H6) подавлен
на всё время окна между синхронным восстановлением `_view` и асинхронным
переключением `_mode` через `_requestMode(restoredMode, false, true)`;
- после `updateComplete` и двух `requestAnimationFrame` (реальная
расстановка раскладки после переключения режима) код берёт фактический
`stage.clientWidth/Height` как новый `_lastValidStageSize` и снимает
guard (`:918-924`) — следующий настоящий resize сравнивается с верной
базой, а не с устаревшей;
- guard снимается на всех путях выхода: неудача `_ensureEditorRuntime()`
(`:903`), опережающий запрос или дисконнект во время `await`
(`:906-907`, добавленная проверка `!this.isConnected`), обычная смена
режима пользователем через `_setMode` (`:6662`), и
`disconnectedCallback` (`:2549`) — то есть защита не может «залипнуть».
- `_warmAdoptViewport`/`adopt=true` вызывается только на **свежесозданном**
экземпляре карточки при warm-remount (`:3110, 3243`), где `_refitRaf`/
`_pendingRefitSize` заведомо в начальном состоянии — отмена «на всякий
случай» на `:899-900` не теряет ничьих легитимных запросов.
- **Эмпирически гонка не воспроизводится.** `smoke_warm_dialogs.mjs` — тот же
файл и тот же сценарий, что дал 5 отказов из 7 в r2 — **7/7 OK** на этом
SHA после `bundle:sync`. Это выполняет дисциплину «тест умеет падать»: он
падал большинством прогонов на предыдущей ревизии кода и на новой этого не
делает ни разу — не совпадение единичного зелёного прогона.
- **Соседние сценарии той же зоны не пострадали**: `warm_owners`,
`preloader_lifecycle` (H1-зона), `kiosk` (H2-зона), `room_resize`,
`optimize_geometry_preflight` (H3-зона), `mode_transition`,
`resize_wall_thickness`, `warm_remount` — все зелёные лично.
- **Бюджет (AC1)** — 255 910 B ≤ 256 000 B, посчитано лично. Запас **90 B**,
сократился с 222 B (r2) — тенденция, названная риском уже в r1/r2,
продолжается и здесь и стоит держать в уме на любой следующей правке;
формально AC1 выполнен, не блокирую этим отдельно.
- **Mutation gate по коммиту** — 30/30 применимых guard'ов на
`16c616be^..16c616be` ловят подмену; изменённая зона осталась под
контролем существующих guard'ов, новых guard'ов эта точечная правка не
требовала.
- **Трейлеры.** `Issue: #337`, `User-Visible: no` — корректно: пользовательский
changelog для #337 уже зафиксирован отдельным коммитом ранее (r1), сама
правка не добавляет нового видимого поведения, а завершает контракт до
релиза (задача ещё не в `dev`, User-Visible относится к тому, что увидит
пользователь после выпуска).
- **Одно число — один источник.** Диф не вводит и не дублирует ни одной
видимой пользователю величины: `_view`/`_zoom`/`_lastValidStageSize` — это
внутреннее состояние камеры рендера, не выводится текстом/подписью нигде
в UI. Правило `test/single-source-numbers.test.mjs` не затронуто и
проходит в общем `npm test`.
## Закрытие раунда r2
| Находка r2 | Чем закрыта | Где видно |
|---|---|---|
| **H6** (потеря бит-точности камеры warm-remount, 5/7 fail на `smoke_warm_dialogs`) | `_refitView` подавлен на время адаптации новым `_warmModeRequest`-guard'ом; после двух `rAF` после `updateComplete` берётся фактический размер сцены как новая база для рефита — устраняет саму гонку резайза во время переключения режима | `src/houseplan-card.ts:806, 894-925, 2549, 5880, 6662`; эмпирически — `smoke_warm_dialogs.mjs` 7/7 OK (прогнал лично) против 5/7 FAIL на предыдущей ревизии |
## Унаследовано из r2
Принято без повторной проверки в этом раунде — дельта (`16c616be`, один
src-файл + зеркальные `dist/`) их не касается:
- **AC1 (initial budget)** — пересчитан лично в этом же раунде (см. таблицу
гейтов), не просто унаследован: 255 910 B ≤ 256 000 B.
- **AC2 (lazy boundary), AC5 (loader atomicity), AC6 (failure сохраняет View)** —
вне дельты; вывод CODE-REVIEW-337-r2.md (SHA `2beafc06`, раздел «Закрытие
раунда r1» + «Что проверено и корректно») остаётся в силе.
- **AC7 (asset security), AC8 (полнота distribution), AC9 (CSS-минификатор)** —
не тронуты этой дельтой; вывод r2 (наследующий вывод r1, SHA `6d338b78`)
остаётся в силе.
- **AC10 (fingerprint mismatch runtime — версии не смешиваются), AC11
(onboarding/async config editor), AC12 (no model drift)** — вне дельты,
наследуются из r2/r1.
- **H2, H3, H4, M1–M3 (r1)** и **H5/M4 → #346 (golden baselines, вне скоупа
#337)** — закрыты и подтверждены независимым прогоном ещё в r2
(CODE-REVIEW-337-r2.md, SHA `2beafc06`); эта дельта их зону не трогает.
- **Полный browser-smoke census (195/195) и `golden:verify` (127/131, 4
«different» = #346)** — пройдены дважды (r1 SHA `6d338b78`, r2 SHA
`2beafc06`) на неизменной этим раундом части дерева; не перезапускал
целиком, см. «Как проверялось».
Отдельно от AC-цепочки: **AC13 (docs) не наследуется** — именно на нём найдена
новая находка H7 этого раунда (было зелёным на SHA r2, стало красным на SHA r3).
## Чего не проверял
- **Полный HA-harness backend** (`python -m pytest tests_backend -q` с
установленным `homeassistant`) — недоступен в этой песочнице, дельта не
трогает `custom_components/**/*.py`.
- **`node scripts/model-invariants.mjs`** — не запускал, дельта не меняет
формат геометрической записи (см. «Как проверялось»).
- **Полный browser-smoke census (195 файлов) и `golden:verify`** — не
перезапускал в этом раунде; обоснование выше и в разделе «Унаследовано».
- **Perf-профили** — не запускал, не названы в AC и не затронуты этой
дельтой.
- **Частоту гонки H6 в headless Linux CI** отдельно от своей песочницы — не
измерял; 7/7 подряд в своём окружении расцениваю как достаточное
доказательство закрытия конкретно найденной гонки (тот же порог, что
задавал сам r2-документ), но абсолютную частоту в CI не оцениваю числом.
- **Ручное тестирование в реальном Home Assistant** — не выполнялось (в
процессе нет фазы ручного тестирования, код-ревью отвечает за AC вместо
него).
## Итог
Фикс H6 логически и эмпирически закрывает единственную блокирующую находку
r2: разобран по коду, зона правки атомарна и не расширяет скоуп, гонка не
воспроизводится ни разу за 7 прогонов там, где раньше падала в 5 из 7.
Однако тот же самый коммит красит обязательный по AC13 гейт `check-docs` —
не унаследованная, а новая, введённая этим раундом находка (H7), с уже
дважды на этой задаче отработанным способом закрытия. Вердикт остаётся
красным по формальному правилу «High блокирует», при том что содержательная
причина возврата — узкая: один пропущенный шаг (пересъёмка/приёмка
fingerprint), а не дефект самого фикса камеры.