mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -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=<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 зелёный на точном материале;
|
||||
трейлеры и класс изменений верны; документация согласована с кодом.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/620-mutants-cost`, коммит `44ee23ee33b6` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `a02720092a4e4a4188d499cd0f761a840554af7b`
|
||||
```
|
||||
git log --all --format='%H %T' | grep a02720092a4e
|
||||
```
|
||||
- Тело issue: `a4faabe94f0f4a4c0c0b40302b5d8388d19bcb788fe4697dd9103f053fc87efe`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
@@ -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<ref> в 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` |
|
||||
|
||||
Reference in New Issue
Block a user