mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-30 11:49:16 +00:00
claude[bot]
@@ -0,0 +1,115 @@
|
||||
# CODE-REVIEW-511-r1
|
||||
|
||||
**Issue:** [#511](https://github.com/Matysh/houseplan-card/issues/511) — release-gate: судить по последнему завершённому прогону на SHA, отменённые не блокируют
|
||||
**Этап:** code (PROCESS.md §2.7)
|
||||
**Заход:** r1 · блокирующих циклов израсходовано 0 из 2 (лёгкий трек)
|
||||
**SHA материала:** `9d8f89d2600d573a7d57834659c1de0f0445f2dd` (рабочая копия закреплена на нём, `git fetch`/`checkout` не выполнялись)
|
||||
**Класс изменения:** B — гейты и тулинг (`scripts/**`, `test/**`, `docs/**`); issue переиспользован из аналитики/spec, статус `S7-code-review`, лёгкий трек (`small`)
|
||||
|
||||
## Скоуп
|
||||
|
||||
Диапазон `origin/dev..HEAD` — один коммит `9d8f89d2`:
|
||||
|
||||
```
|
||||
demo/performance/README.md | 4 +++-
|
||||
docs/DEVELOPMENT.md | 9 ++++++---
|
||||
scripts/mutation-gate.mjs | 22 ++++++++++++++++++++++
|
||||
scripts/release-gate.mjs | 33 ++++++++++++++++++++++++---------
|
||||
test/release-gate.test.mjs | 40 ++++++++++++++++++++++++++++++----------
|
||||
5 files changed, 85 insertions(+), 23 deletions(-)
|
||||
```
|
||||
|
||||
`src/**`, `custom_components/**`, `.github/workflows/**`, `dist/**` не тронуты — чистая инфраструктура сборочного/релизного гейта, продуктовый код не меняется. Ни одного класса A файла в диффе.
|
||||
|
||||
Продуктовая рамка (docs/SCOPE.md): задача не про J1–J7 напрямую — это внутренний инструмент релизного процесса (инфраструктура, не UX). SCOPE.md её не ограничивает; проверка по AGENTS.md/PROCESS.md релевантна вместо неё.
|
||||
|
||||
## Материал ревью
|
||||
|
||||
Issue #511, тело (проблема · решение владельца · AC1–AC3 · откат) и вся история комментариев:
|
||||
- S2-аналитика (Codex) → трек `small`.
|
||||
- Spec-review r1 (жёлтый, Medium в скоупе: AC3 указывал на несуществующий `docs/performance/README.md`).
|
||||
- Spec-review r2 (зелёный): AC3 переформулирован на реальный `docs/DEVELOPMENT.md:359-360`, раздел «Откат» добавлен, способ доказательства AC2/AC3 назван. Отмечена пограничная (не блокирующая) двусмысленность второй части AC3 про `demo/performance/README.md` — «та же фраза» может значить либо правку существующей, либо добавление новой поясняющей фразы; оба чтения ведут к одному безопасному результату.
|
||||
- Хендофф S6→S7 (Codex): описание реализации, тестов, мутантов, локального прогона.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Материал — сам диплой (сборка) не запускался, зелёного Validate на этом SHA нет, поэтому дешёвые гейты прогнаны вручную:
|
||||
|
||||
| Гейт | Команда | Результат |
|
||||
|---|---|---|
|
||||
| Типы | `npx tsc --noEmit` | чисто, без вывода |
|
||||
| Юнит-тесты (весь набор) | `npm test` | `# tests 2408 / pass 2407 / fail 0 / skipped 1` — единственный skip не в `test/release-gate.test.mjs` и не связан с диффом (не найден в измененных файлах; предсуществующий, не регрессия этой задачи) |
|
||||
| Юнит-тесты (точечно) | `node --test test/release-gate.test.mjs` | `6/6 pass` |
|
||||
| Сборка | `npm run build` | успешно, `tsc --noEmit && rollup -c` зелёные; `git status --short` после сборки пуст — три копии бандла (`dist`, `custom_components/houseplan/frontend`, `demo/srv/assets`) уже в синхроне, диффом не задеты (src/** не менялся) |
|
||||
| Синтаксис `mutation-gate.mjs` | `node --check scripts/mutation-gate.mjs` | ОК |
|
||||
|
||||
Не прогонялись и почему:
|
||||
- `node scripts/check-docs.mjs` — не требуется: диплом не трогает `src/**` (отпечаток скриншотов документации завязан на `src/**`, здесь он не устарел).
|
||||
- Инварианты модели (`npm run invariants`) — диплом не трогает геометрию, рёбра комнат, `layout`, толщину, `open_spans`; неприменимо.
|
||||
- Browser-смоки — прогнал `node scripts/smoke-select.mjs --base origin/dev --head HEAD`: вывод — *«Исполняемого frontend-диффа нет (src/**/*.ts не тронут). Browser-smoke этим диффом не выбираются — это не "пропустить проверки", а "выбирать нечего": смоки проверяют собранную карточку. Тронуто файлов: 5.»* Диплом состоит из Node-скрипта, его теста и markdown — карточка не пересобирается по-другому, смоки бить нечего.
|
||||
- `npm run golden:verify` — диплом не меняет рендер/геометрию/стили/слои.
|
||||
- `python -m pytest tests_backend -q` — `custom_components/**/*.py` не тронут.
|
||||
- Перфоманс-профили — не названы в AC, диплом не трогает чувствительные к перфу пути (только формулировку в README).
|
||||
|
||||
## Проверка AC по коду
|
||||
|
||||
**AC1** — `[green(older), red(newer)]→fail`; `[red(older), green(newer)]→success`; `[cancelled]→wait`; `[cancelled(newer), green(older)]→success`; незавершённый последний → wait; пустой список → wait.
|
||||
|
||||
Доказано автотестом `test/release-gate.test.mjs` (тест `#511: the latest non-cancelled run is the verdict…`), покрывающим ровно эти шесть пар плюс порядок-независимость и `latestRelevantRun` напрямую. Читкой кода (`scripts/release-gate.mjs:16-25`) подтверждено соответствие алгоритма формулировке AC: `latestRelevantRun` отбрасывает `conclusion === 'cancelled'`, сортирует по `run_started_at`/`created_at` (убыв.), при равенстве — по `id`; `classifyValidateRuns` возвращает `wait` при отсутствии/незавершённости последнего релевантного прогона, иначе `success`/`fail` по его `conclusion`.
|
||||
|
||||
Дисциплина «тест умеет падать» проверена не декларативно, а вручную для обоих заявленных мутантов — временно применил патчи `release-gate-counts-cancelled-runs` и `release-gate-oldest-run-wins` к рабочей копии `scripts/release-gate.mjs`, прогнал `node --test test/release-gate.test.mjs`, вернул файл (`git diff --stat` после отката — пусто):
|
||||
- мутант «cancelled считаются» → `5 pass / 1 fail`;
|
||||
- мутант «побеждает самый старый» → `4 pass / 2 fail`.
|
||||
|
||||
Оба мутанта ловятся тестом реально, не только по утверждению автора. **AC1 выполнен.**
|
||||
|
||||
**AC2** — `release.yml` не меняется, вызовы `release-gate.mjs` для Validate и Full Performance те же.
|
||||
|
||||
`git diff --stat origin/dev...HEAD` не содержит `.github/workflows/**` вовсе. Прочитан текущий `.github/workflows/release.yml:43` (`node scripts/release-gate.mjs "$SHA"`) и `:52` (`node scripts/release-gate.mjs "$SHA" --workflow=performance.yml --label=…`) — совпадают с состоянием до задачи, CLI-контракт скрипта (`invokedDirectly`-блок, `scripts/release-gate.mjs:80-95`) не менялся. **AC2 выполнен**, доказано чтением, не исполнением workflow (GitHub Actions здесь не прогонялись — сам гейт CI, его тело неисполнимо локально).
|
||||
|
||||
**AC3** — `docs/DEVELOPMENT.md` и `demo/performance/README.md` описывают новую семантику; доказательство — grep фраз в `release-gate.test`.
|
||||
|
||||
Тест `#511 AC3: the release documents describe the latest-run semantics` (`test/release-gate.test.mjs`) читает оба файла и проверяет: (а) `docs/DEVELOPMENT.md` содержит новую формулировку и не содержит старую («A missing, failed,\ncancelled or one-hour-timed-out Validate withholds»); (б) `demo/performance/README.md` содержит новую фразу. Прогнан — зелёный. Читкой самих файлов (`docs/DEVELOPMENT.md:356-363`, `demo/performance/README.md:121-127`) подтверждено, что текст не просто проходит regex, а содержательно верен: старая ложная фраза про `cancelled` удалена, новая описывает алгоритм «последний нетронутый cancelled прогон» с явной отсылкой на #511 и на прецедент v1.73.0.
|
||||
|
||||
Пограничное чтение AC3, отмеченное в spec-review r2 (правка существующей фразы vs добавление новой), разрешилось в пользу более безопасного варианта — добавления новой поясняющей фразы в оба файла без порчи соседнего контекста; расхождения между двумя чтениями не осталось: результат проверяем и корректен при любом из них. **AC3 выполнен.**
|
||||
|
||||
## Прочее замечаемое (не блокирует)
|
||||
|
||||
- `waitForGreenWorkflow` (`scripts/release-gate.mjs:52-53`) вычисляет `latestRelevantRun(runs)` отдельно от `classifyValidateRuns(runs)`, которая внутри себя тоже вызывает `latestRelevantRun` — двойной проход по тому же массиву на каждой итерации поллинга. Функционально безвредно (массив — максимум 100 элементов по `per_page=100`, опрос раз в 30 с), не находка, а наблюдение; не поднимаю до Low, так как эффективность здесь не в AC и не влияет на корректность.
|
||||
- Единственный `skipped` тест в `npm test` не входит ни в один из изменённых файлов и не упомянут в хендоффе как связанный с #511 — не регрессия этой задачи, дальше не копал (вне скоупа).
|
||||
|
||||
## Трейлеры
|
||||
|
||||
Коммит `9d8f89d2`: `Issue: #511`, `User-Visible: no`. Соответствует духу изменения — чисто внутренний CI-гейт, ни одно наблюдаемое пользователем поведение карточки не меняется, оба changelog корректно не тронуты.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Алгоритм `latestRelevantRun`/`classifyValidateRuns` соответствует AC1 буква в букву, включая все шесть заявленных пар и порядок-независимость сортировки.
|
||||
- `release.yml` не изменён и не должен был быть.
|
||||
- Оба канонических документа (`docs/DEVELOPMENT.md`, `demo/performance/README.md`) синхронизированы с новой семантикой, старая ложная формулировка удалена.
|
||||
- Мутанты `release-gate-counts-cancelled-runs` и `release-gate-oldest-run-wins` действительно ловятся тестом (проверено ручным патчем, не только заявлением автора).
|
||||
- Полный юнит-набор, typecheck и сборка зелёные на этом SHA.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- CI/GitHub Actions поведение `release.yml` вживую (реальный тег/релиз) — вне возможностей ревью-цикла, риск закрыт чтением кода (CLI-контракт и вызовы не изменились) и модульными тестами самого гейта.
|
||||
- Golden, browser-смоки, backend pytest, инварианты модели, перф-профили — неприменимы к диффу (обоснование в разделе «Как проверялось»).
|
||||
|
||||
## Вердикт
|
||||
|
||||
High: 0, Medium: 0. Все три AC доказаны — автотестом с подтверждённой способностью падать (AC1, AC3) и чтением кода/workflow (AC2). Гейты (tsc, весь юнит-набор, build, точечный тест, smoke-select) зелёные. Трейлеры корректны. Не в скоупе задачи находок нет.
|
||||
|
||||
**Зелёный.**
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/511-release-gate-latest-run`, коммит `9d8f89d2600d` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `a22d70bc0a55770f667bb6afb4d3f943b50518a6`
|
||||
```
|
||||
git log --all --format='%H %T' | grep a22d70bc0a55
|
||||
```
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user