From eed7239aa3198e2f32184cf0f1d3e65bb2866b64 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Wed, 9 Sep 2026 14:45:45 +0000 Subject: [PATCH] docs: review document for #511 Issue: #511 User-Visible: no --- docs/reviews/CODE-REVIEW-511-r1.md | 115 +++++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-511-r1.md diff --git a/docs/reviews/CODE-REVIEW-511-r1.md b/docs/reviews/CODE-REVIEW-511-r1.md new file mode 100644 index 00000000..578cdef6 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-511-r1.md @@ -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) зелёные. Трейлеры корректны. Не в скоупе задачи находок нет. + +**Зелёный.** + +--- + + + +## Материал раунда + +- Ветка: `issue/511-release-gate-latest-run`, коммит `9d8f89d2600d` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `a22d70bc0a55770f667bb6afb4d3f943b50518a6` + ``` + git log --all --format='%H %T' | grep a22d70bc0a55 + ``` +- Вердикт конвейера: `green` · High 0