mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-30 11:49:16 +00:00
docs: review document for #249
Validate / docs (push) Failing after 36s
Validate / reuse (push) Successful in 57s
Validate / process-gate (push) Failing after 1m36s
Validate / changes (push) Successful in 1m38s
Validate / hacs (push) Skipped
Validate / hassfest (push) Skipped
Validate / backend (push) Skipped
Validate / provenance (push) Successful in 1m47s
Validate / frontend (push) Successful in 9m13s
Validate / golden (push) Failing after 16m25s
Validate / performance_smoke (push) Failing after 20m41s
Validate / smoke (push) Failing after 41m28s
Validate / docs (push) Failing after 36s
Validate / reuse (push) Successful in 57s
Validate / process-gate (push) Failing after 1m36s
Validate / changes (push) Successful in 1m38s
Validate / hacs (push) Skipped
Validate / hassfest (push) Skipped
Validate / backend (push) Skipped
Validate / provenance (push) Successful in 1m47s
Validate / frontend (push) Successful in 9m13s
Validate / golden (push) Failing after 16m25s
Validate / performance_smoke (push) Failing after 20m41s
Validate / smoke (push) Failing after 41m28s
Issue: #249 User-Visible: no
This commit is contained in:
@@ -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 интерактивного
|
||||
инструмента — снимаю с запиской, без возврата на цикл.
|
||||
Reference in New Issue
Block a user