diff --git a/docs/reviews/CODE-REVIEW-551-r4.md b/docs/reviews/CODE-REVIEW-551-r4.md new file mode 100644 index 00000000..1758ebb0 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-551-r4.md @@ -0,0 +1,188 @@ +# CODE-REVIEW-551-r4 + +## Скоуп + +Issue #551 (инфраструктурная, метка `infra`) — разнесение конвейера ревью на +три job с независимыми бюджетами. Раунды r1–r3 уже признали конструкцию +выполненной (зелёный, без находок); в задачу этот вердикт до сих пор не +применён — оба последних захода (r2, r3) закончились зелёной моделью, но +**стадия публикации/интеграции падала** на живых прогонах уже после +слияния/повторной активации `process.yml`, и автор возвращал задачу в +`S6-in-progress` для фикса именно интеграции, а не по замечанию ревью. + +Материал этого раунда — ровно `62e0eff21999eebfc9aecbfe99edb14d9ccf716f`. +Дельта отсчитывается от материала r2/r3 (`23f48829c26deec968af6645301900880899139e`, +дельта r2 от r3 — нулевая, см. ниже): один коммит `62e0eff2` поверх +`23f48829`, 3 файла, +26/−5 строк (`git diff --shortstat 23f48829..62e0eff2`): + +- `.github/workflows/process.yml` — шаг «Проверить полноту и происхождение + результата» в job `integrate`: запись `structured_output` в + `$GITHUB_OUTPUT` заменена с `{ echo <> file` — это отдельный `write()`, который ничего не подставляет между +предыдущим выводом и своим: без LF в конце JSON delimiter приклеивается к +последней строке значения, и GitHub Actions не находит начало delimiter'а на +отдельной строке. + +Фикс (`process.yml:1212-1216` после дельты): +``` +printf 'structured_output<> "$GITHUB_OUTPUT" +cat "$dir/verdict.json" >> "$GITHUB_OUTPUT" +printf '\nEOF_RESULT\n' >> "$GITHUB_OUTPUT" +``` +Явный `\n` перед `EOF_RESULT` гарантирует делимитер на отдельной строке +независимо от того, кончается ли `verdict.json` переводом строки. Если файл +уже кончается LF — на выходе будет один лишний пустой перенос перед +delimiter'ом; это не влияет на `jq`-парсинг ниже по конвейеру (`OUT: ... +structured_output`, `printf '%s' "$OUT" | jq -r`) — JSON с хвостовым +whitespace для `jq` эквивалентен исходному. Прочитано и проверено вручную. + +Это прямое попадание в AC3 issue («timeout/cancel/**failure** каждой стадии +оставляет понятное состояние; лимиты review cycles не расходуются на +невыполненный review») — двумя живыми прогонами (r2→r3 и r3→r4) подтверждено, +что счётчик циклов действительно не тратился на несостоявшуюся интеграцию +(«блокирующих циклов израсходовано 0 из 4» в постановке этого захода), т.е. +защитное свойство AC3 сработало даже при поломке самой публикации. + +## Проверено + +- `git diff 23f48829..62e0eff2` — три файла, ровно эти три хунка, ничего + лишнего; job `guard`, `prepare`, `model_review`, структура контракта, + проверки sha256/полей — не тронуты ни строкой. +- Прочитано построчно: новый код в `process.yml:1212-1216` синтаксически и + семантически корректен (см. разбор выше), остальной шаг («Проверить + полноту и происхождение результата») не менялся вне этих трёх строк. +- Независимо (не полагаясь на заявление автора) прогнал: + - `node --test --test-name-pattern="#551" test/review-doc-guard.test.mjs` + → 1/1 pass; + - `node scripts/mutation-gate.mjs --id=review-integration-delimiter-follows-json-without-newline` + → «чистый прогон» + «заявленный тест покраснел на мутанте» → **поймано 1 + из 1**. Мутант возвращает ровно старый баг (`printf 'EOF_RESULT\n'` без + ведущего `\n`), и новый `assert.match(resultOutput, /cat ...>> + "\$GITHUB_OUTPUT"[\s\S]*printf '\\nEOF_RESULT\\n' >>.../)` действительно + красится на этом мутанте — дисциплина «тест умеет падать» соблюдена для + прогнанного мной гейта. + - `node scripts/smoke-select.mjs --base 23f48829 --head 62e0eff2` → + «Исполняемого frontend-диффа нет… Browser-smoke этим диффом не + выбираются» — выбирать нечего, `src/**` не тронут. +- ID нового мутанта уникален в реестре (`grep -n + 'review-integration-delimiter-follows-json-without-newline' + scripts/mutation-gate.mjs` — одно определение), guard-команда та же, что и + у соседних #551-мутантов, конфликтов нет. +- Трейлеры коммита `62e0eff2`: `Issue: #551`, `User-Visible: no` — верно, + правка меняет только служебный CI-механизм передачи данных между stages, + видимое поведение продукта не меняется; changelog не требуется. + +Не перегонял `npx tsc --noEmit` / `npm test` / `npm run build` + сверку трёх +копий бандла — Validate зелёный на этом точном SHA: +https://github.com/Matysh/houseplan-card/actions/runs/34754874119 (упомянут +автором и подтверждён как ссылка на прогон именно этого коммита). Не +перегонял `check-docs.mjs`, `golden:verify`, backend pytest, инварианты +модели, performance-профили и полный набор browser-смоков — diff не касается +`src/**` (кроме тестового файла `test/review-doc-guard.test.mjs`, не +влияющего на отпечаток документации), Python, геометрии/`layout`/ +`marker.space`/`open_spans`, визуального рендера или производительности; +`smoke-select.mjs` подтверждает пустой отбор. + +## Закрытие раунда r3 + +r3 не содержал находок («Находок нет», зелёный, High 0 / Medium 0) — этому +раунду нечего закрывать по замечаниям. Причина существования r4 — +операционный сбой стадии публикации/интеграции ПОСЛЕ зелёного вердикта r3 +(run [34754400084](https://github.com/Matysh/houseplan-card/actions/runs/34754400084)), +диагностированный автором как отдельный дефект границы стадий (delimiter +приклеен к JSON). Дельта этого раунда — прямой фикс именно этого сбоя (см. +разбор выше); других изменений кода в диапазоне r3→r4 нет. + +## Унаследовано из r3 (без повторной проверки) + +Документ r3 не был закоммичен в `docs/reviews/` (интеграция того захода +упала на дефекте, который правит именно материал этого раунда) — источник +вывода r3: комментарий ревью в issue #551 от 2026-09-13T11:29:02Z, материал +`23f48829c26deec968af6645301900880899139e` (дельта от r2/r3 к r4 — только +три файла выше, всё остальное дерево идентично). Наследуется: + +- разнесение конвейера на три job (`prepare`/`model_review`/`integrate`) с + независимыми бюджетами 55/45/55 минут и их назначение по коду — + подтверждено в r1 (`docs/reviews/CODE-REVIEW-551-r1.md`, материал `31ef70ce`) + и повторно не тронуто ни в r2, ни в r3, ни в этой дельте; +- контракт передачи между стадиями (`prepared.json`/`manifest.sha256`, + полная сверка полей через `jq -e`, `git rev-parse HEAD`/`HEAD^{tree}` + против заявленного материала) — из r1, не задет; +- поведение при timeout/cancel/failure каждой стадии и правило «ранний отказ + не расходует цикл» — из r1, не задет; двумя живыми провалами интеграции + (r2→r3, r3→r4) это правило дополнительно эмпирически подтверждено вне + ревью; +- отказ на неполном/чужом evidence (точный набор имён файлов artifact'а, + sha256, отказ от failed/cancelled/skipped результата модели) — из r1, не + задет; +- фикс r2 «structured verdict сохраняется как объект, а не boolean предиката + `jq -e`» (`process.yml`, шаг «Запечатать результат модели», строки до + `- name: Передать результат интеграции`) и его мутационный свидетель + `review-model-seals-verdict-as-boolean` — заявлен в r2, независимо + перепроверен в r3 (`node --test --test-name-pattern="#551"` + прогон + мутанта, «поймано 1 из 1»); дельта r3→r4 этот шаг не трогает + (`git diff 23f48829..62e0eff2` не включает диапазон `seal`), поэтому + принимается без повторного прогона в этом раунде; +- три исходных мутационных свидетеля r1 (`review-model-checks-out-moving-dev`, + `review-integration-skips-evidence-checksum`, + `review-integration-trusts-failed-model`) — из r1, не задеты ни одной + дельтой r2/r3/r4. + +## Найдено + +Находок нет — ни High, ни Medium, ни Low. + +## Не проверялось (и почему) + +- Полный набор `tsc`/`test`/`build`+сверка бандла, `check-docs.mjs`, + `golden:verify`, backend pytest, инварианты модели, performance-профили, + полный набор browser-смоков — не требуются по соразмерности (PROCESS.md + §8) и по факту зелёного Validate на этом точном SHA (см. выше) плюс + пустому отбору `smoke-select.mjs`. +- Реальное поведение исправленного шага в проде за пределами уже + показанных run — не переисполнялось повторно мной (GitHub Actions + недоступен для интерактивного запуска из этого ревью); проверка — + чтением кода плюс воспроизведением локального юнит-теста и мутанта, + которые моделируют тот же самый путь (`process.yml`-текст плюс regex по + нему), а не реальным запуском workflow. + +## Вердикт + +Зелёный. Дельта r4 — точечный фикс одной строки протокола передачи данных +между стадиями (`GITHUB_OUTPUT` delimiter), выявленный вторым живым прогоном +после r3. Фикс корректен (разобран построчно), закреплён тестом и +мутационным свидетелем, который я прогнал независимо и подтвердил, что он +красит именно старое поведение. Всё содержательное из AC issue #551 +(разнесение бюджетов, контракт, отказ на неполном evidence, поведение при +провале стадии) наследуется из r1 без изменений; правка r2 (boolean-подмена) +перепроверена в r3 и не задета этой дельтой. Находок нет, скоуп не расширен. + +--- + + + +## Материал раунда + +- Ветка: `issue/551-review-stage-budgets`, коммит `62e0eff21999` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `8ea4718a882bb877b6288a234823406709f5659c` + ``` + git log --all --format='%H %T' | grep 8ea4718a882b + ``` +- Тело issue: `824f489aab52c7d76ade6634cbdb819c9c36ff9261ae2982484d5199a4bc23d0` +- Вердикт конвейера: `green` · High 0