mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-01 12:18:51 +00:00
@@ -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 (`<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) сняты с записью, без правки текста.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/610-discard-copy-icon`, коммит `e29dfdeb7230` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `88e8b07c965637f0e02f38636200266b65a9fdcf`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 88e8b07c9656
|
||||
```
|
||||
- Тело issue: `ba5e9ea73d14fd143edf181ea90e230889146e7e2a6ce3ed6a46625deae355f1`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user