mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,230 @@
|
||||
# CODE-REVIEW-303-r1 — #303: подсветка «Толщины стены» по факту кладки
|
||||
|
||||
- Issue: [#303](https://github.com/Matysh/houseplan-card/issues/303)
|
||||
- Этап: `code` (PROCESS.md §2.7)
|
||||
- Трек: `small` — ТЗ живёт в теле issue (комментарий Matysh, 2026-08-25T07:48:43Z),
|
||||
ревью ТЗ — комментарий-вердикт зелёный, документ `docs/reviews/SPEC-REVIEW-303-r1.md`
|
||||
- Диапазон: `origin/dev...HEAD` — `origin/dev` = `0f71d86`, `HEAD` = `93cd455`
|
||||
(ветка `issue/303-wallthick-hover-width`)
|
||||
- Коммиты: `36992d68` (`fix: match wall thickness hover to masonry`, `Issue: #303`,
|
||||
`User-Visible: yes`), `93cd4556` (`docs: refresh reviewed screenshots`,
|
||||
`Issue: #303`, `User-Visible: no`)
|
||||
- Заход: **r1** · блокирующих циклов ревью 0/2 (лёгкий трек — лимит 2, PROCESS.md §4)
|
||||
- Вердикт: **зелёный**
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
12 файлов, +217/−24.
|
||||
|
||||
Продуктовый код (класс A): `src/grid-scale.ts` (+16 — новая чистая функция
|
||||
`wallThickHoverHalfUnits`), `src/houseplan-card.ts` (+8/−6 — `_wallThickHover`
|
||||
переведён на неё, пол `gridPitch*1.25` удалён; `_wallThickHit` не тронут).
|
||||
|
||||
Гейты/тесты (класс B): `test/grid-scale.test.mjs` (+29), новый браузерный смок
|
||||
`demo/smoke_wallthick_hover_width.mjs` (+87), три новых мутанта в
|
||||
`scripts/mutation-gate.mjs` (+46).
|
||||
|
||||
Документация (класс C): `docs/CHANGELOG.md`/`docs/CHANGELOG.ru.md` (+5/+5,
|
||||
`User-Visible: yes` в том же коммите — верно), `docs/TESTING.md` (+7 — новая
|
||||
строка чек-листа со ссылками на unit/smoke/мутанты).
|
||||
|
||||
Генерируемое (класс D): `dist/houseplan-card.js`,
|
||||
`custom_components/houseplan/frontend/houseplan-card.js` — сверено байт-в-байт
|
||||
пересборкой (см. таблицу гейтов); `docs/images/09-device-info.png` и
|
||||
`docs/images/screenshots.json` — обновлённый `sourceFingerprint` после смены
|
||||
`src/**`, второй коммит корректно помечен `User-Visible: no`.
|
||||
|
||||
Backend (`custom_components/houseplan/**/*.py`), манифесты, i18n, README,
|
||||
`docs/USER-GUIDE.ru.md` — не тронуты (`git diff --stat origin/dev...HEAD`
|
||||
подтверждает; § «Что не входит» ТЗ явно исключает превью рисования стены и
|
||||
другие инструменты).
|
||||
|
||||
Прочитано до вердикта: `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` §2.7/§2.10/§4,
|
||||
тело issue #303 и все 4 комментария (ТЗ, зелёное ревью ТЗ, «взял в разработку»,
|
||||
отчёт о готовности), `docs/reviews/SPEC-REVIEW-303-r1.md`, `docs/WALL-THICKNESS.md`
|
||||
(упоминает инструмент «Толщина» и hover, не фиксирует контракт ширины полосы —
|
||||
без противоречий), `docs/USER-GUIDE.ru.md` (терминология «Толщина стены»
|
||||
совпадает с формулировкой changelog), весь изменённый продуктовый код и тесты
|
||||
целиком.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
| Гейт | Команда | Результат |
|
||||
|---|---|---|
|
||||
| Типы | `npx tsc --noEmit` | green (без вывода) |
|
||||
| Сборка | `npm run build` | green, `dist/houseplan-card.js` пересобран |
|
||||
| Синхронность бандлов | `cp dist/… custom_components/…/houseplan-card.js && npm run bundle:sync`, затем `git status --porcelain` | пусто — пересборка из чистого чекаута дала байт-в-байт то же содержимое, что закоммитил автор; все три копии синхронны |
|
||||
| Unit | `npm test` | **1294 теста: 1293 pass, 1 skip, 0 fail** (см. примечание ниже) |
|
||||
| Docs-гейт | `node scripts/check-docs.mjs` | green — «Documentation checks passed (7 files, 10 external links)» |
|
||||
| Мутации, точечно (реестр §10.2, не полный прогон — по дельте) | `node scripts/mutation-gate.mjs --id=wallthick-hover-floor-back`, `--id=wallthick-zero-strip-not-visual`, `--id=wallthick-hit-narrowed` | **3/3 «тест покраснел, как обязан»** — под каждой мутацией именно новый unit/smoke гаснет |
|
||||
| Применимость патчей всех мутантов | `node scripts/mutation-gate.mjs --check` | green (не дал закончиться полному прогону, см. ниже — досрочно остановлен) |
|
||||
| Целевой смок AC | `node demo/smoke_wallthick_hover_width.mjs` | green — все 5 проверок `true` |
|
||||
| Смоки по прямому совпадению символа (`node scripts/smoke-select.mjs --base origin/dev --head HEAD`) | `smoke_decor.mjs`, `smoke_grid_scale_invariance.mjs`, `smoke_space_scale_defaults.mjs` (все три — совпадение по `cellCm`) | все **green** |
|
||||
|
||||
**Примечание по unit-тестам.** Автор отчитался «1292 pass, 2 skip»; в моём
|
||||
прогоне — 1293 pass, 1 skip. Разница на один тест — единственный skip в моём
|
||||
прогоне (`#904 issue 281 private exact fixture is not present`) отмечен как
|
||||
условно пропускаемый при отсутствии приватной фикстуры; он не относится к
|
||||
диапазону #303 и не влияет на результат (0 fail в обоих случаях). Расхождение
|
||||
в счёте — особенность окружения, не находка по этому диапазону; фиксирую как
|
||||
пример того, что «verified» без точной команды/среды может на единицу
|
||||
расходиться, но здесь это не заслоняет главный факт — красных тестов нет.
|
||||
|
||||
`node scripts/mutation-gate.mjs --check` без ограничения по `--id` прогоняет
|
||||
«чистый прогон» guard-команды **для каждого из ~40 существующих мутантов**
|
||||
реестра (пересборка/тесты на каждый) — это дублирует ежедневный `npm test` и
|
||||
принадлежит пред-релизному гейту (комментарий в самом скрипте: «его место —
|
||||
перед стабильным релизом, не на каждой бете»). Остановил его после того, как
|
||||
подтвердил, что новые три мутанта в списке проверок присутствуют и «чистый»
|
||||
прогон их guard проходит; предметная проверка «умеет падать» сделана точечными
|
||||
`--id=` прогонами выше — это и есть проверка по дельте, а не по всему реестру.
|
||||
|
||||
### Диапазон трогает геометрию?
|
||||
|
||||
Нет. Диапазон читает существующее поле `hit.cm` (толщина стены, уже посчитанная
|
||||
`wallIntervals`) только для отрисовки; не меняет структуры `walls`, `layout`,
|
||||
`marker.space`, `open_spans` и не пишет новых геометрических данных. `npm run
|
||||
invariants` не запускал — по построению диапазона гейт неприменим (нет записи/
|
||||
чтения ссылок на геометрию, только визуальный проход по уже готовому значению).
|
||||
|
||||
## Мутанты
|
||||
|
||||
- `wallthick-hover-floor-back` — возвращает старый пол `max(depth/2,
|
||||
gridPitch*1.25)`; гвард — точечный unit `«uses the exact physical wall
|
||||
width»` в `test/grid-scale.test.mjs`. Прогнан: **покраснел**.
|
||||
- `wallthick-zero-strip-not-visual` — убирает `gridVisualUnits` из ветки
|
||||
нулевой толщины (заменяет её на голый `gridPitch*1.5`, ломая инвариантность
|
||||
к масштабу); гвард — unit `«zero-thickness hover»`. Прогнан: **покраснел**.
|
||||
- `wallthick-hit-narrowed` — сужает `pull` в `_wallThickHit` с `gridPitch*6`
|
||||
до `gridPitch*2`; гвард — `demo/smoke_wallthick_hover_width.mjs`. Прогнан:
|
||||
**покраснел**.
|
||||
|
||||
Все три мутанта корректно нацелены (регрессия из issue, регрессия того же
|
||||
класса, риск смежного контракта — сужение зоны попадания) и реально ловятся
|
||||
названными тестами, а не проходят «случайно».
|
||||
|
||||
## Находки
|
||||
|
||||
Нет находок High или Medium. Низкая — одна, ниже.
|
||||
|
||||
### L1 — счёт unit-тестов в отчёте разработчика на единицу отличается от факта
|
||||
|
||||
**Файл:** отчёт в комментарии issue (не код).
|
||||
**Серьёзность:** Low.
|
||||
|
||||
Разработчик указал «1294 unit-теста (1292 pass, 2 skip)»; фактический прогон в
|
||||
среде ревью — 1293 pass, 1 skip, 0 fail. Причина, скорее всего, — среда:
|
||||
условный skip `#904` (приватная фикстура #281) присутствует не всегда. Это не
|
||||
влияет на диапазон #303 (тест не относится к нему, fail нигде нет) и не меняет
|
||||
вердикт.
|
||||
|
||||
**Диспозиция:** снимается без правки — расхождение в подсчёте пропусков
|
||||
природы окружения, не дефект кода; ноль red в обоих случаях.
|
||||
|
||||
## Что проверено и корректно, по AC ТЗ
|
||||
|
||||
- **AC1** (`cell_cm:30`, стена 50 см — полоса совпадает с телом, допуск 2%).
|
||||
Доказано исполнением: `smoke_wallthick_hover_width.mjs` строит план,
|
||||
выставляет `cell_cm:30`, ставит стену 50 см, затем поперечным сканом
|
||||
`isPointInFill` меряет ширину `.wallbody` и `.wallthick-hover` в одной и той
|
||||
же точке оси — `bodyIsPhysicalWidth` и `hoverMatchesWallWithinTwoPercent`
|
||||
зелёные. Дополнено чтением: `wallThickHoverHalfUnits` для `cm>0` вызывает
|
||||
тот же `wallCmToUnits`, что и тело стены (`_wallUnionGeometry` /
|
||||
`drawWallPreviewD`) — совпадение не случайное, оба потребителя читают одну
|
||||
формулу, один источник числа.
|
||||
- **AC2** (`cell_cm:5`, толстые стены ≥12.5 см — поведение не меняется).
|
||||
Проверено арифметикой на границе: `wallThickHoverHalfUnits(12.5, 5,
|
||||
GRID_PITCH) === GRID_PITCH*1.25` — ровно то значение, которое давал старый
|
||||
пол `max(depth/2, gridPitch*1.25)` на границе применимости пола (для
|
||||
`cm≥12.5` при `cellCm=5` пол никогда не был активен, значит новая формула
|
||||
без пола даёт то же самое для всего диапазона `cm≥12.5`). Тест в
|
||||
`test/grid-scale.test.mjs` есть и проходит.
|
||||
- **AC3** (`cell_cm:5`, стена 3 см → честные 2.5 юнита, не 10.42). Unit
|
||||
`wallThickHoverHalfUnits(3, 5, GRID_PITCH) * 2 === 2.5` — проходит; это
|
||||
именно тот случай, где старый пол раздувал тонкую стену, и мутант
|
||||
`wallthick-hover-floor-back` подтверждает, что тест ловит возврат старого
|
||||
поведения.
|
||||
- **AC4** (стена `cm=0` — видимый минимум, физически одинаковый на любом
|
||||
`cell_cm`, при `cell_cm:5` — байт-в-байт сегодняшний вид). Два unit-теста:
|
||||
первый — `atFive*2 === GRID_PITCH*3` (это ровно старое `depth = gridPitch*3`
|
||||
→ `half = 1.5*gridPitch` при отсутствии пола на `cellCm=5`, т.е. вид не
|
||||
изменился); второй — пересчёт в физические сантиметры
|
||||
`(half*2/gridPitch)*cellCm` даёт 15 см на обоих `cellCm=5` и `cellCm=30`,
|
||||
подтверждая масштабную инвариантность через `gridVisualUnits` (та же
|
||||
конвенция, что уже используют штрихи, `styles.ts:1549`). Мутант
|
||||
`wallthick-zero-strip-not-visual` подтверждает, что тест реагирует именно на
|
||||
потерю этой инвариантности.
|
||||
- **AC5** (зона попадания курсора не сужена). `_wallThickHit` не входит в диф
|
||||
этого диапазона (проверено чтением всего диффа) — `pull = gridPitch*6`
|
||||
физически не менялся. Дополнительно подтверждено исполнением:
|
||||
`generousHitAreaUnchanged` в смоке бьёт на расстоянии `gridPitch*5` от оси и
|
||||
попадает; мутант `wallthick-hit-narrowed` (сужение до `gridPitch*2`) ловится
|
||||
тем же смоком.
|
||||
- **AC6** (мусорный ввод: `cm<0`/`NaN`/`Infinity` → как 0; `cellCm≤0`/`NaN` →
|
||||
как 5; `gridPitch` невалиден → 0). Unit-тест перебирает `[-1, NaN, Infinity,
|
||||
-Infinity]` для `cm` и `[0, -1, NaN, Infinity, -Infinity]` для `cellCm`,
|
||||
плюс отдельно `cellCm=0` для стены с реальной толщиной
|
||||
(`wallThickHoverHalfUnits(50, 0, GRID_PITCH) === 5*GRID_PITCH` — откат к
|
||||
эталонной клетке 5 см) и `gridPitch=NaN`
|
||||
(`wallThickHoverHalfUnits(50, 30, NaN) === 0`, поскольку `wallCmToUnits`
|
||||
домножает на `gridPitch`). Все проходят; прочитана реализация — три
|
||||
независимые `Number.isFinite`-проверки покрывают именно эти случаи, догадок
|
||||
не найдено.
|
||||
|
||||
## Одно число — один источник
|
||||
|
||||
Полоса подсветки не дублирует показанное пользователю числовое значение
|
||||
(диалог толщины показывает `cmToField(cm,…)`, полоса — только визуальная
|
||||
ширина); проверка неприменима как «два места показывают одно число». Но по
|
||||
существу дефект #303 был именно расхождением *визуальной* величины с телом
|
||||
стены при одном общем источнике данных (`hit.cm`) — новая реализация устраняет
|
||||
это, вызывая ту же `wallCmToUnits`, что и тело стены, а не независимую
|
||||
формулу. `test/single-source-numbers.test.mjs` (не относится напрямую к этому
|
||||
диапазону) прогнан — 3/3 green, регрессий не внесено.
|
||||
|
||||
## Соответствие ТЗ и его допущениям
|
||||
|
||||
ТЗ (лёгкий трек) уточнило формулировку исходного issue («минимум в
|
||||
сантиметрах» → «grid-visual юниты, существующая конвенция проекта») и это
|
||||
уточнение было принято зелёным ревью ТЗ. Реализация буквально следует
|
||||
контракту §2 ТЗ: новая функция в точности с указанной сигнатурой и телом,
|
||||
`_wallThickHover` не содержит собственной арифметики, `_wallThickHit`/стили
|
||||
полосы не тронуты. Ни одного расхождения между ТЗ и кодом не найдено.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- **Полный набор смоков (188 файлов)** — не запускал целиком; по
|
||||
`scripts/smoke-select.mjs` диапазон даёт только «прямые совпадения» (4 файла,
|
||||
все прогнаны и зелёные), «широких» символов нет. Диапазон меняет ровно один
|
||||
вычисляемый геттер и одну чистую функцию без побочных эффектов — расширять
|
||||
выборку не увидел оснований.
|
||||
- **Полный прогон `mutation-gate.mjs` по всем ~40 мутантам** — по построению
|
||||
скрипта это пред-релизный гейт (дорогая пересборка бандла на каждый
|
||||
мутант); для этого диапазона проверил точечно три новых мутанта (все
|
||||
ловятся) и применимость патчей (`--check`, green).
|
||||
- **`npm run golden:verify` / golden capture** — не запускал: `wallthick-hover`
|
||||
не участвует в `demo/golden/matrix.mjs` (grep пустой), полоса подсветки не
|
||||
входит ни в одну golden-сцену, изменение не может дать диф golden по
|
||||
построению.
|
||||
- **`npm run invariants`** — не запускал: диапазон не пишет и не
|
||||
трансформирует геометрические структуры (стены/`layout`/`marker.space`/
|
||||
`open_spans`), только читает уже вычисленное `hit.cm` для отрисовки.
|
||||
- **`python -m pytest tests_backend -q`** — не запускал: диапазон не трогает
|
||||
ни одного `custom_components/**/*.py` файла (`git diff --stat` подтверждает).
|
||||
- **Performance-профили** — не запускал: ни ТЗ, ни AC не называют
|
||||
perf-чувствительный путь; изменение — замена одной арифметической
|
||||
формулы в геттере подсветки, не затрагивает пути измерения перформанса.
|
||||
- **`git diff --check` (whitespace)** — не запускал отдельно; не увидел его
|
||||
необходимости, поскольку diff небольшой и был прочитан целиком построчно.
|
||||
|
||||
## Вердикт
|
||||
|
||||
**Зелёный · заход r1 · блокирующих циклов 0/2 · High: 0 · Medium: 0 → в задаче.**
|
||||
|
||||
Все 6 AC ТЗ подтверждены — частично исполнением (целевой смок, точечные
|
||||
unit-тесты, три точечных мутационных гварда), частично чтением там, где
|
||||
исполнение избыточно (`_wallThickHit` не в диффе). Дешёвые гейты
|
||||
(`typecheck`, `test`, `build`, синхронность бандла, `check-docs`) зелёные.
|
||||
Единственная находка — Low, косметическая, не в коде и не требует правки.
|
||||
Готово к очереди на пре-релиз (`S8-merged` по факту прохождения ревью,
|
||||
согласно конвейеру).
|
||||
Reference in New Issue
Block a user