From 06c7934667f6eb232da9ce24b4463294bb947f9c Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sat, 22 Aug 2026 08:40:24 +0000 Subject: [PATCH] docs: review document for #238 Issue: #238 User-Visible: no --- docs/reviews/SPEC-REVIEW-238-r1.md | 86 ++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-238-r1.md diff --git a/docs/reviews/SPEC-REVIEW-238-r1.md b/docs/reviews/SPEC-REVIEW-238-r1.md new file mode 100644 index 00000000..60c18539 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-238-r1.md @@ -0,0 +1,86 @@ +# SPEC-REVIEW-238-r1 + +- Issue: [#238](https://github.com/Matysh/houseplan-card/issues/238) — «Превью проёма: показывать расстояния до внутренних граней комнаты и стен» +- ТЗ: `docs/specs/238-opening-inner-distances.md` +- SHA ревью: `0498dda7cc49e5769eee51a402204bba92ac304b` (ветка `issue/238-opening-inner-distances`) +- Заход: r1 · блокирующих циклов израсходовано 0 из 4 +- Этап: ТЗ на ревью (PROCESS.md §2.4) +- Метка `small`/`trivial` отсутствует (сложность 7/10, новый UX-контракт) → полный трек, спека в `docs/specs/`, что и сделано корректно. + +## Скоуп проверки + +Первый заход — разбор полный (§2.10 неприменим до второго цикла). Проверено: + +1. `docs/SCOPE.md` — привязка к J4/J6, персона «администратор дома», Plan editor desktop-first. +2. `AGENTS.md`, `PROCESS.md` §2.4/§7.1/§7.2/§4 — обязательные разделы, лимит циклов, формат вердикта. +3. Тело issue #238 и оба комментария (аналитика S2, «ТЗ готово к ревью»). +4. `docs/USER-GUIDE.ru.md` (терминология проёмов, размеров Resize, инвариант Shift) и `docs/CANVAS.md` §9.4, `docs/WALL-THICKNESS.md` — канонические документы затронутой подсистемы. +5. Сам файл `docs/specs/238-opening-inner-distances.md` целиком (406 строк) и запись в `docs/specs/README.md`. +6. Исходный код, на который ссылается диагностика ТЗ (§3, §7–§11): `src/houseplan-card.ts`, `src/opening-placement.ts`, `src/logic.ts` (`openingShoulders`), `src/wall-thickness.ts` (`wallIntervals`, `roomWallProfile`, `innerContourForRoom`, `innerEdgeSpan`, `ownEdgeOffsets`), `src/physical-geometry.ts` (`partitionBody`, `wallBodiesGeometry`). +7. Существование упомянутых регрессионных smoke-файлов и golden-baseline. + +## Как проверялось + +Не код-ревью, поэтому автотесты не гонялись — это ревью ТЗ (§2.4): проверялась выполнимость и проверяемость, а не реализация. Конкретно: + +- `git log`/`git show 0498dda --stat` — коммит содержит только `docs/specs/238-opening-inner-distances.md` и `docs/specs/README.md`, `User-Visible: no`, `Issue: #238` — трейлеры корректны для чисто документационного коммита. +- `grep`/`Read` по `src/*.ts` — сверка каждого технического утверждения §3 «Подтверждённый диагноз» с реальным кодом (см. таблицу ниже). +- `grep -n shiftKey src/` — проверка утверждения AC10 про Shift на предмет «догадки, выданной за факт». +- Сверка списка файлов §14 и regression-smoke §17 с реальным деревом (`ls demo/smoke_*.mjs`, `ls demo/golden/baselines`). + +## Находки + +**High: 0 · Medium: 0 · Low: 0** (после проверки один кандидат в Low снят как необоснованный — см. ниже). + +Блокирующих и требующих правки замечаний нет. Ниже — два пункта, которые были целенаправленно проверены как потенциальные «догадки, выданные за факт», и оба сняты с объяснением, а не молча пропущены. + +### Проверенный кандидат №1: AC10 и упоминание Shift + +AC10: «Centre magnet/tick остаётся осевым; Shift не выключает его...». В коде `_resolveOpeningPlacement`/`_opRuler` (`src/houseplan-card.ts:11935-12238`) нет обращения к `shiftKey` вообще — центровочный магнит проёма безусловен. На первый взгляд это похоже на факт о поведении, которого в коде нет и который не помечен как предположение (что было бы замечанием по инструкции ревью). + +Проверка канона показала обратное: `docs/CANVAS.md` §9.4 фиксирует общий, не привязанный к конкретному инструменту, инвариант редактора — «`Shift` never suspends coordinate snapping» — и явно перечисляет, что для прочих инструментов Shift меняет только текущий жест (квадрат/круг, независимые оси resize, компас, обход furniture-магнита), но никогда не снимает базовый snap. AC10 — корректное применение уже существующего канонического правила к новому UX-элементу (центр-магнит проёма), а не изобретённая деталь. Поскольку в текущем коде Shift вообще не читается на этом пути, AC тривиально выполняется и работает как regression guard на будущее — не дефект. **Снято, находка не подтвердилась.** + +### Проверенный кандидат №2: §3 не называет `innerEdgeSpan`/`ownEdgeOffsets` + +§3 «Подтверждённый диагноз» перечисляет уже существующую физическую базу (`wallIntervals`, `roomWallProfile`, `innerContourForRoom`, `partitionBody`), но не упоминает `innerEdgeSpan`/`ownEdgeOffsets` (`src/wall-thickness.ts:876-959`), которые #233 уже завёл для ровно того же продуктового принципа — «мерить до внутренней физической грани, а не до оси» — и импортированы в `houseplan-card.ts:74`. + +Прочитал обе функции: `ownEdgeOffsets` сэмплирует толщину соседей **по одной точке — середине** цельного нарисованного ребра комнаты, а `innerEdgeSpan` режет это ребро целиком по двум соседним рёбрам. Для Resize (#233) это корректно: перетаскивается всё ребро целиком. Для §238 этого недостаточно **по существу**: превью проёма стоит в произвольной точке ребра, и на этой точке может лежать не тот атомарный отрезок толщины, что в середине ребра (ребро комнаты может быть склеено из нескольких atomic-интервалов с разной толщиной соседей — ровно то, что описывает модель в `docs/WALL-THICKNESS.md` §1, «atomic collinear spans»). Требование ТЗ §7.1 п.1 — «участок, покрывающий **центр candidate**», а не середину всего ребра — поэтому обоснованно требует нового резолвера на уровне atomic-профиля, а не переиспользования #233 «как есть». Отсутствие явной ссылки на `innerEdgeSpan`/`ownEdgeOffsets` в §3 — стилистический пробел (было бы полезно явно написать «недостаточно, потому что…», как самому ревью пришлось восстанавливать), но не ошибка и не выдуманное поведение. **Не поднимаю как находку** — ниже, чем Low, чисто редакторское замечание. + +## Что проверено и корректно + +Построчная сверка §3 «Подтверждённый диагноз» с кодом — всё подтвердилось: + +| Утверждение ТЗ | Проверка | +|---|---| +| `openingShoulders()` считает по осевым `roomEdges(rooms)`, не видит толщины | `src/logic.ts:269-308` — использует `pts` из `room.poly` напрямую, без offset/half-width | +| `_resolveOpeningPlacement()`/`resolveOpeningPlacementResult()` — единая точка разрешения candidate для hover/click | `src/houseplan-card.ts:11935,11956`; `_openingHoverCandidate` кеш переиспользуется кликом (`:12041-12044`, `:12588-12598`) | +| `OpMeasure` хранит только точку и текст, без геометрии сегмента | `src/houseplan-card.ts:688-691`: `{ labels: {x,y,text}[], guide }` — подтверждено, `from/to` действительно нет | +| `wallIntervals()` даёт half-thickness и roomId по каждому атомарному участку | `src/wall-thickness.ts:1294-1360`: `WallInterval{roomId, half, open, ...}` | +| `innerContourForRoom()`/`roomWallProfile()` строят внутренний контур через `insetContour` | `src/wall-thickness.ts:1545-1562` | +| `partitionBody()` строит тело независимой перегородки с её толщиной | `src/physical-geometry.ts:80-85` | +| Дорогой plan-wide boolean union существует и должен избегаться на pointermove | `wallBodiesGeometry` — тяжёлый вызов на весь `space.rooms`+`walls`+`openCuts` (`src/houseplan-card.ts:5175-5190,15583-15588`); кеш-инвалидация по `_cfgEpoch` — реальное поле (`:3329`, инкрементируется на изменениях конфига, уже используется как ключ кеша структурной геометрии, `:6711,6739`) | +| Разные `roomId` на одном сегменте → общая стена, распад до legacy при недоказуемости | Логика в `wallIntervals()` (по одному входу на каждую комнату) делает это семантически корректным без дополнительной дедупликации в исходнике | + +Проверено также: + +- Все обязательные разделы §7.1 PROCESS.md присутствуют и в верном порядке (сценарий/что увидит человек — первыми): §1–§21 покрывают сценарий, персону, диагноз/проблему, скоуп и не-скоуп, контракт поведения, модель данных/миграцию, i18n, touch/безопасность, AC1–AC15 с проверяемым доказательством (`unit`/`smoke`/`golden`/`ревью кода`), план автотестов, риски, откат, release-артефакты. +- AC1 арифметически проверен вручную: ось 400u, проём 80u по центру → legacy плечо (400-80)/2=160u; примыкающая стена глубиной 20u даёт half=10u инсет на каждом конце по правилу «growth half in / half out» (`docs/WALL-THICKNESS.md` §2) → 160-10=150u. Число в AC не взято с потолка. +- Регрессионные smoke-цели существуют: `demo/smoke_opening_measure.mjs`, `demo/smoke_opening_preview.mjs` — по факту; `demo/smoke_partition_opening_jamb.mjs` из текста не существует, но ТЗ прямо и корректно оговаривает это как предположительное имя («сверить с inventory») — реальный файл `demo/smoke_partition_openings.mjs` содержит нужный jamb-margin тест (issue #132/#185/#186). +- Golden: существующие `opening-placement-*-thick-wall-{light,dark}.png` в `demo/golden/baselines/` подтверждают, что сцена placement-preview для переиспользования/расширения реально существует, как и оговорено в §17 «либо новая минимальная». +- `docs/specs/README.md`: строка добавлена в верном месте таблицы, в двустороннем формате issue↔ТЗ, ссылки консистентны. +- Не-скоуп (§5.2) корректно опирается на формулировки issue: все три упоминания «размещения»/«превью проёма» в теле issue относятся к **новому** проёму, а не к drag уже сохранённого — сужение до `AC11`/§5.2 не является домысливанием. +- §21 «Принятые предположения» — блок присутствует, все 5 пунктов действительно технические/интерпретационные, не продуктовые в смысле §7.1 PROCESS.md (что видит/делает человек уже прямо зафиксировано issue), помечены как оспоримые. +- Аналитика (S2) корректно классифицировала задачу как обычный трек — сложность 7/10 превышает порог `small` (≤3). +- Откат (§19) реалистичен: persisted-полей нет, откат — чисто код. + +## Чего не проверял + +- Реализацию — её не существует, стадия ТЗ. +- Golden/визуальные baseline для новых 2/4 линий — они появятся только в коде; на этой стадии оценивался только план их получения (§17). +- Полный HA backend/`tests_backend` — задача не трогает `custom_components/**`, ТЗ явно это фиксирует (§12), не перепроверялось глубже grep по `custom_components`. +- Производительность фактическая (только план кеширования по тексту ТЗ, не измерения — на этой стадии измерений и не может быть). +- Не проверял весь `docs/UX-MODES.md`/`docs/TOUCH-SUPPORT.md` построчно — только целевые фрагменты про Shift/canvas и точечно про touch/best-effort инвариант, поскольку §13 ТЗ не меняет touch-контракт и это подтверждается отсутствием новых жестов в тексте. + +## Вердикт + +Зелёный. ТЗ выполнимо, каждый AC проверяем и привязан к способу доказательства, диагностика подтверждена чтением исходного кода, продуктовых вопросов владельцу нет — 5 явных предположений корректно оформлены и оспоримы. High: 0, Medium: 0.