21 KiB
ТЗ #454 — счёт заходов и циклов ревью по артефактам, а не по прозе вердикта
- Issue: https://github.com/Matysh/houseplan-card/issues/454
- Приоритет: P2,
bug,infra,process - Статус ТЗ: готово к ревью
- Маршрут: full; меняется механизм, управляющий всеми остальными задачами
- Класс изменения: B (инфраструктура). Продуктовый код,
requirements, бандл и модель данных не трогаются - Touch editor: not exposed — задача не касается интерфейса вовсе
- Связанные контракты: #227 (разделение
attemptиspent), #89 (вердикт чужого этапа съедал чужой бюджет), #220 и #365 (потеря артефакта ревью), #414 (якоря материала в документе ревью)
Сценарий
Ревьюер заканчивает вердикт фразой «Документ: см. артефакт ревью (публикуется шагом конвейера)» — не называя имя файла, потому что имя определяет движок. Автор правит находки и возвращает статусную метку. Следующий заход получает тот же номер, что и предыдущий, публикует документ по тому же пути и затирает предыдущий артефакт ревью. Одновременно счётчик израсходованных циклов показывает на единицу меньше, чем было на самом деле.
Что человек увидит до и после
До. В docs/reviews/ под именем …-r1.md лежит документ второго раунда;
документа …-r2.md не существует; первый раунд читается только из истории git
по конкретному SHA. В строке вердикта написано «блокирующих циклов 1/4», хотя
израсходовано 2.
После. Каждый заход получает собственное имя файла, ни один документ ревью не перезаписывается, а число израсходованных циклов совпадает с числом блокирующих вердиктов этого этапа независимо от того, как ревьюер сформулировал последнюю строку.
Подтверждённая проблема
Проверено исполнением на dev effb3629 и ветке issue/449-double-fit-all.
process.yml:87-101 считает обе величины по телу комментария:
of_stage="[.comments[] | select(.body | test(\"Вердикт:\")) | select(.body | test(\"$marker\"))]"
attempt=$(( $(… "$of_stage | length") + 1 ))
spent=$(… "$blocking | length")
Вердикт, не содержащий подстроки SPEC-REVIEW/CODE-REVIEW, не попадает в
of_stage навсегда: подсчёт каждый раз идёт заново по всей истории
комментариев, накопленного значения нет.
attempt — не только номер в метке: process.yml:661 и :803 строят из него
путь docs/reviews/${marker}-${NUM}-r${CYCLE}.md. Поэтому недосчёт означает
коллизию имени.
Факты по #449:
| Время | Вердикт | Маркер в теле |
|---|---|---|
| 14:46 | spec, жёлтый, «заход r1» | нет |
| 14:56 | spec, жёлтый, «заход r2» | да |
| 15:17 | spec, зелёный, «заход r3» | да |
| 16:01 | code, красный, «заход r1» | да, но случайно |
docs/reviews/SPEC-REVIEW-449-r1.md имеет две ревизии: 1ce62613 — 223 строки,
заголовок «заход r1»; 070c276e — 226 строк, заголовок «заход r2». Файл
…-r2.md (b39f99b3) содержит третий раунд.
Красный вердикт код-этапа от 16:01 маркер содержит — но не в строке «Документ:»,
а внутри прозаической фразы «путь соберёт шаг публикации, CODE-REVIEW-449-r1».
Проверено исполнением тем же выражением, что и в process.yml: сегодня guard
видит на #449 два спек-вердикта из трёх (attempt=3, верно случайно) и один
код-вердикт из одного (attempt=2, верно). То есть счёт спасён тем, что
ревьюер два раза из четырёх упомянул имя файла в свободном тексте. Это и есть
предмет задачи: корректность зависит от прозы, а не от факта.
Недосчёт был осознанным решением (process.yml:88-93: «Недосчёт даёт лишний
заход, перерасчёт остановил бы работу досрочно: из двух ошибок выбрана
обратимая»). Критерий выбран верно, но цена оценена неверно: недосчёт
уничтожает артефакт, то есть попадает в тот же класс, против которого в этом же
шаге стоят три отдельные защиты (#220, #365).
Скоуп
- Вынести подсчёт
attempt/spentиз inline-shell в чистые функцииscripts/review-doc-guard.mjs— модуля, у которого уже есть тесты и который уже вызывается конвейером. - Считать по артефактам: список опубликованных
docs/reviews/${marker}-${NUM}-r*.mdна ветке задачи. - Считать
spentпо строке вердикта внутри этих документов, а не по телу комментария. - Сохранить страховку от отказа публикации: итог — максимум из счёта по файлам и прежнего счёта по комментариям.
- Прогнать оба счётчика в
guardчерез новый скрипт. - Синхронизировать
process.ymlвmainиdev.
Не-скоуп
- Правка задним числом опубликованных комментариев #449 (пункт 3 issue) — решение владельца, не код.
- Изменение лимитов §4, правил перехода меток, формата вердикта.
- Требование к ревьюеру называть файл (вариант 2 из issue) как обязательный гейт: обсуждается в «Принятых предположениях», но в реализацию не берётся — оно возвращает зависимость от прозы, ради ухода от которой задача и заведена.
- Восстановление затёртого документа
SPEC-REVIEW-449-r1.md: отдельная ручная операция владельца, к механизму не относится. Отсюда следствие, важное для чтения AC2: жёлтый вердикт первого спек-раунда #449 утрачен безвозвратно — его файл перезаписан тем самым дефектом, который чинится, а его комментарий маркера не содержит. Ни счёт по файлам, ни страховка по комментариям его не воскрешают, поэтому на сегодняшних данныхspent=1, и это правильный ответ, а не недосчёт. Исправление возвращает верным будущий счёт, а не прошлую историю.
Контракт поведения
1. Источник истины — опубликованные документы
Для этапа stage и issue NUM множество раундов определяется именами файлов
на ветке задачи, отвечающих ^docs/reviews/(SPEC|CODE)-REVIEW-<NUM>-r(\d+)\.md$
для соответствующего маркера.
roundsPublished= число таких файлов;attemptFromFiles=max(r) + 1, а неcount + 1: пропуск в нумерации (например, из-за прошлых коллизий) не имеет права выдать уже занятое имя.
2. Блокирующие циклы
Для каждого найденного документа читается его строка вердикта — первая строка,
отвечающая Вердикт: — и цикл считается израсходованным, если в ней есть
жёлт или красн (регистр не важен), тем же правилом, что и сейчас.
3. Страховка от отказа публикации
Шаг публикации может упасть уже после того, как вердикт опубликован — такой случай в проекте уже был. Тогда документа нет, а цикл израсходован. Поэтому:
attempt = max(attemptFromFiles, attemptFromComments)
spent = max(spentFromFiles, spentFromComments)
Недосчёт возможен только при отказе обоих источников. Удвоение невозможно по построению — берётся максимум, а не сумма, — но это НЕ значит, что завышение невозможно вовсе: максимум наследует ошибку той компоненты, которая завысила. Поэтому у каждого источника своё правило точности.
Счёт по комментариям в первой редакции ТЗ предполагался неизменным. Ревью
реализации показало, что оставлять его нельзя: правило «в теле есть подстрока
Вердикт: и подстрока маркера» протекает на прозе. Поймано на самой #454 —
разбор чужих задач в комментарии содержал CODE-REVIEW, и первый же код-ревью
получил заход r3 вместо r1. Поэтому комментарий засчитывается вердиктом этапа,
только если он (а) объявляет вердикт той же строгой строкой, что и документ, и
(б) называет документ ЭТОЙ задачи и ЭТОГО этапа — <MARKER>-<NUM>. Голая
подстрока маркера больше не годится: именно она и протекала.
4. Ветка задачи
guard сегодня ветку не знает. Имя ищется тем же шаблоном issue/<NUM>-* и с
тем же правилом «свежая по дате коммита», что и шаг ревью (process.yml:216-221),
через git ls-remote/gh api — без checkout, guard остаётся лёгким.
Ветки нет → счёт по файлам даёт ноль, работает страховка §3, поведение совпадает с сегодняшним.
5. Что не меняется
- Формат и текст вердикта, лимиты 4/2, метка
review-4, все отказыrefuse(). - Разделение ролей
attemptиspent(#227): зелёный вердикт цикла не тратит. - Правило «вердикты только своего этапа» (#89): по файлам оно выполняется строго, потому что имя файла содержит маркер этапа.
Ошибки и крайние случаи
| Случай | Поведение |
|---|---|
| Ветки задачи нет | Счёт по файлам = 0, работает счёт по комментариям |
В docs/reviews/ нет ни одного файла этапа |
attempt = 1, spent = 0 |
Имя файла с нечисловым суффиксом (-rX.md) |
Игнорируется, в лог ::warning:: |
Дыра в нумерации (r1, r3) |
attempt = 4 — занятые имена не переиспользуются |
Документ есть, строки Вердикт: в нём нет |
Цикл не засчитан по файлу; страховка §3 добирает по комментарию |
gh api/ls-remote недоступны |
Счёт по файлам = 0 плюс ::warning::; конвейер не падает |
Acceptance criteria и доказательства
| AC | Критерий | Доказательство |
|---|---|---|
| AC1 | attempt берётся от максимального номера опубликованного файла этапа, а не от их количества |
unit |
| AC2 | Вердикт без маркера в теле больше не занижает attempt: фикстура «#449 как есть» (файлы …-r1.md с телом второго раунда, жёлтый, и …-r2.md с телом третьего, зелёный; три комментария, у первого маркера нет) даёт attempt=3, spent=1 |
unit на фикстуре, снятой с реальных файлов и комментариев #449 |
| AC2b | На той же истории, прожитой уже с исправлением, ничего не теряется: фикстура трёх файлов r1 (жёлтый), r2 (жёлтый), r3 (зелёный) даёт attempt=4, spent=2 |
unit на реконструированной фикстуре |
| AC3 | Имя документа никогда не повторяет уже существующее на ветке | unit + мутант |
| AC4 | Зелёный вердикт цикла не тратит (#227 не сломан) | unit |
| AC5 | Вердикт чужого этапа не влияет на счёт (#89 не сломан) — ни по имени файла, ни по прозе комментария | unit + мутант |
| AC6 | Отказ публикации (документа нет при существующем вердикте) не занижает счёт — работает максимум | unit |
| AC7 | Отсутствие ветки/недоступность API не роняет guard и не меняет сегодняшнего поведения |
unit |
| AC8 | process.yml идентичен в main и dev |
существующий шаг Validate |
| AC9 | Логика живёт в тестируемом модуле, а не в inline-shell | ревью + факт наличия тестов |
План тестирования
test/review-doc-guard.test.mjs— новые случаи на каждую строку таблицы крайних случаев и на AC1–AC7. Фикстур две, и они разные: «#449 как есть» — буквальный слепок сегодняшнего состояния ветки (два файла, три комментария), ожиданиеattempt=3,spent=1; «#449, прожитая с исправлением» — реконструкция из трёх файлов, ожиданиеattempt=4,spent=2.- Мутанты (правило «мутант на каждый защитный контракт»):
review-round-counts-files-not-max—attemptсчитает количество файлов вместо максимума; краснеет AC1/AC3;review-round-drops-file-source— счёт по файлам выключен, остаётся только проза; краснеет AC2;review-round-drops-comment-insurance— вместо максимума берётся счёт только по файлам; краснеет AC6. (В первой редакции ТЗ мутант называлсяreview-round-takes-comments-over-max; в таком виде он был неотличим от предыдущего — обе мутации дают результат, равный счёту по комментариям, — и AC6 не краснил, потому что там комментарии как раз больше файлов. Ломать страховку нужно противоположной мутацией.)review-comment-source-ignores-issue-number— резервный матч по голому маркеру вместо<MARKER>-<NUM>; краснеет AC5.
- Ручная проверка на самом себе: эта задача проходит spec-review и code-review штатным конвейером; после её мержа номера документов #454 обязаны идти подряд.
Карта реализации
scripts/review-doc-guard.mjs: чистыеreviewRoundsFromFiles(names, marker, num),blockingFromDocs(docs),reviewCounters({files, docs, comments})+ CLI-режим, печатающийattempt/spentв форматеGITHUB_OUTPUT.test/review-doc-guard.test.mjs: случаи AC1–AC7..github/workflows/process.yml: шагdecideзовёт скрипт вместо inline-jq; добавляется резолв ветки и чтение списка файлов черезgh api.scripts/mutation-gate.mjs: три мутанта.- Синхронизация
process.ymlвmain.
Риски и rollback
- Риск: конвейер ломается для всех задач. Мера: логика чистая и покрыта юнитами; shell-часть сводится к одному вызову; при любой ошибке скрипта guard обязан продолжить со счётом по комментариям (сегодняшнее поведение), а не упасть.
- Риск: перерасчёт остановит работу досрочно — то, чего боялся автор исходного решения. Мера: максимум из двух счётов не может дать больше, чем фактическое число опубликованных документов и вердиктов.
- Rollback: возврат одного коммита в
process.yml(и его зеркала вmain); скрипт и тесты остаются безвредными.
Release-артефакты
Класс B: docs/CHANGELOG.md и .ru.md не пополняются (пользователь изменения
не видит). Обновляется docs/PROCESS.md — раздел про счёт циклов, если он
описывает нынешний механизм подсчёта по комментариям.
Терминальные трейлеры:
Issue: #454
User-Visible: no
Принятые предположения
- Имя файла документа ревью — контракт, а не деталь: на него уже опирается
review-doc-guardи якоря #414. - Вердикт пишет модель, поэтому его проза не может быть источником машинных величин; требование называть файл (вариант 2 issue) оставлено как возможная дополнительная диагностика, но не как основа счёта.
- Максимум из двух источников предпочтён сумме и приоритету одного из них: сумма дала бы двойной счёт, приоритет файлов — недосчёт при отказе публикации.