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

SPEC-REVIEW-233-r1

• Issue: #233 — «Ресайз комнаты: показывать внутренние размеры (от стены до стены), а не по осевым»
• Документ ТЗ: docs/specs/233-resize-inner-dimensions.md, ветка issue/233-resize-inner-dimensions, SHA 77fa698
• Этап: S4-spec-review · заход r1 · блокирующих циклов 0/4
• Ревьюер: Claude (роль «ревьюер ТЗ», сессия отдельна от автора — Codex)

Скоуп

Задача меняет только текст/число подписей ресайза в редакторе разметки: _rszEdgeLabels (три длины) и _rszScaleLabels (габарит угловой рамки) в src/houseplan-card.ts, плюс новую чистую функцию innerEdgeSpan в src/wall-thickness.ts. Площадь и конфигурация не меняются. На момент ревью в ветке присутствуют только два документных коммита (docs(spec): measure resize labels between wall faces, docs(spec): register the #233 spec in the index) — продуктового кода нет, git diff origin/dev...HEAD --stat показывает только docs/specs/**. Ревью корректно ведётся на этапе ТЗ, без кода.

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

1. Прочитан docs/SCOPE.md — задача попадает в J6 («keep the plan true… drag/resize»), редактор разметки — admin-only поверхность, View/киоск не затронуты; ТЗ верно ограничивает поверхность (§8).
2. Прочитан docs/WALL-THICKNESS.md целиком, в частности §2–4 (рост стен, inset/флэт-кэп на стыке нулевой и ненулевой толщины, инвариант площади).
3. Прочитан текущий код: src/houseplan-card.ts (_rszEdgeLabels, _rszScaleLabels, _fmtLen, _renderResizeLayer, _rszEdgeDown) и src/wall-thickness.ts (inwardNormal, insetContour, thicknessCmAt, lookupWall, exactCoveringWall, atomicPolyForRoom, cmsForPoly, roomWallProfile, innerContourForRoom, wallCmToUnits) — сверка диагноза §3 ТЗ построчно с исходником. Диагноз подтверждён точно: площадь уже внутренняя, длины — осевые по вершинам полигона комнаты.
4. Проверена математика контракта §6 ручным разбором трёх сценариев: прямой угол с равными толщинами (AC1), прямой угол с разными толщинами соседей (AC2), угол с одной нулевой стороной (AC4) — с конкретными координатами, не на словах.
5. Сверено, действительно ли thicknessCmAt на смешанном по толщине ребре возвращает «толщину участка по середине ребра», как заявляет §7 ТЗ — найден существующий юнит-тест, который прямо противоречит этому утверждению (см. находку H2).
6. Прочитан docs/USER-GUIDE.ru.md (Resize, §406–424) — сверена текущая формулировка («живые длины и чистую площадь по внутреннему контуру», строка 417), подтверждён факт, что она описывает именно ту рассинхронизацию, которую фиксит issue, и что ТЗ верно планирует её переписать.
7. Проверены обязательные разделы ТЗ по PROCESS.md §7.1 — все присутствуют (сценарий, до/после, диагноз, продуктовые решения, скоуп/не-скоуп, контракт, источник данных, поверхности, файлы/i18n, AC1…AC9, mutation guards, план тестов, перф/безопасность/touch, откат, риски, release-артефакты, блок предположений).
8. Гейты этого раунда: код не менялся, поэтому tsc/test/build прогонять бессмысленно — рассматривался только текст ТЗ. Не прогонял ничего намеренно (см. «Чего не проверял»).

Находки

H1 — Контракт §6 не покрывает открытую сторону, к которой ведёт продуктовое решение §4.3 (High)

Файл: docs/specs/233-resize-inner-dimensions.md, §4 п.3, §6 (строки 60–62, 104–112)

Что не так. Issue прямо требует: «Проёмы и незамкнутые стороны… длина считается по осевой, как сейчас… Явно назвать в ТЗ, а не оставлять на догадку» — то есть ребро, которое само является проёмом/открытой стороной (oSelf = 0), должно всегда показывать осевую длину, независимо от соседей. Контракт §6 формулирует только три отступления:

1. oSelf и оба соседа равны нулю → осевая длина;
2. пересечение даёт ≤0 → 0;
3. соседняя сторона с нулевой толщиной → сокращения на этом конце нет.

Отступление (3) защищает стену (oSelf > 0) от ошибочного сокращения, когда её соседняя сторона открыта — это ровно случай AC4. Но оно не покрывает обратный, явно названный в issue случай: сама измеряемая сторона открыта (oSelf = 0), а соседняя — настоящая стена (oPrev/oNext > 0). Для него работает только общий алгоритм пересечения линий (шаги 1–3 §6), и я проверил его на конкретных координатах:

Комната-прямоугольник, V0=(0,0) V1=(300,0) V2=(300,400) V3=(0,400).
Нижняя сторона V0→V1 — проём (oSelf = 0).
Правая сторона V1→V2 — стена 15 см (половина = 7.5).

Внутренняя линия нижней стороны при oSelf=0 не смещается: y = 0.
Внутренняя линия правой стороны смещена на 7.5 внутрь: x = 292.5.
Пересечение: (292.5, 0) — на 7.5 см левее исходной вершины V1=(300,0).

То есть буквальная реализация контракта §6 сократит осевую длину проёма на половину толщины соседней стены (тут — 7.5 см из 300), хотя issue требует показывать полную осевую длину.

Это не теоретическая тонкость: то же самое место в insetContour (src/wall-thickness.ts:900-907, документировано в docs/WALL-THICKNESS.md §3 «A variable-offset join where exactly one adjacent edge has zero depth is a local flat cap, not a mitre») — то есть код, который считает площадь (AC8, которая не должна меняться) — обрабатывает этот же стык иначе: он не пересекает линии, а оставляет вершину нулевой стороны нетронутой (flat cap). Я прогнал ту же геометрию через логику insetContour: для стыка oA=0/oB=7.5 функция кладёт pa = v (нетронутая вершина (300,0) — конец инсета нижней стороны) и pb = (292.5, 0) (начало инсета правой стороны) как два отдельных узла контура, то есть нижняя (открытая) сторона в площади не укорачивается на этом конце.

Последствие. Если реализовать §6 буквально, для любой комнаты с открытой стороной, у которой хоть один конец примыкает к настоящей стене (обычный случай: открытая планировка, кухня-гостиная, любой проём во всю стену), длина проёма будет показана короче осевой — притом что площадь (через innerContourForRoom) для того же угла не укоротится. Это воспроизводит ровно тот класс дефекта, который весь issue #233 создан устранять («длина и площадь по разным конвенциям»), только на границе проёма, а не на равномерной толщине. Ни один AC (AC1…AC9) и ни один mutation guard (§11) этот случай не проверяют — регрессия останется незамеченной кодом-ревью, раз нечему падать.

Как закрыть. Добавить в §6 четвёртое, явное отступление: «если oSelf == 0, сторона сама является проёмом/открытой стороной — возвращается осевая длина |b−a| без обращения к соседям» (симметрично уже описанному третьему пункту, но для своей, а не соседской нулевой толщины). Нужно также явно назвать, что происходит с соседней стеной в этом случае — судя по разбору insetContour, она тоже не должна сокращаться на этом конце, иначе AC4-подобная защита развалится в паре с новым правилом.

H2 — §7 приписывает thicknessCmAt поведение, которого у неё нет: заявлено фактом, не проверено (High)

Файл: docs/specs/233-resize-inner-dimensions.md, §7 (строки 117–126)

Что не так. §7 утверждает как установленный факт: «Если одно ребро комнаты разрезано на атомарные участки с разной толщиной, thicknessCmAt вернёт толщину участка по середине ребра. Это принятое упрощение… Записано здесь, чтобы ревью не считало это недосмотром.» Формулировка прямо просит ревью поверить автору на слово — и именно поэтому её нужно было проверить.

Проверка показала обратное. В test/wall-thickness.test.mjs:169-173 есть именно такой тест:

test('thicknessCmAt exact-parent fallback does not leak from partial or
     unrelated spans', () => {
  const partial = setWallThickness([], [0, 0], [4, 0], 20, pitch);
  assert.equal(thicknessCmAt(partial, [0, 0], [10, 0], pitch), 0);
  ...

Толщина задана только на части ребра ([0,0]-[4,0], 20 см); запрос полного ребра [0,0]-[10,0] возвращает 0, а не 20 (толщину куска у середины или у начала). lookupWall ищет точное совпадение ключа полного ребра или толерантный фолбэк по совпадению середины/угла — оба рассчитаны на случай «одна запись = один цельный отрезок» (комментарий AUD-159B6-01 в wall-thickness.ts:224-233), а не на «одно ребро комнаты собрано из нескольких разно-толщинных атомарных кусков». exactCoveringWall — обратный случай (короткий атомарный запрос против длинной родительской записи), он тоже не покрывает «длинный запрос против нескольких коротких записей».

Более того, в кодовой базе уже есть правильный, атомарно-осведомлённый конвейер для этой задачи — atomicPolyForRoom → cmsForPoly → roomWallProfile, тот самый, которым innerContourForRoom считает площадь (и который AC8 обязана не менять). §6/§7 предлагают вместо него отдельный, более грубый путь: thicknessCmAt прямо на вершинах полигона комнаты — тем самым для длины и для площади толщина одного и того же ребра с атомарной разнотолщинностью резолвится двумя разными механизмами, и один из них (по цитированному тесту) для такого ребра тихо возвращает 0 вместо реальной толщины.

Последствие. Для любой комнаты, где одна сторона имеет неоднородную по длине толщину (штатный, документированный случай — docs/WALL-THICKNESS.md §1: «a different thickness or a virtual gap remains a real break»), подпись длины по контракту §7 получит oSelf/oPrev/oNext = 0 там, где физически стена есть, — то есть либо не сократится вовсе, либо (в связке с находкой H1) попадёт в ветку «осевая длина», разойдясь с площадью, которая по roomWallProfile эту толщину видит и учитывает. Ни один AC это не ловит.

Как закрыть. Либо явно доказать (ссылкой на код/тест, а не декларацией), что вызывающий код в _rszEdgeLabels/_rszScaleLabels не может столкнуться с таким ребром — но это неверно: сторона со сплит-толщиной — обычное состояние после точечной правки инструментом «Толщина» на части стены; либо переключить §7 на roomWallProfile/cmsForPoly (тот же путь, что уже даёт согласованную с площадью толщину), а не на голый thicknessCmAt по вершинам комнаты.

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

• Диагноз (§3) точен и подтверждён построчно: площадь уже строится через innerContourForRoom + floorMinusBodies, длины — через _fmtLen по вершинам полигона комнаты (осевые). Не догадка — цитаты совпадают с кодом.
• Ловушка с insetContour, не сохраняющим число вершин (§3), — подтверждена чтением insetContour: митра даёт 1 точку, бевел/коллинеарный стык/нулевая сторона — 2, что действительно делает наивный «взять ребро i внутреннего контура» неверным. Решение — пересечение линий трёх рёбер, а не индексация — обосновано корректно для основного случая (не-нулевые соседи).
• AC1/AC2/AC6 математически согласованы с алгоритмом §6 для прямых углов: сокращение равно половине толщины СОСЕДНЕЙ стены на каждом конце, независимо от толщины самого ребра — я перепроверил это на числах (300 см, 15/15 → 285; 15/30 → 300−7.5−15=277.5, что соответствует «сокращает на 7.5+15»). AC6 (обрезка отрицательного результата до 0) — прямое следствие шага 3 алгоритма.
• AC5 (диагональ) — корректно указывает, что наивная формула |b−a| − o − o неверна для не прямых углов и что верный ответ — пересечение линий; сама механика для диагоналей не отличается от прямого угла (не требует доп. кода), что верно, поскольку inwardNormal/lineIntersect не привязаны к прямым углам.
• Границы задачи (§5) — area, раскладка толщин по интервалам, подписи вне ресайза корректно выведены из скоупа; ни одна из них не требует правки для выполнения продуктового решения issue.
• i18n/миграция/compatibility (§9) — обоснованно «не затронуты»: новых строк и полей конфигурации нет, что подтверждается описанным изменением (число в существующей подписи, не текст).
• Touch (§8) — ресайз на тач уже работает и не меняется этой задачей; ссылка на docs/TOUCH-SUPPORT.md корректна, задача не вводит новый UX.
• Откат (§14) и release-артефакты (§16) — соразмерны задаче (чистая функция, одна ревизия, golden не участвует, поскольку подписи ресайза не входят в матрицу golden).
• Все обязательные разделы ТЗ по PROCESS.md §7.1 присутствуют, включая блок принятых технических предположений (§17), корректно отделённый от продуктовых решений владельца (§4).
• Автор не задавал владельцу вопросов сверх пяти уже отвеченных в issue — корректно для этой задачи: оставшаяся неоднозначность (H1/H2) техническая, относится к тому, «что реализация должна сделать, чтобы соответствовать уже принятому продуктовому решению», а не к новому продуктовому вопросу.

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

• Не прогонял tsc/test/build — на этом этапе в ветке нет продуктового кода (только docs/specs/**), гонять гейты не на чём.
• Не оценивал независимость _rszScaleLabels от H1/H2 тестом: этот метод берёт габарит из innerContourForRoom (уже атомарно-осведомлённого через roomWallProfile), поэтому предварительно похоже, что находки H1/H2 не задевают угловую рамку — но это подтверждено только чтением кода, не исполнением, и стоит перепроверить в код-ревью, когда появится реализация.
• Не пытался предугадать финальную реализацию innerEdgeSpan — находки адресованы к контракту (§6/§7) ТЗ, а не к коду, которого ещё нет.
• Golden/смоки/backend не запускал — задача не меняет визуальную матрицу и не трогает custom_components/**/*.py, а код ещё не написан.

Вывод

Диагноз и большая часть контракта — качественная, проверенная работа: автор сам нашёл и явно задокументировал главную ловушку (insetContour не сохраняет число вершин) и синхронизировал возражение с существующим mutation-гейтом. Но контракт §6/§7 не полностью реализует уже принятое продуктовое решение issue (открытые/незамкнутые стороны — п.3 issue, §4.3 ТЗ) и содержит непроверенное, фактически неверное утверждение о поведении thicknessCmAt на разнотолщинном ребре. Оба дефекта — в скоупе задачи (они прямо про контракт длины, который задача и переписывает), оба обнаруживаются без исполнения кода — чтением существующих тестов и геометрии insetContour, и оба, если не исправить в ТЗ, с высокой вероятностью воспроизведут в коде именно ту рассинхронизацию длины и площади, которую issue #233 просит устранить. Возврат автору для правки §6 (явное правило для oSelf == 0) и §7 (источник толщины на смешанном по толщине ребре).