diff --git a/docs/reviews/CODE-REVIEW-201-r1.md b/docs/reviews/CODE-REVIEW-201-r1.md new file mode 100644 index 00000000..ea4cca34 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-201-r1.md @@ -0,0 +1,157 @@ +# Код-ревью #201 — r1 + +- Issue: [#201](https://github.com/Matysh/houseplan-card/issues/201) +- ТЗ: [docs/specs/201-atomic-thickness-lookup.md](../specs/201-atomic-thickness-lookup.md) + (ревью ТЗ зелёное: [SPEC-REVIEW-201-r1.md](SPEC-REVIEW-201-r1.md)) +- Диапазон: `git log --oneline origin/dev..HEAD` + - `f7abf14` fix: inherit parent thickness for atomic walls (`Issue: #201`, `User-Visible: yes`) + - `8b8b9ed` docs: review document for #201 (`Issue: #201`, `User-Visible: no`) + - `7b759f3` docs: specify atomic wall thickness lookup (`Issue: #201`, `User-Visible: no`) +- Роль: ревьюер кода, свежая сессия без контекста реализации. + +## Скоуп + +Баг: `thicknessCmAt()` возвращал 0 для атомарного child-сегмента, покрытого +более длинной exact wall-записью, из-за чего единственный продуктовый +потребитель — `thicknessOnClose()` в `src/open-spans.ts` — терял реального +соседа и подставлял `DRAW_WALL_DEFAULT_CM` (15 см) вместо фактических 20/22 см +при закрытии виртуальной границы, разделённой третьей комнатой. Скоуп по +non-scope ТЗ: только `thicknessCmAt()` + regression-покрытие; `wallIntervals()`, +`cmsForPoly()`, рендер, schema, Optimize (#198) не трогаются. + +Относится к J4/J6 (`docs/SCOPE.md`): «Keep the plan true as the home evolves» — +Close не должен незаметно подменять сохранённую физическую толщину значением +по умолчанию. + +## Как проверялось + +Дешёвые гейты прогнаны лично, не только по слову автора: + +| Гейт | Команда | Результат | +|---|---|---| +| Typecheck | `npx tsc --noEmit` | зелёный, без вывода | +| Unit | `npm test` | `912/912`, `fail 0` — совпадает с заявленным | +| Build | `npm run build` | зелёный | +| Bundle parity | `sha256sum dist/... custom_components/.../houseplan-card.js demo/srv/assets/houseplan-card.js` | все три `35ad6b54...a89e` — совпадает с хендоффом | +| Docs gate | `node scripts/check-docs.mjs --external` | `Documentation checks passed (7 files, 10 external links)` — уместен, т.к. коммит трогает `docs/images/screenshots.json` | +| Mutation gate | `node scripts/mutation-gate.mjs --check` | все записи `ok`, включая новую `atomic-child-thickness-parent-fallback` | +| Targeted smoke | `node demo/smoke_resize_virtual_thick.mjs` (после свежего `npm run build` + copy) | все поля `true`, включая три новых `atomicParentClose*`, `OK` | + +**Тест умеет падать — проверено исполнением, не по слову автора.** Временно +откатил `src/wall-thickness.ts` до состояния `origin/dev` (`git apply -R` на +diff файла), пересобрал `test-build` (`npx tsc -p tsconfig.test.json && node +scripts/fix-test-build.mjs`) и прогнал: +`node --test --test-name-pattern="exact parent|atomic solid children" test/wall-thickness.test.mjs test/open-spans.test.mjs` +— оба новых теста красные (`0 !== 20`, `15 !== 22`). Затем `git checkout -- +src/wall-thickness.ts`, пересобрал `test-build`, `node --test +test/wall-thickness.test.mjs test/open-spans.test.mjs` → зелёные (`97` тестов, +`0` fail), рабочее дерево чистое (`git status --short` пусто) — эксперимент не +оставил следов. + +Читал построчно: `thicknessCmAt()`, новый приватный `exactCoveringWall()`, +`entrySpan()`, `distToSeg()`, `angleClose()`, `segAngle()`, и параллельно — +`cmsForPoly()` (уже существующий алгоритм «наиболее узкий покрывающий exact +span», строки 1081–1099/1107–1140), чтобы убедиться, что новый helper — +переиспользование того же контракта, а не новая независимая эвристика. +Читал `thicknessOnClose()`/`applyThicknessOnClose()` в `src/open-spans.ts` — +diff по этому файлу пуст, что ожидаемо: единственный вызов уже шёл через +`thicknessCmAt()`, фикс на уровне resolver'а автоматически чинит потребителя. + +## Гейты, которые НЕ прогонял, и почему + +- **`node demo/smoke_*.mjs` (остальные 126 из 127).** Diff ограничен одной + чистой функцией с одним продуктовым потребителем внутри `src/wall-thickness.ts`; + AC7/AC8 сами называют только `smoke_resize_virtual_thick.mjs`. Остальные + smoke не касаются `thicknessCmAt`/Close и не входят в затронутые поверхности + ТЗ (§4/§9). +- **`npm run golden:verify`.** ТЗ прямо говорит golden не обязателен (§10.2): + видимая форма уже численно проверена browser-смоуком, общий wall renderer + (`wallIntervals()`/`cmsForPoly()`/body geometry) не менялся. Диф + `docs/images/screenshots.json` — только пересчитанный `sourceFingerprint` + под изменившийся `src/**`, `imageSha256` не менялся ни для одного сценария + → новой визуальной сцены нет. +- **`python -m pytest tests_backend -q`.** Диапазон не касается + `custom_components/**/*.py` (см. таблицу файлов diff) — backend гейт не + относится к этой задаче. +- **performance-профили.** Не названы в AC, риск в ТЗ явно закрыт (§11: поиск + идёт только при editor Close, линейный проход сравним с существующим lookup, + вне render tick) и не затрагивает горячий путь рендера. + +Это сужение соразмерно объёму: один pure-helper и один тестовый/smoke файл; +полный набор гейтов — предрелизный, а не гейт код-ревью (PROCESS.md §8). + +## Проверка AC (ТЗ §9) + +| AC | Как доказано | Вердикт | +|---|---|---| +| AC1 | `wall-thickness.test.mjs`: `thicknessCmAt inherits the narrowest exact parent...` — прогнан, зелёный; откатом кода воспроизведён красным (`0 !== 20`). | Доказано автотестом, тест умеет падать. | +| AC2 | Тот же тест покрывает `coordScale = 1` и `1000` и обратное направление endpoints — прогнан в составе `npm test`. | Доказано автотестом. | +| AC3 | `thicknessCmAt exact-parent fallback does not leak from partial or unrelated spans` — partial `[0..4]`→`[0..10]`, parallel offset, perpendicular, malformed (`a:['bad',0]`, вырожденный `[0,0]-[0,0]`) — все дают 0; прогнан в `npm test`. Прочитан код: `entrySpan()` отбрасывает нечисловые/неполные endpoints, `spanLen<1e-12` и `spanLen+tol