Files
2026-10-01 03:06:49 +00:00

18 KiB
Raw Permalink Blame History

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 после — пусто):
    1. Убрана проверка reclassify при зелёном вердикте из verdictProblems (scripts/review-result-gate.mjs) → упал test/review-result-gate.test.mjs («#726 AC1», 15/16).
    2. 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 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: 15b43118d1ebe033d66d2b797a4846c6f8efbada
    git log --all --format='%H %T' | grep 15b43118d1eb
    
  • Тело issue: daf13164d67eb2612f80437127463957f152876d86adf9f60eda40d55848ef33
  • Вердикт конвейера: green · High 0