From ac1d25a08cb6da1e4154e857df761504a7f79d1d Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sat, 19 Sep 2026 14:05:03 +0000 Subject: [PATCH] docs: review document for #598 Issue: #598 User-Visible: no --- docs/reviews/SPEC-REVIEW-598-r1.md | 196 +++++++++++++++++++++++++++++ 1 file changed, 196 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-598-r1.md diff --git a/docs/reviews/SPEC-REVIEW-598-r1.md b/docs/reviews/SPEC-REVIEW-598-r1.md new file mode 100644 index 00000000..9b3897fe --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-598-r1.md @@ -0,0 +1,196 @@ +# SPEC-REVIEW-598-r1 + +**Issue:** https://github.com/Matysh/houseplan-card/issues/598 +**Заголовок:** Три оставшихся диалога настроек на общий набор контролов: Пространство, Общие, +Устройство (шаг 3 эпика #591) +**Этап:** ревью ТЗ (PROCESS.md §2.4) +**Заход:** r1 · блокирующих циклов израсходовано (до этого раунда) 0 из 4 +**Материал:** тело issue #598, раздел `## ТЗ`, на момент чтения 2026-09-19. Комментариев к +issue на момент ревью — 0 (`gh api repos/Matysh/houseplan-card/issues/598/comments` → `length: 0`), +поэтому раздела «Унаследовано из r0» не требуется — это первый заход. + +## Скоуп ревью + +Ревью ТЗ, не код: код по этому issue не писался (веток `issue/598-*` в репозитории нет, +рабочая копия на `dev`, `git status` — чисто). Проверялась (а) полнота обязательных разделов +ТЗ по PROCESS.md §7.1, (б) однозначность и проверяемость AC1…AC10, (в) отсутствие догадок, +выданных за факт — каждое числовое или структурное утверждение ТЗ сверено с реальным деревом +`src/`, `demo/`, `docs/`, а не принято на слово. + +## Как проверялось + +Технической реализации нет, поэтому «прогон гейтов» неприменим — сверка шла чтением дерева на +`dev` (SHA `958ab366`, рабочая копия чистая) против конкретных утверждений ТЗ: + +1. Счётчики файлов и разметки — `wc -l` трёх диалогов и `form-kit.ts` (177/318/760/140 строк) — + совпадают с таблицей «Что предстоит переработать» построчно. +2. Классы-опоры К5 — `grep` по `demo/*.mjs`: `.srcrow` встречается в 9 файлах + (`smoke_discovery_filters`, `smoke_ha_controls`, `smoke_hide_layers`, `smoke_room_settings`, + `smoke_room_tooltip_toggle`, `smoke_space_create_display_defaults`, `smoke_sun`, + `smoke_tap_run`, `smoke_ux_fixes`), `.dispsection` — в 2 (`smoke_backup_transfer`, + `smoke_general_settings`). Совпадает с текстом ТЗ буквально. +3. Существующие примитивы набора (`form-kit.ts`): `formCard`, `formRow`, `segmented`, + `colorRow`, лёгкий CSS-лист через `ensureFormKitStyles` — реально существуют и уже лежат в + ленивом редакторском графе (комментарий в файле прямо называет измеренный бюджет из #594), + что подтверждает К7 и AC9 не как обещание, а как состояние кода. +4. `hp-color-opacity` используется во всех трёх целевых файлах плюс в уже переведённом + `room-settings-dialog.ts` — К3 не изобретает нового компонента. +5. `hpf-switch` в коде не существует (`grep -n "hpf-switch" src/editors/*.ts` — 0 совпадений, + `withSwitch: false` зашито в текущей сборке листа) — ТЗ сама называет это «принято + предположительно, поменять свободно» и даёт откат («дешевле оставить `.srcrow` как есть»). + Корректно помечено, не выдано за решённый факт. +6. Мутанты `dialog-control-writes-to-a-neighbour-key` и `dialog-segment-drops-radio-semantics` + в `scripts/mutation-registry.mjs` пока не существуют, но по нему уже есть точный прецедент + того же автора того же приёма — `form-kit-writes-to-a-neighbour-key` (патч в + `room-settings-dialog.ts`, страж `smoke_room_settings.mjs`) и + `form-kit-segment-drops-radio-semantics` (патч в `form-kit.ts`, страж `test/form-kit.test.mjs`) + из #594. Именование и форма ссылки на ещё не написанный мутант соответствует уже принятой в + реестре практике — это не догадка, а перенос доказанного шаблона. +7. Все 11 поимённых golden-сцен AC7 существуют в `demo/golden/baselines/baselines-index.json` + (`scenarios` содержит все 11 id, всего сцен в индексе — 173) — сцены не изобретены, это + реальные существующие кадры, часть которых пересматривается намеренно. +8. `space.display_section`/`space.roomcard_section` реально существуют и переиспользуются как + заявлено; арифметика счётчика новых заголовков (5 «Общих» + 2 «Пространства» + 5 + «Устройства» = 12 × 4 языка = 48 строк) подтверждается построчным разбором UX-таблиц — + см. находку M1 про второй ряд той же таблицы. +9. Терминология «карточки-группы» уже используется `docs/USER-GUIDE.ru.md:572` для диалога + комнаты (#594) — ТЗ #598 не изобретает новый термин интерфейса, а продолжает принятый. +10. 44 px в К6/AC5 — не число из воздуха, это документированный минимум `docs/TOUCH-SUPPORT.md` + (строки 14, 72, 97). +11. `smoke_backup_transfer.mjs` собирает `.dispsection` через `.some(node => textContent === …)`, + а не по индексу/соседству — перекладка в карточки такую проверку не ломает, заявление AC2 + «без единой правки» для этого смока правдоподобно. +12. `smoke_general_settings.mjs` сегодня собирает `out.groups` как список ВСЕХ `.dispsection` — + девять штук. Из УX-таблицы следует, что `gs.glow_group` и `gs.sun_group` в новой раскладке + становятся заголовками карточек (`

` в `formCard()`, без класса `.dispsection`), а не + остаются подзаголовками — то есть реальный состав `.dispsection` после переделки — семь, не + девять. Это ровно то расширение, которое ТЗ называет единственным допустимым в AC3: вывод + ревью — AC3 корректно предвидит настоящую причину изменения смока, а не создаёт её. + +## Находки + +### M1 (Medium, в скоупе) — судьба `marker.run_target_gone` не решена, счётчик i18n не сходится + +**Где:** раздел «i18n», строка `.help` + `.help.aria` для семи существующих пояснений +(14 ключей × 4 языка = 56 строк); раздел «УX» → «Устройство на плане»; К4. + +**Воспроизведение:** + +- Вводная таблица ТЗ («Что предстоит переработать») называет ровно одно + пояснение-абзац диалога устройства — `marker.run_target_gone` (строка + `src/editors/marker-dialog.ts:334`, `
`). +- УX-раздел расписывает, какую карточку и чей текст получает «?», для каждого + переносимого пояснения **кроме этого**: «Общие» — 4 явные пары «карточка → текст» + (`gs.hint`→«Цвета заливки комнат», `gs.radar_show_live_hint`→«Отображение», + `gs.backup_hint`/`gs.grid_hint`→«Данные»); «Пространство» — явно + `space.hide_decor_tip`/`space.hide_openings_tip`→«Внешний вид». Итого явно + названо **6** пояснений. Для «Устройства» ни один из пяти рядов таблицы карточек + не называет пояснение и не связывает `marker.run_target_gone` ни с одной из + пяти карточек. +- Таблица i18n утверждает «семь существующих пояснений». Явно поимённо в тексте + ТЗ названо шесть. Седьмым может быть только `marker.run_target_gone` — но тогда + ТЗ обязано сказать, в какую из пяти карточек устройства едет его «?», как это + сделано для всех шести остальных. +- Альтернатива — `marker.run_target_gone` остаётся инлайновым callout'ом, как + `gs.sun_missing` (который К4/УX прямо называют исключением: «gs.sun_missing + остаётся callout'ом»). Но для `run_target_gone` такого исключения текст не + формулирует, хотя по факту (строка кода) это ровно тот же паттерн: условный + инлайновый предупреждающий текст с переменной (`{id}`), а не статичное + объяснение настройки. +- Разница ощутима для AC4: «абзацев-пояснений в трёх формах не осталось» — + формула безусловная. Если `run_target_gone` остаётся как есть, AC4 в этой + редакции не выполняется буквально (абзац остался); если он переезжает под «?», + разработчику неизвестно, в какую карточку, и ревьюеру кода нечем судить + результат, кроме собственной догадки. +- Сама арифметика тоже расходится: если верно, что пояснений было 6 (гипотеза + «run_target_gone — callout, как sun_missing»), то строка i18n должна быть + «6 × 2 × 4 = 48», а не «7 × 2 × 4 = 56»; итоговая сумма ключей/строк ТЗ (26 + ключей / 104 строки) в этом случае тоже должна быть на 1 ключ и 4 строки меньше. + +**Почему это Medium, а не Low:** это не опечатка в тексте, а нерешённый вопрос, +прямо влияющий на проверяемость AC4 в самом рискованном из трёх файлов (устройство +— 760 строк, единственный без единой существующей группы, по риску №2 самого ТЗ). +Без решения код-ревьюер не может отличить правильную реализацию от нарушения AC4 +чтением диффа — придётся гадать так же, как автор кода. + +**Почему в скоупе:** диалог устройства прямо входит в скоуп этого issue +(`src/editors/marker-dialog.ts` — первая строка таблицы скоупа), решение +технического характера (куда девать один инлайн-текст), не продуктовое — автору +ТЗ решать самому, без вопроса владельцу. + +**Чем закрывается:** один из двух вариантов, явно вписанный в текст ТЗ — +(а) добавить `marker.run_target_gone` в перечень пояснений с картой «карточка → +текст» вроде остальных шести (тогда i18n-таблица подтверждает «7», разночтения не +возникает), либо (б) явно объявить его исключением, аналогичным `gs.sun_missing`, +и убрать из счётчика (тогда i18n-таблица и итоговая сумма ключей пересчитываются +на «6»/48/96 вместо 7/56/104). Любой из двух вариантов делает AC4 проверяемым без +догадки. + +## Что проверено и признано корректным + +- Обязательные разделы §7.1 присутствуют все: сценарий, что человек увидит до/после, + проблема, скоуп/не-скоуп, контракт поведения (К1–К7), UX, модель данных и + миграция, i18n, AC1–AC10 с доказательством и способом покраснения, план + автотестов, риски, откат, release-артефакты. Ничего не пропущено. +- Первые два раздела продуктовые и отвечают на «какая персона/поверхность/момент» и + «что видно одной фразой без терминов реализации» — соответствует §7.1 буквально, + и персона (администратор дома) верно привязана к SCOPE.md («Editors are + admin-only tools»). +- Скоуп/не-скоуп — образцовый: не-скоуп перечисляет ровно то, что архивный `SPEC.md` + эпика #591 предлагал сверху (активность «Сохранить», инлайн-цвет, плитки, компас, + футер, ширина) и что уже было осознанно оставлено вне #594 — граница + воспроизведена, а не придумана заново. +- К1–К7 проверяемы и опираются на существующий код, а не на воображаемое поведение + (см. «Как проверялось» пп. 3–6, 10). +- Классы риска §2.6 разобраны все шесть, неприменимые обоснованно помечены + («геометрия — не применимо», «async — не применимо: перекладка синхронной + разметки»), а не пропущены молча. +- AC7 (golden) называет ровно те 11 сцен, которые реально существуют в индексе, и + корректно суммируется с построчным разбором «7 устройство + 3 общие + 1 + пространство» из раздела «Риски» №2. +- AC2/AC3 логически согласованы: AC3 верно предсказывает, что `smoke_general_settings` + — единственный смок, требующий правки, и по правильной причине (переход + `gs.glow_group`/`gs.sun_group` из подзаголовков в заголовки карточек меняет + реальный состав `.dispsection`, а не просто «на всякий случай»). +- «Принято предположительно» блок в конце корректно отделяет технические решения + (имена ключей, включение `hpf-switch`, порядок карточек, `label.dispsection` для + подзаголовков) от продуктовых — ни одно продуктовое решение туда не спрятано. +- Открытых вопросов владельцу в тексте нет, и по итогам проверки такого вопроса не + появилось: единственная находка (M1) — техническая, автор ТЗ решает её сам. + +## Чего не проверял + +- Не запускал `npm test` / `npm run typecheck` / `npm run build` — на этапе ревью ТЗ + кода ещё нет (ветки `issue/598-*` не существует), гейты неприменимы; они + относятся к код-ревью этого issue. +- Не проверял, действительно ли CI сможет прогнать смоки/golden с текущими путями — + это код-ревью проверит по факту диффа; на этапе ТЗ проверялась только + правдоподобность заявленных путей и существующих файлов, а не их прогон. +- Не оценивал соответствие присланного архивного `SPEC.md` (файлов + из архива эпика #591) построчно — ТЗ прямо и осознанно берёт из него только + раскладку и явно называет, что остальное (поведение) отклонено; сверять + отклонённые части не было предмета ревью. +- Не проверял французскую (`fr`) локаль на предмет реальных значений строк — только + то, что счётчик «4 языка» соответствует существующим `en/ru/de/fr` словарям. + +## Вердикт + +Единственная находка — Medium, в скоупе задачи, без High. Согласно PROCESS.md §2.4/§7.2, +это жёлтый вердикт: автор правит ТЗ (решает судьбу `marker.run_target_gone` явно и +приводит счётчик i18n в соответствие), фикс проходит повторный цикл ревью ТЗ. + +`Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 1 → в задаче` + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `958ab36652eb` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `232dd22abb22fc04ea01c124ac4332a6060a4fc8` + ``` + git log --all --format='%H %T' | grep 232dd22abb22 + ``` +- Тело issue: `aee54691f0fa3651f53c32dcb2934b5d0ebb983714e8c297783696b476976331` +- Вердикт конвейера: `yellow` · High 0