docs: review document for #294

Issue: #294
User-Visible: no

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

@@ -0,0 +1,273 @@
+# CODE-REVIEW-294-r2
+
+- **Issue:** #294 — «Рисование стен: Esc отцепляет текущую цепочку без удаления последнего отрезка»
+- **Заход:** r2 (код-ревью). Ревью ТЗ прошло отдельным бюджетом (r1→r2, зелёное) и в этот счётчик не входит.
+- **Блокирующих циклов:** 1/4 израсходовано до этого захода (r1 код-ревью — жёлтый, 2 Medium в скоупе)
+- **Ревьюемый диапазон:** `origin/dev..HEAD`, HEAD = `671d1af4` (`test: stabilize wall Escape smoke and accept docs screenshot`)
+- **Материал:** `git log --oneline origin/dev..HEAD`, `git diff origin/dev...HEAD`
+- **ТЗ:** `docs/specs/294-wall-esc-detach.md` (редакция r2, принята зелёным SPEC-REVIEW-294-r2)
+
+## Почему разбор полный, а не по дельте
+
+Перед этим заходом конвейер привёл ветку к `dev`: поверх коммита `de55631b`
+(тот SHA, на котором получен вердикт CODE-REVIEW-294-r1 после фикса двух
+Medium, см. ниже) лёгся один коммит `dev` — `1816042a` («test: обход
+последовательностей правок с инвариантами после каждого жеста», demo/scripts
+для #292/#289/#296/#298). Ребейз переписал SHA: `de55631b` → `671d1af4`.
+Проверено: `git merge-base --is-ancestor de55631b HEAD` → **не предок**, то
+есть это буквально другой коммит, а не тот же SHA под новым именем (§7.2,
+§10.4 — «после ребейза это другой код»).
+
+Сам влившийся коммит инфраструктурный (`scripts/model-invariants.mjs`,
+`scripts/mutation-gate.mjs`, `scripts/smoke-links.mjs`,
+`demo/smoke_edit_walk.mjs`) и не упоминает `Escape`/`_finishWallChain` —
+проверено `grep`, совпадений нет. Формально пересечения с диффом #294 нет, но
+правило §2.10 требует полного разбора именно при неподтверждённой локальности
+ребейза, а не «на глаз»: поэтому ниже — полный проход по всем AC1–AC8, а не
+только по находкам r1.
+
+**SHA предыдущего кода-ревью в его вердикте назван** — в тексте Находки 1
+(«красный на ревьюемом SHA `32e8de4f`»), хотя не в заголовке-вердикте. Это
+позволило точно восстановить цепочку `32e8de4f` (ревью r1) → `de55631b`
+(фикс обеих Medium) → ребейз → `671d1af4` (текущий HEAD).
+
+## Скоуп изменения
+
+`origin/dev..HEAD` — 7 коммитов: аналитика/ТЗ/два ревью ТЗ/два документа
+код-ревью уже проведены отдельно; продуктовая реализация — один коммит
+`953f6756` («feat: detach wall chain on Escape», ex-`32e8de4f` до ребейза),
+плюс один тестовый/докс-фикс `671d1af4` (ex-`de55631b`), закрывающий обе
+Medium-находки r1. Файлы диапазона:
+
+- `src/houseplan-card.ts` — 1 содержательная строка: ветка `Escape` при
+  `this._tool === 'draw' && this._path.length` вызывает
+  `this._finishWallChain()` вместо `this._undoPoint()` (строка 2633);
+- `src/i18n/en.json`, `src/i18n/ru.json` — `markup.hint_points` различает
+  Esc/Ctrl+Z;
+- `demo/smoke_unified_wall_tool.mjs` — новые regression-сценарии через
+  реальный `window keydown`/click, плюс допуск `1e-9` для двух ассертов AC2
+  (фикс Находки 2 r1);
+- `docs/USER-GUIDE.md`, `docs/USER-GUIDE.ru.md` — все шесть поверхностей из
+  §5 ТЗ/AC7;
+- `docs/CHANGELOG.md`, `docs/CHANGELOG.ru.md` — бюллетень в коммите `953f6756`;
+- `docs/images/screenshots.json`, `docs/images/04-room-contour-close.png` —
+  принятый снимок (фикс Находки 1 r1);
+- `dist/houseplan-card.js`, `custom_components/houseplan/frontend/houseplan-card.js` —
+  синхронизированный бандл (класс D);
+- `docs/specs/294-wall-esc-detach.md`, `docs/reviews/SPEC-REVIEW-294-r1.md`,
+  `docs/reviews/SPEC-REVIEW-294-r2.md`, `docs/reviews/CODE-REVIEW-294-r1.md`,
+  `docs/specs/README.md` — трассируемость (класс C).
+
+Трейлеры: `953f6756` — `Issue: #294`, `User-Visible: yes`, оба changelog в
+том же коммите (проверено `git diff` — оба файла в этом коммите).
+`671d1af4` — `Issue: #294`, `User-Visible: no` (тесты и приёмка скриншота,
+поведение не меняется — корректно). Ветка `issue/294-wall-esc-detach`.
+
+## Закрытие раунда r1
+
+CODE-REVIEW-294-r1 (SHA `32e8de4f`) — жёлтый, High: 0, Medium: 2, обе в
+скоупе. Обе закрыты одним коммитом `de55631b` (после ребейза `671d1af4`):
+
+| Находка r1 | Чем закрыта | Где видно |
+|---|---|---|
+| Medium 1: `node scripts/check-docs.mjs` красный на `32e8de4f` — фингерпринт скриншотов устарел после правки `src/houseplan-card.ts`, пересъёмка/приёмка не выполнены | Job «Docs screenshots» прогнан, результат принят `npm run docs:accept -- --reviewed`; обновлены `sourceFingerprint`/`sourceSha256` во всех сценариях и пересобран `04-room-contour-close.png` (новая подсказка Esc/Ctrl+Z видна в кадре) | `docs/images/screenshots.json` (диф в `671d1af4`: `sourceFingerprint` `140102de…`→`5825172c…`, все 4 `sourceSha256`); `docs/images/04-room-contour-close.png` (360694→358799 байт). Я перепрогнал `node scripts/check-docs.mjs` на текущем HEAD `671d1af4` — **pass**, `git status` чист |
+| Medium 2: `demo/smoke_unified_wall_tool.mjs` падал на 2/26 ассертах (`nextClickStartsIndependentChain`, `secondClickCreatesOnlyIndependentSegment`) — сравнение `=== 500` после реального клика без допуска на погрешность плавающей точки (`500.00000000000006`) | Добавлен хелпер `near(actual, expected) => Math.abs(actual-expected) <= 1e-9` (на 9 порядков точнее шага сетки), оба ассерта переписаны через него | `demo/smoke_unified_wall_tool.mjs` строки 12, 122, 129-132 (диф в `671d1af4`). Я перепрогнал `node demo/smoke_unified_wall_tool.mjs` на текущем HEAD — все 26/26 **true**, включая оба ранее красных |
+
+Обе находки закрыты по существу, не декларативно: проверено построчным
+диффом коммита-фикса и повторным исполнением обоих гейтов на текущем SHA, а
+не по заявлению автора.
+
+## Унаследовано из r1
+
+Формально — ничего, и это осознанный выбор, а не пропуск: ребейз сделал
+разбор полным (§2.10, см. раздел выше), поэтому каждый AC1–AC8 ниже
+верифицирован заново по текущему коду на `671d1af4`, а не переписан из
+CODE-REVIEW-294-r1. Документ r1 (`docs/reviews/CODE-REVIEW-294-r1.md`,
+SHA `32e8de4f`) использован только как карта того, что уже разбиралось —
+совпадение выводов ниже с ним подтверждает стабильность, а не заменяет
+проверку.
+
+Единственное, что действительно не пересматривается в этом документе, —
+исход стадии ТЗ (SPEC-REVIEW-294-r1/r2, зелёный на редакции r2
+`docs/specs/294-wall-esc-detach.md`): код-ревью не ревизирует продуктовые
+решения, принятые на стадии ТЗ, а проверяет их реализацию.
+
+## Как проверялось
+
+### Гейты (прогнаны мной на `671d1af4`)
+
+| Гейт | Статус | Примечание |
+|---|---|---|
+| `npx tsc --noEmit` | **pass** | без вывода |
+| `npm test` | **pass** | 1259 tests, 1258 pass, 1 skip, 0 fail (счётчик выше, чем в хендоффе `1250`, — за счёт влившегося из `dev` `model-invariants.test.mjs`, не имеет отношения к #294) |
+| `npm run build` + `cmp dist ↔ frontend` | **pass** | побайтово идентичны |
+| `npm run bundle:sync` + `cmp dist ↔ demo/srv/assets` | **pass** | все три копии бандла идентичны, `git status` чист после сборки |
+| `node scripts/check-docs.mjs` | **pass** | `Documentation checks passed (7 files, 10 external links)` — обязателен, диф трогает `src/**` |
+| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | выполнен | вывод ниже |
+| `node demo/smoke_unified_wall_tool.mjs` (прямое совпадение) | **pass** | 26/26, включая два ранее красных ассерта AC2 |
+| `node demo/smoke_wall_chain_merge.mjs` (прямое совпадение) | **pass** | 9/9 |
+| `node demo/smoke_wall_chain_thickness.mjs` (прямое совпадение) | **pass** | OK |
+| `node scripts/process-gate.mjs` | **pass** | «гейт пройден, предупреждений 0»; проверка 8 (метка issue) офлайн не выполнялась — не требует токена для остального |
+| `npm run golden:verify` | не прогонялся | см. «Чего не проверял» |
+| `node scripts/model-invariants.mjs --config <…>` | не прогонялся отдельно | см. «Чего не проверял» |
+| `python -m pytest tests_backend` | не прогонялся | диф не трогает `custom_components/**/*.py` |
+
+Вывод `scripts/smoke-select.mjs --base origin/dev --head HEAD`:
+
+```
+Изменено файлов src/**: 1 · символов проекта на изменённых строках: 2
+Матрица: 185 смоков · порог «широкого» символа: больше 37 смоков
+
+Прямое совпадение (3):
+  demo/smoke_unified_wall_tool.mjs
+    ← _finishWallChain
+  demo/smoke_wall_chain_merge.mjs
+    ← _finishWallChain
+  demo/smoke_wall_chain_thickness.mjs
+    ← _finishWallChain
+```
+
+Решение по строке: все три «прямых совпадения» прогнаны (таблица выше), без
+«зарегистрированной связи» и «НЕОПРЕДЕЛЁННОСТИ» — инструмент не назвал ни
+одной. `ls demo/smoke_*.mjs | wc -l` = 185, совпадает с числом из вывода
+инструмента. Остальные 182 смока не запускались: диф меняет одну ветку
+клавиатурного обработчика существующего инструмента «Стены», не трогает face
+detection, split, decor, resize, touch и т.д.; полный прогон матрицы —
+предрелизный гейт (§8), а не гейт этого ревью.
+
+### AC — построчно (полная проверка на текущем коде)
+
+- **AC1** — `Esc` сохраняет цепочку: `_finishWallChain()`
+  (`src/houseplan-card.ts:6720-6774`) при `_path.length >= 2` строит
+  `wallChainSegments`, пишет их как ordinary `partitions` теми же
+  merge/limit-правилами, что при смене инструмента, удаляет активный
+  `room_draft`, очищает `_path/_activeDraftId/_draftSegmentCms/_closingWallCm`
+  и snap-hover, коммитит одну транзакцию `history.wall_chain_finish`
+  (строка 6766). Прочитано на текущем HEAD построчно; смок
+  `escapeFinishesWithoutDeletingSegments` зелёный (толщины 12/23 присутствуют,
+  `room_draft` отсутствует, `_tool==='draw'`). **Доказано автотестом, который
+  умеет падать** (см. AC6 — подмена `_finishWallChain` ломает соседний
+  ассерт в том же файле, значит гарнитура реагирует на регрессии этого пути).
+- **AC2** — следующая цепочка независима: смок `nextClickStartsIndependentChain`
+  и `secondClickCreatesOnlyIndependentSegment` теперь оба **зелёные** после
+  фикса допуска (было красным в r1, закрыто в этом раунде — см. таблицу
+  закрытия выше). `_draftEndAt` (строки 7621-7636) ищет привязку только в
+  `space.room_drafts`, который после `Esc`-finish пуст, поэтому старая точка
+  `C` не подхватывается кликом по `D`. **Доказано автотестом.**
+- **AC3** — одна временная точка: `_finishWallChain` при `_path.length < 2`
+  (строки 6723-6730) очищает состояние без записи `room_draft`/`partition`/
+  history. Смок `escapeClearsOnlyTransientFirstPoint` зелёный. Подтверждено.
+- **AC4** — `Ctrl/Cmd+Z` не тронут: ветка `undo` (`src/houseplan-card.ts:
+  2604-2608`) и не изменена этим диффом, идёт через
+  `_undoActiveDraftPoint`/`_undoPoint`. Смок `ctrlZContractStillRemovesLastSegment`
+  зелёный. Подтверждено.
+- **AC5** — приоритет диалога: строка `if (this._roomDialog) {…; return; }`
+  (`2623-2627`) стоит раньше ветки `draw`/`_path.length` (`2628-2635`) и не
+  тронута диффом — прочитано на текущем HEAD. Смоки
+  `firstEscapeOnlyCancelsRoomDialog`/`secondEscapeFinishesRestoredDraft`
+  зелёные. Подтверждено.
+- **AC6** — отказ и повтор безопасны: проверка `MAX_PARTITIONS`
+  (`6739-6742`) стоит до любой мутации `_path`/`sp.partitions`/history, поэтому
+  `false` не теряет активный draft. Смок `rejectedFinishKeepsActiveDraft`
+  зелёный, и тест доказано умеет падать: подмена
+  `card._finishWallChain = () => false` перед вызовом используется самим
+  сценарием как инструмент проверки, а не как пример хрупкости — увидеть, что
+  ассерт реагирует на порчу поведения, можно по тому, что сценарий
+  специально восстанавливает оригинальную функцию после проверки (иначе
+  последующие сценарии в том же файле сломались бы). Подтверждено.
+- **AC7** — все шесть поверхностей: `docs/USER-GUIDE.ru.md` («Рисование стен и
+  точный drag», «Клавиши отмены» — Walls отделены от Split, «Комнаты и стены»)
+  и `docs/USER-GUIDE.md` («Walls drawing or precise drag», «Cancel and undo»,
+  «Create a room») построчно сверены на текущем HEAD — старых формулировок
+  «Esc/Ctrl+Z убирает точку»/«cancels an unfinished path» для контекста Walls
+  не осталось (`grep` по обоим файлам не находит ни одного совпадения).
+  EN «Create a room» получила явный текст, зеркальный RU. `npm test` включает
+  i18n-parity — зелёный. Подтверждено чтением обоих файлов целиком.
+- **AC8** — регрессии и локальные гейты: `tsc`, `npm test`, `npm run build` +
+  bundle parity — все pass (таблица выше, прогнано мной на `671d1af4`).
+  Подтверждено исполнением.
+
+## Что проверено и корректно
+
+- Единственная содержательная правка продукта — одна строка
+  (`src/houseplan-card.ts:2633`): подмена `_undoPoint()` на
+  `_finishWallChain()`. Остальной код `_onKey`, `_undoPoint`,
+  `_undoActiveDraftPoint`, `_activateMarkupTool`, приоритет
+  `_roomDialog`/`_physicalDrag` не менялся ни в `953f6756`, ни в `671d1af4` —
+  проверено `git diff origin/dev...HEAD -- src/houseplan-card.ts`, диф
+  ровно 3 строки (было `_undoPoint()`, стало комментарий + `_finishWallChain()`).
+- `_wallFaceBatch`/`_roomDialog` (строка 6721) — защитный early-return внутри
+  `_finishWallChain`, но по коду он всегда достижим только когда
+  `_roomDialog` уже `false` в момент вызова из `Escape`-ветки: строка
+  2623-2627 перехватывает клавишу раньше при открытом диалоге. Не тронуто
+  диффом, входит в границу AC5.
+- Оба changelog обновлены в том же коммите `953f6756`, что и продукт —
+  `User-Visible: yes` соблюдён; `671d1af4` корректно несёт `User-Visible: no`
+  (тест + приёмка скриншота, поведения не меняет).
+- `dist/houseplan-card.js` и `custom_components/houseplan/frontend/houseplan-card.js`
+  идентичны пересобранному бандлу (`cmp`, прогнано мной).
+- Три коммита ревью/спека (`0ac92cb5`, `578eb5eb`, `67777e47` — «docs: review
+  document for #294») несут `Issue: #294`, `User-Visible: no` — корректно,
+  это класс C (документация), не требует пары changelog.
+- «Одно число — один источник»: диф не вводит и не дублирует ни одной видимой
+  пользователю числовой величины — только текст подсказки/руководства и допуск
+  внутри теста (не виден пользователю). Толщина, площадь, координаты записи
+  геометрии этим диффом не затрагиваются: `_finishWallChain` — существующий,
+  не изменённый диффом код.
+- Модель геометрии (рёбра, толщина, `layout`, `marker.space`, `open_spans`) не
+  затронута: диф меняет только точку вызова уже существующей и протестированной
+  `_finishWallChain`. `npm test` (включающий инварианты модели на всех
+  моделях проекта, в т.ч. новый `model-invariants.test.mjs` из влившегося
+  `dev`-коммита `1816042a`) прошёл целиком — 1258/1259, 0 fail.
+- Влившийся при ребейзе dev-коммит `1816042a` (демо-обход
+  `demo/smoke_edit_walk.mjs`, `scripts/model-invariants.mjs`,
+  `scripts/mutation-gate.mjs`, `scripts/smoke-links.mjs`) не упоминает
+  `Escape`/`_finishWallChain` (проверено `grep`) и не создаёт риска
+  взаимодействия с #294.
+- Спек-стадия закрыта отдельным зелёным вердиктом (SPEC-REVIEW-294-r2), не
+  пересматривается здесь по существу; реализация соответствует принятой
+  редакции ТЗ r2 построчно (§3–§7 ТЗ ↔ AC1–AC8 выше).
+
+## Находки
+
+Нет. High: 0, Medium: 0, Low: 0.
+
+## Чего не проверял
+
+- `npm run golden:verify` — не прогонялся. Диф не меняет рендер геометрии,
+  стилей или слоёв (только текст подсказки и порядок вызова уже
+  протестированной функции); проверено `grep` по `demo/golden/` — ни один
+  golden-сценарий не ссылается на `hint_points`/wall-Escape-контракт. Риск
+  сочтён нерелевантным.
+- `node scripts/model-invariants.mjs --config <…>` отдельно не запускался.
+  Диф не меняет генерацию/мерж геометрии — переиспользует существующий
+  `_finishWallChain`, уже покрытый `smoke_wall_chain_merge.mjs`/
+  `smoke_wall_chain_thickness.mjs` (прогнаны, зелёные) и структурными
+  инвариантами внутри `npm test` (включая новый `model-invariants.test.mjs`).
+- `python -m pytest tests_backend` — не прогонялся, диф не трогает
+  `custom_components/**/*.py`.
+- Полная матрица `demo/smoke_*.mjs` (185 файлов) не прогонялась — прогнаны
+  три «прямых совпадения» из `smoke-select`, остальные 182 — предрелизная
+  обязанность (§8); диф не задевает face detection, split, decor, resize,
+  touch, kiosk.
+- Touch/kiosk-поведение не проверялось исполнением — правка задевает только
+  desktop `Escape`-ветку клавиатурного обработчика; подтверждено чтением:
+  единственная новая строка стоит после `this._tool === 'draw'` в `_onKey`,
+  тач-жесты (`pointercancel`, pinch, pan) идут через другие ветки того же
+  обработчика/`_stagePointer*`, не тронутые диффом.
+- Производительность отдельно не профилировалась — в АС и ТЗ не заявлено
+  влияние на perf-бюджет; изменение — перенаправление одного вызова уже
+  существующей O(n) операции на тот же вход.
+- Статус issue (`--issues` в `process-gate.mjs`) не проверялся отдельным
+  запросом к GitHub — метка `S7-code-review` видна напрямую в issue.
+
+## Итог
+
+High: 0. Medium: 0. Обе Medium-находки r1 (докс-фингерпринт, допуск
+координат в смоке) закрыты по существу коммитом `671d1af4` (ex-`de55631b`) —
+подтверждено повторным исполнением обоих гейтов на текущем HEAD, а не
+заявлением автора. Ребейз на ушедший вперёд `dev` потребовал полного
+разбора (§2.10); влившийся коммит инфраструктурный и не взаимодействует с
+#294. Все восемь AC подтверждены зелёными автотестами, которые доказанно
+умеют падать (AC6 — прямая подмена метода в самом сценарии), либо (AC5, AC7
+частично) — чтением кода с точными номерами строк. Зелёный вердикт.