mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -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-<hash>.js`) гарантированно не отработает.
|
||||
|
||||
К3 предписывает заменить это на `import("./houseplan-assets/…").catch(…)`
|
||||
— **не** `export`, а голый вызов внутри тела entry-чанка, судя по
|
||||
формулировке и по аналогии с `editorRuntimeRetryUrlPlugin` (пост-обработка
|
||||
уже сгенерированного `chunk.code` строковой заменой в `generateBundle`,
|
||||
после того как Rollup уже посчитал граф). Promise такого `import()` никуда
|
||||
не экспортируется и не await'ится потребителем entry-модуля. Значит,
|
||||
top-level выполнение entry-чанка завершается почти сразу — до того, как
|
||||
динамически запрошенный `houseplan-card-<hash>.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.
|
||||
Reference in New Issue
Block a user