From 609a9bef2dfcd0ce06c257f953e08095060a8e5a Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 06:15:34 +0000 Subject: [PATCH] docs: review document for #482 Issue: #482 User-Visible: no --- docs/reviews/SPEC-REVIEW-482-r1.md | 215 +++++++++++++++++++++++++++++ 1 file changed, 215 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-482-r1.md diff --git a/docs/reviews/SPEC-REVIEW-482-r1.md b/docs/reviews/SPEC-REVIEW-482-r1.md new file mode 100644 index 00000000..411fc124 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-482-r1.md @@ -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 для непрямоугольной комнаты» (§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). + +--- + + + +## Материал раунда + +- Ветка: `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 + ```