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

20 KiB
Raw Permalink Blame History

CODE-REVIEW-303-r1 — #303: подсветка «Толщины стены» по факту кладки

  • Issue: #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 по факту прохождения ревью, согласно конвейеру).