diff --git a/docs/reviews/CODE-REVIEW-515-r1.md b/docs/reviews/CODE-REVIEW-515-r1.md new file mode 100644 index 00000000..7c26e262 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-515-r1.md @@ -0,0 +1,149 @@ +# CODE-REVIEW-515-r1 + +Материал: `5a1cddeaf0fbac3e96bdc7ace20091a4549691c1`, ветка +`issue/515-material-anchors-after-rebase`, `origin/dev` не ушёл с этой темы +дальше — единственный коммит в диапазоне `origin/dev..HEAD`. + +## Скоуп + +Issue #515 (light track, ТЗ в теле issue): якоря материала ревью (`sha`, +`tree`, `specs`), которые публикация зашивает в машинный блок документа, +снимались в шаге «Перейти на ветку задачи» — **до** шага «Привести ветку к +dev». Когда `dev` ушёл вперёд, конвейер ребейзит ветку и делает +force-push; коммит и дерево, снятые до ребейза, становятся недостижимыми +на origin. Это ломало reuse (#499, всегда `reuse=false` в свежем клоне) и +красило пост-шаг «Материал раунда воспроизводим (#413)» на зелёном раунде. + +Диффа два файла, оба класса B (Гейты и инструменты, PROCESS.md §1): +`.github/workflows/process.yml` (перенос снятия `tree`/`specs` из шага +`branch` в шаг `material`, который уже существовал и уже снимал `sha` +после ребейза; смена источника `MATERIAL_SHA/TREE/SPECS` в шаге публикации +с `steps.branch.outputs.*` на `steps.material.outputs.*`) и +`test/review-doc-guard.test.mjs` (новый контрактный тест на текст +workflow). Класса A (продуктовый код) диффа не задевает. + +Замечание не по существу проверки: задача помечена `infra`/`process` и по +механическому признаку §1 (ни одного файла класса A) могла бы идти «вне +флоу» без код-ревью вовсе — но провести пайплайн-ревью пайплайна через сам +пайплайн разумно именно потому, что часть AC (см. ниже) в принципе +проверяется только живым прогоном следующей задачи. Решение маршрутизации +не моё, и на итог ревью не влияет. + +## Как проверялось + +- Полное чтение диффа (`git diff origin/dev...HEAD`) и контекста вокруг + него в `process.yml`: шаги `branch` (id=branch), `rebase` (id=rebase), + `material` (id=material), `reuse` (id=reuse, #499), «Материал раунда + воспроизводим» (#413), «Опубликовать документ ревью», «Слить ветку в + dev», «dev ушёл вперёд, пока шло ревью» — все места, где раньше или + теперь читаются `steps.branch.outputs.*` / `steps.material.outputs.*`. +- Чтение `scripts/review-doc-guard.mjs`: `anchorTreeFrom`, `anchorLiveness`, + `reusableGreenVerdict`, ветки `--reuse` и `--doc=` в CLI — то есть + фактическую механику, которая потребляет якоря, записанные новым кодом. +- `grep -rn "steps\.branch\.outputs\.\(tree\|specs\)"` по всему репозиторию + (md/yml/mjs) — ноль совпадений, старый источник нигде не остался. +- **Тест умеет падать** — исполнено, не заявлено: временно подменил + `.github/workflows/process.yml` на версию `origin/dev` (до фикса), + прогнал `node --test test/review-doc-guard.test.mjs`. Новый тест `#515` + красный: + `error: 'дерево — из шага material'`, актуальный текст шага `material` + в выводе ассерта — старая версия без `tree=`. Вернул файл на HEAD + (`git status --short` — пусто, рабочая копия чистая), прогнал тест ещё + раз: `49/49 pass`, включая новый. +- `python3 -c "import yaml; yaml.safe_load(...)"` — YAML `process.yml` + валиден. +- `bash -n` на извлечённых телах трёх изменённых `run:`-блоков (`branch`, + `material`, «Опубликовать документ ревью») — синтаксис чист. +- `node scripts/smoke-select.mjs --base origin/dev --head HEAD` → + «Исполняемого frontend-диффа нет… Тронуто файлов: 2» — смоки не + выбираются, подтверждено инструментом, не по названию. +- Дешёвые гейты (`tsc --noEmit`, `npm test`, `npm run build` со сверкой + бандла) на этом SHA уже зелёные в Validate + (https://github.com/Matysh/houseplan-card/actions/runs/34404945290) — + не перегонял. + +### Что не проверял и почему + +- `npm run golden:verify`, `check-docs.mjs`, `npm run invariants`, + `pytest tests_backend` — diff не трогает `src/**`, рендер, геометрию или + `custom_components/**/*.py`; неприменимо. +- Полный набор browser-smoke — инструмент явно сказал «выбирать нечего». +- **AC2 и AC3 живым прогоном** — не выполнено и не могло быть выполнено в + рамках этого ревью: оба требуют повторного `S7` конвейера на реальной + задаче после зеркалирования `process.yml` в `main` (#454, п.3 ТЗ, вне + этого коммита). Сам текст ТЗ отдаёт эту проверку живому прогону, а не + ревью. Ниже — «проверено чтением, не исполнением» с прослеженной + цепочкой вызовов, но не заменяющее реальный прогон. + +## Разбор по AC + +**AC1** (после ребейза блок якорей называет пост-ребейзный коммит/дерево, +достижимые с origin; тест краснеет на возврате к `steps.branch.outputs`). +Доказано автотестом, тест проверен на падение (см. выше). Дополнительно +прочитан путь данных: `material.sha/tree/specs` → env `MATERIAL_*` шага +«Опубликовать документ ревью» → `review-doc-guard.mjs --anchor=... --sha= +--tree= --specs=` → `materialAnchorBlock` в тексте документа. Момент снятия +(`material`, после `git push --force-with-lease` в шаге `rebase`) гарантирует, +что дерево уже лежит в запушенном коммите. **Выполнен.** + +**AC2** (повторный `S7` без изменений кода поверх зелёного документа даёт +`reuse=true` без вызова модели). Проверено чтением: `differs(tree)` в +`--reuse` сначала делает `git cat-file -e ^{tree}` — до фикса дерево +было пре-ребейзным и в свежем клоне не существовало физически (осиротело +force-push'ем), это и давало жёсткое `differs=true` независимо от +реального содержимого, что дословно совпадает с симптомом в issue. +После фикса дерево — то, что реально запушено на ветку, и `cat-file` +находит его в клоне следующего прогона; дальше сравнение идёт по +содержимому, как и задумано. Логика заявленный дефект устраняет. Живым +прогоном не подтверждено — по объективной причине (см. выше). + +**AC3** (пост-шаг #413 не красит зелёный раунд с ребейзом). Проверено +чтением: `anchorLiveness` для типа `tree` ищет объект перебором `%T` по +`--remotes=origin --tags`; при верном (запушенном) дереве это ровно тот +поиск, который срабатывает. До фикса — то же дерево, что и в AC2, +осиротевшее и не входящее ни в один `%T` с origin. Логика согласуется. +Живым прогоном не подтверждено — та же причина. + +**AC4** (`User-Visible: no`). Коммит содержит трейлеры `Issue: #515` и +`User-Visible: no` — соответствует. Продуктовых CHANGELOG-файлов дифф не +трогает, что и требуется при `User-Visible: no`. + +## Что проверено и корректно + +- Единственное place, где раньше писались `tree`/`specs` (шаг `branch`), + теперь только поясняющий комментарий, само снятие убрано полностью — не + осталось дублирующего, конфликтующего источника. +- `material`-шаг выполняется на обеих стадиях (`spec` и `code`): его + `if: steps.rebase.outputs.conflict != 'true'` истинен и когда `rebase` + пропущен целиком (стадия `spec`, где ребейза не бывает) — поведение для + `spec` не регрессирует, дерево там совпадает что до, что после (ребейза + нет, снимать раньше или позже — один и тот же коммит). +- Через `material.sha` (не через сегодняшний дифф — использовалось и + раньше) уже строился системный промпт ревьюера («Материал ревью — ровно + `…`»); эта правка выравнивает и опубликованный якорь с тем же источником + — раньше `MATERIAL_SHA` в анкоре и SHA в промпте ревьюера могли + расходиться, теперь один источник для обоих. +- Комментарии перенесены по смыслу, а не скопированы бездумно: у `branch` + теперь короткое объяснение «почему не здесь», у `material` — полное + объяснение «почему здесь». + +## Находки + +Нет ни High, ни Medium. + +## Вердикт + +Зелёный. + +--- + + + +## Материал раунда + +- Ветка: `issue/515-material-anchors-after-rebase`, коммит `5a1cddeaf0fb` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `ec563a09b642fd1e7742dbcfa089c25ef7b8b8df` + ``` + git log --all --format='%H %T' | grep ec563a09b642 + ``` +- Вердикт конвейера: `green` · High 0