diff --git a/docs/reviews/CODE-REVIEW-566-r2.md b/docs/reviews/CODE-REVIEW-566-r2.md new file mode 100644 index 00000000..32380d5e --- /dev/null +++ b/docs/reviews/CODE-REVIEW-566-r2.md @@ -0,0 +1,181 @@ +# CODE-REVIEW — issue #566, заход r2 + +**Материал:** `733cd5b86b301fd29c1eddebd88d6b09cd231071` (ветка +`issue/566-stale-layout-notes`, один коммит поверх материала r1: `733cd5b8` +на `f724cca6`). Рабочая копия уже стояла на этом SHA, `git fetch`/`checkout` +на другой коммит не выполнялись. + +**Заход:** r2 · блокирующих циклов израсходовано 1 из 4 (r1 был жёлтым и +цикл потратил). + +**Трек:** инфраструктурный (§1), тот же, что в r1. Коммит `733cd5b8` трогает +только `scripts/model-invariants.mjs`, `scripts/mutation-registry.mjs`, +`test/geometry-corpus.test.mjs`, `test/model-invariants.test.mjs` — те же +четыре файла, что были в скоупе r1, новых файлов и подсистем дельта не +касается. Ребейза на ушедший вперёд `dev` не было (материал лежит прямо на +`f724cca6`, где его оставил r1), контракт поведения не менялся — менялась +только точность одной уже введённой этим же issue диагностики. Разбор веду +по дельте, а не заново. + +**Вердикт: жёлтый на входе r1 → зелёный на r2 · High: 0 · Medium: 0** + +## Дельта раунда + +`git diff f724cca6..733cd5b8` — 260 строк, 5 файлов (из них 201 — сам +документ ревью r1, которым эта дельта была вызвана, содержательных 59): + +- `scripts/model-invariants.mjs:213-244` — ветка `stale_layout_space` стала + трёхзначной: `owner` теперь `'absent' | 'live' | 'unverified'` вместо + булева `ownerGone`. `unverified` (ключ не резолвится ни как `rl_`, ни как + `grp_`, ни как маркер) получил собственный `kind: 'unknown_owner'` с той же + формулировкой, что уже использует ветка `unknown_owner` живого пространства + пятью строками ниже (`src/model-invariants.mjs:269-270`). +- `scripts/mutation-registry.mjs` — новый мутант + `invariants-claim-proof-for-an-unknown-owner` (сводит `unverified` к + `live`), плюс перепривязка `find`-якорей двух прежних мутантов #566 к + новым строкам (`owner === 'absent'`, `removedMarkerIds.has(key) ? 'absent'`). +- `test/model-invariants.test.mjs` — тест #566 теперь сверяет `owner` по + каждому из трёх `kind` отдельно (`stale_layout_space`/`unknown_owner`) и, + главное, сверяет ТЕКСТ `detail` для каждого, включая отрицательное + утверждение `assert.doesNotMatch(note.detail, /владелец жив/)` для + `unknown_owner` — именно эта проверка отсутствовала в r1 и пропустила + находку. +- `test/geometry-corpus.test.mjs` — фикстура `c6-stale-layout` меняет + ожидание с `['references/stale_layout_space', 'references/stale_layout_space']` + на `['references/unknown_owner', 'references/unknown_owner']`. + +## Закрытие раунда r1 + +| Находка r1 | Чем закрыта | Где это видно | +|---|---|---| +| Medium: наблюдение `stale_layout_space` утверждало «владелец жив» и для ключа, который не резолвится ни во что (только `removedMarkerIds.has(key)` проверялся, обратное трактовалось как `live`) | Введено третье состояние `unverified`: `removedMarkerIds.has(key) ? 'absent' : activeMarkerIds.has(key) ? 'live' : 'unverified'`. `unverified` получает `kind: 'unknown_owner'` с формулировкой «владелец не найден в конфигурации (возможно устройство HA); пространства тоже нет» — не утверждает то, чего не знает | `scripts/model-invariants.mjs:222-243`; воспроизведённый в r1 пример (`unknown_device_id_1234` на мёртвом пространстве) при повторной проверке ниже даёт `unknown_owner`, а не `stale_layout_space` | +| Указанный в r1 механизм пропуска: тест сверял только `kind`, не текст `detail`, поэтому асимметрия (первая по ключу запись случайно была живой комнатой) не ловилась | Тест переписан: `assert.deepEqual` по `owner` для каждого `kind` отдельно, `assert.match`/`assert.doesNotMatch` на `detail` внутри цикла по всем нотам своего вида, а не по первой найденной | `test/model-invariants.test.mjs:150-172`, дважды прогнан лично, зелёный | +| Предписанная r1 правка: различить `live`/`unverified` «по образцу уже существующего `unknown_owner`», используя `activeMarkerIds` | Использован именно `activeMarkerIds` (тот же Set, что вычислен в начале функции для живого пространства), формулировка `detail` дословно перекликается с формулировкой соседней ветки `unknown_owner` | `scripts/model-invariants.mjs:227,241-243` vs `scripts/model-invariants.mjs:269-270` | +| Побочный эффект находки: фикстура `c6-stale-layout` (реальный кейс из корпуса #560) — оба «сиротских» ключа там на самом деле незарегистрированные владельцы, а не подтверждённо живые маркеры, то есть сама фикстура иллюстрировала баг | `expectedNotes` фикстуры пересмотрены на `unknown_owner` вместо `stale_layout_space` — не подогнаны молча, тест явно проверяет новую классификацию на реальных данных | `test/geometry-corpus.test.mjs:253-256`; сквозной CLI-прогон ниже подтверждает | + +Мутант находки (`invariants-claim-proof-for-an-unknown-owner`) прогнан лично, +«поймано 1 из 1» — не принято на слово автора. + +## Унаследовано из r1 + +Без повторной проверки приняты пункты раздела «Проверено и не вызывает +вопросов» документа `docs/reviews/CODE-REVIEW-566-r1.md` (материал `f724cca6`), +поскольку дельта r2 их не касается: + +- классификация `rl_`/`grp_` по `allRoomIds`/`allAreas`, собранным по всем + пространствам, и сравнение с приёмом `space-reference-repair.ts:148` — + дельта не трогает эту ветку; +- сами мутанты #566 (`invariants-blame-every-stale-position`, + `invariants-forgive-a-vanished-position-owner`) как факт существования и + назначения — но их привязка к коду **перепроверена заново** в этом раунде + (см. «Как проверялось»), поскольку рефакторинг сдвинул их якоря; +- три реанимированных мутанта #568-приёма (`inner-span-reads-whole-edge-thickness`, + `safe-resize-legacy-midpoint-fail-open`, `optimizer-micro-interval-cleanup-disabled`) + в `src/wall-thickness.ts`/`src/plan-optimizer.ts` — файлы вне дельты r2, + не тронуты; +- трейлеры коммитов `dedcedc8`/`7ecdafef`/`f724cca6` (`Issue: #566`, + `User-Visible: no`) — не переоцениваю, дельта их не меняет; +- сквозной CLI-прогон на корпусе #560 как метод проверки (не только юнит-тест) + — метод унаследован, но **результат перепрогнан** на новом коде (см. ниже), + так как дельта прямо меняет вывод этой команды; +- отсутствие golden/check-docs/pytest/perf — `src/**` и + `custom_components/**/*.py` дельтой r2 тоже не тронуты, вывод не изменился. + +## Как проверялось + +Дешёвый Validate (tsc/test/build/bundle-ratchet, mutation-gate по шардам) на +`733cd5b8` подтверждён зелёным CI-прогоном (ссылка дана автором в issue), +поэтому не перегонялся целиком. Сверх этого лично прогнано в песочнице +ревью — то, что относится к точечной дельте r2, плюс сквозная проверка на +реальных данных: + +| Гейт | Команда | Результат | +|---|---|---| +| Новый мутант находки | `node scripts/mutation-gate.mjs --id=invariants-claim-proof-for-an-unknown-owner` | поймано 1 из 1 | +| Мутант «обвинить всех» (перепривязка) | `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 | +| Структура реестра | `node scripts/mutation-gate.mjs --check` | exit 0, 733 записи `ok`, `find`-якоря уникальны | +| Юнит-тесты дельты | `tsc -p tsconfig.test.json && node scripts/fix-test-build.mjs && node --test --test-name-pattern="#566\|#252\|#254" test/model-invariants.test.mjs test/geometry-corpus.test.mjs` | 8 + 26 тестов, всё зелёное | +| Полный `test/geometry-corpus.test.mjs` | `node --test test/geometry-corpus.test.mjs` | 26/26 зелёных | +| Инварианты сквозным CLI на фикстуре #560 | `node scripts/model-invariants.mjs --config test/fixtures/560-corpus/c6-stale-layout.json` | «Инварианты выполнены… Наблюдений (не нарушения): 2. 2 — позиции без записи маркера» (было «2 — позиции живых владельцев на удалённых пространствах» до правки) | +| Дословное совпадение `find`-строк трёх мутантов с текущим кодом | точечная проверка через `String.includes` на содержимом файла | все три совпали | +| Сводка по `kind` в CLI-выводе не хардкодит текст помимо словаря | чтение `scripts/model-invariants.mjs:790-798` (`noteSummary`) | подтверждено чтением: `titles[kind]` общий словарь, переклассификация транслируется без отдельной правки | + +`check-docs`, golden, backend pytest, performance-профили, полный отбор +мутантов (`selectForDiff`) не запускались — те же основания, что в r1: +`src/**` и `custom_components/**/*.py` не тронуты дельтой r2, рендер и +Python-бэкенд не задеты, ничего перфочувствительного в диффе нет; полный +отбор для дельты избыточен — все затронутые дельтой мутанты (3 из отбора) +прогнаны поштучно, остальные 91 из ранее выбранных 94 не касаются +изменённых строк и уже подтверждены зелёными шардами Validate в предыдущих +заходах. + +## Находки + +Нет. Правка `733cd5b8` делает ровно то, что предписал r1: вводит третье +состояние `unverified`, использует уже вычисленный `activeMarkerIds`, +дословно перекликается формулировкой с соседней веткой `unknown_owner`, и +закрывает механизм пропуска (тест теперь сверяет текст причины, а не только +вид). Побочная проверка (`noteSummary`, реальная фикстура #560) подтверждает, +что переклассификация корректно доходит до пользователя CLI без скрытых мест, +где старое двухзначное деление могло остаться зашитым. + +## Проверено и не вызывает вопросов + +- **Мутант находки r1** (`invariants-claim-proof-for-an-unknown-owner`) — + прогнан лично, «поймано 1 из 1», а не принят на слово. +- **Перепривязка двух прежних мутантов #566** к сдвинутым строкам — прогнаны + лично после рефакторинга, оба всё ещё «поймано 1 из 1»; `--check` + подтверждает уникальность и валидность новых `find`-якорей. +- **Тест `test/model-invariants.test.mjs`** — теперь различает три состояния + по `owner` и по тексту `detail` для каждого, включая отрицательную проверку + для `unknown_owner`; асимметрия из r1 таким тестом уже не проходит. +- **Фикстура `c6-stale-layout`** — пересмотрена не формально: два «сиротских» + ключа в ней (`c6-orphan-1`, `c6-orphan-2`) не зарегистрированы как маркеры + вообще, то есть по существу это и есть `unverified`, а не `live` — старое + ожидание фикстуры само было артефактом найденного в r1 дефекта. +- **Комментарии в коде** (`scripts/model-invariants.mjs:215-219, 237-240`) + называют находку r1 по существу («сказать «владелец жив» про ключ, который + ни во что не резолвится, значит заявить доказанность там, где её нет») — + не общие слова, а прямая привязка к причине правки. +- **Трейлеры коммита `733cd5b8`** — `Issue: #566`, `User-Visible: no`; верно, + правка меняет только текст диагностики CLI-скрипта, не продукт. +- **Один коммит-заход**, как требует процесс после возврата на доработку. + +## Чего не проверял + +- `tsc --noEmit`, `npm test` целиком, `npm run build` со сверкой бандла — + не перегонял: зелёный Validate на `733cd5b8` уже есть, дельта r2 не + расширяет класс изменений (по-прежнему только инфраструктурные файлы). + Полный `npm test` не гонял отдельно от полного `tsc -p tsconfig.test.json`, + который выполнил для сборки тестов — это тот же шаг, что и в CI. +- `check-docs`, golden, `pytest tests_backend`, performance — не запускал: + `src/**` и `custom_components/**/*.py` дельтой r2 не тронуты. +- Полный отбор 94 мутантов, которые дифф затягивает в CI (`selectForDiff`) — + как и в r1, проверил только те, что относятся к содержанию дельты этого + раунда (3: новый плюс два перепривязанных); на остальные полагаюсь на + зелёный Validate CI по всем шести шардам. +- Полевой конфиг владельца issue (4 записи на 2 пространствах) по-прежнему + недоступен ревьюеру — как и в r1, использовал фикстуру `c6-stale-layout` из + репозитория и подтвердил сквозным CLI-прогоном, а не только юнит-тестом. + +## Что дальше + +Задача закрыта: единственная Medium-находка r1 устранена по существу, новый +код доказан мутантом, тест больше не пропускает асимметрию, которая привела +к находке. High нет, Medium нет. Возврат в `S7-code-review` не требуется — +вердикт зелёный, к автору не возвращается. + +--- + + + +## Материал раунда + +- Ветка: `issue/566-stale-layout-notes`, коммит `733cd5b86b30` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `7ce15af6d80e3c0746cc4399044e7e9e79da35de` + ``` + git log --all --format='%H %T' | grep 7ce15af6d80e + ``` +- Тело issue: `b30fafcfa79bb1bfe11625206ac445bb83a257f0b821202f064d1c4cfdcb2dd7` +- Вердикт конвейера: `green` · High 0