diff --git a/docs/reviews/CODE-REVIEW-620-r1.md b/docs/reviews/CODE-REVIEW-620-r1.md new file mode 100644 index 00000000..47747215 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-620-r1.md @@ -0,0 +1,191 @@ +# CODE-REVIEW #620 · заход r1 + +Материал: `44ee23ee33b6b1b4c4370a78697b6da4cff81faf` (единственный коммит поверх +`origin/dev`). Класс B (`.github/workflows/**`, `scripts/**`, `test/**`, +`docs/TESTING.md`); класса A нет — подтверждено (`src/**`, +`custom_components/**/*.py` не тронуты). Инфраструктурная задача, ускоренный +вход прямо в `S7-code-review` (AGENTS.md «Agent-neutral workflow») — корректно. + +## Скоуп + +Issue #620 (аудит стоимости CI от 22.09, T2/D): ночной мутационный реестр +(`mutation-gate.yml`) гонял все 800+ мутантов каждую ночь, даже когда `dev` не +менялся 4 ночи подряд; диспатч `changed_mutants` (`validate.yml`) ставил Python +и Chromium на каждый шард с непустым планом, даже если гардам шарда браузер не +нужен. Правки: + +1. `mutation-gate.yml`: зелёный полный прогон (агрегатор доказал все 6 шардов) + оставляет маркер в кэше Actions (tree материала + `workflow_sha`); ночь по + расписанию с тем же деревом и тем же workflow, маркер не старше 7 суток, + переиспользует прогон и пишет «reused from run N», шарды не гоняются. + Красный или неполный прогон маркера не оставляет — отказ по-прежнему заводит + issue (#472). Ручной dispatch гонит полный реестр всегда. +2. `validate.yml` (`changed_mutants`): план шарда (`--plan-only`) теперь + называет окружение своих гардов (`plan-browser=`/`plan-python=`, + `scripts/mutation-environment.mjs`); Python/pip и Chromium ставятся только + шарду, чьи гарды их реально используют. +3. Перевод смок-гардов на `node --test` (п.3 issue) — автор явно вынес из + скоупа этой задачи и предложил отдельный issue. Это не находка, а заранее + объявленное сужение скоупа: п.3 не входил ни в один AC (AC1–AC3 покрывают + только пп.1–2), отдельный issue заводить не требуется. + +Первый круг ревью этой задачи (issue упоминает зависший раунд prepare/dispatch +на `2a95cb1f` — сбой пробуждения оркестратора, отслеживается отдельно как +#636/#555; к материалу и коду этого ревью отношения не имеет). + +## Как проверялось + +**Целостность материала.** Сверил `git hash-object` всех изменённых файлов с +блоками `blob …` из раздела «Материал раунда» комментария автора — совпадение +байт-в-байт по всем 10 файлам, кроме `mutation-registry.mjs` (ожидаемо: реестр +рос от параллельно влившихся задач после ребейза на новый `origin/dev`). + +**Дешёвые гейты.** Validate на этом точном SHA зелёный (run 35961600335, +`headSha` подтверждён через `gh run view` = `44ee23ee…`) — `tsc`, `npm test`, +`npm run build` не перегонял. Дополнительно прогнал сам (не покрыто Validate, +т.к. относится к мутационному гейту): + +- `node scripts/mutation-gate.mjs --check` → 888 ok, 0 FAIL (чистые прогоны + всех гардов реестра, включая новые). +- Каждый из 10 новых мутантов индивидуально: + `node scripts/mutation-gate.mjs --id=` для всех + `nightly-reuse-ignores-tree`, `nightly-reuse-accepts-stale-marker`, + `nightly-reuse-on-manual-dispatch`, `green-marker-without-green-aggregator`, + `nightly-reuse-decision-error-skips-registry`, `browser-shard-skips-chromium`, + `unread-plan-environment-skips-install`, `environment-misses-playwright-import`, + `environment-misses-backend-files`, `environment-ignores-spawned-scripts` — + все дали «поймано 1 из 1». Тесты умеют падать. +- `node scripts/process-gate.mjs` → «гейт пройден, предупреждений 0». +- `node scripts/action-pins.mjs` → все сторонние Actions закреплены полным SHA. +- `node scripts/smoke-select.mjs --base origin/dev --head HEAD` → + «Исполняемого frontend-диффа нет», браузерные смоки не выбираются — верно, + diff не трогает `src/**`. + +**Не прогонял и почему:** +- `npx tsc --noEmit` / `npm test` / `npm run build` со сверкой копий бандла — + Validate уже зелёный на этом точном SHA (#343), дифф не трогает `src/**`, + сверять бандл нечего. +- `npm run golden:verify` — User-Visible: no, рендер не менялся, смоки это же + подтвердили («исполняемого frontend-диффа нет»). +- `python -m pytest tests_backend -q` — `custom_components/**/*.py` не + затронут. +- `npm run invariants` — геометрия не менялась. +- Живой ночной прогон с переиспользованием маркера и живой dispatch шарда без + окружения — сам автор пометил как непроверенное и обоснованно: кэш Actions и + `github.workflow_sha` не воспроизвести локально, только на реальном + расписании/дispatch после слияния и зеркалирования `mutation-gate.yml` в + `main`. Логика (чистая функция, CLI, структура workflow-YAML построчно) + проверена мной чтением и мутационными тестами — см. ниже. + +**Разбор по коду.** +- `scripts/mutation-nightly-reuse.mjs`: `decideNightlyReuse` — чистая функция, + проверил все ветки отказа (не schedule, чужое дерево, чужой workflow, нет + маркера/схемы/runId, маркер из будущего/старше 7 суток/нечитаемое время) — + каждая покрыта юнит-тестом и мутантом. Проверено чтением и исполнением + (`node --test test/mutation-nightly-reuse.test.mjs`, транзитивно через + Validate). +- `.github/workflows/mutation-gate.yml`: прошёл all four job-и (`material`, + `mutants`, `evidence`, `green_marker`, `report`) вручную по семантике + GitHub Actions `if:` (implicit `success()`-обёртка при отсутствии + `always()`/`failure()`/`cancelled()` в выражении). Убедился, что: + - при `reuse=true` `mutants` и `evidence` оба получают `if=false` и не + запускаются (не просто «material упал»); + - `green_marker` пишется только когда `mutants.result == 'success' && + evidence.result == 'success'` — без `always()`, значит ещё и implicit + `success()` требует, чтобы сам `material` не упал; + - `report` не срабатывает на переиспользованную ночь (`reuse != 'true'` в + условии), но по-прежнему срабатывает на настоящий частичный/красный прогон + (`mutants`/`evidence` не skip, а реальный `!= 'success'`) — регрессии к + #472 нет. + Проверено чтением, не исполнением (сам workflow на реальном раннере не + гонял — это и есть заявленное «не проверял» выше). +- `.github/workflows/validate.yml`: условия установки (`setup-python`, + `pip install`, кэш/установка Chromium) корректно завязаны на + `steps.plan.outputs.{python,browser} == 'true'` вместе с `count != '0'`; + `npm ci` и сам прогон гардов от окружения не зависят — верно, это шаги, + нужные любому непустому плану независимо от типа гардов. Дефолт `${browser:-true}` + /`${python:-true}` на непрочитанную строку — сторона ошибки верная (лишняя + установка, не пропущенная). +- `scripts/mutation-environment.mjs`: граф `guardRuntimeFiles` — точки входа + (файлы из строки гарда), объявленные `GUARD_INPUTS` обёрток (только когда в + строке гарда нет других явных файлов — иначе `wrapperInputs` не + вызывается), путь-литералы **только у точек входа** (не у всех + транзитивно импортированных файлов) → относительные импорты рекурсивно. + Проверил на реальном `scripts/backend-test-guard.mjs`: даже если бы граф не + дотянулся до объявленного `GUARD_INPUTS`, сам файл обёртки содержит + `process.env.PYTHON` в исходнике — второй независимый путь детекции. Эвристика + документированно однобокая (недо-обнаружение → лишний круг задачи через + красный чистый прогон, не пропущенная поломка) — направление ошибки + безопасное, задокументировано в шапке модуля и в `docs/TESTING.md`. + Реестровый тест (`#620 (реестр)`) подтверждает калибровку на реальных 556 + уникальных гардах: 315 без окружения, `bare > guards.length/3` — признак не + выродился в «ставить всё». + +## AC — таблица из хендоффа, проверено самостоятельно + +| AC | Чем доказано (перепроверено) | Чем краснеет (перепроверено) | +|---|---|---| +| AC1: ночь на неизменённом дереве пропускает шарды, «reused from run N» в отчёте | `test/mutation-nightly-reuse.test.mjs` (10 тестов, все прошли), `test/mutation-gate.test.mjs` «#620: пропуск ночи…» (проводка workflow построчно) | 4 мутанта индивидуально прогнаны — все «1 из 1» | +| AC1: отказ по-прежнему заводит issue (#472) | маркер только после `mutants==success && evidence==success`; `report` исключает только `reuse=='true'` | `green-marker-without-green-aggregator`, `nightly-reuse-decision-error-skips-registry` — оба «1 из 1» | +| AC2: диспатч без браузерных гардов не ставит Chromium ни на одном шарде | `test/validate-workflow.test.mjs` «#620 AC2», `mutation-gate.test.mjs` реестровый тест на всех гардах | `browser-shard-skips-chromium`, `unread-plan-environment-skips-install`, `environment-misses-playwright-import`, `environment-misses-backend-files`, `environment-ignores-spawned-scripts` — все «1 из 1» | +| AC3: тесты на оба правила, `docs/TESTING.md` | тесты выше существуют и падают на мутантах; `docs/TESTING.md` описывает оба правила согласованно с кодом (сверено построчно с реализацией: имя скрипта, имена флагов, срок 7 суток, формула графа) | документация — не защитный AC | + +Пустых третьих столбцов не осталось ни у одной защитной строки. + +## Что проверено и корректно + +- Оба новых файла (`mutation-nightly-reuse.mjs`, `mutation-environment.mjs`) + документируют направление ошибки в шапке и держатся этого направления в + коде — недоустановка окружения или ложный полный прогон невозможны без + явного отказа скрипта решения (который сам по себе трактуется как «нужен + полный прогон»). +- `docs/TESTING.md` обновлён в двух местах, согласован с реализацией + построчно (имена флагов, формула графа, срок годности маркера). +- Трейлеры коммита: `Issue: #620`, `User-Visible: no` — верно (CI-инфраструктура, + видимого пользователю поведения нет), changelog не тронут — согласуется. +- `demo/golden/baselines/**` не тронут — доп. трейлеры `Release`/ + `Baseline-Reviewed*` не требуются. +- `.github/workflows/**` изменения не расширяют `permissions:` ни одной job — + сверено по диффу. +- Note про зеркало `mutation-gate.yml` в `main` и ожидаемое кратковременное + покраснение preflight после слияния до зеркалирования — принято к сведению, + это эксплуатационный шаг владельца, не дефект кода (прецедент #604/#636, + логика воспроизведена верно: `workflow_sync` сравнивает `origin/main` с + `origin/dev`, не с веткой, поэтому на самой ветке preflight зелёный). + +## Находки + +Нет ни одной High или Medium находки. Задача решает заявленный сценарий, +инвариант «отказ всегда заводит issue» (#472) не сломан, окружение шардов не +недоустанавливается ни в одном проверенном случае. + +## Чего не проверял + +- Живой ночной прогон с реальным переиспусзованием маркера через кэш Actions — + локально не воспроизвести (кэш, `github.workflow_sha`, реальное расписание). + Первая живая проверка — вторая ночь после слияния и зеркалирования в `main`. +- Живой dispatch `changed_mutants` на реальном PR-диффе, где видно, какие + именно шарды пропустили `setup-python`/Chromium в логах Actions. +- Golden/perf-профили — дифф не рендерит и не меняет геометрию/производительность + продукта. + +## Вердикт + +Зелёный. AC1–AC3 доказаны автотестами, которые проверены на умение падать +(все 10 новых мутантов индивидуально дали «1 из 1»); защитные AC имеют +непустой столбец «чем краснеет»; Validate зелёный на точном материале; +трейлеры и класс изменений верны; документация согласована с кодом. + +--- + + + +## Материал раунда + +- Ветка: `issue/620-mutants-cost`, коммит `44ee23ee33b6` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `a02720092a4e4a4188d499cd0f761a840554af7b` + ``` + git log --all --format='%H %T' | grep a02720092a4e + ``` +- Тело issue: `a4faabe94f0f4a4c0c0b40302b5d8388d19bcb788fe4697dd9103f053fc87efe` +- Вердикт конвейера: `green` · High 0 diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index 0aae89a8..4c8fd170 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -1,6 +1,6 @@ # Индекс ревью -Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1024, issue: 362. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. +Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 1025, issue: 363. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы | |---|---|---|---|---:|---:|---|---| @@ -36,6 +36,7 @@ | #624 | [CODE-REVIEW-624-r1.md](CODE-REVIEW-624-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | | #622 | [CODE-REVIEW-622-r1.md](CODE-REVIEW-622-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | | #621 | [CODE-REVIEW-621-r1.md](CODE-REVIEW-621-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | +| #620 | [CODE-REVIEW-620-r1.md](CODE-REVIEW-620-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | | #619 | [CODE-REVIEW-619-r1.md](CODE-REVIEW-619-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | | #618 | [SPEC-REVIEW-618-r1.md](SPEC-REVIEW-618-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | нотация h в B5 не встречается в коде | `docs/FILTERING.md` | | #617 | [SPEC-REVIEW-617-r1.md](SPEC-REVIEW-617-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | «новый необязательный параметр» уже существует | `src/backdrop-pick.ts` `houseplan-editor-runtime.ts` |