mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -0,0 +1,148 @@
|
||||
# SPEC-REVIEW-607-r1
|
||||
|
||||
## Скоуп
|
||||
|
||||
Issue [#607](https://github.com/Matysh/houseplan-card/issues/607) — в реальном
|
||||
Home Assistant (ветка `ha-dialog`, не нативный `<dialog>` демо-стенда) крестик
|
||||
диалога настроек + «Продолжить» в подтверждении «Отменить изменения?» оставляет
|
||||
диалог невидимым с живым состоянием формы до перезагрузки страницы. ТЗ живёт в
|
||||
теле issue под заголовком `## ТЗ` (решение владельца 2026-09-10, #517), трек —
|
||||
полный (аналитик назвал нарушенный критерий §5: не проходит «одна поверхность»,
|
||||
несёт host/input-риск). Это первый заход ревью ТЗ (r1), возврата на правки нет,
|
||||
раздел «Унаследовано» не применим.
|
||||
|
||||
Разбирается вся ТЗ-часть тела issue: сценарий, скоуп/не-скоуп, контракт
|
||||
поведения, UX, модель данных, i18n, AC1–AC4, план автотестов, риски, откат,
|
||||
release-артефакты, блок технических предположений — плюс предшествующая ей
|
||||
часть тела (симптом/механизм/предварительный `## AC`), поскольку sha256-якорь
|
||||
ревью считается по нормализованному телу issue целиком
|
||||
(`scripts/review-doc-guard.mjs:275-277`), а не только по секции `## ТЗ`.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Не ручное тестирование продукта (для этапа ТЗ его и не может быть) — сверка
|
||||
каждого фактического утверждения ТЗ с текущим кодом на SHA
|
||||
`61d51dd934d5cfc7b7286be51b462d71844da88e`, чтение канонических документов и
|
||||
трассировка терминологии.
|
||||
|
||||
| Проверка | Результат |
|
||||
|---|---|
|
||||
| `docs/SCOPE.md` — попадание в Core user jobs | J4/J6 (администратор безопасно настраивает план без потери черновика) — подтверждено, конфликтов с «never delete a user's file» и lock-инвариантом нет (задача не трогает locks/actuation) |
|
||||
| Механизм бага: `src/hp-dialog.ts` (`_requestClose`, `rejectClose`, оба `@closed=${this._requestClose}` в ha-dialog-ветке) | Строки 449–462, 564, 579 — совпадают с заявленными в issue. `rejectClose()` действительно только сбрасывает `_closing` и вызывает `requestUpdate()`; биндинг `.open=${true}` неизменен между рендерами, Lit его не диффует — заявление не догадка, а точное чтение кода |
|
||||
| Четыре хоста используют `rejectClose()` на отказе от discard | `src/editors/marker-dialog.ts:171,180`, `room-settings-dialog.ts:109,118`, `general-settings-dialog.ts:43,51`, `space-form.ts:113,122` — паттерн `if (discard) close(); else dialog?.rejectClose?.();` подтверждён во всех четырёх (строки в ТЗ отличаются от фактических максимум на 1 — граница блока, не по сути) |
|
||||
| Ссылки на `CODE-REVIEW-600-r1.md:84` и `-r2.md:132` | Точное совпадение номеров строк и содержания: «живые ha-dialog» / «настоящий ha-dialog» не поднимались ни в одном из раундов #600 |
|
||||
| `demo/helpers/ha-dialog-fixture.mjs` + `capture_summary_panel_505.mjs` | Файл существует, `grep` по репозиторию подтверждает единственного текущего потребителя — `capture_summary_panel_505.mjs` (плюс упоминание в `scripts/check-inputs.mjs:63`), как и написано в ТЗ. `launchHaDialogFixture` — офлайн, без HA-сервера/auth/WebSocket, с pin-проверкой оригинального `home-assistant-frontend==20260729.7`: план автотестов «переиспользовать безопасную loopback-фикстуру #505» реалистичен |
|
||||
| Четыре существующих `smoke_*_settings_form` (AC3) | `demo/smoke_device_settings_form.mjs`, `smoke_room_settings_form.mjs`, `smoke_space_settings_form.mjs`, `smoke_general_settings_form.mjs` — все существуют |
|
||||
| `test/hp-dialog-contract.test.mjs` (упомянут как возможное место для нового теста) | Существует, сейчас покрывает только #508 (`flex-content`); ТЗ формулирует его как «либо новый соседний contract test» — не жёсткая привязка, ложных ожиданий не создаёт |
|
||||
| Терминология кнопок подтверждения: «Отменить изменения?» / «Отменить» (discard) / «Продолжить» (keep-open) | `src/i18n/settings/ru.json:46-49` — `dialog.discard_title`="Отменить изменения?", `discard_confirm`="Отменить", `discard_keep`="Продолжить». Сценарий и AC1–AC3 внутри `## ТЗ` используют «Продолжить» корректно и согласованно с кодом и `docs/USER-GUIDE.ru.md:409-411` |
|
||||
| `docs/ARCHITECTURE.md:189-197` («One modal contract») | Нативная `<dialog>`-ветка уже содержит аналогичный self-heal паттерн («если retained open flag пережил top-layer entry — закрывает и переоткрывает нативный диалог один раз»). Предлагаемый в ТЗ подход (двухфазная перестановка `open` либо пересоздание HA-shell) архитектурно согласован с уже принятым в проекте решением для другой ветки, не изобретение с нуля |
|
||||
| §7.1 обязательные разделы ТЗ | Все присутствуют под `## ТЗ`: сценарий, что человек увидит до/после, проблема, скоуп, не входит, контракт поведения, UX, модель данных и совместимость, i18n, критерии приёмки AC1–AC4 (каждый с «Доказательство: …»), план автотестов, затронутые файлы, производительность и размер, touch и доступность, риски, откат, release-артефакты, плюс отдельный обязательный блок «Принятые технические предположения» |
|
||||
| Трек и критерий §5 | Аналитик назвал нарушенный критерий («не проходит критерий одной поверхности и несёт host/input-риск») — соответствует требованию §2.2/§5: не «обычный трек» без обоснования |
|
||||
| Продуктовые вопросы владельцу | Отсутствуют — и обоснованно: ожидаемый результат, варианты Continue/Discard и обязательность сохранения нативной ветки уже однозначно зафиксированы существующим кодом/i18n/UX, гадать не о чем. Все технические решения (где чинить — в `HpDialog.rejectClose()`; как переоткрывать — two-phase `open` либо пересоздание) вынесены в блок предположений «можно менять свободно», что и требует §7.1 |
|
||||
|
||||
## Находки
|
||||
|
||||
### Low — предварительный блок `## AC` в начале тела issue называет несуществующую кнопку
|
||||
|
||||
Перед секцией `## ТЗ` в теле issue (часть исходного аудита, тоже входит в
|
||||
sha256-якорь) стоит:
|
||||
|
||||
```
|
||||
## AC
|
||||
- AC1. ... крестик HA → «Продолжить» → диалог виден, черновик тот же. ...
|
||||
- AC2. «Не сохранять» закрывает и обнуляет состояние; чистый диалог закрывается без вопроса.
|
||||
- AC3. Нативная ветка не меняет поведения ...
|
||||
```
|
||||
|
||||
Кнопки с текстом «Не сохранять» в продукте нет: подтверждение «Отменить
|
||||
изменения?» имеет ровно две кнопки — `discard_confirm`="Отменить" (закрывает и
|
||||
сбрасывает черновик) и `discard_keep`="Продолжить" (возвращает форму, это и
|
||||
есть баг). Сама секция `## ТЗ` ниже эту ошибку не повторяет: её
|
||||
«### Критерии приёмки» (AC1–AC4) и «### Контракт поведения»/«### UX» везде
|
||||
корректно используют «Отменить»/Discard. Формальный, действующий список AC —
|
||||
именно в `## ТЗ`, поэтому находка не блокирует разработку: реализация и
|
||||
код-ревью будут сверяться с корректной секцией. Риск чисто читательский —
|
||||
два непронумерованных одинаково списка `AC1/AC2/AC3` в одном теле issue, один
|
||||
из которых называет кнопку, которой нет.
|
||||
|
||||
**Снимается решением ревьюера с записью**, не требует возврата на цикл:
|
||||
устаревший предварительный список из аудита семантически поглощён разделом
|
||||
`### Критерии приёмки` внутри `## ТЗ`, который и является контрактом для
|
||||
разработки и код-ревью. Автору стоит (не обязательно, до следующей правки
|
||||
тела issue) убрать или явно пометить как «дублируется, см. ТЗ» верхний блок
|
||||
`## AC`, чтобы не тратить внимание будущего читателя.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Все фактические утверждения ТЗ о механизме бага и о существующем коде
|
||||
(номера строк в `src/hp-dialog.ts` и в четырёх хостах, ссылки на
|
||||
`CODE-REVIEW-600-*`, существование `ha-dialog-fixture.mjs` и его текущего
|
||||
единственного потребителя, существование четырёх `smoke_*_settings_form`)
|
||||
сверены с деревом на SHA `61d51dd9` — совпадают буква в букву, включая
|
||||
номера строк. Автор не выдал догадку за факт: там, где утверждение не
|
||||
проверено исполнением («проверяется настоящим компонентом, а не только
|
||||
стабом» в разделе «Риски»), это явно названо риском, а не тихо принято как
|
||||
решённое.
|
||||
- Сценарий и «что человек увидит до/после» отвечают на оба продуктовых
|
||||
вопроса §7.1 одной фразой без терминов реализации, персона и поверхность из
|
||||
`docs/SCOPE.md` названы (администратор, настройки устройства/комнаты/
|
||||
пространства/общих настроек, реальный HA).
|
||||
- Контракт поведения (7 пунктов) закрывает все обсуждаемые риски: обычное
|
||||
восстановление, восстановление после того как `ha-dialog` уже обработал
|
||||
`closed`, сохранность черновика, защита от повторного `hp-close`/цикла,
|
||||
отсоединённый элемент/подтверждённый отказ, чистое закрытие без вопроса,
|
||||
сохранение семантики нативного fallback.
|
||||
- AC1–AC3 — каждый с доказательством (адресный diagnostic/smoke на закреплённой
|
||||
офлайн-фикстуре HA либо существующие form smokes), AC4 — защитный AC и сразу
|
||||
сформулирован в терминах будущего код-ревью («способная покраснеть при
|
||||
удалении guard»), что заранее закрывает требование §2.7 о «чем краснеет».
|
||||
- Скоуп/не-скоуп разделены чётко и по существу отсекают соседние темы
|
||||
(тексты/расположение кнопок, логика Save/Discard, замена `ha-dialog`
|
||||
собственной оболочкой, обновление версии закреплённого HA frontend) —
|
||||
предотвращает расползание задачи.
|
||||
- Технические решения (где чинить, как переоткрывать поверхность, формат
|
||||
адресного теста) вынесены в явный блок предположений «можно менять
|
||||
свободно» — ровно так, как требует §7.1, продуктовых вопросов владельцу нет
|
||||
и обоснованно нет.
|
||||
- Откат, i18n («нет новых строк»), миграция («не нужна»), touch (не создаёт
|
||||
нового жеста, только распространяет фикс на touch-активацию крестика) —
|
||||
все явно закрыты, а не пропущены молчанием.
|
||||
- Архитектурная согласованность подтверждена: `docs/ARCHITECTURE.md` уже
|
||||
фиксирует аналогичный self-heal паттерн для нативной ветки — предполагаемый
|
||||
в ТЗ подход не является новым классом решения для проекта.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не проверял исполнением ни один тест/smoke — на этапе ревью ТЗ кода ещё
|
||||
нет, оценивался только сам текст спецификации против текущего дерева.
|
||||
- Не проверял актуальность версии `home-assistant-frontend==20260729.7`,
|
||||
зафиксированной в `ha-dialog-assets.mjs`, против последней реальной версии
|
||||
HA — вне скоупа ревью ТЗ и вне скоупа самой задачи («обновление версии
|
||||
закреплённого HA frontend» прямо исключено из «Не входит»).
|
||||
- Не проверял `docs/UX-MODES.md` целиком построчно — целевой grep по
|
||||
`rejectClose`/`hp-close`/`ha-dialog` совпадений не дал; относящийся контракт
|
||||
диалогов канонически описан в `docs/ARCHITECTURE.md` §5 «One modal
|
||||
contract», который проверен.
|
||||
|
||||
## Вывод
|
||||
|
||||
ТЗ полно по §7.1, каждый AC однозначен и снабжён способом доказательства,
|
||||
технические предположения явно выделены и не требуют решения владельца,
|
||||
фактические утверждения о коде проверены построчно и не являются догадкой.
|
||||
Единственная находка — Low, снимается ревьюером с запиской, цикл не тратит.
|
||||
|
||||
**Вердикт: зелёный.**
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/607-ha-dialog-close`, коммит `61d51dd934d5` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `11896314f247ec0cb06bd20fbe0205751b9f7652`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 11896314f247
|
||||
```
|
||||
- Тело issue: `c78f51ffc0d5268ea3a82d980f9061b9f2756d0a850072fec514342889af8c9e`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user