docs: review document for #660

Issue: #660
User-Visible: no
This commit is contained in:
claude[bot]
2026-09-26 07:23:13 +00:00
parent 54f197209d
commit 4b8a82c08a
2 changed files with 326 additions and 1 deletions
+2 -1
View File
@@ -1,9 +1,10 @@
# Индекс ревью
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1072, issue: 378. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1073, issue: 379. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
| Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы |
|---|---|---|---|---:|---:|---|---|
| #660 | [SPEC-REVIEW-660-r1.md](SPEC-REVIEW-660-r1.md) | spec · r1 | 🔴 красный | 2 | 1 | Раздел ## ТЗ в теле issue отсутствует целиком; Изменение прямо противоречит двум местам; AC «расстояние уменьшено ровно вдвое» не | `docs/process/AUTHOR.md` `REVIEWER.md` `test/core-file-budget.test.mjs` `scripts/smoke-select.mjs` `demo/helpers/hp-test.mjs` `docs/UX-MODES.md` `docs/reviews/SPEC-REVIEW-647-r1.md` |
| #656 | [CODE-REVIEW-656-r1.md](CODE-REVIEW-656-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
| #654 | [SPEC-REVIEW-654-r1.md](SPEC-REVIEW-654-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | «Release-артефакты» не называют обновление docs/ISOMETRIC.md | `docs/ISOMETRIC.md` `docs/CHANGELOG.md` `docs/CHANGELOG.ru.md` `docs/reviews/INDEX.md` |
| #654 | [SPEC-REVIEW-654-r2.md](SPEC-REVIEW-654-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — |
+324
View File
@@ -0,0 +1,324 @@
# SPEC-REVIEW-660-r1
- **Issue:** https://github.com/Matysh/houseplan-card/issues/660
- **Этап:** `S4-spec-review` (ревью ТЗ, PROCESS.md §2.4)
- **Трек:** полный (аналитика автора обосновывает отказ от `small`: >1
поверхности — layout шапки + summary-runtime/mobile-меню + клавиатурный
автомат Esc — и новый UX-контракт положения крестика и обработки `Esc`)
- **Материал:** тело issue #660 целиком (создано 2026-09-26T06:50:32Z,
событий `edited` в таймлайне issue нет — тело не менялось с момента
создания); комментарии: «Аналитика» (`#issuecomment-5844066158`) и ответ
владельца на Q1/Q2 (`#issuecomment-5844172216`)
- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4 (полный трек)
- **Роль:** ревьюер ТЗ (не автор)
## Скоуп ревью
Задача — полировка шапки основной панели после #647: (1) inline-кнопки
сводной панели остаются видимыми/активными во всех трёх редакторах на
ширинах >480 px; (2) расстояние между блоком кнопок редакторов и блоком
зума уменьшается вдвое; (3) резерв под крестик закрытия переезжает внутрь
общей подложки блока редакторов и всегда стоит сразу после активной
кнопки; (4) `Esc` получает единый терминальный выход в View из всех трёх
редакторов, если нет более приоритетного слоя/действия.
Проверялось: наличие и полнота обязательных разделов ТЗ (PROCESS.md §7.1),
однозначность и проверяемость каждого AC вместе со способом доказательства
(§2.5 DoR), соответствие описания коду «как есть», согласие с
`docs/SCOPE.md` и каноническим документом подсистемы (`docs/UX-MODES.md`),
отсутствие домыслов, выданных за факт.
## Как проверялось
1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `docs/process/REVIEWER.md` и
разделы PROCESS.md §2.3, §2.4, §2.5, §4, §7.1, §7.2.
2. Прочитано тело issue #660 целиком и оба комментария.
3. Сверена история меток issue (`gh api .../timeline`): `S1-new` →
`S2-analysis` (06:52:45) → `S3-spec` + `blocked` (06:54:38, тем же
моментом, что и комментарий «Аналитика») → владелец отвечает на Q1/Q2
(07:12:46) → `S3-spec`/`blocked` сняты, `S4-spec-review` поставлена
(07:12:48, через 2 секунды после ответа владельца). Событий `edited`
для тела issue в таймлайне нет — значит тело **никогда не
редактировалось** после первичного создания владельцем в 06:50:32.
4. Сопоставлен формат #660 с образцовым ТЗ того же полного/лёгкого трека —
`docs/reviews/SPEC-REVIEW-647-r1.md` и текстом раздела `## ТЗ` в теле
issue #647 (Сценарий, Что человек увидит до/после, Проблема по коду,
Скоуп/Не-скоуп, Контракт поведения К1…К7, UX/i18n/модель данных, таблица
«AC · чем доказан · чем краснеет», План автотестов, Риски, Откат,
Release-артефакты, «Принято предположительно»).
5. Прочитан `docs/UX-MODES.md` целиком (канонический документ подсистемы,
явно указан для этой задачи) — строки 56-61 и 116-125 описывают именно
ту архитектуру, которую #660 заменяет.
6. Прочитан код, который ТЗ описывает как причину бага:
`src/summary-panel-runtime-loaded.ts:262-263` (`renderControls`
возвращает `nothing`, если `this.host._mode !== 'view'`),
`src/header-menu.ts:79` (пункты сводной панели добавляются только при
`input.mode === 'view'`), `src/houseplan-card.ts:10812-10829`
(`.editor-close-slot` — отдельный сосед `.modes`, между ними и
`.zoomctl` — `.spacer`), `src/styles/chrome.styles.ts` (раздельные gap
на `.modes`/`.editor-close-slot`/`.zoomctl`) — описание проблемы в теле
issue совпадает с кодом.
7. Прочитан `src/houseplan-card.ts:2758-2992` (`_onKey`) — проверены
утверждения комментария «Аналитика» построчно:
- `decor` (Background): цепочка отмены действительно оканчивается
`else this._setMode('view')` (строка 2872) — уже выходит в View в
нейтральном состоянии, как и заявлено.
- `devices`: `if (e.key === 'Escape' && this._deviceDrag) { ...
_cancelDeviceDrag(); } return;` (2888-2892) — при отсутствии
активного drag `Escape` не делает ничего, выхода в View нет —
подтверждено.
- `plan` (`_markup`): цепочка (2936-2992 и далее) отменяет
draw/resize/split/merge/physicalSel и т.д., но ни одна ветка не
вызывает `_setMode('view')` — в нейтральном состоянии `Esc` тоже
ничего не делает — подтверждено.
8. Гейты не гонялись: этап `spec`, диапазон ревью — текст тела issue, а не
код. Ожидаемо для ревью ТЗ.
## Находки
### High-1 (в скоупе). Раздел `## ТЗ` в теле issue отсутствует целиком —
есть только исходная постановка владельца и два продуктовых ответа,
без единой из обязательных частей §7.1
**Файл:** тело issue #660 (весь документ, п.3 «Как проверялось» — снимок
таймлайна подтверждает отсутствие правок тела после создания).
**Что не так.** Решение #517 и `docs/process/AUTHOR.md`/`REVIEWER.md`
фиксируют: ТЗ — это раздел `## ТЗ` в теле issue, который автор ТЗ пишет на
шаге «ТЗ в работе» (`S3-spec`), а не исходная жалоба владельца. Для
сравнения — прямой аналог того же класса задачи, #647, на том же полном
формате прошёл ровно этот шаг: после `---` в теле появился раздел `## ТЗ`
с Сценарий → Что человек увидит до/после → Проблема (по коду) → Скоуп/
Не-скоуп → Контракт поведения (К1…К7) → UX/i18n/модель данных → таблица
«AC · чем доказан · чем краснеет» → План автотестов → Риски → Откат →
Release-артефакты → «Принято предположительно».
В #660 таймлайн показывает, что `S3-spec` был выставлен **в тот же
момент**, что и комментарий «Аналитика» (06:54:38), тут же получил
`blocked` в ожидании ответа владельца, и сразу после ответа владельца
(07:12:46) — **через 2 секунды** (07:12:48) — был выставлен
`S4-spec-review`, без единой правки тела issue. То есть шаг «написать
полную первую редакцию по §7» (выход из `S3-spec`, PROCESS.md §2.3) не
состоялся: то, что попало на ревью — это исходная жалоба владельца
(«Проблема» / «Требуемое поведение» / «Принятые решения владельца» /
«Критерии приёмки» / «Регрессионные проверки» / «Связанные задачи» /
«Пользовательский результат»), без единой из следующих обязательных
частей §7.1:
- **Сценарий** — какая персона, на какой поверхности, в какой момент,
явным разделом (сейчас это можно только реконструировать из контекста).
- **Что человек увидит до и после** — одной фразой без терминов
реализации, в паре «до»/«после» (сейчас есть только «Пользовательский
результат» — фраза о результате без явного «до»).
- **Не-скоуп** — ни слова о границах: не тронуты ли kiosk-заголовок,
диалоги, крестики `.barclose` нижних панелей редакторов, контракт View
overlay сводной панели вне ширины >480 px, порядок/действия других
кнопок шапки.
- **Модель данных и миграция** — не упомянуты вообще (даже как явное
«нет»), при том что «show/hide меняет сохранённый локальный выбор» —
это чтение/запись существующего состояния, которое стоило явно назвать.
- **i18n** — не упомянут вообще: новых ключей нет или переиспользуются
старые (`title.close_editor` и т.п.) — это нигде не сказано явно.
- **Способ доказательства для каждого AC** — отсутствует полностью. Ни
одна из 10 строк чек-листа «Критерии приёмки» не помечена
`unit`/`backend`/`smoke`/`golden`/«ревью кода», что прямо требуется
DoR §2.5 («у каждого указано, чем он доказывается») и является одним из
двух пунктов, которые ревьюер ТЗ обязан проверить (§2.4, §7.1). Раздел
«Регрессионные проверки» называет типы смоков в общем виде («Browser
layout smoke», «Keyboard smoke»), но не привязывает ни один из них к
конкретному AC и не называет ни одного файла/механизма (в отличие от
ТЗ #647, где план автотестов ссылается на конкретные
`test/core-file-budget.test.mjs`, `scripts/smoke-select.mjs`,
`demo/helpers/hp-test.mjs`, фасад `data-hp="mode-tab"`).
- **Риски** — не названы. Не оценён риск golden-регресса (шапка почти во
всех кадрах меняется третий раз подряд после #647/#195), риск задеть
общий CSS-токен gap (текущее расстояние `.modes` → `.zoomctl` — не один
`gap`, а слот + два межэлементных gap + `.spacer`, по признанию самой
«Аналитики»; если хотя бы один из участвующих CSS-токенов переиспользуется
в другом месте шапки/диалогов, «уменьшить вдвое» механической правкой
переменной заденет то, что не должно меняться), риск сломать
существующие смоки #647/#195, которые ищут крестик в `.editor-close-slot`
как отдельного соседа `.modes`, а не как элемент внутри общей подложки.
- **Откат** — не назван (обычно тривиален — `git revert» — но должен быть
явно написан, а не подразумеваться).
- **Release-артефакты** — не названы: не сказано, что `User-Visible: yes`
требует правки обоих changelog в том же коммите, и не назван
`docs/UX-MODES.md` (см. High-2 ниже).
- **«Принято предположительно, поменять свободно»** — блок отсутствует, а
нерешённых технических деталей достаточно: точный CSS-токен(ы),
участвующие в «уменьшить вдвое» (общий или новый), конкретные ширины,
на которых проверяется расстояние (текущее расстояние differs: 100 px на
768–1400 px и 81 px на 481–620 px по данным самой «Аналитики» — ТЗ не
говорит, должны ли оба значения быть независимо разделены пополам или
сведены к одному новому), формат сдвига кнопок правее активной (анимация
или мгновенно), что именно считается «активным отменяемым действием
инструмента» для точной границы приоритета в K8/AC8 сверх трёх примеров
из текста.
Эффект: ни один из 10 AC нельзя признать проверяемым в смысле §2.4/§2.5 —
не потому, что формулировки плохи (большинство однозначны как продуктовое
намерение), а потому, что для каждого из них отсутствует обязательная
часть требования — способ доказательства. Это не редакционная мелочь: это
именно тот шаг ревью ТЗ, который явно поручен ревьюеру («Проверь
обязательные разделы §7.1, однозначность каждого AC и указание способа
доказательства»), и по нему обязательные части не выполнены ни на одну
из десяти строк.
**Как закрыть.** Автору ТЗ нужно фактически выполнить шаг `S3-спецификация`
(PROCESS.md §2.3) — дописать раздел `## ТЗ` в теле issue по образцу
`docs/reviews/SPEC-REVIEW-647-r1.md`/тела issue #647: Сценарий · Что
человек увидит до/после · Проблема (по коду, с конкретными строками) ·
Скоуп/Не-скоуп · Контракт поведения (по каждому из 4 пунктов «Требуемого
поведения») · UX/i18n/модель данных · таблица AC с указанием способа
доказательства (и, где применимо, «чем краснеет») · План автотестов с
именами существующих механизмов (`scripts/smoke-select.mjs`,
`demo/helpers/hp-test.mjs`, фасад `data-hp`) · Риски · Откат ·
Release-артефакты (см. High-2) · «Принято предположительно».
### High-2 (в скоупе). Изменение прямо противоречит двум местам
канонического `docs/UX-MODES.md`, а обновление этого документа нигде не
запланировано
**Файл:** `docs/UX-MODES.md:56-61`, `docs/UX-MODES.md:116-125`; тело
issue #660 (раздел «Release-артефакты» отсутствует — см. High-1).
**Что не так.** Действующий канон дословно описывает архитектуру, которую
#660 меняет:
> its own fixed 24 × 24 px slot right after the mode tabs (#647): the slot
> keeps its size, empty and hidden from assistive technology, **outside the
> editors** (docs/UX-MODES.md:57-58)
— #660 переносит крестик внутрь общей подложки блока редакторов и требует,
чтобы он «всегда располагался непосредственно после активной кнопки»
(т.е. позиция внутри группы меняется в зависимости от режима) — это прямо
не «fixed slot right after the mode tabs», а другая, режимо-зависимая
компоновка.
> All summary surfaces disappear in Plan, Devices and Background;
> `houseplan-space-card` never renders them. (docs/UX-MODES.md:124-125)
— это ровно то предложение, которое AC1 из #660 отменяет для широких
экранов (inline-кнопки остаются видимыми и активными во всех трёх
редакторах). После реализации #660 это предложение канона станет неверным
буквально в момент мержа, и ни один раздел ТЗ (потому что раздела
«Release-артефакты» просто нет — High-1) не требует его исправить —
прецедент такого требования уже был для этой же подсистемы: SPEC-REVIEW-
647-r1 находкой Medium-2 обязал автора включить `docs/UX-MODES.md` в
Release-артефакты и обновить обе строки про крестик; ТЗ #660 повторяет
тот же пробел на том же документе.
**Как закрыть.** Добавить в ТЗ явную правку `docs/UX-MODES.md`: заменить
строки 57-58/60-61 описанием новой, режимо-зависимой позиции крестика
внутри общей подложки (без «outside the editors» и без «fixed... right
after the mode tabs» в прежнем смысле) и заменить строку 124 указанием,
что на ширинах >480 px inline-кнопки сводной панели остаются активными в
Plan/Devices/Background, а сам overlay — по-прежнему нет. Внести
`docs/UX-MODES.md` в Release-артефакты рядом с changelog.
### Medium-1 (в скоупе). AC «расстояние уменьшено ровно вдвое» не
называет допуск и не привязан к конкретным ширинам, при том что исходное
расстояние неоднородно по брейкпоинтам
**Файл:** тело issue #660, раздел «Критерии приёмки», пункт 2; сверено с
комментарием «Аналитика» (`#issuecomment-5844066158`, п.3).
**Что не так.** Формулировка — «расстояние… уменьшено ровно вдвое
относительно текущего `dev`» — не указывает допустимую погрешность (для
сравнения, соседний AC о ширине блока редакторов прямо называет «допуск
не более 1 CSS px»). Собственный разбор автора в «Аналитике» установил,
что на `dev` это расстояние **не одно число**: 100 px на ширинах
768/1000/1200/1400 px и 81 px на 481–620 px — то есть при буквальном
прочтении AC нужно раздельно проверять двоение к 50 px и к 40.5 px
(дробный px) на разных диапазонах ширины, либо свести к единому новому
значению, а ТЗ не говорит, какой из двух вариантов верен, и не задаёт
допуск для сравнения с дробным px. Без этого браузерный смок,
который должен ловить регресс, не может быть написан однозначно — два
разных исполнителя закономерно напишут два разных (оба «соответствующих
тексту ТЗ») смока с разными числовыми порогами.
**Как закрыть.** В ТЗ явно перечислить целевые значения по диапазонам
ширины (например: 50 px на 768–1400 px, 40 px на 481–620 px, округление
до ближайшего целого CSS px) и указать допуск (по аналогии с другими AC —
не более 1 CSS px).
## Что проверено и корректно
- Аналитика корректно относит задачу к J4/J6 и desktop-first контракту
редакторов (`docs/SCOPE.md`) — это обоснованный polish существующего,
уже принятого механизма (#647), а не новая функция вне скоупа продукта.
- Отказ от лёгкого трека обоснован верно: задействовано больше одной
поверхности (layout шапки, summary-runtime/мобильное меню, клавиатурный
автомат `Esc`) и вводится новый UX-контракт положения крестика — оба
критерия §5 нарушены, полный трек — правильный выбор.
- Оба вопроса владельцу (Q1, Q2) — продуктовые по существу («что видит и
делает пользователь при клике внутри редактора», «меняется ли состав
мобильного меню»), поставлены с предлагаемым вариантом по умолчанию, как
требует §7.1; отдельных технических вопросов, вынесенных владельцу
неправомерно, не найдено.
- Утверждения о текущем коде, на которых строится обоснование задачи,
проверены и подтвердились дословно: `renderControls()` гасится вне View
(`src/summary-panel-runtime-loaded.ts:263`), пункты сводной панели в
мобильном меню гасятся так же (`src/header-menu.ts:79`),
`.editor-close-slot` — отдельный сосед `.modes` со `.spacer` перед
`.zoomctl` (`src/houseplan-card.ts:10812-10829`); поведение `Esc` в
каждом из трёх редакторов (`_onKey`, `src/houseplan-card.ts:2825-2992`)
соответствует описанному: `decor` уже выходит в View в нейтральном
состоянии, `devices` и `plan` — нет.
- Критерии приёмки как продуктовые формулировки (без учёта отсутствующего
способа доказательства, см. High-1) в основном однозначны и не содержат
скрытых домыслов, кроме AC о расстоянии (Medium-1).
- «Регрессионные проверки» верно называют, что нужно защитить: контракт
#647 (стабильная ширина) и #195 (hit-target крестика) — упомянуты явно
и не ослабляются на словах.
## Чего не проверял
- Гейты (`tsc`, `npm test`, `npm run build`, `check-docs.mjs`, смоки,
golden, инварианты) — не прогонял: этап `spec`, продуктового кода к этой
правке ещё нет, диапазон ревью — текст тела issue, а не диапазон
коммитов. Это предмет код-ревью после реализации.
- Не измерял в браузере фактическое расстояние `.modes` → `.zoomctl` и не
проверял независимо числа 100/81 px из «Аналитики» — принял их как
добросовестное измерение автора; при следующем заходе, если числа войдут
в исправленный ТЗ (Medium-1), стоит перепроверить смоком на заявленных
ширинах.
- Не оценивал производительность/анимацию перехода между режимами — ТЗ не
меняет этот механизм, вне скоупа задачи.
- Не проверял touch-контракт по `docs/TOUCH-SUPPORT.md` подробно за
пределами констатации, что редакторы остаются desktop-first/best-effort;
вопрос, нужно ли обновлять `docs/TOUCH-SUPPORT.md:74-79` (описание
сводной панели как «View surfaces»), не поднят ни ТЗ, ни этим ревью и
должен быть закрыт вместе с переписыванием раздела Release-артефакты.
## Вердикт
Найдено 2 High и 1 Medium, все в скоупе задачи. Раздел `## ТЗ` в теле
issue отсутствует целиком: то, что дошло до ревью, — исходная постановка
владельца плюс два продуктовых ответа, без единой из обязательных частей
§7.1 (Сценарий, Не-скоуп, модель данных/i18n, способ доказательства по
каждому AC, План автотестов, Риски, Откат, Release-артефакты, «Принято
предположительно»). Отдельно от этого системного пробела — конкретное
противоречие с каноническим `docs/UX-MODES.md` (крестик описан как
«fixed slot… outside the editors», сводная панель — как «disappears in
Plan, Devices and Background»), которое #660 меняет, но не планирует
исправить в документации, и отсутствие допуска/целевых чисел в AC о
расстоянии до блока зума. Красный вердикт: ТЗ возвращается автору на
полную первую редакцию раздела `## ТЗ` по образцу #647, отдельные issue
не заводятся.
Вердикт: красный · заход r1 · блокирующих циклов 1/4 · High: 2 · Medium: 1 → в задаче
---
<!-- material-anchors: сгенерировано конвейером (#414) -->
## Материал раунда
- Ветка: `dev`, коммит `9287f798b382` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
- Дерево материала: `268f8f32087b5c4866a4a6a695ae66bcfce7bc71`
```
git log --all --format='%H %T' | grep 268f8f32087b
```
- Тело issue: `70fd6272cab8ef94f29ce0fb31389d319d247b8c435d9eed3e4200c5e3d23cb0`
- Вердикт конвейера: `red` · High 2