houseplan-card/docs/reviews/CODE-REVIEW-172-r2.md

Code review #172 — r2

• 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 (HEAD detached at origin/issue/172-zero-divider-taper)
• Reviewed range: origin/dev..HEAD = 93f86f8 (spec) → 56834c5 (spec review doc) → c2112db (fix, User-Visible: yes) → eeb9c34 (code review r1 doc)
• Base: origin/dev at 4d71f57 (уже включает #150 «preserve wall thickness transitions»)
• Reviewer: Claude, независимая сессия без контекста реализации
• Причина цикла r2: r1 был зелёным (High: 0 · Medium: 0), но слияние в dev конфликтовало; PROCESS.md §2.6/§10.4 требует повторного код-ревью после ребейза на ушедший вперёд dev, потому что это другой код. Ветка перебазирована автором на origin/dev 4d71f57 (включает #150), конфликт разрешён, коммит реализации переименован в c2112db. Цикл считается по этапу (§10.4): вердикт по ТЗ не расходует бюджет код-ревью, это первая расходующая бюджет код-ревью правка → r2/4.

Вердикт

Зелёный · цикл r2/4 · High: 0 · Medium: 0.

Продуктовый диф после ребейза содержательно идентичен дифу, уже проверенному в CODE-REVIEW-172-r1.md: те же 25 строк в src/wall-thickness.ts (симметричная ветка «локальный cap» в insetContour()/outsetContour()), тот же набор тестов, тот же smoke, та же golden-сцена (версия матрицы 25), та же документация и оба changelog в одном коммите. Единственное отличие — коммит стал c2112db вместо dfd56e8 (другой SHA после ребейза на dev, содержащий #150) и второй код-ревью документ (eeb9c34) добавлен как отдельный класс-C коммит.

Я не унаследовал вывод r1 не глядя: пересобрал бандл, независимо повторил дисциплину «тест умеет падать» (временно откатил обе новые ветки в коде и получил 2 красных unit-теста и 2 красных поля в named-smoke), прогнал полный юнит-регресс и четыре смежных/зависимых browser-smoke, включая smoke #150 (smoke_wall_thickness_transition.mjs), которого не было в списке r1, потому что на момент r1 #150 не был частью проверяемого дерева — теперь он есть, и обе правки одной и той же общей geometry-функции сосуществуют без конфликта поведения.

Скоуп

Единственный продуктовый файл — src/wall-thickness.ts: insetContour() и outsetContour() получили симметричную ветку if ((oA > 0) !== (oB > 0)), которая перехватывает переход «положительный offset ↔ ровно нулевой offset» до общей mitre/bevel- и collinear-логики, помещённую перед веткой collinearJoint(). Для точного коллинеарного перехода обе ветки вычисляют одну и ту же точку (nA === nB при совпадающем направлении), поэтому перестановка порядка проверок не меняет поведение AC3 (существующая точная ступень).

Сопутствующие изменения (не поменялись с r1): test/wall-thickness.test.mjs (два новых теста), demo/smoke_zero_divider_taper.mjs (новый), demo/golden/{harness,matrix}.mjs + test/golden-matrix.test.mjs (новая сцена split-zero-divider-taper-dark, GOLDEN_MATRIX_VERSION 24→25, baseline сознательно не принят), docs/WALL-THICKNESS.md §3 (контракт cap задокументирован), docs/CHANGELOG.md/docs/CHANGELOG.ru.md, три синхронные копии бандла, docs/specs/README.md.

Ровно один продуктовый коммит c2112db, трейлеры Issue: #172 / User-Visible: yes на месте, оба changelog в том же коммите (проверено git show --stat c2112db). Ветка называется по правилу, process-gate.mjs проходит на всём диапазоне (4 коммита, 0 предупреждений).

Как проверялось

Гейт	Результат
npx tsc --noEmit	pass, без вывода
npm test	833/833 pass (было 830/830 в r1 — разница объясняется тремя тестами #150, которые вошли в базовый dev при ребейзе; сами тесты #172 те же два)
npm run build + сверка трёх копий бандла	pass; sha256sum dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js demo/srv/assets/houseplan-card.js → один и тот же хеш 21f8ffc…3233e; git status --short после копирования — пусто, бандл в дереве уже актуален
Откат обеих новых веток в insetContour/outsetContour, повторный npm test	831/833, красные ровно variable-offset contours keep a local cap at angled positive-to-zero joins и near-collinear zero-depth Split divider never grows a masonry taper — дисциплина «тест умеет падать» подтверждена мной лично на пересобранном дереве, а не переиспользована из r1
Пересборка без фикса + node demo/smoke_zero_divider_taper.mjs	planHasNoTaper: false, lightHasNoTaper: false, FAILED (2) — smoke тоже подтверждённо умеет падать
Восстановление фикса, пересборка, синхронизация трёх копий бандла	все три sha256 совпадают между собой и с закоммиченным деревом
node demo/smoke_zero_divider_taper.mjs (AC6/AC7)	pass, все 13 полей true
node demo/smoke_split_corner_wall.mjs (смежная поверхность, AC3/AC9)	pass
node demo/smoke_wall_thickness.mjs (смежная поверхность)	pass
node demo/smoke_wall_junctions.mjs (смежная поверхность, T/L-стыки)	pass
node demo/smoke_wall_thickness_transition.mjs (#150 — та же общая функция, слита при ребейзе)	pass — правка #172 не сломала соседнюю правку #150 в том же файле
node scripts/process-gate.mjs --range origin/dev..HEAD --target-ref refs/heads/issue/172-zero-divider-taper	pass, 4 коммита, 0 предупреждений (без --issues, офлайн-режим — токен GitHub здесь не нужен для проверки трейлеров/веток/changelog)
git show --stat c2112db	подтверждает оба changelog, документацию и три копии бандла в одном коммите с User-Visible: yes

Не прогонялось, и почему

• npm run golden:verify. Новая сцена split-zero-divider-taper-dark (GOLDEN_MATRIX_VERSION 25) не имеет baseline — проверил напрямую: demo/golden/baselines/ не содержит записи split-zero-divider-taper-dark. verify по контракту (demo/golden/README.md) обязан упасть на отсутствующем эталоне независимо от корректности геометрии; AC8 сознательно откладывает принятие baseline на предрелизный Linux-гейт (golden:accept -- --reviewed). Локальный прогон дал бы только ожидаемый «missing baseline» без новой информации.
• python -m pytest tests_backend. Ни один файл custom_components/**/*.py не тронут этим диффом.
• Performance-профили. Не названы в AC; диф ограничен одним if-блоком на переход, асимптотика не меняется — то же заключение, что и в r1, диф не изменился.
• Полный набор из 127+ browser-smoke. Задача — точечное изменение одной геометрической функции; прогнаны названный в AC смок плюс четыре смежных (corner-split, общая толщина стен, T/L-стыки, и — дополнительно к списку r1 — smoke #150, слитый той же общей функцией при ребейзе). Остальные смоки не используют insetContour/outsetContour в зоне, задетой этим диффом.

Проверка AC1–AC11

Продуктовый код и тесты идентичны r1; переисполнил или перепроверил каждую строку самостоятельно, ссылки на r1 — только там, где вывод не может измениться при неизменном диффе.

AC	Метод по ТЗ	Статус	Как закрыт
AC1	unit	✅	variable-offset contours keep a local cap… — прогнан лично, подтверждён красным без фикса
AC2	unit	✅	near-collinear zero-depth Split divider never grows a masonry taper — матрица outerCm ∈ {1,15,100}, deltaY ∈ {-5,-2.5,2.5,5}, permutation room order/winding; прогнан лично, подтверждён красным без фикса
AC3	unit	✅	Полный регресс 833/833 не покраснел; читал код (src/wall-thickness.ts:810-817 до collinearJoint) — при точном коллинеарном стыке новая ветка вычисляет ту же точку, что и старая (nA===nB), логический регресс исключён; smoke_split_corner_wall.mjs зелёный
AC4	unit	✅ (см. Low-1 ниже, унаследована из r1)	Отдельного нового теста на точную AC1-fixture нет и не появилось при ребейзе (диф теста не изменился). Я предпринял независимую попытку пересчитать инвариант собственным скриптом (innerContourForRoom по каждой комнате в отдельности) и получил числа, не сопоставимые напрямую с методологией r1 (моя примитивная сумма per-room floor не воспроизводит точно то же сечение, что r1 мерил полосой вдоль разделителя) — не нашёл основания усомниться в выводе r1, но и не воспроизвёл его число независимо. См. «Чего не проверял»
AC5	unit	✅ (чтением)	splitRoomPath() не тронута диффом; тест «rendering does not materialize or mutate saved geometry» + smoke anglePreserved/renderDoesNotRewriteConfig
AC6	smoke	✅	node demo/smoke_zero_divider_taper.mjs, лично прогнан, dividerStaysZero/planHasNoTaper true
AC7	smoke	✅	Тот же smoke: planUsesCanonicalBody, planViewParity, kioskParity, isoUsesCanonicalBody, staticParity, lightHasNoTaper, renderDoesNotRewriteConfig — все true
AC8	golden	✅ (отложено по контракту)	Сцена в матрице v25, test/golden-matrix.test.mjs проверяет состав; baseline отсутствует — проверено напрямую по demo/golden/baselines/
AC9	unit+smoke	✅	833/833 + четыре смежных/зависимых smoke зелёные (включая #150)
AC10	код-ревью	✅	Диф ограничен общей variable-offset геометрией wall-thickness.ts, ни одного renderer-specific ветвления
AC11	код-ревью	✅	Новых DOM-узлов/событий/таймеров/сетевых вызовов/HA-сервисов нет

Находки

Новых находок в этом цикле нет — диф не изменился по существу с r1, только SHA после ребейза. Обе находки Low из r1 остаются в силе с тем же решением («снимается без правки»); переношу их сюда без повторной эскалации, чтобы не плодить фиктивный «новый» цикл вокруг уже закрытого вопроса.

Low-1 (унаследована из r1) — AC4 не имеет отдельного исполняемого теста на fixture из АК1

Не изменилось с r1: отдельного unit-теста на clean-floor invariant именно для near-collinear нулевого разделителя по-прежнему нет. r1 закрыл разрыв прямым исполнением innerContourForRoom + wallBodiesGeometry и получил расхождение ≈0,011%, идентичное точному 0° (то есть ранее существующий квантование-артефакт, а не то, что фикс должен был закрыть). Моя собственная попытка независимо пересчитать тот же инвариант (см. AC4 выше и «Чего не проверял») использовала другую, более грубую методологию и не дала сопоставимого числа — это ограничение моей проверки, а не найденное расхождение с выводом r1. Диф, на котором сделан вывод r1, не изменился.

Вердикт: остаётся снятой без правки, как в r1. Не переоткрываю как новую находку — методологическое расхождение в моей повторной проверке не опровергает измерение r1 и не является само по себе дефектом кода.

Low-2 (унаследована из r1) — неточная ссылка на процесс в хендоффе первого цикла

Касалась исходного implementation-хендоффа (цитата §11.4 не по адресу для пропуска named-smoke). Автор сам прогнал smoke перед вторым хендоффом («Повторный хендофф после ребейза» явно перечисляет node demo/smoke_zero_divider_taper.mjs → pass, все 13 проверок true), так что для r2 вопрос уже неактуален практически, а не только формально.

Вердикт: снимается окончательно, без дальнейших действий.

Что проверено и корректно

• Ребейз не изменил продуктовую логику: git diff origin/dev...HEAD -- src/wall-thickness.ts даёт тот же 25-строчный диф, что описан в r1, только с другим базовым SHA.
• Правка сосуществует с #150 без конфликта поведения: обе используют одну и ту же общую функцию insetContour/outsetContour, smoke_wall_thickness_transition.mjs (#150) зелёный на дереве, содержащем обе правки.
• Дисциплина «тест умеет падать» подтверждена мной лично на пересобранном дереве (не переиспользовано заявление r1): 2 unit-теста и named-smoke красные без фикса, зелёные с фиксом.
• Три копии бандла побайтово идентичны друг другу и рабочему дереву (git status --short пуст после пересборки).
• Трейлеры, оба changelog, docs/WALL-THICKNESS.md, docs/specs/README.md — в одном продуктовом коммите c2112db (git show --stat).
• process-gate.mjs проходит на всём диапазоне origin/dev..HEAD (4 коммита, 0 предупреждений).
• Golden-сцена добавлена в матрицу без преждевременного baseline — проверено прямым просмотром demo/golden/baselines/, а не только чтением ТЗ.
• Симметрия inset/outset, ограничение локального cap физической half-depth, независимость от порядка комнат/winding — те же гарантии, что в r1, диф не изменился, регресс логически исключён (см. AC3 выше).

Чего не проверял

• Визуальный итог новой golden-сцены — baseline не существует по контракту до предрелиза.
• Backend/HA harness — не затронут.
• Полный набор из 127+ browser-smoke и performance_smoke — не относятся к этому точечному изменению; обязательны на предрелизном гейте.
• Мобильный/touch путь Split — ТЗ фиксирует независимость сохранённой геометрии от типа указателя, инструмент desktop-first и не менялся этим диффом.
• Независимое числовое воспроизведение AC4 (clean-floor invariant на точной AC1-fixture) — моя попытка пересчитать инвариант собственным скриптом использовала иную методологию, чем r1 (суммирование innerContourForRoom по комнате вместо измерения полосы вдоль разделителя), не дала сопоставимого числа и была отброшена как неубедительная, а не доведена до совпадения с результатом r1. Полагаюсь на измерение r1 (Low-1), поскольку диф, на котором оно сделано, не изменился.
• Правильность конкретной оценки владельца (8/10 · 6/10 · P2) и легитимность полного трека по существу — уже подтверждены SPEC-REVIEW-172-r1.md, не предмет код-ревью.