docs: review document for #728

Issue: #728
User-Visible: no
This commit is contained in:
claude[bot]
2026-09-30 23:34:30 +00:00
parent 3847baa5a0
commit a49f7095ce
2 changed files with 253 additions and 1 deletions
+2 -1
View File
@@ -1,11 +1,12 @@
# Индекс ревью
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 216, issue: 108. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). H/M — число High/Medium по строке вердикта или заголовкам находок. Файлы — пути, названные в находках; ищите по имени файла: `grep form-kit INDEX.md`.
Генерируется `node scripts/reviews-index.mjs` (#635) — не редактировать руками. Документов: 217, issue: 109. Вердикт: 🟢 зелёный · 🟡 жёлтый · 🔴 красный · ⚪ не распознан (свободная форма старых документов). 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 | — | — |
| #728 | [SPEC-REVIEW-728-r1.md](SPEC-REVIEW-728-r1.md) | spec · r1 | 🟡 жёлтый | 1 | 0 | 1. АС3/К3: заявленное различение причин validate-red и conflict «на текстах из констант…; 2. АС8: заявленное доказательство правки process-metrics.yml (fetch-depth: 0, timeout-m… | `wait-verdict.mjs` `review-doc-guard.mjs` `scripts/wait-verdict.mjs` `.github/workflows/_process.yml` `_process.yml` `_process-metrics.yml` `test/process-metrics.test.mjs` |
| #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` |
+251
View File
@@ -0,0 +1,251 @@
# SPEC-REVIEW-728-r1 — «Метрики процесса по трекам: отрезки, причины возврата, job-минуты, сравнение до/после»
Issue: [#728](https://github.com/Matysh/houseplan-card/issues/728)
Этап: spec · трек `ask`
Заход: r1 (первый; разделы «Унаследовано из r0» и «Закрытие раунда r0» не нужны — §2.10 применяется со второго захода)
## Вердикт
**Жёлтый.** High: 1. Medium в скоупе: 1. Medium вне скоупа: 0. Low: 0.
## Скоуп разбора
Полный разбор: тело issue #728 целиком (раздел `## ТЗ` и всё, на что он ссылается
— «Проблема», «Предложение») и единственный комментарий (оценка/трек). Прочитаны
`docs/SCOPE.md`, `AGENTS.md`, `docs/process/REVIEWER.md`, `PROCESS.md` §1, §2.3–
§2.5, §4, §5/§5.1, §7.1, §7.2.
Почти все содержательные утверждения ТЗ — проверяемые факты о текущем коде, а не
только контракт для будущей реализации, поэтому разбор включает чтение
затронутых и импортируемых модулей на `dev`/`HEAD` (`40607aa3`; ТЗ цитирует
`origin/dev` `108427dc` — между ними нет ни одного коммита, трогающего
перечисленные файлы, проверено `git log --oneline 108427dc..HEAD -- scripts/
process-metrics.mjs scripts/process-track.mjs scripts/wait-verdict.mjs scripts/
review-doc-guard.mjs scripts/ship-review.mjs .github/workflows/_process-metrics.yml
.github/workflows/_process.yml` → пусто, факты актуальны и на материале ревью):
- `scripts/process-metrics.mjs` (306 строк) — текущий `issueMetrics`/`buildReport`/
`renderMarkdown`/`fetchSnapshot`, описанные в разделе «Проблема» как отправная
точка;
- `scripts/process-track.mjs` — `trackFromLabels`, `hasTrackLabel`, `resolveTrack`
(именно отсюда К1 предлагает взять приоритет меток и правило «метки нет →
show для инфраструктуры/ask для продукта»), импортирует `classify` из
`./process-gate.mjs`;
- `scripts/change-classes.mjs` — источник `classify` (класс A/B/C/D по путям);
`process-gate.mjs` реэкспортирует её — ссылка ТЗ на `change-classes.mjs` как
источник `classify` фактически верна;
- `scripts/wait-verdict.mjs` — `PIPELINE_EVENTS` (6 записей: `conflict`, `stale`,
`merge-conflict`, `failure`, `exhausted`, `refused`) и точные regex/тексты;
- `scripts/review-doc-guard.mjs` — `verdictDeclaration`, `isBlockingVerdict` и
соседние экспорты;
- `scripts/ship-review.mjs` — `anchorBlock`/`parseAnchorBlock` (формат
`issues a,b` / `high N` / `medium N` / `low N`, один агрегат на документ, без
разбивки по задаче — то, что и описывает АС4);
- `.github/workflows/_process.yml` — имена jobs (`Страж: ребейз на dev и
предпосылки ревью` → `guard`, `Ревью: материал и deterministic gates` →
`prepare`, `Ревью: работа модели` → `model_review`, `Ревью: публикация и
интеграция` → `integrate`) и оба текста комментария «Ревью не запускалось:»
(строки 708 и 850, см. находку High);
- `.github/workflows/_process-metrics.yml` — текущие `fetch-depth: 1`,
`timeout-minutes: 15` (см. находку Medium);
- `test/process-metrics.test.mjs` (157 строк) — все тесты `#637`/`#682`, на
зелёность которых ссылается АС8, прочитаны целиком, включая утверждения,
которые они **не** делают;
- `test/process-track.test.mjs` — подтверждён приём «временный git-репозиторий
через `mkdtempSync`», на который ссылается АС7.
## Проверка §7.1 — комплектность
Присутствуют все обязательные разделы: сценарий · что человек увидит до и после
· проблема · скоуп и не-скоуп · контракт поведения (К1–К8) · UX/данные/i18n/
миграция/perf/touch · граничные случаи · критерии приёмки AC1–AC9 с
доказательством и oracle · план автотестов (включая столбец «чем краснеет») ·
риски · откат · release-артефакты. Блок «Принято предположительно» (7 пунктов)
— все технические (окна, границы корзин, формат причин), ни один не подменяет
продуктовое решение. Трек `ask` обоснован по §5: критерий «ожидаемое поведение
не зафиксировано» и «сложность > 3» (шесть источников данных) — оба
применимы, задача — инфраструктура (только файлы класса B), и именно поэтому
не идёт по умолчанию `show`. Обоснование корректно, продуктовых вопросов
владельцу нет (это чисто инфраструктурный, только-чтение отчёт, что и
разрешает не выносить вопросы за пределы техники).
## Находки
### High (блокирует)
**1. АС3/К3: заявленное различение причин `validate-red` и `conflict` «на
текстах из констант `wait-verdict.mjs`» не подтверждается кодом — единственный
существующий признак сливает оба случая в одну причину.**
Таблица К3 требует, чтобы `trackAt`-подобный классификатор различал шесть
причин возврата, в том числе `validate-red` (красный Validate) и `conflict`
(конфликт ребейза на `dev`) как **разные** строки, и оба находятся «по
`**Ревью не запускалось:**` о …» — с разным продолжением. АС3 прямо требует
«по одному возврату на каждую причину таблицы К3 **на текстах из констант**
`wait-verdict.mjs`/`review-doc-guard.mjs`», а раздел «Риски» обещает: «Мера:
константы импортируются из тех же модулей. Их переименование ломает импорт, а
не молча даёт `unknown`».
Фактически `scripts/wait-verdict.mjs:34` содержит **один** объект
`PIPELINE_EVENTS` для этого текста:
```js
{ re: /^\*\*Ревью не запускалось:\*\*/m, kind: 'conflict', text: '… ветка не ребейзится на dev — конфликт разрешает автор' }
```
И конвейер действительно публикует **два разных по смыслу, но тождественных по
префиксу** комментария под этим же заголовком:
- `.github/workflows/_process.yml:708` (конфликт ребейза): `**Ревью не
запускалось:** ветка \`$BRANCH\` не ребейзится на \`dev\` без конфликта. …`
- `.github/workflows/_process.yml:850` (красный Validate): `**Ревью не
запускалось:** $kind на материале \`$short\` (ветка \`$BRANCH\`) —
**$RESULT**: $NOTE. …`
Оба текста матчатся одним и тем же regex и получают один и тот же `kind:
'conflict'` — в `wait-verdict.mjs` нет НИ одной отдельной константы,
различающей «конфликт ребейза» от «красный Validate». Различить их можно
только по продолжению текста («не ребейзится» против «на материале … —
**$RESULT**»), но это продолжение нигде не оформлено как экспортируемая
константа ни в `wait-verdict.mjs`, ни в `review-doc-guard.mjs` (проверено:
`grep -i "Validate\|валид" scripts/review-doc-guard.mjs` — пусто).
Значит АС3 в заявленном виде невыполним без **новой**, не импортированной
логики — то есть ровно того случая, который раздел «Риски» называет закрытым
(«переименование ломает импорт»). На деле переименование/уточнение текста внутри
`_process.yml:708` или `:850` (например, смена формулировки «не ребейзится» на
другую) не сломает ни один импорт и тихо превратит один из двух случаев в
другой без единого красного теста — то есть именно тот сценарий подмены,
от которого риск обещает защиту.
Это утверждение о поведении («причины различимы на текстах констант»), которого
не существует ни в одном из названных модулей, подано как факт, не как
предположение — по правилу разбора такое утверждение фиксируется находкой.
Затронут прямо пронумерованный, входящий в скоуп критерий (К3/АС3), поэтому
без исправления автором реализовать АС3 как написано нельзя: нужно либо
(а) явно смести оба текста в одну причину (например, переименовать `conflict`
в такую, что покрывает оба смысла, признав потерю разрешения), либо
(б) явно описать НОВЫЙ, не заимствованный признак различения (например,
разбор части текста после общего префикса) и снять формулировку «импорт, не
копия» как универсальную защиту для этой строки таблицы.
**Как проверить исправление:** ТЗ должно либо убрать `validate-red` как
отдельную причину из таблицы К3, либо явно описать, по какому признаку (не
через `wait-verdict.mjs`/`review-doc-guard.mjs` целиком, а через конкретную
под-строку) она отличается от `conflict`, и снять/уточнить формулировку риска
про «импорт, не копия».
### Medium (в скоупе, чинится в этом же issue)
**2. АС8: заявленное доказательство правки `_process-metrics.yml`
(`fetch-depth: 0`, `timeout-minutes: 30`) — несуществующее; ни один
существующий тест эти поля не проверяет.**
АС8 пишет: «`_process-metrics.yml`: `fetch-depth: 0`, `timeout-minutes: 30`,
записи нет (тест `#637 workflow: еженедельный запуск читает только…`
зелёный)». Тест `test/process-metrics.test.mjs:131-142` прочитан целиком:
он проверяет `schedule`/`workflow_dispatch` в тонком файле, отсутствие
`issues: write` в обоих файлах, точный блок `permissions` и наличие команды
`node scripts/process-metrics.mjs … --output=…` — про `fetch-depth` или
`timeout-minutes` там нет ни одного `assert`. Поиск по всему дереву
(`grep -rn "fetch-depth|timeout-minutes" test/ --glob "*process*"`) не находит
ни одной другой проверки.
Это не мелочь бухгалтерии: сам К8 объясняет, зачем нужен `fetch-depth: 0` —
«нужна история для К1 `infra` и К7» (без полной истории `git log origin/dev
--numstat` за окно сравнения работает неполно или падает). Если после
реализации кто-то (или последующий ребейз/правка workflow) вернёт
`fetch-depth` к `1`, ни один тест это не поймает, а К1 «инфраструктура по
диффу коммитов» и К7 «сравнение объёма» молча вернутся к неполным данным без
единого красного прогона — именно тот сценарий тихой деградации, от которого
обычно защищает договор «тест умеет падать».
**Как проверить исправление:** добавить в АС8 (или в тест) утверждение,
которое реально проверяет оба поля YAML (например, прямой `assert.match` на
`fetch-depth: 0` и `timeout-minutes: 30` в теле `_process-metrics.yml`), либо
явно снять численные значения этих полей из контракта, если автор считает их
реализационной деталью, не требующей отдельного красного случая.
## Что проверено и корректно
- Трек `ask` и обоснование (§5, «ожидаемое поведение не зафиксировано» +
«сложность > 3») — корректны, инфраструктурный дифф без трековой метки
по умолчанию не должен идти этим маршрутом сам по себе, здесь это верно
аргументировано письменно.
- Имена jobs конвейера (`guard`/`prepare`/`model_review`/`integrate`) и их
человекочитаемые названия в К5 — сверены дословно с `.github/workflows/
_process.yml`, совпадают.
- Формат машинного блока `ship-review.mjs` (`anchorBlock`/`parseAnchorBlock`)
— сверен с АС4: агрегированные `high`/`medium`/`low` на документ, без
разбивки по задаче, именно так и читает `parseAnchorBlock`.
- Существование и сигнатуры всех перечисленных «импортов без правки»
(`PIPELINE_EVENTS`, `verdictDeclaration`, `parseAnchorBlock`, `classify`,
`reviewDocNames`) подтверждены чтением кода.
- Пример в АС2 (фикстура с часами `S1 0 → S5 1 → S6 2 → S7 5 → S6 6 → S7 8 →
S8 9`, `blocked 3–4`) арифметически сходится с определением отрезков из
таблицы К2: `queue=2, work=2, blocked=1, review=2, rework=2`, сумма равна
`lead=9` — проверено вручную по каждому отрезку.
- Приём «unit + временный git-репозиторий» для АС7 не изобретается заново:
`test/process-track.test.mjs` уже использует `mkdtempSync` + `git init` для
похожей задачи (рамки `ship` по `git diff --numstat`), техника воспроизводима.
- Дата `TRACKS_CUTOVER = '2026-09-28'` сверена с PROCESS.md §5 («Решение
владельца 2026-09-28, issue #695») — совпадает, не выдумана.
- Не-скоуп и обязательный вынос записи фактического расхода токенов в
отдельный issue F (вместо того чтобы тянуть её в этот диапазон) —
корректное разделение по критерию «что человек видит/делает» не входит
в основание, здесь граница чисто техническая и обоснована честно («нет
данных» вместо выдумывания оценки).
- Job `Мутанты` по-прежнему существует (`_mutation-gate.yml`, `validate.yml`),
хотя в самом `_process.yml` её больше нет (#709 вывел мутанты из ревью-
конвейера) — К5 корректно ограничивает сбор `jobsByRun` прогонами `process
#NN · …` и `Проверка (CI)», и job «Мутанты» живёт именно во втором,
так что существующий `jobMinutes`/`mutantShare` не потеряет данные.
- Продуктовых вопросов владельцу нет, и с этим можно согласиться: всё
содержимое — внутренний отчёт только для чтения, без видимого поведения
продукта (`User-Visible: no` обоснованно).
## Чего не проверял
- Не запускал `npm run gate:small`, `npm test`, `typecheck`, `build` — кода
ещё нет (ветки под issue #728 не существует), гонять их не над чем; это
этап ТЗ, а не код-ревью.
- Не проверял реальные данные GitHub Actions/issues (прогон
`fetchSnapshot`/`gh api`) — АС1–АС8 рассчитаны на unit-фикстуры, живой
прогон по кнопке назван в плане тестов явно как наблюдение после слияния,
не как AC.
- Не проверял ограничение «600 прогонов» и ветку «усечено» эмпирически —
численная граница разумна и не противоречит ничему в коде, но это
реализационная деталь, которую программирует и тестирует автор на фикстуре.
- Не проверял, действительно ли в `.github/workflows/_process.yml` встречаются
и другие, третьи по смыслу тексты «Ревью не запускалось» помимо двух
найденных (строки 708 и 850) — для находки High достаточно, что минимум два
разных по смыслу случая уже сейчас сливаются в одну причину; если их больше
двух, находка не слабее.
- Не оценивал стоимость правки `PROCESS.md` §5 (одна строка со ссылкой на
отчёт) — заявлено тривиальной документационной правкой, вне AC, замечаний
не вызывает.
## Материал раунда
Issue #728, тело на момент вынесения вердикта (раздел `## ТЗ`). Ветки
`issue/728-*` не существует — задача ещё не входила в реализацию. Код читан с
`HEAD` = `40607aa37f13819c9db37a392a1d99f30382b3c0` (ветка `dev`); ТЗ цитирует
`origin/dev` `108427dc`, между этим коммитом и материалом ревью нет изменений
ни одного файла, упомянутого в ТЗ (проверено `git log --oneline
108427dc..HEAD -- scripts/process-metrics.mjs scripts/process-track.mjs
scripts/wait-verdict.mjs scripts/review-doc-guard.mjs scripts/ship-review.mjs
.github/workflows/_process-metrics.yml .github/workflows/_process.yml`).
---
<!-- material-anchors: сгенерировано конвейером (#414) -->
## Материал раунда
- Ветка: `dev`, коммит `40607aa37f13` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
- Дерево материала: `901e6cbd1964ef2dcd9a0e08e8ec01a5c47dcac7`
```
git log --all --format='%H %T' | grep 901e6cbd1964
```
- Тело issue: `693749c0f65b33c7d23c81cd7a6ba7ab3b1d0ad1875a1867e5bde4de0c718d2d`
- Вердикт конвейера: `yellow` · High 1