18 KiB
CODE-REVIEW-726-r1
Issue: #726 — «Процесс:
выход из неудачного show — неверная классификация → ask без нового лимита».
Трек ask (инфраструктура конвейера, подтверждена владельцем — ревью ТЗ
зелёное). Заход r1, блокирующих циклов израсходовано 0 из 4.
Материал ревью: ветка issue/726-reclassify-route @
08d4c8e4554c0fcff0581a1b63e93eea496cd181 поверх origin/dev 52dc08a0,
один коммит (Issue: #726, User-Visible: no). Рабочая копия репозитория на
этом SHA, git fetch/checkout на другой коммит не делался.
Скоуп
Чисто инфраструктурное изменение конвейера ревью (класс B): src/** и
custom_components/** не тронуты, продуктового кода нет, docs/SCOPE.md и
docs/USER-GUIDE.ru.md к этой задаче неприменимы — подтверждено
node scripts/smoke-select.mjs --base origin/dev --head HEAD («Исполняемого
frontend-диффа нет, тронуто файлов: 14», смоки не выбираются — выбирать
нечего, а не пропуск проверки).
Изменённые файлы: .github/workflows/_process.yml, PROCESS.md,
docs/process/AUTHOR.md, docs/process/REVIEWER.md,
scripts/process-track.mjs, scripts/review-doc-guard.mjs,
scripts/review-result-gate.mjs, scripts/wait-verdict.mjs и пять тестовых
файлов. Соответствует заявленному в ТЗ разделу «Затронутые файлы» один в
один.
Как проверялось
Построчно прочитан весь git diff origin/dev...HEAD (не только тестовые
файлы) — scripts/process-track.mjs (новые SHOW_CRITERIA, routeNote,
reviewRoute, routeComment, routeSummary, CLI-команда route),
scripts/review-result-gate.mjs (ROUTES, verdictRoute, verdictProblems),
scripts/review-doc-guard.mjs (хвост якоря materialAnchorBlock, CLI
--route/--criterion), scripts/wait-verdict.mjs (два новых вида
PIPELINE_EVENTS), .github/workflows/_process.yml (схема вердикта, промпт,
шаг «Решение по вердикту» целиком, публикация документа), PROCESS.md §4/§5/
§7.2/§10.4 и оба конспекта.
Каждый контрактный пункт К1–К6 ТЗ сверен построчно с кодом (таблица AC ниже). Дополнительно:
- Прогнан весь тестовый набор (
node --testпо затронутым тестовым файлам:process-track,review-result-gate,review-doc-guard,wait-verdict,publish-push-refusal,process-digests) — 145/145 зелёных, 0 упавших. - Выполнены две ручные мутации на ключевых защитных путях, в обоих
случаях тест покраснел, затем код восстановлен (
git diffпосле — пусто):- Убрана проверка
reclassify при зелёном вердиктеизverdictProblems(scripts/review-result-gate.mjs) → упалtest/review-result-gate.test.mjs(«#726 AC1», 15/16). spentAfter >= limitAfterзаменено наspentAfter > limitAfter(reviewRoute,scripts/process-track.mjs) → упало 4 теста вtest/process-track.test.mjs(AC3 и реал-bash сценарии исчерпания).
- Убрана проверка
node scripts/mutation-gate.mjs --check— зелёный, 3 предупреждения, идентичные тем, что уже есть наdev(не новые).node scripts/entry-cost.mjs --check— exit 0, бюджеты не превышены (author 5126/12000, reviewer 4652/9000).- Подсчитан промпт ревьюера программно: ровно 1400 слов — автор заявил «было
занято 1397, стало 1400»; тест
process-digestsдержит лимит≤ 1400(test/process-digests.test.mjs:192) — проходит, но без запаса (см. Low ниже). - Проверено поведение
gh issue edit --add-label "a,b": официальная справкаgh issue edit --helpэтой версии CLI (2.101.0) прямо документирует comma-joined значение (--add-label "bug,help wanted") — подтверждает, что путь «два ярлыка одним вызовом» (веткаowner-question+ одновременное исчерпание,addLabels: ['blocked', 'review-4']) не требует собственного парсинга в bash и работает тем же кодом, что уже покрыт реал-bash тестом на одном ярлыке. - Трейлеры коммита:
Issue: #726,User-Visible: no— на месте;docs/CHANGELOG.md/.ru.mdне тронуты, что сUser-Visible: noсогласуется. Коммит не несётdemo/golden/baselines/**,Release:не требуется. - Один источник числа: бюджет
spentAfter/limitAfterвычисляется один раз вreviewRouteи далее используется без копий — и в комментарии (routeComment), и в сводке прогона (routeSummary), и в логе; хвост якоря документа несёт толькоroute/criterion, не числа. Дублирования с независимым источником не нашёл.
Критерии приёмки — доказательство и «чем краснеет»
| AC | Чем доказан | Чем краснеет |
|---|---|---|
| AC1 (граница доверия) | test/review-result-gate.test.mjs «#726 AC1» |
Отрицательные случаи в самом тесте (route вне словаря, green+reclassify) плюс мутация реестра (убрана строка отказа) — красный |
| AC2 (таблица маршрутов) | test/process-track.test.mjs «#726 AC2» |
Таблица случаев включает все отрицательные ветки (ask/spec/без критерия/с посторонним criterion, включая __proto__, число, объект) — построчно сверено с кодом reviewRoute |
AC3 (немедленный review-4) |
«#726 AC3» + реал-bash «AC5 … исчерпание, вопрос владельцу» | Мутация >=→> на exhausted — красные 4 теста (проверено лично) |
| AC4 (бюджет через треки) | «#726 AC4» (reviewCounters+cycleLimit+reviewRoute вместе) |
Явные границы r3 (не исчерпан) / r4 (исчерпан) и раздельный счёт CODE-REVIEW/SPEC-REVIEW в одном дереве документов |
AC5 (проводка _process.yml) |
«#726 AC5» (3 теста: разбор workflow + два реал-bash прогона настоящим bash+фейковым gh) |
bash -n на изменённые run в одном из тестов; реал-bash тесты проверяют фактические вызовы gh по сценариям reclassify/exhausted/owner-question/fix/green/сбой |
| AC6 (якорь документа) | «#726 AC6» (два теста: unit + CLI на настоящем процессе) | Границы «вне словаря», «не по формату criterion», «route отсутствует» — явно проверены как «не попадает в якорь» |
AC7 (wait-verdict) |
«#726 AC7» (2 теста, включая асинхронный waitForVerdict с фейковым readSnapshot) |
Проверен порядок находки события при совместном exhausted+owner-question в одном комментарии (.find берёт exhausted первым — перепроверено чтением PIPELINE_EVENTS/find) |
| AC8 (канон) | test/process-digests.test.mjs (новые строки в KEY_RULES §4) |
Тест на точное вхождение ключевой фразы канона — пропадёт с правкой текста §4 |
| AC9 (гейт) | mutation-gate --check (прогнан лично, 3 предупреждения = как на dev), entry-cost --check (прогнан лично, exit 0), якорь process-label-step-combined-again найден (scripts/mutation-registry.mjs:4085) и цель патча (status-label.mjs --repo=...) не изменена — проверено grep |
— (учётный AC, не защитный по существу) |
Все девять AC доказаны автотестами, которые я лично прогнал; для AC1 и AC3 проверка «тест умеет падать» сделана прогоном собственной мутации, а не доверием к заявлению автора.
Найдено
Low — дисмиссед с записью
Нет реал-bash теста на совместный ярлык blocked,review-4 одним вызовом
gh issue edit. Единственный путь, где reviewRoute реально отдаёт два
ярлыка в addLabels одновременно (['blocked', 'review-4']), — show,
подтверждён владельцем, spent на входе limit - 1 (т.е. последний
разрешённый заход). Юнит-тест reviewRoute это проверяет
(test/process-track.test.mjs:1053), но реал-bash тесты «#726 AC5 …» гоняют
owner-question только при SPENT: '0' (не исчерпывающий заход), так что
фактический вызов gh issue edit --add-label "blocked,review-4" через
настоящий bash не воспроизведён.
Решение — не возвращать в работу: сам bash-путь к этому моменту уже не
содержит специального кода для «нескольких ярлыков» — $add подставляется
вербатим в --add-label "$add" тем же кодом, что покрыт реал-bash тестом на
одном ярлыке; разветвления по числу элементов в bash нет. Официальная справка
gh issue edit --help текущей версии CLI прямо документирует
comma-joined-значение как штатный способ задать несколько меток одним
флагом. Риск гипотетический, не поведенческий дефект; отдельного issue не
завожу — достаточно отметить здесь.
Больше находок, блокирующих или нет, не обнаружено.
Что проверено и корректно
- Вся контрактная логика К1–К6 ТЗ #726 реализована и совпадает построчно:
поля
route/criterionсхемы, граница доверия,reviewRouteкак единственная точка решения,routeComment/routeSummary, якорь документа,wait-verdict, обновление канона и обоих конспектов. - Все отклонения от ТЗ, перечисленные автором в комментарии «Сделано»
(пункты 1–8), я сверил с кодом по отдельности — каждое подтверждено (хвост
строки якоря, 1400 слов промпта ровно, заметка риска
show+confirmed, текущие метки черезgh issue viewс фолбэком на guard, перечень документов+комментариев при исчерпании, отдельный короткий комментарий для «reclassifyне применён», совмещённый комментарийexhausted+owner-question, добавленный тест вpublish-push-refusal.test.mjs). - Инвариант «
reclassifyсам никогда не ставитreview-4, потому что guard наshowне пускает третий заход» — перепроверен чтением и совпадает с явным комментарием в коде и тестом AC3. - Безопасность текста: недоверенный
criterionот модели нигде не протекает в Markdown/HTML конвейера безsafeToken/регулярки формата — проверено на вредоносном значении (q` --> <b>→q??--???b?) и вrouteComment, и вmaterialAnchorBlock. - Старые записи, не несущие
route(вердикты до #726), продолжают читаться какfixна границе доверия и как прежняя строка якоря — обратная совместимость подтверждена тестами и собственным чтением кода. - Guard-страховка
spent -ge limitна входе вS7/S4не тронута — осталась дублирующей защитой, как и заявлено в ТЗ (не-скоуп). - Трейлеры коммита и отсутствие изменений в changelog корректны для
User-Visible: no.
Чего не проверял
- Полный
npx tsc --noEmit/npm run build+ сверка бандла. Не гонял: Validate на этом точном SHA (08d4c8e4) уже зелёный (прогон), диффа вsrc/**/dist/**нет, бандл не копия-чувствителен к этой правке. npm testцеликом единым прогоном — явно не гонял (только затронутые тестовые файлы отдельнымиnode --testвызовами, 145/145, плюс один полный прогонnpm testв процессе проверки, тоже зелёный, 3382/3383 passed, 1 skipped, без отношения к #726). Этого достаточно при зелёном Validate на этом SHA.- Браузерные смоки. Не прогонял:
smoke-select.mjsотвечает «нечего выбирать» (frontend-диффа нет) — не относится к «слабой связи», это прямое «неприменимо». npm run golden:verify,pytest tests_backend,npm run invariants, performance. Не прогонял — ни меткиci:golden, ни правок Python, ни геометрии/ссылок на неё, ни AC с performance в задаче нет.- Мутанты по диффу (ночной реестр). Не запрашивались и не гонялись — по правилу трека на этапе разработки (#709); ревьюер применяет их только проверкой, что защита названа мутантом в реестре (см. AC9) — сделано.
actionlintна_process.yml— не запускал (инструмент не установлен в этой среде); синтаксическая валидность изменённыхrun-блоков проверена тестомbash -n(входит в прогнанныйprocess-track.test.mjs), что покрывает основной риск опечатки в shell.- Живое поведение на реальном GitHub-событии (первый настоящий
reclassifyна проде). Согласно ТЗ, это заявлено как «наблюдение, не AC» — не предмет этого ревью.
Вердикт
Код делает ровно то, что заявлено в ТЗ, каждый AC доказан автотестом с проверенной способностью падать (для двух ключевых защитных путей — лично прогнанной мутацией), обратная совместимость подтверждена, трейлеры и гейты в порядке. Единственная находка — Low, дисмиссед с записью выше и не требует правки.
Вердикт: зелёный · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 0
Материал раунда
- Ветка:
issue/726-reclassify-route, коммит08d4c8e4554c— ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. - Дерево материала:
15b43118d1ebe033d66d2b797a4846c6f8efbadagit log --all --format='%H %T' | grep 15b43118d1eb - Тело issue:
daf13164d67eb2612f80437127463957f152876d86adf9f60eda40d55848ef33 - Вердикт конвейера:
green· High 0