Files
2026-09-23 06:36:39 +00:00

18 KiB
Raw Permalink Blame History

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 идентичен, английские/немецкие/французские значения не участвуют в правке.
  • Границы не-скоупа реальны, а не декларативны. Проверено, что 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 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: 88e8b07c965637f0e02f38636200266b65a9fdcf
    git log --all --format='%H %T' | grep 88e8b07c9656
    
  • Тело issue: ba5e9ea73d14fd143edf181ea90e230889146e7e2a6ce3ed6a46625deae355f1
  • Вердикт конвейера: green · High 0