mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-05 14:19:04 +00:00
@@ -0,0 +1,287 @@
|
||||
# SPEC-REVIEW-53-r1 — Экспорт пространства в PDF: чистый архитектурный план
|
||||
|
||||
- Issue: https://github.com/Matysh/houseplan-card/issues/53
|
||||
- Этап: ТЗ на ревью (PROCESS.md §2.4), полный трек
|
||||
- Заход: r1 (первый цикл ревью этого ТЗ) · блокирующих циклов израсходовано 0 из 4 до этого раунда
|
||||
- Материал: `docs/specs/053-pdf-export.md` на коммите `cc93e93da9277a5e20c677e35ea1a0be0af57791`
|
||||
(ветка `issue/53-pdf-export`; коммит `48a17f5d` в теле issue устарел на момент
|
||||
ревью — `cc93e93d` содержит уточнение владельца по внутренним/внешним
|
||||
размерам и является актуальной версией ТЗ), сопутствующая правка
|
||||
`docs/specs/052-view-dimensions.md` на том же коммите.
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Первый цикл — разбор полный (§2.10 не применяется, предыдущего раунда нет).
|
||||
Проверялись: обязательные разделы ТЗ (PROCESS.md §7.1), однозначность и
|
||||
доказуемость AC1–AC15, отсутствие выданных за факт догадок, соответствие
|
||||
терминологии `docs/USER-GUIDE.ru.md`, соответствие контракту `#52`
|
||||
(`docs/specs/052-view-dimensions.md`), согласованность с каноническими
|
||||
документами подсистем (`docs/SUN.md`, `docs/WALL-THICKNESS.md`,
|
||||
`docs/BACKDROP.md`, `docs/FURNITURE.md`, `docs/TOUCH-SUPPORT.md`), продуктовая
|
||||
рамка `docs/SCOPE.md`.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
- Прочитаны целиком: `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (§2.3–2.10,
|
||||
§7.1–7.3), тело issue #53 и все 7 комментариев, `docs/specs/053-pdf-export.md`
|
||||
и `docs/specs/052-view-dimensions.md` на `cc93e93d`.
|
||||
- Каждое техническое утверждение ТЗ, которое можно свести к конкретному имени
|
||||
символа/файла/поля, сверено с кодом и документацией отдельным проходом
|
||||
(см. таблицу ниже) — не принято на слово ни одно нетривиальное заявление.
|
||||
- Изучены существующие прецеденты процесса (`docs/reviews/SPEC-REVIEW-473-*`,
|
||||
`docs/reviews/SPEC-REVIEW-445-r1.md`) для калибровки формата вердикта и
|
||||
допустимости ручного способа доказательства (AC12).
|
||||
- Дешёвые гейты не гонялись: на этапе ревью ТЗ они не предусмотрены (в
|
||||
репозитории ещё нет кода фичи — `src/pdf/**` не существует); это ревью
|
||||
контракта, а не кода.
|
||||
|
||||
### Сверка технических утверждений ТЗ с фактическим состоянием кода
|
||||
|
||||
| Утверждение ТЗ | Где смотрел | Итог |
|
||||
|---|---|---|
|
||||
| §7.1 — `spaceModels(cfg)` даёт `wall_segments`, `partitions`, `wall_columns`, проёмы `door/window/gate/passage` | `src/space-geometry.ts:135-189`, `src/types.ts:80-83,218` | подтверждено дословно |
|
||||
| §7.5 — резолвер севера: сначала компас пространства, потом общий `north_deg`, «тот же, что у солнца» | `src/sun.ts:667-671` (`northDegOf`) | подтверждено, порядок совпадает |
|
||||
| §7.6 — поля подложки `plan_x/plan_y/plan_scale*`, поворот, подписанный доступ «как на экране» | `docs/BACKDROP.md:11-19`, `src/signing.ts`, `src/logic.ts:1877`, `src/space-geometry.ts:165` | подтверждено |
|
||||
| §7.4 — `decor[]`, `furniture-art-runtime`, `kind: 'image'` | `scripts/config-schema.json:448`, `src/space-render.ts:338,637`, `src/furniture-art-runtime.ts`, `src/decor-assets.ts:33,41,117` | подтверждено |
|
||||
| §7.1/§8 — `geometryArea` = резолвер площади карточки комнаты | `src/physical-geometry.ts:424`, `src/houseplan-card.ts:9930-9963,12455-12457` (`_cleanFloor` → `geometryArea` → `formatArea`) | подтверждено |
|
||||
| §7.3 — «числа совпадают с живой линейкой ресайза на том же ребре» (грань-в-грань, а не по оси) | `src/houseplan-editor-runtime.ts:3708-3729` (`_rszEdgeLabels`/`_rszInnerSpanCms`), явный комментарий: «Длины — между внутренними гранями... Раньше здесь считалась осевая длина... (#233)» | подтверждено: линейка ресайза с #233 действительно грань-в-грань, а не осевая — заявление ТЗ верно (проверял отдельно, т.к. `052-view-dimensions.md` описывает именно осевую длину для другого, никогда не реализованного слоя — это не тот же измеритель) |
|
||||
| §8.2 — образец `iso-scene-render`, `lazyIsometricFiles`, терминальный отказ с нонсом | `src/iso-scene-render.ts:50`, `src/editor-runtime-loader.ts:8-11,54`, `src/houseplan-card.ts:341-344,761-774`, `scripts/bundle-manifest.mjs:108`, `scripts/bundle-budget.mjs:280,297` | подтверждено |
|
||||
| §11 — потолок `houseplan-card.ts` 13659, факт 13563 после #478 | `test/core-file-budget.test.mjs:22` (`CAPS['src/houseplan-card.ts'] = 13659`), `wc -l src/houseplan-card.ts` = 13563 | подтверждено, запас 96 строк — арифметика верна |
|
||||
| §8.5 — кнопка между «Общие настройки» и «Помощь и обратная связь» | `docs/USER-GUIDE.ru.md:58,2078` | терминология и порядок совпадают |
|
||||
| §1 — «диалог обязан работать пальцем» | `docs/TOUCH-SUPPORT.md` — таблица: «View dialogs and safe device actions» → touch: Fully supported; кнопка живёт в том же блоке шапки, что и «Помощь и обратная связь», доступном в View для `_canEdit` | обоснованно, не голословно |
|
||||
| §15 — «< 200 мс на large-house фикстуре, 60 комнат» | `demo/fixtures/large-house.mjs:8-9,198-205` (`FLOOR_COUNT=3`, `ROOMS_PER_FLOOR=20`, `spaces = Array.from({length: FLOOR_COUNT}, ...)`, каждое пространство получает свои 20 комнат) | **не подтверждено** — см. Medium-4 |
|
||||
| §11 — «`docs/SCOPE.md` (исключение 2026-08-15 — уже записано, проверить формулировку)» | `docs/SCOPE.md` (полный текст, grep `53\|PDF\|2026-08-15`), `git log --oneline -- docs/SCOPE.md` (последний коммит, трогавший файл, — задолго до 2026-08-15) | **не подтверждено** — см. Medium-1 |
|
||||
|
||||
Из двенадцати проверенных фактических утверждений десять подтвердились дословно
|
||||
— автор ТЗ последовательно сверялся с кодом, а не описывал желаемое. Ниже —
|
||||
находки по двум неподтвердившимся пунктам и по двум местам внутренней
|
||||
несогласованности контракта.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium-1 — `docs/SCOPE.md` не содержит заявленного исключения для #53
|
||||
|
||||
Раздел 11 ТЗ утверждает: «`docs/SCOPE.md` (исключение 2026-08-15 — уже
|
||||
записано, проверить формулировку)». Это неверно: в `docs/SCOPE.md` нет ни
|
||||
одного упоминания issue #53, PDF-экспорта или даты 2026-08-15.
|
||||
`git log --oneline -- docs/SCOPE.md` показывает, что последний коммит,
|
||||
трогавший файл, относится к периоду задолго до 2026-08-15 (v1.62-эры), и
|
||||
раздел «Excess-functionality audit (2026-07-22)» — единственная запись такого
|
||||
рода в файле, к #53 не относящаяся.
|
||||
|
||||
Комментарий владельца от 2026-08-15 в самой issue («Владелец подтвердил
|
||||
исключение из `docs/SCOPE.md`: #53 остаётся в продуктовой очереди») — это
|
||||
решение по scope, а не запись в самом guard-rail документе. Прецедент есть:
|
||||
для #89 (2.5D-изометрия) исключение записано прямо в тексте `SCOPE.md`
|
||||
(«A narrow exception approved for #89 is a deterministic 2.5D presentation
|
||||
of the existing canonical plan...», строка ~103). #53 такой записи не
|
||||
получил.
|
||||
|
||||
**Почему это находка, а не мелочь.** `AGENTS.md` называет `SCOPE.md`
|
||||
guard rail'ом: «features are built, improved and accepted only if they serve
|
||||
a job listed here». Печатный архитектурный экспорт прямо конфликтует с
|
||||
пунктом Out of scope («Photorealistic rendering... → niche tools») — этот
|
||||
конфликт и разбирался в комментариях 2026-08-14/2026-08-15/2026-08-30, и
|
||||
исключение из него принято владельцем трижды подряд (2026-08-15, 2026-08-30
|
||||
подтверждение, 2026-09-07 сужение скоупа). Формулировка ТЗ «уже записано»
|
||||
означает, что реализатор с высокой вероятностью не добавит запись в
|
||||
`SCOPE.md` вовсе — раздел 11 не просит этого сделать, только «проверить
|
||||
формулировку» несуществующего текста. Без записи следующий, кто откроет
|
||||
`SCOPE.md` за поиском причины, почему в out-of-scope зоне лежит печатный
|
||||
architectural-tooling экспорт, её не найдёт — ровно то расхождение, которое
|
||||
guard rail обязан предотвращать.
|
||||
|
||||
**Воспроизведение:**
|
||||
```
|
||||
grep -n "53\|PDF\|2026-08-15" docs/SCOPE.md # пусто, кроме несвязанных совпадений
|
||||
git log --oneline -- docs/SCOPE.md | head -1 # последний коммит — не после 2026-08-15
|
||||
```
|
||||
|
||||
**Как чинится в этом же раунде:** раздел 11 переписывается с «уже записано»
|
||||
на «требуется добавить запись» — короткий абзац в `docs/SCOPE.md`
|
||||
(Out of scope или Excess-functionality audit) по образцу #89, со ссылкой на
|
||||
issue #53 и обеими датами решений (2026-08-15, 2026-09-07). Это документация
|
||||
(класс C), не блокирует объём остального ТЗ.
|
||||
|
||||
### Medium-2 — Коллизионное правило противоречит гарантии «нельзя опускать» для непрямоугольных комнат
|
||||
|
||||
§7.3, абзац «Внутренние размеры комнаты», формулирует твёрдую гарантию: «Для
|
||||
непрямоугольных контуров опускать нельзя: каждое ребро — свой размер.» Этим
|
||||
же абзацем обосновано ключевое AC5 («по этим числам комната вычерчивается
|
||||
автономно») и свидетель `pdf-room-edge-dropped` («у непрямоугольной комнаты
|
||||
пропущено ребро — контур невосстановим»), т.е. пропуск ребра непрямоугольной
|
||||
комнаты явно объявлен дефектом.
|
||||
|
||||
Но абзац «Коллизии» того же раздела описывает обратное для того же случая:
|
||||
«подпись... скрывается (кроме случая «непрямоугольный контур» — там
|
||||
уменьшается кегль до 6 pt **прежде чем скрыться**)». Формулировка «прежде чем
|
||||
скрыться» — это не альтернатива скрытию, а отложенный шаг перед ним: если
|
||||
подпись не помещается даже 6-м кеглем (плотный план, комната с 5+ рёбрами
|
||||
рядом с другими подписями — ровно сценарий, для которого в §16 заведён риск
|
||||
«по 4+ числа на комнату» и golden на `large-house`), правило коллизий её всё
|
||||
равно скрывает. Это прямое противоречие: один и тот же раздел одновременно
|
||||
запрещает и разрешает пропуск ребра непрямоугольной комнаты.
|
||||
|
||||
**Почему это находка, а не общая забота о вёрстке.** AC5 — центральный AC
|
||||
этого ТЗ (реализация уточнения владельца от 2026-09-07, ради которого #52 был
|
||||
закрыт и слит с #53). Если приоритет коллизий в реализации выигрывает у
|
||||
«нельзя опускать» (что текст явно допускает своей формулировкой), AC5 будет
|
||||
молча нарушаться на плотных планах — то есть ровно там, где риск и назван.
|
||||
Автор теста для AC5/свидетеля `pdf-room-edge-dropped` не может однозначно
|
||||
решить, должен ли он проверять «никогда не пропущено» или «пропущено только
|
||||
после исчерпания уменьшения кегля», не имея ответа в тексте контракта.
|
||||
|
||||
**Как чинится в этом же раунде:** одно из двух — либо у непрямоугольного
|
||||
контура исключить хвостовое «скрывается» полностью (после 6pt — минимальная
|
||||
короткая засечка без числа по аналогии с правилом < 30 см, но для любой
|
||||
длины при коллизии, а не только для короткого ребра), либо явно ограничить
|
||||
«нельзя опускать» примечанием «в пределах листа при разумной плотности» и
|
||||
завести отдельный, честно описанный fallback для перегруженного случая. Оба
|
||||
варианта укладываются в один абзац §7.3.
|
||||
|
||||
### Medium-3 — AC6 проверяет правило, которого нет в контракте внешних размеров
|
||||
|
||||
AC6: «...текст не вверх ногами; коллизии детерминированы; **рёбра < 30 см —
|
||||
засечка без числа**». По формулировке и позиции в таблице AC6 целиком о
|
||||
внешних размерах планировки («Внешние размеры: у каждой прямой наружной
|
||||
грани... своя размерная линия... виртуальные границы без размера... рёбра
|
||||
< 30 см — засечка без числа»).
|
||||
|
||||
Но правило «короче 30 см — засечка без числа, длина восстановима из
|
||||
соседних» в §7.3 сформулировано только в абзаце «Внутренние размеры комнаты»
|
||||
(«Рёбра короче 30 см на бумаге не подписываются... такие рёбра помечаются
|
||||
короткой засечкой без числа»). Абзац «Внешние размеры планировки» этого
|
||||
порога не содержит вовсе: там нет ни минимальной длины, ни правила засечки
|
||||
для коротких наружных граней — только правило для виртуальных (нулевых)
|
||||
границ.
|
||||
|
||||
AC — это критерий приёмки, а не описание поведения; поведение обязан задавать
|
||||
контракт (§7.3), AC его только проверяет. Здесь AC6 проверяет то, чего
|
||||
контракт для внешних граней не описывает: реализатору придётся либо
|
||||
угадывать (перенести правило по аналогии — которое рискует стать «догадкой,
|
||||
выданной за факт», ровно то, что это ревью обязано ловить), либо писать тест
|
||||
против неопределённого поведения.
|
||||
|
||||
**Как чинится в этом же раунде:** либо в §7.3 «Внешние размеры планировки»
|
||||
добавляется явное предложение о коротких гранях (перенос правила из
|
||||
внутреннего абзаца с той же формулировкой засечки), либо клауза «рёбра
|
||||
< 30 см — засечка без числа» убирается из AC6 (если для внешних граней порог
|
||||
не нужен — наружные грани планировки обычно не бывают короче 30 см, но это
|
||||
тоже должно быть сказано явно, а не подразумеваться).
|
||||
|
||||
### Medium-4 — Порог производительности ссылается на несуществующую конфигурацию фикстуры
|
||||
|
||||
§15: «Генерация — синхронный расчёт сцены (< 200 мс на `large-house`
|
||||
фикстуре, 60 комнат; проверяется в unit как порог)». §16 повторяет: «...
|
||||
проверено на `large-house` фикстуре в golden».
|
||||
|
||||
`demo/fixtures/large-house.mjs` (строки 8-9, 198-233) строит `FLOOR_COUNT = 3`
|
||||
пространства (`spaces = Array.from({length: FLOOR_COUNT}, ...)`), каждое —
|
||||
через `roomGrid(floor)` с `ROOMS_PER_FLOOR = 20` комнатами. Итого 60 комнат
|
||||
на **весь дом**, но ни одно из трёх пространств фикстуры не содержит 60
|
||||
комнат — в каждом ровно 20. ТЗ §5 фиксирует: «Экспортируется только текущее
|
||||
пространство» — экспорт в PDF работает с одним пространством за раз, поэтому
|
||||
релевантное число для порога производительности — 20 комнат на пространство
|
||||
(плюс пропорциональная доля из ~60 partitions/40 columns/500 decor,
|
||||
разделённых на три этажа), а не 60.
|
||||
|
||||
Это конкретная, проверяемая фактическая неточность: автор теста AC (порог
|
||||
«< 200 мс на 60 комнатах») либо не найдёт такого пространства в
|
||||
`large-house` и должен будет угадывать, что имелось в виду, либо build
|
||||
ошибочно просуммирует три пространства в один синхронный вызов, чего сцена
|
||||
PDF не делает по контракту (одно пространство за раз).
|
||||
|
||||
**Как чинится в этом же раунде:** правится число в §15 на фактическое (20
|
||||
комнат на пространство этой фикстуры) либо явно указывается новая фикстура
|
||||
с одним пространством на 60 комнат, если такая плотность нужна для честной
|
||||
проверки порога — в таком случае она также должна быть названа по имени,
|
||||
а не как «`large-house`».
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Обязательные разделы §7.1 PROCESS.md присутствуют: сценарий (§1), что
|
||||
человек увидит до/после (§2), проблема (§3), скоуп/не-скоуп (§5/§6),
|
||||
контракт поведения (§7-8), UX диалога (§8.5), модель данных и миграция
|
||||
(§10 — явно «без изменений»), i18n (§9), AC1-AC15 с указанием доказательства
|
||||
(§12), план автотестов (§13), риски (§16), откат (§17), release-артефакты
|
||||
(§14).
|
||||
- Все двенадцать проверенных фактических технических утверждений о
|
||||
существующем коде и документации — десять подтверждены дословно (см.
|
||||
таблицу выше); ни одно не оказалось выдумкой, кроме двух разобранных выше
|
||||
как находки.
|
||||
- Продуктовые вопросы (§18, Q1/Q2) — не голые догадки: у каждого есть
|
||||
принятое значение по умолчанию с обоснованием и явной пометкой «принято»,
|
||||
ревьюер вправе оспорить. Оба разумны и не требуют эскалации владельцу
|
||||
(объём видимых изменений и умолчания диалога — не то, что процесс требует
|
||||
спрашивать владельца отдельно, когда автор уже сузил вопрос до варианта по
|
||||
умолчанию).
|
||||
- Терминология интерфейса («Общие настройки», «Помощь и обратная связь»,
|
||||
порядок иконок в шапке, видимость только для `_canEdit`, скрытие в
|
||||
киоске) сверена с `docs/USER-GUIDE.ru.md` и совпадает.
|
||||
- Требование «диалог обязан работать пальцем» (§1) обосновано таблицей
|
||||
`docs/TOUCH-SUPPORT.md` («View dialogs and safe device actions» → touch:
|
||||
Fully supported), а не заявлено произвольно.
|
||||
- AC12 (ручная проверка скачивания в мобильном приложении HA) — не новый
|
||||
прецедент: аналогичная форма («вручную... команды и числа в issue») уже
|
||||
принята процессом для AC7 в #473 (`docs/reviews/SPEC-REVIEW-473-r2.md`) как
|
||||
единственный доступный способ доказательства для сценария, не
|
||||
воспроизводимого в CI. Условие того прецедента — ручной способ явно назван
|
||||
и результат фиксируется в issue — здесь выполнено (§8.4, §16).
|
||||
- Все AC1-AC15 пронумерованы без пропусков и дублей, у каждого указан способ
|
||||
доказательства из принятого словаря (`unit`/`smoke`/`golden`/вручную с
|
||||
обоснованием); открытых продуктовых вопросов не осталось непокрытыми.
|
||||
- «Один источник числа»: площадь и внутренние размеры на бумаге явно
|
||||
привязаны к тем же резолверам, что карточка комнаты и живая линейка ресайза
|
||||
(§7.3, AC5) — проверено по коду `_rszEdgeLabels`/`_cleanFloor`, это
|
||||
действительно один источник, а не два похожих числа из разных мест (см.
|
||||
таблицу сверки выше — заявление подтвердилось, это не находка).
|
||||
- Свидетели §13 покрывают по имени каждый значимый защитный сценарий таблицы
|
||||
§7.1/§7.3/§8 (общая стена, галочки, отсутствие ToUnicode, виртуальная
|
||||
стена, устройства на листе, нестандартный масштаб, теперь ещё
|
||||
`pdf-room-edge-dropped`/`pdf-outer-face-inside`), что для этапа спеки
|
||||
(не код-ревью) достаточно — состав мутационных гейтов будет предметом
|
||||
код-ревью.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Код фичи не существует (`src/pdf/**`, `pdf-font.generated.ts`,
|
||||
`scripts/generate-pdf-font.mjs` — всё запланировано, не реализовано):
|
||||
дешёвые гейты (`tsc`, `test`, `build`), golden, смоки, мутационные
|
||||
свидетели не гонялись и не могли гоняться — на этапе ревью ТЗ им нечего
|
||||
проверять.
|
||||
- Не проверял вручную рендеринг PDF-макета (шрифт, коллизии, масштабная
|
||||
линейка) — на этом этапе такого артефакта не существует.
|
||||
- Не оценивал реалистичность оценок «~15 КБ gzip кода», «60-80 КБ raw
|
||||
шрифта» — это целевые ориентиры дизайна (`ожидаемо`), а не факты о
|
||||
существующей системе, и не заявлены как факт.
|
||||
- Не проверял, требует ли реализация фактического похода к
|
||||
`scripts/bundle-budget.mjs`/`bundle-manifest.mjs` для добавления роли
|
||||
`pdf` (код не написан) — это предмет код-ревью.
|
||||
- Q1/Q2 из §18 не эскалировал владельцу: это принятые автором предположения
|
||||
с обоснованием, ревьюер вправе (и в данном случае согласен) их принять без
|
||||
дальнейшего вопроса.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Три Medium-находки внутри скоупа задачи (Medium-2, Medium-3, Medium-4 — все
|
||||
правятся правкой текста ТЗ, без обращения к владельцу) плюс одна
|
||||
Medium-находка о недостающей записи в `docs/SCOPE.md` (Medium-1), тоже
|
||||
внутри скоупа — это правка документации `docs/SCOPE.md`, которую и так
|
||||
называет раздел 11 этого же ТЗ, а не чужой скоуп. High-находок нет. Согласно
|
||||
PROCESS.md §2.4/§2.7 без High это жёлтый вердикт: автор правит четыре
|
||||
абзаца (§7.3 коллизии, §11 запись в SCOPE.md и AC6, §15 число фикстуры) и
|
||||
перевыставляет `S4-spec-review`.
|
||||
|
||||
Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 4 → в задаче
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/53-pdf-export`, коммит `48a17f5dc998` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `be8a4d9c9e368d67691846b74c69de39cfaaf40b`
|
||||
```
|
||||
git log --all --format='%H %T' | grep be8a4d9c9e36
|
||||
```
|
||||
Reference in New Issue
Block a user