From a49f7095ce77def25cc1ec4d48b390ec62d84bfd Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Wed, 30 Sep 2026 23:34:30 +0000 Subject: [PATCH] docs: review document for #728 Issue: #728 User-Visible: no --- docs/reviews/INDEX.md | 3 +- docs/reviews/SPEC-REVIEW-728-r1.md | 251 +++++++++++++++++++++++++++++ 2 files changed, 253 insertions(+), 1 deletion(-) create mode 100644 docs/reviews/SPEC-REVIEW-728-r1.md diff --git a/docs/reviews/INDEX.md b/docs/reviews/INDEX.md index eef52f4b..ff420469 100644 --- a/docs/reviews/INDEX.md +++ b/docs/reviews/INDEX.md @@ -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` | diff --git a/docs/reviews/SPEC-REVIEW-728-r1.md b/docs/reviews/SPEC-REVIEW-728-r1.md new file mode 100644 index 00000000..bace39ac --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-728-r1.md @@ -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`). + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `40607aa37f13` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `901e6cbd1964ef2dcd9a0e08e8ec01a5c47dcac7` + ``` + git log --all --format='%H %T' | grep 901e6cbd1964 + ``` +- Тело issue: `693749c0f65b33c7d23c81cd7a6ba7ab3b1d0ad1875a1867e5bde4de0c718d2d` +- Вердикт конвейера: `yellow` · High 1