Files
2026-10-01 03:25:24 +00:00

25 KiB
Raw Permalink Blame History

CODE-REVIEW-727-r2

Issue: #727 · этап code · трек ask · заход r2 · блокирующих циклов 0/4 Материал: git log --oneline origin/dev..HEAD / git diff origin/dev...HEAD, ровно 00d1a7b6c3e09072c3d3b3381ada3ab37e16aab3 (рабочая копия уже на нём)

Скоуп

Тот же контракт, что в r1: ночной режим пакетного ship-ревью на dev и переиспользование его результата гейтом беты по патч-набору (К1–К9, AC1–AC10 из ТЗ в теле #727, спец-ревью зелёное с r2, docs/reviews/SPEC-REVIEW-727-r2.md). Классы изменений — B (scripts/**, .github/workflows/**, test/**) и C (PROCESS.md, docs/process/REVIEWER.md); файлов класса A нет. User-Visible: no подтверждено — git diff origin/dev...HEAD --stat -- docs/CHANGELOG*.md пуст.

Почему есть r2, хотя r1 был зелёным. Пока шёл код-ревью r1 (материал 93be52eb поверх dev@52dc08a0), dev продвинулся на один коммит (слился 3bb1b534 — «the moon with any background», #718, несвязан с #727). Ветка issue/727-nightly-ship-review ребейзнулась на новую вершину dev@8fac66e1, патч-набор диффа изменил содержимое (другой patch-id), и комментарий автора в issue (2026-10-01T03:12:22Z) прямо зафиксировал: «Дифф изменился при ребейзе — вердикт к нему не применим (§2.10, #492) … новый заход ревью читает актуальный код». Это ровно случай из промпта ревьюера «ребейз на ушедший вперёд dev» — делаю полный разбор, а не только дельту: материал r1 (93be52eb) физически отсутствует в репозитории (git cat-file -t → объект не найден), сравнить дельту напрямую нечем.

Файлы диффа не изменились по составу и почти не изменились по объёму относительно r1: scripts/ship-review.mjs (+428/-…), scripts/reviews-archive.mjs, scripts/reviews-index.mjs, .github/workflows/_nightly.yml, .github/workflows/_ship-review.yml, PROCESS.md (§10.4, §11.7), docs/process/REVIEWER.md, test/ship-review.test.mjs (+580), test/nightly-workflow.test.mjs (+137), test/process-digests.test.mjs (+2); плюс три новых коммита-артефакта публикации r1 (docs/reviews/CODE-REVIEW-727-r1.md, два обновления docs/reviews/INDEX.md) — чисто документные, не продуктовый код. .github/workflows/ship-review.yml (тонкий файл) не тронут — подтверждено (git log origin/dev..HEAD -- .github/workflows/ship-review.yml пуст), как и заявляет ТЗ («входы и права тонких файлов уже в main»).

Как проверялось

Поскольку материал r1 недоступен для прямого git diff <SHA>..HEAD, разбор сделан заново и независимо от выводов r1 — не как повторная вера документу, а как самостоятельное построчное чтение:

  1. Эквивалентность содержимого r1→r2. git diff origin/dev...HEAD --stat даёт тот же список файлов и почти те же цифры строк, что называет r1 (scripts/ship-review.mjs 428, test/ship-review.test.mjs 580, test/nightly-workflow.test.mjs 137) — рабочий код не переписывался заново, только сместился контекст диффа при ребейзе. Родитель коммита e852ee2e — ровно текущий origin/dev (8fac66e1, git log -1 --format='%H %P'), то есть ребейз уже выполнен и ветка не отстаёт.
  2. Построчное независимое чтение всего диффа (не выжимка из r1, а собственное прочтение): git diff origin/dev...HEAD -- scripts/ship-review.mjs целиком (все новые функции — shipReviewMode, nightlyDocPath, shipDocPath, hasReleaseTrailer, countsForPatchSet, commitPatchIds, issuePatchSets, formatPatches/parsePatches, samePatchSet, shipCoverage, planShipReview, reviewSubject, renderShipBrief, anchorBlock/parseAnchorBlock, shipReviewProblems, highCommentBody, highCommentTargets, readRangeDocs); scripts/reviews-archive.mjs (ветка doc.nightly && STABLE_TAG_RE.test(doc.tag) в archivePlan, с проверкой, что ordered в этом файле отсортирован по возрастанию, — то есть .find() действительно берёт ближайшую, не дальнюю линию, АС7(в)); scripts/reviews-index.mjs (SHIP_DOC_NAME с опциональной -dev-[0-9a-f]{12} группой, parseDocName, сортировка renderIndex); .github/workflows/_nightly.yml и _ship-review.yml целиком (порядок шагов, outputs:, if: always(), continue-on-error: true, источник mode/patches публикации); PROCESS.md §10.4/§11.7 и docs/process/REVIEWER.md диффы; весь новый текст test/ship-review.test.mjs и test/nightly-workflow.test.mjs.
  3. Каждый AC1–AC9 тела issue сверен с этим независимым чтением (оракул AC → функция/шаг → тест). AC10 — гейты, ниже.
  4. Тело issue #727 и комментарии прочитаны через gh issue view 727 --json comments (MCP-инструменты mcp__github__get_issue* отказали разрешением в этой песочнице — gh CLI сработал напрямую, уже аутентифицирован как github-actions[bot]). Контракт К1–К9 не менялся с r1/спец-ревью r2; последний комментарий автора — объяснение самого ребейза (выше).

Исполнено, не только прочитано:

  • node --test test/ship-review.test.mjs test/nightly-workflow.test.mjs test/process-digests.test.mjs — 29/29 зелёных; отдельно test/default-branch-workflows.test.mjs — 45/45 зелёных.
  • node scripts/mutation-gate.mjs --check — exit 0, якорь ship-review-ignores-merge-marker и мутанты ship-review-accepts-partial-coverage, ship-review-accepts-high зелёные. Уточнение к r1: эти три мутанта заведены ещё в #696 (git log -L по scripts/mutation-registry.mjs — коммит e1ae8f4a), не новые в #727; scripts/mutation-registry.mjs в дельте origin/dev...HEAD не меняется. Проверено, что их find-строки (if (missing.length) {, else if (block.high > 0) problems.push(, return names.includes('track:ship') || comments.some(...)) совпадают дословно с переписанным кодом shipReviewProblems — переписывание функции в #727 не осиротило мутанты молча.
  • node scripts/entry-cost.mjs --check — зелёный (author/reviewer/canon в бюджете слов).
  • node scripts/reviews-index.mjs --check — «INDEX.md свеж»; независимо пересчитан файл: ls docs/reviews/*.md | grep -v INDEX | wc -l → 224, совпадает со строкой заголовка индекса.
  • node scripts/smoke-select.mjs --base origin/dev --head HEAD — «исполняемого frontend-диффа нет, browser-smoke не выбираются»: src/**/*.ts не тронут.
  • Мутационная проверка вручную (дисциплина «тест умеет падать», независимо от r1 — тот же метод, своя команда): временно cp бэкап, правка, прогон, восстановление, git diff --stat после — пусто.
    • samePatchSet → return true всегда: упало 5 из 19 тестов ship-review.test.mjs (AC3, AC4, AC5, АС7-тест, сквозной _ship-review.yml-тест) — покрытие никогда не становится stale.
    • archivePlan: ordered.find(...) → [...ordered].reverse().find(...): упал ровно тест АС7 (сценарий в, [v1.78.0, v1.78.1, v1.79.0] обязан дать v1.78.1, мутант даёт v1.79.0). Оба файла восстановлены из бэкапа, git diff --stat — пусто, полный прогон после восстановления снова 29/29.
  • Трейлеры всех четырёх коммитов диапазона (git log -1 --format=%B <sha> на e852ee2e, 60581038, ac4dd1c5, 00d1a7b6): Issue: #727, User-Visible: no на каждом; Co-Authored-By/Claude-Session — на коммите продуктового кода.
  • Проверка закрытия Low-наблюдения r1 (issue E «Красная ночь» из ТЗ) — gh search issues/gh issue list --label S1-new: находка снята, см. «Закрытие раунда r1».

Не прогонял и почему: actionlint — бинарь недоступен в этой песочнице (which actionlint → код 1), как и в r1; риск тот же и так же низкий — YAML run:-блоки проверяются bash -n в исполняемых тестах, структурные инварианты — текстовыми assert’ами на реальном файле (см. stepRun/job в test/nightly-workflow.test.mjs, выдержки выше). npx tsc --noEmit / npm test (полный) / npm run build — не перегонял: Validate зелёный на этом точном SHA 00d1a7b6 (ссылка в промпте ревью, прогон 36809481316), дешёвые гейты этим закрыты. Python/pytest tests_backend — нет правки custom_components/**/*.py. npm run golden:verify/invariants — нет ci:golden, нет правки геометрии/рендера. Performance — не назван в AC. Живой ночной прогон на реальном GitHub Actions — не воспроизводим в песочнице ревью, остаётся риском первой ночи, как и в r1.

Закрытие раунда r1

Находка r1 Чем закрыта Где это видно
Low (наблюдение): ТЗ требует завести issue E («Красная ночь: комментарий в задачи, слитые после последней зелёной», S1-new, ссылка на #727) — на момент r1 такой issue не найден Issue заведён gh issue view 736 — заголовок дословно совпадает с текстом ТЗ, тело «Выделено из #727 (issue E его ТЗ)», создан 2026-10-01T03:01:10Z, метки infra, S1-new, process
Low (наблюдение): readRangeDocs не имеет прямого unit-теста на дедупликацию «кандидат vs devRef» Не предмет этого раунда — не находка, требующая действия (r1 явно отметил «риск низкий, повторной сессии не прошу»); код readRangeDocs не менялся между r1 и r2 (дифф идентичен), логика перечитана заново в этом раунде и подтверждена (см. «Как проверялось» п.2) scripts/ship-review.mjs, функция readRangeDocs

Других находок r1 не было (High 0, Medium 0 в r1).

Унаследовано из r1 (без повторной проверки, с явной переподтверждённой частью)

Этот раунд — не формальная дельта (материал r1 недоступен для diff), поэтому весь AC-разбор выполнен заново и независимо (раздел «Как проверялось»), а не унаследован слепо. Без повторного исполнения приняты только факты среды, которые не зависят от содержимого диффа #727 и совпадают с r1 (docs/reviews/CODE-REVIEW-727-r1.md, материал 93be52eb, ныне недостижим как объект — ребейз, но содержательно эквивалентен текущему e852ee2e, доказательство — «Как проверялось» п.1):

  • track:ship и маркер hp:ship-merge как признак ship-задачи (isShipIssue, не изменился с #696/#716, вне диффа #727);
  • поведение git patch-id --stable и git diff-tree как документированное поведение инструментов, не специфика кода задачи;
  • трек ask обоснован (сложность/риск >3, публичный контракт §11.7) — подтверждено спец-ревью r1/r2, код не меняет это решение.

Точечно переподтверждено в этом раунде (не просто унаследовано): все функции, которые r1 называет по имени, перечитаны в текущем диффе заново (список в «Как проверялось» п.2) — совпадение не предполагается, а показано.

Что проверено и корректно

  • Патч-набор по-прежнему исключает ровно Release:-коммит кандидата беты и коммиты только docs/reviews/** — иначе ночной набор никогда не совпал бы с набором на кандидате беты; логика (hasReleaseTrailer, countsForPatchSet) не изменилась между r1 и r2 (идентичный текст в обоих диффах).
  • archivePlan: новая ветка doc.nightly && STABLE_TAG_RE.test(doc.tag) корректно использует уже отсортированный по возрастанию ordered ([...lines].sort((a, b) => compareStable(a.tag, b.tag)), строка 79) — .find(line => compareStable(line.tag, doc.tag) > 0) берёт ближайшую, а не последнюю линию новее базы; без линии новее — kept с объяснением. Это прочитано мной заново (не со слов r1) и подтверждено мутацией «переворот на .reverse().find» (упал АС7-тест).
  • _nightly.yml: head_sha — выход job dispatch, записанный шагом id: validate до шага ожидания (gh run watch --exit-status) — красный Validate не теряет SHA кандидата. ship_review зависит только от dispatch (if: always(), continue-on-error: true) — отказ диспетчеризации ship-ревью не красит ночь. Оба факта перечитаны в текущем YAML, не взяты на веру.
  • _ship-review.yml: mode/patches публикуемого блока — из выходов prepare (needs.prepare.outputs.mode/.patches), не из result.json модели — единственный источник числа patch-id не раздваивается (§8). Строка о High ночью (needs.prepare.outputs.mode == 'nightly') пишется через comment-high, который сам же отказывается молча при mode != nightly или high == 0 — двойная защита от шума в бета-режиме.
  • scripts/mutation-registry.mjs не входит в дельту #727: три мутанта, которые использует этот код, принадлежат #696 и остаются валидными после переписывания shipReviewProblems (их find-строки совпадают дословно).
  • Индекс (docs/reviews/INDEX.md) после публикации r1 корректен: счётчик документов (224) совпадает с фактическим числом файлов, новая строка #727 code · r1 · 🟢 на месте, порядок (ночь/бета/релиз, затем issue по убыванию) не нарушен.
  • Трейлеры всех коммитов диапазона корректны; изменений в обоих CHANGELOG нет, что соответствует User-Visible: no.
  • Единственное Low-наблюдение r1 закрыто заведением issue #736.

Проверка критериев приёмки

Разбор не изменился по существу относительно r1 (код идентичен содержательно — см. «Как проверялось» п.1), но выполнен заново по текущему коду, а не скопирован:

AC Что Доказательство в коде Тест Вердикт
AC1 К1 Патч-набор: git patch-id --stable, без Release:, без коммитов только docs/reviews/**, порядок не важен, cherry-pick даёт тот же id hasReleaseTrailer, countsForPatchSet, commitPatchIds (явные опции PATCH_DIFF), issuePatchSets #727 AC1 — реальный git в temp-репозитории доказано исполнением
AC2 К2 tag=nightly требует candidate; имя документа по базе+SHA12; prepare отдаёт только none/stale; блок несёт mode/patches в хвосте shipReviewMode, nightlyDocPath, shipDocPath, anchorBlock/parseAnchorBlock #727 AC2 (unit) + #727 AC2/AC5 _ship-review.yml (реальный bash) доказано исполнением
AC3 К3 Четыре статуса покрытия; «последний документ главнее»; чужая база не в счёт; документ без patches — по номеру shipCoverage #727 AC3 доказано исполнением; мутация samePatchSet→true ловится (перепроверено лично)
AC4 К4 Гейт: всё clean без документа тега проходит; none/stale/high — отказ с командой shipReviewProblems #727 AC4 доказано исполнением
AC5 К5 Бета читает дельту; бриф называет прочитанное ночью; force=true — всё; пустая дельта — без модели planShipReview, renderShipBrief #727 AC5 + #727 AC2/AC5 (сквозной, реальный bash) доказано исполнением
AC6 К6 Новая job после Validate, if: always(), не красит ночь, ждёт только появления прогона (18×10с) .github/workflows/_nightly.yml (ship_review) #727 AC6 + два теста на реальном bash (runStep) доказано исполнением; порядок шагов/outputs: перечитан лично
AC7 К7 parseDocName узнаёт ночное имя; archivePlan: база-бета → своя линия, стабильная база → ближайшая новее, без линии новее — kept parseDocName (SHIP_DOC_NAME с опциональной -dev-sha12), archivePlan, renderIndex #727 AC7 — все четыре примера (а)-(г) доказано исполнением; мутация find→reverse().find перепроверена лично, ловится
AC8 К8 Строка о High только ночью, с меткой документа, без повтора highCommentBody, highCommentTargets, шаг «High ночью» (if: needs.prepare.outputs.mode == 'nightly') #727 AC8 + #727 AC8 _ship-review.yml (реальный bash) доказано исполнением
AC9 К9 Канон — PROCESS.md §11.7/§10.4, REVIEWER.md, ключевое правило в process-digests диффы прочитаны целиком лично node --test test/process-digests.test.mjs зелёный; entry-cost --check зелёный доказано исполнением
AC10 Гейты — mutation-gate --check, reviews-index --check, entry-cost --check — перепрогнаны мной в этом раунде, зелёные доказано исполнением (моим прогоном в r2)

Чего не проверял

  • actionlint — бинарь недоступен в песочнице (как в r1); заявление автора не перепроверено независимо, риск низкий (см. «Как проверялось»).
  • Полные npx tsc --noEmit / npm test (весь набор) / npm run build — не перегонял: зелёный Validate на точном SHA 00d1a7b6 (ссылка в промпте) закрывает дешёвые гейты для этого материала.
  • Живой ночной прогон (_nightly.yml → ship-review.yml -f tag=nightly на настоящем GitHub Actions, включая git describe с реальными тегами при публикации) — не воспроизводим до продакшена; контрактные тесты на реальном bash/git закрывают то, что можно закрыть без него. Первая реальная ночь остаётся риском, как и в r1.
  • Browser-smoke, golden, performance, HA-pytest — объективно не применимы (src/**/*.ts, геометрия/рендер, custom_components/**/*.py не тронуты).
  • Прямой доступ к issue через MCP (mcp__github__get_issue*) — инструмент отказал разрешением в этой песочнице; использован gh CLI напрямую (уже аутентифицирован), результат тот же набор данных.

Вердикт

Все AC1–AC9 проверены заново и независимо (не унаследованы слепо из r1, так как материал r1 стал недостижим после ребейза на ушедший вперёд dev) — построчным чтением всего диффа плюс исполнением тестов и двух ручных мутаций на самых защитных участках (samePatchSet, выбор архивной линии в archivePlan), обе ловятся тестами. AC10 — гейты пересняты в этом раунде. Содержимое диффа эквивалентно тому, что видел r1 (тот же состав файлов, те же объёмы правок, родитель коммита — ровно текущий origin/dev): разница — это ребейз на влившийся параллельно и несвязанный #718, что явно подтвердил и автор в issue. High и Medium нет. Единственное Low-наблюдение r1 закрыто (issue #736 заведён). Новых находок в этом раунде нет.

Зелёный.


Материал раунда

  • Ветка: issue/727-nightly-ship-review, коммит 00d1a7b6c3e0 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: c1b179ff64fac8b2b842b513e73a7524cae3b957
    git log --all --format='%H %T' | grep c1b179ff64fa
    
  • Тело issue: 8ce2942ca915bc938c8c5b4a720bb71d5a06d18c13db0692ee802f6a55662b7a
  • Вердикт конвейера: green · High 0 · маршрут fix