From 43ba198d7f74d92499e4dfd582c2a00adc4bd076 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Sun, 13 Sep 2026 09:17:38 +0000 Subject: [PATCH] docs: review document for #549 Issue: #549 User-Visible: no --- docs/reviews/CODE-REVIEW-549-r2.md | 189 +++++++++++++++++++++++++++++ 1 file changed, 189 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-549-r2.md diff --git a/docs/reviews/CODE-REVIEW-549-r2.md b/docs/reviews/CODE-REVIEW-549-r2.md new file mode 100644 index 00000000..017bb6b1 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-549-r2.md @@ -0,0 +1,189 @@ +# CODE-REVIEW-549-r2 + +**Issue:** #549 — «Ночные мутации: один неизменяемый SHA для всех shards и итогового отчёта» +**Материал:** `80e3fee527caae72f7a08e6c73846f143350537d` (единственный новый коммит поверх r1: `fix(ci): запускать агрегатор из material (#549)`) +**Заход:** r2 · блокирующих циклов израсходовано 0 из 4 +**Предыдущий раунд:** r1, вердикт зелёный, 0 находок, SHA `723ec6d36210...` (`docs/reviews/CODE-REVIEW-549-r1.md`, смержен в `dev` коммитом `08701ba0`). + +## Почему round r2, если r1 был зелёным + +r1 закрылся зелёным без находок — сам по себе цикл правок не образует (#227). После +r1 на ветку добавлен ещё один коммит `80e3fee5`, не являющийся ответом на находку +ревью, а самостоятельной правкой автора той же задачи (обнаружена, по всей +видимости, при реальном прогоне workflow или при повторном чтении кода). Раз +материал issue продвинулся, конвейер завёл новый раунд ревью на новый SHA — +это и есть предмет r2. + +## Скоуп (дельта r2) + +`git diff 723ec6d3..HEAD` = ровно коммит `80e3fee5`, два файла: + +- `.github/workflows/mutation-gate.yml` — 16 строк: в трёх местах + `--workflow-sha=${{ github.sha }}` → `--workflow-sha=${{ github.workflow_sha }}` + (jobs `mutants`, `evidence`, `report`); в двух местах чекаут кода отчётчика + `ref: ${{ github.sha }}` → `ref: ${{ needs.material.outputs.sha }}` (jobs + `evidence`, `report`; у `mutants` эта форма уже была верной с r1); +- `test/mutation-gate.test.mjs` — расширение существующего теста `#549: + агрегатор требует четыре evidence одного material и report не перечитывает + dev` под новую форму YAML. + +Продуктовый код (`src/**`) не тронут. Трейлеры: `Issue: #549`, +`User-Visible: no` — верно, изменений в changelog не требуется. + +## Суть правки и почему она нужна + +До этого коммита job'ы `evidence` и `report` чекаутили код CLI-отчётчика +(`scripts/mutation-gate-report.mjs`) по `ref: ${{ github.sha }}`. Для событий +`schedule`/`workflow_dispatch` `github.sha` указывает на коммит той ветки/рефа, +относительно которого стартовал сам workflow-файл (практически всегда — +дефолтная ветка `main`), а не на `dev`, где реально живут актуальные CLI-флаги +скрипта. `mutants` уже с r1 чекаутился по `needs.material.outputs.sha` +(зафиксированный `dev`) — то есть шард и агрегатор потенциально работали +**разными версиями одного и того же скрипта**: шард — версией из зафиксированного +material, агрегатор — версией из `main`. Если `main` отстаёт от `dev` (а он +объективно отстаёт, пока цепочка issue не смержена в `main`), `mutation-gate-report.mjs` +из `main` может не знать новых флагов (`--verify-only`, `--require-evidence`, +формат evidence) — job упал бы или, хуже, тихо принял бы неполные данные. +Правка чекаутит все три job'а (`mutants`, `evidence`, `report`) по одному и тому +же `needs.material.outputs.sha` — теперь весь пайплайн этой ночи работает одной +версией отчётчика, ровно тем material, который проверяется. + +Второе изменение — `github.sha` → `github.workflow_sha` в параметре +`--workflow-sha`. `github.workflow_sha` — отдельное документированное поле +контекста `github` (SHA коммита, которым определён сам workflow-файл, +устойчивое к тому, какой ref был передан на дispatch или зафиксирован как +material). Смысл параметра `--workflow-sha` в `mutation-gate-report.mjs` — +identity самого прогона workflow для сверки «все четыре шарда одной ночи +писали evidence в рамках одного и того же запуска» (`validateMutationShardEvidence`, +проверка `foreign workflow SHA`, `scripts/mutation-gate-report.mjs:101`). Это +поле не подменяет `--sha=${{ needs.material.outputs.sha }}` (identity +проверяемого материала) — они проверяются раздельно и оба обязательны +(`scripts/mutation-gate-report.mjs:55, 277, 287`). Замена корректна и не меняет +семантику проверки «foreign workflow SHA»: значение по-прежнему одинаково для +всех job'ов одного запуска, но теперь не совпадает случайно со значением, +которое могло бы иметь отношение к выбранному на dispatch `ref`. + +## Как проверялось + +Дешёвые гейты на этом SHA (`80e3fee5`) уже зелёные — Validate run, ссылка дана +в системном промпте: `tsc --noEmit`, полный `npm test`, `npm run build` (со +сверкой бандла) повторно не гонял. Раз `npm test` в этом прогоне включает +`test/mutation-gate.test.mjs` и `test/mutation-gate-report.test.mjs`, факт +зелёного Validate уже доказывает, что оба файла проходят на текущем коде. + +Дополнительно прогнано мной, целенаправленно под дельту: + +| Команда | Результат | +|---|---| +| `node --test test/mutation-gate.test.mjs test/mutation-gate-report.test.mjs` | 63/63 ok, включая изменённый тест `#549: агрегатор требует...` | +| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | «Исполняемого frontend-диффа нет» — диф не трогает `src/**`, браузерные smoke закономерно не выбираются | +| `grep -n "github.sha\|workflow-sha\|workflow_sha\|ref: \${{" docs/TESTING.md` | пусто — документация не описывает эти детали реализации, обновление не требуется | +| чтение `.github/workflows/mutation-gate.yml` целиком | все три job'а (`mutants`, `evidence`, `report`) теперь единообразно используют `needs.material.outputs.sha` для чекаута и `github.workflow_sha` для параметра `--workflow-sha` — согласованность подтверждена `grep` по всему файлу | + +Тест умеет падать: до этого коммита та же строка теста утверждала +`ref: ${{ github.sha }}` (см. `git show 80e3fee5` — diff теста), то есть при +откате только workflow-файла (с оставленным новым тестом) `assert.match(..., +/ref: \$\{\{ needs\.material\.outputs\.sha \}\}/)` для `evidence`/`report` и +негативная проверка `!report.includes('ref: ${{ github.sha }}')` обе упадут — +проверено рассуждением по diff, не отдельным прогоном на искусственно +откаченном файле (откатывать рабочую копию в ходе ревью не стал, чтобы не +трогать состояние дерева). + +**Что не проверялось и почему:** `npx tsc --noEmit`, `npm test` целиком, +`npm run build` — уже зелёные на этом SHA (Validate). `npm run golden:verify` — +diff не меняет рендер. `python -m pytest tests_backend` — `custom_components/**` +не тронут. `npm run invariants` — geometry/`layout`/`marker.space`/толщина не +задеты, дифф вообще не product-код. `node scripts/mutation-gate.mjs --check` / +прогон мутационного свидетеля — не требуется: этот раунд не трогает +`scripts/mutation-gate-report.mjs` (валидационную логику `validateMutationShardEvidence`), +только workflow YAML и его тест; мутационный реестр за r1 остаётся в силе. + +## Разбор по AC (тело issue #549) — что задевает дельта + +AC1 (единый SHA несмотря на движение `dev`) и AC2 (агрегатор отвергает +смешанный material) дельта затрагивает косвенно: сам механизм фиксации +`material` (job `material`, `needs.material.outputs.sha` как источник) не +изменился — изменился только источник **кода**, который это фиксирует и +проверяет (чекаут `evidence`/`report`), плюс идентификатор запуска workflow. +Логика `validateMutationShardEvidence` (`scripts/mutation-gate-report.mjs`) +дельтой не тронута, её доказательство наследуется из r1 без пересмотра. +AC3 (rerun) и AC4 (negative fixture) дельту не задевают вовсе — ни новый +тестовый сценарий, ни изменение поведения rerun/fixture в диффе нет. + +Единственный новый содержательный вопрос этого раунда — устраняет ли правка +риск «report/evidence читают чужую (main) версию отчётчика» — да, устраняет, +разбор выше. + +## Закрытие раунда r1 + +r1 не имел находок (0 High, 0 Medium, 0 Low) — таблицы «находка → чем +закрыта» не требуется, закрывать нечего. + +## Унаследовано из r1 + +Без повторной проверки в этом раунде принято (документ `docs/reviews/CODE-REVIEW-549-r1.md`, +SHA `723ec6d3621051c1eaa260fab469a7dcb5478398`): + +- AC1 (фиксация material при движении `dev`) — механизм job `material` и + чтение `needs.material.outputs.sha` в `mutants` не менялись этим коммитом; +- AC2 (`validateMutationShardEvidence` отвергает смешанный/неполный material, + включая мутационного свидетеля `mutation-report-accepts-foreign-material`, + лично прогнанного в r1 — «поймано 1 из 1») — код валидации не тронут; +- AC3 (rerun сохраняет/переустанавливает согласованный material) — механика + `byShard` (самый новый attempt при сохранении material) не менялась; +- AC4 (negative fixture обнаруживается) — тесты `test/mutation-gate-report.test.mjs` + не менялись этим коммитом (изменения только в `test/mutation-gate.test.mjs`, + проверяющем форму YAML); +- ограничение issue («полный набор — не часть повседневного цикла», #513) — + `on:` в `mutation-gate.yml` не менялся; +- «один источник числа» — `materialSha`/`materialTree` по-прежнему берутся + из единственного job `material` без параллельного пересчёта, дельта это не + меняет, а лишь унифицирует, откуда job'ы берут **код**, читающий эти числа. + +## Что проверено и корректно + +- Три job'а (`mutants`, `evidence`, `report`), которые ранее могли работать + разными версиями `mutation-gate-report.mjs`, теперь единообразно чекаутятся + по `needs.material.outputs.sha` — согласованность подтверждена чтением всего + файла и `grep`. +- `--workflow-sha` во всех трёх местах синхронно заменён на семантически более + точное поле контекста `github.workflow_sha`; проверка `foreign workflow SHA` + в `validateMutationShardEvidence` не зависит от того, какое именно поле + контекста подставлено — важно только постоянство значения в рамках одного + запуска, что сохраняется. +- Тест обновлён в том же коммите и в той же паре assert-ов, что и код — + расхождения тест/код нет; тест способен упасть при откате правки (см. выше). +- `docs/TESTING.md` не описывает эти детали реализации — актуализация не + требуется. + +## Находки + +Нет находок уровня High или Medium. Low не нашёл: правка узкая, точечная, +устраняет реальный (а не гипотетический) риск рассинхронизации версии +CLI-инструмента между шардами и агрегатором, покрыта тестом, который +содержательно проверяет новую форму и способен упасть на старой. + +## Вердикт + +Зелёный. Дельта r2 — точечный инфраструктурный фикс поверх зелёного r1, +устраняющий реальный риск (агрегатор мог читать устаревшую с `main` версию +CLI-отчётчика). AC1–AC4 из тела issue дельтой не нарушены и не требуют +повторного доказательства сверх унаследованного из r1. Тесты, относящиеся к +дельте, прогнаны лично и зелёные (63/63); более широкие гейты подтверждены +зелёным Validate на этом же SHA и не требовали повторного прогона. + +--- + +--- + + + +## Материал раунда + +- Ветка: `issue/549-nightly-material`, коммит `80e3fee527ca` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `8e80fad3f7bfcc2c8fc1d5975c8bda9ebdb14fd7` + ``` + git log --all --format='%H %T' | grep 8e80fad3f7bf + ``` +- Тело issue: `61a94896cf07a1fabc1d54fcfdb78d67d71e76f0e027e9bb3f9538094ec940fd` +- Вердикт конвейера: `green` · High 0