mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -0,0 +1,233 @@
|
||||
# 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` на SHA `6e1aea3a`
|
||||
(коммиты `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 `_rszSel` changes 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`, вне драга):
|
||||
|
||||
```ts
|
||||
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) и своей же мерой риска называет то самое
|
||||
неверное утверждение о «текущем» поведении. Это ровно тот класс дефекта,
|
||||
о котором предупреждает процесс: догадка о поведении, записанная как факт,
|
||||
проходит ревью, потому что выглядит решением.
|
||||
|
||||
**Требуемое исправление одно из двух** (решает автор, не владелец — это
|
||||
техническая деталь конечного автомата контроллера, не то, что видит
|
||||
пользователь по существу задачи):
|
||||
1. явно зафиксировать, что рефакторинг **меняет** это конкретное поведение
|
||||
(Escape после выбора комнаты в Resize отныне сразу выходит в Draw), и
|
||||
убрать формулировку «как фактически сейчас» — задокументировать как
|
||||
осознанное упрощение; либо
|
||||
2. сохранить двухшаговый 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.
|
||||
Reference in New Issue
Block a user