mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-01 12:18:51 +00:00
@@ -0,0 +1,187 @@
|
||||
# SPEC-REVIEW-233-r2
|
||||
|
||||
- Issue: [#233](https://github.com/Matysh/houseplan-card/issues/233) — «Ресайз комнаты: показывать внутренние размеры (от стены до стены), а не по осевым»
|
||||
- Документ ТЗ: `docs/specs/233-resize-inner-dimensions.md`, ветка `issue/233-resize-inner-dimensions`, SHA `bc8c368`
|
||||
- Этап: `S4-spec-review` · заход r2 · блокирующих циклов израсходовано 1/4 (красный r1 потратил цикл; правки на комментарий владельца/ревьюера не считаются пятой попыткой — просто продолжение того же цикла)
|
||||
- Ревьюер: Claude (роль «ревьюер ТЗ», сессия отдельна и от автора, и от сессии r1)
|
||||
- Предыдущий раунд: `docs/reviews/SPEC-REVIEW-233-r1.md`, вердикт красный, получен на SHA `77fa698`
|
||||
|
||||
## Скоуп
|
||||
|
||||
Разбор по дельте (PROCESS.md §2.10): `git diff 77fa698..HEAD` показывает два
|
||||
коммита, оба класса C — `docs: review document for #233` (кладёт документ r1) и
|
||||
`docs(spec): keep a passage full length, read thickness atomically` (правка
|
||||
самого ТЗ). Продуктового кода в диапазоне нет (`git diff --stat` — только
|
||||
`docs/**`). Правка ТЗ: переписаны §6 (добавлено «правило нуля», идущее первым)
|
||||
и §7 (источник толщин сменён с `thicknessCmAt` на атомарный профиль
|
||||
`roomWallProfile`), добавлены AC6a/AC6b и два мутанта, снята формулировка
|
||||
«упрощение» в §15 п.2. Контракт функции (`innerEdgeSpan(prev, a, b, next,
|
||||
oPrev, oSelf, oNext)`), скоуп, продуктовые решения §4, поверхности, i18n — не
|
||||
менялись.
|
||||
|
||||
Предмет этого раунда — ровно эта дельта: закрытие H1 и H2. Полный разбор
|
||||
диагноза, математики прямых углов и границ задачи не повторялся — он не
|
||||
затронут правкой (см. «Унаследовано из r1»).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Получен вердикт r1 и SHA, на котором он выдан (`77fa698`) — из тела
|
||||
`docs/reviews/SPEC-REVIEW-233-r1.md` и из комментария в issue; в самом
|
||||
вердикте SHA не был назван (комментарий содержит только заход/находки), это
|
||||
было бы находкой, но комментарий-хендофф автора после правок сам назвал SHA
|
||||
(`bc8c368`), поэтому диапазон восстановлен без домысливания.
|
||||
2. `git diff 77fa698..HEAD -- docs/specs/233-resize-inner-dimensions.md` —
|
||||
построчная дельта (см. «Скоуп»).
|
||||
3. Для H1: перепроверил новую формулировку §6 («правило нуля идёт первым») на
|
||||
том же численном примере, что в находке r1 (комната 300×400, нижняя сторона
|
||||
— проём, правая — стена 15 см). С новым правилом `oSelf == 0` для нижней
|
||||
стороны безусловно возвращает `|b−a| = 300`, интерполяция с соседями не
|
||||
вызывается вовсе — сценарий сокращения на 7.5 см, описанный в H1, закрыт по
|
||||
определению, а не по побочному эффекту.
|
||||
Дополнительно перечитал `insetContour` (`src/wall-thickness.ts:863-937`),
|
||||
ветку `(oA > 0) !== (oB > 0)` (строки 900-907, `#172`): для стыка
|
||||
ноль/ненулевая толщина функция кладёт **непотревоженную вершину** на
|
||||
нулевой стороне (`pa = v` при `oA` не `>0`) — то есть для площади открытая
|
||||
сторона у этого угла не укорачивается. Новое правило §6 даёт для длины тот
|
||||
же результат (полная осевая длина), а нетронутая (не переписанная в r2)
|
||||
третья строка списка отступлений («сосед с нулевой толщиной → на этом конце
|
||||
сокращения нет») уже защищает противоположную сторону этого же стыка —
|
||||
саму стену 15 см. Обе половины стыка описаны согласованно, площадь и длина
|
||||
сходятся на этой границе так же, как сходятся во всех остальных.
|
||||
4. Для H2: прочитан `test/wall-thickness.test.mjs:168-176` — тест
|
||||
`thicknessCmAt exact-parent fallback does not leak from partial or
|
||||
unrelated spans` подтверждён построчно: запрос по полному ребру `[0,0]-
|
||||
[10,0]` против частичной записи `[0,0]-[4,0]` (20 см) возвращает `0`
|
||||
(строка 169-170) — именно факт, на который ссылается новый §7. Прочитан
|
||||
`thicknessCmAt`/`lookupWall` (`src/wall-thickness.ts:234-266`) — подтверждён
|
||||
механизм (точный ключ либо толерантный фолбэк по середине/углу, рассчитанный
|
||||
на «одна запись = один цельный отрезок», без атомарной осведомлённости).
|
||||
5. Прочитан `roomWallProfile` (`src/wall-thickness.ts:1209-1228`),
|
||||
`atomicPolyForRoom` (982-1035), `cmsForPoly` (1098-1187): подтверждено, что
|
||||
`offsets[i]` для атомарного участка `i` равен `kinds[i] && cm>0 ?
|
||||
units(cm)/2 : 0` (строки 1224-1226) — то есть у виртуальных/открытых
|
||||
участков (`kind === null`) и у участков без записанной толщины `offsets`
|
||||
действительно 0, как заявляет новый §7.
|
||||
6. Прочитан `innerContourForRoom` (1441-1458): подтверждено, что площадь уже
|
||||
строится как `insetContour(roomWallProfile(...).poly,
|
||||
roomWallProfile(...).offsets)` — то есть новый §7 не изобретает источник
|
||||
толщины, а называет **тот самый** вызов, которым уже считается площадь.
|
||||
Центральное утверждение §7 («длина и площадь одного ребра резолвятся одним
|
||||
механизмом») подтверждено на уровне кода, а не принято на слово.
|
||||
7. Перепроверена атомарная привязка «ребро комнаты → атомарные участки»,
|
||||
которую описывает новый §7 (`oPrev` — атомарный сосед, входящий в вершину
|
||||
`a`; `oNext` — выходящий из `b`; `oSelf` — участок, содержащий середину
|
||||
`a→b`): по `AtomicPoly.parent` (индекс родительского ребра для каждого
|
||||
атомарного участка, `atomicPolyForRoom:1011-1034`) это отображение
|
||||
однозначно для любой комнаты — вершины `a`/`b` по построению являются
|
||||
границами родительских рёбер, поэтому «атомарный участок, входящий в
|
||||
вершину» не может быть двусмысленным независимо от того, сколько разрезов
|
||||
получил сам родительский или соседний участок. Новый текст не вносит
|
||||
догадки, выдаваемой за решение — он описывает существующую структуру данных
|
||||
точно.
|
||||
8. Проверено отсутствие регрессии на AC, не задетых дельтой (§2.10, риск того
|
||||
же класса, что дал регрессию #102): AC1/AC2/AC3/AC5/AC6 по-прежнему верны,
|
||||
поскольку новое правило нуля срабатывает только при `oSelf == 0`, а эти AC
|
||||
имеют `oSelf > 0` (или, для AC3, дают тот же осевой результат по прежнему —
|
||||
и теперь по новому — пути) и не проходят через изменённый источник толщины
|
||||
иначе, чем раньше. AC8 (площадь не меняется) не затронут: делянка §7
|
||||
описывает только источник данных для **длины**, `innerContourForRoom` в
|
||||
дельте не упоминается и не переписывается.
|
||||
9. Гейты этого раунда: код не менялся (диапазон — только `docs/**`), поэтому
|
||||
`tsc`/`test`/`build` прогонять нечего — согласно PROCESS.md §8 они
|
||||
применяются к изменённому коду, а его здесь нет. Не прогонял намеренно (см.
|
||||
«Чего не проверял»).
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка | Чем закрыта | Где это видно |
|
||||
|---|---|---|
|
||||
| H1 (High) — контракт §6 не покрывал `oSelf == 0` со стеной-соседом, буквальная реализация сократила бы проём | Новое правило нуля в §6: `oSelf == 0` безусловно и первым возвращает `|b−a|`, до обращения к соседям; добавлены AC6a и мутант `inner-span-shortens-a-passage` | `docs/specs/233-resize-inner-dimensions.md` §6 (diff 77fa698..HEAD, строки после «Правило нуля идёт первым…»); AC-таблица §10, AC6a; §11, `inner-span-shortens-a-passage`. Проверено численно на том же примере, что в H1, и сверено с flat-cap веткой `insetContour:900-907` |
|
||||
| H2 (High) — §7 приписывал `thicknessCmAt` поведение «толщина участка по середине», опровергнутое существующим тестом | §7 переписан целиком: источник толщин — атомарный профиль `roomWallProfile` (тот же, что даёт `innerContourForRoom` для площади), а не `thicknessCmAt` по вершинам комнаты; добавлены AC6b и мутант `inner-span-reads-whole-edge-thickness` | `docs/specs/233-resize-inner-dimensions.md` §7 (полностью замещённый текст); AC6b; `inner-span-reads-whole-edge-thickness`. Проверено чтением `roomWallProfile`/`atomicPolyForRoom`/`cmsForPoly`/`innerContourForRoom` (`src/wall-thickness.ts`) и теста `test/wall-thickness.test.mjs:168-176`, а не заявлением автора |
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Не перепроверялось повторно в этом раунде — дельта их не касается; принято по
|
||||
`docs/reviews/SPEC-REVIEW-233-r1.md` (получен на SHA `77fa698`):
|
||||
|
||||
- диагноз §3 (площадь уже внутренняя через `innerContourForRoom` +
|
||||
`floorMinusBodies`, длины — осевые через `_fmtLen` по вершинам полигона
|
||||
комнаты) — подтверждён в r1 построчным чтением `src/houseplan-card.ts` и
|
||||
`src/wall-thickness.ts`;
|
||||
- ловушка `insetContour`, не сохраняющего число вершин на углу (митра/бевел/
|
||||
коллинеарный стык/нулевая сторона дают разное число точек) — подтверждена в
|
||||
r1 чтением `insetContour`, обосновывает отказ от индексации внутреннего
|
||||
контура в §6;
|
||||
- математика AC1/AC2/AC6 для прямых углов (285 при 15/15; 277.5 при 15/30;
|
||||
клиппинг отрицательного результата к 0) — перепроверена в r1 на конкретных
|
||||
числах;
|
||||
- AC5 (диагональ: пересечение линий, а не `|b−a| − o − o`) — обоснование в r1
|
||||
признано верным, дельта r2 не трогает эту часть контракта;
|
||||
- границы задачи §5 (area, раскладка толщин, подписи вне ресайза — вне
|
||||
скоупа) — согласились в r1, не изменялись;
|
||||
- i18n/миграция/compatibility §9 («не затронуты» — новых строк/полей нет) —
|
||||
подтверждено в r1, не изменялось;
|
||||
- touch §8 (ресайз на тач не меняется этой задачей) — подтверждено в r1;
|
||||
- обязательные разделы ТЗ по PROCESS.md §7.1 присутствуют — проверено в r1,
|
||||
дельта только добавляет текст внутри существующих разделов, ни один не
|
||||
пропал;
|
||||
- отсутствие лишних вопросов владельцу — в r1 признано корректным, дельта
|
||||
тоже не порождает нового продуктового вопроса (H1/H2 были техническими
|
||||
находками ревью, а не открытыми продуктовыми вопросами);
|
||||
- независимость `_rszScaleLabels` от H1/H2 — в r1 отмечено как «правдоподобно,
|
||||
но проверено только чтением, стоит перепроверить в код-ревью»; статус не
|
||||
изменился, дельта r2 эту оговорку не снимает — переносится в код-ревью как
|
||||
прежде.
|
||||
|
||||
## Что проверено и корректно (этот раунд)
|
||||
|
||||
- Новое правило нуля в §6 закрывает H1 именно на том численном примере, на
|
||||
котором находка была построена, и делает это способом, согласованным с
|
||||
существующей flat-cap веткой `insetContour` — то есть длина и площадь
|
||||
сходятся на границе проёма так же, как сходились бы без этой находки.
|
||||
- Новый §7 не просто меняет источник толщины декларативно — он называет тот
|
||||
самый вызов (`roomWallProfile`), которым уже считается площадь; это
|
||||
подтверждено чтением `innerContourForRoom`, а не принято на слово.
|
||||
- Отображение «ребро комнаты → атомарные участки» (`oPrev`/`oSelf`/`oNext` по
|
||||
`AtomicPoly.parent`) однозначно определено для любого числа разрезов на
|
||||
соседних рёбрах — новый текст не оставляет технической двусмысленности.
|
||||
- AC6a и AC6b сформулированы проверяемо (конкретная геометрия, конкретное
|
||||
ожидаемое число), оба получили парный мутант — соответствует требованию
|
||||
PROCESS.md §7.1 «у каждого AC указано, чем он доказывается».
|
||||
- Дельта не задела и не сломала ни один AC, признанный годным в r1 (AC1, AC2,
|
||||
AC3, AC5, AC6, AC8) — проверено явно, а не предположено, ровно из-за
|
||||
прецедента #102, на который указывает промпт этого раунда.
|
||||
- Формулировка риска §15 п.2 («не упрощение, а физически верный результат»)
|
||||
соответствует уточнённому контракту §7 и не противоречит остальному тексту.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- **Не прогонял `tsc`/`test`/`build`** — в диапазоне дельты нет продуктового
|
||||
кода (только `docs/specs/**` и новый файл в `docs/reviews/**`), гонять
|
||||
гейты не на чём; это тот же случай, что в r1.
|
||||
- **Не запускал юнит-тесты `wall-thickness.test.mjs` заново** — цитируемый тест
|
||||
прочитан и подтверждён визуально по исходнику файла, не исполнением; для
|
||||
спецификации это достаточно (тест уже существует и уже проходит на текущем
|
||||
коде — он не является частью этой правки).
|
||||
- **Не проверял реализацию `innerEdgeSpan`** — её ещё нет, код не написан;
|
||||
находки и их закрытие адресованы контракту, а не коду. Именно поэтому в
|
||||
код-ревью нужно будет заново убедиться, что атомарное отображение
|
||||
`oPrev`/`oSelf`/`oNext` реализовано так, как описано в §7 (не через
|
||||
`thicknessCmAt`), и что мутанты `inner-span-shortens-a-passage` и
|
||||
`inner-span-reads-whole-edge-thickness` действительно способны падать.
|
||||
- **Golden/смоки/backend** не запускал — задача не меняет визуальную матрицу
|
||||
и не трогает `custom_components/**/*.py`, а кода ещё нет.
|
||||
|
||||
## Вывод
|
||||
|
||||
Оба High-заключения r1 закрыты в тексте ТЗ содержательно, а не декларативно:
|
||||
новое правило нуля в §6 проверено на том же примере, которым было
|
||||
доказано H1, и согласовано с веткой `insetContour`, которую использует
|
||||
площадь; новый §7 заменяет неверно описанный `thicknessCmAt` на тот самый
|
||||
вызов (`roomWallProfile`), которым площадь уже считается — центральное
|
||||
утверждение раздела подтверждено чтением кода, включая цитируемый тест.
|
||||
Оба закрытия получили собственные AC и мутанты. Дельта не расширяет и не
|
||||
сужает продуктовый скоуп r1, не задевает контракт функции и не ломает ни один
|
||||
ранее принятый AC. Новых находок в дельте нет.
|
||||
|
||||
**Вердикт: зелёный.**
|
||||
Reference in New Issue
Block a user