mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-04 05:41:34 +00:00
@@ -0,0 +1,280 @@
|
||||
# SPEC-REVIEW-600-r1 — #600: диалоги настроек разошлись с макетами #591
|
||||
|
||||
- Issue: [#600](https://github.com/Matysh/houseplan-card/issues/600)
|
||||
- Этап: `spec` (PROCESS.md §2.4)
|
||||
- ТЗ под ревью: тело issue #600, раздел `## ТЗ` (коммит `94c9d8ad`, ветка
|
||||
`issue/600-settings-dialogs`, `HEAD` детач на этом SHA)
|
||||
- Трек: полный (аналитика #600 явно отказалась от лёгкого — сложность 8/10 > 3,
|
||||
четыре поверхности, новый UX-контракт Save/dirty, влияние на perf-потолок)
|
||||
- Заход: **r1**, блокирующих циклов израсходовано **0/4**
|
||||
- Вердикт: **зелёный**
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Материал этапа — тело issue #600 (`## ТЗ`) и все 9 комментариев: разбор архива,
|
||||
две записи аналитики (S2), пакет из одиннадцати развилок с умолчаниями автора,
|
||||
решения владельца по Q1–Q11, подтверждение выполнения AC0. Плюс коммит
|
||||
`94c9d8ad` (`docs: положить дизайн-референс диалогов настроек в репозиторий`),
|
||||
который уже лежит в дереве на момент ревью и добавляет
|
||||
`docs/design/600-settings-dialogs/` (`SPEC.md`, `IMPLEMENTATION-GUIDE.md`,
|
||||
`OPEN-POINTS.md`, `field-maps/FIELD-MAP-space.md`, `reference/`,
|
||||
`screenshots/`, `README.md`, `ACCEPTANCE.md`) — по решению владельца эти
|
||||
документы **являются частью ТЗ**, а не приложением к нему, поэтому проверены
|
||||
как нормативный материал, а не как справка.
|
||||
|
||||
Референсный контекст, прочитанный до вердикта: `docs/SCOPE.md`, `AGENTS.md`,
|
||||
`PROCESS.md` §1–§9 (в частности §2.2–§2.4, §2.6, §7.1, §7.2), тело и все
|
||||
комментарии #600, `docs/USER-GUIDE.ru.md` (для проверки терминологии
|
||||
переименований Q6 — сверка не потребовалась предметно, переименования не
|
||||
затрагивают уже описанные в гайде формулировки настроек, кроме текстов,
|
||||
которые сама задача обязана обновить как release-артефакт).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Ревью ТЗ, не ревью кода: рабочая копия уже на `94c9d8ad`, продуктовый код
|
||||
(`src/**`) в диапазоне не менялся — только `docs/design/**`. Прогон
|
||||
`npm test`/`npm run build` не требовался: класс изменения C (документация),
|
||||
это подтверждено самим коммитом и `git show --stat 94c9d8ad`.
|
||||
|
||||
| Проверка | Результат |
|
||||
|---|---|
|
||||
| Обязательные разделы ТЗ по PROCESS.md §7.1 | все присутствуют, см. таблицу ниже |
|
||||
| Каждый AC0–AC13 — однозначность + способ доказательства | см. «AC» ниже |
|
||||
| Технические утверждения ТЗ против реального кода | см. «Верификация фактов» |
|
||||
| Прослеживаемость Q1–Q11 → решения владельца → правки ТЗ | сверено построчно |
|
||||
| `docs/design/600-settings-dialogs/` реально существует и соответствует заявленному в AC0 | `ls`, сверка секций `SPEC.md` |
|
||||
| Признаки «догадка выдана за решение» без пометки предположения | целевой поиск |
|
||||
| Продуктовые вопросы владельцу вместо технических | сверено — все 11 вопросов продуктовые или лицензионные |
|
||||
|
||||
### Обязательные разделы ТЗ (PROCESS.md §7.1)
|
||||
|
||||
| Раздел | Есть | Где |
|
||||
|---|---|---|
|
||||
| Сценарий | да | «Сценарий» |
|
||||
| Что человек увидит до/после | да | «Что человек увидит до и после» |
|
||||
| Проблема | да | «Проблема» |
|
||||
| Скоуп и не-скоуп | да, с явным перечнем файлов | «Скоуп / не-скоуп» |
|
||||
| Контракт поведения | да, десять пунктов К1–К10 | «Контракт поведения» |
|
||||
| UX | да — распределён по «Набору контролов», «Поведению на разных экранах», «Составу диалогов», а не под одним заголовком | см. ниже |
|
||||
| Модель данных и миграция | да | «Нет» |
|
||||
| i18n | да | «i18n» |
|
||||
| AC1…ACn с доказательством | да, AC0–AC13, у каждого «чем доказывается» и «чем краснеет» | «Критерии приёмки» |
|
||||
| План автотестов | да | «План автотестов» |
|
||||
| Риски | да, семь пунктов | «Риски» |
|
||||
| Откат | да | «Откат» |
|
||||
| Release-артефакты | да | «Release-артефакты» |
|
||||
|
||||
UX-раздел не собран под одним заголовком «UX», но материально присутствует:
|
||||
«Набор контролов» (таблица примитивов с числами из §3.1 источника),
|
||||
«Поведение на разных экранах (Q7)» (три полосы, семь контрольных ширин) и
|
||||
«Состав диалогов» (ссылка на §4–§7 `SPEC.md`) вместе покрывают то же
|
||||
содержание, что требует §7.1. Как и в прецеденте SPEC-REVIEW-89-r1 (находка
|
||||
L1, риски без отдельного заголовка), рассеянность по разделам не делает текст
|
||||
неисполнимым или непроверяемым — единственный критерий блокировки на этом
|
||||
этапе. Отмечаю как Low-наблюдение, не как находку, требующую действия.
|
||||
|
||||
### Верификация фактов
|
||||
|
||||
Основная часть работы — задача сама объясняет, что предыдущие три захода
|
||||
(#594, #598, #599) провалились именно из-за непроверенных утверждений
|
||||
(«К7 выполнен, значит референс не нужен» и т.п.), поэтому каждое конкретное
|
||||
техническое утверждение ТЗ сверено с текущим деревом на `94c9d8ad`, а не
|
||||
принято на слово:
|
||||
|
||||
- **Дефолтная ширина диалогов 500 px, а не 560.** Подтверждено:
|
||||
`src/hp-dialog.ts:169` — `width: min(var(--hp-dialog-wide-width, 500px), 94vw)`.
|
||||
- **`data-kind` уже проставлен на всех четырёх диалогах.** Подтверждено:
|
||||
`grep -l data-kind src/editors/*.ts` возвращает все четыре файла
|
||||
(`space-settings-dialog.ts`, `general-settings-dialog.ts`,
|
||||
`room-settings-dialog.ts`, `marker-dialog.ts`).
|
||||
- **Три сегмента уже стоят** (`gs-bg-mode`, `space-bg-mode`,
|
||||
`space-zero-wall-style`). Подтверждено по исходникам этих трёх диалогов.
|
||||
- **Двенадцать смоков держатся за `.srcrow`/`.dispsection`.** Подтверждено
|
||||
ровно: `grep -rln "srcrow|dispsection" demo/smoke_*.mjs` даёт 12 файлов —
|
||||
число из аналитики S2 не устарело на момент ревью.
|
||||
- **Потолок ленивого графа: `LAZY_EDITOR_GZIP_CEILING = 222 900`,
|
||||
`LAZY_GRAPH_CEILING_BAND = 2 000`.** Подтверждено дословно в
|
||||
`scripts/bundle-budget.mjs:393-394`.
|
||||
- **`editor.loading` = «Загружаем редактор…» в `ru.json`.** Подтверждено —
|
||||
прямой дефект #9 описан точно, ключ существует и требует правки текста, не
|
||||
переименования ключа; `editor.loading_aria` рядом и намеренно не трогается.
|
||||
- **Замороженные фикстуры панели существуют** (`test/fixtures/summary-panel-editor.css`,
|
||||
К9/AC11) — файл на месте.
|
||||
- **Мутанты, названные существующими** (`state-callout-hidden-under-help`,
|
||||
`dialog-card-loses-its-heading`) действительно есть в
|
||||
`scripts/mutation-registry.mjs`; два новых (`dialog-control-writes-to-a-neighbour-key`,
|
||||
`dialog-segment-drops-radio-semantics`) корректно не найдены — план
|
||||
автотестов честно относит их к работе задачи, а не выдаёт за готовые.
|
||||
Мутант «на инверсию слоёв» и «на Save активен без изменений» из плана
|
||||
автотестов также ещё не существуют — ожидаемо, план их и не называет
|
||||
существующими.
|
||||
- **Смоки, названные по имени в AC6/AC7/AC5** (`smoke_esc_dialogs`,
|
||||
`smoke_help_affordance`, `smoke_color_picker_consumers`) — все три файла
|
||||
существуют в `demo/`.
|
||||
- **`docs/design/600-settings-dialogs/` реально содержит заявленное в AC0**:
|
||||
`README.md`, `ACCEPTANCE.md`, `SPEC.md` (273 строки, секции §1–§12 +
|
||||
«Принятые технические предположения» — совпадает с описанием), `field-maps/FIELD-MAP-space.md`,
|
||||
`reference/`, `screenshots/` — на месте. AC0 на дату ревью уже выполнен, что
|
||||
и заявлено последним комментарием.
|
||||
- **§4.2/§5.2/§6.2/§7.2 архивного `SPEC.md`** (полные карты «контрол → ключ
|
||||
состояния», на которые ссылается К1) действительно существуют как
|
||||
подразделы «Состав карточек» / «Состав и правила» внутри §4–§7 — ссылка ТЗ
|
||||
не бьёт в пустоту.
|
||||
- **Q1–Q11 → решения владельца → правки ТЗ** сверены построчно: Q2 (образцы
|
||||
карточки остаются) и Q5 (только раскладка, без нативного пикера) — оба
|
||||
места, где решение владельца разошлось с умолчанием автора и с `SPEC.md`
|
||||
оригинала, — оба явно и без потери смысла отражены в разделе «Решения
|
||||
владельца 20.09» и продублированы в контракте (К... нет отдельного пункта,
|
||||
но упомянуты в «Набор контролов» и AC5/AC1). Ни одно решение владельца не
|
||||
потеряно и не переврано при переносе в нормативный текст.
|
||||
- **Вопросы владельцу все продуктовые или лицензионные**, ни один не является
|
||||
техническим, замаскированным под продуктовый (Q11 — лицензия на перенос
|
||||
материалов, тоже не техническая деталь реализации, а вопрос прав на
|
||||
контент). Технические развилки (модуль `form-kit` vs новый
|
||||
`settings-form-view.ts`, имена классов/ключей) автор решил сам и вынес в
|
||||
«Принято предположительно», как и предписывает §7.1.
|
||||
|
||||
Ни одного утверждения о текущем состоянии кода, не подтверждённого чтением
|
||||
дерева на `94c9d8ad`, не найдено.
|
||||
|
||||
## Находки
|
||||
|
||||
Ни одной **High**. Ни одной **Medium**. Одна **Low** — разобрана и снята здесь
|
||||
же с записью.
|
||||
|
||||
### L1 — регресс-гарантия для существующих кнопок действий не названа отдельным AC
|
||||
|
||||
Архивный `SPEC.md` (§11, AC8: «Copy, Delete, Hide, Skip, Keep as walls,
|
||||
Export/Import, Optimize, Read ZHA / Update map, Attach, Pin, Open in HA
|
||||
работают как до задачи») содержал отдельный критерий приёмки на то, что
|
||||
существующие кнопки-действия внутри четырёх диалогов не сломаются при
|
||||
перестройке карточек и футера вокруг них. При переносе в тело issue этот AC
|
||||
не появился ни как отдельный пункт, ни как явно объявленное отступление (в
|
||||
отличие от Q2/Q5, которые в тексте прямо помечены «против умолчания автора»).
|
||||
Формально это расхождение с материалом, который сама задача называет частью
|
||||
ТЗ, без объявленной причины — ровно то, чего требует избегать формулировка
|
||||
«расхождение оформляется ссылкой на пункт документа, а не „на глаз“».
|
||||
|
||||
Проверка по коду показала, что фактический риск невысок: клик-обработчики
|
||||
этих кнопок (`_readZha`, `openSpaceCopyDialog`/`_deleteSpace`,
|
||||
`_saveRoom`/`canSaveNew`, обработчик Pin в `marker-dialog.ts:546-549`)
|
||||
находятся в стороне от заменяемых примитивов (радиосписок → сегмент, чекбокс
|
||||
→ строка-тумблер и т.д.) — редизайн трогает только обёртку разметки, а не
|
||||
привязку событий. Часть функций уже защищена независимыми тестами вне
|
||||
двенадцати смоков-опор, которые будут сознательно переписаны
|
||||
(`test/space-copy-runtime.test.mjs`, `test/space-dialog.test.mjs`,
|
||||
`demo/smoke_danger_confirmation.mjs`, `test/zigbee-topology.test.mjs`,
|
||||
`demo/smoke_zigbee_topology_hover.mjs`, `demo/smoke_space_copy.mjs` и другие
|
||||
`smoke_optimize_*`/`smoke_*_reconciliation`). Явного smoke-свидетеля для
|
||||
«Pin» (`marker.icon_pin_auto`) и «Attach»/«Keep as walls» в create-режиме не
|
||||
нашлось, но это уже существующий пробел покрытия, не новый, и он не входит в
|
||||
скоуп задачи (задача не трогает эту логику, только оборачивающую разметку).
|
||||
|
||||
**Вердикт по находке:** снимается с записью. Отсутствие своего номера AC не
|
||||
делает ТЗ неисполнимым — блокирующий критерий этапа ТЗ; AC13 («Полный набор
|
||||
зелёный: `npm test`, `npm run gate:small`») и явное требование плана
|
||||
автотестов «в хендоффе перечисляется, какой смок и почему» правкуется под
|
||||
каждый из двенадцати переписываемых смоков — при выполнении этого требования
|
||||
регресс в перечисленных кнопках будет пойман тем же механизмом, каким ловится
|
||||
любой другой регресс со смены разметки. Рекомендация автору (не обязательна
|
||||
для этой редакции, годится на код-ревью или в хендофф): явно перечислить эти
|
||||
кнопки как часть AC4 или как отдельную строку плана автотестов — «действия
|
||||
Copy/Delete/Skip/Keep as walls/Export/Import/Optimize/Read ZHA/Update
|
||||
map/Attach/Pin/Open in HA не меняют поведение, только окружение» — так
|
||||
код-ревью получит именованный критерий вместо необходимости реконструировать
|
||||
его из архивного документа.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Все обязательные разделы §7.1 присутствуют по содержанию (см. таблицу
|
||||
выше); UX рассредоточен, но не потерян.
|
||||
- AC0–AC13 пронумерованы без пропусков, у каждого явно указано и «чем
|
||||
доказывается», и «чем краснеет» — сильнее минимального требования §2.5
|
||||
(«указано, чем доказывается»).
|
||||
- AC0 на дату ревью уже фактически выполнен и проверен: `docs/design/600-settings-dialogs/`
|
||||
на месте, содержит заявленные документы, референс не участвует в бандле
|
||||
(утверждение коммита `94c9d8ad`, не перепроверялось повторным запуском
|
||||
`bundle-tree` — см. «Чего не проверял»).
|
||||
- Одиннадцать открытых точек (`OPEN-POINTS.md`) корректно доведены до
|
||||
владельца одним пакетным комментарием, каждая с предложенным умолчанием и
|
||||
рекомендацией автора — по форме и духу PROCESS.md §7.1 («вопросы задаются
|
||||
одним комментарием, пачкой, каждый — что неясно / что изменится / вариант
|
||||
по умолчанию»). Отклонения автора от умолчаний оригинального `SPEC.md`
|
||||
(Q1 fallback, Q4, Q7 — «решено молча и неправильно» на прошлом заходе)
|
||||
честно возвращены владельцу как открытые вопросы, а не оставлены
|
||||
свершившимся фактом — именно то, что было нарушено в #598/#599.
|
||||
- Два места, где решение владельца разошлось с рекомендацией/умолчанием
|
||||
автора (Q2 — образцы карточки остаются; Q5 — только раскладка без
|
||||
системного пикера), в тексте ТЗ прямо помечены как расхождение с `SPEC.md`
|
||||
автора — выполнено требование «расхождение оформляется ссылкой на пункт
|
||||
документа».
|
||||
- Контракт поведения (К1–К10) покрывает данные (К1), инверсию только в UI
|
||||
(К2), наследование (К3), скрытые прототипом ветки (К4), видимость
|
||||
сообщений о состоянии (К5), единственный скролл (К6), доступность (К7),
|
||||
граф загрузки (К8), заморозку панели (К9) и Save/закрытие (К10) — то есть
|
||||
ровно те риски, которые аналитика #591/#598/#600 определила как причину
|
||||
прошлого провала (ложный паритет, потеря `rhint`-текстов, скрытие
|
||||
callout'ов под «?», заморозка смоков вместо честной проверки).
|
||||
- Классы риска §2.6 явно пройдены в самом ТЗ (async — н/п, данные и права —
|
||||
К1/AC3, геометрия — н/п, визуал — главный риск с AC10/AC1, объём/perf —
|
||||
AC12, host/input — К7/AC2/AC9) — облегчает будущий DoR и код-ревью.
|
||||
- Технические решения (структура `form-kit` вместо нового модуля, имена
|
||||
классов/ключей, механика dirty-состояния) корректно отнесены в «Принято
|
||||
предположительно, поменять свободно» и не заданы владельцу как вопросы —
|
||||
ни одного технического вопроса в пакете Q1–Q11 не найдено.
|
||||
- Провенанс архива-источника (`SHA-256 06f07fc2…`) сверен дважды (аналитик
|
||||
→ автор архива в #591, затем владелец лично передал архив 20.09) и
|
||||
зафиксирован в `docs/design/600-settings-dialogs/README.md`; вопрос
|
||||
лицензии (Q11) закрыт владельцем явно («сотрудник, внешний грант не
|
||||
нужен»), а не предположен.
|
||||
- Порядок работ (Space → General → Room → Device, один issue, парные отчёты
|
||||
по каждой серии) прослеживается до решения владельца по Q9 без искажения.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Гейты `npm test`/`npm run build`/`npm run bundle:budget` не запускались —
|
||||
на диапазоне этого ревью нет изменений класса A/B/D (только
|
||||
`docs/design/**`), запуск проверял бы пустое множество; численные значения
|
||||
потолка (`222 199`/`222 900`/`2 000`) сверены чтением исходника, а не
|
||||
пересчётом бандла.
|
||||
- Не перезапускал `bundle-tree`/`npm run bundle:sync` для повторной проверки
|
||||
утверждения «в бандл из референса не попадает ничего» (заявлено в
|
||||
предыдущем комментарии «AC0 выполнен») — доверился заявленному результату
|
||||
зелёного `repo-hygiene` и полного набора «2793 pass, 0 fail», это будет
|
||||
ре-проверено гейтами код-ревью на реализации.
|
||||
- Не читал `reference/` (9 модулей, 388 строк CSS) построчно как код — по
|
||||
решению владельца от 18.09 он не исполняется и используется только как
|
||||
текст-источник чисел; численные значения из §3.1 `SPEC.md`, которые ТЗ
|
||||
переиспользует, сверены с таблицей `SPEC.md`, а не с самим CSS референса.
|
||||
- Не оценивал реализуемость конкретных CSS-чисел (радиус 11, gap 16 и т.п.)
|
||||
на реальных темах HA — это предмет код-ревью и golden-эталонов, а не текста
|
||||
спецификации.
|
||||
- Не проверял, действительно ли `smoke_settings_dialog_cards.mjs` и другие
|
||||
файлы, упомянутые в §10 `SPEC.md` как подлежащие правке, содержат ровно те
|
||||
сценарии, которые там описаны — это будет предметом код-ревью, когда
|
||||
правки появятся, а не ревью текста ТЗ.
|
||||
|
||||
## Вердикт
|
||||
|
||||
**Зелёный · заход r1/4 · High: 0 · Medium: 0 → нет новых issue.**
|
||||
|
||||
ТЗ проверяемо, каждый AC0–AC13 имеет однозначный способ доказательства,
|
||||
десять продуктовых развилок закрыты явными решениями владельца без потери
|
||||
смысла при переносе в нормативный текст, а технические утверждения о
|
||||
текущем состоянии кода выдержали построчную сверку с деревом на `94c9d8ad`.
|
||||
Единственная находка (L1) снята с записью и рекомендацией на будущее.
|
||||
Следующий статус — «Готово к разработке» (`S5-ready`).
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/600-settings-dialogs`, коммит `94c9d8addf3f` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `84be0429b2b9a13a50086986b601bdd88bac4317`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 84be0429b2b9
|
||||
```
|
||||
- Тело issue: `2871bca5cd192e74cd862e2a4db109f96d559aecf77c8c2032369475e76aa1c6`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user