diff --git a/docs/reviews/CODE-REVIEW-569-r1.md b/docs/reviews/CODE-REVIEW-569-r1.md new file mode 100644 index 00000000..ac538a6b --- /dev/null +++ b/docs/reviews/CODE-REVIEW-569-r1.md @@ -0,0 +1,183 @@ +# CODE-REVIEW #569 · r1 + +Материал: `542650234de6f2bc92f0a5a73ba9c20febbf6f42` (ветка +`issue/569-revive-dead-witnesses`, один коммит поверх `dev@7d08a28d`). +Трек: инфраструктурный (единственный изменённый файл — `scripts/mutation-registry.mjs`; +ни одного файла класса A). Заход r1, блокирующих циклов израсходовано 0 из 4. + +## Скоуп + +Ночной полный мутационный прогон по расписанию (2026-09-14, `dev@60f93a10`) +дал `726 из 735`: девять именованных свидетелей не дошли до заявленного теста — +все девять падали с исходом `ошибка подготовки до заявленного теста` +(`MUTATION_OUTCOME.SETUP`, см. `scripts/mutation-guard-outcome.mjs:15`), а не +красили тест реальным падением. Задача — вернуть каждому из девяти способность +доказывать себя, не меняя смысла мутации («что именно ломает мутант» из поля +`because`). + +Автор называет три независимых механизма отказа подготовки (не один, как +предполагала гипотеза из #566/#568): + +1. **Статически мёртвая ветка теряет сужение типов, сделанное выше** (4 шт.: + `strict-wall-barrier-accepts-degraded`, `version-recovery-auto-reloads-ordinary-view`, + `summary-index-rebuilds-on-state-value`, `room-drafts-mirror-still-throws`). + `if (false && …)` / `if (true) throw` — компилятор помечает ветку + недостижимой и не удерживает предшествующее сужение (`united` после + `if (united == null) continue;`, `_relation` до варианта `mismatch`, + `previous` после проверки непустоты, остаток функции после безусловного + `throw`). Лечится условием, ложным/истинным в рантайме, но не статически + (`String(x) === 'mutant-never...'`, `drafts.length >= 0`). +2. **Пустой литерал `[]` выводится как `never[]`** (2 шт.: + `space-copy-drops-existing-partitions`, `wall-draw-local-neighbour-dropped`) — + падают не сами мутанты, а их вызывающие (`Property 'id' does not exist on + type 'never'`). Лечится сохранением типа при потере содержимого + (`geometryList(...).slice(0, 0)`, `[] as string[]`). +3. **Несовпадение типов в самой замене** (3 шт.: `lattice-unknown-fields-recursive`, + `resize-audit-resolver-bypassed`, `vacuum-route-warning-stays-silent`) — + `canonicalizeNumber` объявлена `unknown`, функция обязана вернуть `T`; подмена + резолвера литералом сузила union и увела ветку `resolution.reason` в `never`; + бракованная строковая метка `case` ломала `switch` по union (`TS2678`). + Лечится приведением типа (`as T`), сохранением вызова резолвера с + перекрытием только вердикта (`{ ...resolveSafeResize(...), enabled: true } + as SafeResizeResolution`) и возвратом валидных меток соответственно. + +## Как проверялось + +Дешёвые гейты на этом SHA уже подтверждены зелёным Validate +(https://github.com/Matysh/houseplan-card/actions/runs/34815306997), поэтому +`npx tsc --noEmit`, `npm test` целиком и `npm run build` не перегонялись. + +Прогнано мной лично: + +- `node scripts/mutation-gate.mjs --check` — **738 определений, 0 FAIL** + (совпадает с заявленным автором числом). +- Каждый из девяти именованных свидетелей отдельно, `node scripts/mutation-gate.mjs + --id=` (это и есть единственный настоящий гейт этой задачи — не покрыт + Validate, потому что мутация не гоняется в обычном CI): + + | id | результат | + |---|---| + | `strict-wall-barrier-accepts-degraded` | `ok … заявленный тест покраснел на мутанте` — поймано 1 из 1 | + | `version-recovery-auto-reloads-ordinary-view` | то же — поймано 1 из 1 | + | `lattice-unknown-fields-recursive` | то же — поймано 1 из 1 | + | `room-drafts-mirror-still-throws` | то же — поймано 1 из 1 | + | `space-copy-drops-existing-partitions` | то же — поймано 1 из 1 | + | `summary-index-rebuilds-on-state-value` | то же — поймано 1 из 1 | + | `resize-audit-resolver-bypassed` | то же — поймано 1 из 1 | + | `vacuum-route-warning-stays-silent` | то же — поймано 1 из 1 | + | `wall-draw-local-neighbour-dropped` | то же — поймано 1 из 1 | + + Все девять перешли от `ошибка подготовки` к настоящему красному прогону + (`заявленный тест покраснел на мутанте`) — это именно то, что задача обещает. + Дисциплина «тест умеет падать» подтверждена исполнением, не чтением: сам + `--id=` запуск и есть демонстрация падения на мутированном коде. +- Чтение диффа построчно (все 9 хунков `scripts/mutation-registry.mjs`) — + сверил каждый комментарий-обоснование с фактическим патчем и с полем + `because` того же определения (см. «Разбор» ниже). +- `git diff origin/dev...HEAD --stat` — единственный файл, класс B + (`scripts/**`), продуктовый код (класс A) не тронут ни строкой. +- Трейлеры: `Issue: #569`, `User-Visible: no` — корректны (нет пользовательского + поведения), второй changelog не требуется и не тронут. + +Не прогонялись и почему: +- `check-docs.mjs` — дифф не трогает `src/**`. +- `npm run invariants` — дифф не трогает геометрию, `layout`, `marker.space`, + `open_spans`. +- `golden:verify` — дифф не меняет рендер. +- браузерные смоки — дифф не трогает `src/**`/`demo/**`; `smoke-select.mjs` не + применим (нет диффа во фронтенде). +- `pytest tests_backend` — дифф не трогает `custom_components/**/*.py`. +- Полный ночной прогон целиком — дорогой (шардированный, часы), не нужен: + задача проверяется поштучным `--id=` каждого из девяти названных свидетелей, + что и есть точное определение приёмки (ближайший ночной прогон подтвердит + это на реальном расписании, но это не гейт ревью). + +## Разбор + +Для каждого из трёх механизмов проверил, что лечение не меняет **смысл** +мутации (поле `because`), а только делает мутированный код компилируемым: + +- **Мёртвая ветка → рантайм-ложь.** `String(united.status) === 'mutant-never-degraded-extra'` + и аналоги гарантированно `false` в рантайме (значение никогда не равно + специально придуманной строке), но компилятор больше не видит литеральную + `false`, поэтому не сворачивает ветку в `never`. Наблюдаемое поведение + мутанта («ветка `degraded-extra` никогда не обрабатывается») сохранено + бит-в-бит. +- **`never[]` → сохранение типа при потере содержимого.** + `geometryList(source.partitions).slice(0, 0)` — вызов остаётся с правильным + типом элемента, но `.slice(0, 0)` гарантированно возвращает пустой массив: + ровно то же поведение, что и голый `[]`, только типизированное. + `[] as string[]` аналогично. +- **Приведение типа/резолвер/метка.** `resize-audit-resolver-bypassed` — самый + тонкий случай: резолвер теперь реально вызывается, и только его вердикт + перекрывается `enabled: true`. Автор честно оговорил в хендоффе, что слово + «bypassed» в id стало шире правды. Проверил поле `because` этого + определения (`scripts/mutation-registry.mjs:2174-2175`): текст не про + «резолвер не вызывается», а про то, что «всегда-включённый аудит делает + базовую линию бессмысленной» — это утверждение осталось точным, оно не + меняется. Расхождение только в имени id, а id — ключ ledger; автор + аргументированно не стал его переименовывать (переименование обнулило бы + накопленные доказательства). Не блокирует, отдельного Low не завожу — + автор уже назвал это явно и оставил решение ревьюеру: текст `because` не + требует правки, потому что он не утверждает ничего опровергнутого. +- `vacuum-route-warning-stays-silent` — заменил `case 'нет такого'` (заведомо + не входящий в union, ломавший сам `switch`) на исходные валидные метки с + `return null` вместо `return resolution.kind`. Смысл мутации («предупреждение + теряется молча») сохранён: функция теперь возвращает `null` вместо кода + предупреждения — то же наблюдаемое поведение, что описано в `because`. + +Все девять патчей — точечная правка только внутри своего `patches[].replace`; +`find`-якоря не менялись (кроме случаев, где это и есть исправление), поэтому +`--check` (738/0 FAIL) подтверждает, что реестр не разошёлся с деревом ни в +одном из 738 определений, включая не тронутые этой задачей. + +## Что проверено и корректно + +- Инфраструктурный трек подтверждён: диапазон `origin/dev...HEAD` содержит один + коммит, единственный файл — `scripts/mutation-registry.mjs`. +- Все девять свидетелей лично прогнаны через `--id=` и дают + `заявленный тест покраснел на мутанте` — приёмочное условие задачи выполнено + исполнением, не чтением. +- `--check` зелёный на всём реестре (738/0), правка не осиротила ни один + якорь, включая непричастные к задаче 729 определений. +- Каждое лечение проверено против поля `because` своего определения — смысл + мутации не размыт, изменился только способ гарантировать компилируемость. +- Трейлеры корректны: `Issue: #569`, `User-Visible: no`, второй changelog не + нужен. +- Явно названный owner'ом отказ от более общего решения (статический запрет + на `if (false …)`) обоснован исполняемой проверкой (#568: 45 живых мутантов + используют этот идиому) и не входит в скоуп этой задачи — не считаю это + недоработкой. + +## Чего не проверял + +- `npx tsc --noEmit`, `npm test` целиком, `npm run build` — зелёный Validate + на этом точном SHA уже подтвердил. +- Полный ночной мутационный прогон целиком (все ~735-738 мутантов, 4 шарда) — + дорогой, не входит в гейт ревью; приёмка задачи по определению — поштучный + `--id=` для девяти названных, что и прогнано. +- Остальные 45 мутантов с идиомой `if (false …)`, не входящие в список девяти + — они вне скоупа задачи (задача чинит только те, что реально красили ночной + прогон), их прогон не требуется для этого AC. + +## Вердикт + +Зелёный. Находок нет — High: 0, Medium: 0. Все девять свидетелей лично +перепроверены `--id=` и подтверждено, что каждый переходит от «ошибка +подготовки» к настоящему красному исходу на мутированном коде; реестр +остаётся согласованным целиком (`--check`: 738/0 FAIL). Инфраструктурный трек, +трейлеры корректны, класс A не тронут. + +--- + + + +## Материал раунда + +- Ветка: `issue/569-revive-dead-witnesses`, коммит `542650234de6` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `24c8aaaaf21d073e7036204940ce0d3cdf653de4` + ``` + git log --all --format='%H %T' | grep 24c8aaaaf21d + ``` +- Тело issue: `1d221cb3307af4063325876f6ea351b32f823d3c91363c98b9139e890fe3be03` +- Вердикт конвейера: `green` · High 0