Files
houseplan-card/docs/reviews/SPEC-REVIEW-300-r3.md
2026-08-24 19:33:00 +00:00

20 KiB
Raw Permalink Blame History

SPEC-REVIEW-300-r3

  • Issue: #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) и с тех пор не менялась:

  1. Кнопка настроек комнаты остаётся видимой. Если 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