mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-04 05:41:34 +00:00
claude[bot]
@@ -0,0 +1,247 @@
|
||||
# SPEC-REVIEW-233-r1
|
||||
|
||||
- Issue: [#233](https://github.com/Matysh/houseplan-card/issues/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` есть
|
||||
именно такой тест:
|
||||
|
||||
```js
|
||||
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
|
||||
(источник толщины на смешанном по толщине ребре).
|
||||
Reference in New Issue
Block a user