mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,190 @@
|
||||
# SPEC-REVIEW-353-r2
|
||||
|
||||
Issue: #353 — «Устойчивость lazy-загрузки: терминальный отказ редактора после
|
||||
сетевого сбоя и мёртвый entry-лоадер после обновления»
|
||||
Этап: ревью ТЗ (PROCESS.md §2.4), лёгкий трек (`small`), заход r2,
|
||||
блокирующих циклов израсходовано 1 из 2 (лимит §4: 2 на лёгком треке).
|
||||
ТЗ живёт в теле issue #353 (лёгкий трек, файла в `docs/specs/` нет).
|
||||
|
||||
Предыдущий раунд: `docs/reviews/SPEC-REVIEW-353-r1.md` (в дереве на
|
||||
`07b6a1f2`, опубликован коммитом `5044c281`), вердикт жёлтый, High: 1 ·
|
||||
Medium: 1 (в скоупе) · Low: 1 (снят с запиской). Материал предыдущего раунда
|
||||
— тело issue на момент комментария Matysh `2026-08-28T11:01:48Z`
|
||||
(S2→S3). Материал этого раунда — текущее тело issue #353, помеченное автором
|
||||
как «Ревизия 2 (по SPEC-REVIEW-353-r1)» (комментарий Matysh,
|
||||
`2026-08-28T11:14:34Z`, вернувший `S4-spec-review`).
|
||||
|
||||
Для ТЗ нет SHA — предмет ревью текст, а не код; дельта раунда — правки тела
|
||||
issue между ревизией 1 (разобрана в r1) и ревизией 2 (этот раунд), названные
|
||||
автором в комментарии и в приписке в конце тела issue.
|
||||
|
||||
## Вердикт
|
||||
|
||||
**Зелёный.** High: 0 · Medium: 0. Оба блокирующих замечания r1 закрыты
|
||||
текстом ревизии 2 предметно, без появления новых High/Medium. Один Low
|
||||
(остаточная неточность формулировки AC5) отмечаю и снимаю запиской — не
|
||||
блокирует.
|
||||
|
||||
## Скоуп разбора — по дельте (PROCESS.md §2.10)
|
||||
|
||||
Дельта локальна: правки ограничены разделом «К3» (механизм постобработки
|
||||
entry-чанка) и разделом «AC и доказательства» (добавлен AC5). Остальной текст
|
||||
ТЗ — «Проблема», критерии приёмки issue, К1/К2/К4/К5 (кроме способа доставки
|
||||
тоста в К2, который делит AC5), «Откат», `User-Visible` — не редактировался
|
||||
между ревизиями (совпадает дословно с текстом, разобранным в r1). Изменение
|
||||
не является ребейзом (кода ещё нет), не меняет контракт поведения сверх того,
|
||||
что r1 уже потребовал исправить, не задевает новую подсистему и по объёму
|
||||
меньше исходной задачи — полный повторный разбор всего ТЗ не требуется;
|
||||
разбираю дельту и всё, до чего она дотягивается (AC3 целиком, поскольку К3
|
||||
переписан; AC5 целиком, поскольку это новый раздел).
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка r1 | Чем закрыта | Где это видно |
|
||||
|---|---|---|
|
||||
| **[High] K3** — замена статического реэкспорта на «голый» `import().catch()` резолвит `await import(entry)` у потребителя (демо-стенда) раньше, чем главный чанк успевает выполниться и зарегистрировать `customElements.define('houseplan-card', …)`; ломает `document.createElement('houseplan-card').setConfig(...)` в `demo/srv/demo.html:193-197`, то есть счастливый путь всего браузерного слоя гейтов. | К3 переписан на top-level await внутри самого entry-чанка: `try { await import("./houseplan-assets/houseplan-card-<hash>.js"); } catch (e) { /* define fallback */ }`. Это ровно первый из двух путей, которые r1 предложил как исправление («entry сохраняет цепочку ожидания — top-level await внутри самого чанка… чтобы `await import(entry)` по-прежнему резолвился только после того, как главный чанк загружен либо фолбэк-элемент определён»). Семантика TLA в ES-модулях (`format: 'es'`, `rollup.config.mjs:22`) гарантирует, что entry не завершает собственное выполнение, пока вложенный `import()` не разрешится — то есть `await import(entry)` у `demo/srv/demo.html:193` по-прежнему ждёт `customElements.define`, как и до К3. `demo.html` не правится — инвариант сохранён без изменения потребителя, как и требовал r1. | Тело issue #353, раздел «К3 — entry переживает несвежесть, не трогая счастливый путь» (блок кода `try { await import(...) } catch (e) {...}`) + предложение «Семантика TLA: модуль-потребитель… не резолвится, пока entry не довыполнится… гарантия… сохраняется без правки `demo.html`». |
|
||||
| **[Medium, в скоупе] K2** (тост на каждый неудачный цикл, ключ по `terminal`) не имел ни одного AC — реализация, всегда показывающая `refresh_advice`, проходила весь набор AC1–AC4. | Добавлен **AC5**: юнит на чистую функцию `lazyLoadFailureMessage(t, {terminal})` для обеих комбинаций (`true`→`refresh_advice`, `false`→`retry_advice`) + структурная проверка «в духе `test/houseplan-source.mjs`», что `failed`-колбэки ОБОИХ лоадеров (`_editorRuntimeLoader`, `_onboardingRuntimeLoader` в `src/houseplan-card.ts:830-835,861-865` на момент r1) вызывают именно этот общий помощник, а не строят текст тоста сами. | Тело issue #353, раздел «AC и доказательства», пункт «**AC5** (закрывает К2)»; раздел «Контракт», К2: «Оба лоадера… выбирают текст через ОБЩИЙ экспортируемый помощник `lazyLoadFailureMessage(t, { terminal })`». |
|
||||
| **[Low, снято с запиской в r1] USER-GUIDE.ru.md** не в объёме задачи. | Без изменений — уже снято в r1 с запиской «чинится в том же коммите реализации», ревизия 2 её подтверждает явной строкой. | Тело issue #353: «Попутно (Low r1): затронутые места docs/USER-GUIDE.ru.md обновляются в том же коммите реализации.» |
|
||||
|
||||
## Как проверялось (дельта)
|
||||
|
||||
1. Перечитан весь текст тела issue #353 на текущий момент и сверен построчно
|
||||
с цитатами r1 (что изменилось — К3 и «AC и доказательства»; что нет —
|
||||
«Проблема», критерии issue, К1/К4/К5, «Откат», `User-Visible`).
|
||||
2. К3 разобран построчно на согласованность заявленного механизма (TLA) с
|
||||
форматом сборки: `rollup.config.mjs` — `input: 'src/houseplan-card.ts'`,
|
||||
`output.format: 'es'`, `intro` инжектирует фингерпринт, плагины
|
||||
`editorRuntimeRetryUrlPlugin()` → `bundleManifestPlugin(...)` в этом
|
||||
порядке в `generateBundle`. Новый плагин описан «рядом с
|
||||
`editorRuntimeRetryUrlPlugin`… ДО `bundleManifestPlugin`» — та же семья
|
||||
постобработки, тот же порядок, что уже проверен и принят в r1 для
|
||||
аналогичной строковой мутации кода чанка.
|
||||
3. Проверено рассуждение (без исполнения — кода ещё нет): TLA в
|
||||
ES-модулях означает, что evaluation entry-модуля не завершается, пока
|
||||
вложенный `await import(...)` не разрешится; следовательно
|
||||
`await import(entry)` любого потребителя (в т.ч. `demo/srv/demo.html:193`)
|
||||
транзитивно ждёт того же события, что и раньше (выполнение главного чанка,
|
||||
`customElements.define`). Ровно то свойство, разрыв которого был предметом
|
||||
High в r1.
|
||||
4. Сверено, что «именованный экспорт entry исчезает — потребителей нет»
|
||||
осталось тем же утверждением, которое r1 уже независимо перепроверил
|
||||
grep'ом (`demo/srv/demo.html:193` — `await import(...)` без захвата
|
||||
именованного экспорта; в `custom_components/**`, `test/**` потребителей
|
||||
нет) — дельта его не меняет, факт наследуется.
|
||||
5. AC5 сверен с текущим кодом `src/houseplan-card.ts` (класс `HouseplanCard`,
|
||||
оба `EditorRuntimeLoader`, поля `_editorRuntimeLoader`/
|
||||
`_onboardingRuntimeLoader`, оба колбэка `failed: (error) => {...}`,
|
||||
`console.error(...)` + `this._showToast(...editor.refresh_advice)`) —
|
||||
подтверждено, что сегодня оба колбэка игнорируют причину отказа и всегда
|
||||
показывают один и тот же текст; ровно это АС5 обязана ловить.
|
||||
6. Перечитан `test/houseplan-source.mjs` — существующий образец
|
||||
«структурной проверки» в этом проекте: AST-разбор TS-исходников через
|
||||
`typescript` (`ts.createSourceFile`), сверка членов класса, а не
|
||||
простой grep по подстроке. Это делает правдоподобным заявление АС5, что
|
||||
структурная проверка способна отличить «вызывает помощник с прокинутым
|
||||
`info.terminal`» от «вызывает помощник с захардкоженным аргументом» —
|
||||
но ТЗ не проговаривает это явно (см. Low ниже).
|
||||
7. Git: рабочее дерево чистое на `5044c281` (= `origin/dev`), ветки
|
||||
`issue/353-*` нет — код по задаче не начат, соответствует этапу
|
||||
`S4-spec-review`.
|
||||
|
||||
### Гейты — что прогнано и почему
|
||||
|
||||
Не применимо к этому раунду: этап — ревью ТЗ, кода по issue #353 в дереве
|
||||
по-прежнему нет (ветка `issue/353-*` не создана, дельта — правка текста
|
||||
issue, не коммит). `typecheck`/`test`/`build`/`check-docs`/`smoke-select`/
|
||||
`model-invariants` относятся к код-ревью (§2.7, §8) и будут прогнаны, когда
|
||||
появится диапазон коммитов. Это то же решение, что и в r1, не молчаливый
|
||||
пропуск.
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Принято без повторной проверки в этом раунде — не задето дельтой:
|
||||
|
||||
- **Скоуп и трек.** Задача остаётся в J1/J4/J6 SCOPE.md, критерии лёгкого
|
||||
трека (§5) выполнены. — `docs/reviews/SPEC-REVIEW-353-r1.md`, раздел «Что
|
||||
проверено и корректно», на `07b6a1f2`.
|
||||
- **К1 (терминальность loader'а)** и AC1/AC2 — логика «mismatch на любой
|
||||
попытке ⇒ терминально, иначе ⇒ `idle`» согласована с
|
||||
`test/editor-runtime-loader.test.mjs:61-83` и текущим `_loadWithRetry`. —
|
||||
там же, раздел «К1», на `07b6a1f2`.
|
||||
- **К5/N4 (осиротевшие чанки), AC4** — `scripts/bundle-tree.mjs:38-53`
|
||||
подтверждённо не сканирует каталог вне манифеста; фикс и AC4 просты. —
|
||||
там же, раздел «N4/K5», на `07b6a1f2`.
|
||||
- **К4 (immutable Cache-Control)** — согласуется с `frontend_assets.py:34`,
|
||||
`__init__.py:100/109`; откат корректен. — там же, раздел «K4», на
|
||||
`07b6a1f2`.
|
||||
- **i18n** — новый ключ `editor.retry_advice` по образцу существующих,
|
||||
словари синхронны. — там же, на `07b6a1f2`.
|
||||
- **Бюджет #352 / классификация initial-lazy графа.** Постобработка
|
||||
`generateBundle` (строковая мутация уже посчитанного Rollup чанка) не
|
||||
меняет `chunk.imports`/`chunk.dynamicImports`, зафиксированные до
|
||||
генерации бандла — рассуждение в r1 строилось на существующем
|
||||
`editorRuntimeRetryUrlPlugin`, и новый TLA-плагин относится к той же
|
||||
семье постобработки (см. «Как проверялось», п.2). Отдельно не
|
||||
пересчитывал. — `docs/reviews/SPEC-REVIEW-353-r1.md`, раздел «Чего не
|
||||
проверял», на `07b6a1f2`.
|
||||
- **Поведение реального HA-фронтенда** при аналогичной гонке — не
|
||||
оценивалось ни в r1, ни здесь; вывод не меняется — демо-стенд остаётся
|
||||
критерием, и он теперь гонки не имеет.
|
||||
|
||||
## Находки
|
||||
|
||||
### [Low, снято с запиской] AC5 не проговаривает, что структурная проверка обязана отличать «параметр `info.terminal` прокинут в вызов» от «вызов есть, но аргумент захардкожен»
|
||||
|
||||
**Файл/раздел:** тело issue #353, «AC и доказательства», пункт AC5.
|
||||
|
||||
АС5 утверждает: «структурная проверка… `failed`-колбэки ОБОИХ лоадеров…
|
||||
вызывают `lazyLoadFailureMessage` (реализация «всегда refresh_advice»
|
||||
падает)». Формулировка «вызывают `lazyLoadFailureMessage`» буквально
|
||||
описывает только факт вызова. Реализация, которая зовёт помощник, но
|
||||
захардкоживает `{ terminal: true }` вместо прокидывания параметра `info`
|
||||
колбэка, тоже «вызывает `lazyLoadFailureMessage`» — и без уточнения, что
|
||||
проверяется именно происхождение аргумента (идентификатор `info`/
|
||||
`info.terminal`, а не литерал), такая проверка может пропустить именно ту
|
||||
регрессию, которую AC5 призван ловить (тот же дефект N2, просто
|
||||
воспроизведённый на уровне выбора текста, а не состояния лоадера). Это не
|
||||
голословное сомнение: раздел «Мутанты в реестр» ТЗ перечисляет 5 мутантов
|
||||
для AC1–AC4, но ни один — для AC5, хотя ровно AC5 существует для отлова
|
||||
конкретного класса регрессии (игнорирование `terminal`).
|
||||
|
||||
Не блокирую: `test/houseplan-source.mjs` — существующий в проекте образец
|
||||
именно такой AST-проверки (разбирает TS через `ts.createSourceFile`,
|
||||
сверяет структуру, а не текстовые подстроки), так что технически
|
||||
реализуемо и укладывается в стиль проекта; способ доказательства и его
|
||||
точность — по правилу «в теле issue сказано `в духе test/houseplan-source.mjs`»
|
||||
уже названы, а точная форма AST-проверки — техническая деталь реализации,
|
||||
которую решает автор (PROCESS.md: «где хранится состояние… стратегия
|
||||
теста… решай сам»). Записываю как пункт для код-ревью: убедиться, что
|
||||
реализованный тест действительно падает на мутанте «захардкоженный
|
||||
`terminal`» — то есть на дисциплину «тест умеет падать», применённую к
|
||||
этому конкретному AC.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- К3 (ревизия 2) закрывает High r1 предметно: TLA-обёртка сохраняет
|
||||
гарантию «`await import(entry)` резолвится не раньше, чем главный чанк
|
||||
выполнен либо фолбэк определён», без правки `demo.html` — именно то
|
||||
решение, которое r1 предложил как один из двух допустимых путей.
|
||||
- AC5 закрывает Medium r1: К2 (выбор текста тоста по `terminal`, для обоих
|
||||
лоадеров) теперь имеет явный AC и явно названный проверяемый дефект
|
||||
(«всегда refresh_advice»).
|
||||
- Плагин К3 задан в той же точке пайплайна (`generateBundle`, до
|
||||
`bundleManifestPlugin`), что и уже принятый `editorRuntimeRetryUrlPlugin`
|
||||
— не открывает новых вопросов к бюджету #352 сверх уже принятого в r1.
|
||||
- Остальной контракт (К1, К4, К5, «Откат», `User-Visible`) не редактировался
|
||||
между ревизиями — выводы r1 по ним стоят без изменений.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не прогонял `typecheck`/`test`/`build`/`check-docs`/`smoke-select`/
|
||||
`model-invariants` — кода по issue #353 в дереве нет (см. «Гейты» выше).
|
||||
- Не проверял заново К1, К4, К5, критерии лёгкого трека, i18n-синхронность
|
||||
и бюджет-рассуждение — не задеты дельтой r1→r2, унаследованы из
|
||||
`SPEC-REVIEW-353-r1.md` (раздел «Унаследовано из r1» выше).
|
||||
- Не оценивал реальную поддержку top-level await целевыми окружениями
|
||||
(HA frontend resource, `res_type: module`) — техническая деталь
|
||||
реализации; TLA в ES-модулях — часть спецификации ECMAScript, поддержана
|
||||
всеми актуальными эвергрин-браузерами, отдельного продуктового риска не
|
||||
усматриваю и не отношу к продуктовой неоднозначности, которую решает
|
||||
владелец.
|
||||
Reference in New Issue
Block a user