diff --git a/docs/reviews/SPEC-REVIEW-303-r1.md b/docs/reviews/SPEC-REVIEW-303-r1.md new file mode 100644 index 00000000..48f793f8 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-303-r1.md @@ -0,0 +1,175 @@ +# SPEC-REVIEW-303-r1 + +Issue: [#303](https://github.com/Matysh/houseplan-card/issues/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 неприменим. + +## Вердикт + +Зелёный. ТЗ выполнимо, однозначно, каждое числовое утверждение +самостоятельно пересчитано и подтверждено, каждая ссылка на существующий +код/конвенцию проверена чтением, а не принята на слово. Готово к разработке.