mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-01 20:29:00 +00:00
claude[bot]
@@ -0,0 +1,280 @@
|
||||
# SPEC-REVIEW-535-r1
|
||||
|
||||
**Issue:** #535 — «Страница панели грузит карточку по URL без версии: плашка о несовпадении версий переживает перезагрузку на /houseplan»
|
||||
**Этап:** S4-spec-review (ревью ТЗ, PROCESS.md §2.4)
|
||||
**Заход:** r1 · блокирующих циклов израсходовано 0 из 4
|
||||
**Ревьюер:** Claude (Sonnet 5), роль «ревьюер ТЗ»
|
||||
**Автор ТЗ:** Matysh (в теле issue, раздел `## ТЗ`)
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Тело issue #535, раздел `## ТЗ` (включая «Продуктовая рамка», К1–К6, «Принято
|
||||
предположительно», таблица AC1–AC6, «Риски», «Затронутые файлы», «Откат»).
|
||||
- SHA256 нормализованного тела issue (как получено через `gh issue view --json body`,
|
||||
без дополнительной нормализации переносов строк):
|
||||
`77204930eaea05482f2a7da2bac3af79247e7d7356e0449551cb997da2c76fc8`
|
||||
- Оба комментария issue (S2-аналитика, «ТЗ готово — на ревью»).
|
||||
- Код на момент ревью: `dev` @ `1ca3b5fae3e2df9fbd41ead35d98c8d58c87f5b8` (рабочая
|
||||
копия чиста, HEAD == origin/dev). Читался только для проверки выполнимости и
|
||||
однозначности ТЗ, продуктовый код не менялся.
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Проверялись: обязательные разделы ТЗ (PROCESS.md §7.1), однозначность и
|
||||
проверяемость каждого AC, наличие способа доказательства и «чем краснеет»,
|
||||
отсутствие невыделенных догадок о поведении, соответствие `docs/SCOPE.md`,
|
||||
согласованность контракта К1–К6 с текущей реализацией `entryFallbackPlugin`
|
||||
(`scripts/bundle-manifest.mjs`) и существующими тестами `#486` в
|
||||
`test/bundle-assets.test.mjs`, а также спорный вопрос классификации трека,
|
||||
который автор прямо адресовал ревьюеру.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Прочитан `docs/SCOPE.md` — задача не вводит новую функциональность, это
|
||||
правка доставки уже существующего продукта (сходится по духу с J6 «Keep the
|
||||
plan true», хотя формально это баг доставки бандла, а не пользовательской
|
||||
фичи — отдельного пункта J-карты не требует).
|
||||
2. Прочитан `PROCESS.md` §1 (границы классов файлов), §2.4, §2.5 (DoR-чеклист,
|
||||
чтобы понять, что должно быть закрыто до перехода в `S5-ready`), §7.1
|
||||
(обязательные разделы ТЗ), §7.2 (формат вердикта).
|
||||
3. Прочитано тело issue #535 целиком и оба комментария.
|
||||
4. Прочитан текущий код `entryFallbackPlugin` в `scripts/bundle-manifest.mjs`
|
||||
(строки 300–386) — как сегодня панель переписывается на импорт фасада, как
|
||||
считается `imports`/`initialPanelFiles`/`initialPanelOnlyGzipBytes`
|
||||
(`reachable()`, `initialPanelOnly`).
|
||||
5. Прочитаны оба существующих теста `#486` в `test/bundle-assets.test.mjs`
|
||||
(`both stable entries install a visible stale-load fallback`,
|
||||
`panel entry routes through the card facade and fails loudly too`), чтобы
|
||||
проверить, действительно ли их утверждения пиннуют именно фасадную
|
||||
маршрутизацию (К5) и действительно ли они «переворачиваются» предложенной
|
||||
правкой, а не остаются случайно истинными.
|
||||
6. Прочитан фрагмент `docs/ARCHITECTURE.md` про два стабильных входа
|
||||
(строки 69–86) — проверить, не противоречит ли К3/АС6 уже
|
||||
зафиксированному описанию маршрутизации.
|
||||
7. Прослежена логика графа вручную (`reachable()` от `entry`=facade и от
|
||||
`panelEntry`), чтобы проверить арифметику риска №2 (бюджет) и корректность
|
||||
К4/AC4 до и после предложенной правки.
|
||||
|
||||
Гейты (typecheck/test/build/golden/backend) не гонялись — это этап ревью ТЗ,
|
||||
продуктовый код ещё не написан; §2.4 их не требует.
|
||||
|
||||
## Разбор по существу
|
||||
|
||||
### Обязательные разделы ТЗ (§7.1)
|
||||
|
||||
Присутствуют по содержанию, хотя не всегда под буквальными заголовками:
|
||||
|
||||
| Раздел §7.1 | Есть? | Где |
|
||||
|---|---|---|
|
||||
| Сценарий | ✅ | «Продуктовая рамка»: персона — домочадцы и администратор, поверхность — пункт меню House Plan |
|
||||
| Что человек увидит до/после | ✅ | «До.» / «После.» |
|
||||
| Проблема | ✅ | Симптом + «Причина найдена и измерена» + «Как это даёт ровно наблюдаемое поведение» в теле issue до ТЗ |
|
||||
| Скоуп и не-скоуп | ⚠️ частично | Не-скоуп явно перечислен в комментарии S2 («Чего в этой задаче нет»: формулировка плашки, #536, заголовки прокси) и повторён в комментарии «ТЗ готово», но **не продублирован внутри самого раздела `## ТЗ`** |
|
||||
| Контракт поведения | ✅ | К1–К6 |
|
||||
| UX | ✅ | К2 (текст фолбэка дословно, 4 языка, класс элемента) |
|
||||
| Модель данных и миграция | ⚠️ подразумевается | Нет явного «миграция: нет»; из контекста (только `scripts/**`, `test/**`, документация, три копии бандла) очевидно, что миграции нет |
|
||||
| i18n | ✅ (по факту «нет новых ключей») | К2: текст фолбэка не меняется ни в одном из 4 языков |
|
||||
| AC1…ACn с доказательством | ✅ | Таблица AC1–AC6, у каждого «чем доказан» и «чем краснеет» |
|
||||
| План автотестов | ✅ | Встроен в таблицу AC (имена тестов/файлов) |
|
||||
| Риски | ✅ | 4 пункта |
|
||||
| Откат | ✅ | Один абзац, простой и точный |
|
||||
| Release-артефакты | ✅ | Названы в преамбуле ТЗ: `docs/ARCHITECTURE.md`, оба changelog |
|
||||
| Влияние на touch | ❌ не названо явно | Задача не трогает ни один UI/editor/touch-контракт (только сборочный конвейер и панельный/карточный загрузчик); фактическое влияние — «нет», но фраза отсутствует |
|
||||
|
||||
Два ⚠️/❌ пункта (модель данных/миграция и touch — явные «нет» не написаны)
|
||||
разбираю ниже как находки Low.
|
||||
|
||||
### Техническая состоятельность контракта (К1–К6) и AC
|
||||
|
||||
Проверил код `entryFallbackPlugin` вручную, чтобы понять, действительно ли
|
||||
предложенная правка (импортировать `cardAsset` напрямую вместо фасада,
|
||||
сохранив `try/catch`) реализуема ровно так, как описано, и действительно ли
|
||||
существующие тесты `#486` меняют смысл, а не остаются проходить случайно.
|
||||
|
||||
- **К1/AC1/AC2 реализуемы буквально.** Сегодня `panelEntry.code` переписывается
|
||||
regex'ом `panelPattern` (совпадающим с уже существующим статическим импортом
|
||||
`cardAsset`, который туда кладёт сам Rollup) на
|
||||
`try{await import("./houseplan-card.js")}catch{...}`, и `panelEntry.imports`
|
||||
жёстко ставится в `[CARD_ENTRY_FILE]` (`scripts/bundle-manifest.mjs:377–383`).
|
||||
Предложенная правка меняет ровно эти две строки: подстановка в `try{await
|
||||
import("${cardAsset}")}...` и `panelEntry.imports = [cardAsset]`. Никакого
|
||||
скрытого усложнения нет — это буквально «убрать один слой переадресации»,
|
||||
как и заявлено в «Откате».
|
||||
- **К5/тесты `#486` действительно переворачиваются, а не остаются случайно
|
||||
зелёными.** Прочитал оба теста построчно (`test/bundle-assets.test.mjs:305–360`
|
||||
и `:522–546`). Первый матчит `panel` на `try{await
|
||||
import("./houseplan-card.js")}catch` и `bundle['houseplan-panel.js'].imports
|
||||
=== ['houseplan-card.js']` — оба утверждения станут ложными на новом коде и
|
||||
должны замениться на `.../houseplan-assets/card-HASH.js` и
|
||||
`['houseplan-assets/card-HASH.js']` соответственно, что ровно и обещает AC1.
|
||||
Второй тест читает **собранный** `dist/houseplan-panel.js` и матчит ту же
|
||||
строку `./houseplan-card.js` — тоже переворачивается, ровно как обещает AC2
|
||||
(«проверка по `dist`, а не по плагину»).
|
||||
- **К4/AC4 — риск №1 в ТЗ корректно предсказывает реальную поломку
|
||||
существующего утверждения о манифесте**, а не выдуманную. Прошёл вручную
|
||||
логику `reachable()`: сейчас `initial` = reachable(entry=`houseplan-card.js`)
|
||||
включает сам фасад как корень; `initialPanel` = reachable(panel) = {panel} ∪
|
||||
`initial`, потому что `panelEntry.imports=[CARD_ENTRY_FILE]`. Отсюda
|
||||
`initialViewFiles.every(p => initialPanelFiles.includes(p))` истинно
|
||||
тривиально. После правки `panelEntry.imports=[cardAsset]`, и `initialPanel`
|
||||
перестаёт включать сам файл фасада (`houseplan-card.js`) — утверждение в
|
||||
строке 542 текущего теста station станет **ложным**, именно это и называет
|
||||
риск №1 ТЗ («написано под фасад... надо переформулировать через граф
|
||||
реализации»). Это открытое место корректно помечено как риск, а не выдано за
|
||||
решённое — соответствует правилу «размытое место выносится, а не
|
||||
додумывается». Как именно переформулировать сравнение (через граф от
|
||||
`cardAsset`, а не от `entry`) — техническая деталь реализации, которую §7.1
|
||||
прямо отдаёт исполнителю («агенты решают сами... стратегия тестов»), поэтому
|
||||
не считаю это находкой ревью ТЗ.
|
||||
- **Риск №2 (бюджет) арифметически неточен, но не вреден.** По той же
|
||||
логике `initialPanelOnlyGzipBytes` = `initialPanel − initial` — множество,
|
||||
которое **и до, и после** правки состоит только из самого файла панели
|
||||
(фасад всегда был частью `initial`, а не эксклюзивным для панели), то есть
|
||||
сам метрический бюджет `initialPanelOnlyGzipBytes`, скорее всего, не
|
||||
сдвинется на «размер фасада», сдвинется общий размер `initialPanelFiles`
|
||||
(граф целиком). Это неточность в обосновании риска, а не в контракте: ТЗ уже
|
||||
предусматривает митигацию («рекалибруется с датированной записью, а не тихо
|
||||
поднимается») независимо от того, какая метрика реально изменится и на
|
||||
сколько. AC4 всё равно остаётся проверяемым (`bundle-budget.mjs` плюс
|
||||
утверждения теста), поэтому не блокирую — но называю ревьюеру кода: при
|
||||
проверке AC4 стоит явно свериться, какая метрика в итоге изменилась, и что
|
||||
это совпадает с тем, что записано в риске/changelog.
|
||||
- **К3 (фасад карточки не трогается) не имеет отдельного AC, и это
|
||||
корректно** — правка физически не касается кода, рисующего `cardEntry.code`
|
||||
(строки 344–358 `bundle-manifest.mjs`), и существующая (переписываемая, но не
|
||||
удаляемая) часть тех же двух тестов `#486`, которая проверяет
|
||||
карточную сторону, продолжает быть тем самым свидетелем регрессии.
|
||||
- **AC3 (фолбэк дословно)** — проверил, что механизм построения текста
|
||||
(`fallbackDefinition`) вообще не тронут К1; он используется одинаково для
|
||||
обоих контрактов и не зависит от того, что импортируется. Корректно.
|
||||
- **AC5 (общедистрибутивный инвариант)** — новый тест, скоуп ясен
|
||||
(«ни один входной файл не ссылается на карточку без версии» по `dist/*.js`),
|
||||
однозначен и проверяем.
|
||||
- **AC6** помечен как доказываемый «ревьюером кода построчно» — это
|
||||
недефенсивный AC (расположение текста в документации), правило §2.7 про
|
||||
таблицу «чем краснеет» на него не распространяется по собственной оговорке
|
||||
процесса.
|
||||
|
||||
### Классификация трека — вопрос, адресованный ревьюеру
|
||||
|
||||
Автор прямо просит решить: инфраструктурная это задача (по механическому
|
||||
признаку §1 класса файлов — да, только `scripts/**`/`test/**`/документация) или
|
||||
полный трек (по факту видимого поведения). Довод автора: на устаревшем
|
||||
загрузчике сегодня пользователь **молча** получает старую карточку; после
|
||||
правки он **всегда** увидит громкий фолбэк в тех случаях, где раньше молча
|
||||
работал (или не работал вовсе) старый код. Проверил механику вручную
|
||||
(см. выше) — это не гипотеза: до правки возможен путь, где старый `houseplan-panel.js`
|
||||
из кэша браузера подтягивает **свежий** фасад `houseplan-card.js` (тоже без
|
||||
версии, тоже подверженный эвристике, но не обязательно ровно такой же
|
||||
устаревший), который в свою очередь тянет уже новый чанк — рабочая, но
|
||||
рассинхронизированная версия без единой ошибки. После правки этот путь
|
||||
исчезает: старый панельный вход всегда просит старый (и, по контракту сервера
|
||||
из `docs/ARCHITECTURE.md:82–84`, отсутствующий после деплоя) чанк напрямую и
|
||||
надёжно показывает фолбэк. Это действительно новый наблюдаемый контракт (не
|
||||
новый текст, но новое условие срабатывания), и довод автора о нарушении
|
||||
критерия §5 «нет нового UX-контракта» подтверждается чтением кода, а не
|
||||
принимается на веру. **Соглашаюсь с автором: полный трек оправдан.** Это
|
||||
решает технический спор в пользу продолжения ревью, как и предписывает §7.1
|
||||
(«технический спор автора и ревьюера решается вердиктом»).
|
||||
|
||||
### Проверка «не выдана ли догадка за решение»
|
||||
|
||||
Прошёлся по всем формулировкам поведения на предмет непомеченных допущений:
|
||||
|
||||
- Утверждение «загрузчик выбирает версию всей карточки» и таблица заголовков
|
||||
кэша — это измерение на боевом стенде (проверяемый факт, не догадка).
|
||||
- «Через какое-то время окно истекает... и всё само чинится» — помечено как
|
||||
объяснение уже наблюдавшегося поведения («не всегда», «точных шагов не
|
||||
нашёл»), не выдаётся за новый AC.
|
||||
- Три пункта «Принято предположительно» корректно помечены как техническое
|
||||
предположение, которое можно менять свободно, включая явную причину отказа
|
||||
от вариантов B и C.
|
||||
- Риск №1 и №2 корректно поданы как риски, а не как решённые факты (см. выше).
|
||||
|
||||
Непомеченных догадок о поведении, которого нет ни в одном документе, не нашёл.
|
||||
|
||||
## Находки
|
||||
|
||||
Блокирующих (High) находок нет. Находок Medium в скоупе или вне скоупа нет.
|
||||
|
||||
### Low (сняты решением ревьюера, без правки)
|
||||
|
||||
1. **Не-скоуп не продублирован внутри `## ТЗ`.** Перечень «чего в этой задаче
|
||||
нет» (формулировка плашки, побочная находка #536, заголовки прокси) есть в
|
||||
комментарии S2-аналитики и повторён в комментарии «ТЗ готово», но не
|
||||
переписан внутри самого раздела ТЗ в теле issue. Содержательно вопрос
|
||||
закрыт и однозначен — при чтении всего issue целиком путаницы не возникает,
|
||||
поэтому не считаю это основанием для возврата. Снимаю с записью: для
|
||||
исполнителя это не создаёт риска, потому что ветка/PR будут ссылаться на тот
|
||||
же issue целиком.
|
||||
2. **Явное «миграция: нет» и «touch: нет» не написаны отдельной строкой**, как
|
||||
того просит чек-лист DoR §2.5. По содержанию задачи (только
|
||||
`scripts/**`/`test/**`/документация/три копии бандла, никакого нового
|
||||
конфига, никакого UI/editor-контракта) оба пункта очевидно «нет». Снимаю
|
||||
с записью: не считаю нужным гонять задачу на ещё один цикл ради двух строк
|
||||
текста; ревьюер кода или сам автор могут дописать их при переводе в
|
||||
`S5-ready` без нового цикла ревью ТЗ.
|
||||
3. **Риск №2 (бюджет) арифметически не точен** (см. разбор AC4 выше): скорее
|
||||
всего сдвинется не `initialPanelOnlyGzipBytes`, а общий размер
|
||||
`initialPanelFiles`. Митигация в риске («рекалибруется явно, а не тихо»)
|
||||
покрывает оба случая одинаково, поэтому не блокирую, но передаю ревьюеру
|
||||
кода как пункт для проверки при разборе AC4.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Все обязательные по существу элементы §7.1 присутствуют (сценарий, что
|
||||
человек увидит, проблема, контракт, AC с доказательством, риски, откат,
|
||||
release-артефакты).
|
||||
- Каждый AC1–AC6 однозначен, имеет названный способ доказательства и (где
|
||||
применимо как защитный) — «чем краснеет».
|
||||
- К1/К5 реализуемы буквально существующим кодом `entryFallbackPlugin`;
|
||||
проверил вручную, что оба переписываемых теста `#486` действительно меняют
|
||||
истинностное значение своих утверждений на новом коде, а не остаются
|
||||
случайно зелёными.
|
||||
- К4/риск №1 корректно определяет реальную поломку существующего утверждения
|
||||
о манифесте (арифметика `reachable()` проверена вручную) и корректно
|
||||
оставляет способ переформулировки на усмотрение реализации.
|
||||
- Классификация полного трека, оспоренная самим автором, подтверждена
|
||||
независимым разбором механики: изменение действительно вводит новое условие
|
||||
срабатывания видимого фолбэка, а не только смену текста.
|
||||
- Непомеченных догадок о недокументированном поведении не найдено.
|
||||
- Названные затронутые файлы (`scripts/bundle-manifest.mjs`,
|
||||
`test/bundle-assets.test.mjs`, `scripts/mutation-gate.mjs`,
|
||||
`docs/ARCHITECTURE.md`, оба changelog, три копии бандла) соответствуют
|
||||
реальному расположению кода, который придётся менять.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Гейты (`typecheck`/`test`/`build`/golden/backend) — не требуются на этапе
|
||||
ревью ТЗ (§2.4), кода ещё нет.
|
||||
- Реальный прогон сборки/тестов с внесённой правкой (её не существует; это
|
||||
этап кода-ревью).
|
||||
- Формулировку самой плашки о несовпадении версий и побочную находку про
|
||||
`disconnect()`/`_hideBanner()` — явно вне скоупа этой задачи (вынесены
|
||||
автором в отдельный разговор и в #536 соответственно), не проверял по
|
||||
существу.
|
||||
- Заголовки прокси/reverse-proxy перед Home Assistant — вне скоупа по тексту
|
||||
ТЗ, не проверял.
|
||||
- `scripts/mutation-gate.mjs` — не проверял, существует ли уже похожий по духу
|
||||
мутант или конфликт имени `panel-imports-unversioned-card-entry`; это
|
||||
деталь реализации, а не предмет ревью ТЗ.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Вердикт: зелёный · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 0 → в задаче
|
||||
|
||||
Issue #535 переходит в `S5-ready`.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `1ca3b5fae3e2` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `3721f5a271916bfb2cc467947132525042363285`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 3721f5a27191
|
||||
```
|
||||
- Тело issue: `77204930eaea05482f2a7da2bac3af79247e7d7356e0449551cb997da2c76fc8`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user