18 KiB
SPEC-REVIEW-264-r1 — контроллер Resize и commit-инвариант кладки
- Issue: https://github.com/Matysh/houseplan-card/issues/264
- Этап: ТЗ (S4-spec-review), заход r1, лимит циклов 4 (полный трек)
- Ревьюер: независимая сессия, без контекста автора ТЗ
- Материал:
docs/specs/264-resize-controller.mdна SHA6e1aea3a(коммитыa3e32995«specify resize controller extraction»,6e1aea3a«add resize controller user scenario»;git diff origin/dev...HEADзатрагивает только этот один файл, продуктовый код не менялся) - Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 2 → в задаче
Скоуп проверки
Полный разбор (первый заход, дельты нет). Прочитаны:
docs/SCOPE.md, AGENTS.md, PROCESS.md, тело issue #264 и все 4 комментария
(включая обе аналитики владельца от 2026-08-24 и 2026-08-27), docs/RESIZE.md
(канон Resize), docs/specs/034-frontend-decomposition.md (зонтичный слайс),
тело issue #277 (безопасный fixed-topology контракт, prerequisite).
Как проверялось
Поскольку это этап ТЗ, а не код-ревью, тяжёлые гейты (typecheck/test/build)
не запускались — git diff origin/dev...HEAD показывает изменение только
docs/specs/264-resize-controller.md, продуктовый код не тронут. Вместо
прогона гейтов сверял каждое фактическое утверждение ТЗ о текущем состоянии
кода с реальным src/houseplan-card.ts, src/resize.ts, src/wall-thickness.ts,
src/open-spans.ts, scripts/model-invariants.mjs, package.json и demo/,
командами grep/sed — то, что задача прямо требует называть предположением
или доказывать, а не декларировать.
Находки
M1 (Medium, в скоупе) — утверждение «Escape в idle Resize уже сейчас сразу возвращает Draw» не соответствует коду
Файл: docs/specs/264-resize-controller.md, §2 п.7 и §19 (строка риска
про _rszSel).
Утверждение ТЗ:
Мёртвое session-only поле
_rszSelудаляется: после #277 оно не имеет renderer-потребителя и не влияет на пиксели, config или Undo. Escape в idle Resize, как и фактически сейчас, сразу возвращает neutral Draw.
и в таблице рисков (§19):
Removal of
_rszSelchanges warm/Escape behavior | Source/runtime parity test; it has no render consumer and idle Escape ends at Draw.
Воспроизведение по коду. src/houseplan-card.ts:2701-2710
(обработчик Escape, тул resize, вне драга):
if (this._tool === 'resize') {
e.preventDefault();
if (this._rszDrag) {
this._rszCancelDrag();
return;
}
if (this._rszSel) this._rszSel = null;
else this._tool = 'draw';
return;
}
Это ветвится не всегда одинаково: если пользователь щёлкнул по площади
комнаты в инструменте Resize (обработчик клика на src/houseplan-card.ts:7802-7804
ставит this._rszSel = room?.id || null), первый Escape только сбрасывает
_rszSel и остаётся в Resize; тул на Draw переключает только второй
Escape. То есть сегодня «idle Escape» не всегда сразу возвращает Draw —
это зависит от того, была ли до этого выбрана комната кликом.
_rszSel действительно не имеет визуального потребителя (проверено:
demo/smoke_hide_layers.mjs:159 и demo/smoke_pan_any_zoom.mjs:134
намеренно выставляют _rszSel, доказывая лишь отсутствие старой corner-frame
разметки #277 — это подтверждает часть про «не влияет на пиксели», но не
часть про Escape). Значит убрать это поле «как есть» без изменения самого
if (this._rszDrag) {...} if (this._rszSel) ... else this._tool = 'draw';
означает буквально удалить условную ветку — и тогда Escape при выбранной
(невидимо) комнате в Resize станет сразу переключать на Draw, чего
сегодня не происходит.
Почему это находка, а не мелочь. §1 ТЗ прямо обещает персоне, что слайс
«обязан быть полностью незаметен», и в перечне неизменного в самом начале
документа: «DOM, пиксели, тексты, жесты и persisted config не меняются».
Escape-последовательность — это жест. Документ одновременно (а) заявляет
парность с текущим поведением как факт, (б) сам же перечисляет удаление
_rszSel как часть скоупа (§10) и своей же мерой риска называет то самое
неверное утверждение о «текущем» поведении. Это ровно тот класс дефекта,
о котором предупреждает процесс: догадка о поведении, записанная как факт,
проходит ревью, потому что выглядит решением.
Требуемое исправление одно из двух (решает автор, не владелец — это техническая деталь конечного автомата контроллера, не то, что видит пользователь по существу задачи):
- явно зафиксировать, что рефакторинг меняет это конкретное поведение (Escape после выбора комнаты в Resize отныне сразу выходит в Draw), и убрать формулировку «как фактически сейчас» — задокументировать как осознанное упрощение; либо
- сохранить двухшаговый Escape, перенеся понятие «комната выбрана»
в состояние контроллера (например, отдельный
hasSelection/pickedв typed outcome конечного автомата §6), а не как «мёртвое» поле для простого удаления.
Ни один из вариантов не требует нового вопроса владельцу — это находка внутри уже принятой владельцем границы «что делает автор» (§7.1 PROCESS.md).
M2 (Medium, в скоупе) — несуществующий файл смока в планируемом diff
Файл: docs/specs/264-resize-controller.md, §15 «Файлы и модули».
ТЗ перечисляет как существующий поведенческий тест:
существующие
demo/smoke_safe_resize.mjs,demo/smoke_resize_wall_thickness.mjs,demo/smoke_resize_inner_dimensions.mjs— behavioral parity;
demo/smoke_safe_resize.mjs не существует:
$ ls demo/smoke_safe_resize.mjs
ls: cannot access 'demo/smoke_safe_resize.mjs': No such file or directory
Канонический список Resize-смоков — раздел «Verification» в docs/RESIZE.md —
называет demo/smoke_room_resize.mjs (production bundle pointer handlers,
preview/commit/Undo, disabled a11y, production-preflight failure и
cancellation — именно то, что §15 хочет накрыть словом «behavioral parity»)
и demo/smoke_resize_pointer_real_plan.mjs. Оба реально существуют в demo/.
Тело исходного issue #264 в разделе «Как проверять» тоже не называет
smoke_safe_resize, только smoke_resize_wall_thickness и
smoke_resize_inner_dimensions — значит ошибка внесена автором ТЗ, не
унаследована из issue.
Почему это находка. Список файлов в DoR — то, по чему исполнитель и
ревьюер кода сверяют полноту diff'а (§2.5, §7.1 PROCESS.md). Несуществующее
имя тут — либо повод по ошибке создать новый файл-дубликат вместо запуска
существующего smoke_room_resize.mjs, либо просто нерабочая команда при
локальном прогоне. Дешёво чинится заменой имени на demo/smoke_room_resize.mjs
(и, по желанию автора, добавлением demo/smoke_resize_pointer_real_plan.mjs
рядом, раз он тоже покрывает production pointer path).
Что проверено и корректно
- Структура ТЗ покрывает все обязательные разделы §7.1 PROCESS.md: сценарий и персона (§1, добавлен вторым коммитом — реагирует именно на то, что требует шаблон), что человек увидит до/после (§1), проблема (§1), скоуп/не-скоуп (§10/§11), контракт поведения (§4-§9 плюс явная ссылка на канон #277), UX/touch (§14), данные и миграция (§18 — нет), i18n (§18 — нет), AC1…AC12 с методом доказательства (§16), план автотестов (§17), риски (§19), откат (§20), release-артефакты (§18).
- Продуктовый вопрос из тела issue закрыт корректно. Исходный открытый вопрос «что делать при нарушении инварианта» (отменить жест / записать диагностику / показать пользователю) снят ссылкой на уже выпущенный #277 (fail-closed, ноль записи) — это подтверждено и в комментарии владельца от 2026-08-24/27, и напрямую в §2 ТЗ. Новых открытых продуктовых вопросов документ не содержит и не должен: остальные решения (имена типов, файлы, разбиение controller/root) — техническая территория автора по §7.1.
- Все ссылки на существующий код в архитектурном разделе (кроме M2)
фактически верны, что важно для refactor-only задачи с высоким риском
«догадки, выданной за решение»:
- все перечисленные
_rsz*mutable-поля (_rszDrag,_rszPreview,_rszLive,_rszEligibilityCache,_rszSel) реально существуют вsrc/houseplan-card.ts(119 упоминаний_rszв файле); - «inline сравнение внутри Resize» из §0 реально существует —
src/houseplan-card.ts(_rszApplyPreview) сравниваетJSON.stringify(beforeWallCms) !== JSON.stringify(afterWallCms)отсортированных мультимножествcm, что уже сегодня фактически является exact-multiplicity проверкой (включаяcm:0), которую §8 предлагает формально вынести в общий модуль — согласуется; checkWallRecordsPreserved(before, after, { allowClear = false })вscripts/model-invariants.mjs:301действительно фильтруетcm > 0(presence-семантика #254), т.е. новый режимexactMultiplicityиз §8 — это расширение, а не переписывание текущего поведения;- паттерн «CLI импортирует production-модуль из
test-build/*, собранногоtsc -p tsconfig.test.json» уже используется дляplan-geometry-preflight/near-axis/coordinate-canonicalization(scripts/model-invariants.mjs:21-26), иnpm run invariantsвpackage.jsonдействительно гоняетtsc -p tsconfig.test.jsonперед CLI — план §8 по добавлению туда жеwall-record-preservationтехнически исполним без нового механизма; - все геометрические функции, на которые ссылается §4.3/§6
(
resolveSafeResize,clampSafeResize,applySafeResize,validateSafeResize,safeResizePointerDisplacementвsrc/resize.ts;rekeyWallsAfterMoveCheckedвsrc/wall-thickness.ts;rekeyOpenSpansAfterMoveвsrc/open-spans.ts) существуют под теми же именами; docs/TESTING.md,docs/ARCHITECTURE.md,docs/STATUS.md,tsconfig.test.json— все существуют, ссылки на них в §15/§18 корректны;WarmViewport.rszSel(src/houseplan-card.ts:430) — это поле неперсистентного модульного кэша_warmVp(«the memo is module state, never serialised»), не часть сохранённого конфига — утверждение §9 о том, что удалениеrszSelизWarmViewportне меняет сериализуемый config, подтверждено чтением кода.
- все перечисленные
- Performance-бюджеты §13 (pointermove p95 ≤16 мс/≤20% над baseline,
pointerup p95 ≤75 мс) дословно совпадают с уже канонизированными в
docs/RESIZE.md, а не изобретены заново. - Класс изменения и трейлеры. Class A+B+C, полный флоу — верно для
задачи, трогающей
src/**(Class A) даже как рефакторинг;User-Visible: noкорректен для содержания ТЗ после исправления M1 (сейчас документ сам себе противоречит по этому пункту, см. находку). - Соответствие SCOPE.md. Задача не создаёт нового пользовательского поведения, служит устойчивости J6 («Keep the plan true as the home evolves» — drag/resize) снижением риска регрессий в самом багоносном инструменте; входит в уже согласованный владельцем зонтичный слайс #34. Продуктовая ценность честно указана низкой (2/10), инженерная — обоснованно высокой (9/10); это не попытка протащить фичу под видом рефакторинга.
- Порядок slice vs #34. Расхождение «сначала диалог или сначала Resize», которое исходный текст issue называл открытым, закрыто прямым решением владельца («Взято в работу по решению владельца 2026-08-27») — ТЗ корректно это не пересматривает.
Чего не проверял
- Гейты
typecheck/test/build/check-docs/invariantsне запускались — на этом этапе нет продуктового кода, диапазонgit diff origin/dev...HEADсодержит только markdown-файл спецификации. Это решение по объёму, а не пропуск: гейты станут обязательны на код-ревью (§7 PROCESS.md, S7-code-review). - Смоки
demo/smoke_*resize*.mjsне запускал — на этом этапе нечего прогонять, поведение ещё не реализовано; факт существования файлов (кроме M2) подтверждён толькоls, не исполнением. - Не проверял детально типизацию generics
ResizeControllerOutcome<TSpace, TSnapshot, TWallUnion>— §5 сама оговаривает «конкретные имена типов могут уточняться при реализации», это явно техническая, не продуктовая граница, оставляю автору. - Не сверял построчно все 18 пунктов матрицы edge cases (§12) с кодом —
выборочно (пп. 5, 6, 12, 14) сверил с текущей реализацией
_rszMove/_rszUpи они совпадают; полную построчную сверку всех 18 не делал, так как это либо прямое наследование уже выпущенного #277 (не предмет этого ТЗ), либо будет доказано тестами на код-ревью.
Итог
Two Medium findings, both in scope, ни один не является продуктовым вопросом для владельца — оба технические и решаются автором в этом же issue. High-находок нет, вердикт жёлтый, документ засчитывает заход r1 и тратит первый из четырёх циклов бюджета §4.