mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -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 и не требовали повторного прогона.
|
||||
|
||||
---
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/549-nightly-material`, коммит `80e3fee527ca` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `8e80fad3f7bfcc2c8fc1d5975c8bda9ebdb14fd7`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 8e80fad3f7bf
|
||||
```
|
||||
- Тело issue: `61a94896cf07a1fabc1d54fcfdb78d67d71e76f0e027e9bb3f9538094ec940fd`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user