docs: review document for #517

Issue: #517
User-Visible: no
This commit is contained in:
claude[bot]
2026-09-10 07:36:17 +00:00
parent 156835c048
commit d204bf5d77
+162
View File
@@ -0,0 +1,162 @@
# CODE-REVIEW-517-r1
Issue: #517 · этап: code · заход r1 · блокирующих циклов израсходовано 0 из 4
Материал: `156835c048a70497d7f4116c60f4c60457a05107` (ветка `issue/517-spec-in-issue-body`, один коммит на `origin/dev`)
## Скоуп
Класс B/C: `.github/workflows/process.yml`, `PROCESS.md`, `AGENTS.md`,
`docs/specs/README.md`, `scripts/mutation-gate.mjs`, `scripts/process-gate.mjs`,
`scripts/review-doc-guard.mjs`, `scripts/task-packet.mjs`, `test/process-gate.test.mjs`,
`test/review-doc-guard.test.mjs`, `test/task-packet.test.mjs`. Ни одного файла класса A —
`User-Visible: no` корректен, changelog не тронут и не должен быть.
Задача переносит доказуемость «ТЗ полного трека вынесено на этом тексте» из файла
`docs/specs/NN-slug.md` в хеш тела issue, снимаемый конвейером, и замораживает каталог
`docs/specs/` как архив. Решение владельца зафиксировано в самом issue (2026-09-10).
Это первый заход код-ревью (`S6→S7`); дельты к прошлому раунду нет — раздел
«Унаследовано из r<N-1>» не применяется, разбор ниже полный, как и требуется на r1.
## Как проверялось
**Прочитано.** `docs/SCOPE.md` (задача — процессная, персона не описана в SCOPE, что
верно отмечено в самом ТЗ), `PROCESS.md` целиком (§1–14), `AGENTS.md`, тело issue #517
и все пять комментариев (S2-аналитика, вердикт ревью ТЗ r1 жёлтый, правки автора,
вердикт ревью ТЗ r2 зелёный, хендофф S6→S7). Канонические доки подсистем (SUN/LIGHT/…)
не относятся — задача не трогает продукт.
**Гейты.**
| Гейт | Результат |
|---|---|
| `npx tsc --noEmit` | чисто |
| `npm test` | 2469 tests, 2468 pass, 1 skipped, 0 fail |
| `npm run build` | собран; `cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js` — идентичны |
| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | «Исполняемого frontend-диффа нет — браузер-смоки не выбираются» (src/** не тронут) |
| `node scripts/check-docs.mjs` | «Documentation checks passed (7 files, 12 external links)» — не обязателен (src/** не тронут), прогнан для очистки совести |
| `node scripts/process-gate.mjs --issues` | «гейт пройден, предупреждений 1» (WARN п.8: инфраструктурный диапазон, класса A нет — статус issue не требуется) |
| 4 названных мутанта, `node scripts/mutation-gate.mjs --id=<name>` | все 4 «поймано 1 из 1» (ниже) |
Не прогонялись и почему: `golden:verify` (нет визуальных изменений), `pytest tests_backend`
(Python не тронут), `no-new-any.mjs` (правки в `.mjs`, не в `src/**` TS), инварианты модели
(геометрия не тронута), performance-профили (не названы в AC и не задеты).
**Мутанты — таблица «чем краснеет» (§2.7):**
| AC | Чем доказан | Чем краснеет |
|---|---|---|
| AC1 (хеш тела в якорях) | `test/review-doc-guard.test.mjs` + мутант `review-anchor-drops-issue-body` | строка `- Тело issue:` не печатается → тест красный (проверено: `node scripts/mutation-gate.mjs --id=review-anchor-drops-issue-body` → «тест покраснел, как обязан», поймано 1/1) |
| AC2 (находка «ТЗ менялось») | тот же файл + мутант `review-ignores-changed-spec-body` | `issueBodyChanged` всегда возвращает `null` → красный (проверено, поймано 1/1) |
| AC6 (reuse видит правку тела) | тот же файл + мутант `reuse-ignores-changed-issue-body` | сравнение хеша в `reusableGreenVerdict` снято → красный (проверено, поймано 1/1) |
| AC3 (гейт без файла ТЗ) | `test/process-gate.test.mjs` + мутант `process-gate-requires-spec-file` | проверка текста заменена на «только файл» → красный (проверено, поймано 1/1) |
| AC4 (документы без требования файла) | `test/review-doc-guard.test.mjs`, текстовые ассерты по `PROCESS.md`/`AGENTS.md`/README | сравнение прочтением: строки-требования файла ТЗ нет нигде (grep ниже) |
| AC5 (AC из тела первым) | `test/task-packet.test.mjs` — прогнан в составе `npm test`, три сценария (тело/файл/ничего) | явно не воспроизводил мутацию отдельно — чистая функция, покрытие достаточное по трём веткам условия; риск низкий |
Живая проверка `issueBodyDigest` на реальном теле issue #517 (снятом `gh issue view 517
--json body`) пересчитана мной независимо: `d7258d4432fab7757039bc05e1ea932f9e023f61449bdf667da69002a06906c2`
— совпадает с указанным в хендоффе побайтово. Не заявление автора, а мой собственный прогон.
## Проверка AC по существу (чтение + тесты)
- **AC1.** `materialAnchorBlock` пишет `- Тело issue: \`<sha256>\`` только когда передан
`issueBody` (иначе строки нет — старые документы не ломаются, тест это утверждает).
Единый шаг «Опубликовать документ ревью» (`process.yml:912-988`) вызывает
`--anchor` с `--issue-body="$MATERIAL_ISSUE_BODY"` **на обоих этапах** (spec и code) —
не только для code, как можно было бы прочитать из плана поверхностно; проверено
чтением шага целиком, условие `if:` не сужает его до одного этапа. Хеш снимается в
шаге `material` через `gh issue view --json body` — не из `github.event.issue.body`
(У1 выполнен буквально, а не только продекларирован).
- **AC2.** Шаг `spec_body` (`process.yml:652-684`) сравнивает текущий хеш с записью
**последнего зелёного** `SPEC-REVIEW-NN-r*.md` через `issueBodyChanged`, включён в
промпт ревьюера форматированной строкой и запускается только на этапе `code` и только
когда `reuse` не сработал — что корректно, потому что `reuse=true` уже само по себе
гарантирует неизменность тела (см. AC6). Условие «нет зелёного документа или в нём нет
записи → не находка» проверено и тестом, и чтением (`issueBodyChanged` возвращает `null`).
- **AC6.** `reusableGreenVerdict(docs, differs, issueBodyDigest)` — третий параметр
сравнивается с `anchorIssueBodyFrom(latest.text)`; документ без записи (весь бэклог)
судится по дереву, как раньше — проверено тестом с «legacy»-документом без строки.
Единственное отступление от буквы AC6 не найдено.
- **AC3.** `checkSpecs` теперь судит текст тела (`## ТЗ` заголовок или токен `AC1`),
архивный файл всё ещё принимается; офлайн — молчание, `--issues` — предупреждение,
никогда не отказ. Соответствует и ТЗ, и записанному в PROCESS.md §8/§10.5 п.3.
`checkFrozenSpecs` — новая, отдельная, тёплое предупреждение на новый файл в
`docs/specs/**` (README.md исключён явно и по regex, и потому что это правка, не
добавление — `git diff-filter=A` его не найдёт).
- **AC4.** Прочитаны `PROCESS.md` (§2.3, §5, §7.1, §7.3 п.1, §8 правило 3, §13 п.2),
`AGENTS.md` (раздел Specs) и `docs/specs/README.md` целиком: ни один не требует файла
ТЗ для полного трека; `grep -n "docs/specs/NN\|docs/specs/<NN>"` не находит ни одного
места, где файл называется обязательным артефактом — только «архивный, доживает».
Ссылка на раздел «Обязательные release-артефакты» в README сохранена и корректна.
- **AC5.** `task-packet.mjs:buildPacket` — `fromBody = extractAcceptanceCriteria(issue.body)`;
если непусто **или** файлов ТЗ нет — источник тело, иначе файл. Тест на трёх фикстурах
(тело есть/тела нет-файл есть/ни одного) проходит. Небольшая избыточность — тело
парсится дважды (`fromBody` и затем `acSource === issue.body` снова через
`extractAcceptanceCriteria(acSource)`), но это чистая функция без побочных эффектов и
без наблюдаемой разницы в поведении — не нахожу это дефектом.
## Находки
Ни одной находки уровня High или Medium.
**Low-1 (снимаю записью, без правки).** В `reusableGreenVerdict(docs, differs,
issueBodyDigest = null)` имя третьего параметра совпадает с именем экспортируемой функции
`issueBodyDigest`, объявленной выше в том же модуле (`scripts/review-doc-guard.mjs`).
Внутри `reusableGreenVerdict` эта функция не вызывается, поэтому побочного эффекта нет —
подтверждено чтением и тем, что все тесты и мутанты зелёные/красные штатно. Чисто
стилистическая путаница для читателя («это хеш-значение, а не вызов функции»). Ниже
порога, который стоило бы возвращать автору отдельным циклом; исправление тривиально,
если автор решит его сделать при следующей правке этого файла.
## Что проверено и корректно
- Обратная совместимость: документы без строки `Тело issue:` (весь бэклог) продолжают
читаться как раньше во всех трёх местах (`anchorIssueBodyFrom` → `null`,
`issueBodyChanged` → `null`, `reusableGreenVerdict` пропускает новую проверку) —
проверено тестами и чтением.
- `--specs` формат не тронут, страховка #414 и reuse #499 не задеты для старых документов.
- Трейлеры коммита: `Issue: #517`, `User-Visible: no` — оба корректны для этого диффа.
- `process-gate --issues` на самом этом диапазоне — предупреждение п.8, ожидаемое для
инфраструктурного диапазона без класса A (сам гейт это и объясняет).
- docs/specs/README.md переписан по AC4, ссылка на раздел release-артефактов сохранена;
`check-docs.mjs` зелёный.
- Порядок шагов workflow (`material` → `reuse` → `spec_body` → `Review`) — проверено и
тестом на текст `process.yml`, и чтением: хеш снимается до вызова модели, находка
«ТЗ менялось» готова до промпта.
- Live-проверка `issueBodyDigest` независимо воспроизведена мной на актуальном теле
issue #517 и совпала с заявленной автором.
## Чего не проверял
- Живой прогон конвейера end-to-end (событие `labeled` → шаг `material` → анкор в
реальном PR-документе) — невозможен до слияния этой же задачи (курица и яйцо,
честно названо автором как «живая проверка на первой задаче после слияния»).
Проверено вместо этого модульно: все явные и неявные ветки покрыты тестами, я
перепрогнал их и мутанты сам, а не поверил отчёту.
- `golden`, `pytest tests_backend`, `no-new-any`, инварианты модели, performance —
не прогонялись, диф их не касается (см. таблицу гейтов выше).
- Стилистическая придирка Low-1 не проверялась на предмет «а что если где-то ещё в
файле есть похожий шэдоуинг» — искал только по этому диффу.
## Материал раунда
- SHA: `156835c048a70497d7f4116c60f4c60457a05107`
- Ветка: `issue/517-spec-in-issue-body`
- Дерево материала: `git rev-parse HEAD^{tree}` на указанном SHA
- Документы этапа spec: `docs/reviews/SPEC-REVIEW-517-r1.md` (жёлтый), `docs/reviews/SPEC-REVIEW-517-r2.md` (зелёный)
- Это первый документ этапа code — «Унаследовано из r<N-1>» не применяется.
Вердикт: зелёный · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 0 → в задаче
---
<!-- material-anchors: сгенерировано конвейером (#414) -->
## Материал раунда
- Ветка: `issue/517-spec-in-issue-body`, коммит `156835c048a7` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
- Дерево материала: `7a703201a159d0e3a288e467a652af3a618a499e`
```
git log --all --format='%H %T' | grep 7a703201a159
```
- Вердикт конвейера: `green` · High 0