diff --git a/docs/reviews/CODE-REVIEW-729-r1.md b/docs/reviews/CODE-REVIEW-729-r1.md new file mode 100644 index 00000000..870d15da --- /dev/null +++ b/docs/reviews/CODE-REVIEW-729-r1.md @@ -0,0 +1,193 @@ +# CODE-REVIEW-729-r1 + +Материал раунда: `git log --oneline origin/dev..HEAD` / `git diff origin/dev...HEAD` +на SHA `0e44f8a69f01c94b15b1ef48b411f5081b8e686d` (ветка `issue/729-spec-draft`, +один коммит поверх `dev@7ff2b5ae`). Трек: `ask` · заход r1 · блокирующих +циклов 0/4 · маршрут вердикта: `fix`. + +## Скоуп + +Issue #729 (выделено из #707) вводит одно явное исключение из правила №1: +локальный, непушимый черновик кода класса A на `track:ask`, пока задача стоит +в `S4-spec-review`, принимается правилом 10 (`checkCommitEraStatuses`, +`scripts/process-gate.mjs`), если: + +1. коммит несёт ровно один трейлер `Spec-Draft: sha256:<64 hex>`; +2. трек на момент написания (`authorDate`) — `ask`; +3. эпоха `S4-spec-review`, в которой написан коммит, закрыта `S5-ready`; +4. трейлер равен хешу «Тело issue» зелёного (High 0) `SPEC-REVIEW--r*` + этой эпохи, прочитанного из git (вершина диапазона или `origin/dev`). + +Пакет задачи (`scripts/task-packet.mjs`) печатает новый раздел «Черновик +(#729)»: в `S4` — право вести черновик и готовую строку трейлера; в `S5`/`S6` — +какое ревью ТЗ зелёное и изменилось ли тело после него. Канон (`PROCESS.md` +§11.8 и ссылки на него из §1, §2.4–2.6, §3 п.1, §7.2, §9, §10.2, §12), +`docs/process/AUTHOR.md`, `docs/process/REVIEWER.md`, `AGENTS.md` и +`test/process-digests.test.mjs` обновлены синхронно. `_process.yml` не +менялся — так и было заявлено в ТЗ (не-скоуп). + +Изменённые файлы (все входят в заявленный в ТЗ список «Затронутые файлы», +новых — нет): `AGENTS.md`, `PROCESS.md`, `docs/process/AUTHOR.md`, +`docs/process/REVIEWER.md`, `scripts/process-gate.mjs`, `scripts/task-packet.mjs`, +`test/pre-push-gate.test.mjs`, `test/process-digests.test.mjs`, +`test/process-gate.test.mjs`, `test/task-packet.test.mjs`. + +Коммит несёт `Issue: #729` и `User-Visible: no`; изменение не видно продуктовым +пользователям House Plan (процесс разработки, не продукт из `docs/SCOPE.md`), +changelog корректно не тронут. + +## Как проверялось + +Дешёвые гейты на этом SHA подтверждены зелёным Validate (ссылка в промпте) — +`tsc`/`npm test`/`npm run build`+bundle-policy не перегонялись. Дополнительно +к этому прогнано: + +| Гейт | Команда | Результат | +|---|---|---| +| Целевые юниты диффа | `node --test test/process-gate.test.mjs test/task-packet.test.mjs test/process-digests.test.mjs test/pre-push-gate.test.mjs` | 89/89 pass (совпадает с заявленным автором числом) | +| Монолит-якоря (не регрессия) | `node --test test/monolith-text-anchors.test.mjs` | 2/2 pass | +| Мутации (реестр, без новых запусков) | `node scripts/mutation-gate.mjs --check` | 4 предупреждения — совпадает с `dev`, новых не добавилось | +| Бюджет чтения | `node scripts/entry-cost.mjs --check` | author 5367/12000, reviewer 4750/9000 — совпадает с хендоффом | +| Процессный гейт на себя | `node scripts/process-gate.mjs --range origin/dev..HEAD` | чисто, 0 нарушений | +| Трек/маршрут | `node scripts/process-track.mjs stage --stage=code --labels=track:ask,S7-code-review ...` | `track=ask`, route_note → `route: fix` | +| Мутационная проверка AC «чем краснеет» (ручная, не из реестра) | временно `if (false)` вместо сравнения хешей в `judgeDraft`, затем откат | 4 теста AC1/AC2/AC4 красные — подтверждает, что тест на сверку хеша умеет падать | + +Прочитаны построчно: весь диапазон `scripts/process-gate.mjs` (новые функции +`trackAt`, `draftEpoch`, `greenSpecReviewOf`, `judgeDraft`, +`gitSpecReviewReader`, правка `checkCommitEraStatuses`), весь диапазон +`scripts/task-packet.mjs` (`rightsFor`, `latestGreenSpecReview`, +`specDraftState`, `buildPacket`, `renderPacket`, `collectInputs`), полные +диффы `PROCESS.md` §11.8 и правок к §1/§2.4–2.6/§3/§7.2/§9/§10.2/§12, +`AUTHOR.md`, `REVIEWER.md`, `AGENTS.md`, `test/process-digests.test.mjs`, +`test/pre-push-gate.test.mjs`. Проверена полнота `HOOK_FILES` +(`test/pre-push-gate.test.mjs`) прямым чтением `import`-блоков +`review-doc-guard.mjs`, `process-track.mjs`, `change-risk.mjs`, +`review-result-gate.mjs` — список действительно покрывает новое дерево +импортов `process-gate.mjs`. + +Прочитано, но не исполнено: сопоставление ТЗ (АК1–АК10, тело issue #729) с +кодом — построчно, без отдельного прогона (доказательство — тесты автора, +воспроизведённые выше). + +## AC → доказательство + +| AC | Что | Чем доказан | Чем краснеет | +|---|---|---|---| +| AC1 | Черновик принят/отклонён по трейлеру | `test/process-gate.test.mjs` «#729 AC1», прогнан | ручная мутация сравнения хешей в `judgeDraft` красит тест (см. таблицу гейтов) | +| AC2 | Границы эпохи и раунда | «#729 AC2», прогнан | тест сам содержит отрицательные случаи (эпоха закрыта S3/S6, не закрыта, timeline без `allowed`) | +| AC3 | Трек на момент записи и формат трейлера | «#729 AC3», прогнан | формат проверен 4 негативными значениями трейлера в самом тесте | +| AC4 | Сопоставление с зелёным документом своей эпохи | «#729 AC4», прогнан | негативные случаи (yellow, High 1, без «Тело issue», вне окна, более старый раунд) — в тесте | +| AC5 | Совместимость с #738 — без читателя исключение выключено | «#729 AC5», прогнан | тест сравнивает `plain === strip(c)` на 8 сценариях и напрямую дергает `warn`/`fail`-ветки | +| AC6 | Чтение документов из git + CLI | «#729 AC6» (reader) и «#729 AC6» (CLI), оба прогнаны на временном git-репозитории с заглушкой `gh` | тест проверяет оба кода выхода (0/1) и текст находки на 4 сценариях (rebased/fromDev/stale/unproven) | +| AC7 | Один источник хеша | «#729 AC7» в обоих файлах теста, прогнаны | тест читает `_process.yml` и проверяет тождество функций `issueBodyDigest` | +| AC8 | Пакет задачи | «#729 AC8» ×2 в `test/task-packet.test.mjs`, прогнаны | негативные случаи (`track:show`, `blocked`, `review-4`, устаревшее тело) — в тесте | +| AC9 | Канон и конспекты | `test/process-digests.test.mjs`, прогнан целиком | новые ключевые правила и привязка к заголовку `### 11.8` проверены assert'ами на реальный текст файлов | +| AC10 | Гейт | `gate:small`, `mutation-gate --check`, `entry-cost --check` воспроизведены выше | якоря `task-packet-*` и `packet-infra-track-ignores-show-default` не сдвинуты (сверено diff'ом `mutation-registry.mjs` — пусто) | + +Все десять AC — защитные по характеру (гейт либо принимает, либо отказывает +коммит), и у каждого в таблице есть непустой третий столбец — требование +§2.7/REVIEWER.md выполнено. + +## Находки + +Нет High. Нет Medium. Нет Low. + +Читал реализацию на предмет типичных мест ошибок в такой логике (границы +эпохи, порядок проверок «находка одна — по первой невыполненной», ленивое +чтение git, инъекция `specReviews`, согласованность `issueBodyDigest` между +тремя модулями, нормализация `\r\n`) — расхождений с ТЗ не нашёл. Отклонения +от ТЗ, о которых автор написал в хендоффе (правка фикстуры +`test/pre-push-gate.test.mjs`, предупреждение «трейлер вне S4» только с +читателем, кэш документов по задаче, `trailer: null` в `--json` при +запрещённом черновике) — все мелкие технические решения в объявленном +скоупе «реализатору на выбор» (ТЗ, «Принято предположительно», пп. 6, 9, 10), +находок не образуют. + +Побочные дефекты, найденные при реализации и правомерно вынесенные в +отдельные issue (не чинятся в этой задаче, не TODO в этом документе): +#752 (process-metrics), #748 п.1 (устаревшая строка пакета для `S3`), +#751 п.4 (метрика черновиков), плюс названные в хендоффе кандидаты (формат +`Spec-Draft` в `commit-msg`, вывод `HOOK_FILES` из дерева импортов) — по +заявлению автора заведены или добавлены как кандидаты; это корректный путь +по §12 («Оставили в тексте ревью» не считается закрытием), ревью их не +заводит повторно. + +## Что проверено и корректно + +- Правило 10 (`judgeDraft`) проверяет условия строго в порядке ТЗ (К3 п.2: + формат → трек → эпоха → документ → хеш), «находка одна — по первой + невыполненной» выполнено буквально (`return fail(...)` на первой же). +- `draftEpoch` корректно учитывает reconcile (#555): шагает назад по + непрерывной серии `S4-spec-review` в уже отфильтрованном списке статусных + событий, не путает начало эпохи с повторной постановкой метки. +- `trackAt` на коммите без трековых событий возвращает `ask` — соответствует + §5.1 («продуктовая задача без метки трека — ask»), совпадает с + `trackFromLabels([])`. +- `gitSpecReviewReader` ленив (ни одного вызова `git`, пока правило 10 не + спросит документы чернового коммита) и кэширован по issue — подтверждено + тестом AC5 (`reader.reads`) и AC6. +- `issueBodyDigest` — действительно одна функция на гейт, пакет и конвейер: + `scripts/task-packet.mjs` реэкспортирует её же из `review-doc-guard.mjs` + (не копию), `scripts/process-gate.mjs` импортирует оттуда же якоря + `anchorIssueBodyFrom`/`anchorVerdictFrom`. «Одно число — один источник» + (§8) выполнено: хеш тела issue виден в трейлере коммита, в пакете задачи и + в документе ревью — везде один источник. +- `test/pre-push-gate.test.mjs` `HOOK_FILES` расширен корректно и полно: + прямой разбор `import`-блоков показал, что список покрывает всё новое + транзитивное дерево зависимостей `process-gate.mjs` без пропусков. +- Без инъекции `specReviews` (или если в диапазоне нет чернового коммита) + поведение и тексты находок побайтово равны #738 — проверено тестом AC5 и + чтением кода (ветки `if (specReviews ...)` вокруг каждого нового куска). +- `User-Visible: no` обоснован: изменение процесса разработки, не продукта + из `docs/SCOPE.md`; changelog не нужен и не тронут. +- Коммит несёт трейлеры `Issue: #729`/`User-Visible: no` корректно; сам этот + коммит не является черновиком (`Spec-Draft` на нём не требуется — он + сделан после `S5`, что подтверждает хендофф «Взял: ... по зелёному ревью + ТЗ»). + +## Чего не проверял + +- Полный `npm run gate:small`, `npx tsc --noEmit`, `npm run build` с полной + сверкой трёх копий бандла — не перегонял: зелёный Validate на этом SHA уже + подтверждён (ссылка в промпте), диффа в `dist/`, `src/`, + `custom_components/`, `demo/golden/baselines/` нет (проверено + `git diff --stat` — пусто). +- Браузерные смоки, `golden:verify`, `pytest tests_backend`, + `npm run invariants`, performance-профили — не прогонял и не выбирал по + `smoke-select.mjs`: изменение не трогает ни один из путей, которые эти + гейты проверяют (нет файлов `src/**`, `custom_components/**/*.py`, геометрии + или рендера в диффе); ни один AC их не требует. +- Реальный прогон сценария «живой» ревью ТЗ → черновик → гейт на настоящем + GitHub issue (end-to-end через CI) — не воспроизводил; AC6 покрывает CLI с + заглушкой `gh` на временном git-репозитории, этого достаточно для + доказательства логики гейта, но не для проверки, что реальный `gh api + .../timeline` отдаёт точно тот формат событий, который ожидает + `trackAt`/`statusEvents` (риск уже назван автором в разделе «Риски» + хендоффа и в самом ТЗ, п. «Часы автора отстают от GitHub»). +- Не проверял исполнением фактическое поведение `_process.yml` (шаг «Взять + SHA материала», публикация SPEC-REVIEW) — по ТЗ этот файл не менялся + (не-скоуп), а материал ревью кода не включает ревью ТЗ. + +## Вердикт + +Все 10 AC доказаны автотестами, тесты воспроизведены и умеют падать (одна +мутация проверена вручную дополнительно к имеющимся негативным случаям +внутри тестов). Реализация соответствует контракту ТЗ буквально — К1–К5 +покрыты файл-в-файл. High/Medium не найдено. Побочные находки корректно +вынесены в отдельные issue, а не оставлены как TODO. + +**Зелёный.** + +--- + + + +## Материал раунда + +- Ветка: `issue/729-spec-draft`, коммит `0e44f8a69f01` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `c7dc00f61e25ed2b0dae9d0ed9dea6275d3510a9` + ``` + git log --all --format='%H %T' | grep c7dc00f61e25 + ``` +- Тело issue: `94cd1bca3f851c8c9d9fc64b3b6227fb2fd3dac9f47715053a95dd133ae40bed` +- Вердикт конвейера: `green` · High 0 · маршрут `fix`