mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
docs: specify review round counting from published artifacts
User-Visible: no Issue: #454
This commit is contained in:
@@ -0,0 +1,234 @@
|
||||
# ТЗ #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` считает обе величины по телу комментария:
|
||||
|
||||
```sh
|
||||
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
|
||||
маркера не содержит, поэтому дефект повторится на первом же возврате #449 в
|
||||
`S7-code-review`.
|
||||
|
||||
Недосчёт был осознанным решением (`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`: отдельная ручная
|
||||
операция владельца, к механизму не относится.
|
||||
|
||||
## Контракт поведения
|
||||
|
||||
### 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)
|
||||
```
|
||||
|
||||
Счёт по комментариям остаётся ровно тем же, что сегодня. Недосчёт возможен
|
||||
только при отказе обоих источников; перерасчёт невозможен по построению, потому
|
||||
что берётся максимум, а не сумма.
|
||||
|
||||
### 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`, ни `spent`: сценарий #449 (r1 без маркера, r2 с маркером) даёт `attempt=3`, `spent=2` | unit на фикстуре реальных данных #449 |
|
||||
| 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» строится из реальных
|
||||
заголовков документов и строк вердиктов.
|
||||
- Мутанты (правило «мутант на каждый защитный контракт»):
|
||||
- `review-round-counts-files-not-max` — `attempt` считает количество файлов
|
||||
вместо максимума; краснеет AC1/AC3;
|
||||
- `review-round-drops-file-source` — счёт по файлам выключен, остаётся
|
||||
только проза; краснеет AC2;
|
||||
- `review-round-takes-comments-over-max` — вместо максимума берётся счёт по
|
||||
комментариям; краснеет AC6.
|
||||
- Ручная проверка на самом себе: эта задача проходит spec-review и code-review
|
||||
штатным конвейером; после её мержа номера документов #454 обязаны идти
|
||||
подряд.
|
||||
|
||||
## Карта реализации
|
||||
|
||||
1. `scripts/review-doc-guard.mjs`: чистые `reviewRoundsFromFiles(names, marker, num)`,
|
||||
`blockingFromDocs(docs)`, `reviewCounters({files, docs, comments})` + CLI-режим,
|
||||
печатающий `attempt`/`spent` в формате `GITHUB_OUTPUT`.
|
||||
2. `test/review-doc-guard.test.mjs`: случаи AC1–AC7.
|
||||
3. `.github/workflows/process.yml`: шаг `decide` зовёт скрипт вместо inline-jq;
|
||||
добавляется резолв ветки и чтение списка файлов через `gh api`.
|
||||
4. `scripts/mutation-gate.mjs`: три мутанта.
|
||||
5. Синхронизация `process.yml` в `main`.
|
||||
|
||||
## Риски и rollback
|
||||
|
||||
- **Риск: конвейер ломается для всех задач.** Мера: логика чистая и покрыта
|
||||
юнитами; shell-часть сводится к одному вызову; при любой ошибке скрипта
|
||||
guard обязан продолжить со счётом по комментариям (сегодняшнее поведение),
|
||||
а не упасть.
|
||||
- **Риск: перерасчёт остановит работу досрочно** — то, чего боялся автор
|
||||
исходного решения. Мера: максимум из двух счётов не может дать больше, чем
|
||||
фактическое число опубликованных документов и вердиктов.
|
||||
- Rollback: возврат одного коммита в `process.yml` (и его зеркала в `main`);
|
||||
скрипт и тесты остаются безвредными.
|
||||
|
||||
## Release-артефакты
|
||||
|
||||
Класс B: `docs/CHANGELOG.md` и `.ru.md` не пополняются (пользователь изменения
|
||||
не видит). Обновляется `docs/PROCESS.md` — раздел про счёт циклов, если он
|
||||
описывает нынешний механизм подсчёта по комментариям.
|
||||
|
||||
Терминальные трейлеры:
|
||||
|
||||
```text
|
||||
Issue: #454
|
||||
User-Visible: no
|
||||
```
|
||||
|
||||
## Принятые предположения
|
||||
|
||||
- Имя файла документа ревью — контракт, а не деталь: на него уже опирается
|
||||
`review-doc-guard` и якоря #414.
|
||||
- Вердикт пишет модель, поэтому его проза не может быть источником машинных
|
||||
величин; требование называть файл (вариант 2 issue) оставлено как возможная
|
||||
дополнительная диагностика, но не как основа счёта.
|
||||
- Максимум из двух источников предпочтён сумме и приоритету одного из них:
|
||||
сумма дала бы двойной счёт, приоритет файлов — недосчёт при отказе публикации.
|
||||
@@ -171,6 +171,7 @@ GitHub Issues и GitHub Projects (v2) остаются единственным
|
||||
| [#426](https://github.com/Matysh/houseplan-card/issues/426) Отключение информационного окна комнаты при наведении | [426-room-hover-tooltip-toggle.md](426-room-hover-tooltip-toggle.md) |
|
||||
| [#431](https://github.com/Matysh/houseplan-card/issues/431) Канонизация координат пользовательских изображений | [431-image-coordinate-canonicalization.md](431-image-coordinate-canonicalization.md) |
|
||||
| [#432](https://github.com/Matysh/houseplan-card/issues/432) Ограниченный resolve и единая проверка целостности изображений | [432-asset-resolve-authorization-cache.md](432-asset-resolve-authorization-cache.md) |
|
||||
| [#454](https://github.com/Matysh/houseplan-card/issues/454) Счёт заходов и циклов ревью по артефактам | [454-review-round-counter.md](454-review-round-counter.md) |
|
||||
| [#440](https://github.com/Matysh/houseplan-card/issues/440) Полиш аудита v1.71.0-beta.2 | [440-v171-beta2-polish.md](440-v171-beta2-polish.md) |
|
||||
| [#445](https://github.com/Matysh/houseplan-card/issues/445) Магнит мебели к физической поверхности стены | [445-furniture-wall-face-snap.md](445-furniture-wall-face-snap.md) |
|
||||
| [#447](https://github.com/Matysh/houseplan-card/issues/447) Наружная грань для мебели и сдвиг декора стрелками | [447-exterior-furniture-snap-keyboard-nudge.md](447-exterior-furniture-snap-keyboard-nudge.md) |
|
||||
|
||||
Reference in New Issue
Block a user