docs: review document for #172

Issue: #172
User-Visible: no

docs/reviews/CODE-REVIEW-172-r1.md

@@ -0,0 +1,168 @@
+# Code review #172 — r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/172
+- **Spec:** `docs/specs/172-zero-divider-taper.md`, зелёное `SPEC-REVIEW-172-r1.md`
+- **Reviewed branch:** `issue/172-zero-divider-taper`
+- **Reviewed range:** `origin/dev..HEAD` = `4582628` (spec) → `c2c9c8b` (spec review doc)
+  → `dfd56e8` (fix)
+- **Base:** `origin/dev` at `c27185c`
+- **Reviewer:** Claude, независимая сессия без контекста реализации
+
+## Вердикт
+
+**Зелёный · цикл r1/4 · High: 0 · Medium: 0.**
+
+Дефект из #172 устранён в общей geometry-функции, подтверждён исполняемыми
+юнит- и browser-smoke тестами, которые я лично прогнал и проверил на
+способность падать (временный откат фикса красит именно новые проверки).
+Все 11 AC закрыты — либо автотестом, либо чтением кода с явной пометкой. Две
+находки Low сняты в этом документе без блокировки.
+
+## Скоуп
+
+Единственный продуктовый файл — `src/wall-thickness.ts`: `insetContour()` и
+`outsetContour()` получили симметричную ветку «локальный cap», которая
+перехватывает переход `положительный offset ↔ ровно нулевой offset` до общей
+mitre/bevel-логики и явно сохраняет обе точки (offset-точку толстой грани и
+нетронутую вершину нулевой грани), не позволяя bevel-ветке отбрасывать вершину
+нулевой грани и растягивать клин вдоль всего разделителя.
+
+Сопутствующие изменения: `test/wall-thickness.test.mjs` (два новых теста),
+`demo/smoke_zero_divider_taper.mjs` (новый, реальный Split через UI),
+`demo/golden/{harness,matrix}.mjs` + `test/golden-matrix.test.mjs` (новая
+визуальная сцена, `GOLDEN_MATRIX_VERSION` 24→25, baseline сознательно не
+принят), `docs/WALL-THICKNESS.md` §3 (задокументирован контракт cap),
+`docs/CHANGELOG.md` / `docs/CHANGELOG.ru.md`, три синхронные копии бандла.
+Ровно один продуктовый коммит `dfd56e8`, трейлеры `Issue: #172` /
+`User-Visible: yes` на месте, оба changelog в том же коммите. Ветка
+`issue/172-zero-divider-taper` соответствует правилу именования.
+
+## Как проверялось
+
+| Гейт | Результат |
+|---|---|
+| `npx tsc --noEmit` | pass |
+| `npm test` | **830/830 pass** (`npm run inventory` подтверждает канонический счётчик) |
+| `npm run build` + сверка трёх копий бандла | pass, все три `sha256` = `656c68df53108…34a181`, совпадает с закоммиченным |
+| Откат двух новых веток в `insetContour`/`outsetContour` и повторный `npm test` | **2 новых теста красные** (`variable-offset contours keep a local cap…`, `near-collinear zero-depth Split divider never grows a masonry taper`), остальные 828 зелёные — дисциплина «тест умеет падать» подтверждена мной, а не только автором |
+| `node demo/smoke_zero_divider_taper.mjs` (назван в AC6/AC7) | pass, все 13 полей `true`; повторно собрал бандл **без** фикса и прогнал тот же smoke — `planHasNoTaper` и `lightHasNoTaper` красные, с фиксом — зелёные |
+| `node demo/smoke_split_corner_wall.mjs` (смежная поверхность: mitre/bevel для двух положительных толщин, AC3/AC9) | pass |
+| `node demo/smoke_wall_thickness.mjs` (смежная поверхность: общий рендер стен/проёмов) | pass |
+| `node demo/smoke_wall_junctions.mjs` (смежная поверхность: T/L-стыки через ту же join-логику) | pass |
+| Ручная проверка AC4 (clean-floor invariant) на точном fixture AC1 через `innerContourForRoom` + `wallBodiesGeometry`, повторённая для `0°`, `0,477°`, `0,955°`, `-0,955°` | остаточная площадь ≈0,011% от общей (39 из ~340 527 единиц), одинаковая на точном `0°` и под углом — не фикс-специфичный дефект, а ранее существующий квантование-эпсилон atomic-геометрии |
+
+### Не прогонялось, и почему
+
+- **`npm run golden:verify`.** Диффа в `demo/golden/harness.mjs`/`matrix.mjs`
+  добавляет новую сцену `split-zero-divider-taper-dark` без baseline —
+  `verify` по контракту (`demo/golden/README.md`) обязан упасть на
+  отсутствующем эталоне независимо от корректности геометрии. AC8 сознательно
+  откладывает принятие baseline на предрелизный Linux-гейт
+  (`golden:accept -- --reviewed`), это прямо написано в ТЗ §12 и в хендоффе.
+  Локальный прогон дал бы только ожидаемый «missing baseline» без новой
+  информации; риск регрессии существующих сцен уже закрыт тремя целевыми
+  smoke-тестами и полным юнит-регрессом на той же общей geometry-функции.
+- **`python -m pytest tests_backend`.** Ни один файл `custom_components/**/*.py`
+  не тронут.
+- **Performance-профили.** Не названы в AC; спецификация явно фиксирует, что
+  прирост вершин ограничен одной точкой на переход и не меняет асимптотику;
+  диффа в hot-path выше единичного `if`-ветвления нет.
+- **Полный набор из 136 browser-smoke.** Задача касается одной геометрической
+  функции с точечным изменением контракта; прогнаны названный в AC смок плюс
+  три смежных (corner-split, общая толщина стен, T/L-стыки) — поверхности,
+  которые используют ту же `insetContour`/`outsetContour`. Остальные 132 смока
+  не относятся к затронутой геометрии (проёмы без стен, Glow-специфика без
+  стен, UI-хром и т.д.).
+
+## Проверка AC1–AC11
+
+| AC | Метод по ТЗ | Статус | Как закрыт |
+|---|---|---|---|
+| AC1 | unit | ✅ | Новый тест на точном fixture §3 (`0,477°`/`0,955°`); я подтвердил, что он красный на исходном (без фикса) коде |
+| AC2 | unit | ✅ | Тот же тест — матрица `h=1/15/100`, углы по обе стороны, winding-перестановка; изолированный тест `insetContour`/`outsetContour` покрывает обе последовательности `h→0`/`0→h` напрямую на примитиве |
+| AC3 | unit | ✅ | Полный регресс (830/830) не покраснел; точный коллинеарный шаг и пары `1↔15`, `15↔100` — существующие тесты остались зелёными; `smoke_split_corner_wall.mjs` подтверждает facade для 0/15/100 см |
+| AC4 | unit | ✅ (низкая находка, см. ниже) | Отдельного теста именно для AC1-fixture нет; я исполнил тот же helper-pipeline, что и существующий тест clean-floor (строка 936), на fixture с углом — расхождение ≈0,011%, идентичное точному `0°`, то есть не связано с фиксом |
+| AC5 | unit | ✅ (чтением) | `splitRoomPath()` не тронута этим диффом; тест «rendering does not materialize or mutate saved geometry» плюс smoke-поля `anglePreserved`/`renderDoesNotRewriteConfig` эмпирически подтверждают отсутствие snap и мутации конфигурации |
+| AC6 | smoke | ✅ | `node demo/smoke_zero_divider_taper.mjs` — реальный Split из вогнутого угла под ~1°, `dividerStaysZero`, `planHasNoTaper` = true |
+| AC7 | smoke | ✅ | Тот же smoke: `planUsesCanonicalBody`, `planViewParity`, `kioskParity`, `isoUsesCanonicalBody`, `staticParity`, `lightHasNoTaper`, `renderDoesNotRewriteConfig` — все true |
+| AC8 | golden | ✅ (отложено по контракту) | Сцена добавлена в матрицу (v25), `test/golden-matrix.test.mjs` проверяет её состав; baseline не принят — так и требуется до предрелизного Linux-гейта |
+| AC9 | unit+smoke | ✅ | 830/830 + три смежных smoke зелёные, независимые тела и проёмы не меняются |
+| AC10 | код-ревью | ✅ | Прочитан диф: правка только в общей variable-offset геометрии `wall-thickness.ts`, ни одного renderer-specific ветвления, схема данных не тронута |
+| AC11 | код-ревью | ✅ | Прочитан диф: новых DOM-узлов/событий/таймеров/сетевых вызовов/HA-сервисов нет |
+
+## Находки
+
+### Low-1 — AC4 не имеет отдельного исполняемого теста на fixture из АК1
+
+Спецификация назначила AC4 методом `unit`, но фактический diff теста
+(`test/wall-thickness.test.mjs`) не содержит проверки clean-floor invariant
+именно для near-collinear нулевого разделителя — только для
+`dividerCm: 100` (существующий тест до этой задачи). Я закрыл разрыв
+самостоятельно: исполнил `innerContourForRoom` + `wallBodiesGeometry` по тому
+же fixture, что и AC1 (`[600,400]→[900,405]` и соседние углы), и получил
+устойчивое расхождение ≈39 единиц из ~340 527 (≈0,011%) — идентичное значению
+на точном `0°`. Поскольку расхождение не зависит от угла и воспроизводится
+даже без него, это ранее существующий квантование-артефакт atomic-геометрии,
+а не то, что фикс должен был закрыть и не закрыл.
+
+**Вердикт:** снимается без правки. Инвариант AC4 подтверждён мной прямым
+исполнением (не только чтением), отдельный тест на будущее не обязателен —
+дальнейшее покрытие этой границы можно adресовать при следующей правке той же
+области, если она понадобится.
+
+### Low-2 — неверная ссылка на процесс в хендоффе при пропуске named-smoke
+
+Хендофф реализации указывает, что `node demo/smoke_zero_divider_taper.mjs`,
+golden и performance не запускались «по принятому implementation loop
+(`PROCESS.md §11.4`)». §11.4 — это исключение для починки упавших
+предрелизных гейтов после `S8-merged`, а не основание пропускать smoke,
+названный в AC, перед выходом в `S7-code-review`. Правило, которое реально
+требует прогона таких smoke локально, — это правка `AGENTS.md` (issue #151,
+раздел «Гейты»): «перед переводом issue в `S7-code-review` — прогнать смоки,
+названные в её AC, локально». Golden и performance действительно откладываются
+на предрелиз (это верно и без §11.4), но smoke-тест из AC6/AC7 — нет.
+
+**Вердикт:** снимается без правки для этого цикла. Я прогнал
+`demo/smoke_zero_divider_taper.mjs` сам (см. таблицу гейтов) — тест зелёный и
+подтверждённо умеет падать без фикса, так что риска для этого issue нет.
+Отмечаю только неточность цитаты в хендоффе на будущее.
+
+## Что проверено и корректно
+
+- Правка симметрична для `insetContour()`/`outsetContour()`, порядок точек
+  (offset-точка физической грани → нетронутая вершина нулевой грани)
+  детерминирован для inset и outset независимо.
+- Для точного коллинеарного перехода `h↔0` новая ветка производит тот же
+  результат, что и старая `collinearJoint`-ветка (при `oB=0` смещение `nB*0`
+  вырождается в исходную вершину) — регресс на этот случай логически
+  исключён и эмпирически подтверждён (830/830, включая существующий тест на
+  точную ступень).
+- Переход двух положительных offset'ов не затронут: новая ветка
+  активируется только при строгом `(oA>0) !== (oB>0)`, что подтверждено и
+  чтением кода, и тремя зелёными смежными smoke.
+- Локальный cap ограничен физической half-depth и не растёт пропорционально
+  длине разделителя — подтверждено тестом с полосой сэмплирования,
+  масштабируемой от `halfDepth`, для `outerCm ∈ {1,15,100}`.
+- Симметрия относительно порядка комнат и winding подтверждена перестановочным
+  тестом (reversed room order + reversed polygon winding, area-diff = 0).
+- Один канонический источник геометрии для Plan/View/kiosk/static/hidden-Iso и
+  light-барьеров подтверждён smoke-полями `planViewParity`, `kioskParity`,
+  `staticParity`, `isoUsesCanonicalBody`, `lightHasNoTaper`.
+- Рендер не мутирует `rooms`/`walls` (`renderDoesNotRewriteConfig: true`,
+  плюс существующий unit-тест).
+- Трейлеры, changelog RU/EN, `docs/specs/README.md`, `docs/WALL-THICKNESS.md`
+  — все в одном продуктовом коммите, соответствуют правилам PROCESS.md §7.1
+  и §10.1.
+- Golden-матрица и её собственный тест (`test/golden-matrix.test.mjs`)
+  корректно описывают новую сцену без преждевременного принятия baseline.
+
+## Чего не проверял
+
+- Визуальный итог новой golden-сцены (baseline не существует по контракту до
+  предрелиза — см. «Не прогонялось» выше).
+- Backend/HA harness — не затронут.
+- Полный набор из 136 browser-smoke и `performance_smoke` — не относятся к
+  этому точечному изменению; остаются обязательными на предрелизном гейте.
+- Мобильный/touch путь Split отдельно не тестировал: ТЗ фиксирует, что
+  сохранённая геометрия не зависит от типа указателя, а сам инструмент Split
+  desktop-first и не менялся этим диффом.