docs: review document for #566

Issue: #566
User-Visible: no

docs/reviews/CODE-REVIEW-566-r1.md

@@ -0,0 +1,201 @@
+# CODE-REVIEW — issue #566, заход r1
+
+**Материал:** `f724cca61a9822d9e5780f0e171137f3c2c7fc40` (ветка
+`issue/566-stale-layout-notes`, три коммита: `dedcedc8`, `7ecdafef`, `f724cca6`).
+Рабочая копия уже стояла на этом SHA, `git fetch`/`checkout` не выполнялись.
+
+**Трек:** инфраструктурный (§1) — тронуты только `scripts/model-invariants.mjs`,
+`scripts/mutation-registry.mjs`, `test/model-invariants.test.mjs`,
+`test/geometry-corpus.test.mjs`. Ни одного файла класса A, ускоренный вход
+подтверждён.
+
+**Вердикт: жёлтый · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 1 (в скоупе)**
+
+## Скоуп
+
+Issue #566: `checkReferences` в `scripts/model-invariants.mjs` объявлял
+нарушением `references/layout_space` любую позицию `layout`, чьё пространство
+удалено — даже когда владелец позиции (комната/область/маркер) жив и Optimize
+(`src/space-reference-repair.ts`) намеренно хранит запись. Задача сужает
+правило: нарушение — только когда владельца тоже нет по конфигурации;
+остальное — наблюдение `stale_layout_space`.
+
+Коммиты `7ecdafef` и `f724cca6` — не часть замысла #566 напрямую, а реанимация
+трёх мутантов в `src/wall-thickness.ts`/`src/plan-optimizer.ts`, которых
+затянула в отбор правка `mutation-registry.mjs`, и которые ломались на этапе
+компиляции (детально разобрано в комментариях автора, заходы 2 и 3). Отдельный
+пробел («мутант, падающий на компиляции, неотличим от рабочего, пока его не
+выберет дифф») заведён автором как #568 — само собой не проверяется в этом
+ревью, я лишь отмечаю, что проверенное здесь не расширяет скоуп #566
+неоправданно: правка лежит в файле реестра, который #566 и так меняет.
+
+## Как проверялось
+
+Дешёвые гейты подтверждены Validate на этом SHA (run 34788430876, ссылка в
+issue) — `tsc --noEmit`, `npm test`, `npm run build` не перегонялись. Сверх
+этого, для защитных AC и для гейта `invariants`, обязательного при правке
+`layout`-ссылок, прогнано отдельно в песочнице ревью:
+
+| Гейт | Команда | Результат |
+|---|---|---|
+| Мутант «обвинить всех» | `node scripts/mutation-gate.mjs --id=invariants-blame-every-stale-position` | поймано 1 из 1 |
+| Мутант «простить пропавшего» | `node scripts/mutation-gate.mjs --id=invariants-forgive-a-vanished-position-owner` | поймано 1 из 1 |
+| 3 реанимированных мутанта (#568) | `--id=inner-span-reads-whole-edge-thickness`, `--id=safe-resize-legacy-midpoint-fail-open`, `--id=optimizer-micro-interval-cleanup-disabled` | все «поймано 1 из 1» |
+| Структура реестра | `node scripts/mutation-gate.mjs --check` | exit 0, 732 записи `ok`, совпадает с заявленным |
+| `npm run invariants -- --config test/fixtures/560-corpus/c6-stale-layout.json` | CLI end-to-end на реальной фикстуре #560 | exit 0, «Инварианты выполнены… Наблюдений (не нарушения): 2» |
+| Выбор смоков | `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | «Исполняемого frontend-диффа нет», смоки не выбираются — `src/**` не тронут |
+
+`check-docs`, golden, backend pytest, performance-профили не запускались:
+`src/**` не тронут, рендер и питон-бэкенд не задеты, ничего перфочувствительного
+в диффе нет. Это назывался объём необходимого самим диффом, а не пропуск.
+
+## Находки
+
+### Medium (в скоупе) — «наблюдение» ложно утверждает «владелец жив» там, где владелец не подтверждён
+
+`scripts/model-invariants.mjs:213-231`. Новая ветка судит владельца позиции на
+удалённом пространстве по трём случаям (`rl_`/`grp_`/маркер), но только для
+`rl_` и `grp_` реально ДОКАЗЫВАЕТ существование владельца по конфигурации
+(`allRoomIds`/`allAreas` — множества по всем пространствам, факт, а не догадка).
+Для остальных ключей (нет `rl_`/`grp_` — это владелец-маркер или ключ
+устройства HA) код проверяет только `removedMarkerIds.has(key)`. Если ключ не
+в `removedMarkerIds`, это накрывает ДВА разных случая: (а) ключ — живой маркер
+(подтверждено конфигом) и (б) ключ вообще не найден ни в каком маркере —
+ровно тот случай, для которого пятью строками ниже, в ветке `unknown_owner`
+(живое пространство), уже есть честная формулировка «владелец не найден в
+конфигурации (возможно устройство HA)». Для мёртвого пространства оба случая
+(а) и (б) получают одинаковый `detail`: **«владелец жив — Optimize хранит
+позицию намеренно»** — то есть утверждение о доказанности там, где
+доказательств нет, ровно тот стиль ложного сигнала, который сама задача
+исправляет для другого случая (#566 body: «инструмент… отвечает "нет" по
+причине, которую положено игнорировать»; тот же принцип работает в обратную
+сторону — не утверждать то, чего не знаешь).
+
+Продукт (`src/space-reference-repair.ts:329-388`) для этого же случая ведёт
+ТРИ состояния, не два: `live` (маркер активен), `absent` (маркер снят),
+`unverified` (владелец не резолвится вообще — `reason: 'unknown_owner'` или
+`'registry_unavailable'`). Автор в комментарии к задаче прямо ссылается на эту
+трёхчастную модель как на источник правды («Значит дефект целиком на стороне
+инструмента… для мёртвого пространства это рассуждение просто не
+применялось»), но новая реализация воспроизводит только грань
+absent/не-absent, а внутри «не-absent» смешивает `live` и `unverified` под
+одной надписью.
+
+**Воспроизведено исполнением** (импорт функции напрямую, не мутация):
+
+```js
+const config = {
+  spaces: [{ id: 'sp1', rooms: [{ id: 'r1', area: 'kitchen' }] }],
+  markers: [{ id: 'm_live', removed: false }],
+};
+const layout = {
+  'unknown_device_id_1234': { s: 'dead_space' },  // никогда не был маркером, мёртвое пространство
+  'unknown_device_id_5678': { s: 'sp1' },          // тот же владелец, живое пространство
+};
+```
+Результат: `unknown_device_id_5678` (живое пространство) → `unknown_owner`,
+detail «владелец не найден в конфигурации (возможно устройство HA)» — честно.
+`unknown_device_id_1234` (мёртвое пространство) → `stale_layout_space`, detail
+«пространства не существует, владелец жив — Optimize хранит позицию
+намеренно» — то же самое неизвестное состояние владельца, но текст утверждает
+обратное тому, что известно.
+
+Тест задачи (`test/model-invariants.test.mjs`, кейс с ключом
+`'980f1446c4ec1a3a9fa9ff5f6d93caed'`) сам этот случай заводит и в комментарии
+признаёт неразличимость («это устройство HA либо мусор, отличить нельзя»), но
+проверяет только принадлежность к `kind: 'stale_layout_space'` и что ПЕРВАЯ (по
+порядку ключей) запись из четырёх несёт нужный `detail` — этой первой
+оказывается `rl_r1` (истинно живая комната), поэтому асимметрия тестом не
+ловится.
+
+**Почему Medium, не High:** это не ломает AC issue (нарушения не заводятся,
+код возврата CLI остаётся 0, дефект #566 закрыт) и не открывает регрессию в
+существующем контракте — это неточность **внутри новой** функциональности,
+которую сам диф вводит. Влияние — на доверие к тексту диагностики: человек,
+читающий вывод `npm run invariants` на реальном конфиге (ровно сценарий этой
+задачи), получит уверенное «владелец жив» для записи, чей владелец на самом
+деле не установлен.
+
+**Правка в скоупе задачи** (#202, §2.7): различить `live` (маркер в
+`activeMarkerIds`, множество уже вычислено в начале функции) от `unverified`
+(ключ не резолвится ни как `rl_`/`grp_`/маркер) — вторым `kind` или полем
+`reason`, как это уже сделано для `unknown_owner` в живом пространстве.
+
+### Проверено и не вызывает вопросов
+
+- **`rl_`/`grp_` классификация в мёртвом пространстве.** `allRoomIds`/`allAreas`
+  собираются по ВСЕМ пространствам (а не только исходному), что на первый
+  взгляд похоже на риск коллизии id между несвязанными комнатами/областями —
+  но это ровно тот же приём, что использует сам продукт: `existingRoomIds` в
+  `space-reference-repair.ts:148` строится идентично (по всем пространствам),
+  а `roomsByArea` (там же, строка 125) явно допускает несколько кандидатов на
+  одну область и намеренно не резолвит неоднозначность
+  (`uniqueAreaRoom` возвращает `null` при >1 кандидате). Комнатные id
+  генерируются как `` `r${Date.now().toString(36)}-${index}` ``
+  (`src/houseplan-editor-runtime.ts:6975`), так что практическая коллизия
+  ничтожно маловероятна. Отдельной находки не завожу: это соответствует
+  установленному в продукте поведению, а не новый риск диффа.
+- **Мутанты #566** (`invariants-blame-every-stale-position`,
+  `invariants-forgive-a-vanished-position-owner`) — оба прогнаны лично,
+  «поймано 1 из 1» подтверждено, а не принято на слово.
+- **Реанимация трёх мутантов** (#568-приём: статически мёртвая ветка теряет
+  сужение типов TS) — прогнаны лично, поведение мутанта не изменилось по
+  смыслу (условие по-прежнему ложно в рантайме), только перестало быть
+  статически недостижимым.
+- **`mutation-gate --check`** — 732 записи, структура реестра (один `find` на
+  файл) не нарушена.
+- **`npm run invariants` на корпусе #560** — сквозной CLI-прогон (не только
+  юнит-тест) подтверждает исход: 0 нарушений, 2 наблюдения с названной
+  причиной.
+- **Трейлеры.** Все три коммита несут `Issue: #566` и `User-Visible: no` —
+  верно: изменение не видно пользователю, changelog не требуется.
+- **Тест `test/geometry-corpus.test.mjs`** корректно расширен: `violations()`
+  теперь принимает `notes`, оба вызова (`checkReferences`, `checkWallKeys`)
+  прокидывают заранее существовавший параметр `{ notes }`, фикстура
+  `c6-stale-layout` заявляет и нарушения (`[]`), и наблюдения (два
+  `references/stale_layout_space`) — раньше наблюдения корпус не сверял вовсе.
+- **`test/model-invariants.test.mjs`**, новый тест по семи записям на мёртвом
+  пространстве покрывает все три «нарушение»-случая (комната/область/снятый
+  маркер отсутствуют) и корректно требует, чтобы наблюдение называло причину
+  (`assert.match(..., /Optimize хранит позицию намеренно/)`) — сам этот assert,
+  впрочем, не различает «живой» и «неверифицированный» случаи (см. находку
+  выше).
+- Прежний тест #252 пересмотрен честно (запись `ghost` перекласс­ифицирована
+  в наблюдение с объяснением), не подогнан молча.
+
+## Чего не проверял
+
+- `tsc --noEmit`, `npm test`, `npm run build` со сверкой бандла — не
+  перегонял: зелёный Validate на этом же SHA уже есть (run 34788430876),
+  перегон был бы тратой бюджета раунда без нового сигнала.
+- `check-docs`, golden, `pytest tests_backend`, performance — не запускал:
+  `src/**` и `custom_components/**/*.py` не тронуты, рендер не задет.
+- Полный отбор 94 мутантов, которые дифф затягивает в CI (`selectForDiff`) —
+  проверил только те 5, что относятся к содержанию этой задачи и её побочной
+  правке реестра; на остальные полагаюсь на зелёный Validate CI (шардированный
+  прогон), как и разрешает соразмерность гейта.
+- Полевой конфиг владельца (упомянутый в issue, 4 записи на 2 пространствах)
+  недоступен ревьюеру — использовал синтетическую фикстуру `c6-stale-layout`
+  из репозитория, которую подтвердил как реальный CLI-прогон, а не только как
+  юнит-тест.
+
+## Что дальше
+
+Возврат автору (`S6-in-progress`): единственная Medium-находка в скоупе задачи
+и решается в этом же issue — различить `live`/`unverified` в ветке
+`stale_layout_space`, по образцу уже существующего `unknown_owner`. High нет,
+цикл не исчерпан (0/4 израсходовано после этого раунда → 1/4).
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `issue/566-stale-layout-notes`, коммит `f724cca61a98` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `b05238054e4015f6f558b9c563d211d0749385e9`
+  ```
+  git log --all --format='%H %T' | grep b05238054e40
+  ```
+- Тело issue: `b30fafcfa79bb1bfe11625206ac445bb83a257f0b821202f064d1c4cfdcb2dd7`
+- Вердикт конвейера: `yellow` · High 0