docs: review document for #173

Issue: #173
User-Visible: no

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

@@ -0,0 +1,220 @@
+# CODE-REVIEW-173-r2
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/173
+- **Диапазон:** `origin/dev..HEAD`, 5 коммитов: `8a25b52` (ТЗ), `e206e87`
+  (ревью ТЗ), `fc1e795` `feat: unify Plan wall drawing` (единственный коммит
+  класса A/B), `aac2978` (документ `CODE-REVIEW-173-r1.md`), `f54b9c0`
+  (`docs: refresh screenshot fingerprint for #173`)
+- **Роль:** ревьюер кода (не автор), этап `S7-code-review`, сессия без
+  контекста реализации
+- **Цикл:** r2/4 — **не по находкам**. r1 (`docs/reviews/CODE-REVIEW-173-r1.md`)
+  дал зелёный вердикт с High:0, Medium:2 (обе вынесены отдельными issue,
+  #176/#177, не блокировали); слияние в `dev` конфликтовало с параллельно
+  влившимся #174, задача вернулась в `S6-in-progress` для ребейза, а не для
+  правок по находкам ревью (комментарий владельца в issue). После ребейза на
+  актуальный `dev` (`1f11f8f`, объединяющий #173 и #174) и docs-only коммита,
+  чинящего screenshot-fingerprint, издана `S7-code-review` заново. Это другой
+  код (переигранный поверх ушедшего вперёд `dev`), поэтому повторная проверка
+  не формальность.
+
+## Скоуп ревью
+
+`fc1e795` — тот же продуктовый диф, что и в r1 (`f931159` до ребейза): 28
+файлов, `+3304/-1926`, идентичный набор файлов и идентичные суммы
+insertions/deletions. Это сильный признак того, что при ребейзе патч
+`src/wall-face-graph.ts` / `src/houseplan-card.ts` / тестов / smoke применился
+без текстового конфликта — конфликтовали только файлы, которые параллельно
+менял #174 (`docs/specs/README.md`, оба `CHANGELOG*`, три копии бандла).
+Отдельно проверено (см. ниже), что слияние в этих файлах корректно сохранило
+записи обоих issue.
+
+Новое в r2 относительно r1:
+
+- `fc1e795` перебазирован на `dev`, включающий #174 (`S8-merged`);
+- `f54b9c0` — docs-only (`docs/images/*.png`, `docs/images/screenshots.json`),
+  обновляет `sourceFingerprint` после объединения #173+#174 в общий `src/`.
+
+Не в скоупе изменений (не тронуто ни в `fc1e795`, ни в `f54b9c0`):
+`custom_components/houseplan/**/*.py`, backend schema, `demo/golden/**`.
+
+## Что именно проверялось в r2 (сверх r1)
+
+Поскольку продуктовый код (`fc1e795`) идентичен по diffstat уже
+провалидированному в r1, r2 сфокусирован на: (а) действительно ли текущее
+дерево green на гейтах, а не только по отчёту в issue; (б) не испортило ли
+слияние с #174 общие файлы; (в) легитимность нового docs-only коммита; (г)
+что Medium-находки r1 не потерялись и не были тихо «закрыты» вместо issue.
+Алгоритмический разбор `wall-face-graph.ts`, построчная сверка AC1–AC17 по
+существу и мутационная проверка «тест умеет падать» не повторялись заново
+пофайлово — они зафиксированы в `docs/reviews/CODE-REVIEW-173-r1.md` и не
+имеют оснований измениться, так как сам продуктовый диф не изменился.
+
+## Как проверялось — гейты
+
+| Гейт | Команда | Результат |
+|---|---|---|
+| typecheck | `npx tsc --noEmit` | pass, без ошибок |
+| unit | `npm test` | **848/848 pass** (после слияния с #174 набор больше r1-шного 843/843 на 5 тестов #174 — ожидаемо) |
+| build + sync бандла | `npm run build && cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js && cmp dist/houseplan-card.js demo/srv/assets/houseplan-card.js` | pass, три копии побайтно идентичны, SHA-256 `00391dde7211db685c6c3c83c06b53808fb9a8a6a6c6c6b91b8346c5b27fc1ea` — совпадает с числом, которое владелец привёл в хендоффе ребейза |
+| целевой smoke (AC1–AC13) | `node demo/smoke_unified_wall_tool.mjs` | **19/19 pass** |
+| регрессия #138/AC7 | `node demo/smoke_room_autoclose.mjs` | 9/9 pass |
+| регрессия толщины (AC1/AC12) | `node demo/smoke_draw_wall_thickness.mjs` | 11/11 pass |
+| touch safety floor (AC3/AC16) | `node demo/smoke_editor_gestures.mjs` | 5/5 pass |
+| performance (AC15) | `npm run benchmark:large-house-plan-snap` | pass, без брошенных contract-ошибок; `wallFaceAcceptedClickMs` ≈ 2.8–4.3 мс (порог 1000 мс); `wallFaceGraph` cache entries = 2, growth = 0 — совпадает с r1 |
+| process-gate (локально) | `node scripts/process-gate.mjs` | «гейт пройден, предупреждений 0», диапазон `origin/dev..HEAD`, 5 коммитов |
+| docs freshness/links | `node scripts/check-docs.mjs --external` | «Documentation checks passed (7 files, 10 external links)» — совпадает с отчётом владельца; **подтверждает**, что `sourceFingerprint` в `docs/images/screenshots.json` после `f54b9c0` реально соответствует текущему `src/` (`check-docs.mjs:132` сравнивает `manifest.sourceFingerprint` с живым `sourceFingerprint(ROOT)`, а не просто наличие поля) |
+| merge-корректность shared-файлов | чтение `docs/specs/README.md`, `docs/CHANGELOG.md`, `docs/CHANGELOG.ru.md` | обе записи (#173, #174) присутствуют, не дублированы, не потеряны; `grep -rn '^<<<<<<<\|^=======$\|^>>>>>>>'` по репозиторию — 0 совпадений, конфликтных маркеров не осталось |
+
+**Не прогонялось и почему:**
+
+- `npm run golden:verify` — diff (`fc1e795`, `f54b9c0`) не содержит изменений
+  `demo/golden/**`; ни один baseline не принимался. AC12 остаётся проверенным
+  чтением кода, как в r1 (переиспользование немодифицированных canonical
+  хелперов рендера) — предрелизный гейт, не гейт код-ревью.
+- `python -m pytest tests_backend -q` — ни один файл
+  `custom_components/houseplan/**/*.py` не тронут ни в `fc1e795`, ни в
+  `f54b9c0` (`git diff --stat origin/dev...HEAD`).
+- Полный набор из 138+ browser-smoke — как и в r1, тронута ровно одна
+  поверхность (Plan editor); прогнаны целевой smoke плюс три соседних по
+  diff-риску, совпадающих с r1. Продуктовый код не изменился с r1, поэтому
+  расширять выборку в r2 нет причины, вытекающей из diff.
+- Алгоритмическая мутационная проверка «тест умеет падать» (снятие
+  `consumed.has(atom.key)` и т.п.) — не повторялась: код `wall-face-graph.ts`
+  и `_applyWallFaceBatch` в `fc1e795` идентичен по diffstat уже
+  промутированному в r1 коду; повторный прогон того же эксперимента на том же
+  коде не добавляет доказательной силы, только тратит цикл.
+- `npm run benchmark:compare` против baseline SHA — как и в r1, сохранённого
+  отчёта базового SHA нет в этой сессии; сырые цифры сверены вручную с
+  `budgets-large-house-plan-snap.json`, ни один бюджет не ослаблен.
+
+## Проверка AC1–AC17
+
+Все 17 AC остаются подтверждёнными по существу анализа r1
+(`docs/reviews/CODE-REVIEW-173-r1.md`, раздел «Проверка AC1–AC17») — продуктовый
+код не изменился. В r2 дополнительно подтверждено исполнением:
+
+- AC1, AC2, AC3, AC7, AC10, AC11, AC16 — повторным зелёным прогоном целевого и
+  трёх соседних smoke на актуальном дереве (см. таблицу гейтов);
+- AC15 — повторным прогоном `benchmark:large-house-plan-snap` на актуальном
+  дереве, числа совпадают по порядку величины с r1;
+- AC14 — `git diff --stat origin/dev...HEAD` по новому диапазону подтверждает
+  отсутствие Python/schema изменений и в `fc1e795`, и в `f54b9c0`;
+- AC17 — typecheck/unit/build зелёные на актуальном дереве, три копии бандла
+  идентичны, `check-docs.mjs` подтверждает свежесть screenshot-fingerprint
+  (то, чего не хватало непосредственно после ребейза и что чинит `f54b9c0`).
+
+AC4–AC6, AC8, AC9, AC12, AC13 не переисполнялись отдельно в r2 (unit-suite
+`wall-face-graph.test.mjs` зелёный в общем прогоне 848/848, код не менялся) —
+проверено тем, что `npm test` покрывает их без регрессии, и явной пометкой,
+что построчный разбор не повторялся, так как нет предмета для повторного
+разбора (diff идентичен).
+
+## Находки
+
+Находок уровня **High** нет. Новых находок уровня **Medium** нет.
+
+Обе Medium-находки r1 остаются в силе, не блокируют и уже вынесены отдельными
+issue — проверено, что они не потерялись при ребейзе и не были закрыты
+задним числом:
+
+- [#176](https://github.com/Matysh/houseplan-card/issues/176) (`S1-new`,
+  `tech-debt`, `P3`) — мёртвый код старого инструмента `partition` не удалён.
+  Перепроверено: `MARKUP_TOOLS` (строка 532, было 533 — сдвиг на строку из-за
+  не связанной с #173 правки, не регрессия), `_partitionClick` определён на
+  6943 и вызывается на 6644, `'partition'` встречается 51 раз в
+  `src/houseplan-card.ts` — состояние не изменилось между r1 и r2.
+- [#177](https://github.com/Matysh/houseplan-card/issues/177) (`S1-new`,
+  `tests`, `tech-debt`, `P3`) — AC8 не имеет unit/smoke-доказательства именно
+  для новой интеграции `_offerWallFaces`/`_applyWallFaceBatch`. Код,
+  реализующий AC8, не изменился с r1, находка не переоткрывается заново.
+
+Обе Low-находки r1 (`if/else` без скобок в `_activateMarkupTool`;
+`wallFaceAcceptedClickMs` не входит в отслеживаемые performance-бюджеты)
+остаются как записано в r1 — не блокируют, решение прежнего ревьюера в силе,
+код не изменился.
+
+### Проверка самого ребейза — отдельный предмет r2
+
+Единственный содержательно новый риск этого цикла — не в продуктовом коде, а
+в слиянии с параллельной веткой #174. Проверено:
+
+1. **Нет дублирования/потери записей.** `docs/specs/README.md` содержит
+   ровно по одной строке на #173 и #174; оба `CHANGELOG.md`/`CHANGELOG.ru.md`
+   содержат ровно по одной записи `Unreleased` на каждый issue, без
+   дублирующихся или осиротевших абзацев.
+2. **Нет маркеров конфликта.** `grep` по всему репозиторию на
+   `<<<<<<<`/`=======`/`>>>>>>>` — 0 совпадений.
+3. **Бандл пересобран из объединённого дерева, а не унаследован от старой
+   ветки.** Собственная пересборка (`npm run build`) даёт три побайтно
+   идентичные копии с тем же SHA-256, который владелец указал в хендоффе
+   ребейза — то есть закоммиченные копии не устарели относительно текущего
+   `src/`.
+4. **`docs/images/screenshots.json` не рассинхронизирован.**
+   `scripts/check-docs.mjs` явно сравнивает закоммиченный `sourceFingerprint`
+   с фингерпринтом живого `src/` (`check-docs.mjs:132`) — прогон зелёный,
+   то есть коммит `f54b9c0` не просто «обновил какие-то числа», а актуален
+   прямо сейчас, на этом дереве, а не только на дереве владельца в момент
+   коммита.
+5. **#174 действительно уже принят** (`S8-merged`) — рероллы его кода этим
+   ребейзом не создают повторного риска, требующего отдельной ревью-нагрузки
+   в рамках #173.
+
+## Что проверено и корректно
+
+- Всё содержание раздела «Что проверено и корректно» `CODE-REVIEW-173-r1.md`
+  остаётся в силе без изменений: алгоритм planar-graph, дисциплина «тест
+  умеет падать» (мутационная проверка r1), finish-контракт на трёх точках
+  выхода, cancel/escape restore terminal draft, лимиты до mutation, clean
+  split reuse, provenance/толщина, совместимость (AC14), i18n RU/EN,
+  трейлеры/changelog — всё это проверялось на идентичном продуктовом дифе.
+- Дополнительно к r1: ребейз на `dev`, включающий #174, произведён без
+  потери/дублирования продуктовых записей в shared-файлах (`docs/specs/
+  README.md`, оба changelog), без оставленных маркеров конфликта, с
+  пересобранным и сверенным бандлом.
+- `f54b9c0` — легитимный docs-only коммит: трогает только
+  `docs/images/*.png` и `docs/images/screenshots.json` (не
+  `demo/golden/baselines/**`), поэтому не требует `Release:`/
+  `Baseline-Reviewed:` трейлеров; несёт `Issue: #173`, `User-Visible: no`
+  корректно (сами скриншоты — не продуктовое поведение); `check-docs.mjs`
+  подтверждает, что обновлённый fingerprint соответствует текущему `src/`.
+- `process-gate.mjs` зелёный на новом диапазоне `origin/dev..HEAD` (5
+  коммитов, было 3 в r1) — трейлеры на всех коммитах класса A/B/C корректны.
+
+## Чего не проверял
+
+- Не повторял построчный разбор алгоритма `wall-face-graph.ts` и AC4–AC6/AC8/
+  AC9/AC12/AC13 по существу — сделано в r1 на идентичном по diffstat коде;
+  повторный построчный разбор того же самого кода не производит новой
+  информации, только повторяет уже потраченный цикл. Подтверждено только
+  косвенно: `npm test` зелёный (848/848) на актуальном дереве без изменений
+  в затронутых файлах.
+- Не повторял мутационную проверку «тест умеет падать» — тот же аргумент,
+  код `wall-face-graph.ts`/`_applyWallFaceBatch` не менялся с момента, когда
+  она была выполнена в r1.
+- Не прогонял `golden:verify` и `pytest tests_backend` — как в r1, diff не
+  задевает golden/backend поверхности; это предрелизный гейт.
+- Не прогонял полный набор 138+ browser-smoke — продуктовый код не изменился
+  с r1, поверхность та же (Plan editor), расширять выборку без нового риска
+  в diff не оправдано.
+- Не воспроизводил multi-client optimistic-lock conflict живым прогоном
+  (AC13) — как в r1, проверено чтением, что batch не вводит новый conflict-
+  путь; код batch не менялся.
+- Не запускал `npm run benchmark:compare` против сохранённого baseline SHA —
+  такого отчёта нет в этой сессии; сверил абсолютные числа вручную с
+  бюджетным файлом, как в r1.
+
+## Вердикт
+
+Зелёный. High: 0, Medium: 0 новых (обе Medium-находки r1 остаются в силе,
+уже вынесены отдельными issue — [#176](https://github.com/Matysh/houseplan-card/issues/176),
+[#177](https://github.com/Matysh/houseplan-card/issues/177), — и не
+переоткрываются, так как код, к которому они относятся, не изменился). Low: 2,
+перенесены из r1 без изменений, не блокируют.
+
+Причина цикла r2 не в качестве кода, а в необходимом по процессу повторном
+прогоне после ребейза на ушедший вперёд `dev` (слияние с #174). Продуктовый
+диф идентичен уже принятому в r1 по составу файлов и объёму изменений; новая
+проверочная нагрузка этого цикла была сосредоточена на самом слиянии и на
+корректности docs-only коммита `f54b9c0` — обе проверки пройдены. Ни одна
+находка не свидетельствует, что изменение не решает заявленный сценарий или
+ухудшает смежное поведение.