13 KiB
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),
поэтому 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,
который я лично прогнал и подтвердил, что он ловит снятие фильтра. Гейты,
относящиеся к диффу, зелёные. Документация и трейлеры в порядке.
Вердикт: зелёный
Материал раунда
- Ветка:
issue/546-wait-round, коммит5fc596c748c5— ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. - Дерево материала:
2350e7169f6f98b97900e4aabaebd5ea47dfe492git log --all --format='%H %T' | grep 2350e7169f6f - Тело issue:
0968fbd2f409d44f80911b445838422e0563d955aa08ab72109bdec5253ca3d5 - Вердикт конвейера:
green· High 0