diff --git a/docs/reviews/SPEC-REVIEW-353-r1.md b/docs/reviews/SPEC-REVIEW-353-r1.md new file mode 100644 index 00000000..7594c952 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-353-r1.md @@ -0,0 +1,274 @@ +# SPEC-REVIEW-353-r1 + +Issue: #353 — «Устойчивость lazy-загрузки: терминальный отказ редактора после +сетевого сбоя и мёртвый entry-лоадер после обновления» +Этап: ревью ТЗ (PROCESS.md §2.4), лёгкий трек (`small`), заход r1, +блокирующих циклов израсходовано 0 из 2 (лимит §4: 2). +ТЗ живёт в теле issue #353 (лёгкий трек, файла в `docs/specs/` нет). +Материал ревью: тело issue #353 на момент разбора (комментарий +Matysh, S2→S3, `2026-08-28T11:01:48Z`), метки `bug · P1 · S4-spec-review +· small`. + +## Вердикт + +**Жёлтый.** High: 1 · Medium: 1 (в скоупе) · Low: 1 (снят с запиской, см. +ниже). Причина: K3 (устойчивый entry) как буквально описан ломает +инвариант, на который опирается сам демо-стенд, а значит и весь браузерный +конвейер гейтов, построенный поверх него — при этом ни один AC этого не +ловит. Остальной контракт (K1, K2, K4, K5) и критерии приёмки методологически +крепкие и не требуют доработки. + +## Скоуп разбора + +Первый заход — разбор полный, весь текст ТЗ в теле issue #353. Раздел +«Унаследовано» и «Закрытие предыдущего раунда» не нужны — предыдущих раундов +нет. + +## Как проверялось + +1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (полностью, включая + §2.4, §2.9/2.10, §5, §7.1, §8). +2. Прочитано тело issue #353 и единственный комментарий (переход S2→S3). +3. Прочитан связанный issue #337 (родительский контракт lazy-бандла) для + понимания, что «терминально»/«лениво» значило до этой задачи. +4. Построчно сверены все технические утверждения ТЗ с текущим кодом: + - `src/editor-runtime-loader.ts` (класс `EditorRuntimeLoader`, текущая + семантика retry/terminal); + - `src/houseplan-card.ts:802-930` (оба лоадера, `_requestMode`, тосты + `editor.load_failed`/`editor.refresh_advice`, `customElements.define` + на строке 12889-12890); + - `dist/houseplan-card.js` — реальное содержимое собранного entry-чанка: + `globalThis.__HOUSEPLAN_BUILD_FINGERPRINT__="...";export{d6 as + HouseplanCard}from"./houseplan-assets/houseplan-card-BarpA18i.js";` + (подтверждает описание N3 буквально); + - `rollup.config.mjs`, `scripts/bundle-manifest.mjs` + (`editorRuntimeRetryUrlPlugin`, `bundleManifestPlugin`, + `buildBundleManifest` — как считаются `initial`/`lazy` графы); + - `scripts/bundle-tree.mjs` (`verifyBundleTree` — подтверждён пробел N4: + проверяет только файлы ИЗ манифеста, не сканирует каталог на лишнее); + - `scripts/bundle-budget.mjs` + `test/bundle-assets.test.mjs` (гейт #352, + как считается `initialViewGzipBytes`); + - `custom_components/houseplan/frontend_assets.py`, + `frontend_asset_manifest.py`, `__init__.py:85-132` (текущие + `Cache-Control`, fail-closed резолвер, регистрация `res_type: module`); + - `demo/srv/demo.html:180-209` (загрузка карточки демо-стендом — ключевая + находка, см. ниже); + - `test/editor-runtime-loader.test.mjs` (существующее покрытие: retry + ровно один раз для любой ошибки, терминальность только для mismatch — + подтверждает описание N2 буквально); + - `src/i18n/en.json`, `ru.json`, `de.json` (текущие `editor.load_failed`/ + `editor.refresh_advice`, три словаря синхронны, новый ключ вписывается + по образцу). +5. `git status`/`git log` — рабочее дерево чистое на `07b6a1f2` (= `origin/dev`), + ветки `issue/353-*` нет: код по этой задаче ещё не начат, что ожидаемо на + этапе `S4-spec-review`. + +### Гейты — что прогнано и почему + +Этап — ревью ТЗ, кода по issue #353 в дереве нет (задача ещё не покидала +`S4-spec-review`, ветки `issue/353-*` не существует). `typecheck`/`test`/ +`build`/`check-docs`/`smoke-select`/`model-invariants` — гейты код-ревью +(PROCESS.md §8, §2.7) и относятся к диффу кода, которого здесь нет; на этапе +ТЗ (§2.4) прогонять их нечего и не о чем — сверять было бы не с чем. +Ничего не пропущено умалчиванием: раздел «Гейты» в этом документе не +применим к этапу spec, и это явное решение, а не пропуск. + +## Находки + +### [High] K3: замена статического реэкспорта на «голый» `import().catch()` рвёт гарантию `await import(entry) ⇒ элемент зарегистрирован`, на которую опирается сам демо-стенд + +**Файл/раздел:** тело issue #353, раздел «К3 — entry переживает +несвежесть». + +**Что не так.** Сейчас `dist/houseplan-card.js` — это ES-реэкспорт: +```js +export{d6 as HouseplanCard}from"./houseplan-assets/houseplan-card-BarpA18i.js"; +``` +Это **статический** import-edge: спецификация ES-модулей гарантирует, что +`await import('/assets/houseplan-card.js')` не резолвится, пока весь граф, +достижимый через `export...from`, не будет получен, слинкован и +выполнен — то есть пока `customElements.define('houseplan-card', +HouseplanCard)` (строка `houseplan-card.ts:12889-12890`, лежит внутри +чанка `houseplan-card-.js`) гарантированно не отработает. + +К3 предписывает заменить это на `import("./houseplan-assets/…").catch(…)` +— **не** `export`, а голый вызов внутри тела entry-чанка, судя по +формулировке и по аналогии с `editorRuntimeRetryUrlPlugin` (пост-обработка +уже сгенерированного `chunk.code` строковой заменой в `generateBundle`, +после того как Rollup уже посчитал граф). Promise такого `import()` никуда +не экспортируется и не await'ится потребителем entry-модуля. Значит, +top-level выполнение entry-чанка завершается почти сразу — до того, как +динамически запрошенный `houseplan-card-.js` успеет загрузиться и +выполниться, — и именно в этот момент резолвится `await +import('/assets/houseplan-card.js')` у любого потребителя. + +Демо-стенд, на котором держится вся браузерная часть гейтов, устроен именно +так — `demo/srv/demo.html:193-197`: +```js +await import('/assets/houseplan-card.js'); +const card=document.createElement('houseplan-card'); +card.setConfig({type:'custom:houseplan-card', ...}); +``` +После К3 `await import(...)` резолвится раньше, чем элемент `houseplan-card` +зарегистрирован. `document.createElement('houseplan-card')` в этот момент +создаёт обычный неапгрейженный `HTMLElement` (в спецификации Custom Elements +апгрейд элемента до пользовательского класса происходит именно в момент +`customElements.define`, а не раньше) — метода `setConfig` на нём ещё нет. +Следующая строка `card.setConfig(...)` бросает `TypeError: card.setConfig +is not a function` **синхронно**, в самом бутстрапе demo.html. + +**Почему это не мелочь и не ловится AC.** Это не побочный редкий кейс — +это обычный, счастливый путь загрузки (когда главный чанк грузится +успешно). Он ломает бутстрап demo-стенда **для каждого** существующего +`demo/smoke_*.mjs`, для `golden:capture`/`golden:verify`, для +`performance_smoke` и для `demo/docs/capture.mjs` (снятие скриншотов +документации) — то есть весь браузерный слой CI одномоментно. +AC3 из ТЗ этого не поймает: (а) юнит-тест — это проверка текста +собранного файла на подстроки (`dynamic import`, `catch`, `define-фолбэк`), +не поведения при исполнении; (б) `demo/smoke_entry_stale.mjs` намеренно +воспроизводит **испорченный** главный чанк — сценарий, где `import()` +падает и код уходит в `catch` быстро; обычный работающий путь загрузки +(когда `import()` успешен) в AC не проверяется вовсе, а именно там гонка +и стреляет. + +**Как воспроизвести рассуждение (без исполнения — стадия ТЗ, кода ещё +нет):** прочитать текущее содержимое `dist/houseplan-card.js` (статический +реэкспорт), заменить его мысленно на буквальный текст К3 +(`import(...).catch(...)` без `export`/`await`), проследить, что +`await import(entry)` у потребителя больше не транзитивно ждёт вложенный +`import()`, и сопоставить это с `demo/srv/demo.html:193-197`, где +`document.createElement` + `setConfig` идут немедленно и синхронно после +`await import(entry)`. + +**Что нужно поправить в ТЗ.** Явно решить один из двух путей и записать +его в контракт K3: +- либо entry сохраняет цепочку ожидания — например top-level `await` внутри + самого чанка (`await import(...).catch(...)`, ES2022 top-level await, + Rollup format `es` его поддерживает) с реэкспортом результата, чтобы + `await import(entry)` по-прежнему резолвился только после того, как + главный чанк загружен либо фолбэк-элемент определён; это сохраняет + сегодняшний инвариант без правок demo.html; +- либо явно принять, что гонка есть, и добавить в объём задачи правку + `demo/srv/demo.html` (и любого другого потребителя entry, включая то, как + HA резолвит `res_type: module`) на ожидание + `customElements.whenDefined('houseplan-card')` вместо немедленного + `document.createElement`+`setConfig` — тогда это меняет объём файлов и, + вероятно, поверхность (`demo/**` — класс B, но задачу это не выводит из + лёгкого трека автоматически; решить нужно явно, а не молчанием). + +Без этого уточнения ТЗ не выполнимо буквально: реализация по тексту К3 +ломает работающий сегодня демо-стенд и, транзитивно, красит `smoke`, +`golden`, `performance_smoke`, `docs` — не после дефекта в проде, а сразу +на первом прогоне гейтов реализации. + +### [Medium, в скоупе] K2 не имеет ни одного AC/доказательства + +**Файл/раздел:** тело issue #353, «К2 — тост на каждый неудачный цикл» vs +раздел «AC и доказательства». + +К2 — не мелочь: это ядро продуктового сценария issue-AC1 («каждая неудача +видима тостом», сетевая — новым ключом `editor.retry_advice`, терминальная +— старым `editor.refresh_advice`, для **обоих** лоадеров, editor и +onboarding). Но ни один из перечисленных AC1…AC4 его не проверяет: + +- AC1 — юнит на уровне модуля `editor-runtime-loader.test.mjs`, проверяет + только сам класс `EditorRuntimeLoader` (колбэк `failed(error, + {terminal:false})`, переход в `idle`) — не то, какой текст тоста выберет + `houseplan-card.ts` в своих двух колбэках `failed: (error) => {...}` + (строки 832-835, 860-863), которые как раз и нужно переписать на `(error, + info) => info.terminal ? refresh_advice : retry_advice`. +- AC2 — про терминальность mismatch, тоже не про текст тоста. +- AC3/AC4 — про entry-чанк и orphan-проверку, к тостам отношения не имеют. + +Реализация, которая продолжит всегда показывать `refresh_advice` +(игнорируя `info.terminal`) для обоих лоадеров, пройдёт весь названный +набор AC без единого красного теста — при том что это именно та регрессия, +из-за которой issue вообще завели (N2: «тост… показывается один раз… все +последующие нажатия — молчаливый no-op»; K2 — прямое лекарство от этого). + +**Фикс, полностью в скоупе задачи:** добавить AC5, например: юнит на +уровне `houseplan-card.ts` (или на уровне колбэков `failed`, вынесенных в +проверяемую точку), который стабом задаёт `failed(error, {terminal: +true|false})` для `_editorRuntimeLoader` и `_onboardingRuntimeLoader` и +проверяет, что показанный тост содержит `editor.refresh_advice` в +терминальном случае и `editor.retry_advice` в сетевом, для обоих лоадеров +(4 комбинации). С High это не связано и не требует отдельного цикла как +самостоятельная находка — фиксируется тем же заходом, что и K3. + +### [Low, снято с запиской] docs/USER-GUIDE.ru.md не упомянут в объёме задачи, хотя описывает контракт, который эта задача меняет + +`docs/USER-GUIDE.ru.md:108-111` и `:1740` документируют **сегодняшнее** +поведение («после одной повторной попытки… предлагает обновить страницу»; +«После обновления старая версия → перезапустите HA, перезагрузите +ресурсы/браузер»). После K1–K4 часть этого текста устареет: сетевой сбой +станет самостоятельно устраняемым повторным нажатием «Редактировать», а +несвежий entry — понятным баннером вместо тишины. ТЗ (как и полагается +лёгкому треку, §5) не обязано перечислять release-артефакты явно, а +`docs/**` — документация в тему той же задачи по DoD (PROCESS.md §2.6, +правило 11: «документация — в том же коммите, что поведение»), так что +формально это не пробел ТЗ. Снимаю находку записью здесь, а не как +блокирующую: автору стоит обновить эти два места в USER-GUIDE.ru.md в том +же коммите, что и код, но это не требует правки самого ТЗ. + +## Что проверено и корректно + +- **Скоуп и трек.** Задача чинит регресс поведения уже принятого механизма + #337 (терминальность/несвежесть lazy-доставки), не расширяет продукт — + укладывается в J1/J4/J6 SCOPE.md («редакторы остаются доступны», + «план остаётся верным»). Критерии лёгкого трека (§5) все выполнены + формально: одна логическая поверхность (конвейер доставки lazy-бандла), + риск заявлен ≤3, миграций конфига нет, новый UX-контракт не вводит новых + элементов интерфейса (то же нажатие «Редактировать», тот же тост). +- **K1 (терминальность loader'а).** Логика «mismatch на ЛЮБОЙ попытке ⇒ + терминально, иначе ⇒ `idle` и новый явный `ensure()`» согласована с + существующим кодом `_loadWithRetry` (attempt 1 использует + cache-buster — то есть mismatch на attempt 1 после успешного обхода кэша + действительно означает версионный дрейф, а не транзиентный сетевой сбой) + и с уже существующим тестом (`editor-runtime-loader.test.mjs:61-83`), + который остаётся зелёным без правки ассертов. AC1/AC2 однозначны и + проверяемы юнитом. +- **N4/K5 (осиротевшие чанки).** Утверждение «`verifyBundleTree` не + детектирует лишние файлы вне манифеста» подтверждено чтением + `scripts/bundle-tree.mjs:38-53` буквально — функция итерирует только + `manifest.files`, каталог не сканирует. Фикс и AC4 просты и корректны. +- **Проверенный факт вместо догадки.** Утверждение К3 «именованный экспорт + entry исчезает — потребителей нет… проверено grep'ом» подтверждено + независимо: единственное место, где entry импортируется как модуль — + `demo/srv/demo.html:193`, `await import(...)` без захвата именованного + экспорта; в `custom_components/**`, `test/**` потребителей нет. Это не + голословная догадка, а верно проверенный факт. +- **K4 (immutable Cache-Control).** Согласуется с текущим кодом + (`frontend_assets.py:34` — `no-cache` меняется на `public, + max-age=31536000, immutable`; `__init__.py:100/109` — entry остаётся + `cache_headers=False`, K4 явно это не трогает). Хэшированные имена чанков + делают инвалидацию неактуальной проблемой — решение обратимо, откат + корректен. +- **Откат.** Один revert, конфиг/схема/миграции не затронуты — соответствует + фактическому масштабу изменения (два флага поведения + один rollup-плагин + + один HTTP-заголовок + одна проверка каталога). +- **i18n.** Новый ключ `editor.retry_advice` вписывается по образцу + существующих `editor.load_failed`/`editor.refresh_advice`, которые уже + синхронны между `en.json`/`ru.json`/`de.json`. + +## Чего не проверял + +- Не прогонял `typecheck`/`test`/`build`/`check-docs`/`smoke-select`/ + `model-invariants` — на этапе ревью ТЗ кода по issue #353 в дереве нет + (ветка `issue/353-*` не создана), сравнивать не с чем; эти гейты + относятся к код-ревью (§2.7, §8) и будут прогнаны в следующем цикле, + когда появится диапазон коммитов. +- Не оценивал реальное поведение HA Lovelace (`res_type: module`) при + гонке из находки High — оценка ограничена наблюдаемым фактом, что + demo-стенд ломается гарантированно; поведение реального HA-фронтенда + (который, вероятно, толерантен к асинхронной регистрации через + `customElements.whenDefined` с таймаутом) не проверялось и не входит в + этот репозиторий — не меняет вывода: даже если прод HA переживёт гонку, + собственный демо-стенд проекта, на котором держится весь браузерный слой + гейтов, не переживёт, и этого достаточно для блокирующей находки. +- Не пересчитывал вручную содержимое `initialViewGzipBytes`/бюджета #352 + после гипотетической реализации K3: проверено чтением `bundle-manifest.mjs`, + что `chunk.imports`/`chunk.dynamicImports` — метаданные, которые Rollup + фиксирует до `generateBundle`, и последующая строковая мутация + `chunk.code` (аналогично уже существующему `editorRuntimeRetryUrlPlugin`) + их не меняет; отдельной находки по бюджету #352 в этом ТЗ не завожу, + так как классификация initial/lazy остаётся корректной независимо от K3.