From 5a285a6a4bb3c754e08475af6156e75cb2b50eff Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 09:23:00 +0000 Subject: [PATCH] docs: review document for #363 Issue: #363 User-Visible: no --- docs/reviews/SPEC-REVIEW-363-r1.md | 318 ++++++++++++++--------------- 1 file changed, 159 insertions(+), 159 deletions(-) diff --git a/docs/reviews/SPEC-REVIEW-363-r1.md b/docs/reviews/SPEC-REVIEW-363-r1.md index c8102bda..1022321f 100644 --- a/docs/reviews/SPEC-REVIEW-363-r1.md +++ b/docs/reviews/SPEC-REVIEW-363-r1.md @@ -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» не завожу — по счётчику +конвейера это первый действительный заход, и я перепроверил всё заново, а +не по дельте. ## Как проверялось -Не поверил ни одному фактическому утверждению ТЗ и аналитики на слово — -перепроверил каждое чтением текущего дерева `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=`, а обновлённый + `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` +подтверждён повторно.