docs: review document for #363

Issue: #363
User-Visible: no

docs/reviews/SPEC-REVIEW-363-r1.md

@@ -0,0 +1,182 @@
+# SPEC-REVIEW-363-r1
+
+Issue: #363 · Этап: spec (PROCESS.md §2.4) · Трек: `small` (лёгкий) · Заход r1 ·
+блокирующих циклов израсходовано 0 из 2 (лимит лёгкого трека — 2, §5)
+
+## Материал ревью
+
+- Тело issue #363 (владелец: требование + контекст + «Что было до #29»),
+  далее секция «ТЗ · revision 1 · small track», написанная автором ТЗ.
+- Единственный комментарий issue — аналитика владельца (S2), метки:
+  `P2`, `feature`, `polish`, `small`, `S4-spec-review`.
+- Файла в `docs/specs/` нет и не должно быть — метка `small` это разрешает
+  (`scripts/process-gate.mjs` строка `NO_SPEC_FILE = ['small', 'trivial']`,
+  проверено чтением).
+- Кода по задаче ещё нет: `git log --all --oneline | grep 363` и
+  `git branch -a | grep 363` ничего не находят, кроме случайных совпадений с
+  другими номерами issue (`3633a3db`, `58363731` и т.п., не относятся к #363).
+  Гейты `typecheck`/`test`/`build`/`check-docs` в этом заходе не прогонялись:
+  прогонять их не над чем — это ревью текста, не кода (§2.4, не §2.7).
+
+## Как проверялось
+
+Не поверил ни одному фактическому утверждению ТЗ и аналитики на слово —
+перепроверил каждое чтением текущего дерева `dev` (`HEAD=c19a0540`):
+
+1. **`_openMarkerDialog()` жив и не требует правок.**
+   `src/houseplan-card.ts:9343` — приватный метод карточки, делегирует в
+   `src/houseplan-editor-runtime.ts:7628` `public _openMarkerDialog(d?: DevItem)`.
+   Вызов без аргумента — тот же путь, что и раньше. Номера строк в аналитике
+   (`:9308`) слегка разошлись с текущими из-за более поздних коммитов, но метод
+   и его контракт те, что описаны — не находка.
+2. **Ключи i18n действительно отсутствуют и восстанавливаются побайтово.**
+   `grep devbar\.|title.add_device src/i18n/{en,ru,de}.json` не находит ничего,
+   кроме `devbar.rules`. Проверил сам коммит-виновник:
+   `git show cab8d128^:src/i18n/en.json` и `...ru.json` дают ровно
+   `"title.add_device": "Add a device to the plan"` / `"devbar.add": "Add"` и
+   `"Добавить устройство на план"` / `"Добавить"` — таблица ТЗ совпадает
+   посимвольно. Немецкий перевод `"Hinzufügen"` уже используется в словаре для
+   того же действия (`device_inbox.add`, `de.json:462`) — предположение
+   согласовано с терминологией, а не придумано.
+3. **Паритет словарей действительно ловит забытую локаль.**
+   `test/i18n.test.mjs:109-112` — `assert.deepEqual(Object.keys(dictionary).sort(), enKeys)`
+   для каждого языка реестра. Тест умеет падать: пропуск любого из шести
+   значений (2 ключа × 3 локали) даёт красный `npm test`. AC4 доказуемо.
+4. **Место вставки существует.** `src/houseplan-editor-runtime.ts`,
+   `_renderDevicesBar()` — первая кнопка `device_inbox.button` («Устройства»),
+   вторая `devbar.rules` («Правила иконок»). До #29 первой кнопкой была
+   «Добавить» (подтверждено чтением `cab8d128^:src/houseplan-card.ts:20752-20753`).
+   Вставка перед «Устройства» восстанавливает этот порядок.
+5. **Каталог и его добавление виртуального устройства не затронуты.**
+   `_renderDeviceInbox()` и `openVirtual()` в том же файле не упоминаются в
+   диапазоне правок; `demo/smoke_device_inbox.mjs` существует и его заголовок
+   прямо ссылается на #29 («one lifecycle catalog replaces the separate Add /
+   hidden-device paths») — правильный существующий регресс-щуп для AC3.
+6. **RU-таблица тулбара действительно потеряла строку.**
+   `docs/USER-GUIDE.ru.md:779-786` — таблица «Редактор устройств» содержит
+   «Устройства» и «Добавить виртуальное устройство», строки про «Добавить» нет.
+   EN-версия (`docs/USER-GUIDE.md:519-529`) — список, а не таблица, тоже без
+   пункта «Devices editor: Add». Оба документа в «Затронутых файлах» названы.
+7. **Golden-сцены разведены правильно.** `demo/golden/matrix.mjs:381` —
+   `geometry-devices-editor-dark` снимает `mode: 'devices'` **без** открытого
+   диалога — тулбар виден целиком, кнопка на нём действительно появится.
+   `device-inbox-*` (там же, строки 383-390) открывают `dialog: 'device-inbox'`,
+   который перекрывает тулбар модальным окном — эти три сцены обоснованно не
+   должны измениться. `--expect-change=geometry-devices-editor-dark` — точно
+   нужная и единственная нужная пометка.
+8. **Проверил ловушку, в которую эта задача могла попасть незамеченной:**
+   риск незаявленного изменения одного и того же числа/кадра в двух местах.
+   Открыл сам `docs/images/06-device-editor.png` (документационный скриншот,
+   отдельный от golden) — тулбар редактора устройств на нём **полностью
+   перекрыт открытым диалогом** (диалог начинается от x=0 и шире, чем
+   `editbar-tools`), виден только правый `barclose` («✕»), который эта задача
+   не трогает. Значит пиксели этого конкретного файла не изменятся — здесь
+   утверждение ТЗ «новый docs screenshot не требуется» дословно верно.
+   Но это не то же самое, что «шаг с docs-скриншотами не нужен вовсе» — см.
+   находку M1 ниже: сам гейт `docs` не привязан к пикселям.
+9. **UX-MODES.md, TOUCH-SUPPORT.md** — прочитаны на предмет конфликта с
+   контрактом «постоянный инструмент в основном тулбаре, Close в своём торце»
+   (UX-MODES.md:38-41) и «редакторы desktop-first, touch — best effort»
+   (TOUCH-SUPPORT.md:10). Конфликтов нет: кнопка — персистентный инструмент,
+   в правый торец не претендует, новых touch-целей не создаёт.
+
+## Находки
+
+### M1 (Medium, в скоупе задачи) — Release-артефакты не называют обязательный шаг для гейта `docs`
+
+**Что не так.** Раздел «Release-артефакты» ТЗ утверждает: «новый docs
+screenshot не требуется: канонический набор не фиксирует Device editor
+toolbar как отдельный читаемый пользовательский фрагмент» — и это верно только
+в узком смысле «новый файл сцены заводить не нужно» (проверено в п.8 выше).
+Но ТЗ нигде не называет действие, которое всё равно обязательно: обновить
+`docs/images/screenshots.json` через `.github/workflows/docs-screenshots.yml` +
+`npm run docs:accept -- --reviewed --from=...`, коммитом в задаче.
+
+**Почему это обязательно, а не перестраховка.** Гейт `check-docs.mjs`
+сравнивает `manifest.sourceFingerprint` с `visualFingerprint(ROOT)`
+(`scripts/source-fingerprint.mjs`), а `visualFingerprint` считается по **всему**
+`src/**` без исключений для i18n-словарей (`fingerprintFiles()` включает
+`sourceFiles(resolve(root, 'src'))` целиком). Задача правит
+`src/houseplan-editor-runtime.ts` и все три `src/i18n/*.json` — оба пути внутри
+корпуса отпечатка. Значит `manifest.sourceFingerprint` разойдётся с деревом для
+**всех 9** сценариев `demo/docs/screenshots.mjs`, не только для
+`device-editor`, и `docs` job в CI Validate станет красным независимо от того,
+что ни один PNG не изменится по пикселям — сам гейт различий по пикселям не
+делает, он сверяет хэш дерева.
+
+Это ровно тот сценарий, который уже дважды стоил продукту дня простоя `dev`
+(#230, #234, исправлено в #237) — и который прецедент в этом же репозитории
+разбирал явно: `docs/reviews/CODE-REVIEW-337-r4.md` фиксирует «H7-фикс»
+коммитом `0bd5570a` — пересъёмку и приёмку ради одного изменившегося
+`imageSha256` после правки того же тулбара. АС7 ссылается на «docs/process
+gates» как на доказательство, но этого недостаточно как формулировки: она не
+называет действие, которое делает этот гейт зелёным, и без него автор с
+высокой вероятностью повторит #230/#234 — сам факт, что ТЗ не упомянуло
+`visualFingerprint`, показывает, что автор про этот механизм не думал.
+
+**Серьёзность и скоуп.** Medium, внутри уже заявленного скоупа задачи (класс C,
+`docs/**`, часть DoD пункта «Release-артефакты» и AC7) — не отдельный issue
+(владелец, 2026-08-19, #202).
+
+**Требуемая правка ТЗ.** В «Release-артефакты» и/или AC7 добавить явный пункт:
+после правки `src/**` — прогнать `Скриншоты документации`
+(`workflow_dispatch` на ветке задачи), принять артефакт
+`npm run docs:accept -- --reviewed --from=<распакованный>` и закоммитить
+получившийся `docs/images/screenshots.json` (плюс любые PNG, которые всё же
+разойдутся побайтово из-за среды рендера — прецедент #231 упоминает разницу в
+7-8 байт при неизменном кадре). Ожидаемый результат — обновлённый
+`sourceFingerprint`/`sourceSha256` во всех 9 сценариях при явном ожидании: все
+9 PNG остаются пиксельно идентичными (в отличие от golden, где ожидается
+измениться ровно один кадр).
+
+## Что проверено и корректно
+
+- Скоуп/не-скоуп разведены точно и не заходят на каталог, backend, View/kiosk,
+  Plan/Background editors — соответствует `docs/SCOPE.md` (персона 1,
+  администратор дома, оба редактора) и «View mode is the product» инварианту:
+  режим View не тронут вовсе.
+- Все AC1–AC8 проверяемы и каждой указан способ доказательства; ни один не
+  опирается на несуществующий инструмент — `test/i18n.test.mjs` (AC4),
+  `demo/smoke_device_inbox.mjs` (AC3), `demo/golden/matrix.mjs` записи (AC8)
+  реально существуют и умеют падать на нарушении соответствующего утверждения.
+- Ни одной догадки, выданной за решение: все продуктовые детали (подпись,
+  порядок, иконка) обоснованы либо прямой цитатой владельца («как до #29»),
+  либо байт-в-байт восстановленным историческим состоянием — раздел «Принятые
+  предположения» содержит только техническое (CSS wrap, ариа помимо
+  существующей практики, немецкая формулировка), что и предписано процессом
+  (владельцу — только продуктовые вопросы).
+- Трек `small` подтверждён по всем пяти критериям §5 одновременно (сложность
+  ≤3, одна поверхность — тулбар, без миграции конфига, без нового
+  UX-контракта — поведение и подпись это то же самое, что было до #29, без
+  влияния на перф/touch).
+- i18n-таблица (en/ru/de) соответствует существующей терминологии словаря
+  (`Hinzufügen` уже используется для того же действия).
+- Golden-план различает сцену, которая обязана измениться
+  (`geometry-devices-editor-dark`, тулбар виден целиком), от сцен, которые не
+  должны (`device-inbox-*`, тулбар закрыт диалогом) — реальное чтение сцен это
+  подтверждает.
+- Обе версии USER-GUIDE названы в затронутых файлах и обе действительно
+  требуют правки (EN — список, RU — таблица, оба без строки «Добавить»).
+- Откат описан и достаточен: удалить кнопку, 6 i18n-записей, тестовые ожидания
+  и документацию; миграции данных нет, потому что persisted-модель не менялась.
+
+## Чего не проверял
+
+- Не запускал `npm run typecheck`/`test`/`build`/`check-docs.mjs` — кода нет,
+  это стадия ревью ТЗ (§2.4), не код-ревью (§2.7); гейты будут обязательны на
+  выходе из «В разработке».
+- Не оценивал реальный визуальный вид будущей кнопки (макета/скриншота нет) —
+  оценивал только текстовый контракт (иконка/подпись/позиция/порядок), это и
+  есть предмет ТЗ-ревью.
+- Не проверял `npm run bundle:budget` фактически — утверждение «budget не
+  повышается» для одной кнопки и 6 строк i18n правдоподобно на глаз, но не
+  измерено; риск оцениваю как пренебрежимо малый, отдельной находкой не делаю.
+- Не проверял немецкую формулировку `title.add_device` носителем языка —
+  только на согласованность с существующим словарём; ТЗ само помечает её как
+  предположение, которое ревьюер вправе поправить одной строкой в код-ревью.
+
+## Вердикт
+
+Единственная находка — Medium, в скоупе, без High. По PROCESS.md §2.4/§7.2 это
+жёлтый вердикт: ТЗ возвращается автору на правку раздела Release-артефакты/AC7,
+без создания отдельного issue.