diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index ce5b565c..eef52f4b 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -1,11 +1,12 @@ # Индекс ревью -Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 215, issue: 107. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. +Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 216, issue: 108. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`. | Issue | Документ | Этап · раунд | Вердикт | H | M | Находки | Файлы | |---|---|---|---|---:|---:|---|---| | бета v1.79.0-beta.1 | [SHIP-REVIEW-v1.79.0-beta.1.md](SHIP-REVIEW-v1.79.0-beta.1.md) | пакетное ревью ship · — | ⚪ — | 0 | 0 | — | — | | #732 | [CODE-REVIEW-732-r1.md](CODE-REVIEW-732-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | +| #727 | [SPEC-REVIEW-727-r1.md](SPEC-REVIEW-727-r1.md) | spec · r1 | 🟡 жёлтый | 0 | 1 | К7/AC7: архивирование ночного документа с базой-стабильным-тегом не имеет ни одного про… | `scripts/reviews-archive.mjs` | | #726 | [SPEC-REVIEW-726-r1.md](SPEC-REVIEW-726-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — | | #725 | [SPEC-REVIEW-725-r1.md](SPEC-REVIEW-725-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | устаревший номер строки в «Проблема» п.3 / «Не-скоуп» | `src/iso-scene-render.ts` `src/houseplan-card.ts` `houseplan-card.ts` `header-menu.ts` `iso-scene-render.ts` | | #724 | [CODE-REVIEW-724-r1.md](CODE-REVIEW-724-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — | diff --git a/docs/reviews/SPEC-REVIEW-727-r1.md b/docs/reviews/SPEC-REVIEW-727-r1.md new file mode 100644 index 00000000..e63736ea --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-727-r1.md @@ -0,0 +1,245 @@ +# SPEC-REVIEW-727-r1 + +Issue: #727 · этап: spec · трек: ask · заход: r1 · блокирующих циклов израсходовано (после этого раунда): 1/4 + +## Скоуп + +#727 — первая из двух независимых частей, выделенных из #707 п.3: ночное +пакетное ship-ревью на `dev` и переиспользование его результата в гейте беты +(`ship-review.mjs check`). Контракт — К1 (патч-набор задачи по `git patch-id +--stable`), К2 (ночной режим `tag=nightly`, имя документа +`SHIP-REVIEW-<база>-dev-.md`), К3 (`shipCoverage`: `clean`/`high`/ +`stale`/`none`), К4 (гейт `check` по покрытию), К5 (ревью беты читает только +дельту `none`/`stale`), К6 (job в `_nightly.yml`, dispatch `ship-review.yml` с +`tag=nightly`), К7 (индекс и архив узнают ночное имя), К8 (комментарий в +задачу при High), К9 (канон — PROCESS.md §10.4/§11.7, REVIEWER.md). Красная +ночь (комментарий по задачам, слитым после последней зелёной) — отдельный +issue E, в #727 не входит. Продуктового кода задача не трогает (класс A +файлов нет), `User-Visible: no`. ТЗ живёт в теле issue под `## ТЗ`; комментарий +владельца 2026-09-30 передаёт на ревью с треком `ask`, обоснование которого — +первая строка раздела «ТЗ» (сложность/риск >3: четыре поверхности; публичный +контракт: правило гейта беты §11.7 меняется с «документ тега покрывает все +ship-задачи» на «задачи покрыты документами диапазона с тем же патч-набором») +— соответствует критериям §5, трек `ask` обоснован корректно. + +## Как проверялось + +Ревью текстовое (этап spec, продуктовой ветки нет). Три слоя: + +1. Обязательные разделы §7.1 и однозначность каждого AC — чтением тела issue. +2. **Проверка каждого технического утверждения по текущему коду на материале + ревью** (`HEAD` рабочей копии `40607aa37f13819c9db37a392a1d99f30382b3c0`, + совпадает с `origin/dev`): ТЗ называет конкретные функции, регэкспы, имена + файлов, мутанты и исторические коммиты — каждое утверждение либо + подтверждается чтением, либо остаётся непроверенной догадкой (находка по + §7.1). Список проверенного — ниже. +3. Гейты не прогонялись: задача не меняет ни одного файла репозитория на этом + этапе (см. «Чего не проверял»). + +## Находки + +### Medium — К7/AC7: архивирование ночного документа с базой-стабильным-тегом не имеет ни одного проверяемого примера, а текущий код его не реализует + +**Файл:** тело issue #727, раздел «Контракт поведения» → К7, второй подпункт +(«база — стабильный тег → первая архивируемая стабильная линия новее базы»); +AC7 в таблице «Критерии приёмки». + +**Воспроизведение.** `archivePlan` (`scripts/reviews-archive.mjs:91-97`) сегодня +для `doc.stage === 'ship'` вычисляет единственную целевую линию так: + +```js +const line = doc.tag.replace(/-beta\.\d+$/, ''); +if (tags.has(line)) moves.push({ ... to: `${ARCHIVE_DIR}/${line}/${name}` ... }); +else kept.push({ name, reason: `ревью линии ${line} не входит в архивируемые линии` }); +``` + +Это находит ТОЧНОЕ совпадение (после снятия `-beta.N`) между базой документа и +одной из архивируемых стабильных линий. Для сегодняшнего единственного формата +имени (`SHIP-REVIEW-<тег-беты>.md`) это корректно: база всегда сама и есть та +линия, в которую документ должен уйти. + +К7 вводит новый случай — ночной документ, чья база (`doc.tag` после парсинга +нового формата К2/AC7) может быть **стабильным** тегом (ночь, прошедшая после +стабильного релиза и до первого `beta.1` следующей линии — это не редкий +случай, а обычное окно между двумя циклами). Для него К7 явно требует: «первая +архивируемая линия **новее** базы», то есть не точное совпадение, а поиск +следующего по порядку тега. Текущая реализация `archivePlan` такого поиска не +делает вообще: если `doc.tag` (стабильный) не входит дословно в `tags` +(например, база `v1.78.0`, а архивируется уже `v1.79.0`), ветка попадёт в +`else` и документ останется в `docs/reviews/` с причиной «линия не входит в +архивируемые», а не переедет в `v1.79.0`, как того хочет К7. Это значит: правило +К7 для этой ветки требует новой логики в `archivePlan` (поиск ближайшего +большего тега, не поиск по равенству), а не простого использования +существующей функции. + +AC7 при этом даёт ровно три конкретных примера — `parseDocName(...)` на новое +имя, `renderIndex` его перечисляет, `archivePlan` уносит документ с базой-бетой +в `v1.79.0` (это старая, уже поддержанная ветка кода). Для ветки «база — +стабильный тег» AC7 ограничивается пересказом самого правила («База `v1.79.0` +→ первая архивируемая линия новее базы») без единого конкретного входа и +ожидаемого выхода: какая база, какой набор архивируемых тегов, какой +результат. Без такого примера реализатор и ревьюер кода не смогут отличить +осмысленную имплементацию нового алгоритма от случайной: нет фиксированной +пары «вход → выход», по которой можно требовать тест, а «способ доказательства» +AC7 называет только файл и раздел теста, не конкретный случай. + +**Почему это находка, а не мелочь.** Правило К7 в этой части меняет публичный +контракт архивирования (куда физически переедет документ, на который будущие +ревью ссылаются как на «унаследовано из r», §2.10) — ошибка здесь тихая: +`archivePlan` возвращает план, который применяется git-мультивом в `--apply`, и +неверный путь обнаружится не тестом, а человеком, читающим архив postfactum. +Ветка реально достижима (первое же окно между релизом и следующей бетой), не +теоретический край. + +**Что нужно от автора.** Добавить в AC7 (или в само К7) один конкретный пример +для ветки «база — стабильный тег»: например, «архивируемые линии +`[v1.78.0, v1.79.0]`, документ с базой `v1.78.0` (стабильный) → переезжает в +`v1.79.0`» и отрицательный случай «архивируемых линий новее базы ещё нет → +документ остаётся (`kept`), как сегодня для тега, которого нет в `tags`». Это +дёшево (одна строка в таблице AC) и снимает риск, что «первая линия новее +базы» останется нереализованной или реализованной с ошибкой в граничном +случае (нет более новой линии, несколько линий новее одновременно). + +Без High в задаче это жёлтый вердикт, правка ТЗ и повторный цикл (§2.4, §4). + +## Что проверено и корректно + +ТЗ необычно тщательно сверено с текущим кодом и историей; кроме находки выше, +все проверенные утверждения подтвердились дословно: + +- Существование и сигнатуры `readCandidateHistory`, `issueTrailers` + (`scripts/release-membership.mjs`, импортируются в `scripts/ship-review.mjs:28`), + `shipReviewDocPath`, `isShipIssue`, `specSection`, `shipIssuesInRange`, + `renderShipBrief`, `anchorBlock`, `parseAnchorBlock`, `shipReviewProblems`, + `readShipDoc` в `scripts/ship-review.mjs` — все существуют как названо. +- `RELEASE_TAG_RE` (`scripts/ship-review.mjs:33`) и `shipReviewDocPath` + (падает на нерелизном теге) — подтверждают, что литеральное `tag=nightly` + не может случайно совпасть с реальным тегом релиза (риск «неявный вход», + который ТЗ сам называет и откладывает). +- Текущий машинный блок документа (`docs/reviews/SHIP-REVIEW-v1.79.0-beta.1.md`, + якорь ``, поля `tag/candidate/base/issues/ + high/medium/low/run`) совпадает построчно с тем, что генерирует `anchorBlock` + — К2's «прежние строки блока не меняются», «дополняется mode/patches» + корректно описывает расширение, а не переписывание существующего формата. +- Существующий тег `v1.79.0-beta.1` и документ `SHIP-REVIEW-v1.79.0-beta.1.md` + реальны (нет фантомных примеров); пример имени ночного документа в К2/AC7 + (`SHIP-REVIEW-v1.79.0-beta.1-dev-108427dc1234.md`) корректно использует + реальный тег как базу. +- Коммит `dca0fd28` («Release v1.79.0-beta.1 candidate») реально несёт трейлеры + `Issue: #661/#692/#693/#711/#713` и `Release: v1.79.0-beta.1` — обоснование + исключения `Release:`-коммитов из патч-набора (К1) фактически точное, не + гипотетическое. +- `SHIP_DOC_NAME` в `scripts/reviews-index.mjs:32` и `parseDocName` + (`scripts/reviews-index.mjs:45-58`) сегодня действительно узнают только + `SHIP-REVIEW-<тег>.md` без суффикса — К7's требование расширить регэксп под + `-dev-` с `nightly: true` корректно описывает необходимое изменение, + а не несуществующий пробел. +- `parseDocName`/`archivePlan` в `scripts/reviews-archive.mjs` импортируются и + используются именно так, как описывает К7 (кроме разобранного пробела). +- Существующий тест `test/nightly-workflow.test.mjs` (29 строк) уже содержит + `test('nightly ждёт запущенный Validate и падает вместе с ним (#492 §7)', …)` + и тест про русское имя job — ровно те тесты, которые АC6 требует оставить + зелёными; файл не новый, «Затронутые файлы» его называет верно. +- Текущий `_nightly.yml` — одна job `dispatch`, делает `gh workflow run + validate.yml --repo "$REPO" --ref dev -f full=true` токеном + `GH_TOKEN: ${{ github.token }}`, ждёт появления прогона до 3 минут (18×10с) + — это ровно прецедент, на который К6 ссылается («как у dispatch Validate», + «до трёх минут, как у Validate»); новая job для ship-review по аналогии + технически реализуема без смены токена или прав. +- `ship-review.yml` (тонкий, `main`) сегодня действительно объявляет входы + `tag` (required), `candidate` (optional, default `""`), `force` (optional + boolean) и права `contents: read` / job `dev`: `contents: read, issues: read` + — К6's «тонкие файлы не меняются: входы... и права... уже есть в main» + подтверждается дословно; `tag=nightly` не требует нового входа. +- `HP_PROCESS_TOKEN` — существующий секрет, уже используется для публикации + комментариев/меток конвейером (`_process.yml`, `_process-resume.yml`, + `_beta-derived.yml` и др.) — К8's «Токен — HP_PROCESS_TOKEN, как у + публикации» ссылается на реальный, а не придуманный прецедент; в + `_ship-review.yml` сегодня комментариев в issue нет — это действительно + новая часть работы, и ТЗ её как новую не маскирует. +- Мутант `ship-review-ignores-merge-marker` существует в + `scripts/mutation-registry.mjs:13732` (AC10). +- `node scripts/entry-cost.mjs --check` — реальная команда + (`scripts/entry-cost.mjs:12,75`), AC9 её не выдумывает. +- `test/process-digests.test.mjs`, `test/ship-review.test.mjs` существуют — + план автотестов ссылается на реальные файлы, не на планируемые с нуля + (кроме `test/nightly-workflow.test.mjs`, который тоже существует). +- Обязательные разделы §7.1 (сценарий, что человек увидит, проблема, скоуп и + не-скоуп, контракт поведения, UX/данные/миграция/i18n/perf/touch, критерии + приёмки AC1–AC10 со способом доказательства, план автотестов, риски, откат, + release-артефакты) — присутствуют, в правильном порядке. +- «Принято предположительно» — 6 пунктов, все — реальные технические развилки + (способ dispatch, гранулярность severity по документу vs по задаче, + критерий «последний документ», имя файла), ни один не маскирует продуктовый + вопрос: гранулярность «High снимает покрытие со всех задач документа» + совпадает с уже действующим сегодня правилом гейта §11.7 («документ обязан + покрывать их все и не нести High» — это уже документ-уровневый, а не + задаче-уровневый гейт), так что это не новый прецедент, а перенос старого. +- Факты сверены автором с `origin/dev 108427dc` — коммиты между этим SHA и + материалом ревью (`git diff 108427dc..40607aa3 --stat`) не затрагивают ни + один из файлов, которые называет ТЗ (`scripts/ship-review.mjs`, + `scripts/reviews-index.mjs`, `scripts/reviews-archive.mjs`, + `.github/workflows/_nightly.yml`, `.github/workflows/_ship-review.yml`, + `.github/workflows/ship-review.yml`) — материал не устарел за эти 4 коммита. +- Продуктовых вопросов владельцу в ТЗ нет; единственный открытый пункт, + который я бы мог счесть техническим спором (гранулярность severity), + оказался уже действующим прецедентом — оспаривать нечего. + +## Чего не проверял + +- **Гейты (`npx tsc --noEmit`, `npm test`, `npm run build`) не прогонял** — + задача не меняет ни одного файла репозитория на этом этапе; они станут + обязательны на `S7-code-review` к реальному диффу. +- Не проверял golden/смоки/бэкенд-pytest/инварианты модели — задача класса A + не содержит, `User-Visible: no`, геометрии и визуала не касается (сама ТЗ + это явно фиксирует в разделе «UX · данные · i18n · миграция · perf · touch» + и «Release-артефакты»). +- Не проверял корректность `git patch-id --stable` на реальных cherry-pick + сценариях (АС1 «тот же дифф в другом коммите даёт тот же patch-id») — + это задокументированное поведение самой команды git, не специфика этого + кода; проверка на встроенном временном git-репозитории — задача автора + теста, не спецификации. +- Не оценивал реальную стоимость job `prepare` ship-ревью в ночи («около + минуты, без npm ci») — измеримо только на первом живом прогоне; ТЗ сама + называет первый живой прогон «наблюдением, не AC» (раздел «Риски»). +- Не проверял, действительно ли GitHub допускает `workflow_dispatch` через + `gh workflow run` с `GITHUB_TOKEN` для **другого** workflow-файла + (`ship-review.yml`) из job с правом `actions: write`, отличным от той же + операции для `validate.yml` в существующем коде, — по документированному + поведению GitHub Actions разницы по целевому workflow нет (`workflow_dispatch` + API не различает вызываемый файл), и это тот же механизм, что уже работает в + `_nightly.yml`; отдельно на реальном runner не проверял. + +## Вердикт + +Один Medium **в скоупе задачи** (К7/AC7: правило архивирования ночного +документа с базой-стабильным-тегом не реализуемо существующим `archivePlan` и +не имеет ни одного конкретного примера входа/выхода в AC) — без High это +жёлтый вердикт с возвратом автору по §2.4. Все прочие технические утверждения +ТЗ — имена функций, регэкспы, номера строк, реальные теги и коммиты — сверены +построчно с `dev` и подтвердились; трек `ask` обоснован верно; продуктовых +вопросов владельцу нет. + +**Вердикт: жёлтый · заход r1 · блокирующих циклов 1/4 · High: 0 · Medium: 1 → в задаче** + +## Материал раунда + +- Этап: spec. Материал — тело issue #727 на момент комментария Matysh + 2026-09-30T23:25:21Z («Оценка и ТЗ: трек ask… Передаю на ревью ТЗ»). +- Кода/ветки продукта не существует (инфраструктурная задача до реализации); + факты сверены с рабочей копией на `HEAD` = `origin/dev` = + `40607aa37f13819c9db37a392a1d99f30382b3c0`. + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `40607aa37f13` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `901e6cbd1964ef2dcd9a0e08e8ec01a5c47dcac7` + ``` + git log --all --format='%H %T' | grep 901e6cbd1964 + ``` +- Тело issue: `1c9e15704953110b6c7e68babaa7500b8d3e2331e4583fcfff52d41462321e7a` +- Вердикт конвейера: `yellow` · High 0