mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -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-
|
||||
экспорту.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `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
|
||||
```
|
||||
Reference in New Issue
Block a user