mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-04 13:48:57 +00:00
claude[bot]
@@ -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` не требуется —
|
||||
вердикт зелёный, к автору не возвращается.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/566-stale-layout-notes`, коммит `733cd5b86b30` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `7ce15af6d80e3c0746cc4399044e7e9e79da35de`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 7ce15af6d80e
|
||||
```
|
||||
- Тело issue: `b30fafcfa79bb1bfe11625206ac445bb83a257f0b821202f064d1c4cfdcb2dd7`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user