diff --git a/docs/reviews/CODE-REVIEW-568-r1.md b/docs/reviews/CODE-REVIEW-568-r1.md new file mode 100644 index 00000000..a9179e4c --- /dev/null +++ b/docs/reviews/CODE-REVIEW-568-r1.md @@ -0,0 +1,170 @@ +# CODE-REVIEW #568 · r1 + +Материал: `264fba0902ff7522114979ac0fbc4e3422e8e3f5` (ветка +`issue/568-setup-failure-attribution`, один коммит поверх `dev@60f93a10`). +Трек: инфраструктурный (только `scripts/**`, `test/**`, `docs/TESTING.md` — +ни одного файла класса A). Заход r1, блокирующих циклов израсходовано 0 из 4. + +## Скоуп + +Issue #568 — устранить обнаруженный на #566 зазор: `setup-failure` свидетеля +красит гейт той задачи, чей дифф его выбрал, даже когда причина лежит в чужом +коммите. Решение владельца (комментарий 2026-09-14): отказ подготовки в +дифф-режиме атрибутируется прогоном того же мутанта на дереве базы диапазона; +предсуществующий отказ гейт задачи не красит, но называется машиночитаемой +строкой и обязан покраснеть в ночном полном прогоне (адресат — #472). Пункты 1 +и 3 из тела issue автор снял в S2-анализе (см. комментарий от 2026-09-14 +05:34) — оба обоснования проверены исполнением (реальный ночной прогон, grep +по 45 живым мутантам с `if (false …)`), возражений нет. + +## Как проверялось + +Дешёвые гейты на `264fba09` уже подтверждены зелёным Validate +([run](https://github.com/Matysh/houseplan-card/actions/runs/34811726621)), +поэтому `npx tsc --noEmit`, `npm test` целиком и `npm run build` не +перегонялись. Прогнано мной лично, целевым образом по дельте: + +- `node --test test/mutation-guard-outcome.test.mjs` — 16/16 green (7 новых + тестов `#568`, исполнение, не regexp: проверяется `ran === ['BASE']` через + подмену `registryOf`/`run`). +- `node --test --test-name-pattern="#558" test/mutation-gate.test.mjs` — 4/4 + green, включая тест границы модулей («registry, selection, evidence and + execution stay behind bounded module boundaries»): `mutation-gate.mjs` 273 + строки (< 300), `mutation-execution.mjs` 238 (< 250) — совпадает с числами, + которые автор назвал в хендоффе. +- `node scripts/mutation-gate.mjs --check` — 738 определений, 0 FAIL (у автора + в хендоффе названо 736 — расхождение на 2, см. «Находки», не блокирует). +- Демонстрация «тест умеет падать» на всех трёх новых мутантах и на + переанкеренном `ledger-records-escaped` — каждый через + `node scripts/mutation-gate.mjs --id=`, результат **«поймано 1 из 1»** + у каждого: + - `attribution-judges-the-head-definition-on-the-base` + - `attribution-lets-a-new-witness-off` + - `attribution-still-reddens-a-foreign-failure` + - `ledger-records-escaped` (якорь сдвинулся при рефакторинге `mutation-gate.mjs`) +- `node scripts/process-gate.mjs --issues` — «гейт пройден, предупреждений 1» + (единственное предупреждение — ожидаемое: инфраструктурный диапазон без + статусной метки класса A, п.8 корректно её не требует). +- Чтение кода: полный дифф `mutation-gate.mjs`, `mutation-attribution.mjs`, + `mutation-execution.mjs`, `mutation-guard-outcome.mjs`, + `mutation-registry.mjs`, `docs/TESTING.md` — построчно, не по диагонали. + +Не прогонялись и почему: `npx tsc --noEmit`/`npm test`(целиком)/`npm run +build` — уже зелёные на этом SHA (Validate); `check-docs` — диф не трогает +`src/**`; `npm run invariants` — диф не трогает геометрию; `golden:verify` — +диф не меняет рендер; `pytest tests_backend` — диф не трогает +`custom_components/**/*.py`; браузерные смоки — диф не трогает `src/**`, +`scripts/smoke-select.mjs` тут не применим (нет соответствующих входов). + +## Разбор логики (`scripts/mutation-gate.mjs`) + +Прочитан построчно узел «SETUP → атрибуция → преExisting»: + +```js +if (outcome.kind === MUTATION_OUTCOME.SETUP) { + const verdict = await attributeSetupFailure(entry.mutant, outcome, rangeBase); + if (verdict === 'pre-existing') { ...; preExisting.push(entry.mutant.id); continue; } + if (verdict === 'introduced') { ... } +} +if (outcome.kind !== MUTATION_OUTCOME.SURVIVED) unverifiable = true; +``` + +Ключевое: ветка `pre-existing` уходит через `continue` **до** строки +`unverifiable = true`, то есть предсуществующий отказ действительно не красит +`unverifiable` (и, значит, не тянет `return 2`) и не входит в +`toRun.length - preExisting.length` при финальном сравнении. Любой другой +исход (`verdict === null` — «сказать нечего», `verdict === 'introduced'`) +падает в старую ветку и красит гейт как раньше. Проверил это чтением и +демонстрацией трёх мутантов выше, а не только по тексту комментария в коде. + +`rangeBase` вычисляется только внутри `if (changedArg)` — в полном прогоне +(`--changed` отсутствует) он остаётся `null`, `attributeSetupFailure` сразу +возвращает `null` (`if (!rangeBase) return null;`), и поведение полного +прогона не меняется ни на строку — соответствует требованию «атрибуция только +в дифф-режиме, ночной прогон продолжает краснеть как раньше». + +`baseRegistry(rangeBase)` — не новая функция: переиспользована та же, +что уже читает `MUTANT_DEFINITIONS` с указанного `ref` для отбора по +определениям (#492 §6.4). Riск дублирования логики отсутствует. + +Границы модулей (#558) реально в силе, не только заявлены: тест +«bounded module boundaries» прогнан выше и зелёный; строки CLI/execution +совпадают с числами хендоффа. + +## Находки + +**Low — неиспользуемый импорт.** `scripts/mutation-execution.mjs:13` +добавляет `setupFailureOwner` в импорт из `mutation-guard-outcome.mjs`, но +нигде в файле не использует (логика по факту живёт в +`mutation-attribution.mjs`, которая импортирует `setupFailureOwner` +самостоятельно). Не нарушает границу модулей (`mutation-guard-outcome.mjs` не +входит в список запрещённых зависимостей для `execution`), гейтов не красит +(в `scripts/**` нет линтера, отслеживающего неиспользуемые импорты) — чистый +мусор из промежуточной версии рефакторинга. Фиксится или снимается автором +без цикла ревью. + +**Low — неточность в отчёте хендоффа.** Автор указал «`mutation-gate --check` +736 якорей без нарушений»; фактическое число определений в реестре на этом +SHA — 738 (проверено `MUTANTS.length` и подсчётом строк `ok` в выводе +`--check`, обе цифры сходятся на 738). Расхождение не влияет на вердикт +`--check` (0 FAIL в обоих случаях) и не является дефектом кода — просто цифра +в тексте комментария не совпадает с деревом. Отмечаю, чтобы не закрепилось +как «сверено» без сверки. + +Оба Low не блокируют и не требуют возврата автору по отдельности. + +## Что проверено и корректно + +- Атрибуция сравнивает подобное с подобным (определение базы на дереве базы), + что и было целью задачи — не определение головы на дереве базы, ошибка + которой автор поймал на себе и закрепил тестом + `attribution-judges-the-head-definition-on-the-base`. +- Свидетеля, которого в базе нет, оправдать нельзя (`introduced` без запуска + на базе) — закреплено тестом и мутантом + `attribution-lets-a-new-witness-off`. +- «Недоказанная невиновность не оправдание»: `null` от атрибуции (нет базы, + реестр не прочитан, чтение бросило, голова упала не на подготовке) не + снимает отказ с задачи — покрыто тестами и логикой `setupFailureOwner`. +- Предсуществующий отказ называется машиночитаемой строкой + `pre-existing-setup-failures=`, читаемой человеком и потенциально CI; + формат подписан комментарием «менять синхронно» — учтено на будущее. +- `docs/TESTING.md` описывает механизм точно тому, что реализовано в коде, + включая абзац про статически мёртвые ветки (`if (false)` и т.п.) как первую + причину, ради которой всё это понадобилось на #566. +- Инфраструктурный трек подтверждён: `git diff --stat` не содержит ни одного + файла класса A; трейлеры `Issue: #568`, `User-Visible: no` корректны, вторых + changelog не требуется. + +## Чего не проверял + +- Не гонял `npm test` целиком и `npm run build` — зелёный Validate на этом + SHA уже это подтвердил. +- Не воспроизводил полный E2E-сценарий через реальные `git worktree` на живой + истории репозитория (как это сделал автор в хендоффе) — вместо этого + прочитал логику построчно и подтвердил тем же классом доказательства + (`--id=` запуск зарегистрированных мутантов, которые как раз и эмулируют + «голова сломана / база чиста» через патч реестра). Сочтено достаточным для + инфраструктурной задачи такого размера. +- Не проверял поведение при экзотических диапазонах `--changed` (тройная + точка `a...b`, отсутствие `..`) — код здесь не новый, использует тот же + `range.split('..')[0]`, что уже применялся строкой выше для отбора по + реестру (#492 §6.4); новый риск не вносится. + +## Вердикт + +Зелёный. Оба найденных Low не блокируют; правятся автором без ревью или +снимаются записью. + +--- + + + +## Материал раунда + +- Ветка: `issue/568-setup-failure-attribution`, коммит `264fba0902ff` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `90d8fa04c2ff69c55ed17f3d2ac8184691810647` + ``` + git log --all --format='%H %T' | grep 90d8fa04c2ff + ``` +- Тело issue: `0eb6dbcb878c18d6c9a4337a7b3963ce367cca574717e016d54b4977ae846760` +- Вердикт конвейера: `green` · High 0