houseplan-card/docs/reviews/SPEC-REVIEW-277-r1.md

SPEC-REVIEW-277-r1

• Issue: https://github.com/Matysh/houseplan-card/issues/277 — «Безопасный Resize: переносить только одну осевую стену без изменения топологии»
• Этап: ТЗ на ревью (PROCESS.md §2.4), заход r1, блокирующих циклов израсходовано 0/4 до этого вердикта
• Артефакт ТЗ: docs/specs/277-safe-resize.md (322 строки) + одна строка индекса в docs/specs/README.md, коммит 6436eafb на ветке issue/277-safe-resize
• Ревьюер: Claude, свежая сессия, без контакта с автором ТЗ

Скоуп ревью

Задача обычного трека (small/trivial явно исключены автором и это верно: меняется interaction contract, несколько persisted подсистем, geometry preflight и широкая матрица регрессий — критерии §5/§5.1 PROCESS.md не выполняются). Ревью — только ТЗ; продуктовый код не менялся (diff ограничен docs/specs/**, класс C).

Проверено:

1. docs/SCOPE.md — задача закрывает J4/J6 (безопасное редактирование архитектурного плана, правдивая геометрия); в скоупе, конфликтов с out-of-scope списком нет.
2. AGENTS.md, PROCESS.md — процесс, классы файлов, обязательные разделы §7.1, лимит циклов, шаблон вердикта.
3. Тело issue #277 и все 5 комментариев — включая аналитику владельца, два продуктовых вопроса Q1/Q2 и решение владельца по ним, занятие автором.
4. docs/USER-GUIDE.ru.md (разделы Resize/Толщина/Touch) и docs/RESIZE.md (текущий канон механики) — сверены термины и текущее поведение, которое ТЗ заменяет.
5. docs/WALL-THICKNESS.md, docs/CANVAS.md, docs/UX-MODES.md — сверена терминология (exact endpoints, atomic collinear spans, _rszMove → _snap, hide_openings anchors resize) и отсутствие конфликтов с новым контрактом.
6. Связанные issue #253 (closed), #199 (closed), #264 (S1-new), #276 (S4-spec-review), #278 (S3-spec) — сверен реальный статус против того, как ТЗ на них ссылается.
7. Выборочная проверка кода на предмет «догадка выдана за факт»: applyRoomScale, clampRoomScale, shiftSharedSpans, _rszApplyPreview, simplifyPoly (упомянуты в аналитике/§9 ТЗ как удаляемые) и src/plan-geometry-preflight.ts (переиспользуется по §12.5) — все существуют в src/. Претензии ТЗ к текущему механизму и ссылки на существующий preflight #199 подтверждены, не изобретены.

Как проверялось

Гейты typecheck/test/build/check-docs/model-invariants не запускались: diff — только docs/specs/** (класс C), продуктового и тестового кода нет, изменённых поверхностей для этих гейтов не существует. Это соответствует этапу — ревью ТЗ разбирает текст, а не прогоняет автотесты; на этапе код-ревью (после S5-ready) полный набор гейтов обязателен снова.

Находки

Все три находки — Medium, в скоупе задачи: без них ни один AC не становится невыполнимым или непроверяемым, но каждая — прямое несоответствие обязательным разделам §7.1 либо требованию самого issue. High-находок нет.

M1 — нет обязательного раздела «риски» (§7.1)

Файл: docs/specs/277-safe-resize.md

Что не так: §7.1 PROCESS.md требует раздел «риски» в числе обязательных; DoR (§2.5) отдельно требует «риски перечислены» как условие перехода в S5-ready. В документе такого раздела нет вовсе — слово «риск» встречается один раз, в шапке, как оценочная метрика аналитики («риск 10/10»), а не как перечень конкретных рисков. При заявленной сложности 9/10 и риске 10/10 (сам issue называет несколько связанных подсистем, в трёх из которых — #264, #276, #278 — ещё нет собственного принятого ТЗ) отсутствие явного перечисления рисков — не формальность.

Как проявится: issue физически не может перейти в S5-ready по чек-листу §2.5, потому что нет раздела, из которого «риски перечислены» можно бы было прочитать. Пример риска, который стоило назвать явно и не назван: #264 (контроллер-рефакторинг, с которым §12.1 требует «согласоваться») сейчас в S1-new, то есть даже не разобран — если #277 внедряется первым, придётся вписывать новый transaction-контракт прямо в монолит без обещанного слайса, и это не зафиксировано как риск/план Б.

Исправление: добавить раздел «Риски» перед «Откат» с 3–5 пунктами (минимум: параллельная работа над #264/#276/#278 и что делает #277 независимо от их порядка; вероятность регрессий на реальных пользовательских планах вне синтетической матрицы; стоимость новой матрицы mutation-тестов).

M2 — нет обязательного «что человек увидит до и после» одной фразой без терминов реализации (§7.1, AGENTS.md)

Файл: docs/specs/277-safe-resize.md, §1 «Сценарий и персона»

Что не так: AGENTS.md прямо требует для ТЗ два продуктовых раздела: (а) какая персона/поверхность/момент, и (б) «что человек увидит до и после, в одном предложении, без терминов реализации». §7.1 повторяет то же самое как отдельный обязательный пункт цепочки. §1 текущего ТЗ описывает и «до», и «после», но исключительно терминами реализации — «room polygons», «thickness records», «independent partitions», «hosted openings», «переносит одну... стену... меняет длину её двух примыкающих стен... никогда не меняет число сегментов». Ни одного предложения, которое администратор увидел бы буквально на экране («стена сдвигается, а всё остальное на плане остаётся как было; там, где безопасно продолжать нельзя, стена просто не едет дальше и ручка показывает почему»), в документе нет.

Как проявится: ТЗ, которое не может ответить на этот вопрос одной фразой, описывает работу, а не изменение продукта (формулировка самого PROCESS.md, §7.1). При отсутствии этой фразы ревьюеру и последующим читателям приходится реконструировать пользовательскую картину из §4–§9, что и является риском для будущего code-review — легко упустить, действительно ли что-то видимое поменялось не по плану.

Исправление: добавить 1–2 предложения в начало §1 в чисто пользовательских терминах (что видно до/после, без «polygon», «vertex», «segment», «thickness record»).

M3 — из плана тестов выпала обязательная анонимизированная fixture реального плана

Файл: docs/specs/277-safe-resize.md, §16 «План тестов» (и §15 AC1–AC16)

Что не так: сам issue в разделе «Обязательные тесты» называет её первым пунктом: «анонимизированная before/after fixture из пользовательского плана» — то есть регрессионный тест, воспроизводящий именно тот реальный случай, который запустил задачу (кладка исчезла целиком после Resize на реальном плане администратора). §16 ТЗ перечисляет только synthetic fixtures (rectangle/L-room, corner clamp, third room, partition duplicate, opening types) и не упоминает анонимизированную реальную fixture ни там, ни в каком-либо AC1–AC16. Не помечено как решённая владельцем правка скоупа и не вынесено в §18 как принятое предположение — то есть требование issue тихо выпало, а не было сознательно снято.

Как проявится: после реализации у ревьюера кода не будет прямого доказательства, что именно тот зарегистрированный баг (не абстрактный класс случаев, а конкретный экспорт с 25 walls/8 partitions/14 openings) действительно исчез — только косвенное покрытие через синтетическую матрицу, которая вероятно закрывает те же корневые причины (partial-shared/third-room/partition-duplicate), но не доказуемо тождественна воспроизведению.

Исправление: добавить в §16 пункт про анонимизированную before/after fixture (без пользовательских данных, только геометрия) и сослаться на неё явно в одном из AC (например, расширить AC10 или завести отдельный AC17), либо, если автор сознательно считает синтетическую матрицу достаточной заменой, записать это explicit решение в §18 с обоснованием — сейчас это ни то ни другое.

Что проверено и корректно

• Оба продуктовых вопроса владельца (Q1 — видимость disabled handle, Q2 — что считается безопасной shared-стеной) закрыты в ТЗ буквально по формулировкам решения владельца (§2 п.3–7); угадывания нет.
• §18 «Принятые технические предположения» — уместный набор: все пять пунктов действительно технические (внутренние enum-имена, tooltip-примитив, семантика «упора в угол», thickness-совместимость по атомарным участкам, порядок #276/#277), ни один не подменяет продуктовое решение.
• AC1–AC16 однозначны, у каждого указан способ доказательства из допустимого словаря (unit/smoke/golden/mutation/benchmark), ни один AC не описывает недоказуемое поведение.
• Технические утверждения о текущем механизме (applyRoomScale, clampRoomScale, shiftSharedSpans, _rszApplyPreview, simplifyPoly, переиспользование src/plan-geometry-preflight.ts из #199) подтверждены в коде — не выдумка.
• Терминология согласована с канонами подсистем: exact endpoints/atomic collinear spans (WALL-THICKNESS.md), grid-snap через _rszMove → _snap (CANVAS.md), View/kiosk без handles (UX-MODES.md, SCOPE.md — лок-инвариант не затронут, замки в задаче не участвуют).
• Скоуп/не-скоуп (§10–11) чётко разграничивают #277 от #253/#264/#276/#278 — пересечения с их предметом нет, отдельный issue не нужен ни на одну находку (все три находки — недоработки самого документа #277, не чужой скоуп).
• Откат (§17) описан: revert реализационного коммита, миграции нет, старые версии карточки читают новые планы без проблем.
• release-артефакты (§17) перечисляют оба changelog, RESIZE.md, USER-GUIDE(.ru).md, CANVAS.md, ARCHITECTURE.md, TESTING.md, STATUS.md, i18n, tracked bundles — полный список, ничего очевидного не упущено.

Чего не проверял

• Гейты typecheck/test/build/check-docs/model-invariants/смоки — не запускал: diff не содержит src/**, test/**, demo/**, гонять их не по чему (только docs/specs/**).
• Не проверял реальный воспроизводящий экспорт пользователя — он не публикуется (issue явно об этом говорит), только текстовое описание в issue и аналитике.
• Не оценивал математическую полноту границ safe range (§3, §7) построчно как формальное доказательство — это годится для код-ревью после того, как появится код и тесты на конкретных fixtures; на этапе ТЗ проверялась проверяемость критерия, а не корректность формулы.

Вердикт

Все три находки — Medium, в скоупе задачи #277, без High. Возвращаю ТЗ автору на правку; исчерпание бюджета циклов ТЗ-ревью — 1 из 4 после этого захода.