18 KiB
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 (<ha-icon icon=...>) — сдвиг на 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: первая<button class="btn ghost" ... autofocus data-hp="dialog-cancel">привязана кcancelLabel=dialog.discard_keep; - «Escape/крестик/scrim эквивалентны безопасному действию» — подтверждено:
dismiss-on-scrim+@hp-close=${() => this._decide(false)}, и все четыре вызывающих сайта трактуютaccepted === falseкакrejectClose()(остаться на форме), аtrue— какclose()(сброс черновика); - «изменяются только два существующих RU-ключа» — подтверждено: во всех
четырёх локалях (
ru/en/de/fr) набор ключейdialog.discard_title/ _message/_confirm/_keepидентичен, английские/немецкие/французские значения не участвуют в правке.
- «первая кнопка — безопасная, с autofocus» — подтверждено
- Границы не-скоупа реальны, а не декларативны. Проверено, что
discard-radar-setup(src/radar-setup.ts) — той же формы ключ (discard-*), но использует отдельные i18n-ключи (radar.discard_setup_title,btn.close,btn.cancel), поэтому переименованиеdialog.discard_keep/dialog.discard_confirmего не затронет — п.3 «Не входит» не проходит мимо скрытого пересечения. То же дляvacuum-maps-section.ts,space-copy-runtime.ts,houseplan-card.ts,houseplan-editor-runtime.ts,summary-panel-runtime-loaded.ts— все используют другие ключи/сообщения (vac.route_*, и т. п.), не пересекаются с изменяемыми. - Риск 320 px назван и он реален. «Не сохранять» (12 симв. с пробелом)
длиннее старого «Отменить» (8 симв.); AC2 явно требует прогон на 320 px, и это
ровно тот геометрический оракул, который уже покрыт
smoke_dialog_polish_603.mjs(проверено — файл существующий, содержит цикл по ширинам320/360/560/640, языкам и темам). - Golden-сцена и registry-состояние названы точно, не выдуманы:
room-discard-dialog-mobile-ruсуществует вdemo/golden/matrix.mjs:1054и вdemo/golden/baselines/baselines-index.json:177— п.10 корректно требует её принятия только черезgolden:accept -- --reviewedиз полного Linux CI артефакта, что совпадает с AGENTS.md/PROCESS.md. HpConfirmRequestрасширение реализуемо без слома fallback: полеicon?для заголовка уже существует и уже используется как override с fallback (request.icon || (destructive ? ... : 'mdi:lock-open-alert-outline')); кнопка подтверждения иконку изrequestсейчас не берёт вовсе (жёсткоmdi:trash-can-outline/mdi:lock-open-variant) — ровно то расхождение, которое п.2 ТЗ просит закрыть новым отдельным полем, не трогая существующее поведение остальных вызовов без явного override (AC4).- Ссылка на коммит
0a3e0674(«сокращено ... под 320 px», #603) существует и соответствует описанию (fix(dialogs): finish discard, switch and room polish (#603)). - Обе предыдущие спецификации того же диалога (#603, #607) существуют в
docs/reviews/и описывают ту же пару кнопок и тот же безопасный порядок — новое ТЗ с ними не расходится, только сужает конкретно RU-подписи и иконку.
Чего не проверял
- Не запускал
npm run typecheck/npm test/npm run buildи любыеdemo/smoke_*.mjs— на этапе spec нет диффа кода, гейтам нечего проверять; это относится к код-ревью после реализации. - Не проверял фактическую вёрстку/CSS-перенос текста на 320 px эмпирически (браузером) — это доказывается смоком AC2 в код-ревью, а не в спецификации.
- Не пересматривал классификацию
лёгкий трек/приоритетP3по существу — это решение этапа аналитики (§2.2 PROCESS.md), а не спецификационного ревью; см. L2 как наблюдение, а не отмену. - Не проверял, действительно ли между комментарием «Вопросы владельцу» и
фиксацией ТЗ в теле issue существовал отдельный акт согласия владельца
помимо самих комментариев — доступные через
gh issue viewданные показывают только эти два комментария и итоговое тело с разделом## ТЗ, совпадающим с предложенными дефолтами; расхождений между вопросом и решением нет, поэтому дальше не докапывался.
Вердикт
Зелёный. Спецификация полна по §7.1, каждый AC проверяем и привязан к способу доказательства, продуктовые развилки заданы владельцу и закрыты явным решением (не догадкой), границы скоупа подтверждены чтением кода, а не предположением. Три Low-наблюдения (L1-L3) сняты с записью, без правки текста.
Материал раунда
- Ветка:
issue/610-discard-copy-icon, коммитe29dfdeb7230— ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. - Дерево материала:
88e8b07c965637f0e02f38636200266b65a9fdcfgit log --all --format='%H %T' | grep 88e8b07c9656 - Тело issue:
ba5e9ea73d14fd143edf181ea90e230889146e7e2a6ce3ed6a46625deae355f1 - Вердикт конвейера:
green· High 0