diff --git a/docs/reviews/CODE-REVIEW-709-r1.md b/docs/reviews/CODE-REVIEW-709-r1.md new file mode 100644 index 00000000..69bb250f --- /dev/null +++ b/docs/reviews/CODE-REVIEW-709-r1.md @@ -0,0 +1,193 @@ +# CODE-REVIEW-709-r1 + +Материал раунда: `git log --oneline origin/dev..HEAD` = один коммит +`a8321e32cc8299a521657b71aba6c43f41d93e6b` поверх `dev`@`18c9f8e7`. +Заход r1, трек `show`, блокирующих циклов израсходовано 0 из 2. + +## Скоуп + +Класс B (инфраструктура — `scripts/**`, `.github/**`, `PROCESS.md` и её +конспекты). Файлов класса A нет. Задача убирает прогон мутантов по диффу из +разработки на всех треках: `resolveTrack().mutants` всегда `false`, +`mutantsRequested()` всегда `false`, `pre-push-gate.mjs` не гоняет мутанты ни в +ручном режиме, конвейер (`_process.yml`, `validate.yml`) по умолчанию +диспатчит Validate без мутантов. Канон (`PROCESS.md`, `AUTHOR.md`, +`REVIEWER.md`, `TESTING.md`, `AGENTS.md`) переписан под новое правило; в +`PROCESS.md` §8 добавлены правила скорости для `ship`/`show` (AC3). Это не +продуктовая задача — она не закрывает и не должна закрывать ни одну строку +`docs/SCOPE.md`; относится к самому конвейеру ревью, поэтому первый вопрос +ревьюера («какую работу из SCOPE она обслуживает») здесь неприменим по +конструкции задачи (инфраструктура процесса, не продукт). + +## Как проверялось + +Дельта — первый раунд, разбор полный. + +По AC: + +- **AC1** (`resolveTrack`/`mutantsRequested` всегда `mutants=false`; Validate + не запрашивает job мутантов ни на dispatch, ни на PR; ночной реестр не + меняется). Прочитан код: `scripts/process-track.mjs` (`const mutants = + false`), `scripts/classify-changes.mjs` (`mutantsRequested()` без аргументов, + всегда `false`), `scripts/ci-proof.mjs` (комментарий и политики не + изменились по значениям — верно, это не входит в AC1). Прогнаны + `test/classify-changes.test.mjs`, `test/process-track.test.mjs` — 39/39, + зелёные. Тест умеет падать: перед патчем `mutants = track === 'ask' || + labels.includes('ci:mutants')` — новый тест «#709: мутантов в разработке нет + ни на одном треке» упал бы на `track:ask`; аналогично старая ветка + `mutantsRequested` с `eventName === 'pull_request' → true` красит тест + «#709: Validate не запрашивает мутантов…». Оба случая — реальные мутанты в + `scripts/mutation-registry.mjs` (`dev-mutants-requested-again`, + `track-pays-for-mutants-again`), статически проверены `node + scripts/mutation-gate.mjs --check` → `ok` по обоим id, 0 FAIL (3 + предсуществующих WARN о `--test-name-pattern` с `${…}`-именами, #650, не + относятся к этой задаче). Ночной реестр (`mutation-gate.yml`) в диффе не + тронут — проверено чтением: `.github/workflows/` не содержит правок этого + файла. **AC1 доказан.** +- **AC2** (канон говорит одно; `ci:mutants` снята; ревьюер мутанты не + применяет). Прочитаны все правки `PROCESS.md` (§2.7, §5.1, §10.4, метка + снята из таблицы модификаторов), `docs/process/AUTHOR.md`, + `docs/process/REVIEWER.md`, `docs/TESTING.md`, `AGENTS.md` — + формулировки согласованы **кроме одного места**, см. находку Medium ниже. + `test/process-digests.test.mjs` (сверяет цитаты `REVIEWER.md` ↔ `PROCESS.md`) + прогнан — зелёный, но он проверяет только внесённый в `KEY_RULES` список + цитат, не весь файл `TESTING.md`, поэтому находка ниже мимо него прошла. + **AC2 доказан частично** — с находкой Medium, в скоупе. +- **AC3** (правила скорости `ship`/`show`: попутный флак — отдельное issue, + одно доказательство на пункт ТЗ, `gate:small -- --smokes` не обязателен на + `ship`). Прочитаны новые абзацы `PROCESS.md` §8 (строки ~767–777) и + `docs/process/AUTHOR.md` (раздел handoff). Флаг `--smokes` — реальный, + существующий (`scripts/gate-small.mjs:34`), не изобретён. Проверка чтением, + не исполнением — это формулировка процесса, а не код с автотестом; для неё + автотеста и не требуется (правило адресовано автору/ревьюеру, не коду). + **AC3 доказан чтением.** + +### Гейты — что прогнано и что нет + +| Гейт | Прогнан | Результат | +|---|---|---| +| `node --test test/classify-changes.test.mjs test/process-track.test.mjs test/process-digests.test.mjs` | да | 39/39 зелёные | +| `node --test test/pre-push-gate.test.mjs test/validate-workflow.test.mjs test/merge-candidate.test.mjs` | да | 65/65 зелёные | +| `node scripts/mutation-gate.mjs --check` | да | 0 FAIL, 3 предсуществующих WARN (#650, вне скоупа) | +| `npx tsc --noEmit`, полный `npm test`, `npm run build` + сверка бандла | **нет** | не перегонялись — Validate на этом SHA (`a8321e32`) уже зелёный, run 36623839784 (#343 разрешает не дублировать) | +| Смоки/golden/`pytest tests_backend`/инварианты модели/performance | **нет** | правок `src/**`, Python, геометрии, `demo/golden/**` в диффе нет — гейты неприменимы по AC и по дельте | +| `smoke-select.mjs --base --head` | **нет** | дифф не содержит исполняемого продуктового кода (класс B/C), браузерный смок не назван в теле issue (#696); трек `show` не ставит Chromium без такого указания | +| `actionlint`, `process-gate --range` | нет, со слов автора | автор заявил «чисто» / «0 предупреждений» в хендоффе; не переисполнял отдельно — дешёвый повторный прогон не даёт нового сигнала при уже зелёном Validate на этом SHA, решил положиться на комбинацию Validate + собственный прогон юнитов/mutation-gate | + +## Находки + +### Medium (в скоупе задачи, возвращается автору) — canon TESTING.md противоречит себе + +`docs/TESTING.md`, раздел «Локальный набор перед пушем»: + +- строка 267 (добавлена этим коммитом): «Ручной `node scripts/pre-push-gate.mjs` + из этого раздела — расширенный прогон: типы, юниты и смоки; **мутантов в + нём с #709 нет**.» +- но строки 276–277 (не тронуты этим коммитом) в примере команд: + ``` + node scripts/pre-push-gate.mjs --no-smokes --no-mutants + node scripts/pre-push-gate.mjs --max-smokes=3 --max-mutants=1 + ``` +- и следом, строка 280–282 (не тронуты): «Что прогоняется: … смоки, выбранные + `scripts/smoke-select.mjs` по диффу, **и мутанты, выбранные + `scripts/mutation-gate.mjs --changed` по тем же файлам**.» + +Воспроизведение: `scripts/pre-push-gate.mjs` больше не парсит `--max-mutants` +(флаг убран из диффа этой же задачи, `manualGate()` больше не объявляет +`maxMutants`) и безусловно пишет `skipped.push('мутанты — в разработке не +гоняются (#709)…')` — секция «мутанты по диффу» из ручного режима удалена +целиком. Автор сам поправил тот же паттерн чуть ниже по файлу (строка 308: +«Лимит по умолчанию — шесть смоков; мутантов ручной режим с #709 не +гоняет.» — раньше было «шесть смоков и два мутанта»), но пропустил +идентичное по смыслу место двумя абзацами выше. Ровно то, что AC2 требует +исключить: «Канон (PROCESS, AUTHOR, REVIEWER, TESTING) говорит одно» — здесь +`TESTING.md` говорит разное в двух соседних абзацах одного раздела, и это не +размечено как история (в отличие от блока «До #709 в CI `changed_mutants` +бежал…» чуть выше по файлу, который прямо помечен историческим). + +Почему это не поймал `process-digests.test.mjs`: тест сверяет только цитаты, +явно занесённые в `KEY_RULES`, а не весь текст файла — отсутствие записи не +значит отсутствие противоречия. + +Классификация: Medium, в скоупе (тот же файл, что уже правит эта задача, +тривиальная правка — убрать `--no-mutants --max-mutants=1` из примера и +переписать «Что прогоняется» без мутантов). Без High это жёлтый вердикт по +правилам §3 п.8 / §2.7 — блокирующий цикл не расходуется по нему одному, +но раунд возвращается автору. + +## Что проверено и корректно + +- Логика `resolveTrack`/`mutantsRequested`/`ci-proof` согласована между собой + и с обновлёнными тестами; якоря новых мутантов реестра совпадают с текущим + текстом файлов побайтово (проверено `mutation-gate --check`, 0 FAIL). +- `.github/workflows/_process.yml` и `validate.yml`: дефолты `MUTANTS:-false` + везде, где раньше был `MUTANTS:-true` (шаг Validate на материале ревью и + шаг слияния кандидата) — оба места правлены синхронно, несогласованности + между «жди Validate с мутантами» и «дефолт без мутантов» нет. + `mutants`-вход `validate.yml` оставлен намеренно (описание помечено + «не действует с #709»), обоснованно — старые `-f mutants=…` вызовы не + падают; это явно названо временным до отдельной уборки (#622), не находка. +- `pre-push-gate.mjs`: секция мутантов убрана из `manualGate`, `--no-mutants`/ + `--max-mutants` больше не влияют ни на что — согласуется с самим текстом + header-комментария файла (`* node scripts/pre-push-gate.mjs --no-smokes` без + `--no-mutants`), который был обновлён корректно. +- `PROCESS.md` §2.7: формулировка «мутант пишется, но в разработке не + гоняется… якоря реестра сверяет статический `mutation-gate --check`… + поимку проверяет только ночной полный прогон» согласована с + `docs/process/REVIEWER.md` («Мутанты в разработке не гоняются ни на каком + треке — ревьюер их тоже не применяет; проверяет, что защита названа + мутантом в реестре») и с `docs/process/AUTHOR.md`. Таблица модификаторов + меток (`PROCESS.md`, раздел §5.1/§9) больше не содержит строку `ci:mutants` + ни в одной из двух таблиц, где она раньше встречалась — сверено обоими + местами диффа. +- AC3: новый раздел §8 «Цена ship и show — без добровольных надбавок» + корректно ссылается на реально существующий флаг `--smokes` + (`scripts/gate-small.mjs:34`) и не вводит несуществующих команд. +- Трейлеры коммита: `Issue: #709`, `User-Visible: no` — верно, изменение не + меняет поведение продукта, changelog не тронут, и это корректно (не найдено + ни одного пользовательского числа/поведения, «видимого дважды»: задача не + трогает `src/**`, только процесс и CI). +- Метка `ci:mutants` не удалена из GitHub (описание сменено на «Retired», + решение об удалении оставлено владельцу) — соответствует тому, что написал + автор в хендоффе, и не противоречит AC2 (AC2 требует снять метку **из + процесса**, не обязательно удалить сам label-объект). +- `legacy/specs/510-*.md` и старые `docs/reviews/*` с упоминаниями + `mutants=true` не трогались и не должны — это архив прежних раундов, не + канон (§2.3, `docs/specs/` и `docs/reviews/` не входят в список AC2). + +## Чего не проверял + +- Полный `npx tsc --noEmit`, `npm test`, `npm run build` + троекратную сверку + бандла — не перегонял отдельно; опирался на зелёный Validate на этом же SHA + (`a8321e32`, run 36623839784, см. #343). Точечно перепроверил только + тесты, которые правит дифф, плюс смежные (`pre-push-gate`, + `validate-workflow`, `merge-candidate`) — 65/65. +- `actionlint` и `node scripts/process-gate.mjs --range` не переисполнял — + положился на заявление автора в хендоффе (`actionlint — чисто; + process-gate --range — 0 предупреждений`); эти гейты не относятся к + списку «обязательно перепрогнать ревьюеру» (§8) при уже зелёном Validate. +- Смоки/golden/backend/инварианты модели/performance — не применимы: дифф не + трогает `src/**`, Python, `demo/golden/**` или геометрию; `smoke-select` + не запускал, так как исполняемого фронтенд/бэкенд-кода в диффе нет и тело + issue не называет смоук или браузер (условие "по диффу и AC" не + выполняется ни по одному критерию). +- Не проверял, действительно ли ночной `mutation-gate.yml` в текущем виде + подхватит все мутанты реестра без прогона по диффу в течение дня — + вопрос эксплуатации ночного расписания, не этой задачи, и он не входит ни + в один AC709 (ночной реестр прямо назван неизменным в AC1). +- Не проверял историю прежних раундов по этой задаче — раунд первый, + раздела «Унаследовано из r0» и «Закрытие раунда r0» нет по правилам §2.10. + +--- + + + +## Материал раунда + +- Ветка: `issue/709-mutants-nightly-only`, коммит `a8321e32cc82` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `f1cb9e76e348f568523a765dc393c9cf4c6383f2` + ``` + git log --all --format='%H %T' | grep f1cb9e76e348 + ``` +- Тело issue: `914787f126c442dc2e4cacb96385538eb2f0e315dbaa828e3e0241a2a9ba81e5` +- Вердикт конвейера: `yellow` · High 0