mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,196 @@
|
||||
# SPEC-REVIEW-300-r2
|
||||
|
||||
- Issue: [#300](https://github.com/Matysh/houseplan-card/issues/300) — Подписи при ресайзе: подсвечивать измеряемые стены, убрать размер перетаскиваемой, площадь показывать по бокам от неё
|
||||
- Этап: spec (PROCESS.md §2.4)
|
||||
- ТЗ: `docs/specs/300-resize-measurement-layout.md`, коммит `2b7c98e6bcc4a7fed920c05f2298a06a138683fd` на ветке `issue/300-resize-labels`
|
||||
- Заход: r2 · блокирующих циклов израсходовано (до этого раунда) 1/4
|
||||
- Ревьюер: Claude (роль «ревьюер ТЗ», не автор)
|
||||
|
||||
## Скоуп разбора
|
||||
|
||||
Раунд не первый → разбор по дельте (PROCESS.md §2.9, issue #214), не заново.
|
||||
|
||||
1. Вердикт r1 найден в комментарии issue #300 (2026-08-24T19:14:59Z): жёлтый,
|
||||
заход r1, блокирующих циклов 1/4, High 0 / Medium 2 → в задаче. SHA, на
|
||||
котором получен вердикт r1, в самом комментарии не назван — это находка,
|
||||
отдельно не заводится (не Medium, не блокирует), восстановлен из
|
||||
`docs/reviews/SPEC-REVIEW-300-r1.md` (шапка документа) и из тела следующего
|
||||
коммита: **`ab9a83e755211a599ba8d73be0a3716696a5168b`**.
|
||||
2. Автор ответил комментарием 2026-08-24T19:19:16Z: правки внесены и запушены
|
||||
в **`2b7c98e6bcc4a7fed920c05f2298a06a138683fd`**, явно указана дельта для
|
||||
повторного ревью: `git diff ab9a83e..2b7c98e -- docs/specs/300-resize-measurement-layout.md`.
|
||||
3. Дельта проверена: `git diff ab9a83e7..2b7c98e6 -- docs/specs/300-resize-measurement-layout.md`
|
||||
— правка ограничена файлом спецификации, 40 добавленных / 18 удалённых
|
||||
строк. Дополнительно `git diff ab9a83e7..2b7c98e6 --stat` показывает, что
|
||||
между раундами появился только сам документ `SPEC-REVIEW-300-r1.md`
|
||||
(публикация предыдущего ревью) — это не продуктовый код и не меняет предмет
|
||||
разбора.
|
||||
4. Дельта локальна: тот же файл, тот же раздел (правки только в §3, §4 п.6,
|
||||
§5, §6.3, §7, §10, §11 (AC7/AC11), §12, §13, §14, §18, заключительный
|
||||
абзац §18) — не ребейз на ушедший вперёд `dev` (весь `origin/dev..HEAD`
|
||||
всё ещё только markdown), не смена контракта поведения целиком, не задета
|
||||
новая подсистема. Полный повторный разбор не требуется; проверяются:
|
||||
закрытие M1/M2, и все AC/разделы, чьё доказательство эта дельта задевает
|
||||
(AC6, AC7, AC11 и связанные с ними §6.3, §7, §12, §13, §14, §18).
|
||||
5. Гейты кода неприменимы, как и в r1: класс изменений — C (документация),
|
||||
диапазон `origin/dev..HEAD` не содержит `src/**`/`test/**`.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Проверено чтением (не исполнением), только по дельте:
|
||||
|
||||
| Утверждение правки | Где проверено |
|
||||
|---|---|
|
||||
| Класс `.rlgearbtn` — реальное имя кнопки настроек комнаты (не `.roomgear`, как было в r1) | `src/styles.ts:1000`, `src/houseplan-card.ts:18705` (`_renderRoomGear`) |
|
||||
| `demo/benchmark_safe_resize_render.mjs` действительно использует абсолютный потолок `RENDER_P95_MS = 25`, без relative-сравнения | `demo/benchmark_safe_resize_render.mjs:9,64,75` — совпадает с формулировкой нового AC11 |
|
||||
| Утверждение нового §6.3/§18.4 «screen-fixed footprint `.rlgearbtn`» проверено против реального CSS кнопки | `src/styles.ts:996-1008` — `--gear-h: calc(var(--icon-size, 2.5cqw) * 0.77)`; `--icon-size` вычисляется `iconCqw()` |
|
||||
| `iconCqw()` явно и намеренно **зависит от текущего zoom/viewBox**, а не постоянна в screen px | `src/space-geometry.ts:463-489` — докстринг: «An icon is a percentage of the PLAN, not of the viewport: it scales with the plan as you zoom… (owner, 2026-08-03)»; формула `(iconPct * iconUnit(space) * k) / w`, где `w = viewW` |
|
||||
| Тот же вывод подтверждён комментарием в стилях | `src/styles.ts:997-998`: «icon-size already rescales with the view… so the button zooms WITH the plan instead of keeping a constant screen size (owner's spec)» |
|
||||
| `_renderRoomGear` позиционирует кнопку в процентах контейнера (`left/top: %`), что синхронно с zoom — сама позиция не проблема | `src/houseplan-card.ts:18701-18703` |
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка r1 | Чем закрыта | Где это видно |
|
||||
|---|---|---|
|
||||
| **M1** — временное скрытие `.roomgear` во время Resize объявлено «acceptance contract» без подтверждения владельца и без прецедента в коде | Автор не стал спрашивать владельца и не оставил догадку — убрал сам guess: кнопка теперь **всегда видима** (не меняется относительно текущего поведения, значит не требует продуктового решения), а перекрытие снимается сдвигом area-плашки вдоль стены. То, что реально новое и видимо пользователю («площадь не перекрывает видимую кнопку»), явно оставлено в контракте; конкретный алгоритм сдвига явно вынесен в §18 п.4 как оспариваемое техническое предположение («допустима другая pure screen-space стратегия») | §4 п.6, §6.3 (новый абзац «Для collision check…»), §7 («Room settings buttons продолжают рендериться и не получают нового состояния»), §18 п.4, заключительный абзац §18 («…отсутствие overlap с видимой room settings button — это acceptance contract») |
|
||||
| **M2** — AC11 требовал относительный регресс-бюджет (10%/1ms), которого не считает `demo/benchmark_safe_resize_render.mjs` | AC11 переформулирован под реальный механизм инструмента — абсолютный потолок `RENDER_P95_MS = 25`; §10 теперь явно называет этот файл в списке участвующих файлов с пояснением «исходник менять не требуется»; §13 п.5 явно требует прогонять его перед код-ревью | AC11 (раздел 11), §10 («`demo/benchmark_safe_resize_render.mjs` — существующий real-render gate с абсолютным `RENDER_P95_MS = 25`»), §13 п.5 |
|
||||
|
||||
Оба закрытия проверены не на слово: `.rlgearbtn` и `RENDER_P95_MS = 25`
|
||||
сверены с реальным кодом (таблица выше). Оба Medium из r1 закрыты корректно.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium — M3: новый текст утверждает, что footprint `.rlgearbtn` «screen-fixed», хотя размер кнопки явно и намеренно зависит от zoom
|
||||
|
||||
**Файл:** `docs/specs/300-resize-measurement-layout.md`, §6.3 (новый абзац «Для
|
||||
collision check…») и §18 п.4 — обе фразы введены именно этой правкой (в r1
|
||||
их не было вовсе, тема отсутствовала, так как r1 просто прятал кнопку).
|
||||
|
||||
**В чём проблема.** Новый механизм collision-avoidance описан так:
|
||||
«консервативный **screen-space footprint** плашки, вычисленный из
|
||||
форматированного текста, font/padding tokens и **screen-fixed footprint**
|
||||
`.rlgearbtn`» (§6.3), и повторно в §18 п.4: «screen-fixed footprint room
|
||||
gear». В документе термин «screen-fixed» уже занят и используется
|
||||
последовательно в другом, точном смысле — «не меняется при zoom SVG»: то же
|
||||
§6.3 про leader-линию («Перевод screen px в render units использует текущий
|
||||
viewBox/stage size; stroke остаётся **screen-fixed**») и §6.2 про подсветку
|
||||
(`vector-effect="non-scaling-stroke"`, «одинаково читается при zoom»).
|
||||
|
||||
Реальный `.rlgearbtn` этому определению не соответствует.
|
||||
`--gear-h: calc(var(--icon-size, 2.5cqw) * 0.77)`, а `--icon-size`
|
||||
вычисляется `iconCqw()`, чей докстринг прямо говорит: «An icon is a
|
||||
percentage of the PLAN, not of the viewport: it scales with the plan as you
|
||||
zoom… (owner, 2026-08-03)» — то есть размер кнопки в CSS px на экране
|
||||
**растёт и уменьшается вместе с zoom плана**, а не остаётся константой. Это
|
||||
подтверждено ещё и соседним комментарием в `styles.ts`: «icon-size already
|
||||
rescales with the view… so the button zooms WITH the plan instead of keeping
|
||||
a constant screen size **(owner's spec)**» — то есть зависимость от zoom не
|
||||
случайность реализации, а явное продуктовое решение того же владельца.
|
||||
|
||||
Если реализация буквально возьмёт формулировку ТЗ («screen-fixed footprint
|
||||
`.rlgearbtn`») и один раз посчитает/захардкодит размер кнопки без учёта
|
||||
текущего zoom (что и подсказывает слово «screen-fixed» рядом с «не читает
|
||||
layout в pointermove»), консервативный footprint будет верным только на том
|
||||
zoom, для которого его посчитали, и разойдётся с реальным на любом другом —
|
||||
ровно то, от чего должен защищать сам механизм (AC7: «её фактический DOM
|
||||
rectangle не пересекает area-плашку»). Существующий риск-пункт 4 в §15
|
||||
(«Консервативный footprint разойдётся с фактическим CSS… smoke сравнивает
|
||||
реальные `getBoundingClientRect()`») называет только font/padding-погрешность,
|
||||
а не zoom-масштабирование, и план тестов (§13 п.2) не называет конкретный
|
||||
нестандартный zoom как fixture — то есть смок на дефолтном zoom формально
|
||||
пройдёт даже с неверной («screen-fixed») трактовкой формулы.
|
||||
|
||||
**Сценарий проявления.** Администратор дома открывает Resize на плане,
|
||||
предварительно отдалённом (zoomed out) от дефолтного масштаба — сценарий
|
||||
никак не запрещён и не редок при работе с большими планами. `.rlgearbtn`
|
||||
на экране меньше, чем при дефолтном zoom (или больше — при приближении).
|
||||
Реализация, посчитавшая «screen-fixed» footprint по дефолтному размеру,
|
||||
либо пропускает реальное пересечение (кнопка настроек оказывается частично
|
||||
под площадью — ровно проблема 3 из тела issue, которую задача должна решить),
|
||||
либо наоборot излишне отодвигает плашку туда, где реального пересечения нет.
|
||||
Оба исхода — нарушение AC7 на zoom, не покрытом smoke-фикстурой.
|
||||
|
||||
**Почему не High.** Правится на этом же этапе без пересмотра остального
|
||||
документа: либо явно указать, что footprint `.rlgearbtn` должен пересчитываться
|
||||
по той же zoom-зависимой формуле, что и `--icon-size`/`iconCqw()` (то есть
|
||||
заменить «screen-fixed» на «screen-space, пересчитываемый от текущего
|
||||
view.w» в двух местах — §6.3 и §18 п.4), либо явно добавить в §13 п.2 fixture
|
||||
с нестандартным zoom для доказательства, что консервативный footprint
|
||||
остаётся консервативным не только на дефолтном масштабе. Технический
|
||||
алгоритм и так помечен в §18 п.4 как свободно оспариваемое предположение —
|
||||
менять нужно только формулировку факта о самой кнопке, не продуктовый
|
||||
контракт AC7.
|
||||
|
||||
## Что проверено и признано корректным (в рамках дельты)
|
||||
|
||||
- M1 и M2 закрыты корректно и проверены по коду, не на слово (таблица выше).
|
||||
- Новое решение по M1 не создаёт нового недекларированного guess: сохранение
|
||||
видимости кнопки — это отсутствие изменения существующего поведения, а не
|
||||
новое решение, требующее подтверждения владельца; конкретный алгоритм сдвига
|
||||
корректно оформлен как оспариваемое предположение в §18 п.4, а сам факт
|
||||
«кнопка видима и не перекрыта» верно зафиксирован как acceptance contract.
|
||||
- AC7 переформулирован непротиворечиво: «unit + smoke» вместо старого
|
||||
«smoke», согласуется с тем, что появился чистый collision-helper, который
|
||||
можно проверить unit-тестом отдельно от production DOM-пути.
|
||||
- Переименование mutation guard `resize-labels-gear-during-drag` →
|
||||
`resize-labels-ignore-gear-collision` (§12) соответствует новому механизму
|
||||
(сдвиг вместо скрытия) и продолжает указывать на AC7.
|
||||
- §13 п.5 корректно добавляет прогон `demo/benchmark_safe_resize_render.mjs`
|
||||
как обязательное доказательство AC11 перед код-ревью — согласуется с новой
|
||||
формулировкой AC11.
|
||||
- §7 («Area projection использует тот же вычисленный visual centre, что
|
||||
`_renderRoomGear()`») — корректное техническое требование по *позиции*
|
||||
(совпадает с `poleOfInaccessibility(r.poly)` в `_renderRoomGear`); в отличие
|
||||
от заявления о *размере* (M3), про позицию текст не грешит против кода.
|
||||
- Риск-пункт 4 в §15 добавлен обоснованно (несовпадение расчётного и
|
||||
фактического CSS — реальный риск), но не покрывает конкретно
|
||||
zoom-масштабирование — см. M3.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Полные гейты (`typecheck`/`test`/`build`, `check-docs`, `smoke-select`,
|
||||
golden, invariants, backend, `python -m pytest`) — не запускались: диапазон
|
||||
`origin/dev..HEAD` содержит только markdown, класса A/B изменений нет,
|
||||
как и в r1. Эта проверка относится к будущему код-ревью того же issue.
|
||||
- Реализацию проекции (`src/resize-labels.ts` и т.д.) — она ещё не написана.
|
||||
- Прочие AC (AC1–AC5, AC8–AC10) и разделы §1, §2, §6.1, §6.2, §8, §9, §16,
|
||||
§17, §18 п.1–3,5 — не затронуты дельтой, повторно не проверялись, см.
|
||||
«Унаследовано из r1».
|
||||
- Численно, действительно ли `--icon-size`/`iconCqw()` в реализации даёт
|
||||
«консервативный» (заведомо не меньше фактического) footprint при любом
|
||||
zoom, если формула будет исправлена по M3 — это вопрос реализации, не
|
||||
наблюдаем на этапе ТЗ.
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Всё, что не затронуто дельтой между `ab9a83e7` и `2b7c98e6`, принято без
|
||||
повторной проверки на основании `docs/reviews/SPEC-REVIEW-300-r1.md`
|
||||
(SHA `ab9a83e755211a599ba8d73be0a3716696a5168b`), раздел «Что проверено и
|
||||
признано корректным»:
|
||||
|
||||
- обязательные разделы §7.1 присутствуют и не перепутаны местами; сценарий
|
||||
и «что человек увидит» — первые два раздела;
|
||||
- диагноз текущего кода (три длины, `poleOfInaccessibility` для площади,
|
||||
отсутствие отдельной подсветки измеряемого ребра) подтверждён чтением кода;
|
||||
- технический контракт §6 согласован с реальной формой `SafeResizePlan`
|
||||
(`roomIds`, `edgeByRoom` в `src/resize.ts`);
|
||||
- продуктовое решение по Q1 (narrow-room fallback — площадь всегда видна,
|
||||
выносится за контур с leader-линией) верно перенесено из решения владельца
|
||||
дословно;
|
||||
- граница с #233 (математика длины/площади) и #277 (eligibility/commit/Undo)
|
||||
проведена верно и не переоткрывает их;
|
||||
- модель данных/миграция/i18n корректно поданы как «без изменений»;
|
||||
- AC1–AC5, AC8–AC10 однозначны, у каждого указан способ доказательства;
|
||||
golden-сцена `safe-resize-handles-clamp-{light,dark}` действительно
|
||||
эмулирует активный preview (`safeResizePreview: true`);
|
||||
- откат описан верно (одна frontend-ревизия, без миграции);
|
||||
- смежные issue (#233/#277/#238/#52) корректно отнесены к «не входит».
|
||||
|
||||
## Вердикт
|
||||
|
||||
Одно новое Medium (M3) в скоупе задачи, устранимо без пересмотра остального
|
||||
документа — правкой формулировки в двух местах (§6.3, §18 п.4) либо
|
||||
добавлением non-default-zoom fixture в §13 п.2. High нет. Оба Medium
|
||||
предыдущего раунда (M1, M2) закрыты корректно и проверены по коду.
|
||||
|
||||
Вердикт: жёлтый · заход r2 · блокирующих циклов 2/4 · High: 0 · Medium: 1 → в задаче · Документ: docs/reviews/SPEC-REVIEW-300-r2.md
|
||||
Reference in New Issue
Block a user