docs: review document for #309

Issue: #309
User-Visible: no

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

@@ -0,0 +1,191 @@
+# SPEC-REVIEW-309-r1
+
+Issue: https://github.com/Matysh/houseplan-card/issues/309 «Стыковочные узлы: визуальный лимит mitre и устранение паразитных парных патчей»
+Этап: spec (обычный трек — сложность/риск владельца 6/10 и 7/10, `small` невозможен)
+Заход: r1 · блокирующих циклов израсходовано 0 из 4
+Ревьюер: Claude (сессия ревью ТЗ), артефакт: `docs/specs/309-junction-visual-limit.md` в ветке `issue/309-junction-visual-limit`, коммит `ff623cd4` (только этот файл; `git diff origin/dev...HEAD --stat` — 1 файл, 72 строки, никакого кода).
+
+## Скоуп
+
+ТЗ описывает три класса визуальных артефактов кладки в узлах стыка стен (шип, горб,
+ступенька), обнаруженных владельцем на реальном экспорте после #302 (полный mitre).
+Решения владельца зафиксированы 2026-08-25 в аналитике issue: визуальный порог среза
+`1.5·max(h)` вместо санитарного `MITRE_LIMIT=4`, форма среза — плоская фаска
+перпендикулярно биссектрисе, устранение паразитных mitre-патчей между не-соседними
+по азимуту лучами узла. Продуктовая рамка — рендер кладки стен как таковой, то есть
+J1 SCOPE.md («show the whole home... spatially», точность отображения) и J6
+(«keep the plan true»): задача не расширяет и не меняет job, чинит форму существующей
+геометрии, принятой в #302. Соответствие подтверждаю.
+
+## Как проверялось
+
+Не на веру автору — с чтением текущего кода `src/wall-thickness.ts`,
+`src/physical-geometry.ts`, `demo/golden/matrix.mjs`, `docs/WALL-THICKNESS.md`,
+`docs/specs/README.md`, `PROCESS.md` §2.4/§2.5/§7.1/§7.2, `AGENTS.md`, `docs/SCOPE.md`:
+
+1. `src/wall-thickness.ts:80` — `MITRE_LIMIT = 4` существует и является ровно тем
+   санитарным пределом, о котором говорит §3.1 ТЗ.
+2. `src/wall-thickness.ts:1082-1150` (`linearWallJoinPatches`) — построчно
+   подтверждает: (a) функция перебирает **все** пары лучей `i<j` без фильтра
+   соседства по азимуту, хотя лучи уже отсортированы по `atan2` (строка ~1125) —
+   это буквально подтверждает механизм «ступеньки», описанный автором во втором
+   комментарии issue (паразитный патч между не-соседними лучами); (b) строка 1144
+   — `limit = MITRE_LIMIT * Math.max(a.halfDepth, b.halfDepth)` — ровно та ветка,
+   на которую ссылается аналитика issue как источник «шипа»; контракт §3.2 ТЗ
+   правит именно эту ветку.
+3. `src/wall-thickness.ts:2087` (веера `junctionNodeGeometry`) — тот же
+   `MITRE_LIMIT`-предел на ветке фанов, соответствует ссылке аналитики «:2087» и
+   контракту §3.4 ТЗ.
+4. Проверил область действия `linearWallJoinPatches` по всем вызовам:
+   `src/physical-geometry.ts:242` (канонические draft/partition-тела),
+   `src/houseplan-card.ts:19961` (live-превью рисования),
+   `src/wall-thickness.ts:1211` внутри `drawWallPreviewD` (превью открытого
+   контура). Все три — один и тот же экспорт. Это прямо подтверждает заявление
+   §3.5 ТЗ «превью использует ту же парную логику» — не декларация, а следствие
+   того, что все три сайта вызывают одну функцию; менять код в одном месте нельзя.
+5. `src/wall-thickness.ts:2166` (`junctionContractHoles`) и
+   `demo/smoke_junction_holes.mjs` — оба существуют, соответствуют ссылкам §3.5/AC5.
+6. `demo/golden/matrix.mjs:577-621` — блок `#302: junction close-ups` содержит
+   ровно **15** сцен `junction-*` (посчитано `grep -c`) плюс отдельно
+   `junction-owner-repro-dark` (строка 620) = 16 сцен итого, не 16 крупноплановых
+   + repro = 17, как можно прочитать в тексте ТЗ (см. Находки, Low).
+7. `docs/specs/README.md` — «Обязательные release-артефакты номерного ТЗ»: явный
+   перечень (changelog RU+EN, затронутая документация, golden/screenshots и способ
+   review) обязателен, если задача меняет пользовательское поведение. Рендер стыков
+   — видимое поведение (форма кладки в Plan/View/kiosk/Static/hidden-Iso, общий
+   структурный кэш по `docs/WALL-THICKNESS.md` §3), значит раздел обязателен.
+8. `PROCESS.md` §7.1 — обязательные разделы ТЗ: сценарий · что человек увидит ·
+   проблема · скоуп и не-скоуп · контракт · UX · модель данных и миграция · i18n ·
+   AC1…ACn · план автотестов · риски · откат · release-артефакты.
+9. `git log -1 --format=%B ff623cd4` — трейлеры `Issue: #309` / `User-Visible: no`
+   на месте (документация, код не менялся).
+
+## Находки
+
+### Medium (в скоупе задачи — правится в этом же документе)
+
+**Отсутствуют несколько обязательных разделов §7.1, включая оба продуктовых.**
+
+Файл `docs/specs/309-junction-visual-limit.md` содержит: Проблема (§1), Решения
+владельца (§2), Контракт (§3), Затрагиваемые поверхности (§4), AC (§5), План
+тестов (§6), Откат (§7). Отсутствуют как отдельные разделы:
+
+- **Сценарий** — какая персона, на какой поверхности, в какой момент встретит
+  изменение (PROCESS.md §7.1, первый из двух обязательных продуктовых разделов).
+  Проблема формулирует технический механизм (mitre, лучи, паразитные патчи), но
+  не называет персону/поверхность. Это не мелочь: рендер узлов — общий структурный
+  кэш (`docs/WALL-THICKNESS.md` §3: «Full and Static render paths reuse the same
+  structural cache»), поэтому фикс проявится не только в Plan editor (где владелец
+  заметил дефект), но и в View, kiosk, Static, hidden-Iso — у всех трёх персон
+  SCOPE.md, не только у Home admin. Это технический факт, не продуктовый вопрос —
+  автор может и должен написать его сам, без обращения к владельцу.
+- **Что человек увидит до и после**, одной фразой без терминов реализации (второй
+  обязательный продуктовый раздел). Ближайшее к этому — техническое описание в §1
+  (шип/горб/ступенька), но это не заменяет требуемую формулировку «до: кладка узла
+  выступает зубцом/пикой/уступом за естественный контур; после: кладка узла
+  ограничена аккуратной фаской, без выступов».
+- **Скоуп и не-скоуп** как явный раздел (сейчас скоуп восстанавливается только из
+  §4 «Затрагиваемые поверхности», не-скоуп не сформулирован вовсе — например, не
+  сказано explicitly, что #249-шеврон для near-orthogonal узлов (#279) и общая
+  политика `thickLength` (#271) не пересматриваются, кроме одной фразы в §3.4).
+- **UX** — раздел отсутствует; для чисто рендер-геометрической задачи корректный
+  ответ короткий («интерактивных элементов нет, только форма кладки»), но раздел
+  должен присутствовать явно, а не подразумеваться.
+- **Модель данных и миграция** — не упомянуто явно. Ответ тривиален («нет модели
+  данных, нет миграции — чистая рендер-геометрия из уже сохранённых `walls`/
+  `partitions`»), но DoR (§2.5) требует явного «нет», а не молчания.
+- **i18n** — не упомянуто явно. Ответ тривиален («нет новых строк»), нужен явный
+  раздел с этим ответом (DoR §2.5).
+- **Риски** — не назван явный раздел (частично риск виден из §3.3 — «решение
+  фиксируется при реализации измерением» — но общего перечня рисков нет: например,
+  риск того, что после запрета несоседних пар в узлах ≥3 лучей часть существующих
+  16 golden-сцен изменится непредсказуемо и потребует пересмотра больше, чем
+  ожидается).
+- **Release-артефакты** — не выделены как раздел. §4 упоминает
+  `docs/WALL-THICKNESS.md §3`, но нет явного перечисления `docs/CHANGELOG.md` +
+  `docs/CHANGELOG.ru.md` (обязательны по `docs/specs/README.md`, поскольку меняется
+  пользовательское поведение — форма отображаемой кладки) и способа ревью golden
+  (`--reviewed`, кто и как принимает изменившиеся сцены).
+
+Все перечисленные пункты решаются самим автором без вопроса владельцу — ни один
+не требует продуктового решения, которого ещё нет (персона/поверхность вытекают
+из общего рендер-пайплайна, i18n/миграция/UX ответы тривиальны «нет»,
+release-артефакты — механическое перечисление). Без High-находок это жёлтый
+вердикт: разделы дописываются в этом же файле, следующий заход проверяется по
+дельте (§2.10).
+
+### Low (не блокирует, отмечаю для полноты)
+
+- §3.5 и §4 повторяют формулировку «Все 16 junction-сцен» из
+  `docs/WALL-THICKNESS.md` («Junction tooling»: «sixteen close-up golden scenes
+  (`junction-*`) plus the owner's repro scene»). Фактически в
+  `demo/golden/matrix.mjs:580-615` ровно **15** близких-планов `junction-*`
+  (посчитано `grep -c`), плюс `junction-owner-repro-dark` — итого 16 сцен, не
+  16+1=17. Формулировка ТЗ («Все 16 junction-сцен + junction-owner-repro-dark»)
+  наследует ту же неточность канонического документа. Не блокирует ни один AC —
+  AC6 («весь сет зелёный») не зависит от точного числа, golden-раннер прогоняет
+  фактический список сцен, а не названное количество. Рекомендация: при правке
+  раздела не привязываться к числу («весь текущий junction-сет»), либо явно
+  посчитать `grep -c "id: 'junction-" demo/golden/matrix.mjs` перед фиксацией
+  цифры — это тот же класс дефекта, о котором предупреждает `AGENTS.md`
+  («Never copy test counts into documents by hand; they go stale»), просто
+  применённый не к тестам, а к golden-сценам. Снимаю без правки по решению
+  ревьюера: не влияет на проверяемость ни одного AC.
+
+## Что проверено и корректно
+
+- Обязательные технические разделы §7.1 (проблема, контракт поведения, AC1…7 с
+  доказательством, план автотестов, откат) присутствуют и полны.
+- Корневые причины всех трёх артефактов не являются догадкой: автор подтвердил
+  их исполнением на реальном экспорте (dev @ dc68868) и указал точные строки кода
+  (`:80`, `:1144`, `:2087`), которые я перепроверил построчно — они существуют и
+  делают ровно то, что написано в issue и в ТЗ.
+- Инвариант «превью = персист» (§3.5, п.3) — не заявление, а прямое следствие
+  того, что `linearWallJoinPatches` имеет единственный экспорт и три вызывающих
+  сайта (`physical-geometry.ts:242`, `houseplan-card.ts:19961`,
+  `wall-thickness.ts:1211`); правка одной функции автоматически меняет все три.
+- Порог `1.5·max(h)` и граница «прямые/тупые углы не затрагиваются» (AC4)
+  математически корректны: для равных толщин под 90° вершина mitre лежит на
+  `h·√2 ≈ 1.41h`, что действительно `< 1.5h` — контракт не ломает обычные углы.
+- Контракт §3.3 (запрет паразитных пар в узлах ≥3 лучей) технически реализуем на
+  существующей структуре: лучи в `linearWallJoinPatches` уже сортируются по
+  `atan2` перед перебором пар (строка ~1125), так что «соседние по азимуту»
+  проверяется без новой инфраструктуры — просто ограничением диапазона `j`.
+- Мутанты AC7 (a)-(d) однозначно целятся в четыре разных пункта контракта
+  (порог, форма среза, фильтр соседства, направление фаски) и их легко отличить
+  друг от друга — не избыточны и не дублируют друг друга.
+- «Без дыр» (#302) и «без фантомов» (#271) named как обязательные инварианты с
+  конкретными существующими инструментами (`junctionContractHoles`,
+  `smoke_junction_holes.mjs`, `thickLength`-лимиты) — все три существуют и делают
+  заявленное.
+- Аналитика issue и передача в S4 (владелец) корректно ссылаются на #302/#271 как
+  на предшествующие задачи, не пересекаются с #303 (заявлено и не противоречит
+  прочитанным документам).
+- Трейлеры коммита `ff623cd4` (`Issue: #309`, `User-Visible: no`) корректны для
+  чисто документационного коммита.
+
+## Чего не проверял
+
+- Не прогонял `npx tsc --noEmit` / `npm test` / `npm run build` — на этом этапе
+  нет изменений кода (`git diff origin/dev...HEAD` — один файл документации),
+  гейты кода относятся к этапу код-ревью (§2.7).
+- Не запускал `npm run golden:capture`/`golden:verify` и не пересчитывал вручную
+  контур `junctionContractHoles` на реальном экспорте владельца — фикстура
+  `test/fixtures/309-junction-teeth.json` из плана тестов ещё не создана, это
+  предмет реализации и последующего код-ревью.
+- Не проверял файл экспорта владельца (`houseplan-space-convergence-test-...json`)
+  построчно на точное число «13 комнат/24 перегородки» (AC5) — доверяю
+  подтверждённому исполнением заявлению автора аналитики; при код-ревью это
+  число будет видно по фикстуре теста.
+- Не оценивал `docs/USER-GUIDE.ru.md` на предмет новой терминологии — задача не
+  вводит пользовательских терминов (чистая геометрия рендера), проверка
+  нерелевантна.
+
+## Вердикт
+
+Жёлтый. High-находок нет. Один Medium **в скоупе** задачи: отсутствуют
+обязательные разделы ТЗ §7.1 (Сценарий, Что человек увидит, явный Скоуп/не-скоуп,
+UX, Модель данных и миграция, i18n, Риски, Release-артефакты) — все решаются
+автором самостоятельно, без вопроса владельцу, и дописываются в этом же файле.
+Технический контракт (§3.1-3.5) и критерии приёмки (AC1-7) при этом полны,
+однозначны, обоснованы прочтением кода и не содержат догадок, выданных за факт.