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

SPEC-REVIEW-484-r1

Issue: #484 — PDF: внешняя размерная цепь теряет размеры ступенчатого фасада ТЗ: docs/specs/484-pdf-exterior-dimension-chain.md (ветка issue/484-pdf-exterior-dimensions, SHA на момент ревью — вершина ветки при отправке на ревью) Этап: spec (PROCESS.md §2.4) · Заход r1 · блокирующих циклов израсходовано 0 из 4

Скоуп

Полный трек (не small): задача не проходит порог сложности/риска ≤3 и затрагивает контракт печатной геометрии PDF-экспорта. Аналитика владельца в issue уже подтвердила это явно, отдельно проверять выбор трека не требовалось.

Продуктовая рамка (docs/SCOPE.md): PDF-экспорт — узкое разрешённое исключение из «Out of scope → General CAD», зафиксированное для #53 и подтверждённое в этой задаче как обслуживающее того же Home admin, который передаёт уже поддерживаемую геометрию монтажнику/подрядчику. Задача не расширяет исключение (не добавляет новых настроек, не вводит второй геометрический движок) — соответствует ограничителю SCOPE.md.

Как проверялось

Прочитаны в порядке инструкции: docs/SCOPE.md, AGENTS.md, PROCESS.md (§2.3– §2.10, §3, §4, §7.1, §7.2), тело issue #484 и оба комментария владельца (аналитика + ссылка на ТЗ), docs/USER-GUIDE.ru.md (раздел «Сохранение текущего пространства в PDF»), docs/PDF-EXPORT.md (канонический документ подсистемы), сам файл ТЗ docs/specs/484-pdf-exterior-dimension-chain.md целиком.

Технические утверждения ТЗ сверены построчно с действующим кодом на SHA ветки, а не приняты на слово:

• src/pdf/pdf-collision.ts (pdfSegmentTouchesGeometry, строки 155–173): подтверждено, что allowStartBoundary сейчас разрешает касание только в точке start (t≈0), а isSolid(midpoint)/isSolid(end) и любое пересечение с hit.t * distance > ε остаются коллизией без исключения для собственного примыкающего ребра. Диагноз ТЗ («любое последующее коллинеарное наложение и пересечение считаются коллизией») соответствует коду буквально.
• src/pdf/pdf-scene.ts (строки 486–546, externalSegments, externalPlacementTouchesArchitecture, цикл dedupeOppositeDimensionEdges → groupCollinearDimensionEdges → лейн-перебор 0…80 mm шагом 4): структура ровно та, что описана в §3 и §6 ТЗ — allowStartBoundary: true выставлен именно для двух extension lines ({start: entry.a, end: entry.oa, ...}), как и заявлено.
• src/pdf/pdf-dimensions.ts: dedupeOppositeDimensionEdges, groupCollinearDimensionEdges, compareStableDimensionEdges существуют и реализуют ровно то поведение («один из пары равных, стабильный tie-break, независимые лейны для разных линий фасада»), на которое ссылается ТЗ в §6.1 («существующий stable source key»).
• test/pdf-dimensions.test.mjs:103 уже проверяет, что ступенчатый контур корректно порождает два параллельных верхних ребра на уровне edge-вычисления — то есть регрессия действительно локализована на стадии scene-level коллизий (pdf-scene.ts), а не в вычислении рёбер, что соответствует диагнозу ТЗ. Ни в test/pdf-scene.test.mjs, ни в test/pdf-collision.test.mjs нет существующего фикстура со ступенчатым внешним контуром — заявленное отсутствие покрытия регрессии подтверждено.
• Perf-бюджет <200 ms для large-house сцены — существующий тест test/pdf-scene.test.mjs:641, ссылка в ТЗ корректна, новый тест не нужен.
• docs/PDF-EXPORT.md и docs/USER-GUIDE.ru.md (раздел про PDF) сверены на терминологию: «clean-floor geometry», «external dimensions follow the outer physical outline», «equivalent opposite measurements are shown once» уже задокументированы для #482 — ТЗ не изобретает новых терминов, а расширяет существующую формулировку правилом для выступов (§13).
• docs/specs/README.md:31 — обратная ссылка issue ↔ ТЗ на месте.
• Минимальный fixture-ring (0,0)→(10,0)→(10,3)→(12,3)→(12,6)→(10,6)→(10,10)→ (0,10) в ТЗ дословно совпадает с приведённым в теле issue.

Гейты кода не гонялись — на этапе spec-ревью нет кода для проверки (диапазон изменений — только docs/specs/**, класс C). Это не «непрогнанный гейт», а корректный объём для этапа: typecheck/test/build/check-docs/golden здесь не применимы, а появятся предметом код-ревью после реализации.

Проверка обязательных разделов ТЗ (PROCESS.md §7.1)

Все обязательные разделы присутствуют: сценарий (§1) · что человек увидит до/ после (§2) · проблема (§3) · цели/не-скоуп (§4/§5) · контракт поведения (§6) · геометрические инварианты (§7) · модель/миграция (§8) · UX/i18n/touch (§9) · производительность и безопасность (§10) · план реализации с планом тестов (§11, п.3–5) · AC1…AC8 с указанием доказательства и отдельная таблица защитных AC/мутантов (§12/§12.1) · release-артефакты (§13) · риски (§14) · откат (§15) · явный блок «принято предположительно» (§16).

Сценарий называет персону из SCOPE.md (Home admin), поверхность (View, экспорт в PDF) и момент (передача плана монтажнику/подрядчику/страховщику) — без этого раздел был бы техническим описанием, а не продуктовым.

Проверка на «догадку, выданную за факт»

Каждое нетривиальное утверждение о поведении сверено с кодом или существующей документацией (см. «Как проверялось») — расхождений не найдено. Технические решения, не наблюдаемые пользователем (форма helper-функции, формат intersection sweep, где именно живёт fixture — golden сценарий или новый файл), явно вынесены в §16 «принято предположительно, можно менять на ревью» с обоснованием, почему они не требуют продуктового ответа владельца. Открытых продуктовых вопросов, ошибочно решённых втихую, не найдено.

AC — проверка однозначности и доказуемости

Все восемь AC пронумерованы, формулируют проверяемое состояние (не процесс) и у каждого указан способ доказательства (unit-файл или browser/golden + код-ревью). Для двух защитных AC (AC3, AC4) отдельно указан обязательный мутант с точным описанием того, что он обязан сломать — соответствует формату «AC · чем доказан · чем краснеет», ожидаемому на код-ревью (§2.7). AC1–AC6 сопоставлены с минимальным fixture, приведённым дословно из issue; AC7 требует Linux golden с явной приёмкой; AC8 требует документацию и оба changelog. Прямое сопоставление с чек-листом issue показывает полное покрытие всех шести пунктов исходных критериев приёмки без потери ни одного.

Находки

Не найдено ни одной High- или Medium-находки. ТЗ технически точно описывает подтверждённую причину регрессии, узко и симметрично формулирует контракт исключения (допустимый начальный выход vs запрет повторного входа), явно перечисляет что не входит в задачу, и не смешивает техническую догадку с продуктовым решением.

Low, снятые без правки:

• §2 («до и после») содержит лёгкий технический оттенок («без диагоналей и линий сквозь посторонние стены») в разделе, который должен быть свободен от терминов реализации. Формулировка тем не менее описывает то, что увидит человек на листе (отсутствие лишних линий), а не внутренний механизм — снимаю без правки.
• Раздел «план автотестов» не оформлен отдельным заголовком, а распределён между §11 (шаги 3–5) и §12/§12.1 (таблицы AC и мутантов). Содержание полностью покрывает требование §7.1, отдельная секция ничего бы не добавила — снимаю без правки.

Что проверено и корректно

• Диагноз причины регрессии в pdf-collision.ts/pdf-scene.ts — построчно подтверждён по текущему коду.
• Контракт §6.2 (допустимый начальный префикс / запрет повторного входа) внутренне непротиворечив, включая крайние случаи (общая вершина двух граней, касание в точке перехода, invalid/неоднозначная классификация → fail closed).
• Совместимость с #482 (дедупликация, независимые лейны, stable tie-break, правило одного H/V для прямоугольника, отсутствие диагоналей) заявлена и подтверждена ссылками на реально существующий код и тесты.
• Модель/миграция, i18n, touch, производительность, откат — корректно оценены как «без изменений», решения обоснованы.
• Traceability issue ↔ ТЗ ↔ docs/specs/README.md — на месте в обе стороны.
• Продуктовых вопросов, требующих владельца, не осталось; технические решения явно помечены как «принято предположительно».

Чего не проверял

• Не запускал никакие гейты (typecheck/test/build/golden/smoke) — на этапе spec-ревью нет диапазона кода для их применения (диапазон изменений — только новый файл docs/specs/484-...md, класс C).
• Не проверял, как будущая реализация фактически справится со всеми перечисленными в матрице случаями (обычный выход, начальное коллинеарное наложение, проход через инцидентное тело, повторный вход, посторонняя стена) — это предмет код-ревью с мутантами pdf-own-exit-prefix-disabled и pdf-post-exit-collision-ignored, а не текущего этапа.
• Не оценивал возможные пиксельные различия golden-эталона pdf-export-polish- light или нового сценария — они появятся вместе с реализацией и требуют полного Linux-артефакта.

Вердикт

Зелёный. ТЗ выполнимо, проверяемо, не содержит догадок, выданных за факт, верно ограничивает скоуп и совместимо с ранее принятыми решениями по PDF- экспорту.

---

Материал раунда

• Ветка: issue/484-pdf-exterior-dimensions, коммит d53e4f70d33e — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: a17262e2ded74767c8b8f6648c2fee1c15705205

  git log --all --format='%H %T' | grep a17262e2ded7
  

• ТЗ docs/specs/484-pdf-exterior-dimension-chain.md, блоб 84fb3a65e309f90c2c8c149b6f7bb4f6bf876f4d

  git log --all --find-object=84fb3a65e309f90c2c8c149b6f7bb4f6bf876f4d -- docs/specs/484-pdf-exterior-dimension-chain.md