diff --git a/docs/reviews/SPEC-REVIEW-302-r1.md b/docs/reviews/SPEC-REVIEW-302-r1.md new file mode 100644 index 00000000..4b04d764 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-302-r1.md @@ -0,0 +1,154 @@ +# SPEC-REVIEW-302-r1 + +- Issue: [#302](https://github.com/Matysh/houseplan-card/issues/302) — «Артефакты на стыках стен: дыры в union-геометрии. Переработка механизма стыков + полноценный сет скриншот-тестов» +- ТЗ: `docs/specs/302-junction-node-material.md` +- Ветка: `issue/302-junction-node-material`, ревьюемый SHA: `3b19111aa5894f624e608b9f7fef31736a4ecdf5` (родитель `4b8f17ba`, ровно вершина `dev` на момент написания ТЗ) +- Заход: r1 · блокирующих циклов израсходовано 0/4 +- Класс изменения: A (продукт), трек: обычный (не `small`) — корректно, задача явно затрагивает perf-бюджет и переработку подсистемы, критерии лёгкого трека не выполняются +- Вердикт: **зелёный** + +## Скоуп ревью + +Первый заход, разбор полный (§2.9 не применяется — нет r0). Проверялись: +тело issue #302 и единственный комментарий автора, `docs/specs/302-junction-node-material.md` +целиком, `docs/SCOPE.md`, `PROCESS.md` §1–§9, `docs/WALL-THICKNESS.md` целиком, +исходник `src/wall-thickness.ts` (4145 строк) — выборочно, по каждому +идентификатору и утверждению ТЗ, требующему проверки на существование/точность. + +## Как проверялось + +1. **Продуктовая рамка.** `docs/SCOPE.md` — задача улинии с миссией + («live spatial overview») через качество геометрии плана; это не новая + фича, а устранение искажений в самом визуальном представлении дома — + прямое следствие J1. Конфликта со SCOPE нет. +2. **Процесс.** Issue помечен `S4-spec-review`, что и запускает этот этап; + размер задачи (обычный) обоснован — трогает `src/wall-thickness.ts` + целиком, влияет на perf-бюджет, не подходит под критерии `small` (§5 + PROCESS.md требует «нет влияния на производительность» — здесь влияние + прямо названо в §10 ТЗ). +3. **Фактчекинг утверждений о коде и истории**, чтобы отделить подтверждённые + факты от гипотез, выданных за решение (обязательный пункт инструкции): + - `MITRE_LIMIT`, `openEps`, `buildMultiWallNodeMap`, `wallIntervals`, + `multiWallNodeAt`, `outsetContour`/`insetContour`, + `bevelMultiWallBody`/`bevelMultiWallPaper`, `unionJunctionPatches`, + `linearWallJoinPatches`, `stableJunctionPatch`, `wallBodiesGeometry`, + `wallBodiesUnionPath` — все существуют в `src/wall-thickness.ts` ровно с + тем поведением, которое им приписывает ТЗ (grep + прочтение сигнатур). + - `EPS_NODE = openEps × 4` — совпадает буквально с вызовом в + `multiWallNodesForGeometry` (`wall-thickness.ts:2035-2036`). + - `wallBodiesUnionPath` действительно единственная точка входа для обоих + рендереров: `src/houseplan-card.ts:11687` (Full/Plan) и + `src/space-render.ts:432` (Static) — вызывают именно её. Утверждение §2 + ТЗ «оба рендерера получают результат из одного и того же кода» не + догадка, а факт. + - Ссылки на прежние ТЗ (#141, #172, #197, #229, #249, #271, #272, #275, + #279, #288, #290, #296) сверены с содержанием `docs/WALL-THICKNESS.md` — + каждая деталь («R = 1.25×H», `NEAR_AXIS_MAX_DEGREES = 0.25`, + «near-orthogonal», «short ray handoff», сращивание коллинеарных равных + стен из #229) находит буквальное соответствие в каноническом документе + подсистемы либо в исходнике (`src/near-axis.ts:6-7`). + - Все девять смоков из AC9 (`smoke_wall_junctions`, + `smoke_junction_patch_resilience`, `smoke_split_corner_wall`, + `smoke_zero_divider_taper`, `smoke_wall_thickness*`) и + `smoke_render_perf` (AC8) существуют в `demo/`. + - `demo/golden/matrix.mjs` существует и это действительно data-only + каталог golden-фикстур (`GOLDEN_MATRIX_VERSION`), куда естественно + ложится новый сет из §13. + - Нового «детектора дыр» и `isPointInPath`-инструментария в репозитории + сейчас нет (`grep` пусто) — заявка «новый чистый модуль» корректна, это + не дублирование существующего. + - SHA `4b8f17b`, на котором получены цифры 359/360 дыр, существует в + истории и совпадает с прямым родителем ревьюемого коммита — воспроизвести + довод «дыры родились в базовой фазе» можно на том же дереве. +4. **Смысловая проверка контракта §8**, а не только буквенная. Рассмотрены + вырожденные и граничные случаи: узел из 2 лучей (веер строится на обеих + угловых секторах — внутренней и внешней стороне, поскольку у двух лучей при + круговом обходе ровно два соседства); дегенерат mitre при малом угле + (обрезается `MITRE_LIMIT`, переходит в bevel — предотвращает уход материала + в бесконечность); связь с историческим классом дефектов «короткий луч + короче радиуса ремонта» (#271/#288) — новая механика по построению не + продолжает луч вдоль его оси вовсе («конец полосы» — точка ровно у узла, + офсетная по перпендикуляру, а не отложенная на расстояние `MITRE_LIMIT` + вдоль луча), поэтому класс дефектов «репэйр перерос короткий луч» не имеет + аналога в новой механике — не пробел ТЗ, а архитектурное упрощение, которое + стоит явно назвать в фиксации, но не блокирует ревью. +5. **Проверка DoR-состава** (§2.5, §7.1 PROCESS.md): сценарий ✓, «что человек + увидит» ✓, проблема/причина ✓ (§3 с исполненным доказательством), скоуп и + не-скоуп ✓ (§6–7), контракт поведения ✓ (§8), данные/i18n/a11y/privacy ✓ + (§9, всё «не меняется» — обосновано отсутствием миграции конфига), perf ✓ + (§10, бюджет назван), риски ✓ (§11, три риска с конкретными мерами), AC ✓ + (§12, 9 штук, пронумерованы), план автотестов ✓ (§13–14), откат ✓ (§16), + release-артефакты ✓ (§15). + +## Находки + +Ни одной находки уровня High или Medium. Два пункта Low — решение ревьюера: +сняты с записью, правки не требуют. + +1. **Low — способ доказательства AC не продублирован буквально на каждой + строке §12.** AC1–AC5, AC8 описывают ожидаемый результат словами + («детектор §8.4», «попиксельное совпадение», «смок сверяет пути»), а не + явным тегом `unit`/`golden`/`smoke` при каждом пункте, как того просит + буква §2.5 PROCESS.md. Решение: снимается без правки — §13 однозначно + называет механику («каждая сцена: golden крупным планом + детектор §8.4», + «юниты: чистые функции веера... и детектора»), и разработчик/ревьюер кода + не может трактовать способ доказательства иначе. Формальный тег ничего не + добавил бы к однозначности. +2. **Low — нет отдельного раздела «UX».** Шаблон §7.1 называет UX отдельным + разделом; в этом ТЗ его содержание разнесено между шапкой («Touch editor: + not exposed») и §2 («Что человек увидит до и после»). Решение: снимается — + задача не меняет ни одного интерактивного пути (только геометрия отрисовки + тел стен), содержательно вопрос закрыт, отдельный заголовок был бы пустой + формальностью. + +Открытых продуктовых вопросов к владельцу нет и это обоснованно: контракт +поведения продиктован самим владельцем в комментарии к issue (кандидат-алгоритм +из тела issue почти буквально формализован в §8.2), формальные константы +детектора отмечены как техническое предположение в §17 с направлением +уточнения («только в сторону строгости»), стадийность демонтажа старых слоёв — +там же, помечено как свободное для реализации техническое решение. Это +редкий случай, когда молчание не является риском: сам факт задачи и её решение +исходят от одного и того же человека. + +## Что проверено и корректно + +- Соответствие SCOPE.md — задача в рамках мандата, не расширяет функциональность. +- Соответствие процессу — статус, класс изменения, трек, обязательные разделы, + трейлеры/release-артефакты названы верно. +- Все технические идентификаторы, числа и ссылки на историю, на которые + опирается контракт §8, проверены на существование и точность прямым чтением + кода и `docs/WALL-THICKNESS.md`, а не приняты на слово. +- Контракт §8.2–8.4 логически замкнут: покрывает все перечисленные в §8.3 + вырожденные случаи, не оставляет геометрического механизма для дыры между + соседними веерами (общая вершина — узел, дуги секторов покрывают полный + оборот), и делает объективно проверяемым единственное новое утверждение + («нет дыр») через детектор с собственной самопроверкой на дырявой фикстуре + (мутант `hole-detector-blind`). +- AC9 корректно называет регрессионный периметр — все перечисленные смоки + существуют и по названию действительно покрывают стыковую механику. +- Откат (§16), release-артефакты (§15) и риски golden-переснятия (§11.1) + учитывают процессные уроки #230/#235 именно так, как того требует PROCESS.md. + +## Чего не проверял + +- Я не запускал никакие гейты (`tsc`, `npm test`, `npm run build`, + `golden:verify`, смоки) — на этапе ревью ТЗ кода ещё нет, гейты этапа + относятся к код-ревью (§2.7 PROCESS.md), а не к ревью ТЗ (§2.4). +- Не проверял геометрию mitre/bevel численно (не строил прототип и не считал + вручную пересечения офсетных прямых) — на этапе ТЗ это не требуется: контракт + верифицируется исполнением на код-ревью через детектор §8.4 и golden, а не + через ручной пересчёт геометрии ревьюером ТЗ. +- Не читал полностью весь `src/wall-thickness.ts` (4145 строк) построчно — + прочитаны и сверены только фрагменты, релевантные утверждениям ТЗ + (сигнатуры функций, константы, точки вызова `wallBodiesUnionPath`). +- Не проверял содержание единственного комментария к issue построчно на + предмет более ранних версий ТЗ — комментарий один, зафиксированное решение + соответствует финальному ТЗ. + +## Итог + +`docs/specs/302-junction-node-material.md` готово к переходу в `S5-ready`. +Продуктовая рамка ясна, контракт поведения проверяем объективным детектором и +golden-эталонами, риски демонтажа старого стека названы и покрыты +регрессионным периметром (AC9), технические предположения отделены от решения +и помечены как свободные к изменению.