From 66ffd6fdda347a005993a8d396ee61c14ccc3adc Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sat, 22 Aug 2026 07:22:40 +0000 Subject: [PATCH] docs: review document for #233 Issue: #233 User-Visible: no --- docs/reviews/CODE-REVIEW-233-r2.md | 162 +++++++++++++++++++++++++++++ 1 file changed, 162 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-233-r2.md diff --git a/docs/reviews/CODE-REVIEW-233-r2.md b/docs/reviews/CODE-REVIEW-233-r2.md new file mode 100644 index 00000000..d6fba99a --- /dev/null +++ b/docs/reviews/CODE-REVIEW-233-r2.md @@ -0,0 +1,162 @@ +# CODE-REVIEW — issue #233, заход r2 + +- Issue: [#233](https://github.com/Matysh/houseplan-card/issues/233) — «Резайз показывает внутренние размеры, а не осевые» +- ТЗ: `docs/specs/233-resize-inner-dimensions.md`, принят зелёным на r2 (`docs/reviews/SPEC-REVIEW-233-r2.md`) +- Предыдущий раунд кода: r1, вердикт **жёлтый**, документ `docs/reviews/CODE-REVIEW-233-r1.md` (коммит `bff47f5`), + проверенный коммит **`abfaae3`** — feat: measure resize labels between wall faces +- Заход: r2 (код), блокирующих циклов израсходовано 1 из 4 (потрачено на r1; зелёный этого раунда цикл не тратит) + +## Скоуп ревью + +Это возврат по находкам M1/M2 предыдущего раунда, не новый заход задачи. Дельта +локальна: `git diff abfaae3..HEAD` (= коммит `81f3868`) трогает ровно три файла — +`test/wall-thickness.test.mjs`, `scripts/mutation-gate.mjs` и сам документ r1-ревью. +Продуктовый код (`src/wall-thickness.ts`, `src/houseplan-card.ts`) не тронут — +это подтверждено не только диффом, но и байт-в-байт совпадением бандла (см. +«Как проверялось», п.2). Никаких признаков «ребейз на ушедший вперёд dev», +смены контракта поведения или новой подсистемы нет — `origin/dev...HEAD` +идентичен тому, что видел r1, плюс этот один коммит. Поэтому разбор сокращён +до дельты и наследования, а не полного прогона AC1–AC9. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| **M1** — тест AC6b (`test/wall-thickness.test.mjs:1533-1552` на `abfaae3`) не создавал настоящий сплит-разрыв: `setWallThickness` писал толщину на **весь** осевой отрезок ребра, поэтому `thicknessCmAt` по целому ребру совпадал с профилем, и тест не отличал бы исправленный код от регресса к H2 | Тест переписан на реальный сплит: `setWallThickness([], [0,0], [0.6,0], 20, pitch)` при ребре комнаты `[0,0]–[1,0]` | `test/wall-thickness.test.mjs:1533-1571` (коммит `81f3868`). Явные assert'ы: `thicknessCmAt(split, [0,0], [1,0], pitch) === 0` (это и есть H2), `thicknessCmAt(split, [0,0], [0.6,0], pitch) === 20`, `ownEdgeOffsets(...)[0] === halfDepth` (не 0), плюс `assert.notEqual(viaProfile[0], thicknessCmAt(...))` — тест теперь явно фиксирует расхождение источников, а не совпадение | +| **M2** — `scripts/mutation-gate.mjs` (диф `abfaae3`) содержал три мутанта из четырёх, названных в принятом §11 ТЗ; `inner-span-ignores-neighbour-thickness` (страж AC2) отсутствовал | Добавлена запись мутанта, патчащая `cutAt` в `innerEdgeSpan` (`const o = Math.max(0, Number(offsets[edge]) \|\| 0);` → `const o = own;`), привязанная к тесту `innerEdgeSpan measures between wall faces` | `scripts/mutation-gate.mjs:111-125` (коммит `81f3868`). Независимо прогнан: `node scripts/mutation-gate.mjs --id=inner-span-ignores-neighbour-thickness` → «поймано 1 из 1» | + +Обе находки были про доказательную базу, не про поведение продукта (r1 это +установил независимым исполнением сценария сплит-ребра вне репозитория) — и +продуктовый код в этом раунде действительно не менялся, значит поведение, +подтверждённое в r1, не могло сломаться этой правкой. + +## Как проверялось + +1. Построчно сверен диф `81f3868` с рецептами правок из документа r1 (см. + таблицу выше) — обе правки текстуально соответствуют тому, что r1 просил + исправить, а не что-то похожее. +2. Проверено, что продуктовый код не менялся: `git diff abfaae3..HEAD -- src/` + пуст; после `npm run build` все три копии бандла дают тот же md5, что + заявляли и автор, и r1-ревью — `5ffd65073d92c4845aecb1a4145f0948` + (`dist/houseplan-card.js`, `demo/srv/assets/houseplan-card.js`, + `custom_components/houseplan/frontend/houseplan-card.js`). Это делает + независимую повторную проверку поведения продукта (AC1–AC9) избыточной — + она физически не могла измениться. +3. Гейты прогнаны заново на дереве `81f3868` (не унаследованы из r1, так как + код тестов/гейта изменился): + - `npx tsc --noEmit` — чисто; + - `npx tsc -p tsconfig.test.json` — чисто (это конфиг, которым фактически + компилируются тесты и мутационный гейд); + - `npm test` — **1026/1026**, включая переписанный тест M1; + - `node scripts/mutation-gate.mjs --check` — **88/88 `ok`**, exit 0 + (было 87 на r1, теперь на один больше — учтён новый мутант M2); + - адресный прогон всех четырёх мутантов задачи по одному, чтобы не + доверять агрегату: + - `--id=inner-span-ignores-neighbour-thickness` → поймано 1 из 1 (новый, M2); + - `--id=inner-span-reads-whole-edge-thickness` → поймано 1 из 1 + (переписанный тест M1 всё ещё ловит его, но теперь по правильной + причине — падает конкретно на сплит-ребре, а не потому что путь через + профиль не исполнился); + - `--id=inner-span-shortens-a-passage` → поймано 1 из 1; + - `--id=resize-labels-show-centreline` → поймано 1 из 1. +4. Прочитан патч M2 в контексте реального `src/wall-thickness.ts:898` + (`cutAt`, внутри `innerEdgeSpan`) — строка `find` мутанта текстуально + совпадает с текущим кодом, мутация подставляет собственную толщину `own` + вместо толщины соседнего ребра `offsets[edge]`, что и есть штраф AC2 + («сокращение считается только по своей толщине, соседи игнорируются»). + Патч не «на что-то похожее», а именно на тот код, который правка защищает. +5. Проверено, что дифф `81f3868` не трогает `docs/CHANGELOG*.md` и + `docs/USER-GUIDE*.md` — ожидаемо, коммит несёт `Issue: #233`, + `User-Visible: no` (тестовый коммит класса B), changelog не требуется. + +## Находки + +Нет новых находок. Обе находки r1 закрыты доказательно (см. таблицу выше и +«Как проверялось», пп. 1, 3, 4), High: 0, Medium: 0. + +Отдельно отмечаю: в предпоследнем комментарии автор сообщил про красный job +`docs` в `validate.yml` (несвежий `sourceFingerprint` в +`docs/images/screenshots.json` из-за слияния #234 без пересъёмки) и завёл под +это отдельный issue. Это не находка этого ревью — дефект не в ветке #233, не +в скоупе задачи, и уже обслуживается отдельно самим автором, значит нового +issue от ревьюера не требуется (правило дублирования наблюдений, §12: не +заводить второй issue на то же наблюдение). + +## Что проверено и корректно + +- **M1, M2** — закрыты, доказано исполнением, не только чтением коммит-сообщения + (см. «Как проверялось»). +- **Продуктовое поведение (AC1–AC9)** — не могло измениться этой правкой: + бандл байт-в-байт идентичен версии, которую r1 уже проверил чтением, + исполнением гейтов и независимым ручным сценарием сплит-толщины (H2). +- **Трейлеры** `81f3868`: `Issue: #233`, `User-Visible: no` — корректны для + тест-only правки; changelog не трогается, как и должно быть при + `User-Visible: no`. +- **Формальный контракт §11 ТЗ** (четыре мутационных стража) — теперь + выполнен полностью: все четыре присутствуют в `scripts/mutation-gate.mjs` + и каждый independently подтверждён «1 из 1». + +## Унаследовано из r1 + +Без повторной проверки в этом раунде принято из `docs/reviews/CODE-REVIEW-233-r1.md` +(документ на коммите `bff47f5`, проверенный коммит продукта `abfaae3`): + +- **AC1–AC5** — численно сверенные длины (285/385 см на 300×400 при стенах 15 см), + диагональные рёбра, случай «стены толще комнаты» → 0. +- **AC6a (H1, правило нуля для проёмов)** — `if (!(own > 0)) return centre;` + идёт первым в `innerEdgeSpan`, перепроверено математически на кейсе H1. +- **AC6b (H2, источник толщин)** — механизм `ownEdgeOffsets` → `roomWallProfile` + → `atomicPolyForRoom` подтверждён независимым исполнением сценария сплит-ребра + вне репозитория (не тем тестом, который чинит M1, — отдельным ручным прогоном + r1). В этом раунде довеском стало то, что теперь и сам юнит-тест воспроизводит + этот же сценарий (закрытие M1 выше). +- **AC7** — три подписи `_rszEdgeLabels` и габарит `_rszScaleLabels` внутренние; + подтверждено прогоном `demo/smoke_resize_inner_dimensions.mjs` (OK) на дереве + `abfaae3`. Не перепрогонялось в r2, так как бандл (см. п.2 выше) идентичен — + прогон на неизменившемся артефакте не может дать другой результат. +- **AC8** — площадь не менялась (`innerContourForRoom` + `floorMinusBodies`), + подтверждено тем же смоком (`dragAreaStillInner`, `frameAreaAgreesWithSize`). +- **AC9** — трейлеры и changelog для продуктового коммита `abfaae3` (`Issue: #233`, + `User-Visible: yes`, `docs/CHANGELOG.md`/`.ru.md` и `docs/USER-GUIDE*.md` + правлены в нём же). +- **i18n** — диф `abfaae3` не трогает `src/i18n/*.json`. +- **Golden** — вне риска по построению (`_rszLive` живёт только на время жеста + ресайза, обнуляется на `pointerup`); проверено r1 чтением, не исполнением + `golden:verify`. +- **Регрессия толщины стен** — `smoke_draw_wall_thickness.mjs`, + `smoke_wall_thickness_transition.mjs` зелёные на дереве `abfaae3`. +- **Touch** — диф не трогает pointer-пути, `docs/TOUCH-SUPPORT.md` не затронут. + +Все эти пункты наследуются с уверенностью, потому что предмет наследования — +поведение бандла, а бандл в r2 не пересобран из другого исходника: он тот же +файл, что и на момент r1-вывода (совпадение md5 подтверждено заново в этом +раунде, не взято на веру). + +## Чего не проверял и почему + +- **`demo/smoke_resize_inner_dimensions.mjs` и регрессионные смоки повторно** — + не перепрогонял: продуктовый код не менялся (бандл идентичен), результат + прогона на неизменившемся артефакте не может отличаться от того, что уже + зафиксировал r1. Перепрогон был бы тратой минут без шанса найти что-то новое. +- **`npm run golden:verify`** — не гонял, по той же причине плюс аргумент r1 + (оверлей `_rszLive` вне активного жеста не рендерится, статический golden + его не видит); дифф этого раунда вообще не в `src/**`. +- **`python -m pytest tests_backend`** — не гонял, диф не трогает + `custom_components/**/*.py`. +- **performance-профили** — не гонял, не названы в AC, диф не в горячих путях + рендера и вообще не в продуктовом коде. +- **`demo/smoke_*.mjs` вне названных в AC/§12** — не прогонял весь набор из + 127: дифф ограничен тестовым файлом и мутационным гейтом, у которых нет + браузерной поверхности вовсе. + +## Вердикт + +Обе находки r1 (M1, M2) закрыты именно так, как требовал документ r1: тест +теперь строит настоящий сплит-разрыв и различает исправленный код от +регресса к H2, мутационный гейт получил четвёртый страж контракта §11 и +независимо подтверждён «1 из 1» по каждому из четырёх мутантов задачи. +Продуктовый код не менялся — бандл байт-в-байт идентичен версии, которую r1 +уже проверил чтением, гейтами и ручным сценарием, так что AC1–AC9 наследуются +без риска регрессии из этой правки. Новых находок нет. + +**Вердикт: зелёный · заход r2 · блокирующих циклов 1/4 · High: 0 · Medium: 0**