docs: review document for #727

Issue: #727
User-Visible: no
This commit is contained in:
claude[bot]
2026-09-30 23:33:52 +00:00
parent 5ce83feffa
commit 3847baa5a0
2 changed files with 247 additions and 1 deletions
+2 -1
View File
@@ -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 | — | — |
+245
View File
@@ -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-<sha12>.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<N-1>», §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`,
якорь `<!-- hp-ship-review-anchors -->`, поля `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-<sha12>` с `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`.
---
<!-- material-anchors: сгенерировано конвейером (#414) -->
## Материал раунда
- Ветка: `dev`, коммит `40607aa37f13` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
- Дерево материала: `901e6cbd1964ef2dcd9a0e08e8ec01a5c47dcac7`
```
git log --all --format='%H %T' | grep 901e6cbd1964
```
- Тело issue: `1c9e15704953110b6c7e68babaa7500b8d3e2331e4583fcfff52d41462321e7a`
- Вердикт конвейера: `yellow` · High 0