docs: review document for #309

Issue: #309
User-Visible: no

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

@@ -0,0 +1,242 @@
+# CODE-REVIEW-309-r1
+
+Issue: https://github.com/Matysh/houseplan-card/issues/309
+Ветка: `issue/309-junction-visual-limit`
+Ревьюер: Claude (код-ревью), сессия отдельна от автора (Codex)
+Диапазон: `origin/dev...HEAD`, HEAD = `e85bae70f0e13a70f0395e8199a2bdcc3d775a26`
+Заход: r1 (первый заход этапа код-ревью; этап ТЗ уже прошёл 3 захода и закрыт
+зелёным вердиктом SPEC-REVIEW-309-r3, бюджет циклов код-ревью отдельный — 0/4).
+
+## Скоуп проверки
+
+Три коммита класса A/B поверх ТЗ:
+
+1. `101cf709` `fix: cap junction mitres at the visual limit and drop
+   foreign-sector pair patches (#309)` — `User-Visible: yes`, changelog
+   RU+EN в этом же коммите.
+2. `e85bae70` `test: accept the #309 junction-teeth baselines` — класс D,
+   `Release:`/`Baseline-Reviewed:` присутствуют.
+
+Единственный тронутый продуктовый файл — `src/wall-thickness.ts` (4 хунка):
+константа `VISUAL_MITRE_LIMIT` + функция `chamferApex`; ветка парных патчей
+`linearWallJoinPatches` (порог фаски + скип узлов ≥3 лучей через локальный
+`buildMultiWallNodeMap`); ветка вееров `junctionNodeGeometry` (та же фаска).
+Диффом подтверждено (не заявлением автора): `MULTI_WALL_JOIN_LIMIT`,
+`multiWallBevelCutsAt`, mitre контуров комнат (:1475/:3875) вне диффа —
+AC8 закрыт чтением `git diff origin/dev...HEAD -- src/` (единственный файл —
+`wall-thickness.ts`, только 4 названных хунка).
+
+## Как проверялось — таблица гейтов
+
+| Гейт | Команда | Результат |
+|---|---|---|
+| typecheck | `npx tsc --noEmit` | зелёный |
+| unit | `npm test` | **1 из 1310 красный** (см. Finding H1/M1) — не относится к продуктовому коду #309 |
+| build + копии бандла | `npm run build && cmp dist/... custom_components/.../houseplan-card.js` | зелёный, байты совпадают |
+| bundle:sync (3-я копия для смоков) | `npm run bundle:sync` | зелёный |
+| docs fingerprint | `node scripts/check-docs.mjs` | зелёный (7 файлов, 10 ссылок) |
+| выбор смоков по диффу | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | см. ниже, вывод приложен |
+| смок (прямое совпадение) | `node demo/smoke_junction_holes.mjs` | OK |
+| смок (прямое совпадение) | `node demo/smoke_real_plan_masonry.mjs` | OK (лично прогнан ревьюером — не назван в хендоффе автора) |
+| смок (зарегистрированная связь) | `node demo/smoke_multiwall_junction.mjs` | OK |
+| смок (зарегистрированная связь) | `node demo/smoke_junction_patch_resilience.mjs` | OK |
+| golden | `npm run golden:verify` | **129/129 зелёный**, включая 3 новые `junction-309-{step,spike,hump}-dark` |
+| perf (риск §3.10.4 спеки) | `node demo/smoke_render_perf.mjs` | OK |
+| мутанты #309 (AC7 a/b/c/d) | `node scripts/mutation-gate.mjs --id=<4 id>` | **все 4 поймали регрессию** (см. ниже) |
+| мутант #302, адаптированный диффом | `node scripts/mutation-gate.mjs --id=junction-fans-disabled` | поймал |
+| model-invariants | не прогонялся | diff не создаёт/не переносит записи толщины, не меняет `walls`/`layout`/`marker.space`/`open_spans` — чистая рендер-геометрия узлов, критерий §8 не наступает |
+| backend pytest | не прогонялся | диффом не тронут `custom_components/**/*.py` |
+
+Вывод `smoke-select.mjs` (приложен полностью):
+
+```
+Изменено файлов src/**: 1 · символов проекта на изменённых строках: 7
+Матрица: 190 смоков · порог «широкого» символа: больше 38 смоков
+
+Прямое совпадение (2):
+  demo/smoke_junction_holes.mjs        ← MITRE_LIMIT, buildMultiWallNodeMap, junctionNodeGeometry
+  demo/smoke_real_plan_masonry.mjs     ← MITRE_LIMIT
+
+Зарегистрированная связь (3):
+  demo/smoke_junction_patch_resilience.mjs  ← buildMultiWallNodeMap
+  demo/smoke_multiwall_junction.mjs         ← buildMultiWallNodeMap
+  demo/smoke_multiwall_strip_containment.mjs ← buildMultiWallNodeMap
+```
+
+Решение по каждой строке: оба прямых совпадения прогнаны лично (второе —
+`smoke_real_plan_masonry.mjs` — автор его в хендоффе не называл, прогнал сам,
+результат OK). Из трёх «зарегистрированных связей» прогнаны две
+(`junction_patch_resilience`, `multiwall_junction`); `multiwall_strip_containment`
+не прогонялась лично, но автор заявляет её прогон OK в хендоффе, а смысловая
+связь (только `buildMultiWallNodeMap`, без AC на плотных T-стыках #275) —
+не первоочередная для риска этой задачи (плотная выборка защищённых полос
+#275/#279, не тронутых диффом). Остальные смоки из хендоффа автора
+(`wall_junctions`, `free_walls`, `wall_thickness×3`, `active_chain_ink`,
+`hatch_density`) не перепроверялись — не входят ни в прямые, ни в
+зарегистрированные совпадения инструмента, доверие заявлению автора.
+
+Мутационный гейт AC7, вывод для каждого:
+
+```
+visual-mitre-limit-back-to-4:               поймано 1 из 1
+chamfer-disabled-full-mitre:                 поймано 1 из 1
+pair-patches-at-multiwall-nodes:             поймано 1 из 1
+chamfer-chord-instead-of-perpendicular:      поймано 1 из 1
+junction-fans-disabled (адаптирован #302):   поймано 1 из 1
+```
+
+## Находки
+
+### Medium (в скоупе) — M1: `npm test` красный на проверяемом SHA
+
+**Файл:** `test/golden-matrix.test.mjs:370`
+
+**Воспроизведение:** `npm test` → `not ok 378 - sun-ray golden requires
+browser-painted light from a state-only sun entity` →
+`AssertionError: 46 !== 45` на строке `assert.equal(GOLDEN_MATRIX_VERSION, 45)`.
+Детерминированно (перепроверено дважды, включая изолированный прогон
+`node --test test/golden-matrix.test.mjs`).
+
+**Причина:** коммит `101cf709` корректно поднимает
+`GOLDEN_MATRIX_VERSION` 45 → 46 в `demo/golden/matrix.mjs` (добавлены 3 новые
+сцены `junction-309-*`, что по правилу `demo/golden/README.md` требует
+бампа) — но не обновляет соседний, не тронутый диффом
+`test/golden-matrix.test.mjs`, который хардкодит ожидаемую версию `45` в
+отдельном тесте. Файл `test/golden-matrix.test.mjs` в диффе не фигурирует
+вовсе (не входит в список изменённых файлов коммита).
+
+**Внешнее подтверждение:** CI на точном SHA `e85bae70` (run
+[32886114649](https://github.com/Matysh/houseplan-card/actions/runs/32886114649))
+— job `frontend` **failure**, из-за чего `golden`/`smoke`/`performance_smoke`
+в этом прогоне пропущены (`skipped`) каскадом. Это значит, что ни один из
+golden/смок-результатов, заявленных в хендоффе автора, не подтверждён CI на
+этом SHA — только моим локальным прогоном после `npm run bundle:sync`.
+
+**Важный контекст:** на удалённой ветке уже существует более новый коммит
+`6c1534f1` (`test: pin the golden matrix guard to version 46 (#309)`,
+запушен в 21:55, через 5 минут после `e85bae70`), который правит именно эту
+строку 45→46 и заявляет «Полный npm test — 1309/0». Он **не входит в
+диапазон `origin/dev...HEAD` этого ревью** (HEAD зафиксирован на `e85bae70`
+на момент старта ревью) и потому формально не проверялся — CI на его SHA
+(run 32886626656) зелёный, но это вне scope этого захода. Фикс уже есть,
+но не в проверяемом дереве — по букве процесса это правка, ожидающая
+собственного цикла ревью, а не «уже закрыто».
+
+**Не является дефектом фичи #309**: все 6 юнитов «issue 309», все 4 новых
+мутанта, `junctionContractHoles` и 129/129 golden (включая три новые сцены)
+зелёные и на этом SHA, и после `bundle:sync`. Ломается только не относящаяся
+к геометрии проверка версии матрицы в другом файле.
+
+**Почему Medium, а не High:** дефект не про продукт (пользователь его не
+видит, AC #309 не задет), это красный обязательный гейт `npm test`,
+входящий в «всегда»-набор §8 PROCESS.md — так что не блокирующий сам факт
+не в счёт, а исправление обязательно в этой же задаче.
+
+### Low (снята с записью) — AC5 доказана не буквально как записано в ТЗ
+
+**Файл:** `docs/specs/309-junction-visual-limit.md:100`, AC5.
+
+AC5 требует: «`junctionContractHoles` пуст на всех сценах сета и на **полном
+экспорте отчёта (13 комнат/24 перегородки)**». Фикстура, добавленная этой
+задачей (`test/fixtures/309-junction-teeth.json`), — не полный экспорт, а
+минимальный вырез трёх узлов (0 комнат, 9 перегородок, ровно как записано в
+плане тестов §6: «минимальный вырез трёх узлов»). Существующий смок-детектор
+`smoke_junction_holes.mjs`, который AC5 называет вторым доказательством,
+использует фикстуру `302-junction-artifacts.json` (3 комнаты / 7 стен) — тоже
+не «13/24».
+
+Ни в диффе, ни в репозитории нет фикстуры с 13 комнатами/24 перегородками;
+полный экспорт владельца, судя по всему, не коммитился (аналогично практике
+`scripts/wall-strip-containment.mjs` — внешние бэкапы не копируются в Git),
+но ТЗ не пометило это как «принято предположительно», и хендофф не назвал
+расхождение явно.
+
+**Снимаю без правки** (решение ревьюера, PROCESS §2.7): риск дыр на большом
+реальном плане закрыт равноценным, хоть и не тем же самым тестом — лично
+прогнанным `demo/smoke_real_plan_masonry.mjs`, который сэмплирует контур
+кладки на двух реальных многокомнатных фикстурах (`real-plan-first-floor`,
+`real-plan-second-floor`) после этого диффа: `gapCount: 0` на обоих этажах
+(45757 и 8048 сэмплов). Это не то же самое, что владельческий экспорт из
+issue, но по порядку величины (реальный многокомнатный план, а не
+изолированный узел) закрывает тот же риск, который AC5 называет явно. Автору
+на будущее — либо приложить факт эквивалентности прямо в ТЗ/хендофф, либо
+всё же закоммитить полный экспорт как ещё один `test/fixtures/309-*.json`.
+
+## Проверка AC
+
+| AC | Доказательство | Вердикт |
+|---|---|---|
+| AC1 Шип 10+20 | юнит `issue 309 the acute 10/20 pair keeps its tail within the visual limit` (падает при мутантах a/b) + golden `junction-309-spike-dark` | доказан |
+| AC2 Горб 3×50 | юнит `issue 309 the 3×50 node fans stay within the visual limit` + golden `junction-309-hump-dark` | доказан |
+| AC3 Ступенька 15/15/30/30 | юнит `issue 309 the mixed-thickness cross has no step in a foreign quadrant` (точечный пробник в чужом квадранте) + golden `junction-309-step-dark` | доказан |
+| AC4 Прямые углы неизменны | юнит `issue 309 a square corner of equal depths keeps its byte-identical mitre` + математически: 1.41h < 1.5h ⇒ та же ветка кода, что и раньше (1.41h < 4h) — проверено чтением | доказан |
+| AC5 Без дыр | юнит на фикстуре-вырезе + смок; **не на «полном экспорте 13/24»** — см. Low выше, снята с эквивалентной заменой | доказан частично, снято с записью |
+| AC6 Golden весь сет зелёный | `npm run golden:verify` → 129/129, 3 новые сцены + 16 старых `junction-*` + `junction-owner-repro-dark` байтово прежние (подтверждено прогоном, не по заявлению) | доказан |
+| AC7 Мутанты a/b/c/d | 4 прогона `mutation-gate.mjs --id=...`, все поймали регрессию (см. таблицу гейтов) | доказан |
+| AC8 (не-скоуп #249) | `git diff origin/dev...HEAD -- src/` — единственный файл `wall-thickness.ts`, 4 хунка, ни один не касается `MULTI_WALL_JOIN_LIMIT`/`multiWallBevelCutsAt`/room-contour mitre; существующие тесты/сцены paper envelope не в диффе → зелёные по построению | доказан диффом |
+
+## Разобрано по коду (сверх AC)
+
+- Математическое доказательство, что `chamferApex` никогда не проваливается
+  (не возвращает `null` из-за вырождения) при вызове из обоих сайтов: и в
+  `linearWallJoinPatches`, и в `junctionNodeGeometry` точки `pA`/`pB`(`EA`/`EB`)
+  всегда находятся на расстоянии ровно `halfDepth` от узла по построению
+  (`node + n·halfDepth`), значит их проекция на ось «узел→вершина» строго
+  меньше `visual = 1.5·max(hA,hB)`, а вызывающая сторона уже гарантирует
+  `d > visual` — отсюда `t ∈ (0,1)` строго для обеих граней. Разрыв
+  «фаска не считается → сырой mitre остаётся» (который был бы дефектом в
+  ветке `junctionNodeGeometry`, где фолбэк — именно неусечённый `mitre`, а
+  не безопасный бевел) математически недостижим для текущих вызывающих
+  конструкций. Проверено чтением, не только тестами.
+- Скип узлов ≥3 лучей (`coveredByFans`) корректно не затрагивает 2-лучевые
+  узлы: `buildMultiWallNodeMap` регистрирует только узлы с достаточным
+  числом лучей, что подтверждено и напрямую тестом на остром 10/20-стыке
+  (patches.length ≥ 1 при 2 сегментах).
+- `out.fans` в `junctionNodeGeometry` содержит все три случая (mitre,
+  reflex-хорда, бевel) — значит, полный скип пар в `linearWallJoinPatches`
+  для узлов ≥3 лучей не оставляет непокрытых секторов по построению функции,
+  не только по факту прохождения детектора дыр.
+- Отклонение от буквы ТЗ §3.2 («фаска перпендикулярна биссектрисе сектора»):
+  код режет перпендикулярно направлению «узел→вершина mitre», а не
+  геометрической биссектрисе лучей A/B. Для равных полутолщин это одно и то
+  же (симметрия), для разных (случай AC1, 10 vs 20 см) — строго говоря,
+  разные направления. Технический выбор не оспариваю: canonical-документ
+  (`WALL-THICKNESS.md:206-219`) описывает его именно так («perpendicular to
+  the apex direction»), реализация и документация согласованы, юниты
+  проверяют радиальное ограничение (AC1/AC2 формулируют именно его — «вылет
+  ≤ 1.5h»), а не направление разреза, и golden-скрин зафиксировал
+  фактическую форму. Технические споры автор/ревьюер решает вердикт, а не
+  владелец — вердикт: не возражаю, это законная детализация «как сделано».
+
+## Не проверялось
+
+- `python -m pytest tests_backend` — диффом не тронут `custom_components/**/*.py`.
+- `node scripts/model-invariants.mjs --config ...` — диффом не создаются и не
+  переносятся записи толщины/`walls`/`layout`/`marker.space`/`open_spans`;
+  критерий §8 «трогает геометрию или ссылки на неё» для сохраняемой модели не
+  наступает (это рендер-геометрия существующих узлов, не новая структура
+  данных).
+- Смоки `smoke_wall_junctions.mjs`, `smoke_free_walls.mjs`,
+  `smoke_wall_thickness*.mjs`, `smoke_active_chain_ink.mjs`,
+  `smoke_hatch_density.mjs`, названные автором в хендоффе, — не
+  переисполнялись лично; смок-селектор их не выделил ни прямым, ни
+  зарегистрированным совпадением, доверие заявлению автора без дублирования.
+- `demo/smoke_multiwall_strip_containment.mjs` — не переисполнялся лично (см.
+  раздел «Как проверялось»), доверие заявлению автора.
+- Полный владельческий экспорт (13 комнат/24 перегородки) — не существует в
+  репозитории как фикстура; см. Low-находку.
+
+## Вывод
+
+Геометрия исправления корректна и хорошо защищена: 6 юнитов + 4 новых
+мутанта (все ловят регрессию) + 129/129 golden + прямые/зарегистрированные
+смоки зелёные, включая лично прогнанный `smoke_real_plan_masonry.mjs` на двух
+реальных многокомнатных планах. AC1–AC4, AC6–AC8 доказаны без оговорок; AC5
+доказана частично и снята с запиской (Low). Единственная блокирующая для
+зелёного вердикта находка — M1: обязательный гейт `npm test` красный на
+проверяемом SHA `e85bae70` из-за забытого обновления хардкода
+`GOLDEN_MATRIX_VERSION` в стороннем тестовом файле; подтверждено и локально,
+и упавшим job `frontend` в CI на этом же SHA. Фикс уже есть на ветке
+(`6c1534f1`), но вне диапазона этого захода — по процессу его нужно внести
+(или подтвердить) в рамках возврата на правки и пройти код-ревью заново на
+приведённом дереве.