From 05a2c68da23b890de049d8507dd41c1ba2c9910e Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Fri, 11 Sep 2026 22:42:21 +0000 Subject: [PATCH] docs: review document for #535 Issue: #535 User-Visible: no --- docs/reviews/CODE-REVIEW-535-r1.md | 211 +++++++++++++++++++++++++++++ 1 file changed, 211 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-535-r1.md diff --git a/docs/reviews/CODE-REVIEW-535-r1.md b/docs/reviews/CODE-REVIEW-535-r1.md new file mode 100644 index 00000000..b53c4e65 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-535-r1.md @@ -0,0 +1,211 @@ +# CODE-REVIEW-535-r1 + +Issue: #535 · Заход r1 · блокирующих циклов израсходовано 0/4 +Материал ревью: `063de0eef4bfb96802e8ff94fcc0e35177cfde0b` (`origin/dev...HEAD`, один коммит `063de0ee`, база `1ca3b5fa`) +Ревьюер: Claude (код-ревью) · Автор кода: Codex + +## Скоуп + +Правка меняет один плагин сборки, `entryFallbackPlugin` в `scripts/bundle-manifest.mjs`: +панель (`houseplan-panel.js`) теперь импортирует реализацию карточки по +content-hashed адресу (`houseplan-assets/houseplan-card-.js`) напрямую, а не +через стабильный фасад `houseplan-card.js`, — устраняя единственный адрес в +дистрибутиве, по которому карточка отдаётся без токена версии. `try/catch` и +фолбэк («House Plan обновился — перезагрузите страницу», 4 языка) сохраняются; +при недоступном чанке теперь срабатывает панельный фолбэк, а не карточный внутри +живой панели. + +Диапазон изменённых файлов: `scripts/bundle-manifest.mjs`, `scripts/bundle-tree.mjs` +(инвариант манифеста переформулирован под новую топологию), `scripts/mutation-gate.mjs` +(новый мутант + переоформление существующего), четыре тестовых файла +(`bundle-assets`, `bundle-freshness`, `bundle-sync`, `houseplan-zip`), `docs/ARCHITECTURE.md`, +оба `CHANGELOG`, три копии бандла (`dist/`, `custom_components/houseplan/frontend/`, +манифест `houseplan-assets.json`) и отпечаток скриншотов документации (только +отпечаток исходников, кадры не менялись). + +По механическому признаку §1 диапазон файлов класса A не задет (только B/C/D) — +классификация полным треком по продуктовому доводу автора (видимое поведение +на устаревшем загрузчике) была спорной точкой ревью ТЗ r1 и признана +обоснованной; я согласен с этим выводом (см. «Ответ на продуктовый вопрос» ниже). + +`src/**` не тронут вовсе — подтверждено `git diff origin/dev...HEAD --stat -- src/` +(пусто). + +## Как проверялось + +Дешёвые гейты (`tsc`, `npm test`, `npm run build` + сверка бандла) уже +подтверждены зелёным Validate на этом SHA +(https://github.com/Matysh/houseplan-card/actions/runs/34652944181) — не +перегонялись целиком. Дальше — то, что Validate не покрывает и что нужно +именно этой правке, прогнано лично, командой и результатом: + +| Гейт | Команда | Результат | +|---|---|---| +| Юниты по изменённым файлам (перепроверка независимо от Validate) | `node --test test/bundle-assets.test.mjs test/bundle-freshness.test.mjs test/bundle-sync.test.mjs test/houseplan-zip.test.mjs test/mutation-gate.test.mjs` | 91/91 pass | +| Мутант AC1/AC2 | `node scripts/mutation-gate.mjs --id=panel-imports-unversioned-card-entry` | поймано 1 из 1 | +| Мутант AC4 | `node scripts/mutation-gate.mjs --id=panel-entry-bypasses-card-graph` | поймано 1 из 1 | +| Мутант AC3 | `node scripts/mutation-gate.mjs --id=panel-entry-fallback-rewrite-skipped` | поймано 1 из 1 | +| Бюджет панели (К4) | `node scripts/bundle-budget.mjs` | exit 0; `initial panel-only: 2325 B` при потолке 8192 B (запас 5867 Б) | +| Инвентарь манифеста (К4) | ручное чтение `dist/houseplan-assets.json` | `initialPanelFiles` не содержит `houseplan-card.js`; `initialViewFiles` минус фасад ⊆ `initialPanelFiles` | +| Три копии бандла в синхроне | `cmp dist/houseplan-card.js custom_components/.../houseplan-card.js`, `cmp … houseplan-panel.js`, sha256 по `houseplan-assets/*.js` | совпадают | +| Реальный build детерминирован | `node scripts/bundle-sync.mjs` поверх коммита | 0 изменений в рабочем дереве после — сборка воспроизводима | +| Документация (доп. проверка, `src/**` не тронут, но диффит `docs/`) | `node scripts/check-docs.mjs` | «Documentation checks passed (7 files, 12 external links)» — совпадает с заявленным | +| Скриншоты (отпечаток исходников) | сверка `docs/images/screenshots.json` в диффе | `sourceFingerprint` обновлён на `3eba35b1…`, байты PNG и `imageSha256` не менялись — соответствует заявленному `docs:accept --identical` | +| Смок панели (браузер, счастливый путь) | `node demo/smoke_houseplan_panel.mjs` (после `node scripts/bundle-sync.mjs`, стенд не был синхронизирован) | OK, все проверки true | +| Смок устаревшего входа (браузер, AC3 «чем краснеет» на реальном dist) | `node demo/smoke_entry_stale.mjs` | OK — панельный и карточный фолбэки, все 4 языка, без необработанных ошибок страницы | + +**Не прогонялось и почему:** + +- `npx tsc --noEmit`, полный `npm test`, `npm run build` со сверкой копий — приняты по зелёному Validate на этом SHA (§ дешёвые гейты). +- `python -m pytest tests_backend` — бэкенд (`custom_components/**/*.py`) не тронут. +- `npm run golden:verify` — диффом не задет ни один пиксель карточки (правка — только адрес импорта и текст комментария/лога), сам автор это заявил, и `docs:accept --identical` (нулевые попиксельные различия по 11 кадрам) независимо это подтверждает. +- perf-профили — в AC не названы, диапазон файлов их не касается. +- `npm run invariants` (геометрия/toolThickness/layout) — диффом не задет: правка не касается ни рёбер комнат, ни `layout`, ни `marker.space`. +- Полный набор `demo/smoke_*.mjs` — `smoke-select.mjs --base origin/dev --head HEAD` отвечает «исполняемого frontend-диффа нет» (`src/**` не менялся), выбор двух смоков выше сделан вручную по факту, что именно они гоняют реальный собранный `dist/houseplan-panel.js` и `dist/houseplan-card.js` через браузер — это ровно то, что диффит правка. Остальные ~табор смоков карточки/редакторов не относятся к маршрутизации входов и не выбраны. + +## AC · чем доказан · чем краснеет + +| # | AC | Доказательство (перепроверено) | Чем краснеет (прогнано лично) | +|---|---|---|---| +| AC1 | Плагин переписывает импорт панели на content-hashed чанк, `imports` содержит чанк | `#486 both stable entries install a visible stale-load fallback` (юнит на плагине напрямую, без сборки) — прочитан и перепрогнан | мутант `panel-imports-unversioned-card-entry` — лично: «поймано 1 из 1» | +| AC2 | В собранном `dist/houseplan-panel.js` нет ссылки на карточку без версии | `#535 panel entry reaches the card implementation by its hashed name` (читает `dist/`) + лично: `grep -o 'try{await import(...)}' dist/houseplan-panel.js` → `houseplan-assets/houseplan-card-B09T8YNT.js`; `grep -c houseplan-card.js dist/houseplan-panel.js` → 0 | тот же мутант (юнит-гвард на плагине; дист-тест не пересобирается на мутанте — законно для чистого юнита, §2.7) | +| AC3 | Фолбэк цел: элемент определяется, сообщение на месте (4 языка), статических рёбер нет | поведенческая часть теста без изменений + **браузерный** `demo/smoke_entry_stale.mjs` на реальном `dist/houseplan-panel.js` с 404 на все хешированные чанки | мутант `panel-entry-fallback-rewrite-skipped` — лично: «поймано 1 из 1»; смок независимо воспроизводит тот же сценарий против настоящего собранного файла | +| AC4 | Манифест: граф панели содержит граф реализации карточки, не содержит фасад; бюджет соблюдён | `scripts/bundle-tree.mjs` (`assertBundleManifest`, переформулирован) + `scripts/bundle-budget.mjs` — лично прогнан, exit 0, `initial panel-only: 2325 B` (потолок 8192 Б) | мутант `panel-entry-bypasses-card-graph` — лично: «поймано 1 из 1» | +| AC5 | Ни один входной файл не ссылается на карточку по адресу без версии | новый тест `#535 no built entry reaches the card by an address without a version` + лично: `grep -rl "houseplan-card.js" dist/` → только сам фасад и манифест, никаких других файлов | мутант AC1/AC2 задевает тот же код-путь; дополнительно — ручной `grep` по всему `dist/` подтвердил отсутствие иных ссылок | +| AC6 | Оба чейнджлога и `ARCHITECTURE.md` описывают новую маршрутизацию | построчная сверка (см. ниже) + `node scripts/check-docs.mjs` — «passed (7 files, 12 external links)», лично воспроизведено | — (не защитный AC) | + +## Находки + +### Low — L1: текст `console.error` панели изменён вопреки букве К2 ТЗ + +`scripts/bundle-manifest.mjs`, панельная ветвь `entryFallbackPlugin`. + +К2 ТЗ дословно: «Текст, четыре языка, класс элемента и `console.error` не +меняются.» Дифф меняет именно `console.error`: + +``` +- 'console.error("[houseplan] stale houseplan-panel.js: the card entry is unavailable",e)}', ++ 'console.error("[houseplan] stale houseplan-panel.js: the card implementation ' ++ 'chunk is unavailable",e)}', +``` + +Это не покрыто ни одним тестом (ни один `assert` не проверяет содержимое этой +строки) и не названо в разделе «Отклонения от ТЗ — называю сам» хендоффа, хотя +там перечислены четыре других отклонения. По существу изменение оправдано: +после #535 недоступен именно chunk реализации, а не «card entry» — старый текст +стал бы неточным. Строка видна только в консоли разработчика, не пользователю, +и никак не входит в измеряемый функционал AC1–AC6. + +**Решение ревьюера:** снимаю без цикла — фактическая точность лога важнее +буквы «дословно» применительно к диагностической строке, которую К2 явно +перечисляет вместе с пользовательским текстом (это, возможно, недосмотр самого +К2, а не сознательный контракт). Отмечаю как принятое отклонение, чтобы +дальнейшие ревью не искали здесь скрытую регрессию. + +Других High/Medium/Low не найдено. + +## Одно число — один источник + +Дифф не добавляет и не меняет ни одной пользователем видимой величины +(строку с толщиной, площадью, размером и т.п.) — только адрес импорта и +служебные тексты (лог, комментарии, changelog). Проверка неприменима к этой +задаче. + +## Что проверено и корректно + +- **К1** — построчно: `dist/houseplan-panel.js` содержит ровно + `try{await import("./houseplan-assets/houseplan-card-B09T8YNT.js")}catch(e){…}`, + ссылок на `./houseplan-card.js` — ноль. +- **К2** — фолбэк (4 языка, класс элемента, стили) не тронут; см. L1 про + единственное отклонение — сам текст лога, не сам фолбэк. +- **К3** — `dist/houseplan-card.js` не изменился по контракту: тот же + `try{await import("./houseplan-assets/houseplan-card-B09T8YNT.js")}catch` в + фасаде, отдельная функция `cardEntry`/`cardPattern` в диффе не тронута. +- **К4** — манифест и рантайм-валидатор согласованы: `initialPanelFiles` не + содержит фасад, `initialViewFiles` (минус сам фасад) ⊆ `initialPanelFiles`, + бюджет 2325 Б при потолке 8192 Б. +- **К5** — оба теста `#486` действительно перевёрнуты по смыслу (не + подогнаны): `assert.doesNotMatch(panel, /houseplan-card\.js/)` появился там, + где раньше стоял `assert.match` на тот же паттерн; переименование второго + теста в `#535 …` оправдано сменой контракта, поведенческая часть (customElements, + фолбэк) не ослаблена. +- **К6** — новый тест на весь дистрибутив плюс ручной `grep -rl + "houseplan-card.js" dist/` подтверждают: во всём собранном дереве + единственная ссылка на карточку без версии — она же сам стабильный вход, + что и требуется контрактом (фасад продолжает существовать для Lovelace). +- **Мутанты** — все три названных автором (`panel-imports-unversioned-card-entry`, + `panel-entry-bypasses-card-graph`, `panel-entry-fallback-rewrite-skipped`) + лично перепрогнаны, каждый «поймано 1 из 1»; четвёртый пинned-тест + (`#486 bundle topology and both stale entries have mutation witnesses`) + подтверждает, что реестр не потерял старые id. + Новый мутант `panel-imports-unversioned-card-entry` не входит в pinned-список + `test/mutation-gate.test.mjs` — это нормально: список пиннует только уже + существовавшие до #535 id, новый свидетель добавлять туда не требовалось. +- **Отклонения от ТЗ, названные автором** (файл `bundle-tree.mjs`, девять + задетых фикстур, переоформление мутанта, отсутствие сдвига + `initialPanelOnlyGzipBytes`) — все проверены по диффу и корректны; ни одно + не ослабляет проверяемое утверждение (арифметика gzip-сумм в фикстурах + `bundle-freshness`/`houseplan-zip` сходится: убранный из графа фасад вычтен + из `initialPanelGzipBytes` ровно на его размер). +- **Документация**: `ARCHITECTURE.md` описывает ровно новую маршрутизацию и + причину (единственный неверсионированный адрес, отсутствие + `Cache-Control`, `immutable`-чанки); `CHANGELOG.md`/`CHANGELOG.ru.md` + синхронны по смыслу, ссылка на #535 в обоих, `User-Visible: yes` в коммите + соответствует правкам в обоих чейнджлогах в том же коммите. `USER-GUIDE.ru.md` + не тронут и не должен быть: описывает только фасад карточки (К3, не + изменился), к панели напрямую не относится. +- **Трейлеры и трассируемость**: один коммит, `Issue: #535`, + `User-Visible: yes`, ветка `issue/535-panel-card-url` (хендофф) слита без + дополнительных пушей — ровно то, что заявлено в хендоффе. +- **Сборка воспроизводима**: `node scripts/bundle-sync.mjs` поверх коммита не + дал ни одного изменения в рабочем дереве — три копии бандла (`dist/`, + `custom_components/.../frontend/`, стенд `demo/srv/assets/`) детерминированы + и синхронны. + +## Ответ на продуктовый вопрос автора + +Автор спрашивает, согласен ли я, что громкое сообщение лучше тихой работы на +старом коде, и не стоит ли отдельной задачей различать «фронтенд новее» и +«бэкенд новее» в самой плашке. Согласен с первым — это и есть суть задачи, она +уже была продуктово одобрена на ревью ТЗ r1. Второе — разумное предложение, но +оно за скоуп #535 (и уже названо автором как таковое в ТЗ и аналитике); отдельным +issue его заводить не обязываю, это на усмотрение владельца/автора при следующей +итерации плашки. + +## Чего не проверял + +- Полный `npm test` (2530 тестов) — не перегонял целиком, принят по зелёному + Validate на этом SHA; перепрогнал только пять файлов, прямо затронутых + диффом (91 тест, все зелёные). +- `pytest tests_backend`, golden, perf-профили, model-invariants — не + запускал: diff их не касается (см. таблицу гейтов выше). +- Полный список `demo/smoke_*.mjs` — не прогонял; выбрал два по факту, что они + гоняют именно собранные `dist/houseplan-panel.js`/`dist/houseplan-card.js` + в браузере (`smoke_houseplan_panel`, `smoke_entry_stale`), `smoke-select.mjs` + подтвердил отсутствие исполняемого frontend-диффа. +- Поведение в реальном Home Assistant (proxy-кэш, заголовки боевого стенда) — + вне возможностей код-ревью; измерения из issue (заголовки боевого стенда) + приняты как представленный автором факт, независимая браузерная проверка + (`smoke_entry_stale.mjs`) подтверждает механику фолбэка на уровне + собранного кода. + +## Вердикт + +Зелёный. Контракт К1–К6 реализован и подтверждён чтением кода, юнитами, +лично перепрогнанными мутантами и двумя независимыми браузерными смоками +против настоящего `dist`. Единственная находка — Low (L1, текст лога), +снята без цикла с записью выше. Медиум/High нет. + +--- + + + +## Материал раунда + +- Ветка: `issue/535-panel-card-url`, коммит `063de0eef4bf` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `40cb7affb33a6e9975239d4d9ac9b87fb92b7daa` + ``` + git log --all --format='%H %T' | grep 40cb7affb33a + ``` +- Тело issue: `77204930eaea05482f2a7da2bac3af79247e7d7356e0449551cb997da2c76fc8` +- Вердикт конвейера: `green` · High 0