docs: review document for #238

Issue: #238
User-Visible: no
This commit is contained in:
claude[bot]
2026-08-22 12:25:05 +03:00
committed by Sergey Matyunin
parent 2652bb360d
commit ace6e62f89
+90
View File
@@ -0,0 +1,90 @@
# CODE-REVIEW-238-r2
- Issue: [#238](https://github.com/Matysh/houseplan-card/issues/238) — «Превью проёма: показывать расстояния до внутренних граней комнаты и стен»
- Этап: код-ревью (PROCESS.md §2.7), заход **r2**, блокирующих циклов израсходовано **1/4** до этого разбора (r1 — красный, H1)
- Ветка: `issue/238-opening-inner-distances`
- Диапазон предыдущего вердикта (r1, красный): `origin/dev...e553875` (продуктовый коммит `553fb27`, docs-коммит `e553875`), документ `docs/reviews/CODE-REVIEW-238-r1.md`
- Диапазон этого раунда (дельта): `git diff e553875..HEAD`
- Коммиты дельты:
- `07bd070` docs: review document for #238 (публикация артефакта r1, класс C, не требует статуса)
- `2b48b16` fix: sync #238 bundle and lock shared order — `Issue: #238` · `User-Visible: no`
- Разбор — **по дельте** (PROCESS.md §2.10): дельта не трогает ни один файл класса A (`src/**` не изменён), меняет только три скомпилированные копии бандла (класс D) и добавляет один unit-тест (класс B). Это ровно случай локальной дельты — не ребейз на ушедший вперёд `dev`, контракт поведения не менялся, новая подсистема не задета, объём дельты (246 строк, почти всё — компилированный код и один тест) не сопоставим с исходной задачей (1044 строки в `553fb27`). Полный разбор поэтому не требуется.
## Скоуп раунда
Единственная блокирующая находка r1 (H1) — продуктовый коммит `553fb27` не пересобрал и не скопировал бандл в три отслеживаемые копии, из-за чего закоммиченный артефакт не содержал фичи #238 и заявленные автором прогоны против него не воспроизводились. Скоуп r2 — проверить, что H1 закрыта именно так, как предписал фикс, и что дельта не занесла ничего нового сверх этого.
## Закрытие раунда r1
| Находка r1 | Чем закрыта | Где это видно |
|---|---|---|
| **H1** — три копии бандла (`dist/houseplan-card.js`, `custom_components/houseplan/frontend/houseplan-card.js`, `demo/srv/assets/houseplan-card.js`) не пересобраны в `553fb27`, содержимое идентично точке ветвления `3aba493`, фича не доезжает до Home Assistant, `cmp` и smoke падают на закоммиченном дереве | Коммит `2b48b16` перезаписывает все три файла (120 строк diff в каждом — ровно объём, накопленный с точки ветвления, поскольку бандл не собирался все 4 предыдущих коммита) | `git show 2b48b16 --stat`; я пересобрал `npm run build` из текущего `HEAD` и получил хеш `de9576d3cf6b078eead36a472c20ab7f87284aeb18c34239383d4a31d4a3a707` — **byte-for-byte совпадает** со всеми тремя закоммиченными копиями (`sha256sum dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js demo/srv/assets/houseplan-card.js`, три одинаковых хеша), и `git status --short` после сборки пуст — пересборка не породила diff |
| Производный симптом H1: `node demo/smoke_opening_inner_distances.mjs` падал с `Error: ... is stale` против закоммиченного дерева | Тот же smoke прогнан против дерева **как оно закоммичено в HEAD** (без ручной пересборки перед прогоном, только штатный `npm run build`, который не изменил файлы) — `OK`, весь JSON-отчёт `true` | вывод `node demo/smoke_opening_inner_distances.mjs` в этом раунде, exit 0 |
| Производный симптом H1: `cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js` расходился → `frontend` CI job покраснел бы | То же сравнение теперь совпадает (см. sha256sum выше) | `sha256sum` трёх файлов — идентичны |
Других находок в r1 не было (Medium: 0), закрывать больше нечего.
## Как проверялось в r2
Дешёвые гейты (гоняются в каждом раунде, PROCESS.md §2.10):
| Гейт | Результат |
|---|---|
| `npx tsc --noEmit` | OK, чисто |
| `npm test` | 1049 passed, 0 failed (было 1048 в r1 — ровно +1 новый тест из `2b48b16`) |
| `npm run build` | OK, не изменил рабочее дерево (`git status --short` пуст) |
| сверка `dist/`, `custom_components/houseplan/frontend/`, `demo/srv/assets/` | все три — `de9576d3cf6b078eead36a472c20ab7f87284aeb18c34239383d4a31d4a3a707`, совпадают друг с другом **и** с тем, что закоммичено — H1 закрыта |
| `node scripts/check-docs.mjs` | не обязателен в этом раунде (дельта не трогает `src/**`), прогнан для очистки совести: `Documentation checks passed (7 files, 10 external links)` |
По необходимости — прогнал 4 smoke, названные в r1 как связанные с H1 (grep по изменённым сущностям `openingShoulders`/`OpMeasure`/`.opening-dimension`/`opening-dimensions.ts` даёт тот же список, что и в r1, дельта не добавила новых сущностей):
| Гейт | Результат |
|---|---|
| `node demo/smoke_opening_inner_distances.mjs` | OK, против бандла как закоммичен в HEAD (не пересобирал вручную перед прогоном) |
| `node demo/smoke_opening_measure.mjs` | OK |
| `node demo/smoke_opening_preview.mjs` | OK |
| `node demo/smoke_partition_openings.mjs` | OK |
Не прогонял (наследуется из r1, дельта их не задевает): mutation guards, golden capture, `performance_smoke`, `pytest tests_backend`, остальные ~160 browser smoke, полный `golden:verify`/`golden:accept`, HA-backend harness — обоснование то же, что в r1 (см. «Унаследовано из r1» и «Чего не проверял» ниже).
## Проверка добавленного в дельте теста
`2b48b16` добавляет в `test/opening-dimensions.test.mjs` тест «shared wall dimension order is independent of room config order (#238 AC3)»: комнаты `[b, a]` в обратном порядке дают тот же порядок размеров `['a','b','a','b']`, что и прямой. Прочитан код: `src/opening-dimensions.ts:207` — `rooms.sort((a, b) => a.roomId.localeCompare(b.roomId))` — существующая (не изменённая в этой дельте) сортировка по `roomId`, независимая от порядка в конфиге. Тест умеет падать: без этой строки сортировки порядок следовал бы порядку массива `rooms`, и для `[b, a]` результат был бы `['b','a','b','a']` — assert на `deepEqual` уловил бы расхождение. Это усиление покрытия AC3 сверх того, что проверялось в r1 (там AC3 проверялся на прямом порядке комнат), а не новая логика — `src/opening-dimensions.ts` в дельте не менялся (`git diff e553875..HEAD -- src/` пуст). Не расширяет скоуп, коммит корректно несёт `User-Visible: no`.
## Унаследовано из r1
Документ: `docs/reviews/CODE-REVIEW-238-r1.md`, SHA r1 `e5538752b6ba25a24e90f96fd8ed17e0fe2d6416`. Дельта r2 не трогает `src/**`, поэтому всё, что r1 доказал по коду и поведению (не по бандлу), принимается без повторной проверки:
- **AC1/AC2** (прямоугольная/скошенная стена, unit + mutation guard `opening-dimensions-use-axis-ends`) — принято из r1 §«Что проверено».
- **AC3** (общая стена, 4 значения, unit + mutation guard `opening-dimensions-collapse-shared-side` + smoke `sharedFourLines`/`sharedRoomOrder`/`sharedIndependentValues`/`sharedOppositeFaces`) — принято из r1; дельта добавляет к нему тест на обратный порядок комнат, проверен отдельно выше, не пересматривает исходный вывод.
- **AC4** (вогнутая комната, unit) — принято из r1.
- **AC5–AC7** (независимая перегородка: T/косой стык, one-sided fallback, торец, unit + mutation guard `opening-dimensions-use-crossing-axis`) — принято из r1.
- **AC8** (линии/засечки, pointer-inert/aria-hidden, smoke + mutation guard `opening-dimension-overlay-hidden`) — принято из r1.
- **AC9** (синхронность с pointermove; click/save/jamb/serial-preset не меняются) — принято из r1 (grep-подтверждение, что диффы локализованы вне этих путей).
- **AC10** (центр-магнит/Shift не выключает снаппинг; физические размеры не участвуют в snap) — принято из r1.
- **AC11** (drag существующего проёма не получает новых линий) — принято из r1.
- **AC12** (formatLength, без новых i18n-ключей/persisted fields) — принято из r1.
- **AC13** (light/dark, различимость, golden-капчур трёх сцен) — принято из r1; golden baseline не принимался ни в r1, ни в r2 (это пре-релизный шаг с `--reviewed`).
- **AC14** (кеш статического контекста, без polyclip на pointermove) — принято из r1.
- **AC15** (оба changelog + оба USER-GUIDE в user-visible коммите `553fb27`) — принято из r1; коммит дельты `2b48b16` не несёт `User-Visible: yes`, поэтому новой записи changelog не требует, и её нет — корректно.
- **Скоуп** (`src/opening-placement.ts`, `src/resize.ts` не тронуты; drag существующего проёма геометрически не изменён) — принято из r1, дельта это не меняет.
- **Docs-гейт** (`check-docs.mjs`, синхронный коммит скриншотов `e553875`) — принято из r1; в r2 фингерпринт не мог измениться, так как `src/**` не менялся, что и подтвердил повторный прогон.
## Находки
Нет. H1 закрыта воспроизводимо (см. таблицу выше), новых High/Medium дельта не вносит.
## Чего не проверял
- **Полный `npm run golden:verify`/`golden:accept`** — не гонял; наследуется из r1 (частичный капчур трёх сцен там же признан достаточным, полный прогон — пре-релизный гейт, PROCESS.md §8, §11.4). Дельта r2 не меняет рендер (только бандл того же самого уже проверенного в r1 источника).
- **`performance_smoke`** — не гонял; не назван в AC, дельта не трогает чувствительные к перфу пути (кеш контекста не менялся).
- **`python -m pytest tests_backend -q`** — не гонял; дельта не трогает `custom_components/**/*.py`.
- **Mutation guards (4 шт., специфичные для #238)** — не перезапускал в r2; `src/opening-dimensions.ts` не изменился с r1, где все 4 воспроизводимо красные на своих мутантах.
- **Остальные ~160 browser smoke, не относящиеся к теме** — не прогонял; выбор по тому же grep, что и в r1, дельта не ввела новых сущностей.
- **Полная HA-harness backend-сборка** — не запускал, задача backend не задевает.
## Вердикт
H1 закрыта: все три копии бандла byte-for-byte идентичны друг другу и результату пересборки из текущего `src`, целевые smoke проходят против дерева как оно закоммичено (без ручной подгонки), тесты и typecheck зелёные. Дельта не тронула ни одного файла класса A, добавила только тест, усиливающий уже подтверждённый в r1 AC3. Всё, что r1 доказал по логике, покрытию и документации, остаётся в силе без изменений.
**Вердикт: зелёный · заход r2 · блокирующих циклов 1/4 · High: 0 · Medium: 0 → в задаче**