mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-02 21:01:21 +00:00
claude[bot]
@@ -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.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `a4667562470d` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `f4e7de452fa2f8e4738f9bd9d22c9d6fb642a8a2`
|
||||
```
|
||||
git log --all --format='%H %T' | grep f4e7de452fa2
|
||||
```
|
||||
- Тело issue: `8f8081ec29db1c104189682530643bde421d91127170b2c635107c595950a5a9`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user