From ef32d2326fe92fd902e93b98938e84e667294023 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Thu, 27 Aug 2026 16:58:08 +0000 Subject: [PATCH] docs: review document for #264 Issue: #264 User-Visible: no --- docs/reviews/SPEC-REVIEW-264-r2.md | 134 +++++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-264-r2.md diff --git a/docs/reviews/SPEC-REVIEW-264-r2.md b/docs/reviews/SPEC-REVIEW-264-r2.md new file mode 100644 index 00000000..1e9acdd1 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-264-r2.md @@ -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`.