From 803c0f0eaab3aea1ea9a87215c35b638a679dafb Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sat, 26 Sep 2026 01:54:48 +0000 Subject: [PATCH] docs: review document for #651 Issue: #651 User-Visible: no --- docs/reviews/CODE-REVIEW-651-r2.md | 152 +++++++++++++++++++++++++++++ docs/reviews/INDEX.md | 3 +- 2 files changed, 154 insertions(+), 1 deletion(-) create mode 100644 docs/reviews/CODE-REVIEW-651-r2.md diff --git a/docs/reviews/CODE-REVIEW-651-r2.md b/docs/reviews/CODE-REVIEW-651-r2.md new file mode 100644 index 00000000..f1a747a0 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-651-r2.md @@ -0,0 +1,152 @@ +# CODE-REVIEW-651-r2 + +Материал раунда: `git log --oneline origin/dev..HEAD` / `git diff +c40266431ca1335ed09ebbaace9656b55ee84e23..HEAD`, SHA +`e73e56dd943675846fd3de91d98d55d1cbd7c148` (рабочая копия уже на нём). Один +коммит в дельте: `e73e56dd` (`test(2.5D): cover rigid group boundaries and +fallback`, `Issue: #651`, `User-Visible: no`). + +## Скоуп + +Дельта r1 → r2 — ровно два файла: `test/iso-overlays.test.mjs` (+60 строк, два +новых теста) и `scripts/mutation-registry.mjs` (+27 строк, два новых мутанта). +Продуктовый код (`src/iso-overlays.ts`, `src/iso-scene-render.ts`, +`src/houseplan-card.ts`), документация 2.5D и оба changelog **не менялись** — +это подтверждает и трейлер `User-Visible: no` на единственном коммите. +Дельта строго локальна (PROCESS.md §2.10): она закрывает ровно две находки +CODE-REVIEW-651-r1 и не задевает ничего, что было прочитано и принято в r1. +Разбор ниже ограничен AC1 и AC4 — единственными AC, которые эта дельта +задевает. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| Medium-1 — AC1 «другая комната исключена» не доказан ни тестом, ни мутацией | Новый тест `rigid overlay groups never join close markers owned by different rooms` (`test/iso-overlays.test.mjs:149`) ставит два близких значка по разные стороны общей стены в разные комнаты и требует `nudgeCss` разных знаков/значений; новый мутант `iso-rigid-groups-cross-room-boundary` (`scripts/mutation-registry.mjs`) снимает точно ту же проверку `owner.id`, что была найдена в r1 (`src/iso-overlays.ts:1738`) | Перечитал патч и воспроизвёл вручную: применил патч мутанта к рабочей копии, тест **упал** (`AssertionError: the right-room marker clears the shared wall into its own room`); откатил патч — тест снова зелёный. Тот же мутант зелёно пойман в CI (`Мутанты по диффу 1–6/6`, run 36209228318) | +| Medium-2 — AC4 «деградированный fallback группы» не доказан ни тестом, ни мутацией | Новый тест `rigid fallback prioritizes room, then wall, then overlap inside the 48px cap` (`test/iso-overlays.test.mjs:169`) проверяет `status`/`reason`/`nearWallAfter`/`nudgeDistanceCss` деградированного размещения; новый мутант `iso-rigid-fallback-drops-room-wall-overlap-priority` вырезает найденную в r1 цепочку приоритетов `roomViolations → wallViolations → overlapPenalty` в `rigidFallbackOrder` (`src/iso-overlays.ts:1690`), оставляя только сортировку по расстоянию | Воспроизвёл вручную: применил патч мутанта, тест **упал** (`expected: 'overlay-collision', actual: 'nudge-cap'` — без приоритета алгоритм выбирает более короткий, но упирающийся в 48 px кандидат вместо кандидата, сохраняющего стену/комнату); откатил патч — тест снова зелёный. Тот же мутант зелёно пойман в CI | +| Low — неточная формулировка «проверяется существующими Linux golden-сценами» в комментарии автора к реализации | Снята ревьюером r1 без правки («не меняет вердикт и не требует действий») | `docs/reviews/CODE-REVIEW-651-r1.md`, раздел «Low» — решение зафиксировано, новых действий в r2 не требовалось и не предпринималось | + +Обе Medium-находки были в скоупе задачи (собственный тестовый план §10.1 уже +одобренного ТЗ) — не отдельные issue. + +## Унаследовано из r1 (без повторной проверки) + +Продуктовый код не менялся между `c4026643` и `e73e56dd`, поэтому следующее +принято из `docs/reviews/CODE-REVIEW-651-r1.md` (материал: SHA `c4026643`, +дерево `7afb4dcc4787...`) без повторного разбора: + +- **AC2/AC3** (один вектор на группу, инвариантность к zoom/pan/remount) — + подтверждено в r1 чтением, unit-тестами и двумя браузерными смоками + (`desktopZoomKeepsOverlayScene`, `touchPinchKeepsOverlayScene`). +- **AC5/AC6** (desktop и touch/kiosk сценарии) — те же браузерные смоки, r1. +- **AC7** (визуальный контракт) — полный `golden:verify` (245 сценариев) прогнан + и разобран вручную в r1; 5 ожидаемых `different`, новый baseline не принят + (верно по AC7). Дельта r1→r2 не трогает рендер, поэтому golden не + перегонялся повторно. +- **AC8** (перформанс) — оба профиля прогнаны на точном SHA `c4026643` в r1; + дельта r1→r2 не меняет исполняемый код в горячем пути. +- **AC9** (отсутствие побочных изменений: room labels, vacuum, glow, + проёмы, Flat, редакторы) — подтверждено чтением в r1. +- Три копии бандла (`dist/`, `custom_components/.../frontend/`, + `demo/srv/assets/`) синхронны — подтверждено в r1; дельта r1→r2 не трогает + `src/**`, пересборка не требовалась. +- Трейлеры и changelog исходного функционального коммита `4ab7ecb4` — + проверены в r1. + +## Как проверялось (гейты этого раунда) + +Validate на точном SHA `e73e56dd` зелёный — +https://github.com/Matysh/houseplan-card/actions/runs/36209228318 (проверено +`gh run view`: `headSha` совпадает, `conclusion: success`). Это подтверждает +дешёвые гейты и все 6 шардов «Мутанты по диффу», включая оба новых мутанта +#651. Дифф раунда не трогает `src/**` за пределами `scripts/mutation-registry.mjs` +(не продуктовый код) и не меняет демо/рендер — golden, browser-смоки, +perf-профили и `check-docs.mjs` из r1 остаются в силе без перегона. + +| Гейт | Статус | Кем/чем подтверждено | +|---|---|---| +| `npx tsc --noEmit` / test-build | 🟢 | прогнан лично (`tsc -p tsconfig.test.json && fix-test-build.mjs`), 0 ошибок; тот же прогон подтверждён Validate | +| `npm test` (полностью) | 🟢 (не дублировал целиком) | зелёный job «Фронтенд…» в Validate на этом SHA; лично прогнал целиком `test/iso-overlays.test.mjs` + `test/iso-scene-render.test.mjs` — 52/52 (совпадает с заявкой автора), и отдельно `test/mutation-gate.test.mjs` + `test/mutation-guard-outcome.test.mjs` — 89/89 | +| Мутационные свидетели r1 (2 новых) — «чем краснеет» | 🟢, проверено исполнением | вручную применил оба патча мутантов по очереди к рабочей копии, пересобрал test-build, прогнал целевые тесты — оба упали с ожидаемой ошибкой (см. таблицу закрытия выше); откатил патчи, рабочая копия чиста (`git status` пуст) | +| `npm run build` + сверка 3 копий бандла | не прогонял отдельно | дифф раунда не трогает `src/**`; сборка не может измениться, и Validate это же подтверждает (задача «синхрон бандла» в общем job) | +| `node scripts/check-docs.mjs` | не прогонял | дифф раунда не трогает `src/**` | +| Browser smoke / golden / perf | не перегонял | дифф раунда не меняет исполняемый продуктовый код и рендер; актуальны прогоны r1 (см. «Унаследовано») | +| `node scripts/smoke-select.mjs` | не прогонял | нет нового диффа в `src/**`/демо между r1 и r2; смоки уже отобраны и разобраны в r1 для функционального коммита | +| `npm run invariants` | не прогонял | геометрия/конфигурация не меняются (как и в r1) | +| `python -m pytest tests_backend` | не прогонял | нет диффа `custom_components/**/*.py` | + +## Находки + +Нет. Обе Medium из r1 закрыты именно предписанным способом — новым тестом, +доказавшим падение через названную мутацию с показанным результатом прогона +(требование «чем краснеет», REVIEWER.md), а не переформулированы и не сняты +как «не требуется». High-находок не было и не появилось. + +## Что проверено и признано корректным + +- **Тесты действительно проверяют заявленное, а не самоочевидную геометрию.** + Прочитал оба новых теста построчно: первый использует общую стену между + двумя равными по размеру, но разными по `preferredRoomId` комнатами и + проверяет знаки/неравенство векторов сдвига; второй использует две комнаты + с идентичной геометрией (чтобы изолировать порядок приоритетов от + побочных эффектов формы комнаты) и проверяет конкретные поля деградации + (`status`, `reason`, `nearWallAfter`, `nudgeDistanceCss ≤ 48`). +- **«Тест умеет падать» — не по описанию автора, а по личному эксперименту.** + Оба мутанта применены к рабочей копии вручную (не полагался на CI как на + единственное доказательство), оба целевых теста упали с содержательной, а + не случайной ошибкой, оба отличаются от «просто assert false» — + диагностика показывает, что без защиты алгоритм выбирает геометрически + другое (неверное) размещение. +- **Мутанты синтаксически и структурно корректны.** `test/mutation-gate.test.mjs` + + `test/mutation-guard-outcome.test.mjs` (89/89) подтверждают, что оба новых + `guard` совпадают с реальными именами тестов и не ломают реестр. +- **Изоляция дельты.** `git diff c4026643..e73e56dd --stat` показывает только + `test/iso-overlays.test.mjs`, `scripts/mutation-registry.mjs` и + ранее опубликованный `docs/reviews/CODE-REVIEW-651-r1.md`/`INDEX.md` — + никакой продуктовый файл не тронут, что делает «унаследовано из r1» законным + сокращением объёма, а не догадкой. +- **Трейлеры.** Единственный коммит `e73e56dd` несёт `Issue: #651` и + `User-Visible: no` — верно, т.к. тестовый/registry-only коммит не меняет + видимое поведение, changelog не тронут заслуженно. +- **`dev` не ушёл вперёд.** `git merge-base origin/dev HEAD` = `ec2b8c15`, + тот же коммит, что был базой на момент r1 — ребейза не было, дельта не + расширена внешними изменениями. + +## Чего не проверял и почему + +- **Полный `npm test` целиком одним прогоном** — не дублировал; полагаюсь на + зелёный job Validate этого точного SHA плюс личный целевой прогон + задетых и registry-тестовых файлов (52 + 89 тестов). +- **`npm run build`/`bundle:sync`, `check-docs.mjs`, golden, browser smoke, + perf-профили** — не перегонял в этом раунде: дельта r1→r2 не трогает + `src/**`, демо или рендер, поэтому результаты r1 на функциональном коммите + `c4026643`/`4ab7ecb4` остаются в силе без повторного исполнения (объём + разбора по дельте, PROCESS.md §2.10). +- **`npm run invariants`, `pytest tests_backend`, `smoke-select.mjs`** — как и + в r1, не применимы: геометрия/конфигурация и Python-код не менялись ни в + функциональном коммите, ни в этой дельте. + +## Вердикт + +Обе Medium-находки r1 закрыты предписанным способом: новые тесты с явно +показанным падением при снятии именно той защиты, которая была найдена +отсутствующей, плюс зарегистрированные мутанты, зелёные в Validate на точном +SHA и лично воспроизведённые здесь. Дифф раунда локален, продуктовый код не +менялся, унаследованные из r1 доказательства (AC2/AC3/AC5–AC9, golden, +перформанс, трейлеры) остаются в силе. Новых находок нет. Вердикт — зелёный. + +Документ: `docs/reviews/CODE-REVIEW-651-r2.md` + +--- + + + +## Материал раунда + +- Ветка: `issue/651-iso-device-layout-stability`, коммит `e73e56dd9436` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `79735984216c7cb6d1a8f3bef707d3e990f912d0` + ``` + git log --all --format='%H %T' | grep 79735984216c + ``` +- Тело issue: `699e063899bf674ad775879fc1a0c889489a2f90b14fde467497787cc8f69442` +- Вердикт конвейера: `green` · High 0 diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index 7373ca71..7f982f31 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -1,12 +1,13 @@ # Индекс ревью -Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1067, issue: 376. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. +Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1068, issue: 376. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы | |---|---|---|---|---:|---:|---|---| | #651 | [SPEC-REVIEW-651-r1.md](SPEC-REVIEW-651-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | устаревшая формулировка «экспериментальный 2.5D-вид» противоречит текущему статусу функции | `CHANGELOG.md` `CHANGELOG.ru.md` `docs/ISOMETRIC.md` `docs/USER-GUIDE.ru.md` `docs/STATUS.md` `USER-GUIDE.ru.md` | | #651 | [SPEC-REVIEW-651-r2.md](SPEC-REVIEW-651-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — | | #651 | [CODE-REVIEW-651-r1.md](CODE-REVIEW-651-r1.md) | code · r1 | 🟡 жёлтый | 0 | 2 | AC1 «другая комната исключена» не доказан ни тестом, ни мутацией; AC4 «деградированный fallback группы» не доказан ни тестом, ни мутацией; неточная формулировка в комментарии к реализации (снято без правки) | `src/iso-overlays.ts` `test/iso-overlays.test.mjs` `scripts/mutation-registry.mjs` | +| #651 | [CODE-REVIEW-651-r2.md](CODE-REVIEW-651-r2.md) | code · r2 | 🟢 зелёный | 0 | 0 | — | — | | #650 | [CODE-REVIEW-650-r1.md](CODE-REVIEW-650-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | | #649 | [SPEC-REVIEW-649-r1.md](SPEC-REVIEW-649-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | в скоупе задачи (возвращается автору); принято ревьюером с записью, правки не требует | `lab.js` `houseplan-card.ts` | | #649 | [SPEC-REVIEW-649-r2.md](SPEC-REVIEW-649-r2.md) | spec · r2 | 🟢 зелёный | 0 | 0 | — | — |