docs: review document for #363

Issue: #363
User-Visible: no

docs/reviews/CODE-REVIEW-363-r2.md

@@ -0,0 +1,212 @@
+# CODE-REVIEW-363-r2
+
+## Метаданные раунда — почему это «r2», но полный разбор
+
+Заголовок задачи фиксирует «заход r2 · блокирующих циклов израсходовано 1 из
+2» для этапа **code**. Проверка меток issue (`gh api .../issues/363/timeline`)
+показывает, что `S7-code-review` выставлена **впервые** в 09:49:41 — до этого
+не было ни одного `S7-code-review → S6/S7` цикла. Комментариев с вердиктом
+код-ревью в issue тоже нет (всего 5 комментариев, все относятся к аналитике
+S2 и двум попыткам этапа spec).
+
+Единственный потраченный блокирующий цикл (1/2) — это жёлтый вердикт
+**спек-ревью** r1 (2026-08-29T09:16:10Z, `SPEC-REVIEW-363-r1.md@9d6d6eb9`),
+исправленный в той же попытке до зелёного (09:22:49Z). Бюджет §4 общий на
+issue, а не на этап, поэтому нумерация «заход» продолжает расти при переходе
+в code, хотя это первый разбор кода.
+
+**Следствие:** предыдущего code-review раунда, с которым можно было бы
+сравнивать дельту (§2.9), не существует. Материал этого раунда — весь диапазон
+`git diff origin/dev...HEAD` (4 коммита реализации), разобран полностью.
+Разделы «Закрытие раунда r1» и «Унаследовано из r1» в этом документе
+отсутствуют по той же причине: наследовать не от чего.
+
+## Скоуп проверки
+
+Диапазон: `origin/dev..HEAD`, 4 коммита:
+
+- `e8916c71` feat: restore direct device add shortcut — `User-Visible: yes`
+- `3068a97c` test: align device catalog evidence with shortcut
+- `5ba0d9e1` test: accept reviewed device shortcut frames — golden baselines
+- `e5ecf399` chore: preserve changelog mode after rebase (только смена прав
+  доступа файла, содержимое не менялось)
+
+Трейлеры `Issue: #363` присутствуют во всех коммитах, `User-Visible` расставлен
+корректно (`yes` только там, где меняется видимое поведение и в этом же
+коммите правится оба changelog).
+
+## Как проверялось
+
+Прочитаны в указанном порядке: `docs/SCOPE.md`, `AGENTS.md`/`PROCESS.md`,
+тело issue #363 (ТЗ revision 2) и все 5 комментариев, `docs/USER-GUIDE(.ru).md`
+(раздел Device editor), исходный код (`src/houseplan-editor-runtime.ts`,
+`src/houseplan-card.ts`), тесты (`demo/smoke_editor_tabs.mjs`,
+`demo/smoke_device_inbox.mjs`, `test/i18n.test.mjs`), golden-матрица
+(`demo/golden/matrix.mjs`, `demo/golden/baselines/baselines-index.json`),
+`docs/images/screenshots.json`, история коммита `cab8d128^` (состояние до
+удаления кнопки), CSS тулбара (`src/styles/chrome.styles.ts`), а также журнал
+запусков workflow через `gh run view`/`gh api` (Validate на `e5ecf399`,
+докрышоты на ref `issue/363-add-device-shortcut`).
+
+## AC-за-AC
+
+**AC1 — видимость и порядок.** Подтверждено чтением:
+`src/houseplan-editor-runtime.ts:11541-11555` — `_renderDevicesBar()` рендерит
+кнопки в порядке «Добавить» (иконка `mdi:plus-box-outline`) → «Устройства» →
+«Правила иконок»; `_renderDevicesBar` вызывается только при
+`editorChromeMode === 'devices'` (`houseplan-card.ts:11066-11072`), в других
+режимах не рендерится вовсе. Доказано динамически: `demo/smoke_editor_tabs.mjs`
+проверяет `deviceToolbarButtons.length === 3`, что нулевая кнопка — Add
+(текст, title, иконка), первая — Devices. Тест умеет падать: смена порядка,
+иконки или подписи ломает соответствующее булево поле, которое `checkAll`
+требует истинным по умолчанию.
+
+**AC2 — прежний диалог.** `_openMarkerDialog()` без аргумента
+(`houseplan-editor-runtime.ts:7628,7721-7745`) создаёt черновик с
+`name: '', bindingMode: 'virtual'`, без ключа `devId` (ветка `else`). Смок
+кликает по кнопке и проверяет ровно эту форму
+(`!Object.prototype.hasOwnProperty.call(c._markerDialog, 'devId')`) —
+мутант «вызов с аргументом» ловится: при `d` передан объект `devId` появится.
+Клавиатурная семантика (Enter/Space) не эмулировалась синтетическим
+`KeyboardEvent`, а подтверждена структурно
+(`addDeviceButton.tagName === 'BUTTON' && !addDeviceButton.disabled`) —
+нативная кнопка получает Enter/Space активацию от браузера бесплатно; это тот
+же способ, каким уже проверена соседняя кнопка «Устройства». Отдельный тест
+на сам факт срабатывания нативной семантики тестировал бы браузер, а не код
+продукта — не считаю это пробелом.
+
+**AC3 — каталог сохранён.** `demo/smoke_device_inbox.mjs` не тронут в части
+самого каталога: `_openDeviceInbox()`, 4 вкладки, ряды `on_plan`, поиск по
+комнате — без изменений. Единственная правка — переименование поля
+`oneCatalogEntryPoint` → `catalogAndDirectAddEntryPoints` и добавление
+позитивной проверки, что кнопка `^Add$` **есть** (ранее было наоборот, из
+контракта #29). Путь «Добавить виртуальное устройство»
+(`device_inbox.add_virtual`) не задет диффом вообще — регресс здесь
+структурно исключён (нет изменённых строк).
+
+**AC4 — i18n.** `src/i18n/{en,ru,de}.json` — по 2 строки в каждом. Сверено
+побайтово с `git show cab8d128^:src/i18n/{en,ru}.json` — EN/RU совпадают
+дословно. Немецкий («Hinzufügen», «Gerät zum Plan hinzufügen») согласован с
+уже существующими `device_inbox.add`/`title.add_space` в том же словаре.
+`test/i18n.test.mjs:109-112` сверяет полный набор ключей всех словарей с `en`
+— пропуск любой локали или ключа ломает тест (assert.deepEqual на
+отсортированных ключах), проверено чтением, дополнительных прогонов не
+требовалось — тест общий, не требовал правки под #363.
+
+**AC5 — responsive и доступность.** `.editbar` использует CSS-grid
+(`minmax(0,1fr) auto`, `chrome.styles.ts:193-214`) — колонка `.editbar-end`
+(Close) зафиксирована отдельной grid-колонкой и не участвует в переносе;
+`.editbar-tools` — `flex-wrap: wrap`, кнопки переносятся, а не обрезаются и не
+перекрываются. **Этот CSS не тронут диффом** — риск переполнения тулбара,
+заявленный в ТЗ, снят тем же механизмом, что уже работает для соседних кнопок,
+без новой логики. Прямого browser-смока «ширина X, нет overlap» именно для
+голого тулбара (без диалога) не добавлено, но принятый golden-кадр
+`device-inbox-narrow-ru-dark` (390px) визуально показывает тот же тулбар
+(приглушённым) под диалогом и был вручную проверен автором при приёмке —
+косвенное, но реальное подтверждение при самой узкой из используемых в матрице
+ширин. Считаю риск закрытым; отдельный узкий смок был бы полезен, но не
+обязателен при неизменном layout-механизме — не поднимаю как находку.
+
+**AC6 — данные и логика неизменны.** Диф не касается persisted-схемы,
+save/cancel flow, backend. Новый draft создаётся той же веткой
+`_openMarkerDialog()`, что и раньше для «пустого» диалога — нет второго
+create-flow. Cancel не создаёт marker (ветка `else` только формирует
+локальный `_markerDialog`, запись в `_serverCfg`/`callWS` происходит по
+существующему save-пути, не тронутому диффом).
+
+**AC7 — документация и выпуск.** Оба changelog правятся в `e8916c71`
+(`User-Visible: yes`), USER-GUIDE обновлён в EN и RU в том же коммите. Гейт
+`check-docs.mjs`: `docs/images/screenshots.json` — `sourceFingerprint`
+обновлён для всех 10 сценариев, `imageSha256` изменился **только** у
+`device-editor` (7 байт разницы в PNG, автор задокументировал это как
+antialiasing-флуктуацию инфраструктуры при кросс-сверке 7 прогонов workflow —
+правдоподобно и не относится к продукту). Канонический workflow
+«Скриншоты документации» запущен на `ref: issue/363-add-device-shortcut`
+(проверено логом job `actions/checkout@v7` рана `33245780081` — вопреки
+`headBranch: main` в метаданных API, что для этого репозитория штатно:
+workflow-файл живёт на `main` побайтово синхронизированным с `dev` (#365), а
+checkout идёт по input `ref`). `docs:accept --reviewed` выполнен, обновлённый
+`docs/images/screenshots.json` закоммичен.
+
+**AC8 — визуальный эталон.** `baselines-index.json`: изменились ровно два
+хэша — `geometry-devices-editor-dark` и `device-inbox-narrow-ru-dark`;
+`device-inbox-desktop-en-light` и `device-inbox-desktop-ru-dark` не менялись.
+Здесь есть реальное расхождение с буквальным текстом AC8: ТЗ утверждает, что
+все три `device-inbox-*` не меняются и называет единственный
+`--expect-change=geometry-devices-editor-dark`. По факту
+`device-inbox-narrow-ru-dark` тоже сменил хэш (102015→103240 байт PNG) —
+и это ожидаемо: при ширине 390px приглушённый тулбар виден под диалогом
+каталога, и новая кнопка «Добавить» видна в этом приглушённом фоне. Автор
+явно задокументировал и обосновал это в комментарии к issue до приёмки, само
+изменение содержимого каталога не задело. **Квалифицирую как Low, не
+блокирует**: причина понятна, зафиксирована публично, поймана предсказанным
+самим же спек-ревью механизмом («если поедут — находка»), пиксельная дельта
+согласуется с размером одной кнопки. Рекомендация — одной строкой поправить
+текст AC8 в issue (перечислить `device-inbox-narrow-ru-dark` в ожидаемо
+меняющихся вместе с `geometry-devices-editor-dark`), но это не повод для
+возврата автору.
+
+## Гейты: что прогнано и что нет, и почему
+
+- `npx tsc --noEmit`, `npm test`, `npm run build` (+ сверка 3 копий бандла) —
+  **не прогонял сам**: Validate на `e5ecf399` зелёный целиком
+  (https://github.com/Matysh/houseplan-card/actions/runs/33246304275, job
+  «Фронтенд: типы, юниты, мутанты, синхрон бандла» = success).
+- `node scripts/check-docs.mjs` — тот же Validate-прогон покрывает job
+  «Предполётные проверки: документация, провенанс, процесс» = success;
+  дополнительно проверил содержимое `screenshots.json` вручную (см. AC7).
+- Полный набор golden (`npm run golden:verify`) и все 3 шарда browser-смоков —
+  на самом `e5ecf399` эти джобы **skipped** («Переиспользование: дерево уже
+  проверено»), но проверил цепочку: они реально выполнялись и были зелёными
+  на run `33246023321` (headSha `a043c216`, дерево идентично `e5ecf399` за
+  вычетом смены прав доступа файла) — «Golden-кадры против принятых
+  эталонов» = success, все 3 шарда смоков = success. Повторный прогон не
+  требовался.
+- `node scripts/smoke-select.mjs --base origin/dev --head HEAD` — прогнал.
+  Инструмент не нашёл «прямого совпадения» или «зарегистрированной связи»:
+  вернул НЕОПРЕДЕЛЁННОСТЬ и список из 28 слабых связей (все — смоки,
+  вызывающие `_openMarkerDialog`, распространённое имя). Решение: не
+  прогонять — этот диф не меняет ни сигнатуру, ни поведение
+  `_openMarkerDialog()`, а только добавляет ещё один вызов без аргумента
+  (уже покрытый паттерн, например `demo/smoke_new_device.mjs`); целевые
+  смоки для самой фичи (`smoke_editor_tabs.mjs`, `smoke_device_inbox.mjs`)
+  уже обновлены автором и зелены в Validate.
+- `python -m pytest tests_backend -q` — не запускал, диф не касается
+  `custom_components/**/*.py`.
+- Инварианты модели (`npm run invariants`) — не запускал, диф не касается
+  геометрии, стен, layout, `marker.space`, `open_spans`.
+- Perf-профили — не требуются ни AC, ни диффом; job перф-смока в обоих
+  релевантных прогонах либо success, либо skipped по классификации файлов.
+- «Одно число — один источник» — в диффе нет новых пользовательских числовых
+  величин (только текстовые подписи/tooltip), проверка неприменима.
+
+## Что проверено и корректно
+
+- Порядок, иконка, подпись и tooltip кнопки — по коду и по смоку.
+- `_openMarkerDialog()` без аргумента — тот же прежний путь, каталог не
+  открывается.
+- i18n-паритет трёх локалей, байт-в-байт совпадение EN/RU с состоянием до
+  `cab8d128`.
+- Каталог и путь «Добавить виртуальное устройство» не затронуты.
+- Changelog/USER-GUIDE обновлены в нужном коммите на обоих языках.
+- Docs-фингерпринт и golden-баselines обновлены целиком согласованным
+  набором, посторонних дрейфов не найдено.
+- Docs-скриншоты сняты на верном ref ветки (проверено логом job, а не
+  метаданными API).
+
+## Находки
+
+**L1 (Low).** Текст AC8 в теле issue #363 предсказывает, что
+`device-inbox-narrow-ru-dark` не изменится; по факту эта golden-сцена
+изменилась (ожидаемо и обоснованно — приглушённая кнопка тулбара видна под
+диалогом на узкой ширине). Причина понятна, изменение задокументировано
+автором в issue-комментарии до приёмки, риск регрессии отсутствует.
+Рекомендация: точечно поправить формулировку AC8, не блокирует.
+
+Находок Medium/High нет.
+
+## Вывод
+
+Диф полностью реализует ТЗ revision 2, AC1–AC8 доказаны кодом/тестами/CI,
+трейлеры корректны, changelog и docs synced. Единственное замечание — Low,
+снимается с запиской, отдельный issue не требуется.