mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-02 21:01:21 +00:00
@@ -0,0 +1,134 @@
|
||||
# SPEC-REVIEW-264-r2 — контроллер Resize и commit-инвариант кладки
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/264
|
||||
- **Этап:** ТЗ (S4-spec-review), заход r2, блокирующих циклов израсходовано 1 из 4
|
||||
(лимит полного трека, зелёный вердикт цикла не тратит — §227)
|
||||
- **Ревьюер:** независимая сессия, без контекста автора правки
|
||||
- **Материал:** `docs/specs/264-resize-controller.md` на SHA `536b750d`
|
||||
(HEAD); правка вносится коммитом `536b750d` «docs: address resize
|
||||
controller spec review» поверх r1
|
||||
- **Вердикт: зелёный · заход r2 · блокирующих циклов 1/4 · High: 0 · Medium: 0**
|
||||
|
||||
## Скоуп проверки — по дельте (PROCESS.md §2.10)
|
||||
|
||||
Раунд не первый, поэтому разбор ограничен дельтой и тем, до чего она
|
||||
дотягивается, а не задачей целиком.
|
||||
|
||||
1. **Вердикт и SHA предыдущего раунда.** Найден в `docs/reviews/SPEC-REVIEW-264-r1.md`
|
||||
(коммит `c46c714c`): «Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 ·
|
||||
High: 0 · Medium: 2 → в задаче». В отличие от типового риска этого пункта,
|
||||
SHA материала r1 **назван прямо в самом документе** — «на SHA `6e1aea3a`» —
|
||||
так что находки «SHA не назван» в этом раунде нет.
|
||||
2. **Дельта.** `git diff 6e1aea3a..536b750d -- docs/specs/264-resize-controller.md`
|
||||
— 25 добавленных / 12 удалённых строк в одном файле, всё внутри
|
||||
`docs/specs/264-resize-controller.md` (§2 п.7, §4 п.1/5, §6 таблица
|
||||
переходов, §9, §10, §12 edge cases, §15 список файлов, §19 риски).
|
||||
Продуктовый код не менялся (diff `origin/dev...HEAD` содержит только
|
||||
этот один файл спецификации — этап ТЗ, не код-ревью).
|
||||
3. **Локальность дельты (проверка условия «разбор остаётся полным»).**
|
||||
Рёбейза не было: `git merge-base HEAD origin/dev` = `78850a87` =
|
||||
текущий tip `origin/dev`, ветка не отставала. Контракт поведения не
|
||||
меняется — правка как раз восстанавливает точное соответствие описанного
|
||||
и фактического поведения. Новая подсистема не затронута — оба Medium из
|
||||
r1 лежали внутри уже разбираемого в r1 состояния Resize/Escape и списка
|
||||
тестовых файлов. Объём дельты (37 строк) многократно меньше исходного ТЗ
|
||||
(442 строки). Все четыре условия §2.10 для полного разбора отсутствуют —
|
||||
разбор по дельте оправдан.
|
||||
4. Дополнительно прочитан фрагмент `docs/RESIZE.md` (канон Resize) на
|
||||
упоминание Escape/latent selection — единственное совпадение (строка 218)
|
||||
касается Escape во время активного drag, идло-ветки с `_rszSel` канон не
|
||||
описывает; значит нюанс действительно техническая территория автора, а не
|
||||
пропущенный продуктовый контракт.
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка r1 | Чем закрыта | Где это видно |
|
||||
|---|---|---|
|
||||
| **M1** (Medium, в скоупе) — §2 п.7 и §19 утверждали, что «Escape в idle Resize сразу возвращает Draw», хотя код (`houseplan-card.ts:2701-2710`) делает это только вторым Escape при выбранной кликом комнате (`_rszSel`) | §2 п.7 переписан: `_rszSel` не удаляется, а переезжает в controller как `selectedRoomId`; первый Escape очищает selection и остаётся в Resize, второй — выходит в Draw. §4 п.1/5 добавляют состояние `selectedRoomId` и переходы `select/clearSelection`. §6 (таблица переходов) получает три новые строки: `idle/selectRoom`, `idle/escape` (selection есть → очистить, остаться в Resize), `idle/escape` (selection нет → `exit-tool`, root включает Draw) — это **дословно** воспроизводит найденный в коде код-путь. §9 фиксирует, что `WarmViewport.rszSel` продолжает нести это session-state значение через controller API. §12 добавляет edge case №17 «room-area click в idle Resize + Escape + Escape сохраняет … последовательность». §19 риск переформулирован с «it has no render consumer and idle Escape ends at Draw» (ложное) на «Controller сохраняет latent selection, warm getter/restore и двухшаговый Escape; dedicated unit/browser assertion» | `docs/specs/264-resize-controller.md` §2 п.7 (строки 71-77), §4 п.1/5 (104-112), §6 (194-196), §9 (278-280), §12 п.17 (332-333), §19 (453). Доказательство в AC не потеряно: AC2 «Controller реализует таблицу §6: … дают ровно один детерминированный outcome» — таблица §6 теперь включает новые строки, значит их обязан покрыть unit-тест того же AC; §17 «План тестирования» отдельно требует «for each row of the automat §6 and edge cases §12» |
|
||||
| **M2** (Medium, в скоупе) — §15 называл среди существующих тестов несуществующий `demo/smoke_safe_resize.mjs` | §15 заменяет несуществующее имя на `demo/smoke_room_resize.mjs` и добавляет `demo/smoke_resize_pointer_real_plan.mjs` рядом с уже верными `demo/smoke_resize_wall_thickness.mjs`/`demo/smoke_resize_inner_dimensions.mjs` — ровно рекомендация r1 | `docs/specs/264-resize-controller.md` §15 (строки 374-378). Существование всех четырёх файлов перепроверено заново командой `ls` в этом раунде: `demo/smoke_room_resize.mjs`, `demo/smoke_resize_pointer_real_plan.mjs`, `demo/smoke_resize_wall_thickness.mjs`, `demo/smoke_resize_inner_dimensions.mjs` — все существуют |
|
||||
|
||||
Обе находки r1 были Medium в скоупе задачи, без High — по §2.4/§4 PROCESS.md
|
||||
это корректно закрывается в том же issue повторным циклом ревью ТЗ, что и
|
||||
происходит.
|
||||
|
||||
## Проверка дельты по существу (не за слово автора)
|
||||
|
||||
- **Внутренняя согласованность.** `grep -n "_rszSel\|selectedRoomId\|rszSel"`
|
||||
по всему файлу после правки показывает 9 упоминаний, и все они говорят одно
|
||||
и то же: поле не удаляется, а переезжает в controller под именем
|
||||
`selectedRoomId`, а `WarmViewport.rszSel` остаётся его module/session-хранилищем.
|
||||
Нет ни одного оставшегося места, которое всё ещё утверждало бы «поле
|
||||
удаляется как мёртвое» или «Escape всегда сразу Draw» — прежняя
|
||||
формулировка полностью заменена, а не задублирована.
|
||||
- **§1 («жесты не меняются»)** — единственное место, которое могло бы
|
||||
конфликтовать с исправленным §2 п.7, если бы правка не была полной.
|
||||
Прочитан отдельно: формулировка не изменилась и теперь **соответствует**
|
||||
факту, потому что фикс сохраняет наблюдаемую двухшаговую Escape-последовательность
|
||||
«как есть», а не убирает её.
|
||||
- **AC1** (§16) утверждает «в root нет mutable … `_rszSel`; состояние и
|
||||
cache принадлежат одному `ResizeController`» — это не противоречит фиксу:
|
||||
AC1 про **root**, а поле по фиксу уезжает именно из root в controller, само
|
||||
утверждение AC1 не меняло смысла и не требовало правки.
|
||||
- **Список файлов §15 (M2)** сверен исполнением `ls` в этом раунде (не
|
||||
унаследовано с доверием к r1, так как это прямой предмет правки):
|
||||
все четыре смока на диске.
|
||||
- **Свежесть risk-таблицы §19**: строка про `_rszSel` переформулирована
|
||||
так, что явно требует «dedicated unit/browser assertion» — то есть риск не
|
||||
просто закрыт текстом, а оставляет след для код-ревью (какой тест обязан
|
||||
существовать).
|
||||
|
||||
Открытых продуктовых вопросов правка не создаёт и не решает — обе находки
|
||||
были техническими (автор решает сам, §7.1 PROCESS.md), в соответствии с
|
||||
вердиктом r1.
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Без повторной проверки в этом раунде принято из
|
||||
`docs/reviews/SPEC-REVIEW-264-r1.md` (материал на SHA `6e1aea3a`), поскольку
|
||||
дельта их не задевает:
|
||||
|
||||
- структура ТЗ покрывает все обязательные разделы §7.1 PROCESS.md (сценарий,
|
||||
персона, что человек увидит, проблема, скоуп/не-скоуп, контракт поведения,
|
||||
UX/touch, данные/миграция, i18n, AC с доказательством, план автотестов,
|
||||
риски, откат, release-артефакты);
|
||||
- продуктовый вопрос из тела issue закрыт ссылкой на выпущенный #277
|
||||
(fail-closed, ноль записи при нарушении инварианта);
|
||||
- фактические ссылки на существующий код (`_rsz*`-поля, `_rszApplyPreview`,
|
||||
`checkWallRecordsPreserved` в `scripts/model-invariants.mjs:301`, паттерн
|
||||
`test-build`/`tsconfig.test.json` в `package.json`, геометрические функции
|
||||
`resize.ts`/`wall-thickness.ts`/`open-spans.ts`, `docs/TESTING.md` /
|
||||
`docs/ARCHITECTURE.md` / `docs/STATUS.md`, `WarmViewport.rszSel` как
|
||||
module-state) — верны, кроме уже закрытого M2;
|
||||
- performance-бюджеты §13 совпадают с канонизированными в `docs/RESIZE.md`;
|
||||
- класс изменения A+B+C и `User-Visible: no` — корректны для содержания
|
||||
после фикса M1 (документ больше не противоречит себе по этому пункту);
|
||||
- соответствие `docs/SCOPE.md` (J6), честная низкая продуктовая ценность
|
||||
(2/10) при высокой инженерной (9/10), решение владельца по порядку
|
||||
относительно #34 (взято в работу 2026-08-27);
|
||||
- гейты `typecheck`/`test`/`build`/`check-docs`/`invariants` **не
|
||||
запускались и в этом раунде** — на этапе ТЗ нет продуктового кода, diff
|
||||
затрагивает только markdown; они станут обязательны на код-ревью
|
||||
(S7-code-review).
|
||||
|
||||
## Чего не проверял в этом раунде
|
||||
|
||||
- Генерики/типизацию `ResizeControllerOutcome<...>` — не предмет дельты,
|
||||
§5 сама называет это уточняемым при реализации.
|
||||
- Полную построчную сверку всех 19 пунктов матрицы edge cases (§12) с кодом
|
||||
— вне дельты сверялись выборочно в r1 (пп. 5, 6, 12, 14); новый п.17
|
||||
сверен по коду отдельно в этом раунде (см. таблицу закрытия r1).
|
||||
- Browser/unit-тесты — их ещё нет, поведение не реализовано; это предмет
|
||||
код-ревью, не ТЗ.
|
||||
- Полный повторный обход `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md`, тела
|
||||
issue #264 и всех его комментариев — они не изменились со времени r1 и не
|
||||
затронуты дельтой; их разбор унаследован из r1 без повторного чтения по
|
||||
существу (кроме точечной сверки `docs/RESIZE.md` на Escape/`_rszSel`,
|
||||
сделанной в этом раунде).
|
||||
|
||||
## Итог
|
||||
|
||||
Оба Medium из r1 закрыты в коде документа, а не только заявлением автора:
|
||||
проверено построчным сравнением дельты с найденным в r1 кодом (`_rszSel`/Escape)
|
||||
и исполнением `ls` для списка смоков. Новых находок дельта не порождает.
|
||||
High-находок нет и не было. Вердикт зелёный, документ переводит issue в
|
||||
`S5-ready`.
|
||||
Reference in New Issue
Block a user