From e6dfa9a752db85337f14050b80bfb82d29ca94a0 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Mon, 28 Sep 2026 20:42:31 +0000 Subject: [PATCH] docs: review document for #698 Issue: #698 User-Visible: no --- docs/reviews/CODE-REVIEW-698-r1.md | 222 +++++++++++++++++++++++++++++ 1 file changed, 222 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-698-r1.md diff --git a/docs/reviews/CODE-REVIEW-698-r1.md b/docs/reviews/CODE-REVIEW-698-r1.md new file mode 100644 index 00000000..cff483b9 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-698-r1.md @@ -0,0 +1,222 @@ +# CODE-REVIEW #698 r1 + +Материал: `origin/dev..HEAD`, ровно один коммит `ea2c000af5e4bd47140c2c255fca16ad2a136f6c` +поверх `dev@9e4bfb43` (рабочая копия уже на нём). Диапазон: `.gitattributes`, +`PROCESS.md`, `scripts/merge-candidate.mjs`, `scripts/mutation-registry.mjs`, +`scripts/rebase-generated.mjs`, `test/rebase-generated.test.mjs`. Файлов класса +A нет — инфраструктурный трек (§1), вход сразу на `S7-code-review`, метка +`track:show` на issue. + +## Скоуп + +Issue #698 называет три источника из 14/48 возвратов #600–#691 по общим +файлам, где правки двух задач не противоречат друг другу: (1) записи +`## Unreleased` в обоих CHANGELOG, (2) `docs/images/screenshots.json`, (3) +`scripts/bundle-budget.mjs` / `scripts/monolith-baseline.json`. Формального ТЗ +нет (инфраструктура, PROCESS.md §1) — проверял по тексту раздела «Предложение» +issue и по факту автоматизации, а не по чужому изложению. + +Реализовано: + +- `.gitattributes`: `merge=union` для `docs/CHANGELOG.md` и + `docs/CHANGELOG.ru.md` — строки `## Unreleased` объединяются на ребейзе, + слиянии и `git merge-tree` без остановки. +- `scripts/rebase-generated.mjs`: `UPSTREAM_WINS = ['scripts/monolith-baseline.json']` + — конфликт именно на этом пути решается в пользу `dev` (`checkout --ours`), + без abort; любой другой путь по-прежнему аборт с полным списком. +- `scripts/merge-candidate.mjs`: `PATCH_ID_EXCLUDES` расширяет прежнее + исключение `docs/reviews` на оба CHANGELOG и `monolith-baseline.json` — эти + файлы больше не входят в patch-id кандидата, соседняя строка не отправляет + зелёную задачу обратно на ревью. +- `docs/images/screenshots.json` сознательно не тронут: автор в заявочном + комментарии ссылается на #697 («после #697 ветки задач его не коммитят»). + На момент этого ревью #697 в статусе `S1-new` (не начат, не триажирован) — + конфликты на этом файле продолжат случаться до его реализации. Автор это + прямо раскрыл, не спрятал; тема не входит в делаемый диапазон #698 (см. + «Что проверено»). +- `scripts/bundle-budget.mjs` не включён ни в `UPSTREAM_WINS`, ни в + `PATCH_ID_EXCLUDES`, хотя исходная «Проблема» issue называет его рядом с + `monolith-baseline.json`. Прочитал файл: он вперемешку хранит числовые + потолки и содержательные словари (`GRAPH_LABELS`, + `NAMESPACE_ENGLISH_CONSUMERS`, `SUPPORT_LAZY_MARKERS`), большинство числовых + потолков уже имеют полосу (`INITIAL_VIEW_CEILING_BAND`, + `LAZY_GRAPH_CEILING_BAND`) — это не тот же тип конфликта, что чистый + производный JSON `monolith-baseline.json`, и слепое разрешение «взять dev» + рисковало бы стереть чужой словарь. Решение не включать файл в это ревью не + оспариваю. + +## Как проверялось + +| Гейт | Команда | Результат | +|---|---|---| +| Validate на материале | CI run на `ea2c000a` (ссылка в задаче ревью) | зелёный; дешёвые гейты (`tsc`, `npm test`, `build`+`bundle-policy --verify`) не перегонял — приняты по этой ссылке (#343) | +| Целевой юнит-набор | `node --test test/rebase-generated.test.mjs` | 15/15 pass | +| Целевой юнит-набор | `node --test test/merge-candidate.test.mjs` | 19/19 pass | +| Целевой юнит-набор | `node --test test/rebase-on-dev.test.mjs` | 6/6 pass | +| Согласованность конспекта | `node --test test/process-digests.test.mjs` | 5/5 pass | +| `process-gate.mjs` (офлайн) | `node scripts/process-gate.mjs --issues` | «гейт пройден, предупреждений 1» — предупреждение информационное: инфраструктурный диапазон, статусная метка не обязательна до `S7-code-review` | +| Реестр мутантов, дешёвая половина | `node --test --test-name-pattern="every mutant patch anchors exactly once\|every guard command points" test/mutation-gate.test.mjs` | 2/2 — якоря трёх новых мутантов не отстали от кода | +| Мутант `changelog-union-driver-dropped` | вручную убрал строку `docs/CHANGELOG.md merge=union` из `.gitattributes`, прогнал `node --test --test-name-pattern="#698: записи ченджлога" test/rebase-generated.test.mjs`, восстановил файл | **краснеет** (`AssertionError`, `false !== true`) | +| Мутант `monolith-baseline-conflict-is-manual-again` | вручную заменил `UPSTREAM_WINS` на `Object.freeze([])` в `scripts/rebase-generated.mjs`, прогнал `--test-name-pattern="#698: конфликт в базе"`, восстановил файл | **краснеет** (`ok:false, reason:'manual'` вместо ожидаемого авторазрешения) | +| Мутант `merge-patch-id-sees-changelog` | вручную вернул `PATCH_ID_EXCLUDES` к списку без обоих CHANGELOG в `scripts/merge-candidate.mjs`, прогнал `--test-name-pattern="#698: patch-id"`, восстановил файл | **краснеет** (`AssertionError` на `docs/CHANGELOG.md`) | +| Чтение живого гейта монолита | `scripts/monolith-metrics.mjs: compareWithBaseline` | `band = name === 'bundleBytes' ? bundleBand : 0` — для 5 из 6 метрик допуск нулевой сегодня (см. находку Medium) | +| Провенанс коммита | `git show -s --format=full ea2c000a` | `Issue: #698`, `User-Visible: no` — корректно, ни одно пользовательское поведение продукта не задето | +| Восстановление рабочей копии | `git status --porcelain` после каждой ручной мутации | пусто — дерево вернулось к исходному состоянию перед следующим шагом | + +Не прогонял (и почему): `npx tsc --noEmit`, полный `npm test`, `npm run build` ++ сверка трёх копий бандла, `node scripts/check-docs.mjs`, `mutation-gate --check` +целиком (с пересборкой бандла на мутанта) — приняты по зелёному Validate на +этом SHA (#343); диапазон не трогает `src/**`, `check-docs.mjs` не требуется. +Golden/скриншоты, браузерные смоки, `pytest tests_backend`, инварианты модели, +performance-профили — не применимо: диапазон не трогает визуал, Python или +геометрию плана. + +## Что проверено и корректно + +- **`--ours`/`--theirs` во время rebase не перепутаны.** Комментарий в коде + («На ребейзе `--ours` — сторона, на которую ребейзят, то есть dev») верен + git-семантике (во время `git rebase` HEAD — это цель, `--theirs` — + переигрываемый коммит); тест «конфликт в базе метрик монолита решается в + пользу dev» подтверждает это исполнением реального git: `hostRefs` после + разрешения равен значению из коммита `neighbour` (dev), а не `task`. +- **`planStop`/`rebaseRegenerating` не путают `UPSTREAM_WINS` с `extra`/`index`.** + И `manual`, и возвращаемый `extra` корректно исключают оба новых пути; + `plan.upstream` обрабатывается отдельной веткой (`checkout --ours` + `add`) + до `plan.index`. Тест «база метрик вместе с конфликтом в коде» подтверждает, + что при добавлении обычного (не покрытого) конфликтующего пути срабатывает + прежний полный abort со списком `['a.mjs', 'scripts/monolith-baseline.json']`, + а не частичное разрешение. +- **Оба вызывающих получают правило бесплатно.** `UPSTREAM_WINS` зашита в + `scripts/rebase-generated.mjs`, а не передаётся через `opts.extra` — + и авторский `rebase-on-dev.mjs`, и конвейерный + `merge-candidate.mjs::rebaseOnto` используют общий `rebaseRegenerating` без + дублирования логики. +- **Все три новых мутационных гвардиана реально ловят регресс** — проверено + ручным воспроизведением (патч → красный тест → откат), а не по имени `ok` в + реестре мутантов. +- **Safety net не теряется при неверном бейзлайне.** После авторазрешения в + пользу dev реальное расхождение вычисленных метрик всё равно ловит Validate + самого кандидата: `merge-candidate.mjs` диспатчит `validate.yml` на + смерженный SHA и ждёт результат, красный возвращает задачу в + `S6-in-progress` — молча неверный бейзлайн в `dev` не проходит. +- **Патч-id корректно исключает то, что ребейз сливает сам** — соседняя + правка нейтрального файла не отправляет зелёную задачу обратно на ревью; + подтверждено мутационным тестом. +- **Трейлеры и класс изменений корректны**: `Issue: #698`, `User-Visible: no`, + ни одного файла класса A; `PROCESS.md` документирует изменение тем же + коммитом. +- **`scripts/bundle-budget.mjs` обоснованно вне скоупа этой правки** (см. + «Скоуп») — не переоткрываю как находку. + +## Находки + +### Medium (в скоупе — блокирует зелёный вердикт) + +**PROCESS.md утверждает существование «теста полосы на Validate» для +монолитного бейзлайна, которого сегодня нет.** + +Новый абзац этого же коммита (`PROCESS.md`, рядом с #643): «...а на конфликте +в `scripts/monolith-baseline.json` ребейз берёт сторону `dev` — число для +объединённого дерева судит тест полосы на Validate (#699)». Формулировка в +настоящем времени читается как факт о текущей автоматизации. + +По факту: `scripts/monolith-metrics.mjs:compareWithBaseline` сегодня считает +`band = name === 'bundleBytes' ? bundleBand : 0` — для пяти из шести чисел +(`delegates`, `portMembers`, `hostRefs`, `portPrivates`, `harnessPrivates`) +допуск нулевой, точное совпадение, снижение тоже FAIL. Это буквально проблема, +которую описывает сам #699 («Двусторонние храповики сейчас с нулевым или +минимальным запасом» / «5 чисел монолита... точное совпадение»), а #699 +находится в статусе `S1-new` — не триажирован, тем более не смёржен. «Тест +полосы» реально существует только для одной метрики из шести (`bundleBytes`, +`BUNDLE_BYTES_BAND`) — и этот механизм существовал до #698/#699, не является +их результатом. + +Канон сам формулирует правило для себя (шапка PROCESS.md): «При расхождении +этого документа с... `scripts/*` побеждает фактическая автоматизация. +Расхождение при этом не игнорируется, а заводится issue с меткой `process`». +Данный абзац — ровно такое расхождение, внесённое тем же коммитом, который +должен был его избежать: автор или ревьюер, читающий канон после мержа #698, +но до принятия #699, вправе решить, что небольшой дрейф `hostRefs`/`delegates` +после авторазрешения конфликта в пользу dev будет прощён «полосой» — а на деле +Validate уронит кандидата точным несовпадением, и время уйдёт на выяснение, +почему написанное в каноне не совпадает с поведением автоматизации. + +Не High: реальное расхождение по-прежнему красит Validate кандидата (см. «Что +проверено» — safety net не теряется), неверный бейзлайн не проходит в `dev` +молча. Но это Medium и в скоупе задачи — формулировку добавила именно она, +чинится точечной правкой в этом же issue, например: «текущий Validate +сравнивает число с `dev` точно (нулевой допуск для всех метрик, кроме +`bundleBytes`) — станет полосой после #699; до этого несовпадение красит +Validate кандидата, а не проходит тихо». + +### Low (сняты ревьюером, без правки) + +- `scripts/rebase-on-dev.mjs`: предиктивный `--dry-run`-лог + (`splitConflicts`/`predicted.manual`) не знает ни про `merge=union` в + `.gitattributes`, ни про `UPSTREAM_WINS` — при параллельной правке + `docs/CHANGELOG.md`/`docs/CHANGELOG.ru.md` или `scripts/monolith-baseline.json` + он печатает «менялись с обеих сторон... возможен ручной конфликт», хотя + фактический ребейз (несколько строк ниже, через `rebaseRegenerating`) + разрешает его автоматически. Чисто предсказательный текст для человека, + сам исход ребейза не меняет и уже проверен тестами выше. Снимаю: cosmetic, + не мешает ни одному AC этой задачи; дешевле поправить при следующей правке + файла, отдельный issue для строки лога избыточен. +- Риск «`merge=union` склеит содержательный конфликт двух правок одной старой + строки ченджлога в два дубля» — раскрыт самим автором в хендофф-комментарии + как принятый компромисс для append-only лога (дубль увидит релиз-менеджер + при сборке нот беты). Согласен с оценкой, не переоткрываю. + +## Чего не проверял + +- Живой прогон `merge-candidate.mjs::rebaseOnto` / `_process.yml` на реальном + GitHub Actions с настоящим конфликтом между двумя параллельными PR — не + воспроизводил. Логика проверена реальным git (временные репозитории в + тестах, не мок) и точечными ручными мутациями — это не то же самое, что + живой запуск конвейера на паре реальных веток. **Записываю явно: для факта + исполнения на реальном GitHub Actions — проверено чтением и контролируемым + тестом на настоящем git, не живым запуском workflow.** +- Полный `npm test` (весь набор) не перегонял целиком — принят по зелёному + Validate на этом SHA (#343); точечно перегонял только файлы из дельты + (`rebase-generated`, `merge-candidate`, `rebase-on-dev`, `process-digests`) + плюс дешёвую половину `mutation-gate.test.mjs`. +- `npx tsc --noEmit`, `npm run build` + сверка трёх копий бандла, полный + `mutation-gate --check` (с пересборкой на мутанта) — не перегонял, приняты + по Validate; диапазон не трогает `src/**`. +- golden/скриншоты, браузерные смоки, `pytest tests_backend`, инварианты + модели, performance — не применимо: диапазон не трогает визуал, Python или + геометрию плана. +- Долгосрочная устойчивость find/replace-якорей трёх новых мутантов к + дальнейшим ребейзам `dev` — проверена только на текущем материале. +- Существо #697 и #699 — вне скоупа этой задачи, не проверял по существу, + только прочитал их текущий статус (`S1-new` у обоих) для находки Medium + выше. + +## Вердикт + +Единственная блокирующая находка — Medium, в скоупе: формулировка PROCESS.md, +добавленная этим же коммитом, описывает как факт «тест полосы на Validate» +для монолитного бейзлайна, которого сегодня нет (нулевой допуск для пяти из +шести чисел, полоса существует только для `bundleBytes` и не как результат +этой задачи). Сам механизм разрешения конфликтов — `.gitattributes` union, +`UPSTREAM_WINS`, `PATCH_ID_EXCLUDES` — реализован корректно, проверен +исполнением настоящего git и ручными мутациями по всем трём защищаемым +линиям, и не создаёт риска молчаливого слияния неверного числа (реальное +расхождение по-прежнему красит Validate кандидата). Без High это жёлтый +вердикт: формулировка канона должна отражать факт автоматизации, а не +опережать ещё не принятую задачу (#699). + +Вердикт: жёлтый · заход r1 · блокирующих циклов 1/2 · High: 0 · Medium: 1 → в задаче + +--- + + + +## Материал раунда + +- Ветка: `issue/698-rebase-auto-resolve`, коммит `ea2c000af5e4` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `cb640cf165f861e9b1f8827a5dc40df3c5435e77` + ``` + git log --all --format='%H %T' | grep cb640cf165f8 + ``` +- Тело issue: `e17d18976553c2da4042b649a2073c581aa5108f58a57cf7c2902b6d639859c0` +- Вердикт конвейера: `yellow` · High 0