From 9ba034a32f73bca4f1d5626e725af547c0c02140 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sun, 20 Sep 2026 19:38:10 +0000 Subject: [PATCH] docs: review document for #602 Issue: #602 User-Visible: no --- docs/reviews/SPEC-REVIEW-602-r2.md | 202 +++++++++++++++++++++++++++++ 1 file changed, 202 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-602-r2.md diff --git a/docs/reviews/SPEC-REVIEW-602-r2.md b/docs/reviews/SPEC-REVIEW-602-r2.md new file mode 100644 index 00000000..11e3b026 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-602-r2.md @@ -0,0 +1,202 @@ +# SPEC-REVIEW-602-r2 + +**Issue:** https://github.com/Matysh/houseplan-card/issues/602 +**Заголовок:** Полировка редизайна диалогов пространства и устройства +**Этап:** ревью ТЗ (PROCESS.md §2.4) +**Заход:** r2 · блокирующих циклов израсходовано 1 из 4 +**Материал:** тело issue #602, раздел `## ТЗ`, снято `gh issue view 602 --json body` на момент +чтения 2026-09-20 (188 строк). Комментариев к issue — 5: аналитика автора, вопросы владельцу +Q1/Q2, решения владельца (со снятием `blocked`), вердикт конвейера r1 (жёлтый), ответ автора +«Ответ на ревью ТЗ r1». + +## Скоуп ревью r2 + +Предыдущий вердикт — `docs/reviews/SPEC-REVIEW-602-r1.md`, жёлтый, одна находка Medium (в скоупе) +и одна Low (снята с запиской, не требовала правки для зелёного). Материал того раунда объявлен в +его блоке «Материал раунда»: ветка `dev`@`3701cbd` (коммит `3701c7bd201d`), тело issue — +блоб `345487371ab35ce7a540ebb03b9eeb09cda56792a7322d0783036edb5866f78e`. + +Кода по issue #602 не было и нет: `git branch -a | grep -i 602` пуст, рабочая копия детачед на +`a4667562` — это коммит публикации самого `SPEC-REVIEW-602-r1.md`, `git status` чист. Дельта +r1→r2 — исключительно правка текста тела issue, описанная автором в комментарии «Ответ на ревью +ТЗ r1»: (1) одно/два предложения в §6.6 про семантику «уже настроенного» radar-toggle, (2) замена +неверной пары `CHANGELOG.md`/`docs/CHANGELOG.md` на `docs/CHANGELOG.md`/`docs/CHANGELOG.ru.md` в +§10 и §16. + +GitHub не отдаёт постраничный дифф тела issue, поэтому дельта восстановлена методом r1/598-r2: по +заявленному автором перечню правок, сверенному построчно с текущим текстом (раздел «Как +проверялось»), а не на слово. + +Дельта локальна: рёбейз, смена контракта поведения, новая подсистема — неприменимы к чисто +текстовому ТЗ без кода; объём правки (одно предложение + два исправленных названия файлов) +несопоставимо меньше исходного ТЗ. Разбору заново подлежат ровно §6.6 (сама находка) и §10/§16 +(Low); §5 «Не входит» и AC11–AC13 перепроверены постольку, поскольку новая формулировка §6.6 могла +их задеть. Остальные разделы (§7.1 целиком, §1–§4, §6.1–6.5, §7–9, AC1–AC10 и AC14–AC16, план +тестирования, риски, rollback, release artifacts кроме исправленной пары файлов) дельта не +касается — наследуются из r1 (раздел ниже). + +## Как проверялось + +1. Прочитано текущее тело issue #602 целиком (`gh issue view 602 --json body`) и все 5 + комментариев (`gh issue view 602 --json comments`). +2. Открыт архивный `docs/reviews/SPEC-REVIEW-602-r1.md` — сверено дословно, что находки Medium и + Low сформулированы так, как их описывает раздел «Закрытие раунда r1» ниже. +3. §6.6 текущего тела сверено построчно с правкой, заявленной в ответе автора: + - новое предложение прямо называет обе ветки происхождения («был радар ранее объявлен вручную + или автоматически распознан») и явно фиксирует решение — toggle включён при + `recognition.reason === 'saved'` независимо от происхождения; + - явно сказано «Нового поля происхождения и изменения алгоритма распознавания моделей нет» — + это прямой ответ на вторую половину находки r1 (риск конфликта с §5 «не входит: + …распознавания поддерживаемых моделей радара»). +4. Перечитан код, на который опиралась находка r1, чтобы проверить, что новая формулировка не + создаёт новой неоднозначности и не входит в противоречие с §5: + - `recognizeRadar()` (`src/radar-editor.ts:105–124`) — `reason: 'saved'` возвращается только + когда `device.marker.radar` уже существует и валиден (`isMarkerRadarV1`), независимо от того, + как конфиг туда попал; `reason: 'ld2450'` — отдельная, более узкая ветка **только** для + устройства без сохранённого radar-конфига, чьи `model`/имена entity подошли под эвристику. + Значит новая формулировка §6.6 («для reason === 'saved'») трогает ровно случай «уже есть + сохранённый конфиг любого происхождения» и не переопределяет и не расширяет ветку + первичного распознавания `ld2450` (ещё не сохранённый радар, кнопка «Настроить» в + `src/editors/radar-section.ts:108-118` остаётся как есть) — конфликта с §5 «не входит: + …распознавания поддерживаемых моделей радара» нет: сам алгоритм распознавания + (`recognizeRadar`) новой формулировкой не тронут, меняется только то, где рендерится уже + распознанный/сохранённый конфиг. + - `radarDraft()` (`src/radar-editor.ts:161-164`, вызывается без `forceManual` при открытии + диалога, `src/houseplan-editor-runtime.ts:7585-7587`) возвращает ненулевой draft, если + `original` (сохранённый `device.marker.radar`) существует — то есть `d.radar` уже заполнен на + открытии диалога для любого устройства с сохранённым radar-конфигом, что подтверждает: без + явного изменения `manualEntry`/условий placement описанный r1 разрыв + («форма в Main, а не toggle+inline в Additional») будет воспроизводиться при каждом открытии + ровно так, как и предполагала находка r1 — реализация должна будет провести toggle-ветку + через placement `'additional'` для этого случая; ТЗ формулирует наблюдаемый результат, а не + реализацию, и это на этапе ТЗ достаточно (тот же вывод, что r1 сделал про центрирование + тумблера). +5. §10 и §16 сверены на точное совпадение с реальными путями: `docs/CHANGELOG.md` и + `docs/CHANGELOG.ru.md` названы в обоих местах, корневой `CHANGELOG.md` больше не упоминается; + RU-changelog в §10 теперь присутствует явно. +6. AC11–AC13 перечитаны на предмет того, не разошлись ли они с переписанным §6.6: AC11 («Radar + toggle раскрывает конфигурацию непосредственно под собой и не создаёт вторую radar-секцию + выше») и AC12 («Off → On до Save восстанавливает radar draft…») текстуально не изменились и + остаются верны и для «внутри сессии», и для «уже сохранённого при повторном открытии» — + переписанное §6.6 не требует правки AC, потому что оба AC описывают наблюдаемое поведение + toggle/draft, а не конкретный триггер его появления. AC13 (virtual/saved_unsupported) не + затронут дельтой и не нуждается в правке. +7. `git branch -a`, `git log --oneline -5`, `git status` — подтверждают отсутствие кода по issue + между r1 и r2 и то, что рабочая копия стоит на коммите публикации r1-документа; гейты + (`typecheck`/`test`/`build`) неприменимы на этапе ревью ТЗ, как и в r1. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| Medium — §6.6 не уточняла, что означает «для уже настроенного радара тумблер отображается включённым»: в рамках текущей открытой сессии диалога или для радара, сохранённого в прошлой сессии; `recognizeRadar()` не различает происхождение (`reason: 'saved'` для любого сохранённого конфига), поэтому без уточнения manually-declared и auto-recognized радары рисковали получить разное поведение произвольно | Автор явно выбрал вариант (b) из предложенных r1 альтернатив и записал его текстом: «Для любого уже **сохранённого** поддерживаемого radar-конфига тумблер при каждом повторном открытии диалога отображается включённым, независимо от того, был радар ранее объявлен вручную или автоматически распознан… `recognition.reason === 'saved'` всегда означает включённый toggle и inline-конфигурацию под ним… Нового поля происхождения и изменения алгоритма распознавания моделей нет» | §6.6 тела issue, третий пункт списка «Radar toggle» | +| Low (снята с запиской, не требовала правки для зелёного) — §10/§16 называли несуществующую пару `CHANGELOG.md` + `docs/CHANGELOG.md`, RU-changelog не был назван | §10 и §16 переписаны на реальную пару `docs/CHANGELOG.md` + `docs/CHANGELOG.ru.md` | §10 «Затрагиваемые модули», последний пункт; §16 «Release artifacts», первое предложение | + +Обе находки закрыты по существу, а не декларативно: Medium — явным продуктовым решением с прямым +указанием на используемый код (`recognition.reason === 'saved'`) и явным отказом от нового поля +данных (чем предотвращён конфликт с §5); Low — точным исправлением названий файлов. + +## Унаследовано из r1 + +Без повторной проверки приняты выводы `docs/reviews/SPEC-REVIEW-602-r1.md` (материал: `dev`@ +`3701c7bd201d`, тело issue — блоб +`345487371ab35ce7a540ebb03b9eeb09cda56792a7322d0783036edb5866f78e`), поскольку дельта r1→r2 их не +затрагивает: + +- полнота обязательных разделов §7.1 (сценарий, до/после, проблема, scope/не-входит, контракт §6, + UX-состояния §7, данные/миграция §8, i18n §9, AC1–AC16 с «Свидетелем» §11, план автотестов §12, + риски §14, rollback §15, release artifacts §16) — дельта не удаляла и не переименовывала разделы; +- соответствие `docs/SCOPE.md` (J4/J6, персона Home admin, поверхность — только editor-диалоги); +- заземление фактических утверждений §3/аналитики в коде: фиксированные ширины `4em`/`5.8em`, + `padding-right: 122px/125px`, текущее расположение `useClimateTemp` в карточке света, разрыв + «кнопка в Additional actions → форма в Main params» для radar (два вызова `_renderRadarSection` с + разным `placement`) — все подтверждены чтением кода в r1, дельта эти факты не переписывает; +- соответствие терминологии `docs/USER-GUIDE.ru.md` (футер/`dialog.unsaved`, radar, climate-бейдж) + и существование процитированных i18n-ключей; +- touch/mobile-контракт не вводится заново — установлен в #600 (320/390/480/560/768/1280/1920, + «touch остаётся best effort»), явная метка `Touch editor: …` не требуется по прецеденту + `SPEC-REVIEW-563-r1.md`; +- область задачи (4 диалога) не раздута — Room/General реально импортируют общие хелперы + `toggleRow`/`rangeEnds`/`footerStatus` из `src/editors/form-kit.ts`, общий CSS-баг действительно + их задевает; +- Q1/Q2 — обе действительно продуктовые, обе закрыты явным решением владельца, `blocked` снят + по факту; метки (`bug`, `P2`, `polish`, `S4-spec-review`, без `small`/`trivial`) согласованы с + выбором полного трека; +- незакрытое «Чего не проверял» из r1 (golden/скриншот-покрытие Room/General как отдельная тема, + implementation-детали центрирования тумблера, EN user-guide как производный документ) — дельта + этих тем не касается, статус не меняется. + +## Находки + +Не найдено. Дельта закрывает обе находки r1 по существу: Medium — явным, проверяемым по коду +продуктовым решением без нового поля данных и без конфликта с §5; Low — точным исправлением +названий файлов. Новая формулировка §6.6 перечитана против `recognizeRadar()`/`radarDraft()` и не +создаёт новой неоднозначности: она задевает ровно случай «уже сохранён» (`reason: 'saved'`) и не +переопределяет отдельную, более узкую ветку первичного распознавания `ld2450` — конфликта с §5 +«не входит: …распознавания поддерживаемых моделей радара» нет. AC11–AC13 остаются согласованными +с переписанным §6.6 без необходимости правки нумерации или текста. + +## Что проверено и признано корректным + +- Продуктовое решение по Medium принято автором в правильной форме: не техническая деталь, а явный + выбор наблюдаемого поведения («toggle включён при `reason === 'saved'`, независимо от + происхождения»), с явным отказом вводить новое поле данных — соответствует PROCESS.md §7.1 и не + расширяет тайно скоуп §5. +- Формулировка §6.6 после правки не оставляет открытого вопроса про повторное открытие диалога: + единственный оставшийся триггер старой карточки `radar.title` — устройство без сохранённого + radar-конфига, распознанное эвристикой `ld2450` (первичное распознавание, вне скоупа находки). +- Пара changelog-файлов в §10/§16 теперь совпадает с реальными путями и с тем, что проверяет + `scripts/validate-commit-provenance.mjs:64`. +- Материал раунда r1 не осиротел: между r1 и r2 в `dev` не появлялось кода по issue #602, + рабочая копия ревью стоит на коммите публикации `SPEC-REVIEW-602-r1.md` — отдельная реставрация + дерева не потребовалась. + +## Чего не проверял + +- Наследую нетронутое дельтой из r1: golden/скриншот-покрытие Room/General как отдельную тему, + implementation-детали центрирования шарика тумблера, полный текст `docs/USER-GUIDE.md` (EN). +- Не проверял, как именно реализация проведёт toggle-рендер через `placement === 'additional'` для + случая «уже сохранён» (правки `manualEntry`/условий в `radar-section.ts`) — это код-ревью после + реализации; на этапе ТЗ проверено, что наблюдаемый результат сформулирован однозначно и + проверяем (AC11–AC13 + новое предложение §6.6), реализация вольна выбирать конкретные условия. +- Автотесты и гейты `typecheck`/`test`/`build`/`golden:verify` не запускались — кода по issue #602 + нет (`git branch -a | grep -i 602` пуст), предмет ревью — исключительно текст ТЗ; они относятся + к код-ревью (§2.7) после реализации. + +## Вердикт + +Обе находки предыдущего раунда закрыты по существу и проверяемо: Medium — продуктовым решением +с прямой ссылкой на используемое условие кода и явным отказом от нового поля данных (не конфликтует +с §5); Low — точным исправлением пары файлов changelog. Дельта не вносит новых High или Medium +находок и не разрушает ни один вывод r1: наследуемые разделы не затронуты, а точки соприкосновения +с дельтой (§6.6, AC11–AC13, §10/§16) перепроверены заново и непротиворечивы. ТЗ готово к разработке. + +**Вердикт: зелёный · заход r2 · блокирующих циклов 1/4 · High: 0 · Medium: 0** + +--- + +## Материал раунда + +- Ветка: `dev`, рабочая копия ревью — детачед HEAD на `a4667562470dee913dfdc14b57adb7644eeafa07` + (коммит публикации `SPEC-REVIEW-602-r1.md`); кода по issue #602 между r1 и r2 не появлялось + (`git branch -a | grep -i 602` — пусто). +- Тело issue снято `gh issue view 602 --repo Matysh/houseplan-card --json body` 2026-09-20, + 188 строк — предмет этого раунда; sha256 нормализованного тела для официального якоря пишет + конвейер публикации, как и в r1 (issue #414); для справки sha256 сырого текста, полученного + `gh issue view --json body -q .body`: `c861f9a9fbecbefdec69d3d5dbb3951edc40266586e039bde3efa3f9ceb2490e`. +- Комментарии issue на момент ревью: 5 (`gh issue view 602 --json comments` → 5 элементов) — + аналитика, вопросы владельцу, решения владельца, вердикт конвейера r1, ответ автора на r1. + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `a4667562470d` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `f4e7de452fa2f8e4738f9bd9d22c9d6fb642a8a2` + ``` + git log --all --format='%H %T' | grep f4e7de452fa2 + ``` +- Тело issue: `8f8081ec29db1c104189682530643bde421d91127170b2c635107c595950a5a9` +- Вердикт конвейера: `green` · High 0