docs: review document for #363

Issue: #363
User-Visible: no
This commit is contained in:
claude[bot]
2026-08-29 09:23:00 +00:00
parent 9d6d6eb9e9
commit 5a285a6a4b
+159 -159
View File
@@ -3,180 +3,180 @@
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).
ТЗ в теле issue #363: возврат кнопки «Добавить» в основной тулбар Device
editor рядом с «Устройства» — короткий путь к уже существующему диалогу
нового устройства (`_openMarkerDialog()` без аргумента), убранной в `cab8d128`
(#29) вместе с каталогом «Устройства». Каталог не трогается. Класс изменения —
A (`src/houseplan-editor-runtime.ts`, три словаря `src/i18n/*.json`) плюс C
(`docs/USER-GUIDE*.md`, оба changelog). Метка `small` подтверждена в
аналитике владельца (S2): сложность 2/10, одна поверхность, миграций нет,
нового UX-контракта нет (контракт — тот же, что до #29).
## Важное обстоятельство состояния (проверить перед чтением остального)
В issue уже есть более ранняя попытка этого же захода: комментарий от
`claude` (2026-08-29T09:16:10Z, жёлтый вердикт, `Заход r1 · блокирующих
циклов 1/2 · High: 0 · Medium: 1`) и закоммиченный на `dev`
`docs/reviews/SPEC-REVIEW-363-r1.md` (коммит `9d6d6eb9`, тот же SHA, что и
текущий HEAD). Тот документ нашёл **M1**: раздел «Release-артефакты» не
называл обязательный прогон workflow «Скриншоты документации» и
`docs:accept --reviewed`, из-за чего `docs`-job в CI Validate стал бы
красным по расхождению `sourceFingerprint`, даже если ни один PNG не
изменился (сценарий #230/#234).
Задача даёт этому заходу метаданные `Заход r1 · 0 из 2` — то есть с точки
зрения конвейера предыдущая попытка цикл не потратила: судя по всему, тот
прогон не дошёл до финального структурированного ответа (без него, по
инструкции конвейера, метка не переставляется), поэтому официально не
засчитан, а issue не уходил в `S3-spec`. Тем не менее тело issue уже
переписано в «ТЗ · revision 2» и текстуально закрывает M1 (см. ниже) — то
есть автор (владелец, пишущий ТЗ прямо в issue на лёгком треке) увидел
незасчитанный комментарий и поправил текст до этого захода. Я не доверяю
этому на слово и ниже перепроверяю сам, что revision 2 действительно
закрывает M1 и что остальные утверждения черновика остаются верными на
текущем дереве. Раздела «Унаследовано из r<N-1>» не завожу — по счётчику
конвейера это первый действительный заход, и я перепроверил всё заново, а
не по дельте.
## Как проверялось
Не поверил ни одному фактическому утверждению ТЗ и аналитики на слово —
перепроверил каждое чтением текущего дерева `dev` (`HEAD=c19a0540`):
Ничего не принято на слово из текста ТЗ или прежнего черновика — каждое
утверждение перепроверено чтением дерева на `HEAD=9d6d6eb9`:
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`. Проверил сам коммит-виновник:
1. **M1 закрыт.** AC7 сейчас: «...полный артефакт принимается через `npm run
docs:accept -- --reviewed --from=<artifact>`, а обновлённый
`docs/images/screenshots.json` коммитится... Все PNG канонического набора
(сейчас 10) ожидаются пиксельно неизменными». Раздел «Release-артефакты»
дублирует то же требование и явно называет workflow «Скриншоты
документации» обязательной «даже если видимая композиция кадров не
меняется». Проверил, что названные команда и workflow существуют:
`.github/workflows/docs-screenshots.yml` (шаг публикует
`docs/images/screenshots.json` артефактом), `package.json:16` —
`"docs:accept": "node scripts/docs-accept.mjs"`. Прежний черновик просил
именно это, только называл «9 PNG» — сейчас в дереве
`demo/docs/screenshots.mjs` действительно **10** сценариев (проверил
`grep -n "id:" demo/docs/screenshots.mjs` и `docs/images/screenshots.json
→ scenarios.length`), то есть текущее ТЗ точнее черновика, а не расходится
с деревом задним числом.
2. **`_openMarkerDialog()` жив, контракт не меняется.**
`src/houseplan-editor-runtime.ts:7628` `public _openMarkerDialog(d?:
DevItem)`; вызов без аргумента — путь диалога нового устройства. Кнопка
«Устройства» вызывает другой метод, `_openDeviceInbox()`
(`houseplan-editor-runtime.ts:11545`), а «Добавить виртуальное устройство»
внутри каталога вызывает тот же `_openMarkerDialog()` без аргумента
(`houseplan-editor-runtime.ts:11578`, `openVirtual`). Значит новая кнопка
получает ровно тот же путь, что уже используется каталогом для того же
исхода — вторая, независимая проверка того, что AC2 просит воспроизводимую,
а не новую логику.
3. **i18n-таблица побайтово совпадает с состоянием до `cab8d128`.**
`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-целей не создаёт.
`"devbar.add": "Add"` / `"Добавить"` и `"title.add_device": "Add a device
to the plan"` / `"Добавить устройство на план"` — совпадает посимвольно с
таблицей ТЗ. Оба ключа сейчас отсутствуют во всех трёх словарях
(`grep -n "devbar.add\|title.add_device" src/i18n/{en,ru,de}.json` — пусто),
значит работа по их возврату реальна, а не фиктивна.
4. **Немецкий текст — согласованное предположение, не догадка.** `de.json`
уже использует `"device_inbox.add": "Hinzufügen"` для того же действия и
паттерн «существительное + hinzufügen» для похожих подписей
(`"title.add_space": "Bereich hinzufügen"`). Предложенные `"Hinzufügen"` /
`"Gerät zum Plan hinzufügen"` этому соответствуют. Помечено в ТЗ как
принятое предположение, которое ревьюер вправе оспорить — не оспариваю.
5. **Паритет словарей ловит забытую локаль.** `test/i18n.test.mjs:109-112`:
`assert.deepEqual(Object.keys(dictionary).sort(), enKeys)` для каждого
языка реестра. Пропуск любого из 6 значений (2 ключа × 3 локали) красит
`npm test`. AC4 доказуемо этим тестом, и тест умеет падать (сейчас ключей
нет ни в одном словаре — не найти это на голом дереве, если ключ забыт,
тест обязан споткнуться).
6. **Место вставки и порядок.** Текущий `_renderDevicesBar()`
(`houseplan-editor-runtime.ts:11541-11553`) начинается с кнопки
`device_inbox.button` («Устройства»), затем `devbar.rules` («Правила
иконок»). До #29 первой кнопкой была «Добавить»
(`git show cab8d128^:src/houseplan-card.ts` — фрагмент, приведённый в теле
issue, подтверждён отдельно). Вставка новой кнопки перед «Устройства»
восстанавливает этот порядок; номера строк в S2-аналитике (`:11491`)
немного разошлись с текущими из-за более поздних коммитов — не находка,
метод и разметка на месте.
7. **Каталог и его «Добавить виртуальное устройство» не задеты.**
`_renderDeviceInbox()` / `openVirtual()` вне диапазона правок ТЗ.
`demo/smoke_device_inbox.mjs` существует, его заголовок прямо ссылается на
#29 («one lifecycle catalog replaces the separate Add / hidden-device
paths») — подходящий существующий регресс-щуп для AC3.
8. **RU/EN руководство действительно потеряло строку.**
`docs/USER-GUIDE.ru.md:775-786` — таблица «Редактор устройств» содержит
«Устройства» и «Добавить виртуальное устройство», строки про «Добавить»
нет. `docs/USER-GUIDE.md:519-529` — список без пункта «Add». Оба файла
названы в «Затронутых файлах».
9. **Golden-сцены разведены верно.** `demo/golden/matrix.mjs:381` —
`geometry-devices-editor-dark`: `mode: 'devices'`, **без** ключа `dialog` —
тулбар виден целиком, третья кнопка попадёт в кадр. `device-inbox-*`
(строки 383-390) все три несут `dialog: 'device-inbox'` — модальный
каталог перекрывает тулбар, эти кадры не должны измениться.
`--expect-change=geometry-devices-editor-dark` в AC8 — точная и
единственная нужная пометка.
10. **UX-MODES.md / TOUCH-SUPPORT.md** — персистентный инструмент в основном
тулбаре, `barclose` остаётся в правом торце (`editbar-end`, не тронут);
новых touch-целей и жестов не вводится. Конфликта с «редакторы
desktop-first, touch — best effort» нет.
11. **Обязательные разделы §7.1** — сценарий+результат, скоуп/не-скоуп,
контракт поведения и UX, данные/совместимость (модель не меняется),
i18n, AC1-AC8 с доказательством каждого, план тестов и мутанты, риски и
откат, release-артефакты — все присутствуют в теле issue.
12. **Продуктовые вопросы, которые могли уйти владельцу как технические** —
не найдены: три «принятых предположения» (CSS-классы/wrap, отсутствие
отдельного disabled-state, немецкий перевод) — все технические/оформ-
ительские, ни одно не требует продуктового решения.
## Гейты этого захода
Материал — только текст issue, продуктового кода по #363 в дереве ещё нет
(`git log --all --oneline | grep 363` и `git branch -a | grep 363` не находят
ветки/коммитов задачи, кроме случайных числовых совпадений `3633a3db`
(#159), `9d6d6eb9`/это же ревью). Прогонять `typecheck`/`test`/`build`/
`check-docs` не над чем — это ревью текста (§2.4), а не кода (§2.7); их место
в коде-ревью после реализации. Отдельно перечитал и вручную проверил
исполняемость `test/i18n.test.mjs` (пункт 5) и существование
`demo/smoke_device_inbox.mjs`, `.github/workflows/docs-screenshots.yml`,
`scripts/docs-accept.mjs` чтением, не запуском — их сегодняшнее содержимое,
а не будущее поведение, было предметом проверки.
## Находки
### 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, где ожидается
измениться ровно один кадр).
Нет. M1 предыдущего (незасчитанного) захода закрыт текстом ТЗ (см. пункт 1
выше) и проверен по дереву, а не на слово. Догадок, выданных за решение, не
найдено. Технических вопросов, которые следовало решить самому вместо
эскалации владельцу, не найдено — их и не было.
## Что проверено и корректно
- Скоуп/не-скоуп разведены точно и не заходят на каталог, 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-модель не менялась.
- Обработчик `_openMarkerDialog()` и его контракт (пункт 2).
- Побайтовое совпадение i18n-таблицы с состоянием до `cab8d128` и паритет-тест
(пункты 3, 5).
- Согласованность предложенного немецкого текста с действующим словарём
(пункт 4).
- Место вставки и восстановленный порядок кнопок (пункт 6).
- Независимость каталога и его «Добавить виртуальное устройство» (пункт 7).
- Пропуск строки тулбара в RU/EN руководстве (пункт 8).
- Разведение golden-сцен `geometry-devices-editor-dark` vs `device-inbox-*`
(пункт 9).
- Отсутствие конфликта с UX-MODES.md / TOUCH-SUPPORT.md (пункт 10).
- Полнота обязательных разделов ТЗ §7.1 и отсутствие невынесенных
продуктовых вопросов (пункты 11-12).
- Закрытие M1 предыдущего захода текстом revision 2 (пункт 1).
## Чего не проверял
- Не запускал `npm run typecheck`/`test`/`build`/`check-docs.mjs` — кода нет,
это стадия ревью ТЗ (§2.4), не код-ревью (§2.7); гейты будут обязательны на
выходе из «В разработке».
- Не оценивал реальный визуальный вид будущей кнопки (макета/скриншота нет) —
оценивал только текстовый контракт (иконка/подпись/позиция/порядок), это и
есть предмет ТЗ-ревью.
- Не проверял `npm run bundle:budget` фактически — утверждение «budget не
повышается» для одной кнопки и 6 строк i18n правдоподобно на глаз, но не
измерено; риск оцениваю как пренебрежимо малый, отдельной находкой не делаю.
- Не проверял немецкую формулировку `title.add_device` носителем языка —
только на согласованность с существующим словарём; ТЗ само помечает её как
предположение, которое ревьюер вправе поправить одной строкой в код-ревью.
- Реализацию — её ещё нет; код-ревью (§2.7) проверит diff, а не текст ТЗ.
- Фактический прогон `typecheck`/`test`/`build`/`check-docs`/golden — не
применимо к этому заходу, материала для прогона (diff) не существует.
- Browser-смоки и golden-захват — то же самое, нечего запускать.
- Реальный CI-прогон workflow «Скриншоты документации» — оценивалась только
корректность его упоминания в ТЗ, а не его будущее исполнение.
## Вердикт
Единственная находка — Medium, в скоупе, без High. По PROCESS.md §2.4/§7.2 это
жёлтый вердикт: ТЗ возвращается автору на правку раздела Release-артефакты/AC7,
без создания отдельного issue.
Зелёный. High: 0, Medium: 0. ТЗ полно, однозначно, каждый AC привязан к
способу доказательства, единственная находка предыдущей (незасчитанной)
попытки закрыта текстом и подтверждена чтением дерева. Трек `small`
подтверждён повторно.