docs: review document for #313

Issue: #313
User-Visible: no

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

@@ -0,0 +1,293 @@
+# CODE-REVIEW-313-r1
+
+Issue: #313 · этап: код (code) · трек: `small` (лёгкий, лимит циклов код-ревью — 2) ·
+заход: r1 · блокирующих циклов израсходовано: 0 из 2 · SHA материала ревью: `5e5dad277cbdc238a5af899ff6727aed60d604b1`
+
+## Скоуп
+
+Ветка `issue/313-wallthick-standalone`, один коммит `5e5dad27` поверх `dev`.
+Конвейер привёл ветку к `dev` до ревью (§7.2/#257): поверх лёг 1 коммит `dev`
+(`docs: review document for #313`, `ffe0f0c7` — сам SPEC-REVIEW-313-r1, уже
+принятый и слитый). Это не смысловой ребейз продуктового кода — единственный
+родительский коммит между старым и новым состоянием документ-only, диапазон
+`origin/dev...HEAD` по-прежнему ровно один коммит `5e5dad27`. Разбор в любом
+случае полный: это первый заход код-ревью (`r1`), раздел «Унаследовано из
+r(N-1)» неприменим — предыдущего раунда код-ревью не было.
+
+Реализует ТЗ из тела issue #313 (light-трек, SPEC-REVIEW-313-r1 зелёный):
+инструмент «Толщина» резолвит/пишет также перегородки (`space.partitions`) и
+сегменты сохранённых черновиков (`space.room_drafts[]`), с приоритетом
+независимой кладки при точном наложении, без кнопки «на всю комнату» для неё,
+с отказом нуля/пусто для независимой кладки.
+
+Диф: `src/houseplan-card.ts` (+109/-…), новый смок
+`demo/smoke_wallthick_standalone.mjs`, второй патч мутанта
+`wall-thickness-writer-bypasses-common-barrier` в `scripts/mutation-gate.mjs`,
+оба CHANGELOG, `docs/USER-GUIDE.ru.md`, `docs/images/screenshots.json`
+(только fingerprint), три копии бандла.
+
+## Как проверялось
+
+Читал: `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (§1–§12), тело issue #313 и
+все 3 комментария (аналитика с тремя решениями владельца, зелёный вердикт
+SPEC-REVIEW, хендофф автора), `docs/WALL-THICKNESS.md` целиком (§1, §6, §9
+особенно), срез `docs/USER-GUIDE.ru.md` по «Толщине», `SPEC-REVIEW-313-r1.md`.
+
+Код читался на `HEAD @ 5e5dad27`, полный `git diff origin/dev...HEAD`. Ключевые
+утверждения не приняты на слово, а **исполнены**:
+
+### Гейты — прогнал
+
+| Гейт | Команда | Результат |
+|---|---|---|
+| typecheck | `npx tsc --noEmit` | зелёный, без вывода |
+| unit | `npm test` | 1335 pass / 1 skip / 0 fail (1336 total) — совпадает с хендоффом |
+| build + bundle sync | `npm run build && npm run bundle:sync` | `cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js` → совпадают; `cmp dist/houseplan-card.js demo/srv/assets/houseplan-card.js` → совпадают |
+| docs fingerprint | `node scripts/check-docs.mjs` (diff трогает `src/**`, гейт обязателен) | «Documentation checks passed (7 files, 10 external links)» |
+| выборка смоков | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | 25 «прямое совпадение»; из них прогнал 5 по существу диффа (ниже) |
+| `demo/smoke_wallthick_standalone.mjs` (новый, AC1–AC4) | `node demo/smoke_wallthick_standalone.mjs` | OK — **и подтверждено, что тест умеет падать**: собрал бандл с того же файла на `origin/dev` (`ffe0f0c7`, чистый worktree, свой `node_modules` симлинком) — без правки `_wallThickHit`/`_wallThickApply`/`_wallDialog.source` смок бросает необработанное исключение (`TypeError` в `_wallThickApply`), а не тихо проходит |
+| `demo/smoke_wall_thickness.mjs` (AC4, комнатные стены) | `node demo/smoke_wall_thickness.mjs` | OK, все под-проверки true |
+| `demo/smoke_wallthick_hover_width.mjs` (AC4) | `node demo/smoke_wallthick_hover_width.mjs` | OK |
+| `demo/smoke_resize_wall_thickness.mjs` (AC4) | `node demo/smoke_resize_wall_thickness.mjs` | OK |
+| `demo/smoke_optimize_coincident_partition.mjs` (тематически смежный с AC3 — #308-сценарий, два прямых совпадения символов `_wallDialog`/`_wallThickApply`, не «слабая связь») | `node demo/smoke_optimize_coincident_partition.mjs` | OK, все под-проверки true, включая `thicknessToolSelectsCanonicalWall`/`thicknessChangesSingleBody` |
+| мутация (точечно, не полный набор) | `node scripts/mutation-gate.mjs --id=wall-thickness-writer-bypasses-common-barrier` | «поймано 1 из 1» — но см. находку Medium ниже: результат обманчив |
+| инварианты модели | `npm run invariants -- --config test/fixtures/276-coincident-partition.json` | 1 нарушение — **предсуществующее и ожидаемое** (та же fixture #276/#308: перегородка поверх ребра комнаты; не следствие этого диффа — `plan-geometry-preflight.ts`/`near-axis.ts`/`coordinate-canonicalization.ts`, которые видит скрипт, диффом не тронуты). Не запускал отдельный export с реальной записью толщины независимой кладки: диф не меняет `wall_segments[]`/ключи/`open_spans`, пишет только `partition.cm`/`draft.segments[i].cm` тем же способом, что уже существующий `_savePhysicalDialog` (houseplan-card.ts:8287-8300, select-инструмент) — не новая форма мутации модели |
+
+Остальные 20 «прямых совпадений» от smoke-select — не прогонял: связь через
+широко переиспользуемые символы (`_activeDraftId`, `_geometrySnapshot`,
+`_showToast`, `wallIntervals`) без содержательного пересечения с изменённым
+кодом (`_wallThickHit`/`_wallThickApply`/`_wallDialog`/шаблон диалога); полный
+прогон — предрелизная обязанность, не гейт этого ревью.
+`golden`/`pytest tests_backend`/perf — не запускал: диф не меняет визуальный
+результат (нет новых компонентов рендера — существующая полоса hover и диалог
+переиспользуются как есть) и не трогает `custom_components/**/*.py` или
+performance-чувствительные пути; в AC они не названы.
+
+### AC — как доказаны
+
+- **AC1** (перегородка: hit → диалог → запись `partition.cm` → тело
+  перерисовывается → Undo). Доказано смоком `smoke_wallthick_standalone`
+  (hit/диалог/запись) плюс моей отдельной проверкой исполнением — Undo смок
+  **не тестирует** (комментарий в файле сам это признаёт: «server history is
+  async-free here» без единого вызова undo). Я воспроизвёл отдельно:
+  `_wallThickClick` → правка на 35 → `_wallThickApply` → `Control+z` → значение
+  вернулось к исходным 20, `_geometryHistory.size === 1` (один Undo, как того
+  требует контракт). AC1 закрыт.
+- **AC2** (сегмент драфта: диалог с cm сегмента, запись `draft.segments[i].cm`,
+  активный драфт не виден). Смоком подтверждена happy-path запись (12 → 18) и
+  исключение активного драфта — по коду (`draft.id === this._activeDraftId
+  continue`, houseplan-card.ts:11971-11973). **Не подтверждена находкой ниже** —
+  см. High: для `segments[i].cm === 0` диалог показывает не cm сегмента, а
+  выдуманное значение. Формально AC2 просит «диалог с cm сегмента» — для этого
+  конкретного, документами же допускаемого состояния контракт не выполнен.
+- **AC3** (приоритет наложения; ноль/пусто отказ). Смок подтверждает обе части
+  на #308-фикстуре (перегородка `cm:30` поверх ребра комнаты `left`) и отказ
+  нуля с сохранением значения и открытым диалогом. Дополнительно прогнал
+  `smoke_optimize_coincident_partition` — тот же топологический класс
+  («перегородка поверх ребра») с независимой (не игрушечной) фикстурой,
+  `thicknessToolSelectsCanonicalWall`/`thicknessChangesSingleBody` зелёные.
+  AC3 закрыт.
+- **AC4** (комнатные стены не регрессируют). Три названных смока +
+  `npm test` (1335/1335, включая ~200 юнитов #278/#282 wall-model-barrier) —
+  все зелёные, golden не переприёмывался (диф не меняет рендер-геометрию комнат
+  — только резолвер инструмента и рендер диалога). Код `wallIntervals(...)` в
+  комнатной ветке резолвера байтово не менялся (только обёрнут в `offer(...,
+  false)` — при отсутствии независимых кандидатов `offer` воспроизводит старое
+  сравнение `d < best.d` строго, тай-брейк по независимости не участвует). AC4
+  закрыт.
+
+## Находки
+
+### High — 1
+
+**Смешение «сегмент отсутствует» и «cm сегмента равен нулю» в резолвере
+драфтов; следствие — тихая порча данных при клике «Применить» без правки
+поля.**
+
+`src/houseplan-card.ts:11976`:
+```ts
+open: false, cm: Number(draft.segments[i]?.cm) || 15,
+```
+`RoomDraftCfg.segments[i].cm` — обязательное `number` (`src/types.ts:60`), 0 —
+легитимное для проекта значение того же самого поля: `docs/WALL-THICKNESS.md`
+§6 прямо документирует «Empty / 0 leaves the new wall thin» для сегментов
+черновика, и соседняя функция `validCm` (`src/houseplan-card.ts:13447-13448`,
+`value >= 0 ? value : null`) в этом же файле трактует 0 как валидное **текущее**
+значение того же поля при трассировке уже сохранённой независимой кладки —
+разработчик уже один раз учёл это же поле как «0 — не отсутствие», просто в
+другом методе. `||` в новом коде не различает «сегмента с таким индексом нет»
+(законная защита от рассинхрона `points`/`segments`) и «сегмент есть, cm = 0»
+— оба варианта схлопываются в `15`.
+
+**Воспроизведение (исполнением, не догадкой)** — синтетический конфиг,
+`page.evaluate` на собранном бандле HEAD (`demo/serve.mjs`):
+```js
+room_drafts: [{ id: 'thin-draft', points: [[0.6,0.5],[0.9,0.5]], segments: [{ cm: 0 }] }]
+// tool = 'wallthick'
+_wallThickHit([0.75*NORM_W, 0.5*NORM_W]).cm        // → 15  (истинное значение — 0)
+_wallThickClick(...)
+_wallDialog.value                                   // → "15" (должно быть "" — cmToField(0,…) === '')
+_wallThickApply(false)   // пользователь просто закрывает диалог, ничего не меняя
+serverCfg.spaces[0].room_drafts[0].segments[0].cm   // → 15  (было 0!)
+```
+Результат — ровно то, что напечатал скрипт:
+`{"hitCm":15,"trueStoredCm":0,"dialogValue":"15","cmAfterApplyWithoutEdit":15}`.
+
+**Сценарий отказа.** Черновик, нарисованный «тонким» (без толщины) — состояние,
+которое канон подсистемы называет существующим («Empty / 0 leaves the new wall
+thin»), не рисуется в редакторе как обычная стена по толщине, но геометрически
+существует. Владелец открывает «Толщину», наводит на этот сегмент, видит
+полосу подсветки шириной 15 см (реального 0 не видно — тоже следствие того же
+`hit.cm`), кликает, видит в поле «15», нажимает Enter/Apply, думая, что просто
+закрывает диалог или подтверждает увиденное — сегмент необратимо (без второго
+явного намерения) становится физической стеной 15 см. Это ровно класс «одно
+число — один источник» (PROCESS.md §8): подсветка инструмента, поле диалога и
+итоговая запись показывают одно и то же неверное число, полученное из одного
+неверного источника (`_wallThickHit`), а не три независимых.
+
+Партиционная ветка той же строки (`Number(partition.cm) || 0`, houseplan-card.ts:11963)
+не страдает: 0 — единственное значение, при котором фолбэк вообще участвует, и
+он совпадает с истинным (для `PartitionCfg` нет задокументированного «тонкого»
+состояния и нет пути создания `cm: 0`, в отличие от драфтов).
+
+**В скоупе ли находка.** Да: это тот же файл, тот же метод, что ТЗ описывает
+(«Диалог: для перегородки/драфта — cm сегмента»), правка — одна строка.
+
+### Medium — 1, в скоупе
+
+**Второй патч мутанта `wall-thickness-writer-bypasses-common-barrier` не
+проверяет то, что заявлено в коммите; заявление «краснота обоих проверена
+исполнением» не подтверждается.**
+
+Коммит `5e5dad27` добавляет второй `patches[]`-элемент в
+`scripts/mutation-gate.mjs:1221-1227`, целящий именно новую точку коммита
+(`_wallThickApply`, независимая кладка) — и заявляет в теле коммита: «Мутант
+расширен вторым патчем на новую точку коммита (#278-гвард), краснота обоих
+патчей проверена исполнением». Проверил раздельно, применяя патчи мутанта
+по одному к `src/houseplan-card.ts` (с восстановлением файла после каждой
+проверки, `git status` чист):
+
+- только 1-й патч (легаси-точка, `history.wall_thickness` в комнатной ветке)
+  применён → `node --test --test-name-pattern="production source routes
+  physical writers" test/wall-union-isolation.test.mjs` **красный** (как
+  ожидается);
+- только 2-й патч (новая точка, независимая кладка) применён, легаси-точка
+  цела → тот же тест **зелёный** — не ловит.
+
+Причина: гвард-тест (`test/wall-union-isolation.test.mjs:107`) — это
+регэксп-поиск текста `_commitPhysicalGeometry\(this\._t\('history\.wall_thickness'`
+по всему исходнику одной строкой (без `s`/multiline-режима на `.`), а не
+проверка конкретного места. Он совпадает **только** с легаси-вызовом
+(написан в одну строку), потому что новый вызов
+(houseplan-card.ts:12064-12066) обёрнут переносом строки сразу после
+`_commitPhysicalGeometry(` — до `this._t(...)` в исходнике перевод строки,
+через который движковый `.` без флага `s` не проходит. Поэтому при
+`--id=wall-thickness-writer-bypasses-common-barrier` (оба патча вместе,
+как в проверил и в реальном CI-запуске мутанта) тест краснеет **целиком за
+счёт первого патча**; второй патч можно откатить, ничего не изменится в
+результате — «поймано 1 из 1» в отчёте `mutation-gate.mjs` не отличает эти
+случаи.
+
+Проверил и поведенчески — не только по регэкспу, но и по факту: применил
+только 2-й патч, пересобрал бандл, прогнал сам новый
+`demo/smoke_wallthick_standalone.mjs` — **тоже зелёный**. Мутация
+(`_recordGeometry` вместо `_commitPhysicalGeometry`) не даёт `_saveConfig()`
+(houseplan-card.ts:7402, единственное место записи на бэкенд в
+`_commitPhysicalGeometry`) — смок читает мутированное поле прямо из
+`_serverCfg` (объект уже изменён присваиванием `partition.cm = cmRaw` до
+вызова коммита), поэтому не видит разницы между «записано с проверкой и
+уходом на сервер» и «переживено локально, история есть, на бэкенд ничего не
+ушло». Практическое следствие обхода барьера в этой ветке — не гипотетическое:
+`_commitPhysicalGeometry` пропускаетfail-closed валидацию геометрии
+(`_checkSpacePhysicalGeometry`) и обязательный `_saveConfig()`; будущий
+рефакторинг, случайно подставивший «облегчённый» путь записи именно в этой
+новой ветке, ни один из двух существующих гейтов не поймает.
+
+**В скоупе ли находка.** Да: и файл (`scripts/mutation-gate.mjs`, класс B, но
+через issue этой задачи — AGENTS.md таблица классов, «может использовать issue
+того изменения, которое покрывает»), и утверждение (текст коммита #313) —
+целиком продукт этой задачи.
+
+## Что проверено и корректно
+
+- Приоритет независимой кладки при точном наложении (`offer(...)`,
+  houseplan-card.ts:11940-11946): тай-брейк `independent && !best.independent
+  && d <= best.d + 1e-9` — корректно предпочитает независимую кладку только
+  при равном/лучшем расстоянии, не переопределяет явно более далёкий
+  кандидат. Порядок перебора (комнаты → перегородки → драфты) не создаёт
+  систематического смещения: тай-брейк явный, не полагается на порядок
+  вставки для двух разных источников.
+- `sp` (`_curSpaceCfg`, сырой конфиг) и `space` (`_spaceModel()`, отмасштабированная
+  проекция) — разные объекты, но `id` партиций/драфтов сохраняются 1:1 через
+  проекцию (`src/space-geometry.ts:167-180`); поиск по id в `_wallThickApply`
+  корректен, тот же паттерн, что уже использует `_wallThickClick`→`_wallDialog`
+  для комнатных стен и `_savePhysicalDialog` для партиций/драфтов
+  (houseplan-card.ts:8287-8300) — не новая, а переиспользованная связка.
+- Диалоговая кнопка «на всю комнату» скрывается корректно по
+  `d.source.kind === 'room'` (houseplan-card.ts:12636), без нового UX-контракта
+  — остальной диалог тот же компонент.
+- Валидация диапазона (1–100, отказ 0/пусто для независимой кладки) идёт
+  ДО ветвления по источнику и применяется к обеим веткам одинаково; после
+  ветвления для независимой кладки — отдельный явный отказ на 0/пусто,
+  соответствует решению владельца №2. Тост переиспользован
+  (`toast.physical_range`), новый ключ `toast.wallthick_set` уже существовал в
+  обоих `src/i18n/en.json`/`ru.json:535` до этого коммита (проверил — не
+  добавлен этим диффом, i18n не тронут, как и заявлено в аналитике «новых
+  ключей не требуется»).
+- Оба CHANGELOG (`docs/CHANGELOG.md`/`.ru.md`) правлены в том же коммите,
+  трейлеры `Issue: #313`/`User-Visible: yes` на месте — соответствует
+  AGENTS.md.
+- `docs/USER-GUIDE.ru.md` дополнен двумя строками таблицы («Значение», «Какие
+  стены») терминологией, уже принятой в этом документе (не изобретает новых
+  слов), корректно описывает и отказ нуля, и приоритет наложения.
+- `docs/images/screenshots.json`: изменился только `sourceFingerprint`/
+  `sourceSha256` (совпадает с текущим `src/**`, проверено `check-docs.mjs`),
+  `imageSha256` не тронуты — ни один из 10 задокументированных сценариев не
+  показывает Plan-редактор с инструментом «Толщина», пересъёмка не требовалась;
+  фингерпринт обязан был устареть при любой правке `src/**` и был обновлён
+  правильно.
+- Мутант `wall-thickness-writer-bypasses-common-barrier` при обоих патчах
+  вместе (как запускает CI) действительно ловит регресс — см. Medium выше,
+  почему это не то же самое, что «оба патча по отдельности эффективны».
+
+## Чего не проверял
+
+- Golden/скриншоты, `pytest tests_backend`, performance — не запускал: диф не
+  меняет визуальный рендер и не трогает Python/бэкенд; не названы в AC. Если
+  ошибаюсь и где-то в full/static рендере тело диалога/полосы отличается от
+  существующего компонента — это откроет полный прогон перед бетой.
+- Полный `smoke-select` список (остальные 20 из 25 «прямых совпадений») —
+  осознанно не прогонял, см. таблицу выше; риск считаю низким, но это решение,
+  а не факт.
+- Столбы (`space.wall_columns`) инструментом «Толщина» — вне ТЗ и вне этого
+  диффа, как и зафиксировал ревьюер ТЗ; не проверял, потому что не должен быть
+  затронут (не проверял чтением, что резолвер их не подхватывает — это уже не
+  добавлено кодом, а не «пропущенная проверка»).
+- Не проверял поведение при коллинеарной перегородке РЯДОМ (не точно на
+  ребре) с контуром комнаты, вне #308-фикстуры — тот же пробел, что явно
+  оставил открытым ревьюер ТЗ («это забота код-ревью... или находка для
+  отдельного цикла»); не нашёл в этом диффе признаков, что общий случай
+  (не точное наложение, а близкое соседство) обрабатывается иначе, чем раньше
+  — резолвер использует то же `distToSegment`/`pull`-радиус, что и для
+  комнатных стен исторически, поведение не новое для этого диффа. Оставляю
+  как открытый вопрос для будущей задачи, не завожу отдельный issue: это не
+  находка ПРОТИВ #313, а недоказанное свойство, унаследованное из старого
+  кода.
+- Не проверял `#306` (замена 0 на виртуальную стену) — вне скоупа, ТЗ прямо
+  говорит, что этой задаче нужен только шов (switch по источнику), а не
+  реализация; шов на месте (houseplan-card.ts:12052-12056, единственная точка
+  ветвления по `d.source.kind`).
+
+## Вердикт
+
+Красный. Единственная High-находка — конкретный, воспроизведённый исполнением
+дефект в коде, который сама задача добавляет (`_wallThickHit`, драфт-ветка):
+диалог «Толщины» лжёт о текущей толщине сегмента черновика, когда она равна 0
+(легитимное, задокументированное в `docs/WALL-THICKNESS.md` §6 состояние), и
+безusловный клик «Применить» без правки поля молча превращает такой сегмент из
+0 в 15 см. Это прямое нарушение AC2 («диалог с cm сегмента») для конкретного
+достижимого состояния модели, а не гипотетическое. Medium-находка (обманчивая
+мутационная защита новой точки коммита) в скоупе той же задачи, чинится там же.
+
+Обе находки локальны (одна строка кода на High; тестовый патч + при
+необходимости сам тест на Medium) — переписывания ТЗ/скоупа не требуют.