mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-02 12:49:56 +00:00
claude[bot]
@@ -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 неприменим.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Зелёный. ТЗ выполнимо, однозначно, каждое числовое утверждение
|
||||
самостоятельно пересчитано и подтверждено, каждая ссылка на существующий
|
||||
код/конвенцию проверена чтением, а не принята на слово. Готово к разработке.
|
||||
Reference in New Issue
Block a user