docs: review document for #530

Issue: #530
User-Visible: no
This commit is contained in:
claude[bot]
2026-09-11 11:06:47 +00:00
parent 2e26116bbb
commit 9d35ab35ac
+107
View File
@@ -0,0 +1,107 @@
# CODE-REVIEW-530-r1
**Issue:** [#530](https://github.com/Matysh/houseplan-card/issues/530) — «Печатный план стал крупнее примерно на треть»
**Трек:** полный (критерии `small` нарушены: перф-бюджет сборки сцены, новый UX-контракт — смена ориентации листа и удаление раздела)
**Заход:** r1 · блокирующих циклов израсходовано **0 из 4**
**Ветка:** `issue/530-pdf-sheet-fill`, один коммит. Пайплайн привёл ветку к `dev` до ревью: `49865cd4c6bd9e64193939da541735e918ab077a → 2e26116bbb2d1eca4a75997b058b34d4c885828b` (4 коммита `dev` легли под задачей). После ребейза это другой код (§7.2) — разбор ниже полный, не по дельте.
**Материал ревью:** ровно `2e26116bbb2d1eca4a75997b058b34d4c885828b`. `git fetch/pull/checkout` не выполнялись.
## Скоуп изменения
Единственная product-поверхность — `src/pdf/pdf-scene.ts`:
1. Удалён столбец «Internal dimensions» (нумерованные `R…`, выносные линии, счётчик меток) вместе с ключом `pdf.internal_dimensions` из всех четырёх словарей (`de/en/fr/ru.json`). Значение, которому не хватило места у своей стены, теперь просто не печатается — общее правило вместо fallback только для прямоугольных комнат.
2. Кегли подписей внутри плана уменьшены на четверть через именованные константы `DIMENSION_SIZE=5.25`, `AREA_SIZE=6`, `NAME_SIZE=6.75`, `NAME_SIZE_SMALL=5.25`; хром страницы (14/10/8/7) не тронут.
3. `docs/PDF-EXPORT.md` переписан в трёх местах: состав сцены (без «callouts»), правило подписи без места (не выносится, не печатается), fail-closed сужен до архитектуры.
4. `scripts/mutation-gate.mjs`: два новых мутанта (`pdf-restores-dimension-callouts`, `pdf-keeps-large-in-plan-labels`), три переоформленных (`pdf-room-edge-dropped`, `pdf-overflow-returns-clipped-page`, `pdf-rectangle-restores-unsafe-label`).
5. `test/pdf-scene.test.mjs`: два теста переписаны на противоположный смысл (были заявлены в ТЗ заранее), два новых теста, включая фикстуру-свидетеля («квадратный дом ≈11 м, восемь комнат, три непрямоугольные» — воспроизведение случая владельца).
6. `docs/CHANGELOG.md` / `.ru.md` — обе строки в том же коммите. `dist/**`, `custom_components/houseplan/frontend/**`, `demo/srv/assets/**`, `docs/images/screenshots.json` — сгенерированное (класс D), проверено побайтово.
Ряд масштабов, правило выбора масштаба/ориентации, ядро `pdf-dimensions.ts` не тронуты — контракт п.3 соблюдён буквально (нет ни строки диффа вне `pdf-scene.ts`/`pdf-layout.ts`-комментария).
## Как проверялось
Дешёвые гейты подтверждены зелёным Validate на этом SHA (https://github.com/Matysh/houseplan-card/actions/runs/34590334394) — `typecheck`, `test`, `build`, а также Validate-мутанты по диффу (запускаются пайплайном перед стартом ревью, §10.4). Дополнительно к этому подтверждению я перегонял сам, там где это дёшево и относится к защитным AC:
| Гейт | Команда | Результат |
|---|---|---|
| typecheck + build | `npm run build` | зелёный, `dist` собран |
| bundle sync | `npm run bundle:sync` + `cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js` | три копии бандла идентичны |
| unit-тесты | `npm test` | 2523 тестов, 2522 pass, 1 skipped, **0 fail** — совпадает с хендоффом |
| no-new-any | `node scripts/no-new-any.mjs --base origin/dev --head HEAD` | новых `any` нет (31 добавленная строка в 1 файле) |
| docs freshness | `node scripts/check-docs.mjs` | passed (7 files, 12 external links) |
| диффозависимый отбор смоков | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | **НЕОПРЕДЕЛЁННОСТЬ** — символы `DIMENSION_SIZE/AREA_SIZE/NAME_SIZE*`, `compactRing`, `dedupeOppositeDimensionEdges` и др. ни в одном смоке не зарегистрированы |
| топический смок | `node demo/smoke_pdf_export.mjs` (прогнан мной вручную, раз инструмент вернул неопределённость, а смок по имени прямо про изменённую поверхность) | `OK` |
| **защитные мутанты** (перепрогнаны мной лично, не только со слов автора) | `node scripts/mutation-gate.mjs --id=pdf-restores-dimension-callouts` / `pdf-keeps-large-in-plan-labels` / `pdf-room-edge-dropped` / `pdf-rectangle-restores-unsafe-label` / `pdf-overflow-returns-clipped-page` | все пять: «тест покраснел, как обязан», поймано 1 из 1 |
| golden (визуальный) | `npm run golden:verify` (после `bundle:sync`) | **`different`** ровно на `pdf-export-geometry-light`, `pdf-export-polish-light`, `pdf-export-stepped-dimensions-light` — точь-в-точь как заявлено в хендоффе; ни одного другого кадра из ~170 не разошёлся (шум «device-inbox-*», о котором писал автор из своей песочницы, в этом прогоне не воспроизвёлся — значит, это был шум его окружения, не дефект) |
| bundle budget | `node scripts/bundle-budget.mjs` | initial View 287 241 Б при потолке 288 300 ±2000; тот же преюдикт по запасу 13 825 Б, что и в хендоффе (долг #367/#474, не этой задачи) |
**Не прогонял:** `python -m pytest tests_backend` (ни один `.py`-файл не задет); performance-профили Validate (диффозависимый список триггеров — `src/iso-*`, `src/live-*`, `src/render-*`, `houseplan-render-lifecycle.ts`, `houseplan-card.ts` — ни один не пересекается с `src/pdf/**`); `node scripts/model-invariants.mjs` (диф не трогает рёбра/толщины/`layout`/`marker.space`/`open_spans` модели — `pdf-scene.ts` только читает уже посчитанную геометрию через неизменённый `pdf-dimensions.ts`, ничего не пишет и не мигрирует); полную матрицу из 242 браузерных смоков (диф ограничен одним файлом рендера PDF, топический смок прогнан явно, остальные, по отбору диффа, символьно не связаны).
## AC — доказательство и «чем краснеет»
| # | AC | Доказано | Чем краснеет (перепрогнано мной) |
|---|---|---|---|
| AC1 | Ни столбца, ни меток `R…` ни на одной фикстуре | `#530: dense non-rectangular rooms print what fits…`, `#530: a square house…` — оба зелёные в `npm test` | `pdf-restores-dimension-callouts` — поймано 1/1 |
| AC2 | Кегли 5.25/6/6.75/5.25, хром 14/10/8/7 не тронут | `deepEqual(sizes, [5.25, 6, 6.75, 7, 8, 10, 14])`, чтением подтверждены неизменённые строки хрома (`pdf-scene.ts:692-725`) | `pdf-keeps-large-in-plan-labels` — поймано 1/1 |
| AC3 | Фикстура дачи → 1:75, книжная, план крупнее в 1.33× | `output.scale === 75`, `output.width < output.height` в новом тесте, воспроизводящем форму дома владельца | оба мутанта AC1/AC2 сбивают масштаб — поймано |
| AC4 | Подписей не меньше, чем было (37 ≥ порог 36 против 35 до правки), дублей нет | тест на дачной фикстуре: `values.length >= 36`; `dedupeOppositeDimensionEdges` не тронут | `pdf-room-edge-dropped` (дедуп теряет ребро) — поймано 1/1 |
| AC5 | Заполнение листа на golden не ниже нового `minSceneCoverage` | **отклонение принято, см. ниже** | — |
| AC6 | Бюджет 200 мс CPU | существующий тест `current 20-room large-house space builds… under 200 ms` (не изменён), зелёный в общем прогоне | — |
| AC7 | `docs/PDF-EXPORT.md` без «numbered callouts» и без отказа из-за аннотации | прочитан построчно: `grep -in "callout"` находит только историческую фразу «Until v1.74.0-beta.2 such values went into a numbered…», формулировка fail-closed сужена до архитектуры | построчная сверка ревьюером |
### AC5 — отклонение, проверено самостоятельно, принимается
ТЗ предполагало, что удаление столбца поднимет `sceneCoverage` на golden-кадрах и порог `minSceneCoverage` нужно будет поднять до фактического значения. Автор в хендоффе заявил, что премиса неверна для конкретных golden-фикстур, и порог не поднимал. Я это не принял на слово, а проверил:
- `demo/golden/matrix.mjs:371-429` — сценарии `pdf-export-polish-light` и `pdf-export-stepped-dimensions-light` используют фикстуры `golden-geometry` и `golden-pdf-stepped`; ни в одной нет плотной непрямоугольной комнаты, у которой размер уходил в столбец — то есть у этих конкретных кадров столбца не было **до** правки, и убирать его было неоткуда: рост заполнения физически невозможен на этом материале, единственный эффект правки на них — уменьшение кеглей, слегка снижающее площадь текста.
- `git diff origin/dev...HEAD -- demo/golden/` — пусто: пороги `minSceneCoverage` (0.20 и 0.15) не менялись, что подтверждает: автор не подогнал число.
- Сам `npm run golden:verify` вернул статус `different` (пиксельное расхождение), а не `error`/`failed` — по коду `demo/golden/run.mjs:1157/1165` это означает, что все `pdfSemantic`-проверки (включая `minSceneCoverage`) прошли и на «до», и на «после»; регрессии по семантике нет.
Вывод: как записано, AC5 недоказуемо на выбранных кадрах не из-за дефекта реализации, а из-за неверной посылки самого ТЗ (эти кадры никогда не содержали случай, который отменяет правка). Реальное доказательство роста площади плана — AC3, юнит-тест на точной реконструкции фикстуры владельца (масштаб 1:75 против 1:100, то есть +33% линейно), и это не косвенное рассуждение, а прямое измерение. Поднимать порог до случайно уменьшившегося числа было бы фиксацией регресса под видом защиты — автор прав, что не стал. Принимаю как закрытое **без находки**: продуктовая цель («лист используется полнее») доказана AC3, а формулировка AC5 была ошибочной оценкой на этапе ТЗ, а не пропуском в реализации. Если владелец захочет именно golden-доказательство роста заполнения на плотной непрямоугольной комнате, это отдельная маленькая задача — завести новую golden-фикстуру, но это дополнение, а не починка дефекта этой задачи, отдельным issue заводить не обязательно (не найден дефект — не факт беспорядка).
## Находки
Нет ни одной блокирующей (High) или Medium-в-скоупе находки.
Три названных автором технических отклонения (переоформление мутантов `pdf-overflow-returns-clipped-page` и `pdf-rectangle-restores-unsafe-label`, расхождение «37 вместо прогнозных 38» подписей) — проверены построчно и перепрогонкой; все корректны и не меняют существа AC.
**Low, снимается без правки:** предупреждение `smoke-select.mjs` о «НЕОПРЕДЕЛЁННОСТИ» — законный результат для рефакторинга внутри одного рендер-модуля без изменения публичных точек входа; закрыто вручную прогоном топического `demo/smoke_pdf_export.mjs` (зелёный).
## Что проверено и корректно
- Контракт п.1–п.6 выполнен буквально, построчная сверка кода с ТЗ (`pdf-scene.ts` diff) не нашла расхождений.
- Масштабный ряд, выбор масштаба/ориентации и `pdf-dimensions.ts` не тронуты — риск «заодно поправили ряд» не подтвердился.
- Ключ `pdf.internal_dimensions` удалён из всех четырёх словарей и нигде в `src/**` больше не используется.
- `docs/PDF-EXPORT.md` описывает новое поведение без ложных утверждений (AC7).
- Class D (`dist/**`, `custom_components/houseplan/frontend/**`, стенд) синхронизирован и побайтово совпадает с пересборкой.
- `docs/images/screenshots.json`: только `sourceFingerprint`/`sourceSha256` обновлены (`docs:accept --identical`), ни один `imageSha256` не изменился — экранные скриншоты документации визуально не задеты этой правкой.
- Оба changelog правлены тем же коммитом, трейлеры `Issue: #530` / `User-Visible: yes` на месте.
- «Одно число — один источник»: PDF — разовый экспортируемый артефакт, ни одно из изменённых чисел (кегли, масштаб) не дублируется параллельным источником на экране — неприменимо к этому диффу.
- Golden-расхождение ожидаемо и ограничено тремя PDF-кадрами; принятие новых эталонов — обязанность релиз-кандидата (`npm run golden:accept -- --reviewed` на полном Linux CI артефакте с `Release:`/`Baseline-Reviewed:`), не этого ревью.
## Чего не проверял и почему
- `pytest tests_backend` — диф не касается `custom_components/**/*.py`.
- Performance-профили Validate (`large-house-*`) — диффозависимый триггер их не включает, изменённые файлы вне списка чувствительных путей.
- `node scripts/model-invariants.mjs` — диф не меняет геометрическую модель (рёбра, толщины, `layout`, `marker.space`, `open_spans`); `pdf-scene.ts` только читает уже посчитанные `dimensionEdges`.
- Полная матрица браузерных смоков (242 шт.) — не оправдана объёмом диффа (один файл рендера PDF); топический смок прогнан явно.
- Визуальная читаемость 5.25 pt на бумаге — вопрос, который автор сам адресовал владельцу в хендоффе для пре-бета проверки; это решённый на этапе ТЗ продуктовый компромисс (владелец согласился на −25% явно, см. S2-комментарий), а не открытый вопрос кода — не является находкой код-ревью, но стоит прогнать глазами на кандидате беты, как и планирует автор.
## Вердикт
Зелёный. Все AC доказаны — либо автотестом с подтверждённой мной способностью красить (AC1–AC4), либо чтением с построчной сверкой (AC7), либо существующим зелёным тестом (AC6); отклонение по AC5 разобрано и принято по существу, а не на слово. Находок нет.
---
<!-- material-anchors: сгенерировано конвейером (#414) -->
## Материал раунда
- Ветка: `issue/530-pdf-sheet-fill`, коммит `2e26116bbb2d` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
- Дерево материала: `526b70cd38eb890485064475a2517ec755187233`
```
git log --all --format='%H %T' | grep 526b70cd38eb
```
- Тело issue: `53fbdfb3b618a562787718e73ee5aa952eda2d9846768cdac42273c2de7ec382`
- Вердикт конвейера: `green` · High 0