docs: code review r2 for optimizer micro-interval cleanup (#198)

Issue: #198
User-Visible: no

docs/reviews/CODE-REVIEW-198-r2.md

@@ -0,0 +1,248 @@
+# Код-ревью issue #198 — очистка изолированного микро-интервала толщины через Optimize (цикл r2)
+
+- Цикл: r2/4
+- Вердикт: **жёлтый** · High: 0 · Medium: 1 (в скоупе задачи)
+- Диапазон: `git log --oneline origin/dev..HEAD` = `9d6cd8b` (ТЗ), `8c9241b`
+  (ревью ТЗ r1), `0df8051` (реализация), `f4ab0b3` (ревью-документ r1),
+  `1c9575a` (follow-up тесты на замечания r1), `0625b6f` (обновление
+  provenance скриншотов после изменения `src/`)
+- Диапазон диффа: `git diff origin/dev...HEAD`
+- ТЗ: [`docs/specs/198-optimize-micro-interval.md`](../specs/198-optimize-micro-interval.md)
+  (ревью ТЗ зелёное, r1)
+- Предыдущий цикл: [`docs/reviews/CODE-REVIEW-198-r1.md`](CODE-REVIEW-198-r1.md)
+  (жёлтый · Medium-1 AC5, Medium-2 AC8 — обе «доказательство обещанным
+  способом отсутствует», не дефект поведения)
+
+## Скоуп
+
+Тот же, что в r1: явное действие «Общие настройки → Оптимизировать планы»
+получает строго ограниченный lossy-шаг — схлопывание одиночного
+изолированного интервала толщины стены короче половины шага сетки между
+двумя одинаковыми коллинеарными соседями, при отсутствии room-vertex/
+opening-узла на его концах. Runtime и редактор остаются lossless.
+
+Этот цикл — не новая функциональность, а закрытие двух Medium-находок r1:
+- **AC5**: добавлен прямой unit `collapseIsolatedWallThicknessIslands()` на
+  реверс endpoints, перестановку `walls`, перестановку/реверс winding
+  `rooms`, неизменность входов (`test/plan-optimizer.test.mjs`).
+- **AC8**: добавлен прямой regression unit для `normalizeWallIntervals()` и
+  `degradeWalls()` на изолированном `22→15→22`-профиле вне Optimize
+  (`test/wall-thickness.test.mjs`, ранее не тронут диапазоном).
+
+Дополнительно найден и учтён коммит `0625b6f` — он появился на ветке уже
+после старта этого цикла ревью (см. «Как проверялось»): обновляет
+`docs/images/screenshots.json` + один PNG (`06-device-editor.png`,
+отличие на 1 байт) вслед за изменением `sourceFingerprint` `src/`, класс C,
+`User-Visible: no`, не относится к продуктовому поведению #198.
+
+Продуктовый код (`src/plan-optimizer.ts`) в этом цикле не менялся —
+`git diff 0df8051..HEAD -- src/` пуст. Проверка была нужна только для
+двух добавленных тестовых файлов и одного docs-коммита.
+
+## Как проверялось
+
+Материал — актуальный tip ветки. Важно: сессия ревью изначально была
+развёрнута на коммите `1c9575a`, но `git fetch` показал, что ветка
+`issue/198-optimize-micro-interval` к моменту ревью уже продвинулась на
+`0625b6f` (owner исправил red `docs`-job). Переключился на
+`origin/issue/198-optimize-micro-interval` перед тем, как выносить
+вердикт, — иначе ревью судило бы устаревший срез.
+
+| Гейт | Результат | Команда |
+|---|---|---|
+| typecheck | green | `npx tsc --noEmit` |
+| unit | green, 917/917 (было 915/915 в r1, +2 новых теста) | `npm test` |
+| build + 3 копии бандла | green, побайтово идентичны | `npm run build && cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js && cmp dist/houseplan-card.js demo/srv/assets/houseplan-card.js` |
+| targeted smoke (AC7) | green, все 10 подпроверок | `node demo/smoke_optimize_micro_interval.mjs` |
+| mutation-gate, полный self-check | green, 36/36 | `node scripts/mutation-gate.mjs --check` |
+| mutation-gate, целевой мутант (AC9) | мутант пойман 1/1 | `node scripts/mutation-gate.mjs --id=optimizer-micro-interval-cleanup-disabled` |
+| docs-gate (затронут `0625b6f`) | green | `node scripts/check-docs.mjs --external` |
+| whitespace | чисто | `git diff --check origin/dev...HEAD` |
+| **адверсариальная проверка новых AC5/AC8-тестов мутацией кода** (см. находку ниже) | AC8-тест ловит регрессию; AC5-тест — нет для одного из сценариев | см. «Находки» |
+
+**Не прогонялось и почему (без изменений относительно r1 — диапазон с r1
+по продуктовому коду и UI-поверхности не менялся):**
+- `npm run golden:verify` и смежные `smoke_grid_snap`/`smoke_align_guides` —
+  этот цикл не трогает `src/plan-optimizer.ts`, рендер, геометрию,
+  стили или UI-пайплайн Optimize; изменились только `test/*.mjs` и
+  provenance двух docs-артефактов. Повторный прогон той же UI-поверхности
+  без изменений в ней добавил бы время без нового сигнала — уже
+  перепрогнан и подтверждён зелёным в r1 против того же кода.
+- `python -m pytest tests_backend -q` — ни один файл
+  `custom_components/**/*.py` не входит в диапазон ни в r1, ни в r2.
+- performance-профили — не названы в AC; изменения этого цикла — только
+  тесты и docs-provenance, нет тронутых perf-путей.
+- Полный набор из 127 browser-смоков — диапазон r2 не касается ни одной
+  новой UI-поверхности; единственный релевантный (`smoke_optimize_micro_interval`)
+  прогнан.
+
+### Проверка дисциплины «тест должен уметь падать» для двух новых юнитов
+
+Не ограничился чтением — реально мутировал код и убедился в результате,
+затем вернул рабочее дерево в чистое состояние (`git diff` пуст, `npm
+test` 917/917 после отката).
+
+**AC8-тест (`test/wall-thickness.test.mjs`, «lossless wall helpers preserve
+an isolated sub-half-step thickness island outside Optimize») —
+подтверждённо способен падать.** Внёс в `degradeWalls()` (`src/wall-thickness.ts`)
+однострочную мутацию входа (`walls[0].cm = 999`) сразу после guard `if
+(!walls?.length) return [];`. Прогон `npm test` дал 5 красных тестов,
+включая именно новый (`not ok 843 - lossless wall helpers preserve an
+isolated sub-half-step thickness island outside Optimize`), плюс два
+существующих соседних `degradeWalls`-теста и оба уже покраснели бы сами
+по себе — то есть новый тест не единственная защита, но он сам по себе
+дискриминативен для входной мутации. Откатил правку (`git checkout --
+src/wall-thickness.ts`), `npm test` снова 917/917.
+
+**AC5-тест (`test/plan-optimizer.test.mjs`, «micro-interval cleanup is
+endpoint, input-order and room-order invariant without mutation») — не
+ловит именно тот сценарий регрессии, который Medium-1 из r1 назвал
+целевым.** Подробности — находка ниже.
+
+## Находки
+
+### Medium-1 (в скоупе) — новый AC5-тест не хермитичен к сценарию входной мутации, который он должен доказывать
+
+**Файл:** `test/plan-optimizer.test.mjs`, тест «micro-interval cleanup is
+endpoint, input-order and room-order invariant without mutation» (строки
+127–159 в HEAD)
+
+**Суть:** тест сперва вызывает `baseline = collapseIsolatedWallThicknessIslands(
+[fixture.rooms[0], detached], fixture.walls, ...)` **на непроклонированном**
+`fixture.walls`/`fixture.rooms`, а затем строит `reversedMiddle` и итоговый
+`walls`, **читая** `fixture.walls[1]` уже **после** этого первого вызова
+(`{...fixture.walls[1], a: [...fixture.walls[1].b], b: [...fixture.walls[1].a]}`).
+Если бы `collapseIsolatedWallThicknessIslands` регрессировала ровно так,
+как предупреждал Medium-1 в r1 — начала мутировать входной массив `walls`
+на месте (`wall.cm = target` вместо иммутабельного `{...wall, cm:
+target}` внутри `out.map(...)` на строках 178–182 `src/plan-optimizer.ts`) —
+то `baseline`-вызов сам испортил бы `fixture.walls[1].cm` (15 → 22) ещё
+**до** того, как из него строится `reversedMiddle`/`walls` для проверки
+инвариантности. К моменту, когда тест снимает `beforeWalls =
+structuredClone(walls)`, входной центральный интервал уже был бы виден
+как `cm: 22`, кандидат схлопывания отфильтровался бы условием `centreCm
+=== leftCm` (строка 138 `src/plan-optimizer.ts` — уже «канонично», сливать
+нечего), второй вызов не тронул бы объект вообще, и
+`assert.deepEqual(walls, beforeWalls)` прошла бы тривиально, потому что
+мутировать было уже нечего.
+
+**Проверено исполнением, не только чтением.** Ввёл в
+`src/plan-optimizer.ts` минимальную адверсариальную мутацию — заменил
+`out = out.map((wall) => {... return wall.cm === target ? wall : {...wall,
+cm: target}; })` на цикл `for (const wall of out) { ...; wall.cm =
+target; }` (мутация того же самого объекта, на который ссылаются и
+`out`, и исходный `walls`, поскольку `out = walls.slice()` — мелкое
+копирование массива, не элементов). Прогнал `npm test`: **все 917 тестов
+остались зелёными**, включая AC5-тест. Отдельным изолированным
+node-скриптом против пересобранного `test-build/plan-optimizer.js`
+подтвердил механизм: `fixture.walls[1].cm` меняется с `15` на `22` сразу
+после `baseline`-вызова (для немутированного, реального кода —
+остаётся `15`, для мутированного — становится `22`). Значит именно
+описанный сценарий воспроизводится, а не является гипотетическим.
+
+**Сценарий отказа, который прошёл бы незамеченным:** будущий рефакторинг
+`collapseIsolatedWallThicknessIslands`, меняющий иммутабельный `.map` на
+мутацию на месте (естественная логическая ошибка при, например,
+попытке «оптимизировать» цикл), не будет пойман этим тестом — именно
+тем автотестом, который ТЗ и r1-ревью называют доказательством AC5.
+`test/wall-thickness.test.mjs`-тест того же follow-up-коммита для AC8
+устроен иначе (сравнивает итоговую переменную `walls` с её собственным
+`before`-снимком, снятым до **любых** вызовов) и корректно ловит тот же
+класс регрессии — см. проверку выше. Это подтверждает, что дефект
+специфичен для одного теста, а не общая недостижимая планка.
+
+**Почему Medium, а не Low:** это не стилистика — тест был написан
+специально в ответ на Medium-1 из r1 как «Reversal/permutation/
+immutability unit» для строгого продуктового инварианта неизменности
+персистентных данных (`docs/WALL-THICKNESS.md`), и для сценария, прямо
+названного в тексте самого Medium-1 r1 («если бы будущая правка начала
+мутировать переданный `walls`-массив на месте»). Регрессия именно этого
+класса останется незамеченной автотестами задачи.
+
+**Что нужно:** в начале теста один раз снять `structuredClone` со всего,
+что дальше используется как источник и для `baseline`, и для проверяемого
+вызова (например, клонировать `fixture` целиком перед первым вызовом,
+либо строить `reversedMiddle`/`walls`/`rooms` из независимой копии
+fixture, а не из объекта, уже прошедшего через `baseline`-вызов). После
+правки повторить именно ту адверсариальную мутацию, которой я проверял
+эту находку (иммутабельный `.map` → мутирующий `for`), и убедиться, что
+тест в таком варианте краснеет.
+
+### Medium-2 — нет (закрыта)
+
+Assertion из r1 подтверждена исполнением как реально дискриминативная
+(см. «Проверка дисциплины „тест должен уметь падать“» выше) — новых
+находок по AC8 нет.
+
+### Low — нет
+
+Находок уровня Low не выявлено.
+
+## Что проверено и корректно
+
+- **AC8 закрыт полностью и доказан заявленным способом.** Новый unit в
+  `test/wall-thickness.test.mjs` вызывает `normalizeWallIntervals()` и
+  `degradeWalls()` напрямую (без `collapseIsolatedWallThicknessIslands`,
+  то есть вне Optimize) на изолированном `22→15→22`-профиле короче
+  половины шага и подтверждает: обе lossless-функции не трогают его, вход
+  не мутируется. Дискриминативность подтверждена адверсариальной
+  мутацией (см. выше) — тест реально краснеет на регрессии.
+- **Продуктовый код не менялся с r1.** `git diff 0df8051..HEAD -- src/`
+  пуст — контракт §6 ТЗ, non-cascading snapshot, разрешение
+  неоднозначности через `targets.size === 1` и защиту topology-узлов из
+  r1 не требовалось перепроверять заново по существу; перепроверены
+  только заново пересобранные гейты (typecheck/test/build/mutation-gate/
+  targeted smoke), все зелёные на актуальном коде.
+- **Docs-provenance коммит `0625b6f` корректен и не расширяет скоуп.**
+  `sourceFingerprint` в `docs/images/screenshots.json` актуализирован под
+  изменившийся `src/` (тестовые правки этого же issue меняют fingerprint,
+  т.к. он покрывает весь `src/` + build inputs), `node scripts/check-docs.mjs
+  --external` зелёный, класс C, `User-Visible: no`, никакой продуктовой
+  логики не касается. 1-байтовое отличие PNG `06-device-editor.png`
+  проверено на невоспроизводимость по байтам: два последовательных
+  локальных прогона `npm run build && node demo/docs/capture.mjs`
+  дают разные SHA-256 даже для одного и того же коммита — это известная
+  нестрогая детерминированность самого capture-пайплайна (не связана с
+  #198), а `check-docs.mjs` проверяет только внутреннюю согласованность
+  манифеста и файла, а не побитовую воспроизводимость между запусками,
+  так что это не дефект.
+- **Idempotenность и mutation guard.** `node scripts/mutation-gate.mjs
+  --check` (36/36) и целевой `--id=optimizer-micro-interval-cleanup-disabled`
+  (1/1) зелёные на актуальном HEAD.
+- **Трейлеры.** Все пять новых/изменённых коммитов диапазона несут
+  `Issue: #198`; `User-Visible: no` на всех — корректно, ни один из них
+  (тесты, ревью-документы, docs-provenance) не меняет видимое поведение;
+  changelog-и не тронуты этим циклом и не должны были быть (правки
+  r1 уже внесли оба changelog в `0df8051`, `User-Visible: yes`,
+  подтверждено в r1 и повторно проверено — диапазон не менялся).
+- **Три копии бандла синхронны.** `cmp` дал совпадение бит-в-бит для
+  `dist/`, `custom_components/houseplan/frontend/` и `demo/srv/assets/`
+  после `npm run build` на актуальном HEAD.
+
+## Чего не проверял
+
+- Полный HA backend harness (`tests_backend` под настоящим Home Assistant) —
+  диапазон не содержит правок в `custom_components/**/*.py`.
+- `npm run golden:verify` и полный browser-smoke набор (127 файлов) —
+  диапазон r2 не трогает рендер/геометрию/UI-пайплайн; обоснование то же,
+  что в r1, и продуктовый код не менялся.
+- Производительность/бенчмарки — не названы в AC, изменения цикла — только
+  тесты и docs-provenance.
+- Повторный ручной прогон adversarial-мутаций на самом `exactMatches`
+  (реверс endpoints без общей мутации входа) — этот сценарий был
+  исполнен в r1 (см. `CODE-REVIEW-198-r1.md`, Medium-1) и один раз
+  дополнительно в этом цикле при диагностике находки выше; отдельно
+  третий раз не повторял, так как вывод не менялся между попытками.
+
+## Вывод
+
+High-находок нет. Одна Medium-находка — строго в скоупе этой же задачи
+(это тест, добавленный именно как ответ на Medium-1 из r1 для доказательства
+AC5, и находка о том, что он не доказывает заявленное для конкретного
+класса регрессии), поэтому решение не заводит отдельный issue, а
+возвращает задачу автору жёлтым вердиктом. Продуктовый код и AC8
+подтверждены; исправить нужно только конструкцию AC5-теста (устранить
+разделяемую немутированную ссылку на `fixture` между `baseline`-вызовом
+и построением проверяемых `walls`/`rooms`) и повторно прогнать `npm
+test` + адверсариальную мутацию `.map`→`for`, которой было
+продемонстрировано отсутствие защиты.