mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -0,0 +1,140 @@
|
||||
# CODE-REVIEW-546-r1
|
||||
|
||||
Issue: #546 · Заход: r1 · Материал: `5fc596c748c59e53d87297085a6b053c7f149c6e` (HEAD ветки `issue/546-wait-round`, `git diff origin/dev...HEAD`)
|
||||
|
||||
## Скоуп
|
||||
|
||||
Инфраструктурная задача (класс A файлов не затронут — трек §1 корректен, без S1…S6,
|
||||
сразу на `S7-code-review`). Правит `scripts/wait-verdict.mjs`: waiter больше не
|
||||
считает *любой* последний pipeline-комментарий issue новым событием текущего
|
||||
раунда. Вводится якорь раунда — последнее фактическое применение метки
|
||||
`S4-spec-review`/`S7-code-review`, взятое из timeline issue
|
||||
(`GET /repos/{repo}/issues/{number}/events`, постранично). Комментарии старше
|
||||
этого якоря — baseline и не завершают ожидание; комментарии не раньше якоря
|
||||
(включая уже опубликованные до старта waiter) доставляются как раньше.
|
||||
|
||||
Изменённые файлы: `scripts/wait-verdict.mjs`, `scripts/mutation-gate.mjs`,
|
||||
`test/wait-verdict.test.mjs`, `AGENTS.md`, `PROCESS.md`. Один коммит,
|
||||
трейлеры `Issue: #546` / `User-Visible: no` на месте — корректно: правится
|
||||
только внутренний dev-tooling, пользовательского поведения нет, changelog не
|
||||
требуется.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Validate на этом SHA зелёный ([run 34745280749](https://github.com/Matysh/houseplan-card/actions/runs/34745280749)),
|
||||
поэтому `npx tsc --noEmit` / `npm test` / `npm run build` со сверкой копий
|
||||
бандла не перегонялись — взят как подтверждение дешёвых гейтов. Diff не
|
||||
трогает `src/**`, геометрию, `custom_components/**/*.py` и визуальный
|
||||
результат, поэтому `check-docs.mjs`, `model-invariants.mjs`, `golden:verify`,
|
||||
`pytest tests_backend` и браузерные смоки не применимы (смок-выбор не
|
||||
запускал — diff вне `demo/**`/`src/**`, `smoke-select.mjs` не даёт кандидатов
|
||||
по такому диффу).
|
||||
|
||||
Прогнано мной в этом раунде (код изменился со времени прогона в хендоффе,
|
||||
плюс это ровно предмет находок предыдущих раундов #546-подобных задач —
|
||||
дёшево и целевое):
|
||||
|
||||
| Команда | Результат |
|
||||
|---|---|
|
||||
| `node --test test/wait-verdict.test.mjs` | 8/8 green |
|
||||
| `node --test --test-name-pattern="#546" test/wait-verdict.test.mjs` | 4/4 green (целевые тесты задачи) |
|
||||
| `node scripts/mutation-gate.mjs --check` | зелёный, весь реестр, включая новый мутант |
|
||||
| `node scripts/process-gate.mjs` | «гейт пройден, предупреждений 0» |
|
||||
|
||||
Не прогонял: `npm run gate:small` целиком, `npm run toolchain:check`,
|
||||
performance-профили — не относятся к диффу (нет продуктового/фронтенд кода,
|
||||
нет perf-чувствительных путей), и Validate на этом SHA их уже подтвердил там,
|
||||
где они применимы.
|
||||
|
||||
## AC — доказательство и «чем краснеет»
|
||||
|
||||
| AC (из тела issue) | Чем доказан | Чем краснеет |
|
||||
|---|---|---|
|
||||
| Старый failure + новый running review: ожидание продолжается без ложного выхода | `test/wait-verdict.test.mjs`: «старый failure до нового запроса ревью — baseline, ожидание продолжается (#546)» | мутант `wait-verdict-reuses-historical-failure` (`scripts/mutation-gate.mjs`): снимает фильтр `eventBelongsToReview` — `node scripts/mutation-gate.mjs --check` ловит (строка `ok wait-verdict-reuses-historical-failure`; проверил также прямым запуском guard-команды `node --test --test-name-pattern="#546" test/wait-verdict.test.mjs` с патчем, гейт-скрипт сам применяет и откатывает патч и подтверждает провал под мутацией) |
|
||||
| Текущий failure уже опубликован до начала wait: он обнаруживается сразу | «failure текущего раунда, опубликованный до запуска waiter, виден на первом poll (#546)» | тот же мутант — фильтр общий для обеих веток (baseline и current) |
|
||||
| Новый verdict, отмена/смена material и owner-blocker различаются; не игнорировать все первоначальные comments | «новые outcome и owner blocker текущего раунда не скрываются baseline-фильтром (#546)» — прямое сравнение ожидаемого/фактического, не требует мутанта (ветвление не защитное, а обычная маршрутизация событий, уже покрытая исходным #496-набором тестов) | н/п — не защитный AC в терминах §2.7 (правило прямо выводит из-под мутанта «расположение/текст/формат», сюда же относится «какой code возвращается по какому типу события»); свидетель — сравнение |
|
||||
| Тесты работают детерминированно без реального длительного sleep и без модели | проверено чтением: `sleep` — инъецируемая функция-счётчик, `readSnapshot` — синтетический замыкающий объект без сети; исполнением подтверждено временем прогона (`duration_ms` < 100 мс на всю группу #546) | н/п, не защитный AC |
|
||||
|
||||
Якорь раунда (`reviewRequestFromEvents`) проверен отдельно тестом «якорь
|
||||
раунда — последнее применение любой review-метки (#546)»: смешанный поток
|
||||
`labeled S4 → labeled P1 → unlabeled S4 → labeled S7` корректно даёт
|
||||
последним именно `S7`-событие. Мутанта на этот отбор нет, но неверный выбор
|
||||
анкера транзитивно ломает оба #546-теста через общий путь `stateOf` →
|
||||
`eventBelongsToReview`, поэтому и здесь есть живой свидетель регрессии.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- `ghSnapshotReader` использует `gh api --paginate --slurp
|
||||
repos/{repo}/issues/{number}/events?per_page=100` и правильно разворачивает
|
||||
результат: `--slurp` оборачивает каждую страницу (сама по себе массив) во
|
||||
внешний массив, поэтому `pages` — массив массивов, и `pages.flat()`
|
||||
корректен для одной и для многих страниц; пустая история (`pages = [[]]`
|
||||
или `[]`) сводится к `timelineEvents = []`, а `reviewRequestFromEvents([])`
|
||||
возвращает `null` → `eventBelongsToReview` откатывается к старому
|
||||
поведению «всё принадлежит раунду» (совместимость со снимками без
|
||||
timeline, в том числе с прямыми вызовами `snap()` в тестах 1–4, не
|
||||
передающими `reviewRequest`) — прочитано и подтверждено прогоном тех же
|
||||
тестов (не регрессировали).
|
||||
- Формат события REST `/issues/{number}/events` (`event: 'labeled'`,
|
||||
`created_at`, `label: {name}`) соответствует и коду, и фикстурам тестов —
|
||||
прочитано, совпадает с задокументированной схемой GitHub REST API.
|
||||
- `blocked` и смена метки-вердикта не проходят через
|
||||
`eventBelongsToReview` вовсе (читаются из `labels`, не из `comments`),
|
||||
поэтому сужение к текущему раунду не может случайно скрыть их — прочитано
|
||||
в `decide`/`stateOf`, подтверждено тестом «новые outcome и owner blocker…».
|
||||
- Документация (`AGENTS.md`, `PROCESS.md`) обновлена в том же коммите и
|
||||
точно описывает новое поведение (baseline до последнего `S4`/`S7`, немедленная
|
||||
доставка уже случившегося исхода текущего раунда) — соответствует §2.6/§11
|
||||
DoD.
|
||||
- Трейлеры коммита корректны (`Issue: #546`, `User-Visible: no`); ветка
|
||||
`issue/546-wait-round` соответствует конвенции; `process-gate.mjs` зелёный
|
||||
локально.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Реальный вызов `gh api .../events` против живого GitHub (сетевой путь
|
||||
`ghSnapshotReader`) — не покрыт автотестом (как и раньше не был, для
|
||||
соседнего блока `gh run list` в этом же файле — не новый пробел, а
|
||||
существующий паттерн). Автор заявляет ручной read-only прогон на issue с
|
||||
длинной историей; принимаю как «проверено чтением + разовая ручная
|
||||
проверка автора», не как автотест. Риск низкий: контракт REST-эндпоинта
|
||||
стабилен, фикстуры тестов повторяют его форму дословно.
|
||||
- Тай-брейк `id.localeCompare` в `reviewRequestFromEvents` при **совпадающих
|
||||
секундных** таймстемпах двух labeled-событий сравнивает числовые id как
|
||||
строки (лексикографически, не численно) — теоretически может выбрать не
|
||||
тот из двух событий при коллизии. Не тестировалось и не мутировалось.
|
||||
Вероятность на практике пренебрежимо мала (два применения `S4`/`S7` в один
|
||||
и тот же issue в одну и ту же секунду) и не относится к заявленным AC —
|
||||
фиксирую как Low, не блокирует, правку не считаю обязательной.
|
||||
- Полный `npm run gate:small`, perf-профили, golden, backend pytest — не
|
||||
прогонялись, diff их не касается (см. «Как проверялось»).
|
||||
|
||||
## Находки
|
||||
|
||||
Нет High и Medium в скоупе. Одна Low-наблюдение (тай-брейк по id при
|
||||
одинаковых секундных таймстемпах, см. выше) — не блокирует зелёный вердикт,
|
||||
оставляю как запись без правки: цена коллизии ничтожна и вне AC задачи.
|
||||
|
||||
## Вывод
|
||||
|
||||
Все четыре AC доказаны детерминированными тестами; защитный AC (baseline
|
||||
против ложного немедленного выхода / против маскировки текущего отказа)
|
||||
дополнительно закрыт мутантом `wait-verdict-reuses-historical-failure`,
|
||||
который я лично прогнал и подтвердил, что он ловит снятие фильтра. Гейты,
|
||||
относящиеся к диффу, зелёные. Документация и трейлеры в порядке.
|
||||
|
||||
**Вердикт: зелёный**
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/546-wait-round`, коммит `5fc596c748c5` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `2350e7169f6f98b97900e4aabaebd5ea47dfe492`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 2350e7169f6f
|
||||
```
|
||||
- Тело issue: `0968fbd2f409d44f80911b445838422e0563d955aa08ab72109bdec5253ca3d5`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user