diff --git a/docs/reviews/CODE-REVIEW-768-r1.md b/docs/reviews/CODE-REVIEW-768-r1.md new file mode 100644 index 00000000..a1b9d7ae --- /dev/null +++ b/docs/reviews/CODE-REVIEW-768-r1.md @@ -0,0 +1,77 @@ +# CODE-REVIEW-768-r1 + +Issue: [#768](https://github.com/Matysh/houseplan-card/issues/768) · Трек: `show` · Заход: r1 · блокирующих циклов 0/2 +Материал: `git log origin/dev..HEAD` = `53bcd5f3` (единственный коммит), рабочая копия на `53bcd5f3d0edd1f28ed52ad9c929ea5d89f242da` (`git rev-parse HEAD` сверен). +Validate на этом SHA: success, https://github.com/Matysh/houseplan-card/actions/runs/37536779953 (проверено `gh run view` — `headSha` совпадает, `conclusion: success`). + +## Скоуп + +Инфраструктурная задача (класс B: `scripts/**`, `test/**`, документ-карта `AGENTS.md`), не продуктовый код — маршрут инфраструктуры: реализация → push → `S7-code-review`, минуя `S2`–`S6` (`AGENTS.md`, правило №1). `docs/SCOPE.md` не применяется напрямую (это не фича продукта для конечного пользователя), но задача обслуживает инфраструктуру, которой держится весь конвейер ревью продуктовых задач. + +`scripts/wait-verdict.mjs` (`PIPELINE_EVENTS`) знал два исхода слияния `merge-candidate.mjs` из одиннадцати (через текстовые копии-регэкспы), остальные девять — отказ push (#705), «кандидат красный», «dev движется быстрее», rereview, сбой шага — не узнавал: агент, ждущий вердикт, не останавливался на них и тратил время впустую до таймаута, хотя задача уже вернулась к нему. После #752 `merge-candidate.mjs` экспортирует общий каталог признаков (`OUTCOME_SIGNS`, `outcomeOf`). Правка: `wait-verdict.mjs` сначала разбирает комментарий через `outcomeOf`/`OUTCOME_EVENTS` (новая таблица в самом `wait-verdict.mjs`, сопоставляющая `action`/`stage` каталога с кодом выхода и текстом), затем — через прежние `PIPELINE_EVENTS` (оставлены для тел до #752 и для `process-metrics.mjs`, который импортирует `kind` по ним). + +**Риск по изменённым участкам (#707).** Ни один из рискованных классов (geometry/touch/migration/devices/perf/новый UX-ключ) не задет: правка не касается `src/**`, карточки, рендера, конфигурации пользователя. Единственная risk-поверхность — сам протокол ожидания вердикта, и она по определению описана в этом же диффе (`AGENTS.md`) и в самом коде каталога `OUTCOME_SIGNS`/`outcomeOf` (#752, уже в `dev` до этой задачи). + +→ **route: fix** — критерии §5 пройдены: сложность автор сам оценил 3/10 (подтверждаю чтением — правка сосредоточена в одной функции `pipelineEventOf` плюс таблица сопоставления), одна поверхность (`wait-verdict.mjs` + его тест + одна строка реестра мутаций), нет миграции конфига, нет нового UX-контракта (не продуктовый код), нет влияния на perf/touch, ожидаемое поведение зафиксировано в `OUTCOME_SIGNS`/`commentFor` (#752, уже канон) и в обновлённом абзаце `AGENTS.md` этого же коммита. + +## Как проверялось + +| Гейт | Статус | Как | +|---|---|---| +| `npx tsc --noEmit`, `npm test`, `npm run build` + bundle-policy | не перегонял | Validate green на точном SHA `53bcd5f3` (#343), ссылка выше, `headSha` сверен `gh run view` | +| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | прогнал сам | «Исполняемого frontend-диффа нет (`src/**/*.ts` не тронут)» — смоки не выбираются, и это ожидаемо: диф не трогает карточку | +| `node --test test/wait-verdict.test.mjs` (все тесты, не только `#768`) | **прогнал сам** | 4/4 зелёных; включая три теста, не относящихся к #768 (`#496`, `#546`, `#726`) — регрессий в старых сценариях нет | +| `node --test test/wait-verdict.test.mjs test/merge-candidate.test.mjs test/process-metrics.test.mjs` | **прогнал сам** | 79/79 зелёных — смежные потребители `OUTCOME_SIGNS`/`commentFor` (`process-metrics.mjs`) не задеты | +| `node --test test/mutation-gate.test.mjs test/mutation-guard-outcome.test.mjs test/classify-changes.test.mjs` | **прогнал сам** | 114/114 зелёных — новая запись реестра мутаций синтаксически и структурно корректна (unique id, статический `find`, `guard` матчит реальный тестовый файл) | +| Новый мутант `wait-verdict-merge-outcomes-unknown` — «чем краснеет» | **прогнал сам, вручную** | Применил патч реестра (`const sign = outcomeOf(text)` → `const sign = null; // mutant`) руками поверх `scripts/wait-verdict.mjs`, прогнал `node --test --test-name-pattern="#768" test/wait-verdict.test.mjs` — все 4 теста красные (не no-op). Откатил патч, `git status` чист — рабочая копия не испорчена | +| golden / pytest / invariants / performance | не применимо | диф не трогает рендер, Python, геометрию; в AC не назван performance-гейт | + +Рабочая копия после ручной мутации возвращена в исходное состояние; `git status` чист на момент написания документа. + +## Находки + +### F1 (Medium, вне скоупа #768) — заведён отдельно: [#810](https://github.com/Matysh/houseplan-card/issues/810) + +`PIPELINE_EVENTS[0]` в `wait-verdict.mjs` (`/^\*\*Ревью не запускалось:\*\*/m`, `kind: 'conflict'`, текст «конфликт разрешает автор») — **не изменённая этим диффом** строка, но она матчит оба пре-ревью комментария `_process.yml`: настоящий конфликт ребейза (`_process.yml:727`) и красный/пропавший Validate на материале до ревью (`_process.yml:870`, `"$kind` на материале … — `$RESULT`: `$NOTE"`). Второй случай — не git-конфликт вообще, но `wait-verdict` печатает для него ту же строку «конфликт разрешает автор», что может увести агента чинить несуществующий конфликт вместо разбора прогона Validate. Автор сам отметил это находкой в итоговом комментарии («Ревью не запускалось» всегда описывается как конфликт ребейза, включая возврат по красному Validate) и сознательно не стал чинить — верно, поскольку #768 — только про исходы `merge-candidate.mjs` после вердикта (`OUTCOME_SIGNS`), а это другой код-путь (пре-ревью гейт в `_process.yml`). Не регрессия этого диффа, issue заведён ревьюером, поскольку «оставили в тексте» закрытием не считается (§12). + +Других находок нет — High: 0, Medium в скоупе: 0. + +## Что проверено и корректно + +- **`OUTCOME_EVENTS` покрывает весь `OUTCOME_SIGNS` целиком.** Сверил построчно: `reject-stale`, `conflict`, `validation-red`, `validation-missing`, `give-up`, `error`, `rereview` (плоские), `push-refused-workflow`/`push-refused` (вложенные по `stage: merge|rebase`) — все 11 признаков каталога `merge-candidate.mjs:246-257` имеют запись. Сам тест `#768 исходы слияния …` отдельно проверяет это исполнением (`for (const sign of OUTCOME_SIGNS) assert.ok(covered.has(...))`), а не только моим чтением. +- **`pipelineEventOf` разруливает плоские/вложенные записи корректно** (`entry.kind ? entry : entry[sign.stage]`) — проверено и чтением, и тестовой таблицей из 13 строк (`OUTCOME_TABLE`), построенной на телах `commentFor`/`describePushRefusal`, а не на скопированных строках. +- **`rereview` — единственный нетерминальный исход** (`code: null`): `decide()` печатает строку и не завершает ожидание, только когда смена метки не произошла отдельно в этом же тике (`if (code === null && eventCode !== null) code = eventCode`) — доказано тестом `#768 исход прежнего раунда…`, где `rereview`, пришедший с новым якорем раунда, корректно не повторяется (`relabeled.lastEvent === null`). +- **Старые `kind: undefined` события (легаси `PIPELINE_EVENTS`, `owner-question`, `reclassify`) не регрессировали**: `eventCode = next.lastEvent.code === undefined ? 3 : next.lastEvent.code` сохраняет прежнее поведение «любое событие без явного `code` — код 3»; тесты `#496`/`#546`/`#726` (не относящиеся к #768) прогнаны вместе с новыми и зелёные. +- **Успешное слияние не имеет признака в каталоге и не должно иметь** — `push`/`fast-forward` тела `commentFor` не матчат ни `OUTCOME_SIGNS`, ни legacy `PIPELINE_EVENTS` (проверено чтением текста `commentFor` — не начинается с `**`), о слиянии сообщает только смена метки на S8 (код 0) — ровно то, что заявлено в комментарии автора и в правке `AGENTS.md`. +- **Защитный AC доказан таблицей «чем краснеет»**: мутант `wait-verdict-merge-outcomes-unknown` зарегистрирован (`scripts/mutation-registry.mjs`), его `guard` — реальная тестовая команда, я применил сам патч руками и убедился, что все 4 теста `#768` падают (не бутафорская защита). +- **Трейлеры коммита корректны**: `Issue: #768`, `User-Visible: no` — у инфраструктурной правки без видимого пользователю поведения changelog не требуется, оба `docs/CHANGELOG*.md` не тронуты и не должны быть. +- **Одно число — один источник**: единственное число, которое меняется, — состав `OUTCOME_EVENTS` (11 исходов), и оно получено из единственного источника `OUTCOME_SIGNS`, а не продублировано текстовыми константами — находок по дублированию чисел нет. +- `AGENTS.md`: обновлённый абзац о кодах выхода `wait-verdict.mjs` точно описывает новое поведение (код 3 при неизменной S7 для терминальных исходов, печать причины при уже сменившейся метке, `rereview` — не завершение) — сверено с кодом и тестами построчно, расхождений нет. + +## Чего не проверял + +- Полный `npx tsc --noEmit` / `npm test` (весь набор) / `npm run build` со сверкой бандла — не гонял, принял зелёный Validate на точном SHA (#343); сам прогнал только тесты, прямо относящиеся к изменённым модулям (`wait-verdict`, `merge-candidate`, `process-metrics`, реестр мутаций) — 193 теста суммарно, все зелёные. +- Browser-smoke, golden, pytest `tests_backend`, инварианты модели, performance — не применимо: `smoke-select` подтвердил отсутствие исполняемого frontend-диффа, Python и геометрия не менялись, performance не назван в AC. +- Полный прогон всего `scripts/mutation-registry.mjs` (ночной реестр) — не гонял по правилу track:show (#709, мутанты в разработке не гоняются); единственный новый мутант проверил вручную отдельно от общего гейта. +- Поведение на реальном GitHub Actions (живой прогон `_process.yml` с настоящим GH API) — не воспроизводил: AC доказаны юнит-тестами на телах, которые сами генераторы (`commentFor`/`describePushRefusal`) реально производят, это и есть доказательство «не исполнением GH Actions, а чтением плюс юнит-тестом на точных телах». + +## Вердикт + +High: 0. Medium в скоупе: 0. Medium вне скоупа: 1 → [#810](https://github.com/Matysh/houseplan-card/issues/810) (пре-существующая, не внесённая этим диффом неоднозначность текста `wait-verdict` для пре-ревью «Ревью не запускалось:»). + +Вердикт: зелёный · заход r1 · блокирующих циклов 0/2 · High: 0 · Medium: 0 → в задаче | #810 + +--- + + + +## Материал раунда + +- Ветка: `issue/768-wait-verdict-outcomes`, коммит `53bcd5f3d0ed` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `929670609a48517c80da271513311b5756be5d15` + ``` + git log --all --format='%H %T' | grep 929670609a48 + ``` +- Тело issue: `66d282e8887e83f6b8d16b8d48ae13db37f9314378954ab03488671e16fb52f7` +- Вердикт конвейера: `green` · High 0 · маршрут `fix` +