From aa166b7982ad05dd42de5bf1e7d7eba0621bb2f7 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sun, 23 Aug 2026 03:47:17 +0000 Subject: [PATCH] docs: review document for #249 Issue: #249 User-Visible: no --- docs/reviews/CODE-REVIEW-249-r2.md | 212 +++++++++++++++++++++++++++++ 1 file changed, 212 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-249-r2.md diff --git a/docs/reviews/CODE-REVIEW-249-r2.md b/docs/reviews/CODE-REVIEW-249-r2.md new file mode 100644 index 00000000..f8b530a3 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-249-r2.md @@ -0,0 +1,212 @@ +# CODE-REVIEW-249-r2 + +- Issue: [#249](https://github.com/Matysh/houseplan-card/issues/249) — ограниченная геометрия узла из трёх и более стен +- Этап: code (PROCESS.md §2.7) +- Заход: r2 · блокирующих циклов израсходовано 1 из 4 (r1 — красный, потратил 1; этот заход зелёный и бюджет не расходует, #227) +- Коммит на ревью: `26fa9684777b7557d384883f18bb3d097a171caf` ("fix: preserve bounded multi-wall floor geometry") +- База delta-review: `062a98a1d841a5de633392357dcf0d4264ed3620` (коммит, получивший CODE-REVIEW-249-r1, красный) +- ТЗ: `docs/specs/249-multiwall-junction-bevel.md` (SPEC-REVIEW-249-r2, зелёный) +- Материал: `git diff 062a98a1d841a5de633392357dcf0d4264ed3620..26fa9684777b7557d384883f18bb3d097a171caf` — целевой delta-review по PROCESS.md §2.10, не полный прогон + +## Скоуп проверки (по дельте) + +Автор заявил исправление H1 (floor выходил за пределы здания на несимметричных +многолучевых узлах) и M1 (AC2 не покрывал буквальный кейс «15/50/70 см», три +взаимно разные толщины). Дельта r1→r2 касается: + +- `src/wall-thickness.ts`: переписан `bevelMultiWallBody`/`multiWallBevelTrianglesAt` + (пофрагментная перестройка каждого узла в ограниченной маске вместо + агрегированного вырезания треугольников), новая `clipInnerContourToRoom` / + `largestOuterContour`, `innerContourForRoom` получил необязательный параметр + `sharedRoomWallGeometry` и теперь строит чистый пол как + `difference(room.poly, roomGeom)` с защитным клипом на fallback, + `wallBodiesGeometry`/`wallBodiesUnionPath` возвращают новое поле `roomGeom` + (кэшируемая канонические кладка комнат до вырезания проёмов и independent + bodies). +- `src/houseplan-card.ts`: 6 из 8 вызовов `innerContourForRoom` в путях + рендера пробрасывают `this._wallUnionGeometry()?.roomGeom` пятым + дополнительным аргументом. +- `test/wall-thickness.test.mjs`: восстановлен строгий инвариант + `floor_union == room_union − canonical_bounded_walls` (тест, который H1 + требовал вернуть), добавлен матричный кейс `halves: [1.5, 5, 7]` (три + взаимно разные толщины). +- `demo/smoke_multiwall_junction.mjs`: пересчитана координата + `discardedWedge` (геометрия узла сместилась из-за новой retain-to-limit + логики; проверил формулой в комментарии — не ослабление, а пересчёт). +- `docs/ARCHITECTURE.md`, `docs/WALL-THICKNESS.md`, `docs/TESTING.md`, + оба `docs/CHANGELOG*.md`, docs-скриншоты — обновлены по существу в этом же + коммите. + +Не в дельте и не перепроверялось заново: golden matrix/harness, spec-файл, +общая продуктовая рамка ТЗ, `docs/SCOPE.md` — см. «Унаследовано из r1». + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| **H1** (High). `innerContourForRoom` отдавал floor-точку вне здания на несимметричных многолучевых узлах (`cornerSplitFixture({outerCm:15, dividerCm:100})`, extra=1686.24 против допуска 1e-7) | `innerContourForRoom` теперь вычисляет чистый пол как `difference(room.poly, roomGeom)` — канонической, уже ограниченной `1.25×H` кладки, с защитным `clipInnerContourToRoom` на любом fallback-пути; `roomGeom` кэшируется в `wallBodiesGeometry`/`wallBodiesUnionPath` и пробрасывается в 6 из 8 сайтов рендера | `src/wall-thickness.ts:1797-1835` (сама функция), `src/houseplan-card.ts` (6 обновлённых call site); тест `'corner Split clean floors equal room union minus canonical bounded walls'` (`test/wall-thickness.test.mjs:1522`) — **проверил сам**: на `062a98a` (до фикса, через временный `git worktree`) этот же тест падает с `extra=1686.2432660548943` — ровно то число, что называла находка H1; на `26fa968` тест зелёный | +| **M1** (Medium, в скоупе). AC2 не покрывал буквальный кейс «15/50/70 см» — три взаимно разные толщины | В матрицу `cases` добавлен `{ angles: [45, 102, 230], halves: [1.5, 5, 7], bevel: true }` — три genuinely разные halfDepth, прогоняется через тот же полный цикл проверок, что и остальные кейсы (baseline/permutation/production-scale/`multiWallBevelTriangles`/`makeFanGeometry` + `wallBodiesGeometry` с реальным bevel) | `test/wall-thickness.test.mjs:628` (новая строка массива `cases`), выполнение — `test/wall-thickness.test.mjs:686-731` (тот же `for`-цикл, без специального исключения для нового кейса) | + +## Как проверялось + +Гейты, соразмерные диапазону дельты (диапазон трогает `src/**`, требует +`check-docs`; смоки — по `smoke-select.mjs` относительно базы r1, не по +полному набору): + +| Гейт | Результат | Прогнал | +|---|---|---| +| `npx tsc --noEmit` | зелёный, без вывода | да | +| `npm test` | 1117 passed / 0 failed / 0 skipped | да | +| `npm run build` | зелёный | да | +| SHA-256 трёх копий бандла | совпадают: `69b40f0508a6d3372b6d3acd165a643a27d5d0b11fe845fb4c002e13696a5466` (совпадает с заявленным автором) | да, вручную | +| `node scripts/check-docs.mjs` | зелёный (7 файлов, 10 внешних ссылок) | да, обязателен — diff трогает `src/**` | +| `node --test` целевого теста `'corner Split clean floors equal room union minus canonical bounded walls'` | зелёный на `26fa968`; **красный на `062a98a`** (`extra=1686.2432660548943`) — проверено через временный `git worktree`, удалённый по завершении | да, дисциплина «тест умеет падать» подтверждена явно | +| `node demo/smoke_multiwall_junction.mjs` | зелёный, 15/15 | да — прямой AC1/AC4 гейт | +| `node scripts/smoke-select.mjs --base 062a98a --head HEAD` | 5 прямых совпадений: `smoke_glow_fail_dark`, `smoke_junction_patch_resilience`, `smoke_multiwall_junction`, `smoke_wall_thickness_transition`, `smoke_zero_divider_taper` (все ← символы `wallBodiesGeometry`/`_wallUnionGeometry`) | да | +| `node demo/smoke_glow_fail_dark.mjs` | зелёный, 4/4 полей | да | +| `node demo/smoke_wall_thickness_transition.mjs` | зелёный, 11/11 полей | да | +| `node demo/smoke_zero_divider_taper.mjs` | зелёный, 13/13 полей | да | +| `node demo/smoke_junction_patch_resilience.mjs` | зелёный, 15/15 полей | да — переисполнил заново на этом коммите, не унаследовал из r1 | + +**Не прогонялось** (предрелизные гейты по PROCESS.md §8, не гейт ревью): +полный `npm run golden:verify`/`golden:accept`, полный набор из 170 смоков, +performance-профили, backend/HA-harness (Python не тронут дельтой). Golden +baseline не принимался. + +## Что проверено и корректно (по дельте) + +- **H1 закрыта фактически, не только по имени теста.** Числовое + воспроизведение из CODE-REVIEW-249-r1 (`extra=1686.24`, `missing≈0`) + подтверждено моим независимым прогоном того же теста на коммите r1 — тест + падает с точно той же величиной, значит новый тест действительно + чувствителен к дефекту, а не переименован без содержания. На `26fa968` тот + же тест зелёный с допуском `1e-7` в обе стороны плюс отдельная проверка, + что каждая вершина пола остаётся внутри `[100..900]×[100..700]`. +- **Механизм фикса не подвержен тому же классу дефекта.** `bevelMultiWallPaper` + и путь `difference(room.poly, roomGeom)` только *вычитают* заранее + вычисленные ограниченные фигуры — в отличие от старой ветки + `insetContour`/`outsetContour`, которая *добавляла* смещение по нормали без + проверки соседнего ребра (корень H1). `clipInnerContourToRoom` на любом + fallback-пути дополнительно пересекает результат с `pr.poly`, так что + выход точки за пределы контура комнаты структурно исключён на уровне самой + функции, а не только на happy path. +- **M1 закрыта содержательно.** Новый кейс `[1.5, 5, 7]` — три взаимно разные + halfDepth — проходит через тот же строгий цикл (baseline/permutation/ + production-scale `coordScale=1000`/реальный `wallBodiesGeometry` с fan-топологией + из 3 комнат), что и остальные кейсы матрицы, без специального послабления. +- **AC4 (общий body для всех поверхностей) переподтверждён.** Затронутые этой + дельтой пути — `bevelMultiWallPaper` (paper) и clean-floor consumer в + `innerContourForRoom` — проверены смоком `smoke_multiwall_junction.mjs` + (`paperRemainsSolid`, `cleanFloorConsumerIsPresent`, `planUsesCanonicalPath`, + parity Plan/View/kiosk/Static/hidden-Iso) — все зелёные. +- **6 из 8 вызовов `innerContourForRoom`** в `src/houseplan-card.ts` (полный + рендер: floor fills, room hover/labels, decor clip, glow/light) корректно + пробрасывают `this._wallUnionGeometry()?.roomGeom` — проверено построчно + (`grep -n "innerContourForRoom("`, 8 вызовов, 6 с новым аргументом). +- **AC3 (двухлучевые узлы)** дельтой не задета: путь для `multiWallNodes.nodes.length + === 0` не изменился (`return inset` напрямую, как раньше); подтверждено тем, + что полный `npm test` зелёный без правок существующих двухлучевых ожидаемых + значений. +- **Трейлеры и changelog.** Коммит `26fa968` несёт `Issue: #249` и + `User-Visible: yes`; `docs/CHANGELOG.md` и `.ru.md` правлены в этом же + коммите. +- **Документация обновлена по существу**, не формально: `WALL-THICKNESS.md` и + `ARCHITECTURE.md` описывают именно новый механизм (`roomGeom`, вычитание, а + не старую агрегированную triangle-diff схему) — сверил текст с кодом. + +## Находки + +Блокирующих (High) нет. Medium в скоупе — нет. + +### L1 (Low, снимаю без правки, с записью). Два из восьми вызовов `innerContourForRoom` не получили `sharedRoomWallGeometry` + +`src/houseplan-card.ts:8502` (`_rszEdgeLabels`) и `:8549` (`_rszScaleLabels`) — +подписи площади/размера во время **живого drag** resize/scale — вызывают +`innerContourForRoom` с неполным списком комнат (`res.polys`, только реально +подвинутые комнаты, либо вовсе один элемент для scale) и без пятого +аргумента. Проверил обе стороны эффекта: + +1. **Корректность.** Даже без `sharedRoomWallGeometry`, `innerContourForRoom` + сам вычисляет `wallBodiesGeometry(rooms, ...)` на этом же неполном списке + и, если результат не подойдёт, уходит в `clipInnerContourToRoom(inset, + pr.poly)` — обе ветки ограничены пересечением с `pr.poly` этой же комнаты, + поэтому именно дефект H1 (точка вне здания) здесь физически не + воспроизводим: защита в самой функции универсальна, а не завязана на то, + передан ли кэш. Возможное следствие неполного списка комнат — узел на + границе с посторонней (не двигающейся) комнатой может быть на мгновение + классифицирован как двухлучевой вместо 3+-лучевого в подписи площади во + время drag; это не влияет на итоговую сохранённую геометрию и + самоисправляется на `pointerup`, когда рендер снова читает полный + `_wallUnionGeometry()`. +2. **Производительность.** Раз кэш не передан, а в комнатах есть узел + 3+ (обычная ситуация, не экзотика — см. текст issue), `innerContourForRoom` + пересчитывает `wallBodiesGeometry()` заново при каждом вызове внутри + `_rszEdgeLabels`/`_rszScaleLabels`, то есть на каждое `pointermove` во время + resize-драга, отдельно от того, что тот же union уже пересчитывается для + рендера стен через `_wallUnionGeometry()` (кэш которой инвалидируется на + каждый `pointermove`, потому что `_rszApplyPreview` увеличивает + `_cfgEpoch` перед вызовом этих функций). Это дублирующая, но не новая по + порядку величины нагрузка (сам рендер стен уже пересчитывает то же самое + на каждый кадр драга) и касается только редакторского инструмента resize. + +Не блокирую и не прошу правку в этом цикле: последствие ограничено overlay- +подписью во время интерактивного admin-only drag (§ SCOPE.md — редакторы вне +View), не влияет ни на один AC, самоисправляется по окончании драга, и +дополнительная нагрузка того же порядка, что уже существующий пересчёт стен +на каждый кадр. Снимаю как Low с этой записью; если автор захочет, дешёвое +улучшение — прокинуть `this._wallUnionGeometry()?.roomGeom` и в эти два +вызова, поскольку `_curSpaceCfg`/`_spaceModel()` в момент вызова уже отражает +живой `_rszPreview` (полный список комнат), в отличие от `res.polys`. + +## Чего не проверял + +- Полный `npm run golden:verify`/`golden:accept`, полный набор из 170 смоков, + performance-профили, backend/HA-harness — предрелизные гейты по + PROCESS.md §8, не обязаны быть прогнаны на этом заходе; риск R1 спеки + («широкий blast radius для T-стыков») и в r1, и здесь требует полного + golden-прогона перед бетой отдельно от этого цикла. +- Не искал новых экземпляров L1-паттерна (неполный список комнат) вне + `_rszEdgeLabels`/`_rszScaleLabels` — это единственные два незатронутых + дельтой вызова, остальные шесть проверены построчно. +- Не гонял смоки, не входящие в прямые совпадения `smoke-select` для этой + дельты и не относящиеся напрямую к AC (`smoke_split_corner_wall.mjs`, + `smoke_wall_junctions.mjs` — были зелёными в r1 на функции, которые эта + дельта не меняла повторно; не перезапускал, доверяю r1 по §2.10, дельта их + не касается). +- Backend/Python — не тронут диффом. + +## Унаследовано из r1 (без повторной проверки) + +Источник: `docs/reviews/CODE-REVIEW-249-r1.md`, вердикт по коммиту `062a98a`. + +- **Продуктовая рамка и AC как формулировки** (§7.1 ТЗ, разделы 1–9 спека) — + дельта не меняет ни ТЗ, ни AC; полная перепроверка соответствия ТЗ пройдена + в r1. +- **AC5 (golden фиксирует видимый результат)** — golden matrix/harness файлы + не тронуты дельтой r1→r2 (нет изменений в `demo/golden/**`); вывод r1 + «golden-сценарий синтаксически валиден, но заведомо не поймает H1 (`fill_mode: + 'none'`)» остаётся в силе и не пересматривался. +- **AC7 (гейты типа build/typecheck)** — методика не изменилась; перепрогнаны + заново в этом раунде (см. таблицу выше), не просто унаследованы со слов. +- **`multiWallBevelTriangles`/классификация узла (степень, epsilon, порядок, + winding, production scale) вне нового M1-кейса** — не переоценивались + повторно вне того, что уже покрывает общий `for`-цикл матрицы; сам механизм + классификации (`buildMultiWallNodeMap`) дельтой не тронут. +- **Смоки `smoke_split_corner_wall.mjs`, `smoke_wall_junctions.mjs`** — + зелёные результаты r1 приняты без повторного прогона: дельта не касается + кода, который эти смоки покрывают (только H1/M1-специфичные пути). +- **Отсутствие влияния на backend/i18n/миграцию** — подтверждено в r1 и не + оспаривается дельтой (Python/i18n файлы не тронуты ни r1, ни r2). + +## Резюме + +Обе блокирующие находки r1 закрыты предметно: H1 — новым путём вычисления +чистого пола (`difference` от кэшированной ограниченной канонической кладки, с +универсальным защитным клипом), проверенным через явную бисекцию «тест падает +на старом коммите с той же величиной, что называла находка, и зелёный на +новом»; M1 — буквальным кейсом трёх взаимно разных толщин, прогнанным через +тот же строгий матричный цикл. Новая находка L1 (Low) касается двух +call-site, не получивших кэш геометрии в живом resize-драге; она +структурно не может воспроизвести H1 (защита находится в самой функции) и +ограничена overlay-подписью во время admin-only интерактивного +инструмента — снимаю с запиской, без возврата на цикл.