docs: review document for #373
Проверка (CI) / Классификация изменённых файлов (push) Successful in 26s
Проверка (CI) / HACS: валидация репозитория (push) Skipped
Проверка (CI) / Hassfest: манифест интеграции (push) Skipped
Проверка (CI) / Предполётные проверки: документация, провенанс, процесс (push) Failing after 44s
Проверка (CI) / Переиспользование: это дерево уже проверено (push) Successful in 44s
Проверка (CI) / Бэкенд: pytest в Home Assistant (push) Skipped
Проверка (CI) / Фронтенд: типы, юниты, мутанты, синхрон бандла (push) Failing after 7m28s
Проверка (CI) / Смоки в браузере (шард 1 из 3) (push) Skipped
Проверка (CI) / Смоки в браузере (шард 2 из 3) (push) Skipped
Проверка (CI) / Смоки в браузере (шард 3 из 3) (push) Skipped
Проверка (CI) / Смоки: все шарды зелёные (push) Skipped
Проверка (CI) / Golden-кадры против принятых эталонов (push) Skipped
Проверка (CI) / Перф-смок: бюджет времени кадра (push) Skipped

Issue: #373
User-Visible: no
This commit is contained in:
claude[bot]
2026-08-29 18:33:49 +00:00
parent 41c09ea6a2
commit 720200e3df
+276
View File
@@ -0,0 +1,276 @@
# CODE-REVIEW-373-r2
Issue: [#373](https://github.com/Matysh/houseplan-card/issues/373) — Add a fit-to-content / crop option for `houseplan-space-card`
Материал ревью (`git rev-parse HEAD` перед подведением итогов): `41c09ea6a20d18c88a444436770f3b1f88b7bea7`
Диапазон: `git log origin/dev..HEAD` = `4a343814` (spec), `a143a8e6` (spec-review doc), `0d33691f` (feat), `7ebead5d` (code-review r1 doc), `f56d9a76` (build: rebase-conflict fix + regression test), `41c09ea6` (docs: accept rebased canonical screenshots)
Заход: r2 · блокирующих циклов израсходовано 0 из 4 (r1 был зелёным — §227/#227, зелёный вердикт бюджет не тратит)
## Почему разбор в этом заходе полный, а не по дельте
r1 (`docs/reviews/CODE-REVIEW-373-r1.md`) был **зелёным** на материале `7153e124`
(High: 0, Medium: 0, три Low сняты решением ревьюера). Слияние не удалось:
пока шло ревью, `origin/dev` продвинулся на 4 коммита (#375 landed), ветка
`issue/373-space-card-house-fit` конфликтовала с новым `dev`. Задача вернулась
в `S6-in-progress` не по находке, а по несостоявшемуся мержу — цикл за это не
считается (AGENTS.md «the comment says which», issue-комментарий от
2026-08-29T18:12:38Z). Автор перебазировал ветку на `origin/dev` (`51196c65`),
разрешил конфликт руками и явно запросил полный повторный прогон
(issue-комментарий 18:24:29Z: «после ребейза прошу полноценный повторный
прогон по новому дереву»).
Это ровно случай из PROCESS.md §2.10: «ребейз на ушедший вперёд `dev` — после
ребейза это другой код», разбор остаётся полным, а не по дельте. Старые SHA
r1-диапазона (`a925a1c3`, `ff977a8a`, `95d91595`, `7153e124`, `e7ba22c6`)
больше не существуют в репозитории (`git cat-file -t` отказывает на каждом) —
подтверждение, что это настоящий force-push/ребейз, а не расширение той же
истории.
Практически это означает: AC1–AC8 проверены заново по текущему дереву
(раздел «Проверка по AC» ниже), а не унаследованы как записи. Но поскольку
почти весь диф оказался **побайтово идентичен** содержимому, уже разобранному
в r1 (сверено прямым `git diff` между `origin/dev` (уже содержащим #375) и
`HEAD` — см. ниже), основной объём чтения не переоткрывал вопросы, закрытые в
r1, а подтверждал их неизменность и фокусировался на том, что ребейз
действительно поменял: перестановку блока `canonicalWallGeometry` относительно
кэш-тега #375, новый регрессионный юнит-тест, удаление осиротевших
locale-чанков и переснятые канонические скриншоты.
## Закрытие раунда r1
| Находка r1 | Чем закрыта | Где видно |
|---|---|---|
| Low-1 — нет browser-смок-проверки state-tick/narrow-wide для `fit: house` | Не чинилась (снята решением ревьюера в r1, риск не подтверждён) | `demo/smoke_space_card.mjs` не изменился в этой части в r2-диффе (сверено `git diff origin/dev...HEAD` — блок `fit === 'house'` в `src/space-render.ts` по-прежнему не читает `hass`/`entities`, `amount: 0` жёстко зашит) |
| Low-2 — фолбэк «пустая структура → content frame» не покрыт тестом на уровне проводки | Не чинилась (снята решением ревьюера в r1) | `renderSpaceStatic`: `fr = structuralFrame(structure) || fr` — строка не изменилась при ребейзе, риск тот же, что в r1 |
| Low-3 — константы половины штриха (2.5/0.6) дублируют `stroke-width` без общего источника | Не чинилась (снята решением ревьюера в r1) | `src/space-render.ts`, блок `fit === 'house'` — те же литералы, `.room-outline`/`.wallbody` stroke-width не изменились |
| (r1 не находка, но условие) зелёный вердикт, слияние не удалось | Ребейз на `origin/dev` (`51196c65`), конфликт разрешён, регрессия #375 закрыта отдельным юнит-тестом | Коммиты `f56d9a76`, `41c09ea6`; `test/space-render-caches.test.mjs` (+5 строк, `assert.ok(... < ...)` на порядок кода) |
Все три Low остаются в силе как **сознательно снятые**, не как «забытые»: код,
к которому они относятся, не изменился ни на строку между r1 и r2 (подтверждено
диффом, а не памятью автора). Отдельно значимое новое событие раунда —
конфликт с #375 (полностью новый материал, которого не было в r1) — закрыт
корректно: см. AC-независимую находку ниже отсутствует, потому что решение
проверено и не создаёт нового риска (см. «Проверка по AC», AC4/AC7 и рубрику
про #375 ниже).
## Унаследовано из r1 (сверено, не принято на слово)
Для следующих файлов текущий `git diff origin/dev...HEAD` (где `origin/dev`
уже включает #375) дал содержимое, побайтово совпадающее с тем, что описано и
разобрано в `docs/reviews/CODE-REVIEW-373-r1.md` на SHA `7153e124` —
переставлять код заново не пришлось, но **логика перепроверена по AC в этом
заходе**, а не унаследована слепо:
- `src/space-geometry.ts` — `resolveSpaceCardFit`, `itemOfGeometry`,
`expandItem`, `structuralFrame` (доказательство: полный `git diff`, ни одной
строки отличия от функций, разобранных в r1);
- `src/render/opening-symbol.ts` — `openingVisibleBounds`, `bodyTranslation`,
аналитический sweep для window/door/gate (доказательство: та же строка в
строку, включая комментарии);
- `src/space-card.ts`, `src/space-editor.ts`, `src/i18n/{en,ru,de,fr}.json` —
публичный `fit`-контракт, редактор, i18n-ключи (доказательство: диф
идентичен, включая точные строки лейблов);
- `docs/CHANGELOG.md`, `docs/CHANGELOG.ru.md`, `docs/USER-GUIDE.md`,
`docs/USER-GUIDE.ru.md`, `docs/ARCHITECTURE.md`, `docs/CANVAS.md §4.4` —
released-артефакты AC8, содержимое не изменилось.
Не унаследовано слепо, перепроверено заново из-за прямого затрагивания
рёбейзом: `src/space-render.ts` (порядок вычислений и точка внедрения тега
#375 — см. ниже), `test/space-render-caches.test.mjs` (новый тест),
`demo/srv/assets/**`/`dist/**`/`custom_components/houseplan/frontend/**`
(бандл-хеши и локали изменились из-за нового содержимого `dev`), скриншоты
документации (переснятые из-за нового `oxipng` в `dev`).
## Как проверялось
Диапазон `origin/dev..HEAD` был **не отфильтрован по путям** конвейером
Validate: по логам классификации (`Классификация изменённых файлов`, run
`33267869890`) `BEFORE_SHA=5284bfb0…` не существует в дереве после ребейза
(`git cat-file -e` отказал), что скрипт распознаёт как force-push (#347) и
намеренно запускает **все** тяжёлые job без фильтра путей вместо угадывания
диапазона. Это дало полный, а не «дешёвый», прогон на рёбейзнутом дереве:
| Гейт | Где выполнен | Результат |
|---|---|---|
| `npx tsc --noEmit` | CI Validate, job «Фронтенд…», run [33267869890](https://github.com/Matysh/houseplan-card/actions/runs/33267869890) (SHA `f56d9a76`) | OK |
| `npm test` | тот же job | 1564 tests: 1563 pass, 0 fail, 1 skipped (единственный skip — известная приватная фикстура #281, не связана с задачей) |
| `npm run build` + сверка бандла | тот же job | OK; `npm run bundle:budget`: initial View 276981/300000 B gzip, headroom 23019 B, editor 139742 B, locale 45363 B |
| `node scripts/no-new-any.mjs --base origin/dev --head HEAD` | тот же job | шаг зелёный (job success, шаг не упал) |
| Полный набор `demo/smoke_*.mjs` (3 шарда) | job «Смоки в браузере» × 3 + «Смоки: все шарды зелёные», тот же run | все success |
| `npm run golden:verify` | job «Golden-кадры против принятых эталонов», тот же run | success |
| `performance_smoke` | job «Перф-смок: бюджет времени кадра», тот же run | success |
| `node scripts/check-docs.mjs` | job «Предполётные проверки…», run 33267869890 | **failure** (`screenshot source fingerprint is stale`) → исправлено коммитом `41c09ea6`, тот же гейт green в run [33268157290](https://github.com/Matysh/houseplan-card/actions/runs/33268157290) (SHA `41c09ea6`, текущий HEAD) |
| `process-gate.mjs` / provenance / process.yml sync | оба run | success на обоих SHA |
| Docs screenshots (класс D, канонический прогон) | run [33268042941](https://github.com/Matysh/houseplan-card/actions/runs/33268042941), `workflow_dispatch` | success; `headBranch: issue/373-space-card-house-fit`, `headSha: f56d9a76` (поле ответа API, не вводящий в заблуждение ярлык списка ранов, как в r1) — снято реально с этого дерева |
| Бэкенд/hassfest/HACS | оба run | skipped — диф не касается `custom_components/**/*.py`, `manifest.json`, `hacs.json` (корректно, задача не затрагивает эти поверхности) |
На SHA `41c09ea6` (текущий HEAD) job «Переиспользование: это дерево уже
проверено» легитимно **не перезапускает** smoke/golden/performance_smoke:
ключ переиспользования (`scripts/gate-reuse.mjs`, issue #208) хеширует
`sourceFingerprint` (src/**, demo/fixtures, package.json/lock, rollup,
tsconfig) плюс оснастку каждой job (`demo/smoke_*.mjs` для smoke,
`demo/golden/**` для golden, `demo/performance/**` для perf) — ни один из
этих путей не входит в дельту между `f56d9a76` и `41c09ea6` (только
`docs/images/*.png` и `docs/images/screenshots.json`), поэтому переиспользование
корректно, не молчаливый пропуск. `backend`-маркер не найден в кэше, но job всё
равно `skipped` — по независимой причине (path-classification: диф не
затрагивает `custom_components/**/*.py`), не из-за реюза.
Проверено самостоятельно, вне CI (лёгкая сверка, не гейт):
| Проверка | Результат |
|---|---|
| `ls custom_components/houseplan/frontend/houseplan-assets/ \| grep -E '^(de\|fr)-'` и то же для `dist/` | ровно `de-BwEMXoV0.js`, `fr-Ca0KCtpf.js` в обеих копиях, совпадают с `houseplan-assets.json.lazyLocaleFiles` — осиротевшие `de-tOerxd73.js`/`fr-_8bPsmzm.js` из коммита `f56d9a76` действительно удалены и нигде не переживают |
| `grep -n 373 docs/specs/README.md` | строка на месте (Low спек-ревью r1 закрыт) |
| Построчное сравнение `src/space-render.ts` до/после переноса блока `canonicalWallGeometry` | перенос идентичен по содержимому блоку, уже перенесённому в r1 (диапазон `resolvedRawOpenings`…`hostedCompositeOpenings`), плюс перенос самого вычисления `wallGeometryFingerprint`/`canonicalWallGeometry` (было после `fr`, стало до) с сохранением тега `sourceFingerprint` из #375 внутри того же `cachedStaticWallGeometry(...)` вызова — семантика не изменилась, доказательство ниже |
### Гейты, которые не гонялись повторно и почему
Полный typecheck/test/build/golden/smoke/perf уже зелёные на этом дереве по
CI (см. таблицу выше) — перегонять их локально было бы дублированием того же
прогона тем же кодом. `node scripts/model-invariants.mjs` не запускался:
диф не трогает персистентную геометрическую модель (рёбра комнат, записи
толщины, `layout`, `marker.space`, `open_spans`) — весь диф этой задачи
работает с производной, вычисляемой на лету структурой для одного bounding-box
static-карточки (`ContentItem[]` в памяти рендера), не с сохранённым
конфигом/`layout`. `python -m pytest tests_backend` не запускался — диф не
касается `custom_components/**/*.py` (подтверждено `git diff --stat`, ноль
файлов `.py`).
## Находки
Ни одной High, ни одной Medium. Три Low **унаследованы из r1 без изменений**
(см. таблицу «Закрытие раунда r1» выше) — код, к которому они относятся, не
менялся между r1 и r2, повторное решение ревьюера то же: сняты, реальный риск
на сегодняшнем дереве отсутствует, правка не обязательна.
Новых находок в материале, добавленном рёбейзом (`f56d9a76`, `41c09ea6`), нет.
Отдельно проверено:
- **Порядок вычислений и тег #375 не сломан.** `canonicalWallGeometry`
(используется в `fit === 'house'` для стеновых компонентов) теперь строится
до выбора `fr`, но `Object.defineProperty(built, 'sourceFingerprint', ...)`
внутри `cachedStaticWallGeometry(...)` перенесён вместе с вычислением —
тег не потерян и не переставлен в другую функцию. Новый тест
`test/space-render-caches.test.mjs` («#373 may move canonical geometry
before framing, but must carry the #375 tag with it») проверяет это
текстовым сравнением позиций (`indexOf` определения тега `<` `indexOf`
`const contentFrame =`) в исходнике `space-render.ts`; тест умеет падать —
если правку отменить (вернуть блок геометрии после `contentFrame`, оставив
тег на старом месте), `indexOf` тега станет больше и `assert.ok` упадёт.
Проверено вручную рассуждением, не запуском мутации (запуск полного `npm
test` уже прогнал этот тест зелёным в CI, см. таблицу гейтов).
- **Осиротевшие locale-чанки не остаются в дереве.** Подтверждено чтением
файловой системы (таблица выше) и тем, что `houseplan-assets.json` ссылается
ровно на существующие имена.
- **Коммит `f56d9a76` смешивает класс D (удаление чанков) и класс B
(добавление теста) под сообщением «build: …»**. Не нарушение процесса:
правило о `Release:`/`Baseline-Reviewed:` относится только к коммитам
**исключительно** класса D (PROCESS.md §10.2 п.5), а этот коммит правит и
тестовый файл. Трейлеры `Issue: #373`/`User-Visible: no` на месте. Чисто
косметическая неточность сообщения (не отражает добавление теста) — Low,
снимается: не влияет ни на один AC, не создаёт риска, не стоит отдельного
цикла ревью ради переформулировки уже запушенного сообщения.
## Проверка по AC
- **AC1 (совместимость по умолчанию)** — доказано автотестом:
`test/canvas.test.mjs` («static-card fit literals fail closed…» — все из
`undefined, null, '', 'content', 'cover', 1` резолвятся в `'content'`,
`'house'` резолвится в `'house'`, точный `spaceFrame` фикстуры сверен
`deepEqual`), `test/space-card-fit.test.mjs` (контракт `SpaceCardConfig`/
редактора по исходнику), и живым прогоном `demo/smoke_space_card.mjs` в CI
(`frame === explicitContentFrame === unknownFitFrame`, зелёный в обоих run).
Тест умеет падать: `resolveSpaceCardFit` — явный резолвер с фиксированным
списком, любое отклонение default-ветки меняет `assert.equal`.
- **AC2 (тесная структурная рамка, исключение подложки/декора/маркеров/подписей)**
— доказано юнитом (`structuralFrame` с комнатой+стеной+«detached wing»,
точный `deepEqual` на `{x,y,w,h}`, включая защиту от `Infinity`) и смоком в
CI (реальная карточка с подложкой/декором/маркером/подписью, `tightFrame`
строго меньше `frame` по всем границам — числовое условие в самом смоке,
зелёное). Код-ревью подтверждает: в `structure` (блок `fit === 'house'`)
попадают только `roomItem`, компоненты `canonicalWallGeometry`, `extras`,
`zeroWalls.lines`, `openingVisibleBounds` — ни подложка, ни декор, ни
маркеры, ни подписи не push'атся ни в одну ветку.
- **AC3 (без обрезки структуры)** — доказано: `test/opening-symbol.test.mjs`
(state-независимая огибающая door/window/gate, диагональные углы, толстые
косяки, оба направления ворот, passage → `null`) + `test/canvas.test.mjs`
(collinear-защита `structuralFrame([box(100,250,900,250)])` даёт конечный
`h > 0`) + смок `tightPaintedEnvelope.contained === true` (реальные
`.wallbody/.zero-wall/.static-opening` строго внутри `viewBox` с допуском
0.51 px) — все три зелёные в CI-прогоне на этом SHA.
- **AC4 (безопасный откат)** — collinear/invalid доказаны юнитом (см. выше);
state-tick независимость доказана юнитом напрямую
(`assert.deepEqual(openingVisibleBounds(spec({amount:0})),
openingVisibleBounds(spec({amount:1})))` — сильнее, чем в r1 описано:
это точное равенство всего объекта bounds, не только числового частного
случая); проводка `fr = structuralFrame(structure) || fr` для
image-only/empty — по-прежнему проверено чтением, не исполнением (Low-2,
унаследовано, риск не изменился).
- **AC5 (редактор/i18n)** — доказано автотестом (`test/space-card-fit.test.mjs`
сверяет точные строки `src/space-editor.ts`/`src/space-card.ts` по regex) +
i18n-диффом (en/ru/de/fr получили `editor.framing`, `editor.fit_content`,
`editor.fit_house`, `cover` нигде не предлагается — `assert.doesNotMatch`).
- **AC6 (View/touch)** — ширины теперь 320/900 px (смок обновлён с 640 на 900,
совпадает с AC6 буквально); инертность сцены и кнопка футера — доказаны
смоком (`tightPointerEvents === 'none'`); узкая/широкая ширина и live-смена
состояния датчика для `fit: house` конкретно — по-прежнему не в смоке (Low-1,
унаследовано без изменений: блок `fit === 'house'` не читает `hass`, риск
тот же, что в r1).
- **AC7 (перформанс)** — проверено чтением: ни одна из новых функций
(`structuralFrame`, `itemOfGeometry`, `expandItem`, `openingVisibleBounds`)
не обращается к DOM/`getBBox`; `bundle:budget` зелёный с запасом 23019 B в
CI на этом SHA. Взаимодействие с кэшем #375: перестановка вычисления
`canonicalWallGeometry` раньше по функции не меняет частоту его
инвалидации — фингерпринт (`wallGeometryFingerprint`) как и раньше зависит
только от геометрии, а не от `fit`, поэтому `fit: house` не создаёт новых
кэш-промахов на state-only тиках.
- **AC8 (документация/релиз)** — оба changelog, `USER-GUIDE.md`/`.ru.md`,
`ARCHITECTURE.md`, `CANVAS.md §4.4` на месте и не изменились относительно
r1-содержимого (см. «Унаследовано»); `docs/specs/README.md` содержит строку
#373; канонические скриншоты переприняты на самом SHA рёбейза
(`Baseline-Reviewed: .../runs/33268042941`, независимо подтверждено полем
`headBranch`/`headSha` ответа API, а не текстовым ярлыком UI).
## Одно число — один источник
Как и в r1: `viewBox` — единственная потенциально дважды видимая пользователю
величина в этом диффе, но `content` и `house` — два разных режима с одним
путём вычисления каждый (`spaceFrame(...)` vs `structuralFrame(structure)`),
результат сходится в одной переменной `fr`, из которой строится единый `vb`,
используемый один раз ниже по функции для SVG/маркеров/подписей/подложки/
continuity. Перестановка кода рёбейзом это не затронула — `const vb = [fr.x,
fr.y, fr.w, fr.h]` остался единственным местом присвоения. Внутренние
константы половины штриха (2.5/0.6) по-прежнему дублированы (Low-3,
унаследовано) — не пользовательская величина, правило не про них.
## Что не проверялось
- Локальный повторный прогон `typecheck`/`test`/`build`/`golden:verify`/
полной матрицы смоков — не требовался: CI Validate уже прогнал все эти
гейты на этом самом дереве двумя последовательными push'ами (см. таблицу),
включая полный (не выборочный) набор смоков и golden — сильнее, чем
предписывает §8 для код-ревью.
- `node scripts/model-invariants.mjs` — диф не касается персистентной
геометрической модели (обоснование выше).
- `python -m pytest tests_backend` — диф не касается `custom_components/**/*.py`.
- Мутационный прогон нового теста в `test/space-render-caches.test.mjs` (не
отменял правку вручную, чтобы убедиться в падении) — падение обосновано
логическим разбором `indexOf`, а не эмпирически; сам тест выполнялся и
проходил в составе `npm test` на CI.
- Пиксельное сравнение переснятых `docs/images/*.png` самим ревьюером — принято
по зелёному `check-docs.mjs` на `41c09ea6` и по независимо проверенному факту
(поля API `headBranch`/`headSha` рана 33268042941), что канонический прогон
снят именно с этого дерева.
## Вердикт
Зелёный. High: 0. Medium: 0. Три Low унаследованы из r1 без изменений и
остаются снятыми решением ревьюера (код, к которому они относятся, не менялся
между раундами). Материал раунда, добавленный ребейзом — перестановка кода
вокруг тега #375, новый регрессионный тест, удаление осиротевших
locale-чанков, переснятые канонические скриншоты — проверен по AC и не
создаёт новых находок; единственная мелочь (неточное сообщение коммита
`f56d9a76`, смешавшего класс D и B под «build:») — Low, снимается, не в
скоупе обязательной правки.
**Вердикт: зелёный · заход r2 · блокирующих циклов 0/4 · High: 0 · Medium: 0**