diff --git a/docs/reviews/SPEC-REVIEW-610-r1.md b/docs/reviews/SPEC-REVIEW-610-r1.md new file mode 100644 index 00000000..3b883189 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-610-r1.md @@ -0,0 +1,202 @@ +# SPEC-REVIEW-610-r1 + +Issue: #610 · Этап: spec (PROCESS.md §2.4) · Заход r1 · блокирующих циклов 0/2 (лёгкий трек, лимит 2) +Материал: тело issue #610, раздел `## ТЗ` (одиннадцать пронумерованных подпунктов), плюс +комментарии `Аналитика` и `Вопросы владельцу перед ТЗ`. +sha256 сырого тела issue на момент ревью: `013a1dfe0d5f7860531887d5a3167eb2d4061032fe6cc16a86bbd97ca859cae9` +(вычислено ревьюером через `gh issue view 610 --json body -q .body | sha256sum`; это хеш +необработанного `body`, а не нормализованный хеш конвейера — приводится для трассируемости +раунда, а не как замена якоря конвейера). +Метки на момент ревью: `bug`, `P3`, `polish`, `S4-spec-review`, `small`. +Код не менялся: `git status`/`git diff origin/dev...HEAD` пустые — ревью чисто спецификационное. + +## Скоуп задачи + +RU-копия диалога «Отменить изменения?» (все четыре `discard-*-dialog`: general +settings, room, space, marker) переименовывает две кнопки — `Продолжить` → +`Вернуться`, `Отменить` → `Не сохранять` — и заменяет иконку замка +(`mdi:lock-open-alert-outline` / `mdi:lock-open-variant`) на +`mdi:content-save-off-outline` в заголовке и на кнопке `Не сохранять`, только для +этих четырёх сценариев. По `docs/SCOPE.md` это чистое обслуживание J4/J6 +(«keep the plan true», понятное безопасное действие в редакторах настроек) — +без миграции данных, без нового UX-контракта, поверхность одна (общий +диалог `hp-confirm`/`danger-confirm`, переиспользуемый в четырёх формах). +Лёгкий трек (`small`) — решение уже принято на этапе аналитики тем же +комментарием, спецификационное ревью его не пересматривает по существу (см. +замечание L3 ниже — не блокирует). + +## Как проверялось + +Диф отсутствует, поэтому проверка — построчное сопоставление каждого +утверждения ТЗ с текущим кодом на `HEAD` (`e29dfdeb`), а не запуск гейтов: + +- `src/hp-confirm.ts`, `src/danger-confirm.ts` — текущий рендер, `HpConfirmRequest`, + дефолтная иконка предупреждения, поведение `_decide`/`hp-close`. +- `src/i18n/settings/{ru,en,de,fr}.json` — набор ключей `dialog.discard_*` в + четырёх локалях. +- `src/editors/{general-settings-dialog,space-form,room-settings-dialog,marker-dialog}.ts` — + все четыре вызова `_confirmDanger`/`requestClose` с ключами `discard-*-dialog`. +- `src/radar-setup.ts`, `src/houseplan-card.ts`, `src/houseplan-editor-runtime.ts`, + `src/space-copy-runtime.ts`, `src/summary-panel-runtime-loaded.ts`, + `src/editors/vacuum-maps-section.ts` — прочие `kind: 'warning'` сценарии, чтобы + убедиться, что не-скоуп (§3 ТЗ) действительно про другие i18n-ключи и не + пересекается с изменяемыми. +- `demo/smoke_dialog_polish_603.mjs`, `demo/smoke_room_settings_form.mjs`, + `demo/golden/harness.mjs`, `demo/golden/matrix.mjs`, `demo/golden/baselines/baselines-index.json` — + существование и структура тестов/сцены, которые ТЗ обещает обновить. + `docs/USER-GUIDE.ru.md` — поиск существующих цитат кнопок диалога. + `docs/reviews/{SPEC,CODE}-REVIEW-603-*.md`, `SPEC/CODE-REVIEW-607-*.md` — история + этого же диалога (два недавних раунда касались той же пары кнопок). +- `git show --stat 0a3e0674` — проверка ссылки на коммит #603, на который ссылается + фон задачи («сокращено в `0a3e0674`»). + +Гейты (`typecheck`/`test`/`build`/смоки) не запускались — на этапе spec материал +для них отсутствует (код не менялся); это не пропуск, а отсутствие предмета. + +## Находки + +High: 0. Medium: 0. + +Ниже — три Low-наблюдения; ни одно не мешает разработке, снимаю все три записью +(правка ТЗ не требуется для перехода в «Готово к разработке»). + +**L1 — устаревшие номера строк в фоновом описании дефекта.** +Раздел «Что не так» (текст до заголовка `## ТЗ`, наследие исходного репорта) +ссылается на `src/hp-confirm.ts:44-45, 58-59`; на `HEAD` те же выражения лежат на +строках 42-43 (`.icon=...`) и 59-60 (``) — сдвиг на 1-2 строки, +видимо из-за правок между аудитом (22.09) и текущим коммитом. Раздел находится +до `## ТЗ` и не входит в нормируемые §7.1 разделы (проблема формулируется заново +в п.1 «Сценарий»), контракт и AC от этого расхождения не зависят. Снимаю без +правки: указание достаточно точное, чтобы код-ревьюер нашёл нужный код. + +**L2 — «одна поверхность» — четыре диалога, а не один.** +Критерий лёгкого трека (§5 PROCESS.md) требует «один диалог, один модуль, один +эндпоинт» одновременно с остальными. Задача правит текст и иконку в четырёх +разных пользовательских диалогах (general settings, room, space, marker) — +формально четыре поверхности с точки зрения человека, хотя все четыре +инстанцируют один и тот же `hp-confirm`/`danger-confirm` и один и тот же +i18n-ключ, так что правка одна по существу (общий компонент + общий ключ, не +четыре независимых решения). Аналитика (комментарий) уже перечисляет все +четыре формы отдельно и всё равно ставит `лёгкий трек: да` без явной ссылки на +то, почему «один диалог» в критерии читается как «один переиспользуемый +компонент диалога». Не блокирую: контракт от этого не становится +неоднозначным (§4 ТЗ прямо перечисляет все четыре сценария и их общее +поведение), и решение о треке — компетенция этапа аналитики (§2.2), а не +спецификационного ревью, которое проверяет исполнимость ТЗ, а не заново +классифицирует трек. Фиксирую как наблюдение для код-ревью: если по ходу +реализации выяснится второе расхождение (например, у одной из форм иное +поведение фокуса или scrim), критерий §5 будет нарушен по-настоящему и метка +`small` должна сняться (правило уже описано в PROCESS.md §5, задачу это не +блокирует явно). + +**L3 — AC5 называет один способ доказательства на два разных утверждения.** +AC5 объединяет «USER-GUIDE.ru однозначно объясняет обе кнопки» (текстовая +ясность прозы, доказывается чтением) и «golden-сцена `room-discard-dialog-mobile-ru` +ожидаемо изменится» (визуальный оракул, доказывается `golden:verify`/`golden:accept`) +под одной пометкой «docs/golden harness», тогда как AC3 и AC4 явно называют оба +способа доказательства через `+` («browser smoke + ревью кода», «unit/smoke + +ревью кода»). Разночтения это не создаёт — «docs» как категория доказательства +уже используется в DoR (§2.5 PROCESS.md) для release-артефактов отдельно от +кода — но единообразия ради стоило бы явно написать «ревью кода (USER-GUIDE) + +golden harness (сцена)». Снимаю как чисто редакционное: код-ревьюер и так обязан +прочитать обновлённый `USER-GUIDE.ru.md` при проверке AC5 независимо от точной +формулировки заголовка. + +## Что проверено и корректно + +- Обязательные разделы §7.1 присутствуют все: сценарий и «что человек увидит» + (п.1), проблема (внутри п.1), скоуп/не-скоуп (п.2-3), контракт поведения и UX + (п.4), данные/i18n/совместимость (п.5), список файлов (п.6), AC1…AC6 с + доказательством у каждого (п.7), план автотестов (п.8), риски/perf/touch (п.9), + откат и release-артефакты (п.10), явный блок принятых предположений/решений + владельца (п.11). +- **Продуктовые вопросы были заданы и закрыты до написания ТЗ**, а не + додуманы: комментарий `Вопросы владельцу перед ТЗ` ставит Q1 (русские подписи) + и Q2 (иконка) с вариантами по умолчанию каждый; итоговые формулировки в п.11 + «Принятые решения владельца» совпадают ровно с предложенными дефолтами — + никакого расхождения между вопросом и итоговым решением нет, и решение явно + помечено как решение, а не как факт из стороннего документа. +- **Ни одна строка ТЗ не выдаёт догадку за факт.** Три поведенческих + утверждения п.4 проверены построчным чтением кода, а не поверено на слово: + - «первая кнопка — безопасная, с autofocus» — подтверждено `hp-confirm.ts`: + первая `