mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,215 @@
|
||||
# SPEC-REVIEW-482-r1 — Доводка экспорта пространства в PDF
|
||||
|
||||
- **Issue:** https://github.com/Matysh/houseplan-card/issues/482
|
||||
- **Этап:** ревью ТЗ (PROCESS.md §2.4)
|
||||
- **ТЗ:** `docs/specs/482-pdf-export-polish.md`
|
||||
- **Трек:** полный (аналитика #482 назвала критерии `small`, которые задача не
|
||||
проходит: сложность/риск 7/10, более одной поверхности, уточнение публичного
|
||||
контракта размеров/компоновки — обоснование присутствует)
|
||||
- **Заход:** r1 · блокирующих циклов израсходовано 0 из 4 (до этого вердикта)
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Первый заход. Разбор полный (иного и не может быть на r1): читал тело issue
|
||||
#482 и все три комментария (уточнение владельца, аналитика, ссылка на ТЗ),
|
||||
`docs/specs/482-pdf-export-polish.md` целиком, связанные контракты
|
||||
`docs/specs/053-pdf-export.md` и `docs/specs/052-view-dimensions.md`
|
||||
(объединён в #53), `docs/SCOPE.md`, `docs/TOUCH-SUPPORT.md`,
|
||||
`docs/PDF-EXPORT.md`, `docs/USER-GUIDE.ru.md` (раздел про PDF), и — поскольку
|
||||
ТЗ делает утверждения о текущем поведении кода как обоснование problem
|
||||
statement — сверил каждое такое утверждение с фактическим `src/pdf/*.ts`,
|
||||
`src/near-axis.ts`, `src/wall-thickness.ts`, `src/styles/plan.styles.ts`.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
1. Продуктовая рамка: `docs/SCOPE.md` — задача внутри узкого исключения
|
||||
печатного экспорта (зафиксировано 2026-08-15/2026-09-07 для #53/#52), сама
|
||||
ничего не расширяет (§4 ТЗ явно ограничивает изменяемый контракт частями
|
||||
#53, §6 явно перечисляет не-скоуп).
|
||||
2. Процесс: `AGENTS.md`, `PROCESS.md` §1–§9 — трек, статусы, лимит циклов,
|
||||
требования DoR (§2.5), обязательные разделы ТЗ (§7.1).
|
||||
3. Само ТЗ построчно на однозначность, проверяемость каждого AC и наличие
|
||||
способа доказательства.
|
||||
4. Отдельно — не выдаёт ли автор догадку за факт. Для этого каждое
|
||||
утверждение раздела «Подтверждённые причины» (§3 ТЗ) и технической карты
|
||||
(§20) сверено с реальным кодом:
|
||||
- `compactRing()` (`src/pdf/pdf-dimensions.ts:16-33`) действительно ищет
|
||||
коллинеарные точки по `previous/point/next` **до** какого-либо схлопывания
|
||||
точных/почти точных дублей — корневая причина ложной хорды подтверждена
|
||||
чтением, не предположением;
|
||||
- `WALL = [0.33, 0.33, 0.33]` (`pdf-scene.ts:79`) — заливка без штриховки,
|
||||
подтверждает «нет hatch-прохода»;
|
||||
- `pdf.legend.*` (6 ключей: wall/partition/virtual/door/window/gate) в
|
||||
`pdf-scene.ts:522-533` и `src/i18n/en.json:454-459` — легенда и ключи
|
||||
существуют ровно как описано;
|
||||
- выбор ориентации `landscape = physicalWidth > physicalHeight`
|
||||
(`pdf-scene.ts:236`) и резервы `calloutWidthMm = 48`,
|
||||
`dimensionReserveMm = 30` (`pdf-scene.ts:246,250`) — подтверждают «сырой
|
||||
bbox до размеров» и «приближённые 30/48 mm» из §3.4/§12.1 ТЗ;
|
||||
- цветовой форматтер `fmt = (v) => v.toFixed(2)...` (`pdf-writer.ts:38`) —
|
||||
общий двухзнаковый форматтер действительно даст `0.5` для `127/255`,
|
||||
подтверждает необходимость отдельного high-precision форматтера (§11,
|
||||
риск-таблица);
|
||||
- в `pdf-writer.ts` нет ни одного `W`/`W*` clip-оператора — подтверждает
|
||||
«writer не умеет even-odd clipping» (§3.5);
|
||||
- канонический допуск `NEAR_AXIS_MAX_DEGREES = 0.25` реально существует в
|
||||
`src/near-axis.ts` и используется вне PDF (repair/draw) — ссылка ТЗ §8.2
|
||||
на «канонические 0.25°» не изобретена, это существующая общая константа;
|
||||
- fallback «выноска R<n> для непрямоугольной комнаты» (§10.2 ТЗ) —
|
||||
реальный код `pdf-scene.ts:422-479` (`nonRect`, `callouts`, `R${…}`), не
|
||||
придуманный механизм;
|
||||
- существующий экранный hatch (`houseplan-card.ts:9259-9263`,
|
||||
`space-render.ts:902-906`) действительно повёрнут на 45° —
|
||||
обоснование «как на самом плане» для §11 ТЗ не голословно.
|
||||
5. `docs/PDF-EXPORT.md` подтверждает, что легенда и порядок футера (масштаб,
|
||||
scale bar, north, дата, версия) — уже документированное поведение, которое
|
||||
§14 ТЗ корректно урезает, не придумывая нового.
|
||||
6. `docs/TOUCH-SUPPORT.md` — диалог PDF относится к View и подпадает под
|
||||
«полностью поддерживается», это верно отражено в §16 ТЗ (320 px, а не
|
||||
best-effort).
|
||||
|
||||
Гейты кода на этом этапе не запускались — ревью ТЗ оценивает выполнимость и
|
||||
проверяемость постановки, а не код; кода по этому issue ещё нет (`S4-spec-review`,
|
||||
ветка `issue/482-pdf-export-polish` содержит только ТЗ и предыдущие
|
||||
несвязанные коммиты beta.3).
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium (в скоупе задачи)
|
||||
|
||||
**M1 — источник и лицензия векторного компаса заявлены как факт без
|
||||
проверяемой цитаты, аналогично истории #159.** `docs/specs/482-pdf-export-polish.md`
|
||||
§13: ТЗ утверждает, что приложенный владельцем `compass-svgrepo-com.svg`
|
||||
(так его в issue называет владелец) — это «`compass-line` из VMware Clarity
|
||||
Assets», MIT license, copyright VMware 2018, ссылается на конкретный upstream
|
||||
URL и приводит SHA-256 «переданного владельцем файла», который «совпадает по
|
||||
path с каноническим ассетом».
|
||||
|
||||
Ни владелец в issue, ни один документ репозитория (`docs/FURNITURE.md`,
|
||||
`docs/PDF-EXPORT.md`, лицензионные заметки) не называют этот источник —
|
||||
владелец лишь приложил геометрию SVG и попросил заменить стрелку. Атрибуцию
|
||||
«VMware Clarity Assets, MIT, © 2018» вносит сам автор ТЗ. Это ровно тот
|
||||
класс дефекта, который PROCESS.md требует ловить на этапе ревью ТЗ: «догадка,
|
||||
записанная как факт» — и ровно тот тип дефекта, что уже случился на #159
|
||||
(`SPEC-REVIEW-159-r1.md`, High-1: провенанс/лицензия SVG опирались на
|
||||
комментарий, который не подтверждал ни авторства, ни лицензии).
|
||||
|
||||
В этом ревью у меня нет доступа к внешним URL (инструмент веб-фетча не
|
||||
подтверждён средой — см. «Чего не проверял»), поэтому я не могу ни
|
||||
подтвердить, ни опровергнуть конкретное совпадение path/SHA-256. Именно
|
||||
поэтому фактическая проверка не может остаться на совести одного только
|
||||
автора: пока в самом ТЗ нет воспроизводимого способа перепроверить
|
||||
атрибуцию (например, точный upstream commit/blob вместо общей ссылки на
|
||||
`master`, который может уехать, или явное подтверждение владельца, что он
|
||||
взял файл из названного источника), AC8 «license сохранена» проверяется
|
||||
только по тому, что реализация совпадает с недоказанным утверждением ТЗ, а
|
||||
не с внешней истиной.
|
||||
|
||||
**Почему в скоупе, а не отдельным issue:** это ровно тот же деливерабл, что и
|
||||
пункт 7 issue/§13 ТЗ (векторный компас), не соседняя подсистема — чинится
|
||||
правкой этого же ТЗ, не новым issue (#202).
|
||||
|
||||
**Как закрыть (на выбор автора, ТЗ не подсказывает решение):**
|
||||
- заменить общую ссылку на `master` точным upstream commit SHA/blob, который
|
||||
не уедет, и явно пометить абзац как «принято предположительно, проверить
|
||||
перед реализацией» по механике §7.1 PROCESS.md; или
|
||||
- получить у владельца прямое подтверждение источника одним продуктовым
|
||||
вопросом («откуда взят приложенный SVG-файл — просто подтвердите
|
||||
источник/лицензию, чтобы не приписать чужой копирайт неверно»); или
|
||||
- обойтись без внешней атрибуции вовсе: сохранить в репозитории только то,
|
||||
что доказуемо — сам SHA-256 приложенного владельцем файла как idempotency
|
||||
proof, без утверждений о конкретном третьесторонним репозитории/лицензии,
|
||||
которые ТЗ не может подтвердить.
|
||||
|
||||
Без High-находок это жёлтый вердикт: правка ТЗ и повторный заход в рамках
|
||||
той же задачи, отдельный issue не заводится.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Продуктовая рамка (§1–§2 ТЗ): персона, поверхность и момент названы верно;
|
||||
предложение «до/после» одной фразой без терминов реализации выдержано.
|
||||
- Скоуп/не-скоуп (§5–§6): граница задачи явная, включая явный триггер
|
||||
возврата в `S3-spec` при выходе за неё (новое config-поле, диагональные
|
||||
стены, новые галочки).
|
||||
- Изменяемый контракт (§4): точно называет, какие части #53/#52 заменяются, а
|
||||
какие гарантии сохраняются — не тихая ревизия чужого ТЗ.
|
||||
- Нормализация контуров (§8): порядок очистки (схлопнуть дубли → удалить
|
||||
коллинеарные) и допуски (1 mm физических, 0.25° из существующей константы)
|
||||
однозначны и проверяемы; описание «промежуточной точки» через ненулевые
|
||||
векторы и положительное скалярное произведение исключает как раз тот
|
||||
дефект, что породил issue.
|
||||
- Локальная дедупликация (§9): критерии пары (ось, нормаль, interval, длина,
|
||||
inside-probe) и правило разрешения конфликтов (score → стабильный ключ)
|
||||
полностью детерминированы, включая явные «не следствия» (разные
|
||||
комнаты/rings/оси не трогать).
|
||||
- Размещение размеров (§10): единая полоса, сдвиг целиком, шаги в мм —
|
||||
корректно запрещает старый баг «tangent-jitter отдельного текста».
|
||||
- Материал стен (§11) и компоновка листа (§12): точные числа (127/127/127,
|
||||
45°/3 mm/0.18 mm, допуск 0.5 mm на bbox, порядок выбора масштаба) —
|
||||
каждое либо взято из уже существующей константы (45°, INK), либо является
|
||||
явным новым техническим решением автора, не заявленным как продуктовое
|
||||
наблюдение владельца — то есть законная зона «решает автор».
|
||||
- Footer/i18n (§14): корректно ссылается на реально существующие
|
||||
`pdf.legend.*` ключи и текущий состав footer из `docs/PDF-EXPORT.md`.
|
||||
- Модель/миграция/rollback (§15), touch/a11y/perf/security (§16): все пункты
|
||||
DoR присутствуют; миграции нет, что верно для чисто визуальной доводки.
|
||||
- AC1–AC12 (§17): каждый пронумерован, привязан к разделу контракта и несёт
|
||||
способ доказательства (unit/smoke/golden/mutation witness), включая
|
||||
защитные критерии (дедупликация, axis filter, clipping) — это избыточно
|
||||
подробно относительно минимума DoR (§2.5), что для риска 7/10 уместно.
|
||||
- Тест-план (§18) и границы Windows/Linux для golden корректно повторяют
|
||||
канон `PROCESS.md` §8 (`golden:verify` диагностический на Windows,
|
||||
канонические эталоны — только Linux CI).
|
||||
- Документация и release-artifacts (§19): перечислены все реально
|
||||
существующие затрагиваемые документы, включая замену контракта в
|
||||
`docs/specs/053-pdf-export.md`.
|
||||
- Карта изменений (§20) соответствует существующей структуре `src/pdf/*` —
|
||||
файлы, которые ТЗ называет изменяемыми, действительно существуют
|
||||
(`pdf-dimensions.ts`, `pdf-scene.ts`, `pdf-writer.ts`, `hp-pdf-dialog.ts`);
|
||||
`pdf-compass.ts` — новый файл, что согласовано явно.
|
||||
- Открытых продуктовых вопросов к владельцу нет и не требуется: все
|
||||
технические развилки (где хранить lane-состояние, как разрешать коллизии)
|
||||
решены и явно объявлены зоной ответственности автора/ревьюера
|
||||
(§7.1 PROCESS.md), а не вынесены как недостающие продуктовые решения.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не запускал `npx tsc --noEmit`/`npm test`/`npm run build` — на этапе ревью
|
||||
ТЗ кода по задаче ещё нет (ветка содержит только сам файл ТЗ поверх
|
||||
`v1.73.0-beta.3`), гонять гейты не по чему.
|
||||
- Не проверял независимо через внешний URL, действительно ли приложенный
|
||||
владельцем SVG побайтово совпадает с `vmware-archive/clarity-assets` и
|
||||
действительно ли лицензия этого репозитория — MIT с указанным копирайтом:
|
||||
инструмент веб-фетча запросил разрешение и не получил его в этой сессии.
|
||||
Это прямая причина находки M1, а не молчаливый пропуск.
|
||||
- Не проверял смоки/golden/perf — на стадии ТЗ им нечего мерить; тест-план
|
||||
ТЗ (§18) сам называет их как будущее доказательство AC, что и является
|
||||
предметом код-ревью, а не этого захода.
|
||||
- Не проверял `docs/specs/README.md`/`docs/STATUS.md`/`docs/ARCHITECTURE.md`
|
||||
построчно на предмет того, что именно там устареет — ТЗ (§19) верно называет
|
||||
их условно («при изменении перечисленных модулей»), это станет предметом
|
||||
код-ревью, когда будет виден фактический diff.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Одна находка Medium в скоупе задачи, High нет. По PROCESS.md §2.4/§4: без
|
||||
High-находок это жёлтый вердикт — ТЗ возвращается автору на правку в рамках
|
||||
этого же issue, отдельный issue не заводится (#202). Бюджет циклов ревью ТЗ
|
||||
для полного трека — 4; этот заход тратит один цикл (жёлтый вердикт бюджет
|
||||
расходует, в отличие от зелёного, #227).
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/482-pdf-export-polish`, коммит `bc93babdf8e7` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `38bf116ad9ad52b85ed8e0aaee3799819a5a1e58`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 38bf116ad9ad
|
||||
```
|
||||
- ТЗ `docs/specs/482-pdf-export-polish.md`, блоб `28628a6c925a3a20886cf0315499ffd478ea8a25`
|
||||
```
|
||||
git log --all --find-object=28628a6c925a3a20886cf0315499ffd478ea8a25 -- docs/specs/482-pdf-export-polish.md
|
||||
```
|
||||
Reference in New Issue
Block a user