mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-04 13:48:57 +00:00
@@ -0,0 +1,195 @@
|
||||
# SPEC-REVIEW-300-r3
|
||||
|
||||
- Issue: [#300](https://github.com/Matysh/houseplan-card/issues/300) — Подписи при ресайзе: подсвечивать измеряемые стены, убрать размер перетаскиваемой, площадь показывать по бокам от неё
|
||||
- Этап: spec (PROCESS.md §2.4)
|
||||
- ТЗ: `docs/specs/300-resize-measurement-layout.md`, коммит `4969f46e0e5d1e6440e53fd133f957c0103cf44e` на ветке `issue/300-resize-labels`
|
||||
- Заход: r3 · блокирующих циклов израсходовано (до этого раунда) 2/4
|
||||
- Ревьюер: Claude (роль «ревьюер ТЗ», не автор)
|
||||
|
||||
## Скоуп разбора
|
||||
|
||||
Раунд не первый → разбор по дельте (PROCESS.md §2.9, issue #214), не заново.
|
||||
|
||||
1. Вердикт r2 найден в комментарии issue #300 (2026-08-24T19:25:15Z): жёлтый,
|
||||
заход r2, блокирующих циклов 2/4, High 0 / Medium 1 (M3) → в задаче. SHA
|
||||
документа r2 — `docs/reviews/SPEC-REVIEW-300-r2.md`, шапка называет ТЗ-коммит
|
||||
`2b7c98e6bcc4a7fed920c05f2298a06a138683fd`.
|
||||
2. Автор ответил комментарием 2026-08-24T19:26:21Z: правка внесена и запушена в
|
||||
**`4969f46e0e5d1e6440e53fd133f957c0103cf44e`**, явно указана дельта для
|
||||
повторного ревью: `git diff 2b7c98e..4969f46 -- docs/specs/300-resize-measurement-layout.md`.
|
||||
3. Дельта проверена: `git diff 2b7c98e6..4969f46e -- docs/specs/300-resize-measurement-layout.md`
|
||||
— 12 добавленных / 9 удалённых строк, ограничена тремя местами одного файла:
|
||||
§6.3 (абзац про collision check), §13 п.2 (добавлен zoom-fixture в план
|
||||
смоков) и §18 п.4 (то же техническое предположение). `git diff 2b7c98e6..4969f46e --stat`
|
||||
дополнительно показывает только `docs/reviews/SPEC-REVIEW-300-r2.md`
|
||||
(публикация предыдущего ревью) — не продмет разбора.
|
||||
4. Дельта локальна: один файл, один смысловой узел (формулировка footprint
|
||||
`.rlgearbtn` и его тестовое покрытие) — не ребейз на ушедший вперёд `dev`
|
||||
(`origin/dev...HEAD` по-прежнему только `docs/**`, см. ниже), не смена
|
||||
контракта поведения, не задета новая подсистема. Полный повторный разбор не
|
||||
требуется по формальному признаку; однако закрытие M3 проверялось не только
|
||||
по двум местам, названным в r2, а по всем вхождениям того же термина в
|
||||
документе — см. «Находки»: это и обнаружило неполноту закрытия.
|
||||
5. Гейты кода неприменимы, как и в r1/r2: `git diff origin/dev...HEAD --name-only`
|
||||
даёт только `docs/reviews/SPEC-REVIEW-300-r1.md`, `docs/reviews/SPEC-REVIEW-300-r2.md`,
|
||||
`docs/specs/300-resize-measurement-layout.md`, `docs/specs/README.md` — класс
|
||||
изменений C, `src/**`/`test/**` не тронуты.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Проверено чтением (не исполнением).
|
||||
|
||||
| Утверждение правки | Где проверено |
|
||||
|---|---|
|
||||
| `.rlgearbtn` действительно наследует `--icon-size` от родительского `.devlayer`, а не имеет собственного независимого от zoom размера | `src/houseplan-card.ts:17398` — `<div class="devlayer" style="--icon-size:${iconCqw(iconPct, space, view.w, ...)}cqw...">`; `_renderRoomGear` (`src/houseplan-card.ts:18688-18711`) рендерит `<button class="rlgearbtn">` **внутри** этого `.devlayer` (вызов на `houseplan-card.ts:18705`, разметка `.devlayer` открывается на 17398 и рендерит `_renderRoomGear` на 17406) |
|
||||
| `--icon-size`, наследуемый `.rlgearbtn` через `--gear-h: calc(var(--icon-size, 2.5cqw) * 0.77)`, вычислен именно от `view.w`, а не от какой-то другой опорной ширины | `src/houseplan-card.ts:17398` — первый аргумент `iconCqw(iconPct, space, **view.w**, kiosk)` для `--icon-size` (в отличие от соседнего `--rl-icon-size`, который намеренно использует `this._roomLabelReferenceViewWidth(view)` — другую опорную ширину для шрифта названия комнаты, не для кнопки) |
|
||||
| Формулировка «zoom-dependent footprint `.rlgearbtn`, вычисленный для текущего `view.w` по той же `iconCqw()`-семантике» (новый текст §6.3/§18.4) технически точна | Подтверждено двумя пунктами выше — новая формулировка исправлена корректно, в отличие от прежней «screen-fixed» |
|
||||
| Полный перечень вхождений термина, из-за которого возникла M3, в текущей редакции документа | `grep -n "screen-fixed\|no-fly" docs/specs/300-resize-measurement-layout.md` — 3 совпадения на «screen-fixed» (строки 71, 197, 394) и 1 на «no-fly» (строка 71) |
|
||||
| Диапазон `origin/dev...HEAD` не содержит `src/**`/`test/**` | `git diff origin/dev...HEAD --name-only` |
|
||||
|
||||
## Закрытие раунда r2
|
||||
|
||||
| Находка r2 | Чем закрыта | Где это видно |
|
||||
|---|---|---|
|
||||
| **M3** — §6.3 и §18 п.4 называли footprint `.rlgearbtn` «screen-fixed», хотя размер кнопки зависит от zoom (`iconCqw()`) | В обоих названных местах формулировка заменена на «zoom-dependent footprint `.rlgearbtn`, вычисленный для текущего `view.w` по той же `iconCqw()`-семантике»; дополнительно в план смоков (§13 п.2) добавлен fixture «default и non-default zoom» с проверкой фактического `getBoundingClientRect()`. Оба изменения проверены против кода (таблица выше): `.rlgearbtn` действительно наследует `--icon-size`, вычисленный от `view.w` через `iconCqw()`, то есть новая формулировка точна | `docs/specs/300-resize-measurement-layout.md:182-187` (§6.3), `:389-393` (§18 п.4), `:312-313` (§13 п.2); код — `src/houseplan-card.ts:17398`, `:18688-18711`, `src/styles.ts:996-1001` |
|
||||
|
||||
Закрытие в двух названных местах — корректное и проверенное по коду, не на
|
||||
слово. Но то же самое неверное представление о кнопке («её footprint не
|
||||
зависит от zoom») осталось нетронутым в третьем месте того же документа,
|
||||
которое ни r2, ни правка автора не назвали — см. находку ниже. Поэтому M3
|
||||
закрыта частично: сам механизм и тестовый план исправлены верно, но
|
||||
«Зафиксированные продуктовые решения» (§4) документу самому себе противоречат.
|
||||
|
||||
## Находки
|
||||
|
||||
### Medium — M4: §4 п.6 («Зафиксированные продуктовые решения») по-прежнему называет footprint кнопки «screen-fixed no-fly zone», противореча только что исправленным §6.3/§18 п.4
|
||||
|
||||
**Файл:** `docs/specs/300-resize-measurement-layout.md:70-72`.
|
||||
|
||||
**В чём проблема.** Формулировка введена ещё в правке r1→r2 (M1-фикс,
|
||||
`git diff ab9a83e7..2b7c98e6`, тот же коммит, где появились и обе фразы,
|
||||
которые r2 позже поймал как M3) и с тех пор не менялась:
|
||||
|
||||
> 6. Кнопка настроек комнаты остаётся видимой. Если nominal area-плашка
|
||||
> попадает в её **screen-fixed no-fly zone**, плашка сдвигается вдоль
|
||||
> перемещаемой стены до первого свободного положения; leader сохраняет связь
|
||||
> с исходным midpoint.
|
||||
|
||||
Это тот же самый факт о той же самой кнопке, что и M3 (footprint
|
||||
`.rlgearbtn`, используемый для collision-avoidance), выраженный другими
|
||||
словами — «no-fly zone» вместо «footprint». Ревью r2 сформулировало проблему
|
||||
абстрактно правильно («термин screen-fixed уже занят и означает "не меняется
|
||||
при zoom"»), но искало и нашло только два конкретных вхождения (§6.3, §18
|
||||
п.4); §4 п.6 использует не слово «screen-fixed footprint», а «screen-fixed
|
||||
no-fly zone» — тот же по сути неверный факт, но не совпавший ни с одним из
|
||||
названных мест, поэтому фикс автора (правка именно двух названных мест) его
|
||||
не затронул. `grep -n "screen-fixed\|no-fly" docs/specs/300-resize-measurement-layout.md`
|
||||
подтверждает: строка 71 — единственное оставшееся вхождение с этим смыслом,
|
||||
строки 197 и 394 относятся к leader/highlight strokes и используют
|
||||
«screen-fixed» корректно (там это действительно так: non-scaling-stroke).
|
||||
|
||||
Раздел 4 в этом документе — не техническое обсуждение, а «Зафиксированные
|
||||
продуктовые решения»: раздел, который реализация обязана читать как источник
|
||||
истины наравне с acceptance criteria. Он прямо утверждает то, что r2 уже
|
||||
опроверг чтением кода: `--gear-h` наследует `--icon-size`, вычисленный
|
||||
`iconCqw(iconPct, space, view.w, kiosk)` — то есть растёт и уменьшается вместе
|
||||
с zoom плана (owner's spec, `src/space-geometry.ts:463-489`,
|
||||
`src/styles.ts:997-998`). Реализация, которая по недосмотру откроет §4 п.6, а
|
||||
не §6.3, и буквально прочитает «screen-fixed no-fly zone», один раз
|
||||
посчитает/захардкодит размер зоны без учёта `view.w` — то же нарушение AC7 на
|
||||
нестандартном zoom, которое M3 уже описывало для двух других мест документа.
|
||||
|
||||
**Сценарий проявления.** Идентичен M3: администратор дома открывает Resize на
|
||||
плане, отдалённом (или приближённом) от дефолтного zoom; кнопка настроек на
|
||||
экране меньше или больше, чем при дефолтном масштабе. Реализация, взявшая
|
||||
за основу §4 (первый раздел с продуктовым решением по этому вопросу, а не
|
||||
§6.3 — технический контракт дальше по документу), не пересчитывает no-fly
|
||||
zone от текущего `view.w` и либо пропускает реальное пересечение (площадь
|
||||
частично под кнопкой — проблема 3 из тела issue), либо излишне сдвигает
|
||||
плашку без реальной причины.
|
||||
|
||||
**Почему не High.** Правится без пересмотра остального документа: одна
|
||||
строка, замена «screen-fixed no-fly zone» на формулировку, согласованную с
|
||||
уже исправленными §6.3/§18 п.4 (например «zoom-dependent no-fly zone,
|
||||
пересчитываемую от текущего `view.w`»). Технический механизм не меняется,
|
||||
новых продуктовых вопросов не возникает — это тот же самый M3, просто в
|
||||
третьей копии текста, которую предыдущий раунд не нашёл.
|
||||
|
||||
## Что проверено и признано корректным (в рамках дельты)
|
||||
|
||||
- Обе формулировки, названные в M3 (§6.3, §18 п.4), исправлены корректно и
|
||||
проверены по коду: `.rlgearbtn` наследует `--icon-size` от `.devlayer`,
|
||||
вычисленный `iconCqw(iconPct, space, view.w, kiosk)` — то есть именно от
|
||||
`view.w`, а не от независимой опорной ширины (в отличие от `--rl-icon-size`,
|
||||
который намеренно использует другую опорную ширину для шрифта названия
|
||||
комнаты — не спутаны).
|
||||
- §13 п.2 корректно добавляет fixture «default и non-default zoom» с
|
||||
фактической проверкой `getBoundingClientRect()` — согласуется с новой
|
||||
формулировкой и закрывает часть риска M3 независимо от текста (тест поймает
|
||||
неверную реализацию даже при нечитаемой документации).
|
||||
- Термин «screen-fixed» в оставшихся двух местах документа (leader stroke,
|
||||
§6.3 второй абзац и §18 п.5, highlight strokes) использован корректно —
|
||||
там `vector-effect="non-scaling-stroke"` действительно даёт независимый от
|
||||
zoom экранный размер, путаницы между «не масштабируется при zoom» (leader,
|
||||
highlight) и «зависит от zoom плана» (кнопка) в этих местах нет.
|
||||
- Продуктовый контракт и §18 (принятые технические предположения) в остальном
|
||||
не менялись этой правкой; новых недекларированных guess не добавлено.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Полные гейты (`typecheck`/`test`/`build`, `check-docs`, `smoke-select`,
|
||||
golden, invariants, backend, `python -m pytest`) — не запускались: диапазон
|
||||
`origin/dev...HEAD` содержит только `docs/**`, класса A/B изменений нет, как
|
||||
и в r1/r2. Относится к будущему код-ревью того же issue.
|
||||
- Реализацию проекции (`src/resize-labels.ts` и т.д.) — она ещё не написана.
|
||||
- Прочие AC (AC1–AC10 кроме затронутых M3/M4 частей AC7) и разделы §1, §2,
|
||||
§3, §5, §6.1, §6.2, §7 (кроме упомянутого выше), §8, §9, §10, §11 (кроме
|
||||
AC7/AC11), §12, §14, §15, §16, §17, §18 п.1–3,5 — не затронуты дельтой r2→r3
|
||||
и не переоткрывались; наследуются из r2 (которая сама унаследовала их из
|
||||
r1) — см. ниже.
|
||||
- Влияние M4 на численную точность collision-avoidance для конкретных значений
|
||||
zoom (`iconCqw()` даёт действительно консервативную оценку на любом `view.w`)
|
||||
— вопрос реализации, не наблюдаем на этапе ТЗ; тот же вывод, что и r2 сделала
|
||||
для M3.
|
||||
|
||||
## Унаследовано из r2 (и, транзитивно, из r1)
|
||||
|
||||
Всё, что не затронуто дельтой между `2b7c98e6` и `4969f46e` и не относится к
|
||||
находке M4, принято без повторной проверки на основании
|
||||
`docs/reviews/SPEC-REVIEW-300-r2.md` (SHA `2b7c98e6bcc4a7fed920c05f2298a06a138683fd`),
|
||||
разделы «Закрытие раунда r1» и «Что проверено и признано корректным»:
|
||||
|
||||
- обязательные разделы §7.1 присутствуют и не перепутаны местами;
|
||||
- диагноз текущего кода (три длины, `poleOfInaccessibility` для площади,
|
||||
отсутствие отдельной подсветки измеряемого ребра) подтверждён чтением кода;
|
||||
- технический контракт §6 согласован с реальной формой `SafeResizePlan`
|
||||
(`roomIds`, `edgeByRoom` в `src/resize.ts`);
|
||||
- продуктовое решение по Q1 (narrow-room fallback — площадь всегда видна,
|
||||
выносится за контур с leader-линией) верно перенесено из решения владельца
|
||||
дословно;
|
||||
- M1 (недекларированное скрытие `.roomgear`) закрыта корректно: кнопка
|
||||
`.rlgearbtn` всегда видима, конкретный алгоритм сдвига оформлен как
|
||||
оспариваемое техническое предположение (§18 п.4), сам факт «кнопка видима и
|
||||
не перекрыта» — как acceptance contract;
|
||||
- M2 (AC11 требовал несуществующий относительный regression-бюджет) закрыта:
|
||||
AC11 переформулирован под реальный абсолютный `RENDER_P95_MS = 25`;
|
||||
- граница с #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 (M4) в скоупе задачи — прямое продолжение уже пойманной в
|
||||
r2 находки M3: та же неверная формулировка («screen-fixed» footprint кнопки
|
||||
настроек комнаты вместо zoom-dependent) осталась в третьем месте документа
|
||||
(§4 п.6), которое ни r2, ни правка автора не затронули. Устранимо без
|
||||
пересмотра остального документа — одной строкой, по образцу уже сделанного
|
||||
исправления в §6.3/§18 п.4. High нет. Оба места, названные в M3, закрыты
|
||||
корректно и проверены по коду.
|
||||
|
||||
Вердикт: жёлтый · заход r3 · блокирующих циклов 3/4 · High: 0 · Medium: 1 → в задаче · Документ: docs/reviews/SPEC-REVIEW-300-r3.md
|
||||
Reference in New Issue
Block a user