mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,164 @@
|
||||
# SPEC-REVIEW-294-r1
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/294
|
||||
- **Этап:** ревью ТЗ (PROCESS.md §2.4)
|
||||
- **Заход:** r1 · блокирующих циклов израсходовано 0/4 → 1/4 после этого вердикта
|
||||
- **Ревьюер:** Claude (роль «ревьюер ТЗ», сессия отдельна от автора)
|
||||
- **Предмет ревью:** `docs/specs/294-wall-esc-detach.md` на SHA `8e86b0a5`
|
||||
(ветка `issue/294-wall-esc-detach`), тело issue #294 и оба комментария
|
||||
аналитики/хендоффа автора.
|
||||
- **Трек:** обычный (не `small`) — верно: метка `small` на issue отсутствует,
|
||||
спецификация оформлена файлом, как и требует §5.
|
||||
|
||||
## Скоуп
|
||||
|
||||
Первый заход, дельты нет — разбор полный по §2.4/§7.1: продуктовая рамка,
|
||||
однозначность контракта Esc для активной цепочки стен, полнота AC и их
|
||||
доказуемость, отсутствие выданных за факт догадок, соответствие
|
||||
`docs/USER-GUIDE*.md` и терминологии SCOPE/UX-MODES.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (§1–§9, §7.1 отдельно).
|
||||
2. Прочитаны тело issue #294 и все три комментария (аналитика, «Взял»,
|
||||
хендофф автора).
|
||||
3. Прочитан `docs/specs/294-wall-esc-detach.md` целиком на SHA `8e86b0a5`.
|
||||
4. Утверждения «Подтверждение по коду» из аналитики и §1/§3 ТЗ о текущем
|
||||
поведении сверены с реальным исходником на той же ветке
|
||||
(`src/houseplan-card.ts`, обработчик `_onKey`, `_finishWallChain`,
|
||||
`_undoPoint`, `_undoActiveDraftPoint`, `_resumeLastDraft`,
|
||||
`_resumeDraftBySpace`, `_activateMarkupTool`) — все технические утверждения
|
||||
спецификации подтверждаются кодом, ни одно не является непомеченной
|
||||
догадкой.
|
||||
5. Сверены `docs/USER-GUIDE.ru.md` и `docs/USER-GUIDE.md` на предмет текущей
|
||||
формулировки контракта Esc/Ctrl+Z для инструмента «Стены»/Draw, чтобы
|
||||
проверить полноту AC7 и раздела 5 ТЗ.
|
||||
6. `docs/TOUCH-SUPPORT.md` и `docs/UX-MODES.md` проверены на отсутствие
|
||||
противоречащих утверждений о touch/лестнице Escape — противоречий нет.
|
||||
7. Гейты (`typecheck`/`test`/`build`/`check-docs`) не запускались: изменения
|
||||
этого раунда — только `docs/specs/**` (класс C), продуктовый код не
|
||||
существует до `S5-ready`; сами по себе они ничего не проверяют для ТЗ.
|
||||
Это сознательное решение объёма, а не пропуск.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе задачи, чинится в текущем ТЗ) — AC7 уже формулировкой не покрывает весь текст, который спецификация обязана исправить
|
||||
|
||||
**Файл:** `docs/specs/294-wall-esc-detach.md`, раздел 5 и AC7.
|
||||
|
||||
**Суть.** Раздел 5 обещает: «`docs/USER-GUIDE.md` и `docs/USER-GUIDE.ru.md`
|
||||
должны явно описать тот же контракт в разделах keyboard/cancel и
|
||||
Rooms/Walls». Но проверяемый критерий AC7 сужает обязательство:
|
||||
|
||||
> Старое утверждение «Esc/Ctrl+Z — убрать точку» отсутствует в затронутых
|
||||
> Walls-разделах; i18n parity зелёный.
|
||||
|
||||
AC7 — это единственное, что реально проверяется («ревью кода» +
|
||||
`unit`/i18n-parity), а не раздел 5. А формулировка старого утверждения,
|
||||
которое AC7 обязуется убрать, привязана буквальной цитатой и словом
|
||||
«Walls-разделах» — то есть не покрывает как минимум четыре конкретных места,
|
||||
где тот же неверный контракт «Esc для Draw убирает/отменяет операцию»
|
||||
сформулирован другими словами вне «Walls»-раздела:
|
||||
|
||||
- `docs/USER-GUIDE.ru.md:223` (таблица поверхностей ввода, строка
|
||||
«Рисование и точный drag»): `` `Esc` отменяет операцию ``;
|
||||
- `docs/USER-GUIDE.ru.md:241` (раздел «Клавиши отмены», строка
|
||||
«Незавершённый Draw/Split»): «Убирает последнюю точку или выходит из
|
||||
инструмента»;
|
||||
- `docs/USER-GUIDE.md:207` (таблица поверхностей ввода, строка «Draw or
|
||||
precise drag»): `` `Esc` cancels the operation ``;
|
||||
- `docs/USER-GUIDE.md:216-217` (раздел «Cancel and undo»): «`Esc` cancels an
|
||||
unfinished path, current drag/resize/rotation, or the top dialog without
|
||||
undoing an already committed action.»
|
||||
|
||||
Ни одна из этих четырёх строк не содержит буквальной фразы «Esc/Ctrl+Z —
|
||||
убрать точку» и не лежит в «Walls»-разделе — значит, автоматическая или
|
||||
ручная проверка AC7 «по букве» пройдёт, даже если все четыре останутся
|
||||
нетронутыми. После реализации это даст прямое текстовое противоречие внутри
|
||||
одной документации: раздел про стены будет говорить «Esc завершает цепочку,
|
||||
геометрия сохраняется», а общая таблица «Клавиши отмены»/«Cancel and undo» —
|
||||
по-прежнему «Esc убирает точку/отменяет операцию» для того же инструмента
|
||||
Draw. Это ровно тот класс дефекта, который PROCESS.md §8 требует ловить
|
||||
(«одно число — один источник», расширительно — одно поведение не должно быть
|
||||
описано двумя разными способами в одном документе).
|
||||
|
||||
**Сценарий проявления:** пользователь читает раздел 8 «Rooms and walls» —
|
||||
видит новый корректный контракт; тот же пользователь листает раздел 6
|
||||
«Cancel and undo» или сводную таблицу горячих клавиш — видит старое, теперь
|
||||
неверное утверждение «Esc отменяет операцию/убирает точку». Документация
|
||||
one-source-of-truth не выполняется, при этом AC7 в исполнении «по букве»
|
||||
формально зелёный.
|
||||
|
||||
**Почему это Medium, а не High:** для активной задачи №294 продуктовый
|
||||
контракт (§3 ТЗ) сформулирован полно и однозначно, регресс поведения не
|
||||
грозит — страдает только точность документации, causing user confusion, но
|
||||
не потерю данных или неработающую функцию. Находка в скоупе текущей задачи
|
||||
(тот же файл, тот же тип правки, что и остальные пункты раздела 5) — чинится
|
||||
здесь же, отдельный issue не заводится (§202).
|
||||
|
||||
**Что нужно поправить в ТЗ:** переформулировать AC7 (и, по необходимости,
|
||||
раздел 5) так, чтобы явно перечислить все места, где сегодня утверждается
|
||||
«Esc для Draw убирает точку/отменяет операцию» — включая обе таблицы
|
||||
поверхностей ввода (RU/EN) и разделы «Клавиши отмены»/«Cancel and undo», а не
|
||||
только цитату и «Walls-разделы». Также стоит явно решить для EN «Create a
|
||||
room» (`docs/USER-GUIDE.md:294-299`): в отличие от RU-версии (строка 412), в
|
||||
этом месте английского руководства сегодня вообще нет утверждения про
|
||||
Esc/Ctrl+Z — паритет с RU-версией по этому контракту не назван требованием.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Персона/поверхность/момент действия читаются однозначно из текста, хотя и
|
||||
без отдельного заголовка «Сценарий»: Home admin, Plan editor, инструмент
|
||||
«Стены», активная незамкнутая цепочка. Согласуется с `docs/SCOPE.md`
|
||||
(редакторы — только для Home admin, десктоп).
|
||||
- Никаких догадок, выданных за факт: каждое утверждение о текущем поведении
|
||||
(«`Esc` вызывает `_undoPoint()`», «смена инструмента идёт через
|
||||
`_finishWallChain()`», «переключение Стены→Колонна→Стены уже даёт нужный
|
||||
результат») подтверждено чтением `src/houseplan-card.ts` на той же ветке —
|
||||
см. пункт 4 «Как проверялось».
|
||||
- Контракт Esc (§3.1–3.4 ТЗ) внутренне непротиворечив и совпадает с формой
|
||||
существующего кода: тот же `history.wall_chain_finish`, тот же путь
|
||||
materialization, тот же приоритет диалогов (`_roomDialog` проверяется в
|
||||
обработчике раньше ветки `tool === 'draw'` и уже завершается `return`, то
|
||||
есть требование «этот же keydown не должен дополнительно завершить
|
||||
восстановленный draft» уже выполняется существующей структурой кода, а не
|
||||
требует новой архитектуры).
|
||||
- Различение Esc (finish) и Ctrl/Cmd+Z (undo point) — с AC4 отдельно и явно
|
||||
проверяемо через реальный `keydown`, что закрывает риск «Esc и Undo слились
|
||||
в одну кнопку», названный в разделе 9 ТЗ.
|
||||
- AC1/AC2 намеренно используют различимые толщины `AB`/`BC`, чтобы merge
|
||||
коллинеарных сегментов одной толщины (`_mergeSpacePartitions`) не смазал
|
||||
проверку количества сегментов — разумная защита от ложноположительного
|
||||
прохождения теста.
|
||||
- Не-скоуп (раздел 6) чётко исключает face-detection, touch-команду finish,
|
||||
формат хранения, Escape других инструментов — расширения скоупа не
|
||||
просматривается.
|
||||
- Модель данных/миграция/i18n/touch/perf/security (разделы 8, 13) — сделанные
|
||||
предположения обоснованы и помечены как таковые в разделе 13, спор по ним
|
||||
решается вердиктом ревьюера, а не владельцем; ни один пункт не требует
|
||||
продуктового вопроса владельцу (нет открытых вопросов — подтверждаю).
|
||||
- Trailers коммита спецификации корректны: `Issue: #294`,
|
||||
`User-Visible: no` — спецификация не меняет поведение, только документ;
|
||||
класс C, отдельный issue не требуется (входит в DoD текущего).
|
||||
- Откат (раздел 10) достаточен: чистый revert, миграция назад не нужна из-за
|
||||
отсутствия изменений схемы.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Гейты `typecheck`/`test`/`build`/`check-docs`/смоки — не запускались:
|
||||
раунд правит только `docs/specs/**`, продуктового кода ещё нет
|
||||
(issue не в `S5-ready`), эти гейты не применимы к этапу ТЗ.
|
||||
- Реализуемость AC на уровне конкретного будущего диффа кода — предмет
|
||||
код-ревью, не ТЗ-ревью; здесь проверялась только проверяемость и
|
||||
однозначность формулировок.
|
||||
- Issue #173 не читалась целиком построчно — использована как контекст по
|
||||
цитате в теле #294 и по факту существования кода единого инструмента
|
||||
«Стены», который уже найден и прочитан напрямую.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Жёлтый: единственная находка — Medium в скоупе задачи (пробел в
|
||||
формулировке AC7/раздела 5, из-за которого исправление может формально
|
||||
пройти проверку и оставить документацию внутренне противоречивой). High нет.
|
||||
Автор правит формулировку AC7 (и, если нужно, раздел 5) в этом же ТЗ, фикс
|
||||
проходит повторный цикл ревью по дельте (§2.10).
|
||||
Reference in New Issue
Block a user