mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
docs: review document for #402
Проверка (CI) / Классификация изменённых файлов (push) Successful in 27s
Проверка (CI) / Предполётные проверки: документация, провенанс, процесс (push) Failing after 47s
Проверка (CI) / HACS: валидация репозитория (push) Failing after 23s
Проверка (CI) / Hassfest: манифест интеграции (push) Failing after 18s
Проверка (CI) / Переиспользование: это дерево уже проверено (push) Successful in 1m10s
Проверка (CI) / Бэкенд: pytest в Home Assistant (push) Failing after 4m16s
Проверка (CI) / Фронтенд: типы, юниты, мутанты, синхрон бандла (push) Failing after 6m14s
Проверка (CI) / Смоки в браузере (шард 1 из 3) (push) Skipped
Проверка (CI) / Смоки в браузере (шард 2 из 3) (push) Skipped
Проверка (CI) / Смоки в браузере (шард 3 из 3) (push) Skipped
Проверка (CI) / Смоки: все шарды зелёные (push) Skipped
Проверка (CI) / Golden-кадры против принятых эталонов (push) Skipped
Проверка (CI) / Перф-смок: бюджет времени кадра (push) Skipped
Проверка (CI) / Классификация изменённых файлов (push) Successful in 27s
Проверка (CI) / Предполётные проверки: документация, провенанс, процесс (push) Failing after 47s
Проверка (CI) / HACS: валидация репозитория (push) Failing after 23s
Проверка (CI) / Hassfest: манифест интеграции (push) Failing after 18s
Проверка (CI) / Переиспользование: это дерево уже проверено (push) Successful in 1m10s
Проверка (CI) / Бэкенд: pytest в Home Assistant (push) Failing after 4m16s
Проверка (CI) / Фронтенд: типы, юниты, мутанты, синхрон бандла (push) Failing after 6m14s
Проверка (CI) / Смоки в браузере (шард 1 из 3) (push) Skipped
Проверка (CI) / Смоки в браузере (шард 2 из 3) (push) Skipped
Проверка (CI) / Смоки в браузере (шард 3 из 3) (push) Skipped
Проверка (CI) / Смоки: все шарды зелёные (push) Skipped
Проверка (CI) / Golden-кадры против принятых эталонов (push) Skipped
Проверка (CI) / Перф-смок: бюджет времени кадра (push) Skipped
Issue: #402 User-Visible: no
This commit is contained in:
@@ -0,0 +1,225 @@
|
||||
# CODE-REVIEW-402-r1
|
||||
|
||||
- Issue: https://github.com/Matysh/houseplan-card/issues/402
|
||||
- ТЗ: `docs/specs/402-confirm-outside-main-branch.md` (ревизия 2, зелёное SPEC-REVIEW-402-r2)
|
||||
- Диапазон: `origin/dev..HEAD`, SHA на момент вывода вердикта: `ccd870ddf77c9a03ec4618df3e9ded5f159d174b`
|
||||
- Заход: код-ревью r1 · блокирующих циклов израсходовано 0/4 (первый заход этапа code)
|
||||
- Класс изменения: A (продукт) + B (гейт-мутант) + C (документация/changelog) + D (бандл/скриншоты)
|
||||
|
||||
## Скоуп
|
||||
|
||||
Диагноз и контракт полностью изложены в ТЗ и обоих раундах ревью ТЗ (оба уже
|
||||
пройдены, r1 — красный, r2 — зелёный на `11959e4c`). Реализация:
|
||||
|
||||
- `src/houseplan-card.ts`: `render()` разбит на `_renderBody()` (старая цепочка
|
||||
ранних `return`, без изменений по существу) + новый `render()`-обёртку,
|
||||
которая достраивает `<hp-confirm>` рядом с телом, если тело не `noChange` и
|
||||
не `nothing`; `_confirmDanger` отказывает немедленно (`Promise.resolve(false)`),
|
||||
если `!this._config || !this.hass`.
|
||||
- `scripts/mutation-gate.mjs`: новый мутант `danger-confirm-back-into-the-branch`,
|
||||
возвращающий `hp-confirm` внутрь ветки, guard — новый смок.
|
||||
- `demo/smoke_danger_confirm_branches.mjs` (новый, 140 строк, 9 проверок,
|
||||
touch-эмуляция `hasTouch: true`).
|
||||
- `test/optional-space-model-contract.test.mjs`: перенесена цель чтения на
|
||||
`_renderBody`, добавлены статические проверки обёртки `render()`.
|
||||
- `docs/CHANGELOG.md` / `docs/CHANGELOG.ru.md`: пункт про #402, в том же
|
||||
коммите, что и код (`00b6f412`).
|
||||
- `ccd870dd`: только классы D/C — пересборка бандла и обязательная пересъёмка
|
||||
отпечатка документации (см. «Как проверялось»).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Зелёного Validate на этом SHA не найдено — все гейты ниже прогнаны лично.
|
||||
|
||||
| Гейт | Команда | Результат |
|
||||
|---|---|---|
|
||||
| Типы | `npx tsc --noEmit` | чисто, 0 ошибок |
|
||||
| Юниты | `npm test` | `# tests 1690`, `# pass 1689`, `# fail 0`, `# skipped 1` (совпадает с заявленным в хендоффе) |
|
||||
| Сборка + сверка копий | `npm run build` затем `cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js` | собрано за 15.6с, `cmp` без вывода — байт-в-байт совпадение; `git status` после сборки чист (комитнутый `dist` уже в актуальном состоянии) |
|
||||
| Документация | `node scripts/check-docs.mjs` (обязателен — diff трогает `src/**`) | «Documentation checks passed (7 files, 10 external links)» |
|
||||
| Бюджет (AC7) | `npm run bundle:budget` | `initial View: 287386 B gzip (budget 300000 B, headroom 12614 B)` — совпадает с числом из хендоффа день-в-день; предупреждение про запас бюджета — известный факт #367, не связано с #402 |
|
||||
| Выбор смоков | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | 9 прямых совпадений (`_confirmDanger`/`_dangerConfirm`), 18 слабых (общее имя `_config`) |
|
||||
| Прямые смоки (9/9) | `node demo/smoke_danger_confirm_branches.mjs` и ещё 8, см. ниже | все зелёные |
|
||||
| Мутационная дисциплина | ручной откат правки + пересборка + прогон нового смока | смок подвисает (промис никогда не резолвится) — мутант красит смок, как заявлено в реестре |
|
||||
| process-gate | `node scripts/process-gate.mjs --range origin/dev..HEAD` | «гейт пройден, предупреждений 0» |
|
||||
| Инвентарь | `npm run inventory` | 1690 unit / 211 browser smokes — числа не копировались вручную |
|
||||
|
||||
Прямые смоки и их результат:
|
||||
|
||||
```
|
||||
demo/smoke_danger_confirm_branches.mjs 9/9 → OK (новый)
|
||||
demo/smoke_danger_confirmation.mjs OK (AC6: без правок утверждений)
|
||||
demo/smoke_orphan_space_references.mjs OK
|
||||
demo/smoke_binding_picker.mjs OK
|
||||
demo/smoke_free_walls.mjs OK
|
||||
demo/smoke_hidden_flag.mjs OK
|
||||
demo/smoke_lock_invariant.mjs OK — инвариант блокировок (SCOPE.md) не задет
|
||||
demo/smoke_optional_space_model.mjs OK
|
||||
demo/smoke_registryless_opening.mjs OK
|
||||
```
|
||||
|
||||
**`smoke_danger_confirmation.mjs` печатает необработанный `TypeError`
|
||||
(`_bindingHasHaPage`, чтение `.split` из `undefined` внутри рендера диалога
|
||||
маркера) до итогового `OK`.** Проверил, не регрессия ли это: поднял отдельный
|
||||
git worktree на чистом `origin/dev` (символическая ссылка на тот же
|
||||
`node_modules`), собрал бандл, прогнал тот же смок — **то же самое исключение,
|
||||
слово в слово**, воспроизводится и без диффа #402. Совпадает с тем, что описал
|
||||
автор в хендоффе (неполные фикстуры диалога маркера, которые не относятся к
|
||||
этой задаче). Не регрессия — не блокирует.
|
||||
|
||||
Слабые совпадения (18, общее имя `_config`) не прогонял: diff #402 не трогает
|
||||
ни один из путей, которые эти смоки проверяют (общие настройки, цветовые
|
||||
пикеры, геометрия стен, раскладка вкладок) — общее имя случайное, не
|
||||
зарегистрированная связь.
|
||||
|
||||
**Не прогонял и почему:**
|
||||
- `npm run golden:verify` — diff не меняет видимый статический результат:
|
||||
разметка `<hp-confirm>` не изменилась ни на строку, изменилось только место
|
||||
инстанцирования в дереве рендера; ни один голден-сценарий не открывает этот
|
||||
диалог по умолчанию (ТЗ прямо фиксирует «в статике диалог не открыт»).
|
||||
- `python -m pytest tests_backend -q` — ни один файл `custom_components/**/*.py`
|
||||
не тронут.
|
||||
- `node scripts/model-invariants.mjs` — diff не касается геометрии, `layout`,
|
||||
`marker.space`, `open_spans`.
|
||||
- Performance-профили — не названы в AC, путь не чувствителен к перфу
|
||||
(структурная правка порядка рендера, не алгоритм).
|
||||
|
||||
## Находки
|
||||
|
||||
Ни одной High, ни одной Medium. Ниже — Low-наблюдения, все закрыты чтением
|
||||
кода (не блокируют, сняты с записью, как разрешает §8/§2.7).
|
||||
|
||||
### L1 — AC2 не покрыт смоком буквально (ветки `fixed_floor` pending/invalid, `!space`)
|
||||
|
||||
ТЗ обещает доказательство «тот же смок, три ветки», но
|
||||
`smoke_danger_confirm_branches.mjs` реально входит только в ветку
|
||||
`!model.length` (онбординг) через `enterBranch([])`; веток `fixed_floor`
|
||||
pending/invalid и `!space` смок не касается.
|
||||
|
||||
**Закрыто чтением.** `render()` (`src/houseplan-card.ts:11193-11202`) — общая
|
||||
обёртка над результатом `_renderBody()`: она не знает, какая именно ранняя
|
||||
ветка вернула шаблон, и достраивает `_renderDangerConfirm()` к любому телу,
|
||||
кроме `noChange`/`nothing`. Ветки `fixed.kind === 'pending'/'invalid'`
|
||||
(`:11230-11254`) и `!space` (`:11286`) не трогают `_dangerConfirm` ни прямо, ни
|
||||
косвенно — единственное место, где это поле меняется, это колбэк контроллера
|
||||
(`:2151`, вызывается только из `HpConfirmController`). Значит поведение этих
|
||||
трёх веток идентично уже доказанной ветке онбординга по построению, а не по
|
||||
совпадению. Риск регрессии низкий: правка одна и общая для всех веток.
|
||||
|
||||
### L2 — AC3 не проверен буквальным сценарием issue (клик по корзине → `_deleteServerPlan`)
|
||||
|
||||
Смок вызывает `card._confirmDanger(...)` напрямую, а не через клик по
|
||||
`<button class="btn ghost danger">` (`houseplan-onboarding-runtime.ts:273`) и
|
||||
не проверяет, что план реально удаляется через `hass.callWS`.
|
||||
|
||||
**Закрыто чтением.** `_deleteServerPlan` (`houseplan-onboarding-runtime.ts:216-243`)
|
||||
— тонкая обёртка: `await this.host._confirmDanger({...})`, дальше
|
||||
ревалидация (не в скоупе, введена #32) и `callWS`. Единственный кусок,
|
||||
который меняет #402, — это то, что `_confirmDanger` теперь действительно
|
||||
разрешает промис в ветке онбординга; это доказано AC1. Обвязка вокруг него не
|
||||
менялась.
|
||||
|
||||
### L3 — AC8: тап по scrim не проверен явно
|
||||
|
||||
Смок проверяет тап по кнопке «Отмена» в touch-эмуляции, но не тап по
|
||||
затемнению (`scrim`) — а в тексте AC8 «тап по scrim не проваливается в план»
|
||||
назван отдельно.
|
||||
|
||||
**Закрыто чтением.** Разметка и обработчик `hp-confirm`, включая scrim, не
|
||||
менялись ни на строку (диф не касается файла компонента диалога, только
|
||||
места его инстанцирования в `houseplan-card.ts`); `scrimIsSafe` — уже
|
||||
существующая, непереписанная проверка в `smoke_danger_confirmation.mjs` (тот
|
||||
же прогон выше, зелёная) — покрывает то же поведение того же компонента на
|
||||
мыши. Отдельного тач-пути в разметке `hp-confirm` нет.
|
||||
|
||||
### L4 — `_confirmDanger` не проверяет языковой гейт `warm` явно
|
||||
|
||||
Ранний отказ в `_confirmDanger` (`:2153-2161`) смотрит только на
|
||||
`!this._config || !this.hass`; состояние `languageRenderGate(...) === 'warm'`
|
||||
не проверяется вовсе, хотя ТЗ называет его вторым «неготовым» состоянием
|
||||
наравне с первым.
|
||||
|
||||
**Закрыто чтением, но с оговоркой.** Проследил до конца: `_dangerConfirm` —
|
||||
реактивное поле (`:2594`, `state: true`), а `languageRenderGate`
|
||||
(`src/i18n/language-runtime.ts:99-130`) при входе в `pending` ставит
|
||||
`host.inert = true` и планирует `runtime.ensure(code).then(() =>
|
||||
host.requestUpdate())`. Значит (а) на `warm` карточка `inert`, и ни один из 7
|
||||
call site'ов `_confirmDanger` (все — синхронные обработчики кликов) физически
|
||||
не может сработать по клику; (б) даже если запрос всё же пришёл программно,
|
||||
он не виснет навсегда — как только `ensure()` разрешится (успехом или
|
||||
фоллбэком на английский, оба варианта — settled-состояния), `requestUpdate()`
|
||||
запустит обычный рендер, `_renderBody()` перестанет возвращать `noChange`, и
|
||||
`render()` дорисует `_renderDangerConfirm()` с уже накопленным состоянием.
|
||||
Это не «наглухо висящий промис» — класс дефекта #402 — а отложенный до конца
|
||||
локального переключения языка рендер, что безопасно. Вне скоупа фикс не
|
||||
требуется. Стоит держать в голове на будущее: если когда-нибудь появится
|
||||
вызов `_confirmDanger` не из клика (например, из WS-события синхронизации
|
||||
между клиентами, J6), этот путь перестанет быть защищён кликовым `inert`
|
||||
неявно — но такого вызова сегодня нет ни в одном из 7 мест.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **AC1** (онбординг спрашивает, «Отмена» → `false`) — смок
|
||||
`onboardingBranchShowsConfirm` + `onboardingBranchResolves`. Ветка
|
||||
«согласие → `true`» доказана составным путём (`openConfirmSurvivesBranchChange`
|
||||
решает `true`) плюс чтением: решение резолвится тем же обработчиком
|
||||
`_onDangerConfirmDecision` независимо от того, в какой ветке был открыт
|
||||
диалог.
|
||||
- **AC2** — см. L1, закрыто чтением.
|
||||
- **AC3** — см. L2, закрыто чтением.
|
||||
- **AC4** (открытый диалог переживает смену ветки) — смок
|
||||
`openConfirmSurvivesBranchChange`: диалог остаётся, промис резолвится
|
||||
`true` после перехода в ветку онбординга.
|
||||
- **AC5** (`!_config || !hass` отказывает немедленно) — смок
|
||||
`notReadyCardRefusesInsteadOfHanging`. Часть про `warm` — см. L4.
|
||||
- **AC6** (существующее поведение основной ветки не изменилось) —
|
||||
`demo/smoke_danger_confirmation.mjs` не тронут диффом (`git diff` пуст) и
|
||||
зелёный при личном прогоне.
|
||||
- **AC7** (бюджет не растёт) — `npm run bundle:budget` лично: 287 386 Б,
|
||||
совпадает с цифрой из хендоффа.
|
||||
- **AC8** (touch: диалог появляется, тап по «Отмена» → `false`) — смок
|
||||
`touchTapOnCancelResolvesFalse` под `hasTouch: true, isMobile: true`. Про
|
||||
scrim — см. L3.
|
||||
- **AC9** (соседи `_tapConfirm`/`_vacCalConfirm` не переехали) — по диффу:
|
||||
единственное удаление в старой финальной ветке — блок `_dangerConfirm`
|
||||
(`:11872-11878` в старой нумерации), сама ветка и соседние блоки не
|
||||
задеты; плюс смок `neighbourConfirmsUntouched`.
|
||||
- **Мутационная дисциплина**: откатил правку руками
|
||||
(`return html\`${body}${this._renderDangerConfirm()}\`;` →
|
||||
`return html\`${body}\`;`), пересобрал, прогнал новый смок — подвис
|
||||
(промис из ветки онбординга не резолвится), т.е. красит именно так, как
|
||||
заявляет мутант `danger-confirm-back-into-the-branch` в
|
||||
`scripts/mutation-gate.mjs`. Автор честно указал, что штатный прогон
|
||||
`mutation-gate.mjs` не укладывается в лимит времени песочницы и сделал
|
||||
то же руками — переприверил независимо, совпадает.
|
||||
- **Трейлеры и changelog**: `00b6f412` несёт `Issue: #402`,
|
||||
`User-Visible: yes` и правки в оба changelog в этом же коммите (сверено
|
||||
`git show`). Остальные коммиты — `User-Visible: no`, тоже с трейлером.
|
||||
`node scripts/process-gate.mjs --range origin/dev..HEAD` — «гейт пройден,
|
||||
предупреждений 0».
|
||||
- **Класс D/C коммит `ccd870dd`**: только бандл-деревья и обязательная
|
||||
пересъёмка отпечатка документации (любая правка `src/**` делает его
|
||||
устаревшим, §8) — без продуктового кода, `Baseline-Reviewed` не требуется
|
||||
(это не `demo/golden/baselines/**`).
|
||||
- **Инвариант блокировок (SCOPE.md)**: `smoke_lock_invariant.mjs` зелёный —
|
||||
правка не открыла новый путь актуации.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- `npm run golden:verify` и полный набор `demo/smoke_*.mjs` (211 файлов) —
|
||||
не запускал; обоснование пропуска — в разделе «Как проверялось». Это
|
||||
предрелизная обязанность, не гейт этого ревью.
|
||||
- Полный HA-харнесс / `pytest tests_backend` — Python не тронут.
|
||||
- `node scripts/model-invariants.mjs` — геометрия не тронута.
|
||||
- Ручного тестирования в браузере (кроме smoke-инфраструктуры) не проводил —
|
||||
фазы ручного тестирования в процессе нет.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Зелёный. High: 0, Medium: 0. Четыре Low-наблюдения (L1–L4) — все закрыты
|
||||
чтением кода тем же ревью, без правок; ни одно не открывает issue и не
|
||||
возвращает автору по §202 (все либо в скоупе и не требуют правки, либо
|
||||
структурно недостижимы). Задача чинит именно заявленный класс дефекта:
|
||||
подтверждение больше не привязано к конкретной ветке `render()`, отказ в
|
||||
неготовом состоянии — явный, а не подвешенный, и это подтверждено
|
||||
исполняемым тестом, который умеет падать (проверено лично откатом правки).
|
||||
Reference in New Issue
Block a user