docs: review document for #234

Issue: #234
User-Visible: no

docs/reviews/CODE-REVIEW-234-r2.md

@@ -0,0 +1,187 @@
+# CODE-REVIEW-234-r2
+
+- Issue: [#234](https://github.com/Matysh/houseplan-card/issues/234) — толщина отрезка цепочки стен расходится между превью и записью
+- Этап: код-ревью (PROCESS.md §2.7)
+- Заход: r2 · блокирующих циклов израсходовано **0 из 4** (r1 был зелёным, бюджет не тратил — правило #227)
+- Материал: `git log --oneline origin/dev..HEAD`, `git diff origin/dev...HEAD`
+- Коммит на вершине: `c8e9597` (единственный коммит диапазона, `dev`@`1e2eba9`)
+- Трейлеры коммита: `Issue: #234` · `User-Visible: yes` — оба changelog правятся в этом же коммите
+
+## Почему разбор полный, а не по дельте
+
+Раунд r1 (SHA `a0878cd`) закончился зелёным вердиктом без замечаний, требующих
+правки. Между r1 и этим заходом код задачи не менялся — но ветка была
+**ребейзнута** на ушедший вперёд `dev` (в `dev` за это время влилась #229,
+слияние коллинеарных перегородок, тоже правящая `src/houseplan-card.ts`).
+PROCESS.md §2.10 и §7.2 прямо называют этот случай исключением из сокращения
+по дельте: «после ребейза на ушедший вперёд `dev` это другой код». Поэтому
+этот документ — полный разбор AC1…AC9, а не разбор одной строки диффа.
+
+Практически это означает: код резолвера и всех шести точек вызова я прочитал
+и проверил заново на текущем `HEAD`, а не принял на веру утверждение «правок
+кода не потребовалось» из комментария владельца.
+
+## Что проверил
+
+### 1. Диагноз и контракт по коду
+
+Построчно сверил `src/wall-face-graph.ts` и шесть точек в
+`src/houseplan-card.ts` с контрактом §6 ТЗ (`docs/specs/234-chain-segment-thickness.md`):
+
+- `chainSegmentCms(segmentCount, recorded, activeCm, defaultCm)` — правило
+  «своя запись `> 0` → предыдущий валидный → `activeCm` → `defaultCm`»
+  реализовано буквально (`src/wall-face-graph.ts:78-97`); `defaultCm`
+  невалидный вызывающего приводится к минимальной толщине 1 см, а не к
+  выдуманному значению — это расширяет контракт консервативно, не меняя его.
+- `wallChainSegments` лишена собственного fallback: принимает `cms[i]` как
+  есть (`wall-face-graph.ts:112-121`).
+- Все шесть мест переведены на резолвер:
+  1. превью цепочки (`houseplan-card.ts:18610-18621`);
+  2. незамкнутая цепочка → перегородки, `_finishWallChain`
+     (`:6576-6584`);
+  3. замкнутая цепочка → перегородки (`:13089-13099`);
+  4. замкнутый контур → комната, `edgeCms`/`source` (`:12710-12742`);
+  5. подсветка инструмента «Толщина», `_wallSourceCmAt` (`:12426-12435`);
+  6. батч граней стен, `_applyWallFaceBatch` (`:12491-12497`) — шестое место,
+     найденное автором сверх каталога ТЗ §3; подтверждаю, что оно
+     действительно использовало `wallChainSegments(..., DRAW_WALL_DEFAULT_CM)`
+     до правки и теперь читает тот же резолвер.
+- Инвариант длины: точка и её толщина пишутся одной операцией в
+  `_markupClick` (`:7301-7312`), `_persistActiveDraftSegment` больше не решает,
+  писать ли толщину (`:7426-7442`, комментарий явно называет причину). Четыре
+  пути чтения черновика (`:2618`, `:7243`, `:7383`, `:12817`) приведены к длине
+  через новый `_adoptDraftCms` (`:7432-7442`), который использует тот же
+  резолвер и логирует дозаполнение в `console.debug`.
+
+### 2. Гейты — прогнаны на HEAD (`c8e9597`) в этой сессии
+
+| Команда | Результат |
+|---|---|
+| `npx tsc --noEmit` | чисто |
+| `npm test` | **1019 pass / 0 fail** |
+| `npm run build` | собран |
+| `cmp dist/... custom_components/.../houseplan-card.js` | совпадают побайтово |
+| `cmp dist/... demo/srv/assets/houseplan-card.js` | совпадают побайтово |
+| `node scripts/mutation-gate.mjs --check` | зелёный, все id, включая три новых, структурно валидны |
+| `--id=chain-thickness-falls-back-to-default` | поймано 1 из 1 |
+| `--id=chain-thickness-preview-diverges` | поймано 1 из 1 |
+| `--id=chain-thickness-length-invariant-dropped` | поймано 1 из 1 |
+| `node demo/smoke_wall_chain_thickness.mjs` | **OK** |
+| `node demo/smoke_draw_wall_thickness.mjs` | **OK** (регресс §12.4 ТЗ) |
+| `node demo/smoke_wall_thickness_transition.mjs` | **OK** (регресс §12.4 ТЗ, `sharedKept: true` — общие участки между комнатами не задеты) |
+
+В отличие от автора, у ревьюера Chromium в песочнице доступен — целевой смок
+и оба регрессионных прогнаны напрямую, не со слов.
+
+**Тест умеет падать — проверено, а не предположено.** Временно вернул
+резолверу старую подмену (`own ?? previous ?? fallbackTail` → `own ??
+fallbackTail`), пересобрал бандл, скопировал в `demo/srv/assets` и прогнал
+целевой смок: он покраснел ровно на том свойстве, которое проверяет
+(`gapInheritedPrevious: expected true, got false`). После проверки дерево
+возвращено в исходное состояние (`git checkout -- ...`), пересборка и сверка
+трёх копий подтвердили чистоту рабочей копии.
+
+### 3. AC — чем доказано
+
+| AC | Требование | Статус |
+|---|---|---|
+| AC1 | `chainSegmentCms` возвращает `segmentCount` строго положительных чисел на любом входе, включая явный `0` | зелёный, unit, таблица мусорных входов покрывает границу `> 0` |
+| AC2 | Пропуск наследует предыдущий валидный → `activeCm` → `defaultCm` | зелёный, unit |
+| AC3 | Превью и запись дают идентичный вектор на одних данных | зелёный, unit (`the preview and the writers cannot disagree`) |
+| AC4 | Цепочка 30/30/(пропуск) сохраняется как 30,30,30 | зелёный, unit + смок |
+| AC5 | Контур → комната сохраняет распределение превью, общие участки не задеты | смок `sharedKept: true` + прочитан код: `edgeCms[source]` берётся из того же резолвера, раскладка по интервалам (`applyWallThicknessToNewRoom`/`setWallThickness`) не тронута — вне скоупа задачи и не изменена |
+| AC6 | Подсветка инструмента = записанное значение | зелёный, смок (`highlightMatchesStored`) |
+| AC7 | Черновик с `segments` короче `points-1` дочиняется по правилу §6 | зелёный, unit + смок (`legacyResumedFullVector`, `legacyGapsInheritedThirty`) |
+| AC8 | Точка не добавляется без валидной толщины; длины согласованы | **проверено чтением, не исполнением**: атомарная запись в `_markupClick` структурно исключает расхождение; отдельного сквозного теста последовательности кликов нет — граница признана автором и приемлема, т.к. поведение выражено на уровне одного метода, а не координации двух |
+| AC9 | Обе записи changelog в том же коммите | в диффе `c8e9597`, подтверждено `git show --stat` |
+
+### 4. Взаимодействие с #229 (ребейз)
+
+Автоматическое слияние `src/houseplan-card.ts` и `scripts/mutation-gate.mjs` —
+затронутые участки не пересекаются (резолвер `chainSegmentCms` и `spaceMergeGeometry`
+из #229 работают на разных строках). Косвенное подтверждение: `npm test` даёт
+1019 тестов (было 989 на `a0878cd`, `dev` принёс +30 своих), все зелёные;
+регресс `smoke_wall_thickness_transition.mjs` (тоже смежный с геометрией стен)
+зелёный. Отдельно проверил, что смок `smoke_wall_chain_thickness.mjs` строит
+неколлинеарную цепочку (прямой угол между вторым и третьим отрезком), то есть
+автослияние коллинеарных перегородок (#229) в этом сценарии не участвует —
+это не создаёт риска для AC4, но и не является для них доказательством
+совместной работы; совместная работа проверена лишь тем, что полный набор
+юнит-тестов (включающий тесты #229) зелёный.
+
+## Закрытие раунда r1
+
+r1 (SHA `a0878cd`) — вердикт зелёный, единственная находка Low, снятая с
+записью, без обязательной правки.
+
+| Находка r1 | Чем закрыта | Где видно |
+|---|---|---|
+| Low: недостижимая ветка `if (cm == null) { … return; }` в `_markupClick`, дублирующая проверку `_canAppendRoomDraftPoint` | Не требовала правки в r1 (снята с записью: поведения не меняет, диапазон 1…100 закрыт UI). Ребейз этот код не тронул — строки идентичны | `src/houseplan-card.ts:7301-7308` (проверка `_canAppendRoomDraftPoint` на `:7170-7171`) — прочитано на `HEAD`, содержимое совпадает с описанием в комментарии r1 |
+
+Новых находок в этом заходе нет.
+
+## Унаследовано из r1
+
+Формально это полный разбор (см. раздел выше про причину), поэтому ничего не
+принято «на слово» — каждый пункт ниже перепроверен на текущем `HEAD`, а не
+скопирован из документа r1:
+
+- Диагноз шести точек и их перевод на `chainSegmentCms` — перечитан построчно
+  на `c8e9597`, совпадает с описанием r1 (`docs/reviews/CODE-REVIEW-234-r1.md`,
+  SHA `a0878cd`) и с ТЗ §3/§6.
+- AC1–AC7, AC9 — тесты и смоки, зелёные ранее и сейчас, прогнаны заново.
+- AC8 — тот же способ доказательства («прочитано, не исполнено»), что и в r1;
+  структура кода не изменилась.
+- Продуктовая рамка и touch-классификация (`not exposed`) — унаследованы из
+  ревью ТЗ (`docs/reviews/SPEC-REVIEW-234-r2.md`), рабочий код их не касается.
+
+Единственное, что действительно не перепроверялось повторно, — сам текст ТЗ:
+ребейз его не менял, а спек-ревью r1/r2 уже дало зелёный вердикт по контракту.
+
+## Находки
+
+Нет High. Нет Medium. Low из r1 — не новая, подтверждена присутствующей и
+безвредной (см. таблицу закрытия выше), новой правки не требует.
+
+## Что проверено и корректно
+
+- Единственный резолвер толщины, все шесть точек записи/чтения/подсветки на
+  нём, `wallChainSegments` без собственного fallback — контракт §6 ТЗ
+  реализован буквально.
+- Инвариант длины `_draftSegmentCms`/`_path` закреплён атомарной записью и
+  дозаполнением при чтении черновика.
+- Гейты: typecheck, полный набор юнит-тестов, сборка со сверкой трёх копий
+  бандла, целевой и оба регрессионных смока, точечные прогоны трёх новых
+  мутаций — все зелёные, и целевой смок проверен на способность падать лично.
+- Трейлеры и changelog (RU+EN) в том же коммите.
+- i18n не затронут (строки не менялись, `src/i18n/*.json` в диффе нет) —
+  соответствует §9 ТЗ.
+- Ребейз не создал конфликта с #229 на уровне поведения: полный прогон тестов
+  зелёный, затронутые участки не пересекаются.
+
+## Чего не проверял и почему
+
+- **`npm run golden:verify`** — не прогонял. Diff не меняет ни одной формулы
+  рендера (геометрия, цвет, слои), только источник значения толщины,
+  подаваемого в уже существующие формулы; то же обоснование, что в r1.
+- **`python -m pytest tests_backend`** — не прогонял, `custom_components/**/*.py`
+  не тронут.
+- **Полный прогон 127 смоков** — не прогонял, только целевой плюс два названных
+  в плане автотестов ТЗ (§12.4) регресса на смежных поверхностях (толщина при
+  рисовании, переход толщины при сплите). Остальные смоки не касаются
+  тронутых поверхностей (рисование цепочки, толщина, черновики).
+- **Полный/дорогой прогон `mutation-gate`** (весь набор мутаций, а не
+  структурный `--check` плюс три точечных) — не прогонял, это предрелизный
+  гейт (PROCESS.md §8), не гейт ревью.
+- **Perf-профили** — не прогонял: в AC они не названы, резолвер чистый и
+  вызывается на кадр рисования на массиве ограниченной длины (`MAX_DRAFT_POINTS`),
+  риск не следует из диффа.
+
+## Вывод
+
+Ребейз не изменил код задачи и не создал конфликта с #229. Все шесть точек
+резолвера, инвариант длины и AC1…AC9 проверены на текущем `HEAD` — не унаследованы
+слепо. Единственная находка r1 (Low) осталась на месте, безвредна, новой правки
+не требует. Новых находок нет.
+
+**Вердикт: зелёный.**