docs: review document for #242

Issue: #242
User-Visible: no

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

@@ -0,0 +1,266 @@
+# CODE-REVIEW-242-r1
+
+- Issue: [#242](https://github.com/Matysh/houseplan-card/issues/242) — «Проём в толстой стене рисуется у произвольной грани, а не по центру толщины»
+- Этап: code (PROCESS.md §2.7)
+- Заход: r1 (первый код-цикл; SPEC-REVIEW-242-r1 — зелёный)
+- Диапазон: `origin/dev..HEAD` = `0033138..5e13e34` (5 коммитов), реализация — `6d6db12` (`fix(openings): center symbols across wall depth`, `User-Visible: yes`)
+- Вердикт: **жёлтый**
+
+## Скоуп
+
+ТЗ `docs/specs/242-opening-symbol-center.md` требует: символ проёма (дверь/окно/ворота)
+по умолчанию центрируется по толщине стены на всех render surfaces (Flat/View,
+preview, hosted Static, скрытый Iso); сохранённый `flip_v: true` у двери/окна
+остаётся ручным edge alignment; у ворот `flip_v` **не двигает** створки, а только
+меняет знак существующего 10° поворота (AC4, §4.3, §7 таблица). Направление должно
+быть детерминированным и не зависеть от порядка комнат/направления независимой
+стены (AC2).
+
+## Как проверялось
+
+Прочитаны docs/SCOPE.md, PROCESS.md, docs/specs/242-opening-symbol-center.md,
+docs/WALL-THICKNESS.md, docs/ARCHITECTURE.md, docs/ISOMETRIC.md,
+docs/USER-GUIDE.ru.md, тело issue и все комментарии (аналитика, Q1–Q4, решения
+владельца, handoff разработчика).
+
+Дифф прочитан построчно: `src/opening-symbol-placement.ts` (новый),
+`src/wall-thickness.ts`, `src/render/opening-symbol.ts`, `src/iso-openings.ts`,
+плюс unit/smoke/mutation-gate правки. `src/houseplan-card.ts` и
+`src/partition-openings.ts` диффом не тронуты — они читались как неизменный
+контекст, через который резолвится `face`.
+
+Гейты, реально прогнанные в этой сессии:
+
+| Гейт | Результат |
+|---|---|
+| `npx tsc --noEmit` | green |
+| `npm test` | green, 1095/1095 |
+| `npm run build` + сверка `dist` / `demo/srv/assets` / `custom_components/.../frontend` | green, все три sha256 = `3befe09b…`, `git status` чист |
+| `node scripts/check-docs.mjs` | green (7 файлов, 10 внешних ссылок) |
+| `node scripts/mutation-gate.mjs --check` | green, включая 3 новых мутанта `opening-symbol-*` |
+| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | «Зарегистрированная связь» → `smoke_isometric_contract.mjs`, `smoke_wall_thickness.mjs` (оба помечены автором как прогнанные и зелёные; полный вывод инструмента ниже) |
+| `npm run golden:verify` на HEAD | red (см. раздел «Находки», M1) — прогнан дважды подряд, результат детерминирован |
+| `npm run golden:verify` на `origin/dev` (5bf1868, отдельный `git worktree`) | red по своему меньшему набору сцен — снят как база для сравнения, не для приёмки |
+
+Дополнительно я independently проверил геометрический контракт **прямым запуском
+в headless Chromium** через `demo/serve.mjs` (тот же харнесс, что используют
+существующие `demo/smoke_*`), а не только чтением кода — конкретно AC4 для
+ворот, потому что до-релизная логика face/flip у ворот инвертирует `flip_v`
+дважды (`faceFlipV = !o.flip_v` в `src/houseplan-card.ts` при построении face,
+затем сырой `o.flip_v` ещё раз в `sy` внутри `renderOpeningVisibleGeometry`), и
+я не был уверен в результате по одному чтению formулы. Три сценария (свежее
+состояние карточки на каждый, без переиспользования state):
+
+1. Ворота на **внешней** (однокомнатной) толстой стене (`_wallThickClick`,
+   cm=25) — `flip_v: false` → рычаги `rotate(-10deg)/rotate(10deg)`,
+   `flip_v: true` → `rotate(10deg)/rotate(-10deg)`. Меняется, как требует AC4.
+2. Ворота на **общей** стене двух комнат (тот же сценарий, что и исходный баг
+   — стена между `r1`/`r2`) — `flip_v: false` и `flip_v: true` дают **одинаковый**
+   `rotate(10deg)/rotate(-10deg)`; направление не меняется. Проверено дважды,
+   плюс с перестановкой `sp().rooms` — результат идентичен во всех четырёх
+   комбинациях.
+3. Ворота на **независимой стене** (`host: { kind: 'partition', … }`), прямые и
+   развёрнутые endpoints — `flip_v: false` и `flip_v: true` также дают
+   одинаковый `rotate(10deg)/rotate(-10deg)` в обеих ориентациях перегородки.
+
+Скрипты воспроизведения не коммитились (репозиторий трогать нельзя), но
+последовательность — это буквально `demo/smoke_wall_thickness.mjs` до момента
+создания стены, плюс `sp().openings = [{ type: 'gate', … }]` и чтение
+`.op-leaf` style после `c._cfgEpoch++/requestUpdate/updateComplete`; сравнение
+`flip_v` до/после и общей/независимой стены до/после — воспроизводится любым
+инструментом с доступом к `window.__card`.
+
+## Находки
+
+### H1 (High, в скоупе) — AC4 не выполнен: `flip_v` не меняет направление 10° поворота ворот на общей стене и на независимой стене
+
+**Что не так.** Спецификация (§4.3, §7 таблица: `gate | true | (0,0) |
+противоположный знак 10° поворота`) и AC4 («Gate при обоих значениях `flip_v`
+центрирован, но знак 10° поворота меняется») требуют, чтобы флаг `flip_v` у
+ворот всегда переключал сторону лёгкого поворота створок, при неизменном
+центрированном origin. Перевод «центр не двигается» (translation) реализация
+делает верно — `openingSymbolOffset` безусловно возвращает `{0,0}` для
+`type==='gate'`. Но требование «знак меняется» не выполняется для двух из трёх
+типов host, включая тот, что явно назван в самом issue как источник бага:
+
+- **общая стена двух комнат** — именно тот кейс, ради которого затевалась вся
+  задача («порядок комнат», «то с одной стороны, то с другой»). Створки ворот
+  на общей стене поворачиваются **одинаково** при `flip_v: false` и
+  `flip_v: true` (см. воспроизведение выше, сценарий 2);
+- **независимая (partition) стена** — тот самый путь, про который в issue
+  сказано: «сторона фиксирована знаком... то есть по умолчанию всегда одна и
+  та же грань» (сценарий 3). После правки трансляция действительно исчезла
+  (это была цель), но направление поворота по‑прежнему не реагирует на
+  `flip_v` вообще — то есть первоначальный дефект (фиксированный знак) для
+  этого пути **не устранён**, просто перестал быть заметен из-за убранного
+  смещения.
+
+Направление меняется корректно только для ворот на **однокомнатной внешней**
+стене (сценарий 1) — самом редком практическом месте для ворот.
+
+**Причина по коду.** `src/houseplan-card.ts` инвертирует `flip_v` перед
+резолвом грани только для ворот: `faceFlipV = o.type === 'gate' ? !o.flip_v :
+o.flip_v` (строки 11988, 18551, 12066 и в Iso-ветке 5289), затем передаёт
+**сырой** `o.flip_v` в `visibleSpec.flipV`/`sy`. `src/render/opening-symbol.ts`
+считает `gateAngle = spec.face.side * sy * 10 * amount`. Для стороны,
+получаемой через `partitionOpeningFace` (`side = flipV ? 1 : -1`, не тронут
+этим диффом) и через «общую стену» ветку `wall-thickness.ts`
+(`naturalSide = association.negative && association.positive ? -1 :
+available[0].side`, тоже реагирует на `flip_v` только знаком) двойная
+инверсия (`faceFlipV` затем `sy`) арифметически гасит сама себя: `side * sy`
+даёт одно и то же значение при `flip_v=false` и `flip_v=true`. Единственный
+случай, где это не совпадает — однокомнатная стена, потому что там
+`naturalSide` берётся из `available[0].side`, единственного реального
+кандидата, а не из пары `negative`/`positive`, и по независимым от меня
+причинам (сама схема `_openingFace`/`resolveOpeningWallAssociation`) итоговый
+знак там ведёт себя иначе; я подтвердил это эмпирически, а не через ручной
+вывод формулы (see «Как проверялось»), и не настаиваю на объяснении «почему
+именно этот случай работает» — важен наблюдаемый факт.
+
+**Кто ещё вводится в заблуждение.** Оба changelog, `docs/WALL-THICKNESS.md`,
+`docs/ARCHITECTURE.md`, `docs/ISOMETRIC.md` и `docs/USER-GUIDE.ru.md` **прямо
+утверждают** обратное: «ворота остаются по центру, а флаг меняет только
+направление их 10° открытия» (и англ. эквивалент «a gate flip changes only
+turn direction»). Это неверно для двух из трёх host-путей.
+
+**Почему не поймано тестами.** `test/opening-symbol.test.mjs` для ворот
+проверяет `rotate(10deg)`/`rotate(-10deg)` только у **одного** (не-flipped)
+рендера — второй (`flippedGate`) собирается, но его `rotate(...)` значения
+нигде не сравниваются с первым. `test/iso-openings.test.mjs` сравнивает
+`gate`/`gateFlipped`, но эти два фикстура одновременно меняют и `flipV`, и
+`face.side` вручную — тест не воспроизводит, что в реальном рендере `side`
+сам является функцией от `flip_v` через `faceFlipV`, поэтому не ловит гашение
+знаков. `demo/smoke_isometric_contract.mjs` проверяет только
+`Math.abs(leaf.turnDeg) === 10`, не сравнивая знак с неперевёрнутым вариантом.
+Ни один golden-сценарий с воротами `flip_v: false/true` рядом не добавлен (см.
+M1) — а именно такая сцена сделала бы дефект видимым на глаз.
+
+**Серьёзность.** High: явный, воспроизводимый разрыв между заявленным в ТЗ/
+docs контрактом и фактическим поведением для двух из трёх типов host-стен,
+включая тот, что был первопричиной issue. Блокирует.
+
+**Как исправить (для автора, не мной):** либо не инвертировать `flip_v` при
+резолве `face` для ворот в `houseplan-card.ts` (раз "сторона" в `face.side`
+и так уже несёт нужный знак), либо использовать в `gateAngle` **не**
+`spec.face.side`, а сырой `!!o.flip_v`/детерминированную локальную сторону
+напрямую, независимо от того, как резолвер комбинирует её с `flip_v`
+дважды. Нужна регрессионная проверка знака на пути «общая стена» и
+«partition», а не только «однокомнатная стена».
+
+### M1 (Medium, в скоупе) — заявленный golden-охват в handoff занижен на порядок; обязательные по §12.3 новые сцены не добавлены
+
+Handoff-комментарий разработчика утверждает: «Targeted golden:
+`opening-placement-door-thick-wall-dark` и `openings-thick-wall-dark` ожидаемо
+`different`». Фактический `npm run golden:verify` на точном `5e13e34`
+(прогнан дважды, детерминированно) даёт **`different` по ~48 сценам**, не по
+двум — сравнение с тем же прогоном на чистом `origin/dev` (отдельный
+`git worktree`, тот же локальный Chromium) показывает, что подавляющее
+большинство новых расхождений (`isometric-geometry-*`, `day-cycle-*`,
+`lighting-*`, `hover-*`, `device-*`, `optimize-preflight-dialog-*`,
+`backup-*`, `room-label-parity-*` и др.) на dev были `passed`, а на HEAD стали
+`different`. Причина ожидаемая и не является отдельным багом: эти сцены
+используют общие фикстуры (`golden-geometry`, `golden-lighting`), в которых
+уже стоит дверь/окно/ворота в толстой стене — символ у них теперь центрирован,
+и это меняет пиксели кадра, даже когда сценарий проверяет не связанную с
+проёмами вещь (солнце, hover, диалог). Это не находка «что-то сломалось
+непричастное», а находка «отчёт разработчика не соответствует
+действительности на порядок величины» — то же самое, что ревью #231-r1 уже
+находило (M1 там: «список golden-сцен неполон») и что #231-r2 потребовало
+зафиксировать явным списком в `docs/TESTING.md`, а не оставлять в тексте
+handoff-комментария.
+
+Отдельно: §12.3 ТЗ требует **добавить** новые semantic-сцены — «centered
+door/window/gate на толстой room wall, Light», «те же типы on diagonal
+partition, Dark», «door/window с `flip_v: true` и gate `false/true` рядом»,
+«hidden Iso parity for center and gate direction» — и обновить golden semantic
+guard, чтобы он **до** PNG-сравнения проверял wall centerline, visible-group
+center, jamb depth, `flip_v` и gate angle. Ни одна из этих сцен не добавлена
+в `demo/golden/matrix.mjs` (диф по этому файлу — только правка комментария
+для существующей `opening-placement-door-thick-wall-dark`); никакого нового
+semantic guard тоже нет. Именно сцена «gate `false/true` рядом» — то, что
+сделало бы H1 видимым на глаз ещё до code review.
+
+**Серьёзность.** Medium, в скоупе задачи: тестовый план §12.3 не выполнен
+полностью, а фактический охват регрессии не задокументирован для
+предрелизного `golden:accept -- --reviewed`, который будет принимать эти ~48
+сцен без знания их полного списка и происхождения. Чинится добавлением
+описанных в §12.3 сцен/guard и точного списка затронутых существующих сцен
+(аналогично `docs/TESTING.md` в #231-r2) — без необходимости отдельного issue.
+
+## Что проверено и корректно
+
+- **AC1/AC2 для room-wall пути** (без `flip_v`, центр стены, независимость от
+  порядка комнат). `openingInnerFaceOffsetFromIndex` разделяет физическую
+  половину (`piece.half`/`piece.cm`, не зависит от `flip_v`) и направление
+  (`side` из `resolveOpeningWallAssociation`, вычисляется из локальной нормали
+  открывания `nx,ny = f(opening.angle)` и геометрического `edge.inward`,
+  **не** зависит от порядка `rooms`/`candidateOrder`). `test/wall-thickness.test.mjs`
+  добавил `reversed`/`reversedFlip` deepEqual-проверку — я прочитал и
+  проверил, что тест умеет падать (откатил численно вручную: без замены
+  сортировки `.order` на геометрический `side` возвращается старое значение).
+- **AC2 для partition-пути.** `resolvePartitionOpening` нормализует `angle`
+  через `atan2` + приведение к `[-90,90)`, что инвариантно к развороту
+  `a`/`b` (проверено чтением и совпадает с уже существующим тестом
+  `test/partition-openings.test.mjs:35`, не тронутым этим диффом).
+  `openingSymbolOffset` использует `Math.hypot(face.ox, face.oy)` — модуль, а
+  не знак — поэтому разворот `partitionOpeningFace`'s `ox/oy` (который
+  реально зависит от направления оси, см. `axis.ux/uy`) не протекает в
+  видимое смещение. Это подтверждено и unit-тестом
+  (`test/opening-symbol-placement.test.mjs`: `positiveFace`/`negativeFace` дают
+  одинаковый результат), и моим собственным browser-тестом сценария 3 выше
+  (симметрия сохраняется, хоть и с той же H1-проблемой по направлению).
+- **AC3 (door/window `flip_v: true`, окно целиком).** `test/opening-symbol.test.mjs`
+  проверяет, что `op-glass`/leaves/arc едут одним `translate`; я прочитал
+  `renderOpeningVisibleGeometry` — стекло действительно внутри той же
+  `<g transform="translate(...)">`, что и створки/дуги.
+- **Jambs full-depth.** `jambHalf` в `openingVisibleMetrics` считается из
+  `spec.face.cm` независимо от `swingTx/swingTy` — косяки не входят в
+  переносимую группу, как требует §7. Проверено чтением, не исполнением
+  отдельного визуального теста.
+- **AC7 (без миграции/схемы).** `src/opening-symbol-placement.ts` — чистые
+  функции, `OpeningCfg.flip_v` не тронут, новых полей/типов конфигурации нет.
+  `git diff` подтверждает: единственный новый файл — чистый TS-модуль без
+  side-effects.
+- **User-Visible / трейлеры.** `6d6db12` несёт `Issue: #242` и
+  `User-Visible: yes`; оба changelog обновлены в том же коммите (см.
+  `git show 6d6db12 --stat`).
+- **Три копии бандла.** sha256 `dist/houseplan-card.js`,
+  `demo/srv/assets/houseplan-card.js`,
+  `custom_components/houseplan/frontend/houseplan-card.js` совпадают между
+  собой и с заявленным автором хэшем; `npm run build` воспроизводит их
+  без диффа в рабочем дереве.
+- **Mutation-gate.** Три новых мутанта (`opening-symbol-default-uses-room-face`,
+  `opening-symbol-partition-follows-endpoints`, `opening-gate-flip-translates-leaves`)
+  проходят `--check`; я прочитал патчи — они действительно откатывают именно
+  центрирование/order-independence (не гейт-обход), но, как и unit/golden
+  выше, не покрывают H1 (мутант про ворота проверяет только «не появилась ли
+  трансляция», а не «меняется ли знак поворота»).
+
+## Чего не проверял
+
+- **Полный Linux golden-артефакт и его reviewed-приёмка** — вне цикла код-ревью
+  по правилу §13 (`golden:accept` только по полному Linux release artifact).
+  Локальный `golden:verify`, который я прогнал, обслуживает только M1 (охват
+  дельты), не приёмку эталонов.
+- **`python -m pytest tests_backend`** — не прогонял: `custom_components/**/*.py`
+  диффом не тронут.
+- **Performance-профили** — не прогонял: задача не называет их в AC, изменение
+  чисто геометрическое без новых аллокаций/обходов по стенам (проверено
+  чтением: `openingSymbolOffset` — O(1) арифметика на уже разрешённом `face`).
+- **Ручной пиксельный просмотр каждой из ~48 изменившихся golden-сцен** — я
+  установил только факт и причину расхождения (общая фикстура с дверью), не
+  разбирал каждую сцену построчно; это и есть предмет M1.
+- **AC1 буквально на 1/15/100 см.** Существующие unit-тесты (`wall-thickness.test.mjs`,
+  `opening-symbol-placement.test.mjs`) используют другие конкретные значения
+  cm (15/20/25/30); отдельного sweep по 1/15/100 см для нового contract'а нет.
+  Не поднимаю как отдельную находку — арифметика линейна по `cm`
+  (`piece.half`, масштаб) и не имеет условных ветвей по величине толщины, риск
+  низкий, но формальное доказательство AC1 в заявленном виде отсутствует.
+- **Полная browser-smoke матрица (167 сценариев)** — не прогонял всю. Выбор
+  ограничен `smoke-select`-выводом (2 зарегистрированные связи, обе прогнаны
+  автором и мной перепроверены через отдельные ad hoc browser-скрипты на
+  реальном харнессе, не только через существующие smoke-файлы) плюс
+  дополнительные `smoke_partition_openings.mjs`/`smoke_opening_preview.mjs`,
+  которые автор тоже прогнал зелёными. Полный прогон не требуется: диф не
+  задевает несвязанные подсистемы (unrelated golden-diff в M1 объяснён общей
+  фикстурой, а не изменением поведения самих этих подсистем).