diff --git a/docs/reviews/SPEC-REVIEW-484-r1.md b/docs/reviews/SPEC-REVIEW-484-r1.md new file mode 100644 index 00000000..e1a71f43 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-484-r1.md @@ -0,0 +1,178 @@ +# SPEC-REVIEW-484-r1 + +Issue: [#484](https://github.com/Matysh/houseplan-card/issues/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 + ```