mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-30 11:49:16 +00:00
@@ -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), а не дефект самого фикса камеры.
|
||||
Reference in New Issue
Block a user