mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-01 12:18:51 +00:00
claude[bot]
@@ -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=<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=<id,…>`, читаемой человеком и потенциально 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 не блокируют; правятся автором без ревью или
|
||||
снимаются записью.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/568-setup-failure-attribution`, коммит `264fba0902ff` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `90d8fa04c2ff69c55ed17f3d2ac8184691810647`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 90d8fa04c2ff
|
||||
```
|
||||
- Тело issue: `0eb6dbcb878c18d6c9a4337a7b3963ce367cca574717e016d54b4977ae846760`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user