Files
houseplan-card/docs/reviews/SPEC-REVIEW-303-r1.md
2026-08-25 08:10:51 +00:00

16 KiB
Raw Permalink Blame History

SPEC-REVIEW-303-r1

Issue: #303 — «Подсветка «Толщины стены» шире реальной стены: пол минимума задан в юнитах сетки, а не в сантиметрах». Этап: ТЗ на ревью (PROCESS.md §2.4). Трек: small — ТЗ живёт в теле issue (комментарий автора от 2026-08-25T07:48:43Z), файл в docs/specs/ не создаётся и не должен создаваться (подтверждено: docs/specs/ не содержит 303-*). Заход: r1 · блокирующих циклов израсходовано 0/2 (лимит лёгкого трека — 2, §5).

Скоуп

Инструмент «Толщина» плана-редактора (_wallThickHover / houseplan-card.ts:11741) рисует полосу подсветки шире, чем тело стены, когда cell_cm велико: минимум ширины полосы задан в юнитах сетки (this._gridPitch * 1.25), а не в физических сантиметрах — тот же класс дефекта, что штриховка в #230. ТЗ вводит чистую функцию wallThickHoverHalfUnits(cm, cellCm, gridPitch), убирает пол для стен с реальной толщиной (cm > 0) и переносит видимый минимум для стен без толщины (cm = 0) на существующую grid-visual конвенцию проекта (src/grid-scale.ts). Зона попадания курсора (_wallThickHit, pull = gridPitch*6) и стили полосы не меняются. Один поверхностный класс A-файл (houseplan-card.ts) плюс новая функция рядом с gridVisualUnits. Персона — Home admin, поверхность — Plan editor (десктоп; touch помечен «supported», зона попадания не трогается). Продуктовая рамка: правка не расширяет функциональность инструмента «Толщина» (уже закрывает часть J4/J6 по docs/SCOPE.md), а устраняет визуальное несоответствие подсветки телу стены — конфликта со SCOPE нет.

Как проверялось

Ревью — построчная проверка каждого утверждения ТЗ против фактического кода в dev (SHA 4b8f17ba, тот же, что указан автором в issue), без доверия формулировкам на слово:

  1. Прочитаны docs/SCOPE.md, AGENTS.md, PROCESS.md (§2.4, §2.10, §5, §7.1, §7.2, §4) — заход первый, дельта-режим §2.10 не применяется.
  2. Прочитан docs/WALL-THICKNESS.md (канонический документ подсистемы) — §6 «Tool / hooks / i18n» не описывает ширину полосы hover, противоречий нет.
  3. Прочитан docs/USER-GUIDE.ru.md (§«Толщина стены», строки 515–527) — ширина подсветки там не документирована; заявление автора «USER-GUIDE не требуется» подтверждено чтением, а не принято на слово.
  4. Прочитан код: _wallThickHover/_wallThickHit (src/houseplan-card.ts:11717-11759), wallCmToUnits/clampWallCm (src/wall-thickness.ts:180-207), gridVisualScale/gridVisualUnits (src/grid-scale.ts), GRID_PITCH = NORM_W/GRID_N = 1000/240 (src/space-geometry.ts:201-203), CSS .wallthick-hover (src/styles.ts:1544-1551).
  5. Пересчитана вручную арифметика каждого числового AC с реальным GRID_PITCH = 4.1(6), чтобы исключить «догадку, выданную за факт»:
    • AC3 (cell_cm:5, cm=3): новое depth = wallCmToUnits(3,5,4.1667) = 2.5 — совпадает с заявленным «2.5 юнита». Старое поведение: half=max(1.25, 1.25·4.1667=5.208)=5.208, полоса 2×5.208=10.417 — совпадает с «≈10.42» из описания issue.
    • AC4 (cell_cm:5, cm=0): новое half = gridVisualUnits(1.5·4.1667, 5) = 6.25 (gridVisualScale(5)=1), полоса 12.5. Старое: depth=3·4.1667= 12.5, half=max(6.25, 5.208)=6.25, полоса 12.5 — байт в байт совпадает, заявление автора подтверждено, а не принято на веру.
    • AC4 (cell_cm:30): новое half = 1.5·4.1667·(5/30) ≈ 1.0417, полоса ≈2.083 — константный физический эквивалент 7.5 см на половину (15 см на полную полосу) независимо от cell_cm, ровно то, что предотвращает возврат исходного дефекта для случая cm=0.
    • AC1/AC2 пересчитаны аналогично — оба воспроизводят числа, приведённые автором в описании и ТЗ.
  6. Проверены фактом, а не на слово, три опорные ссылки автора:
    • gridVisualScale/--hp-cell-visual-scale действительно уже используются штриховкой/обводками (src/styles.ts:640-1594, src/space-render.ts:511, src/houseplan-card.ts:17270) — конвенция реальна, не изобретена под эту задачу.
    • Обводка .wallthick-hover уже завязана на --hp-cell-visual-scale (src/styles.ts:1549) — совпадает с пунктом 4 контракта («стили полосы … не меняются»).
    • wallthick-hover действительно отсутствует в demo/golden/matrix.mjs (grep не дал совпадений) — заявление «golden не затрагивается» подтверждено.
  7. Проверены циклический импорт и файловая раскладка: grid-scale.ts сейчас не имеет импортов, wall-thickness.ts не импортирует grid-scale.ts — перенос wallCmToUnits в новую функцию рядом с gridVisualUnits не создаёт цикла (пусть это и техническое, а не продуктовое решение).
  8. Проверено соответствие критериям лёгкого трека (§5): одна поверхность, нет миграции конфига/compatibility-полей, нет нового i18n, зона touch не трогается — все пункты выполняются одновременно, small не оспаривается.
  9. Проверено самосогласование каждой находки предыдущего ревью не требуется — это первый заход (r1), раздел «Унаследовано из r0» неприменим.

Дельта-режим §2.10 не применяется (заход r1). Тяжёлые гейты (golden/смоки/perf) на этапе ТЗ не прогонялись — они не относятся к ревью ТЗ (§2.7 «код-ревью…», не §2.4); assertion о golden проверена чтением конфигурации сцен, а не исполнением гейта.

Находки

Пусто. High — 0, Medium в скоупе — 0, Medium вне скоупа — 0, Low — 0 блокирующих (см. ниже два замечания, снятые ревьюером без правки).

Два места стоило отметить, но ни одно не тянет даже на Low-с-правкой:

  1. План теста AC5 проверяет попадание на удалении ~gridPitch*5, а не gridPitch*6. Дословно AC5 утверждает «зона не сузилась … на прежнем удалении gridPitch*6», а описанный смок-тест бьёт на ~5, что доказывает лишь «зона не уже 5», не «зона равна 6». Практический риск нулевой: _wallThickHit/pull в этом ТЗ не трогается вообще (пункт 4 контракта), и план тестов отдельно называет мутационный гвард wallthick-hit-narrowed, который ловит сужение pull конкретно до gridPitch*2 — комбинация «код не тронут + мутационный гвард» закрывает риск регресса практически полностью. Снято без правки: это точность формулировки плана тестов, не дефект ТЗ, и правка стоила бы дороже, чем экономит.
  2. Имя нового смока demo/smoke_wallthick_hover_width.mjs не повторяет префикс smoke_wall_*, которым названы соседние файлы (smoke_wall_thickness.mjs, smoke_wall_hatch_density.mjs). При этом wallthick (без подчёркивания) — это имя самого инструмента в коде (_tool === 'wallthick', класс .wallthick-hover, toast.wallthick_pick), так что новое имя следует внутренней конвенции идентификатора инструмента, а не файловой конвенции соседних тестов. Это раскладка файлов — из перечня «всё, чего пользователь не наблюдает, агенты решают сами» (§7.1); ревьюер вправе оспорить, но не видит основания: обе конвенции существуют в проекте параллельно, и большего вреда, чем эстетика, здесь нет.

Что проверено и корректно

  • Оба продуктовых раздела присутствуют по существу: сценарий (Plan editor, Home admin, инструмент «Толщина») и «что видит человек до/после» (раздел 3 ТЗ) — без терминов реализации, ровно как требует §7.1.
  • Контракт поведения однозначен и полон для обеих ветвей (cm > 0, cm = 0) и для мусорного ввода (AC6); проверено, что wallCmToUnits и gridVisualScale уже сами защищаются от cellCm ≤ 0/NaN, так что AC6 не требует новой логики защиты сверх тривиального cm > 0 ? … : ….
  • Ни одного утверждения о поведении «из головы»: каждая ссылка на существующий код/конвенцию (gridVisualScale, --hp-cell-visual-scale, wallthick-hover вне golden, отсутствие изменений _wallThickHit) перепроверена чтением исходников этим ревью, а не принята как факт со слов автора — включая арифметику всех числовых AC (см. «Как проверялось», п.5).
  • Все 6 AC однозначны, имеют численный или структурный критерий и назначенный способ доказательства (unit в test/grid-scale.test.mjs, smoke в новом demo/smoke_wallthick_hover_width.mjs) — требование DoR §2.5 «у каждого AC указано, чем он доказывается» выполнено.
  • Границы скоупа названы явно («Не входит»: зона попадания, превью рисования, подсветка других инструментов, формат конфига) — соответствует правилу «скоуп не расширяется» (§3.9).
  • Откат — «один revert, конфиг не меняется» — корректен: функция чистая, без сохраняемого состояния и миграций.
  • Критерии лёгкого трека (§5) выполняются одновременно; small — обоснованная метка, downgrade в S3-spec не требуется.
  • Release-артефакты определены верно: User-Visible: yes → оба changelog; USER-GUIDE не требуется (подтверждено чтением, раздел «Толщина стены» не описывает ширину подсветки); golden не затронут (подтверждено чтением demo/golden/matrix.mjs).
  • Ни одного продуктового вопроса, отложенного на владельца зря: технические развилки (где живёт функция, имя файла смока) author/reviewer решают сами, ни одна не переквалифицирована в продуктовый вопрос ошибочно.

Чего не проверял

  • Не запускал npx tsc --noEmit / npm test / npm run build — на этапе ревью ТЗ кода ещё нет, реализация не начата; эти гейты относятся к циклу «В разработке» (§2.6) и код-ревью (§2.7), а не к ревью ТЗ (§2.4).
  • Не запускал npm run golden:verify, demo/smoke_* и scripts/smoke-select.mjs — те же основания: нечего исполнять, кода нет. Заявление о golden проверено статически (чтением matrix.mjs), это единственно возможный способ на этапе ТЗ.
  • Не проверял python -m pytest tests_backend — задача не трогает custom_components/**/*.py (класс A ограничен houseplan-card.ts и новой функцией в TS).
  • Не оценивал производительность — ТЗ явно и обоснованно заявляет «нет влияния» (чистая функция, вызывается там же, где раньше была инлайн- арифметика, без новых аллокаций/циклов), критерий §5 «нет влияния на производительность» выполняется тривиально.
  • Раздел «Унаследовано из r0» не пишется — это первый заход, дельта-режим §2.10 неприменим.

Вердикт

Зелёный. ТЗ выполнимо, однозначно, каждое числовое утверждение самостоятельно пересчитано и подтверждено, каждая ссылка на существующий код/конвенцию проверена чтением, а не принята на слово. Готово к разработке.