docs: review document for #193

Issue: #193
User-Visible: no

docs/reviews/SPEC-REVIEW-193-r1.md

@@ -0,0 +1,266 @@
+# SPEC-REVIEW-193-r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/193
+- **ТЗ под ревью:** [`docs/specs/193-passage-placement-preview.md`](https://github.com/Matysh/houseplan-card/blob/issue/193-passage-preview/docs/specs/193-passage-placement-preview.md)
+  (коммит `fc22d9a`), обычный трек — не `small`/`trivial`
+- **Роль:** ревьюер ТЗ (не автор), этап `S4-spec-review`
+- **Трек:** обычный, лимит циклов ревью ТЗ — 4 (§4 PROCESS.md)
+- **Цикл:** r1/4
+
+## Скоуп ревью
+
+ТЗ #193: инструмент «Открытый проём» в Plan editor получает специальную
+placement-preview геометрию только для `candidate.type === 'passage'` —
+полупрозрачный сегмент будущего разреза стены и две поперечные засечки
+границ, поверх уже существующих точки привязки и линеек. Сохранённый passage,
+door/window/gate preview, конфиг, backend, i18n, миграция и touch-контракт не
+меняются по тексту ТЗ.
+
+Не в скоупе ревью: продуктовый код. Ветка `issue/193-passage-preview`
+существует и содержит один коммит (`fc22d9a6c55b085dc273a70bdc8e2db22ec9fe19`,
+`Issue: #193 · User-Visible: no`) — только спецификация, реализации нет.
+Гейты (`typecheck`/`test`/`build`) не прогонялись: на этапе ревью ТЗ
+продуктового кода не существует, прогон не относится к этому этапу
+(PROCESS.md §2.4/§8).
+
+## Как проверялось
+
+1. Прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` целиком (действующая
+   редакция, включая §1, §2.4, §2.5, §5, §7.1, §8, §10.2).
+2. Прочитано тело issue #193 и оба комментария: аналитика владельца
+   (`Matysh`, `OWNER`, оценка 6/10 · 3/10 · 3/10 · P2 · `enhancement` ·
+   обычный трек, явное обоснование, почему `small`/`trivial` не подходят —
+   новый UX-контракт preview) и хендофф «ТЗ готово к ревью».
+3. Построчно сверены все технические утверждения раздела «Подтверждённая
+   техническая база» (§4 ТЗ) с кодом на этой ветке:
+   - `_renderOpeningPlacementPreview()` — `src/houseplan-card.ts:17350`
+     (заявлено 17351, расхождение на строку из-за doc-комментария, не
+     дефект); вызывает `renderOpeningVisibleGeometry(visibleSpec)` без
+     ветвления по типу — подтверждает диагноз «инструмент слеп для passage».
+   - `renderOpeningVisibleGeometry()` (`src/render/opening-symbol.ts:51-55`):
+     `if (spec.type === 'passage') return svg\`\`;` — подтверждено дословно,
+     включая обоснование в комментарии кода («negative space», «no jamb, leaf,
+     arc, gate panel or standalone frame»).
+   - `OpeningPlacementCore` (`src/opening-placement.ts:33-48`) содержит ровно
+     заявленные поля: `x`, `y`, `angle`, `renderedLength`, `target`
+     (`OpeningPlacementTarget.physicalHalfWidth`, строка 17), `measure`,
+     `flipH`/`flipV`, `type` — ТЗ не выдумало ни одного поля.
+   - `openingPlacementTargets()` (строки 109-140): для совпадающих
+     `segmentKey` берёт `Math.max(previous.physicalHalfWidth, interval.half)`
+     — подтверждает точную формулировку ТЗ «для совпадающих room-owned копий
+     берётся максимальная реальная половина толщины».
+   - `_opMeasureView` — `src/houseplan-card.ts:11895` (заявлено 11896, тот же
+     класс погрешности) возвращает `OpMeasure`, используется на 15384 —
+     подтверждено, линейки действительно не требуют изменений.
+   - Порядок SVG (`src/houseplan-card.ts:15690-15733`): `_renderWallBodies()`
+     на строке 15723, `_renderOpeningPlacementPreview()` на 15727 — preview
+     реально рисуется после тел стен и будет виден поверх кладки без нового
+     слоя, как заявлено.
+   - `.opening-preview` (группа, `houseplan-card.ts:17366-17368`) уже несёт
+     `aria-hidden="true" pointer-events="none"`; `.opening-preview-dot`
+     (17371-17372) — то же. Оба факта подтверждены буквально.
+   - Дочерний `<circle>` точки использует `r=${this._gridPitch * 0.18}`
+     (17372) — подтверждает §16 п.3 ТЗ: выступ засечек, предложенный тем же
+     коэффициентом, действительно равен радиусу существующей preview-точки,
+     а не выдуман.
+   - Golden harness (`demo/golden/harness.mjs:450`) — allowlist
+     `['window', 'door', 'gate']`, требует `.op-leaf` (строка 474) —
+     подтверждает точный диагноз «harness валидирует только типы с
+     `.op-leaf` и не принимает passage».
+   - Golden matrix (`demo/golden/matrix.mjs:100-106`) — существует ровно один
+     сценарий `opening-placement-door-thick-wall-dark` (без light-варианта);
+     ТЗ корректно называет его существующим прецедентом и не завышает его
+     охват.
+   - CSS `.opening-preview { opacity: 0.5; }` (`src/styles.ts:1372-1375`) —
+     подтверждает реальность риска «opacity применяется дважды» (§13 ТЗ):
+     новый сегмент рисуется внутри той же группы, и наивная реализация с
+     `opacity: 0.35` на самом rect действительно даст `0.5 × 0.35 = 0.175`,
+     как явно предупреждает ТЗ. Риск назван точно, а не гипотетически.
+   - `demo/smoke_opening_preview.mjs:152` — `out.saveMatchesResolver` уже
+     существует под этим именем; AC2 ссылается на реальный, а не придуманный
+     идентификатор.
+   - `test/opening-symbol.test.mjs` существует (`ls test/`), AC3 ссылается на
+     реальный файл.
+4. Прочитан `docs/USER-GUIDE.ru.md` (разделы 1 «Термины», 9 «Двери, окна,
+   открытые проёмы, ворота и замки»): терминология ТЗ («Открытый проём»,
+   «полупрозрачный символ», «толстая стена», ширина по умолчанию 90 см)
+   совпадает с существующим текстом (строки 428-465), включая уже описанное
+   поведение door/window/gate preview («Полупрозрачный символ показывает
+   точный будущий вид... виден поверх тела толстой стены», строки 437-438) —
+   ТЗ не изобретает интерфейсную лексику.
+5. Прочитан `docs/WALL-THICKNESS.md` целиком — модель `half`/толщины,
+   growth ±½, masonry cuts согласуются с использованием
+   `target.physicalHalfWidth` в ТЗ; прямого противоречия канону нет.
+6. Прочитан `docs/TOUCH-SUPPORT.md` целиком, включая «Documentation rule»
+   (строки 142-154): требование буквальной строки `Touch editor: …` для
+   «New editor feature specifications». В тексте ТЗ #193 такой строки нет
+   ни в одном разделе (проверено `grep -n touch` по файлу — единственное
+   упоминание, строка 123 Non-scope: «расширение touch-гарантий Plan editor»)
+   — см. находку Low-1.
+7. Проверены прямые ссылки ТЗ на файлы демо/тестов —
+   `demo/smoke_opening_preview.mjs`, `demo/golden/matrix.mjs`,
+   `demo/golden/harness.mjs`, `test/opening-symbol.test.mjs` — все существуют
+   и содержат ровно те механизмы, на которые ссылается ТЗ (см. п.3 выше).
+8. Проверено соответствие `docs/SCOPE.md`: задача — геометрия Plan editor
+   (J4 «zero to a working plan... room polygons» / J6 «keep the plan true as
+   the home evolves», обе персона home admin, поверхность — desktop Plan
+   editor), View/kiosk/backend не затрагиваются; попадает в допустимый скоуп,
+   не создаёт нового job'а. Родитель #157 (сам open passage) уже принят и
+   закрыт как реализованный J4/J6-функционал — #193 закрывает его собственный
+   UX-пробел, не расширяя скоуп.
+9. Проверено, что issue не помечен `small`/`trivial`: аналитика владельца
+   явно называет причину (новый UX-контракт preview) — критерий §5 PROCESS.md
+   «нет нового UX-контракта» не выполняется, обычный трек обоснован корректно.
+
+Гейты (`typecheck`/`test`/`build`, browser smoke, golden) не прогонялись —
+продуктового кода нет, что и ожидается на этапе ревью ТЗ.
+
+## Обязательные разделы (§7.1 PROCESS.md)
+
+| Раздел | Есть | Комментарий |
+|---|---|---|
+| Сценарий (персона/поверхность/момент) | ✅ | §1: home admin, desktop Plan editor, наведение инструментом до клика |
+| Что человек увидит до/после | ✅ | §2: одна фраза до/после, без терминов реализации |
+| Проблема | ✅ | §3, подтверждена чтением кода (см. «Как проверялось» п.3) |
+| Скоуп / не-скоуп | ✅ | §5/§6, скоуп ограничен геометрией+визуалом+тестами, не-скоуп явно исключает drag, символ passage, i18n, touch-расширение |
+| Контракт поведения | ✅ | §7 (появление, hover/клик, после сохранения, остальные типы, события/доступность) |
+| UX | ✅ | §8, таблица состояний, 7 строк |
+| Модель данных и миграция | ✅ | §9, «не меняется», обоснованно для editor-only transient preview |
+| i18n | ✅ | §10, «новых строк нет» |
+| AC1…ACn с доказательством | ✅ | §11, 6 штук, у каждого назван способ доказательства (unit/smoke/golden/source contract) |
+| План автотестов | ✅ | §12, unit/smoke/golden по отдельности, falsifiability явно потребована («на `origin/dev` до реализации обязаны краснеть») |
+| Риски | ✅ | §13, 8 строк риск/мера, включая подтверждённый двойной-opacity риск |
+| Откат | ✅ | §14, удаление ветки/helper/стилей/тестов, без миграции |
+| Release-артефакты | ✅ | §15, оба changelog, RU/EN guide, TESTING.md, dist-синхронизация, golden candidates отложены на release runbook |
+
+Присутствует также обязательный блок «Принято предположительно» (§16, 5
+пунктов) — технические, не продуктовые решения, корректно отделены от
+продуктовых вопросов, которых, по утверждению автора и аналитики, для этой
+задачи нет. Проверка не нашла в §16 замаскированного продуктового вопроса:
+все пять пунктов (расположение чистого helper'а, имена классов, коэффициент
+выступа засечек, общая golden-fixture, поведение при нулевой толщине)
+касаются исключительно того, чего пользователь не наблюдает или что уже
+предрешено принципом «не выдумывать несуществующую толщину».
+
+## Находки
+
+### Low-1 — отсутствует обязательная декларация `Touch editor: …`
+
+**Файл:** `docs/specs/193-passage-placement-preview.md`, весь документ (нет
+подходящего раздела); ближайшее место — §6 Non-scope, строка 123.
+
+`docs/TOUCH-SUPPORT.md`, раздел «Documentation rule» (строки 142-154):
+«New editor feature specifications and code reviews must state one of:
+`Touch editor: supported`; `Touch editor: best effort / intentionally
+degraded`; `Touch editor: not exposed`.» ТЗ #193 добавляет новую видимую
+geometry в Plan editor (`hover`-preview) — то есть однозначно подпадает под
+«new editor feature specification». Полнотекстовый поиск `touch` по файлу
+даёт единственное совпадение (строка 123 Non-scope: «расширение
+touch-гарантий Plan editor»), которое по смыслу подразумевает ответ (не
+расширяем существующий best-effort статус), но не содержит требуемой
+буквальной строки. Тот же класс проверки в `SPEC-REVIEW-192-r1` (issue #192)
+нашёл строку `**Touch editor:** picker поддерживается как в #57` и закрыл
+вопрос без находки — здесь эквивалентной строки просто нет.
+
+**Обоснование severity:** ответ не является продуктовой неопределённостью —
+Plan editor по умолчанию desktop-first/best-effort (`docs/TOUCH-SUPPORT.md`,
+таблица «Product contract»), preview — чисто presentation, hover-based
+эффект, на устройстве без реального hover просто не активируется тем же
+путём, что и сейчас для door/window/gate; отдельного решения владельца не
+требуется. Дефект — отсутствие обязательной для канона декларации, не
+дефект поведения. AC1…AC6 не зависят от этой строки.
+
+**Решение ревьюера:** Low, не блокирует. Снимаю без возврата ТЗ на правку с
+условием: автор добавляет в ТЗ (в §6 Non-scope или отдельной строкой рядом
+с §7.5) явную декларацию `**Touch editor:** best effort / intentionally
+degraded (наследует существующий статус Plan editor; preview — presentation-only
+hover-эффект, не расширяет touch-гарантии)` до перевода issue в
+`S5-ready` — это чисто редакционное дополнение, не требующее нового цикла
+ревью ТЗ, и должно быть зафиксировано в хендоффе реализации или проверено
+на код-ревью, если автор пропустит этот шаг.
+
+## Что проверено и корректно
+
+- **Соответствие `docs/SCOPE.md`:** J4/J6, персона home admin, desktop Plan
+  editor; View/kiosk/backend не затронуты; #193 закрывает собственный
+  UX-пробел уже принятого #157, не расширяя его скоуп.
+- **Легитимность обычного трека:** аналитика владельца верно называет
+  причину, по которой `small`/`trivial` не подходят (новый UX-контракт
+  preview) — не самоощущение исполнителя, а проверяемый критерий §5
+  PROCESS.md.
+- **Продуктовых вопросов владельцу нет и не додумано новых.** Единственный
+  потенциально продуктовый вопрос — объём видимого эффекта (сегмент + две
+  засечки, без нового цвета/настройки) — уже зафиксирован самим текстом
+  issue (владелец); ревью не нашло скрытой догадки, выданной за факт: каждое
+  утверждение §4 «Подтверждённая техническая база» проверено построчно по
+  реальному коду ветки (см. «Как проверялось» п.3) и подтвердилось без
+  исключений.
+- **Технический диагноз проблемы не голословен** — `renderOpeningVisibleGeometry`
+  дословно возвращает пустой SVG для `passage`, `_renderOpeningPlacementPreview`
+  дословно не ветвится по типу; диагноз «слепой инструмент» доказан кодом.
+- **Скоуп реально согласован с существующим pipeline:** `OpeningPlacementCore`
+  уже содержит все поля, на которые ссылается ТЗ (`x`, `y`, `angle`,
+  `renderedLength`, `target.physicalHalfWidth`, `measure`) — реализация не
+  потребует нового вычисления геометрии, только новую ветку рендера, как и
+  заявлено.
+- **Риск двойного `opacity` реален, а не гипотетичен** — `.opening-preview`
+  действительно несёт CSS `opacity: 0.5` (`styles.ts:1372`); ТЗ верно
+  определило нужную меру (отдельная стилевая область + computed-value
+  проверка в AC6) до того, как эта ошибка попала бы в реализацию.
+- **AC1–AC6 однозначны и проверяемы**, каждый снабжён допустимым по §2.5
+  PROCESS.md способом доказательства (unit/smoke/golden/source contract);
+  план автотестов (§12) явно требует falsifiability до реализации —
+  «unit геометрии и smoke DOM обязаны краснеть на `origin/dev`».
+- **Другие типы openings защищены negative-контрактом** (AC4) и
+  существующим door-golden, который явно не меняется — предотвращает
+  случайную регрессию window/door/gate preview.
+- **Существующие файлы и идентификаторы, на которые ссылается ТЗ, реальны**
+  — `demo/smoke_opening_preview.mjs` (`saveMatchesResolver`, строка 152),
+  `test/opening-symbol.test.mjs`, `demo/golden/harness.mjs` (allowlist
+  `['window','door','gate']`, строка 450), `demo/golden/matrix.mjs`
+  (`opening-placement-door-thick-wall-dark`, строка 100) — ни одна ссылка не
+  выдумана.
+- **Откат тривиален** — preview-only ветка/helper/стили/тесты удаляются без
+  миграции данных; #157 продолжает работать без деградации.
+- **Release-артефакты** ссылаются на существующие файлы (`docs/CHANGELOG.md`,
+  `docs/CHANGELOG.ru.md`, `docs/USER-GUIDE.ru.md`, `docs/TESTING.md`) и
+  корректно откладывают golden baseline acceptance на release runbook, а не
+  выдают его за часть implementation loop.
+- **«Принято предположительно» (§16)** не маскирует продуктовый вопрос под
+  техническое решение — все пять пунктов однозначно технические.
+
+## Чего не проверял
+
+- Реализацию — её нет: ветка содержит один документный коммит
+  (`fc22d9a`, `docs/specs/193-passage-placement-preview.md`), продуктовый код
+  (`src/**`) не менялся — проверено `git show fc22d9a --stat`.
+- Гейты `typecheck`/`test`/`build`/browser smoke/golden — не относятся к
+  этапу ревью ТЗ; предмет будущего код-ревью (PROCESS.md §2.4/§8).
+- `python -m pytest tests_backend` — задача не затрагивает backend ни кодом,
+  ни ТЗ.
+- Реальный визуальный результат (точные пиксельные пороги AC6, computed
+  `fill-opacity` в браузере) — CSS/рендер ещё не написаны; заявленные меры
+  (отдельная стилевая область, semantic pixel guard внутри `.wallbody-fill`)
+  зафиксированы в ТЗ как план, не как исполненный факт.
+- Точность числовых оценок аналитики (ценность 6/10, сложность 3/10, P2) по
+  существу — поле владельца (PROCESS.md §2.2), уже принятое явным решением
+  до написания ТЗ.
+- Возможность технического спора автор/ревьюер по деталям §16 — не
+  возникла: все пять пунктов приняты как разумные без необходимости
+  оспаривать.
+
+## Вердикт
+
+Зелёный. High: 0, Medium: 0, Low: 1 (отсутствует буквальная декларация
+`Touch editor: …`, обязательная по `docs/TOUCH-SUPPORT.md` для новых
+editor-фич; ответ однозначен — best effort, наследуется от существующего
+статуса Plan editor — снимается без возврата ТЗ на правку, с условием
+дописать строку до `S5-ready`). ТЗ подтверждено построчным чтением кода на
+ветке задачи: каждое техническое утверждение §4 оказалось фактом, а не
+догадкой; единственный потенциально продуктовый вопрос уже закрыт
+владельцем в теле issue; AC1–AC6 однозначны, проверяемы и снабжены
+допустимыми способами доказательства; риски названы точно, включая реально
+существующий риск двойного `opacity`.
+
+**Вердикт: зелёный · цикл r1/4 · High: 0 · Medium: 0 → нет · Документ:
+docs/reviews/SPEC-REVIEW-193-r1.md**