From 7180eee0ee029305cf65779c2535581ccdd614e7 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sat, 22 Aug 2026 23:15:25 +0000 Subject: [PATCH] docs: review document for #250 Issue: #250 User-Visible: no --- docs/reviews/CODE-REVIEW-250-r1.md | 188 +++++++++++++++++++++++++++++ 1 file changed, 188 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-250-r1.md diff --git a/docs/reviews/CODE-REVIEW-250-r1.md b/docs/reviews/CODE-REVIEW-250-r1.md new file mode 100644 index 00000000..788cf1a6 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-250-r1.md @@ -0,0 +1,188 @@ +# CODE-REVIEW #250 — заход r1 + +- Issue: [#250](https://github.com/Matysh/houseplan-card/issues/250) +- ТЗ: `docs/specs/250-opening-centerline.md`, зелёный SPEC-REVIEW r1 на `2c776b0` + (`docs/reviews/SPEC-REVIEW-250-r1.md`). +- Ветка: `issue/250-opening-centerline` +- Диапазон: `origin/dev..HEAD` = `2c776b0`, `2534916`, `0eb9ffe`, `7269550` (4 коммита) +- Продуктовый коммит: `0eb9ffe` (`Issue: #250`, `User-Visible: yes`, оба changelog). + Второй коммит `7269550` — принятый Docs screenshots артефакт (`User-Visible: no`). + +## Скоуп проверки + +Диапазон небольшой (production-дельта — 3 файла `src/**`, остальное тесты, +мутанты, golden-контракт, документация), поэтому разбор полный: заход первый, +предыдущего раунда нет, «Унаследовано из r» неприменимо. + +Прочитаны docs/SCOPE.md (J1/J6), тело issue #250 и все пять комментариев, +docs/USER-GUIDE.ru.md, docs/WALL-THICKNESS.md, docs/ARCHITECTURE.md, +docs/ISOMETRIC.md, ТЗ целиком. + +## Как проверялось + +Прочитан весь `git diff origin/dev...HEAD` построчно (29 файлов). Дополнительно +исполнено (все команды дают тот же результат, что заявлен автором): + +- `npx tsc --noEmit` — green. +- `npm test` — 1114/1114 pass, 0 fail (автор сообщил 1113 pass/1 skip; локально + skip не воспроизведён — см. «Чего не проверял», не блокирует). +- `npm run build` — green; `sha256sum` трёх копий бандла совпадает друг с + другом и с хэшем `806648ae6036…`, указанным автором. +- `node scripts/check-docs.mjs` — green (7 файлов, 10 внешних ссылок). +- `node scripts/process-gate.mjs --range origin/dev..HEAD --issues` — green, + 0 предупреждений. +- `node scripts/mutation-gate.mjs --id=opening-symbol-flip-restores-edge-offset` + — новый мутант пойман 1 из 1 (см. AC1 ниже). +- `node scripts/mutation-gate.mjs --id=opening-gate-flip-cancels-turn` — + **0 из 1**, см. находку CR-1. +- `node demo/smoke_wall_thickness.mjs`, `node demo/smoke_isometric_contract.mjs`, + `node demo/smoke_opening_preview.mjs` — все три green на свежепересобранном + бандле (скопирован в `custom_components/houseplan/frontend/` и + `demo/srv/assets/`). +- `node scripts/smoke-select.mjs --base origin/dev --head HEAD` — + НЕОПРЕДЕЛЁННОСТЬ (169 смоков, порог «широкого» символа >33; изменённых + символов проекта — 6: `OpeningCfg`, `OpeningFaceOffset`, `_type`, `_flipV`, + `_angle`, `_face`; ни один смок не назван инструментом напрямую). Решение + ревьюера: три выбранных автором smoke — прямое покрытие темы (wall + thickness/iso/opening preview), этого достаточно вместе с unit/golden-unit + матрицей; полная матрица 167 смоков не оправдана диффом в 6 символов на 3 + файлах. +- `git diff --check origin/dev...HEAD` — чисто, whitespace ошибок нет. +- Трейлеры `0eb9ffe`/`7269550` прочитаны напрямую (`git log -1 --format=...`) + — оба корректны, `User-Visible: yes` действительно несёт оба changelog в том + же коммите. + +Не прогонялось (осознанно, ниже обоснование): `npm run golden:verify` (полный), +`python -m pytest tests_backend` (backend/schema не тронуты, AC7 подтверждён +чтением — модель `flip_v` не меняется), полный `smoke_*` набор (167 смоков), +performance-профили (не названы в AC, дельта — константный helper без цикла). + +## Находки + +### CR-1 (Medium, в скоупе) — мутационный guard `opening-gate-flip-cancels-turn` сломан переименованием теста + +`scripts/mutation-gate.mjs:1678` вызывает +`node --test --test-name-pattern="shared renderer centres defaults" test/opening-symbol.test.mjs`. +Этот тест был переименован этим же диффом +(`test/opening-symbol.test.mjs`, было `'shared renderer centres defaults and +preserves explicit door/window edge alignment'`, стало `'shared renderer +centres every flip while preserving opening direction'`) — подстрока +`"shared renderer centres defaults"` больше не встречается ни в одном имени +теста файла. + +**Воспроизведение:** + +``` +$ node scripts/mutation-gate.mjs --id=opening-gate-flip-cancels-turn +ok чистый прогон: ... node --test --test-name-pattern="shared renderer centres defaults" test/opening-symbol.test.mjs +FAIL opening-gate-flip-cancels-turn: тест остался зелёным на сломанном коде +поймано 0 из 1 +``` + +Причина видна напрямую: паттерн не матчит ни один подтест, `node --test` +выполняет 0 тестов и рапортует `pass 1` (файл целиком) с exit code 0 — и на +чистом дереве, и на мутированном. Guard стал тождественно зелёным независимо +от того, сломано направление ворот или нет. + +Дешёвая проверка `test/mutation-gate.test.mjs` («якоря живы») этого не ловит: +она проверяет только присутствие патч-анкора в исходнике, а не то, что +`--test-name-pattern` matches. Полный `mutation-gate.mjs` (единственное место, +где это видно) — предрелизный/еженедельный гейт +(`.github/workflows/mutation-gate.yml`), не гейт этого code review; поэтому +автор, прогнавший только новый мутант по имени, не был обязан это заметить +типовым implementation-гейтом, но ТЗ §14 прямо требует: «Существующий +`opening-gate-flip-cancels-turn` сохраняется и обязан продолжать падать». +Сейчас это не так — обязательство не выполнено. + +**Почему это Medium, а не High:** реальную регрессию направления поворота +ворот по-прежнему ловит обычный `npm test` (assertions теста выполняются +целиком без фильтра имени, см. `demo/smoke_isometric_contract.mjs` / +`isoGateFlipReversesTurn` — независимое дублирующее покрытие тоже green). +Пострадала только дисциплина «тест обязан уметь падать» применительно к этому +конкретному mutation-guard, не пользовательское поведение. + +**Почему в скоупе, не отдельный issue:** тест переименовало само это изменение +(`test/opening-symbol.test.mjs` — файл диффа задачи), а не соседняя +подсистема; PROCESS.md/#202 требует чинить Medium-в-скоупе на месте. + +**Исправление:** привести `--test-name-pattern` в +`scripts/mutation-gate.mjs:1678` в соответствие новому имени теста (например, +`"shared renderer centres every flip"`) и повторно прогнать +`node scripts/mutation-gate.mjs --id=opening-gate-flip-cancels-turn`, ожидая +`поймано 1 из 1`. + +Других находок нет. + +## Что проверено и корректно + +- **AC1** — `openingSymbolOffset()` возвращает точный `{ox:0, oy:0}` для всех + комбинаций; сигнатура сохранена (`_type/_flipV/_angle/_face`, допущение §19.2). + `test/opening-symbol-placement.test.mjs` — единый тест-матрица: 4 типа × 2 + flip × 5 углов (включая `NaN`) × 5 faces (включая `Infinity`/`NaN`/нулевую + грань) — честная замена прежних трёх сценариев, мутант ловится 1/1 + (проверено исполнением, см. «Как проверялось»). +- **AC2/AC3** — Flat (`src/render/opening-symbol.ts`) и Iso + (`src/iso-openings.ts:87`, не тронут — берёт тот же offset) используют один + helper; `test/opening-symbol.test.mjs` и `test/iso-openings.test.mjs` + раздельно проверяют «один origin» и «противоположный knob направления» + (`scale(1 1)`/`scale(1 -1)`, `quarterVector[1]` знак). Прогонкой + `demo/smoke_isometric_contract.mjs` подтверждено на реальном рендере. +- **AC4** — gate: offset остаётся нулевым, знак 10° поворота меняется; + `opening-gate-flip-cancels-turn` mutant текстуально сохранён (но см. CR-1), + дублирующее покрытие (`isoGateFlipReversesTurn` smoke, полный `npm test`) + подтверждает поведение реально работает. Passage не создаёт символ — не + тронут (`type === 'passage'` ветка убрана из helper, но `renderOpeningVisibleGeometry` + для passage не рисует geometry вообще, проверено чтением). +- **AC5** — room-wall/partition parity: `demo/smoke_wall_thickness.mjs` + (`doorSavedFlipStaysCentered`) и `demo/golden/matrix.mjs` (обе grани + `positiveFace`/`negativeFace` в unit-тесте дают одинаковый ноль) — проверено + исполнением. +- **AC6** — толстая стена: `src/wall-thickness.ts` тронут только в + комментарии, `openingInnerFaceOffset` (physical half-depth/jamb) не задет; + `demo/smoke_wall_thickness.mjs` (`thickWallFullHatched`, + `thickWallStaticHatched`, `openingCutsSlab`) green — проверено исполнением. + Lock badge/hitbox/actions — не тронуты диффом (проверено чтением: ни один + файл лок-гарда/hitbox в diff не участвует). +- **AC7** — модель данных: diff не содержит правок schema/backend/migration + (`git diff --stat` подтверждает — только `src/opening-symbol-placement.ts`, + `src/render/opening-symbol.ts`, `src/wall-thickness.ts` в `src/**`); `flip_v` + остаётся тем же boolean-полем — проверено чтением. +- **AC8** — документы больше не обещают edge-align: + `WALL-THICKNESS.md`, `ARCHITECTURE.md`, `ISOMETRIC.md`, `USER-GUIDE.ru.md`, + `TESTING.md` синхронно правлены, оба changelog заполнены в том же коммите + `0eb9ffe` — прочитано построчно, `check-docs` green. +- **AC9** — typecheck/test/build green, три бандла побайтово идентичны — + подтверждено исполнением (SHA `806648ae6036…` совпал). +- **Golden-контракт**: `demo/golden/matrix.mjs`/`harness.mjs`/`run.mjs` + заменяют `offset: 'edge'` на `'center'` и убирают саму опцию `'edge'` из + допустимых значений; `test/golden-matrix.test.mjs` проверяет обновлённый + контракт четырёх сцен и счётчик impact-сет = 67 (совпадает с числом, + заявленным автором) — прогнано вместе с `npm test`, green. +- **Мутанты (реестр)**: три устаревших якоря (`opening-symbol-default-uses-room-face`, + `opening-symbol-partition-follows-endpoints`, `opening-gate-flip-translates-leaves`) + корректно удалены из `scripts/mutation-gate.mjs` и нигде больше не + упоминаются (`grep` по всему дереву) — контракт §14 ТЗ выполнен, кроме CR-1. +- Трейлеры и changelog-требование `AGENTS.md`/PROCESS.md — оба коммита + корректны. + +## Чего не проверял + +- Полный `npm run golden:verify` (77 different, 2 error, 3 missing — цифры + автора не перепроверены построчно): предрелизный гейт по ТЗ §13.3, локальный + accept всё равно запрещён; не влияет на вердикт code review. +- Полный `smoke_*` набор (167 смоков) — только три выбранных вручную плюс + вывод `smoke-select` (НЕОПРЕДЕЛЁННОСТЬ) учтены; более широкий прогон не + оправдан 6-символьной дельтой в 3 файлах. +- `python -m pytest tests_backend` — backend/schema не в диффе (AC7 разобран + чтением). +- Расхождение «1113/1 skip» (автор) vs «1114/0 skip» (у меня) в `npm test` — + вероятно, разница окружения (Chromium/иное), не влияет на код; не + расследовалось глубже, так как оба результата green по существу задачи. + +## Вердикт + +Единственная находка — CR-1, Medium, в скоупе задачи. High нет. Продуктовое +поведение (offset, направление, jamb, parity, документация, changelog) +подтверждено и исполнением, и чтением, полностью соответствует ТЗ и решению +владельца. Возврат автору для точечного исправления `--test-name-pattern` в +`scripts/mutation-gate.mjs` и повторного прогона мутанта.