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

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 оставлена технической свободой без явной формулы) — ни одна не блокирует приёмку, обе либо правятся косметически при следующей редакции, либо снимаются этой записью без нового цикла.