mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,196 @@
|
||||
# SPEC-REVIEW-300-r1
|
||||
|
||||
- Issue: [#300](https://github.com/Matysh/houseplan-card/issues/300) — Подписи при ресайзе: подсвечивать измеряемые стены, убрать размер перетаскиваемой, площадь показывать по бокам от неё
|
||||
- Этап: spec (PROCESS.md §2.4)
|
||||
- ТЗ: `docs/specs/300-resize-measurement-layout.md`, коммит `ab9a83e755211a599ba8d73be0a3716696a5168b` на ветке `issue/300-resize-labels`
|
||||
- Заход: r1 · блокирующих циклов израсходовано (до этого раунда) 0/4
|
||||
- Трек: обычный (аналитика прямо называет `трек: обычный`; меток `small`/`trivial` на issue нет)
|
||||
- Ревьюер: Claude (роль «ревьюер ТЗ», не автор)
|
||||
|
||||
## Скоуп разбора
|
||||
|
||||
Первый раунд — разбор полный, по PROCESS.md §2.10 сокращение объёма не
|
||||
применяется. Прочитаны в порядке из инструкции: `docs/SCOPE.md`, `AGENTS.md`,
|
||||
`PROCESS.md`, тело issue #300 и все 5 комментариев (аналитика, занятие, вопрос
|
||||
Q1, решение владельца по Q1, публикация ТЗ), `docs/USER-GUIDE.ru.md` (раздел
|
||||
Resize), канонические `docs/RESIZE.md`, `docs/UX-MODES.md`, а также связанные
|
||||
специи `docs/specs/233-resize-inner-dimensions.md` и
|
||||
`docs/specs/277-safe-resize.md`.
|
||||
|
||||
Диагноз ТЗ по коду проверен построчно, а не принят на слово: прочитан
|
||||
`_rszEdgeLabels()`, `_rszMove()`, `_rszEdgeDown()`, `_renderRoomGear()` и тип
|
||||
`SafeResizePlan` в `src/houseplan-card.ts` / `src/resize.ts`, а также
|
||||
существующие golden-сцены `safe-resize-handles-clamp-{light,dark}` в
|
||||
`demo/golden/matrix.mjs` и бенчмарк `demo/benchmark_safe_resize_render.mjs`.
|
||||
|
||||
Ветка на момент ревью содержит ровно один коммит поверх `origin/dev`
|
||||
(`git diff origin/dev..HEAD` = только новый файл ТЗ и правка
|
||||
`docs/specs/README.md`) — ребейз не требовался, `dev` не ушёл вперёд.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Гейты кода в этом раунде неприменимы: класс изменений — только C
|
||||
(документация), продуктовый код не тронут. Прогон `typecheck`/`test`/`build`
|
||||
не требуется — ветка не содержит правок `src/**`/`test/**`. Единственная
|
||||
проверка этапа — соответствие ТЗ формату §7.1 и фактическому состоянию кода,
|
||||
на которое ссылается диагноз.
|
||||
|
||||
Проверено чтением (не исполнением):
|
||||
|
||||
| Утверждение ТЗ | Где проверено |
|
||||
|---|---|
|
||||
| `_rszEdgeLabels()` кладёт три длины: previous/moving/next | `src/houseplan-card.ts:8739-8784`, цикл `for (const edge of [(i-1+n)%n, i, j])` |
|
||||
| Площадь ставится в `poleOfInaccessibility(floor)` | `src/houseplan-card.ts:8776` |
|
||||
| `.roomgear`-кнопка тоже стоит в `poleOfInaccessibility(r.poly)` — заявленное перекрытие реально | `src/houseplan-card.ts:18688-18711` (`_renderRoomGear`) |
|
||||
| `SafeResizePlan` содержит `roomIds`/`edgeByRoom` | `src/resize.ts:65-72` |
|
||||
| Существующий 12px-сдвиг у подписи размера проёма (принятое предположение §18.2) | `src/styles.ts:1227-1233`, `.opdimension` использует `-12px` |
|
||||
| golden-сцена `safe-resize-handles-clamp-{light,dark}` уже симулирует активный preview (`safeResizePreview: true`) | `demo/golden/matrix.mjs:246-250` |
|
||||
| `demo/benchmark_safe_resize_render.mjs` меряет только абсолютный потолок `RENDER_P95_MS=25`, без сравнения с историческим baseline | `demo/benchmark_safe_resize_render.mjs:8-9,58-64` |
|
||||
| Ни один существующий путь рендера не скрывает `.roomgear` или другую интерактивную кнопку во время активного жеста (drag/draw/move) | `grep` по `_roomDrag`/`_moveDrag`/`_drawDrag`/`hide.*drag` в `src/houseplan-card.ts` — 0 совпадений |
|
||||
| `docs/UX-MODES.md` и `docs/USER-GUIDE.ru.md` не описывают исчезновение кнопки настроек комнаты во время Resize | текстовый поиск по обоим файлам |
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium — M1: временное скрытие кнопки настроек комнаты — недекларированная догадка, выданная за принятое решение
|
||||
|
||||
**Файл:** `docs/specs/300-resize-measurement-layout.md`, раздел 4 п.6 и раздел 18
|
||||
(последний абзац).
|
||||
|
||||
**В чём проблема.** П.6 раздела 4 «Зафиксированные продуктовые решения»
|
||||
гласит: «Во время активного Resize кнопки настроек комнат временно скрыты».
|
||||
Раздел 18 явно исключает этот пункт из списка предположений: «Не являются
|
||||
предположениями: … и **временное отсутствие room gear** — это acceptance
|
||||
contract». То есть автор фиксирует новое видимое поведение — интерактивная
|
||||
кнопка пропадает с экрана во время жеста — как решённый факт, не как
|
||||
предположение и не как продуктовый вопрос владельцу.
|
||||
|
||||
Между тем:
|
||||
|
||||
- в теле issue и во всех пяти комментариях (включая единственный заданный
|
||||
владельцу вопрос Q1 и его решение) кнопка настроек комнаты не упоминается
|
||||
вовсе — только требование «плашка площади не пересекается с кнопкой» (AC4
|
||||
issue, AC7 ТЗ);
|
||||
- ни `docs/RESIZE.md`, ни `docs/UX-MODES.md`, ни `docs/USER-GUIDE.ru.md` не
|
||||
фиксируют исчезновение `.roomgear` во время какого-либо жеста;
|
||||
- в коде нет прецедента: ни один существующий drag (move комнаты, draw стены,
|
||||
furniture-drag) не скрывает интерактивные кнопки на время жеста — это будет
|
||||
первый такой случай.
|
||||
|
||||
Согласие/несогласие «не пересекается» можно было бы решить и иначе (например,
|
||||
подвинуть саму плашку так, чтобы она физически не доставала до центра комнаты
|
||||
в обычных пропорциях, либо явно спросить владельца форматом Q1 — «скрывать
|
||||
кнопку на время жеста или…», с default). Автор выбрал одно конкретное решение
|
||||
и записал его как неоспоримый контракт, а не как оспариваемое предположение
|
||||
(§7.1 ТЗ прямо предусматривает для этого блок «принято предположительно,
|
||||
поменять свободно» — сюда это решение не попало).
|
||||
|
||||
Это ровно тот класс дефекта, о котором прямо предупреждает PROCESS.md §7.1:
|
||||
«Догадка, записанная как факт, — худший вид дефекта: она проходит ревью, потому
|
||||
что выглядит решением» — здесь она к тому же явно помечена как «не
|
||||
предположение», то есть застрахована от последующего оспаривания на код-ревью.
|
||||
|
||||
**Сценарий проявления.** Администратор дома тянет стену комнаты; в этот момент
|
||||
кнопка «⚙ Настройки» соседней (или той же) комнаты пропадает с экрана без
|
||||
предупреждения и без документированного контракта — если владелец на самом
|
||||
деле ожидал что-то другое (например, кнопку, отодвинутую в сторону, а не
|
||||
скрытую), это выяснится только после того, как код и AC7/mutation-guard
|
||||
`resize-labels-gear-during-drag` уже реализованы вокруг скрытия.
|
||||
|
||||
**Почему не High.** Правится на этом же этапе без переписывания остального
|
||||
ТЗ: либо явное подтверждение владельца батч-вопросом с default (по образцу
|
||||
Q1), либо перенос пункта в раздел 18 как явно оспариваемое техническое
|
||||
предположение с обоснованием, почему скрытие — единственный практичный вариант
|
||||
и почему это не поменяет продуктовый контракт.
|
||||
|
||||
### Medium — M2: AC11 требует относительный регресс-бюджет, которого не существует у названного инструмента
|
||||
|
||||
**Файл:** `docs/specs/300-resize-measurement-layout.md`, раздел 11 (AC11) и
|
||||
раздел 10 (список файлов).
|
||||
|
||||
**В чём проблема.** AC11 требует: «`benchmark_safe_resize_render` не
|
||||
регрессирует больше чем на 10% либо 1 ms p95 (берётся больший допуск)»,
|
||||
доказательство — «benchmark + code review». Прочитанный
|
||||
`demo/benchmark_safe_resize_render.mjs` не считает такую величину: он меряет
|
||||
20 warm-сэмплов и сравнивает p95 с единственным **абсолютным** потолком
|
||||
`RENDER_P95_MS = 25` (`Object.assign`-объект `budgets: { renderP95Ms:
|
||||
RENDER_P95_MS, … }`). Никакого сохранённого исторического baseline или
|
||||
same-run сравнения «до/после» в этом файле нет — в отличие от соседнего
|
||||
`demo/benchmark_safe_resize.mjs`, который действительно считает `baseline`
|
||||
и `relativeLimit` для *другого* сценария (курсор pointer, не render layer).
|
||||
|
||||
Раздел 10 «Изменяемые файлы и модули» при этом не называет
|
||||
`demo/benchmark_safe_resize_render.mjs` в списке ожидаемых правок — то есть
|
||||
ТЗ не проговаривает, что этот файл придётся переписывать, чтобы у AC11 вообще
|
||||
появился механизм сравнения «регресс не больше 10%». Как AC сформулирован
|
||||
сейчас, его не с чем сверить: либо метрика придумана без учёта реального
|
||||
инструмента, либо реализация должна тихо добавить в бенчмарк baseline-логику,
|
||||
которую ТЗ не анонсирует.
|
||||
|
||||
**Сценарий проявления.** На код-ревью разработчик и ревьюер по-разному прочитают
|
||||
AC11: один добавит baseline-сравнение в бенчмарк (незапланированная работа вне
|
||||
раздела 10), другой просто проверит, что p95 остаётся в старых 25 ms, и
|
||||
формально АC11 «не регрессирует более чем на 10%/1мс» не проверен никем,
|
||||
потому что делать не с чем сравнивать.
|
||||
|
||||
**Почему не High.** Легко устраняется формулировкой на этом же этапе: либо
|
||||
«остаётся в пределах существующего абсолютного бюджета 25 ms p95»
|
||||
(соответствует реальному инструменту), либо явно добавить
|
||||
`demo/benchmark_safe_resize_render.mjs` в раздел 10 с описанием, что baseline
|
||||
записывается тем же способом, что в `benchmark_safe_resize.mjs`.
|
||||
|
||||
## Что проверено и признано корректным
|
||||
|
||||
- Обязательные разделы §7.1 (сценарий, что видит человек, проблема, скоуп/не-
|
||||
скоуп, контракт, UX, модель данных/миграция/i18n, AC с доказательством, план
|
||||
автотестов, риски, откат, release-артефакты) присутствуют и не перепутаны
|
||||
местами; сценарий и «что человек увидит» — первые два раздела, как требует
|
||||
процесс.
|
||||
- Диагноз текущего кода (три длины, `poleOfInaccessibility` для площади и
|
||||
gear, отсутствие отдельной подсветки измеряемого ребра) подтверждён чтением
|
||||
кода — не догадка.
|
||||
- Технический контракт §6 согласован с реальной формой `SafeResizePlan`
|
||||
(`roomIds`, `edgeByRoom` существуют в `src/resize.ts`).
|
||||
- Продуктовое решение по Q1 (narrow-room fallback — площадь всегда видна,
|
||||
выносится за контур с leader-линией) верно перенесено из решения владельца
|
||||
дословно, без искажения; помечено, что заменяет исходный AC6 issue, с
|
||||
ссылкой на комментарий.
|
||||
- Не входит математика длины/площади (#233) и eligibility/commit/Undo (#277) —
|
||||
граница со смежными контрактами проведена верно и не переоткрывает их.
|
||||
- Модель данных/миграция/i18n корректно поданы как «без изменений»: новых
|
||||
config-полей, WebSocket-вызовов и i18n-ключей нет, что соответствует объёму
|
||||
задачи (только layout существующего оверлея).
|
||||
- AC1–AC10 однозначны, у каждого указан способ доказательства (unit/smoke/
|
||||
golden/code review), и golden-сцена `safe-resize-handles-clamp-{light,dark}`
|
||||
действительно уже эмулирует активный preview (`safeResizePreview: true`),
|
||||
так что план «переиспользовать существующую сцену» для AC9 реалистичен.
|
||||
- Раздел 18 «Принятые предположения» корректно оформлен как оспариваемый и
|
||||
содержит только действительно техническую разметку (расположение файла,
|
||||
величина 12px, порядок отрисовки leader) — кроме пункта, вынесенного в M1,
|
||||
который туда не попал, хотя по характеру должен был.
|
||||
- Откат описан верно (одна frontend-ревизия, без миграции).
|
||||
- Не входит в скоуп задачи и не относится к #300 никаких признаков смешения
|
||||
с чужими issue — упомянутые #233/#277/#238/#52 корректно отнесены к «не
|
||||
входит».
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Полные гейты (`typecheck`/`test`/`build`, `check-docs`, `smoke-select`,
|
||||
golden, invariants, backend) — не запускались, так как класса A/B изменений
|
||||
в ветке нет: диапазон `origin/dev..HEAD` содержит только markdown. Проверка
|
||||
гейтов на этом этапе относится к будущему код-ревью того же issue.
|
||||
- Реализацию проекции (`src/resize-labels.ts` и т.д.) — она ещё не написана,
|
||||
это предмет `S6-in-progress`.
|
||||
- Численную величину 25ms/10%/1ms как перф-бюджет по существу (сколько
|
||||
реально стоит рендер двух highlight-полосок и двух area-плашек) — на этом
|
||||
этапе это не наблюдаемо, будет видно в реализации; отмечен только
|
||||
методологический разрыв AC11 (M2).
|
||||
- Доступность (`aria-hidden`, focus-order) заявленного measurement-layer — не
|
||||
верифицируема без DOM; раздел 8 её обещает, содержательных противоречий не
|
||||
найдено при чтении.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Оба blocking-замечания в скоупе задачи и устранимы без пересмотра остального
|
||||
документа. High нет.
|
||||
|
||||
Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 2 → в задаче · Документ: docs/reviews/SPEC-REVIEW-300-r1.md
|
||||
Reference in New Issue
Block a user