docs: review document for #172

Issue: #172
User-Visible: no

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

@@ -0,0 +1,272 @@
+# SPEC-REVIEW-172-r1
+
+- **Issue:** https://github.com/Matysh/houseplan-card/issues/172
+- **ТЗ под ревью:** `docs/specs/172-zero-divider-taper.md` (коммит `4582628`,
+  ветка `issue/172-zero-divider-taper`)
+- **Роль:** ревьюер ТЗ (не автор), этап `S4-spec-review`
+- **Трек:** обычный (не `small`/`trivial`) — сложность/риск 6/7 из 10, задача
+  задевает более одной поверхности (Plan, View/kiosk/static, hidden Iso,
+  clean-floor/room fills, Glow/sun) и физическую геометрию, потребляемую всем
+  рендером; критерии лёгкого трека (§5 PROCESS.md: одна поверхность, риск ≤3)
+  не выполняются ни по одному пункту — полный трек и файл ТЗ выбраны верно.
+- **Цикл:** r1/4
+
+## Скоуп ревью
+
+Проверялось соответствие ТЗ:
+
+- `docs/SCOPE.md` — попадание в Core user jobs (J4/J6), отсутствие расширения
+  скоупа за пределы описанного дефекта;
+- `PROCESS.md` §2.4/§2.5 (DoR), §7.1 (обязательные разделы ТЗ), §5 (критерии
+  лёгкого трека), §3/§12 (запреты, включая «догадка вместо решения»);
+- `AGENTS.md` — классы файлов, имя ветки, трейлеры коммита ТЗ;
+- каноническому документу подсистемы `docs/WALL-THICKNESS.md` (модель
+  толщины, growth ±½, mitre/bevel-контракт, единый источник геометрии для
+  всех потребителей);
+- `docs/USER-GUIDE.ru.md` — терминология инструмента «Split»;
+- фактическому коду `src/wall-thickness.ts` (`insetContour()`,
+  `outsetContour()`, `MITRE_LIMIT`) — чтобы диагноз причины в ТЗ не оказался
+  непроверенной догадкой, выданной за факт;
+- полному треду issue #172 — аналитика Codex, вопросы Q1–Q4 с default'ами,
+  решение владельца, финальный комментарий автора со ссылкой на ТЗ.
+
+## Как проверялось
+
+1. Прочитан весь тред issue #172: исходный баг-репорт пользователя (Г-образная
+   комната, Split из внутреннего угла с отклонением 0,5–1° от нормали, один
+   конец разделителя — толщина 0, другой — толщина примыкающей стены),
+   аналитика Codex (воспроизведение на `origin/dev` `a05aa5d` при углах `0°`,
+   `0,477°`, `0,955°`, `1,909°`, `9,462°`, `26,565°`; при точном `0°` дефекта
+   нет), явные вопросы Q1–Q4 с предложенными default'ами, ответ владельца
+   «принимаю все defaults» и финальная публикация ТЗ.
+2. Сверены обязательные разделы ТЗ (§7.1 PROCESS.md) построчно — таблица ниже.
+3. Прочитан код `src/wall-thickness.ts` и построчно сверен диагноз §3 ТЗ:
+   - `insetContour()` (:773-832): при `collinearJoint(uA, uB)` (:808) —
+     специальная ветка, которая кладёт обе точки `pa`/`pb` (offset-точка И
+     исходная вершина нулевой грани при коллинеарном стыке) — совпадает с
+     утверждением ТЗ §7.2 «строго коллинеарный переход сохраняет ступень»;
+   - при **не**-коллинеарном стыке (реальный fixture отклонён на доли/единицы
+     градуса) код идёт в mitre/bevel-ветку (:817-829); при удалённом mitre
+     (`dist > MITRE_LIMIT × maxO`, :821) срабатывает bevel (:827-829):
+     `if (oA > 0) out.push(...)`; `if (oB > 0) out.push(...)`. Если один из
+     offset'ов равен нулю (наш случай: положительная наружная стена ↔
+     нулевой разделитель), в вывод попадает **только одна** точка — офсетная
+     точка толстой грани; исходная вершина нулевой грани не добавляется
+     нигде. Это ровно механизм, который ТЗ §3/§4 (комментарий-анализ)
+     описывает как причину клина: «bevel-ветка сохраняет только смещённую
+     точку толстой грани и теряет исходную вершину нулевой грани». Диагноз
+     точен, не является догадкой.
+   - `outsetContour()` (:1912-1967) зеркально воспроизводит ту же структуру
+     (:1962-1963) — подтверждает утверждение ТЗ §8.2 о необходимости
+     симметричного исправления в inset и outset.
+   - Геометрически прослежен путь клина: в узле стыка (толстая стена → нулевой
+     разделитель) кольцо после bevel соединяет офсетную точку толстой грани
+     напрямую со следующей вершиной вдоль нулевого разделителя (у которой оба
+     соседних offset = 0, значит она остаётся исходной вершиной), образуя
+     прямую от «почти нулевого» смещения до нуля на другом конце — то самое
+     сечение «растёт от 0 до полной глубины стены», описанное в баг-репорте и
+     §3 ТЗ.
+4. Прочитан `docs/WALL-THICKNESS.md` целиком: подтверждён контракт growth ±½,
+   union колец по комнатам, единый источник геометрии для
+   full/static/hidden-isometric и light occlusion (раздел 2–4) — ТЗ §7.4/§8.6
+   продолжает существующую модель, а не изобретает новую. Раздел 8 документа
+   («Independent partitions… same joined set used by Glow, sun and source
+   placement») также согласуется с требованием ТЗ единого физического тела
+   для всех потребителей.
+5. Проверено, что #172 не дублирует #123 (наружный фасад/выход толщины через
+   вершину при Split) и #150 (breakpoint между коллинеарными внешними
+   интервалами разной толщины): прочитан `docs/specs/123-corner-split-wall.md`
+   — там баг про экстерьерный bbox и наружный зуб от острого митра, здесь —
+   про внутреннюю нулевую границу и bevel, теряющий вершину. Разные механизмы,
+   разные условия срабатывания (там — вершина исходной комнаты, здесь —
+   стык offset>0 / offset=0 в bevel-ветке). Не дубликат.
+6. Проверена терминология: «Split» в ТЗ совпадает с `docs/USER-GUIDE.ru.md:340`
+   («Split | Делит комнату путём от одной стены до другой»); термины «masonry»,
+   «mitre», «bevel», «cap» — это уже принятая в `docs/WALL-THICKNESS.md`
+   английская терминология подсистемы, не изобретены автором ТЗ.
+7. Проверен явно фактический фикстур §3: полигон
+   `[100,100]–[900,100]–[900,800]–[600,800]–[600,400]–[100,400]`, Split
+   `[600,400]→[900,402.5]` даёт `atan(2.5/300) ≈ 0,477°`, а `[900,405]` даёт
+   `atan(5/300) ≈ 0,955°` — числа в ТЗ внутренне согласованы, не выдуманы.
+8. Проверено соответствие `docs/CONFIG-COMPATIBILITY.md`: задача не создаёт
+   нового compatibility-случая (не меняет persisted-представление `RoomCfg`/
+   `WallEntry`), что подтверждено и содержанием реестра (нет полей,
+   относящихся к разделителям/толщине, требующих отдельной миграции).
+9. Проверен явный технический блок §16 «Принятые технические предположения»:
+   все пять пунктов — про место реализации, эпсилон в тестах, отсутствие
+   отдельной post-render маски, переиспользование fixture #123 и границу с
+   возможным отдельным багом boolean-библиотеки — технические, не продуктовые,
+   корректно не эскалированы владельцу (PROCESS.md §7.1: «владельцу — только
+   продуктовые вопросы»).
+10. Проверена трассируемость: `docs/specs/README.md:94` обновлён тем же
+    коммитом `4582628`; ссылка issue → ТЗ и ТЗ → issue двусторонняя.
+    `git diff --stat origin/dev...HEAD` показывает только
+    `docs/specs/172-zero-divider-taper.md` и `docs/specs/README.md` (класс C,
+    ни одного файла класса A — правило №1 не нарушено на этапе ТЗ). Коммит
+    несёт `Issue: #172`, `User-Visible: no` — верно для документа ТЗ, который
+    сам не меняет поведение продукта.
+11. Проверено существование файлов, которые ТЗ называет предполагаемыми:
+    `test/wall-thickness.test.mjs`, `demo/smoke_split_corner_wall.mjs`,
+    `demo/smoke_wall_junctions.mjs`, `demo/smoke_wall_thickness.mjs` — все
+    существуют; новый `demo/smoke_zero_divider_taper.mjs` не пересекается по
+    смыслу с существующим `demo/smoke_split_nonsnap.mjs` (тот проверяет
+    Split на не-grid-aligned полигоне, а не переход толщины на нулевой
+    границе) — не дублирует существующее покрытие.
+
+## Обязательные разделы (§7.1 PROCESS.md)
+
+| Раздел | Есть | Комментарий |
+|---|---|---|
+| Сценарий (персона/поверхность/момент) | ✅ | §1 — администратор дома, desktop Plan editor, момент завершения Split почти вдоль плеча угла |
+| Что человек увидит до/после | ✅ | §2, «До:»/«После:» — см. Low-1 |
+| Проблема (с подтверждённой причиной) | ✅ | §3, причина проверена построчно по коду (см. «Как проверялось» п.3) |
+| Скоуп / не-скоуп | ✅ | §5 / §6, явные границы (без snap, без нового UX, без изменения `MITRE_LIMIT` для двух положительных толщин, без миграции) |
+| Контракт поведения | ✅ | §7 (геометрия) + §8 (архитектурные ограничения реализации) |
+| Модель данных и миграция | ✅ | §9 — явное «форматы не меняются», «читается исправленно, без записи» |
+| UX, i18n, accessibility, touch | ✅ | §10, явно `Touch editor: best effort / intentionally degraded` — буквальная канон-метка `docs/TOUCH-SUPPORT.md` присутствует |
+| AC1…ACn с доказательством | ✅ | §11, 11 штук, каждый помечен `unit`/`smoke`/`golden`/«ревью кода» |
+| План автотестов | ✅ | §12, разбит на unit / browser smoke / golden и pre-release / implementation loop |
+| Риски | ✅ | §13, таблица риск → мера, плюс performance/security |
+| Откат | ✅ | §14 |
+| Release-артефакты | ✅ | §15, конкретный список: оба changelog, `WALL-THICKNESS.md`, тесты, golden, три копии бандла |
+
+Все обязательные разделы присутствуют и содержательны. Дополнительно есть
+раздел решений владельца (§4, фиксирует принятые Q1–Q4) и явный блок принятых
+технических предположений (§16) — соответствует требованию PROCESS.md §7.1
+отделять продуктовое решение от технического и не выдавать догадку за факт.
+
+## Находки
+
+Находок уровня **High** и **Medium** нет — новых issue не требуется.
+
+### Low-1 — «что человек увидит» длиннее одной фразы
+
+**Файл:** `docs/specs/172-zero-divider-taper.md:26-34` (§2)
+
+PROCESS.md §7.1 требует «одной фразой, без терминов реализации». Раздел
+написан двумя-тремя предложениями на «До:»/«После:» (например, «После:»
+содержит два предложения). По существу требование выполнено — язык
+исключительно визуальный («клин», «толщина», «стык»), без имён функций или
+внутренних терминов, — но формально это не «одна фраза». Тот же класс
+находки уже фиксировался как Low и не блокировал приёмку в
+`SPEC-REVIEW-141-r1` (Low-2) и `SPEC-REVIEW-137-r1`.
+
+**Решение ревьюера:** Low, не блокирует. Косметическая правка на усмотрение
+автора при следующей редакции.
+
+### Low-2 — эпсилон/half-depth границы локального cap не формализованы числом
+
+**Файл:** `docs/specs/172-zero-divider-taper.md:124-125, 338-339` (§7.1, §16.2)
+
+Контракт требует, чтобы локальная область примыкания «ограничена физической
+half-depth примыкающей стены и геометрическим epsilon» и «не может расти
+пропорционально длине D», но конкретная формула эпсилон (как, например,
+`max(4% × grid pitch, 1e-9)` для «effectively collinear» в
+`docs/WALL-THICKNESS.md`) не приведена — §16.2 явно оставляет её тестовой
+стратегии автора кода. В `src/wall-thickness.ts` уже есть несколько
+устоявшихся эпсилон-констант (`openEps(pitch, coordScale)`,
+`pitch * coordScale * 0.02`, `1e-9`), так что это не белое пятно, а
+осознанно оставленная техническая свобода, корректно помеченная как
+предположение, которое ревьюер вправе оспорить, но переносить в
+продуктовый вопрос владельцу нет оснований — граница «не растёт
+пропорционально длине» уже достаточна как проверяемый инвариант для AC2.
+
+**Решение ревьюера:** Low, не блокирует. Код-ревью должно убедиться, что
+выбранная константа действительно не масштабируется с длиной грани (что уже
+явно требует AC2), а не что она равна конкретному числу.
+
+## Что проверено и корректно
+
+- **Соответствие `docs/SCOPE.md`:** задача закрывает **J4** («от нуля до
+  плана без Inkscape/YAML» — Split не должен молча создавать кладку, которую
+  пользователь не задавал) и **J6** («план остаётся правдивым по мере
+  развития» — сохранённые планы должны отображаться корректно без ручной
+  переработки). Обе строки Closed, это исправление дефекта внутри принятой
+  функциональности, а не расширение продукта. Общая физическая геометрия
+  также поддерживает согласованность J1/J2/J3 между View и Plan, как верно
+  указано в §1 ТЗ.
+- **Легитимность полного трека:** сложность/риск 6/7 из 10, минимум пять
+  затронутых поверхностей (Plan, View/kiosk/static, hidden Iso, floor/room
+  fills, Glow/sun barriers) — критерии `small` (§5 PROCESS.md, одна
+  поверхность, риск ≤3) не выполняются; полный трек и отдельный файл ТЗ
+  выбраны верно, лёгкий трек владелец и автор корректно не применили.
+- **Продуктовые вопросы закрыты по процессу:** Q1–Q4 заданы одним пакетным
+  комментарием, каждый с предлагаемым default, задача корректно ушла в
+  `blocked`+`S3-spec` до ответа и вышла из `blocked` сразу после решения
+  владельца (все четыре ответа — «да»). Ни один технический вопрос не был
+  ошибочно вынесен владельцу — §16 явно и полностью перечисляет технически
+  свободные решения.
+- **Технический диагноз не голословен.** Причина клина (bevel-ветка
+  `insetContour()`/`outsetContour()` теряет исходную вершину нулевой грани
+  при близком, но не точном коллинеарном стыке) построчно проверена по
+  исходному коду `src/wall-thickness.ts:773-832, 1912-1967` и совпадает с
+  описанием в ТЗ — см. «Как проверялось» п.3. Числовые фикстуры (углы
+  `0,477°`/`0,955°`) внутренне согласованы с геометрией §3.
+- **Не дубликат #123/#150:** механизм и условие срабатывания различны,
+  проверено по `docs/specs/123-corner-split-wall.md`; ТЗ корректно
+  разграничивает три задачи в одном абзаце §3.
+- **Не-скоуп (§6) корректно отсекает соседние соблазны:** snap почти
+  перпендикулярного/коллинеарного Split, изменение допустимости или диалога
+  Split, автоназначение толщины, переработка модели данных, изменение
+  `MITRE_LIMIT` для пар двух положительных offset'ов, публикация изометрии
+  как отдельной фичи — все явно исключены с указанием причины.
+- **Регрессионные гарантии сформулированы явно:** AC3/AC9 поимённо защищают
+  точный коллинеарный/ортогональный переход, пары `1↔15`/`15↔100`, corner
+  Split, wall junctions, wall thickness, opening tunnels — то есть именно те
+  сценарии, которые уже используют ту же `insetContour()`/`outsetContour()` и
+  могли бы негласно пострадать от исправления.
+- **Модель данных и миграция (§9):** корректно заявлено «форматы `RoomCfg` и
+  `WallEntry` не меняются», «чтение и рендер не записывают конфигурацию», без
+  прямой/обратной миграции — сверено с `docs/CONFIG-COMPATIBILITY.md`, задача
+  не создаёт нового compatibility-случая.
+- **UX/touch (§10):** буквально использует канон-формулировку
+  `docs/TOUCH-SUPPORT.md` («Touch editor: best effort / intentionally
+  degraded») и отдельно фиксирует safety floor: View/kiosk/static —
+  блокирующие поверхности, сохранённая геометрия не зависит от типа
+  указателя.
+- **Release-артефакты (§15)** перечисляют оба changelog в одном
+  implementation-коммите, `docs/WALL-THICKNESS.md`, конкретные
+  unit/smoke/golden-файлы и три синхронные копии бандла — соответствует
+  §7.1/правилу 11 PROCESS.md. Golden корректно ограничен только
+  `npm run golden:accept -- --reviewed` по полному Linux-артефакту на
+  предрелизном этапе (§12, согласуется с PROCESS.md §8/§11.4).
+- **Дисциплина «тест должен уметь падать»:** AC1 прямо требует, чтобы фикстура
+  §3 краснела на исходном `dev` из-за taper-клина, и явно передаёт эту
+  проверку на код-ревью («Ревьюер фиксирует эту проверку в code review») —
+  соответствует требованию PROCESS.md §2.7/§18.
+- **Трассируемость:** `docs/specs/README.md:94` обновлён тем же коммитом
+  `4582628`; ветка `issue/172-zero-divider-taper` и трейлеры (`Issue: #172`,
+  `User-Visible: no`) корректны для документа класса C, который сам не меняет
+  поведение. `git diff --stat origin/dev...HEAD` не содержит ни одного файла
+  класса A — продуктовый код не тронут до `S5-ready` (правило №1).
+
+## Чего не проверял
+
+- Не проверял реализуемость конкретного алгоритма из §8.2 («сохранить обе
+  точки в детерминированном порядке») как единственно возможного решения —
+  это техническая свобода автора кода (§16), а не предмет ревью ТЗ; код-ревью
+  должно будет проверить сам факт отсутствия taper, а не конкретный порядок
+  вставки точек.
+- Не запускал автотесты, `golden`, browser-смоки или performance-профили — на
+  этапе `spec` это не требуется; существование названных в ТЗ файлов
+  (`test/wall-thickness.test.mjs`, `demo/smoke_split_corner_wall.mjs`,
+  `demo/smoke_wall_junctions.mjs`, `demo/smoke_wall_thickness.mjs`) и
+  структура `insetContour()`/`outsetContour()` проверены чтением кода, а не
+  исполнением.
+- Не проверял корректность конкретных числовых оценок аналитики (8/10 · 6/10 ·
+  7/10 · P2) по существу — это поле владельца (PROCESS.md §2.2), уже принято
+  явным решением владельца до написания ТЗ.
+- Не проверял связанные issue #123/#150 по существу за пределами того, что
+  понадобилось для верификации отсутствия дублирования (различие механизма и
+  условий срабатывания) — они не входят в предмет этого ревью.
+- Не проверял, обнаружится ли в ходе реализации отдельный дефект
+  boolean-библиотеки (упомянутый как риск в §16.5) — это явно вынесено в
+  будущий отдельный issue, если случится, и не влияет на приёмку ТЗ сейчас.
+
+## Вердикт
+
+Зелёный. High: 0, Medium: 0. Две находки Low (раздел «что человек увидит»
+длиннее одной фразы; численная граница локального cap оставлена технической
+свободой без явной формулы) — ни одна не блокирует приёмку, обе либо
+правятся косметически при следующей редакции, либо снимаются этой записью без
+нового цикла.