mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,163 @@
|
||||
# CODE-REVIEW-621-r1
|
||||
|
||||
**Issue:** [#621](https://github.com/Matysh/houseplan-card/issues/621) — `process.yml`: счёт раундов ревью читает `docs/reviews` через `contents` API с потолком 1 000 записей.
|
||||
**Трек:** инфраструктурный (#562) — вход сразу на `S7-code-review`, статусов `S1…S6` не было.
|
||||
**Заход:** r1 · блокирующих циклов израсходовано **0 из 4**.
|
||||
**Материал:** `50e67c0988d91866b06c9967fae8c2f54f88efe9` (`issue/621-reviews-listing`, база `origin/dev`). Дерево коммита `1f842cfe8bab1f5716bdb3f32d11533f22abaa84` совпадает с деревом, указанным автором в блоке «Материал раунда» — сверено `git ls-tree HEAD` и `git rev-parse HEAD^{tree}`.
|
||||
|
||||
## Скоуп
|
||||
|
||||
Изменены только файлы класса B: `.github/workflows/process.yml` (job `guard`, шаг
|
||||
`decide`) и `test/review-doc-guard.test.mjs`. Класса A нет — задача корректно
|
||||
классифицирована как инфраструктурная (#562), продуктовый код и SCOPE.md не
|
||||
затронуты: это правка внутреннего процесса ревью, а не пользовательского
|
||||
поведения.
|
||||
|
||||
Суть правки: листинг каталога `docs/reviews`, по которому считаются заходы и
|
||||
циклы ревью, переведён с REST `contents` API (молчаливый потолок 1 000 записей)
|
||||
на спуск по Git Trees API (`commits/<ветка>` → `.commit.tree.sha` → `docs` →
|
||||
`reviews` → список блобов), где `truncated:true` — явный отказ листинга, а не
|
||||
частичный список. Предупреждение о потолке 1000 снято вместе с зависимостью.
|
||||
На 23.09 в `docs/reviews` 990 файлов — риск был реальным и близким (задача сама
|
||||
описывает срок «примерно через неделю»).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
| Гейт | Команда | Результат |
|
||||
|---|---|---|
|
||||
| Дешёвые гейты (typecheck/test/build/bundle) | Validate на `50e67c09` | success, см. https://github.com/Matysh/houseplan-card/actions/runs/35844333827 — не перегонялись повторно (см. системную инструкцию раунда) |
|
||||
| Целевой unit-тест | `node --test test/review-doc-guard.test.mjs` | 63 pass / 0 fail (совпадает с хендоффом) |
|
||||
| YAML-валидность | `python3 -c "yaml.safe_load(...)"` | OK |
|
||||
| Синтаксис изменённого shell-шага | извлёк `jobs.guard.steps[id=decide].run` через PyYAML, прогнал `bash -n` | OK, без ошибок |
|
||||
| Семантика `truncated` в jq | `echo '{"truncated":true,...}' | jq 'if .truncated then error(...) ...'` | подтверждено: `jq` возвращает ненулевой код (5) — `gh api --jq` в этом случае тоже завершится с ошибкой, `tree_names` вернёт 1, листинг корректно деградирует к «счёт отключён», как и раньше при отказе `contents` |
|
||||
| Мутация (снятие защиты) | вручную заменил `--jq 'if .truncated then error("truncated") else ... end'` на `--jq '.tree[] | select(...) | .path'` (без обработки `truncated`) и перезапустил тест | AC2-тест **краснеет** (62 pass / 1 fail) — тест умеет падать; изменение отменено, дерево восстановлено `diff` (пусто) |
|
||||
| Провенанс коммита | `node scripts/validate-commit-provenance.mjs` | чисто, трейлеры `Issue: #621` / `User-Visible: no` на месте |
|
||||
| Ветка/классы файлов | `git diff origin/dev...HEAD --stat` | только `.github/workflows/**` и `test/**` — класс B, `User-Visible: no` корректен, changelog не требуется |
|
||||
|
||||
**Не прогонялось и почему:**
|
||||
- `npx tsc --noEmit`, `npm test` (полный), `npm run build` + сверка бандлов — не
|
||||
повторял: Validate зелёный на этом же SHA (см. таблицу), diff не касается
|
||||
`src/**`, `dist/**` или сборки.
|
||||
- `node scripts/check-docs.mjs` — не требуется: diff не трогает `src/**`.
|
||||
- `node scripts/model-invariants.mjs` — не требуется: геометрия, `layout`,
|
||||
`marker.space`, толщина стен не затронуты.
|
||||
- browser-смоки (`demo/smoke_*.mjs`) — не требуется: diff не трогает
|
||||
`src/**`/`demo/**`-рендер, `smoke-select.mjs` неприменим (нет фронтенд-диффа).
|
||||
- `golden:verify` — не требуется: нет визуальных изменений.
|
||||
- `python -m pytest tests_backend` — не требуется: Python не тронут.
|
||||
- Живой прогон нового `tree_names` против настоящего `gh api git/trees` с
|
||||
токеном `HP_PROCESS_TOKEN` — не воспроизводим локально (нет сетевого доступа
|
||||
к API из песочницы ревьюера); автор честно указал это в хендоффе. Первый
|
||||
реальный прогон — этот же guard-job на данном issue при простановке
|
||||
`S7-code-review`: если листинг откажет, в логе появится
|
||||
`::warning::дерево docs/reviews … не получено`, и счёт по файлам отключится
|
||||
(та же деградация, что и раньше при отказе `contents`), не блокируя ревью
|
||||
полностью — есть страховка по комментариям.
|
||||
|
||||
## Разбор по AC
|
||||
|
||||
- **AC1** («Счёт раундов не зависит от числа файлов в каталоге»). Доказано
|
||||
двумя слоями: (а) JS-агрегатор (`reviewRoundsFromFiles`/`attemptFromRounds`)
|
||||
уже не имел ограничения по размеру массива — новый тест на фикстуре 2 400
|
||||
имён это подтверждает, но сам по себе не был местом бага; (б) реальный
|
||||
источник обрезки — REST `contents` API — заменён на Git Trees API с потолком
|
||||
100 000 записей на уровень и явным флагом `truncated`, проверено чтением
|
||||
кода (`tree_names`) плюс локальным воспроизведением семантики `jq
|
||||
error()` (ненулевой код завершения) и мутацией, снимающей эту защиту
|
||||
(тест AC2 краснеет). При текущем росте ~60 файлов/неделю до 100 000 —
|
||||
десятилетия, разумный запас. **Доказано.**
|
||||
- **AC2** («Предупреждение о потолке удалено из `process.yml` вместе с
|
||||
зависимостью от `contents`»). Доказано тестом `guard перечисляет
|
||||
docs/reviews деревом, а не contents, и без предупреждения о потолке` —
|
||||
regex-свидетель на самом тексте job `guard`, подтверждено мутацией (см.
|
||||
таблицу выше: снятие `if .truncated then error(...)` из текста делает тест
|
||||
красным). **Доказано.**
|
||||
|
||||
## Находки
|
||||
|
||||
Блокирующих (High) и находок в скоупе (Medium) нет.
|
||||
|
||||
- **Low, не блокирует, снимаю с записью.** Логика `tree_names` — инлайновый
|
||||
bash внутри `run:` шага workflow, а не отдельный скрипт; проверяется только
|
||||
регулярными выражениями по тексту YAML и моей ручной мутацией, а не прямым
|
||||
исполнением с замоканным `gh api`. Комментарий в самом файле рядом (про
|
||||
вынос счёта раундов в `scripts/review-doc-guard.mjs` «потому что inline-shell
|
||||
не покрывается тестами», #454) — тот же класс риска, что здесь остался
|
||||
непокрытым. Не блокирую: (1) автор прозрачно указал это в разделе «Чего не
|
||||
проверял» хендоффа; (2) я вручную проверил семантику `truncated`/`error()` и
|
||||
подтвердил мутацией, что регресс кода будет пойман тестом; (3) первый живой
|
||||
прогон происходит на этом же issue при следующем событии `S7-code-review`, и
|
||||
деградация при отказе безопасна (откат к счёту по комментариям, как и
|
||||
раньше). Предложение на будущее, не для этой задачи: если `docs`/`reviews`
|
||||
когда-нибудь переименуют или Git Trees API изменит форму ответа, стоит
|
||||
вынести `tree_names` в тестируемый скрипт по образцу
|
||||
`scripts/review-doc-guard.mjs`.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Классификация задачи как инфраструктурной (#562) верна: ни одного файла
|
||||
класса A, вход сразу на `S7-code-review` правомерен.
|
||||
- Трейлеры коммита (`Issue: #621`, `User-Visible: no`) верны для этого диффа:
|
||||
правка не меняет ничего, наблюдаемого пользователем House Plan; changelog не
|
||||
требуется.
|
||||
- Единственный коммит, ветка `issue/621-reviews-listing`, дерево совпадает с
|
||||
заявленным в «Материале раунда» — блок не расходится с фактическим HEAD.
|
||||
- Замена API технически корректна: спуск `commit.tree.sha → docs → reviews`
|
||||
адресует именно плоский список файлов в `docs/reviews/` (без рекурсии,
|
||||
подкаталогов там нет), `truncated:true` трактуется как отказ листинга (я
|
||||
воспроизвёл, что `jq error()` даёт ненулевой код и обрывает `tree_names`),
|
||||
а деградация при отказе (пустой список → `spent`/`attempt` по умолчанию,
|
||||
предупреждение в лог) сохраняет прежнее поведение отказа `contents`.
|
||||
- YAML синтаксически валиден, изменённый `run`-блок проходит `bash -n` —
|
||||
собственное требование файла (шапка `process.yml`, п.3) выполнено.
|
||||
- Тесты добавлены по обоим AC, оба **умеют падать** — подтверждено моей
|
||||
ручной мутацией (снятие `truncated`-обработки на реальном тексте файла),
|
||||
тест 63 краснеет, остальные 62 остаются зелёными.
|
||||
- Других мест в `process.yml`, читающих `docs/reviews` через `contents` с тем
|
||||
же риском, не найдено (`grep -n "docs/reviews"` — единственное другое
|
||||
использование, строка ~737, идёт через локальный `git ls-tree` на checkout
|
||||
с историей, лимита API не имеет и вне скоупа этой задачи).
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Живой прогон нового листинга против настоящего GitHub API (см. таблицу
|
||||
гейтов) — нет сетевого доступа из песочницы ревьюера; первый реальный прогон
|
||||
состоится на этом же issue.
|
||||
- `actionlint` недоступен в окружении ревьюера; заменено связкой
|
||||
`yaml.safe_load` + `bash -n` на изменённый шаг, что покрывает оба класса
|
||||
ошибок, о которых предупреждает шапка файла (некорректный YAML и heredoc,
|
||||
обрезающий скрипт).
|
||||
- Полные тяжёлые гейты (`golden`, browser-смоки, backend pytest,
|
||||
perf-профили) — не запускал: diff их не касается, AC их не требует.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Зелёный. AC1 и AC2 доказаны исполняемыми тестами, оба умеют падать (проверено
|
||||
мутацией), код-риск (некорректная трактовка `truncated`) проверен чтением и
|
||||
воспроизведением семантики `jq`/`gh api`. Единственное отступление — Low,
|
||||
снятое с запиской выше, не требует правки в этой задаче.
|
||||
|
||||
---
|
||||
|
||||
## Материал раунда
|
||||
|
||||
```
|
||||
tree 1f842cfe8bab1f5716bdb3f32d11533f22abaa84
|
||||
blob 3eb81d2a24753cd885ac6f7a0104e24fe36ebc1f .github/workflows/process.yml
|
||||
blob ec7517895d5e90f53d4bf39db133f735145ddcf6 test/review-doc-guard.test.mjs
|
||||
SHA: 50e67c0988d91866b06c9967fae8c2f54f88efe9
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/621-reviews-listing`, коммит `50e67c0988d9` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `1f842cfe8bab1f5716bdb3f32d11533f22abaa84`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 1f842cfe8bab
|
||||
```
|
||||
- Тело issue: `4d9b8e6b2263ab99eaeeedb33ad1d9010a5bdfbf079ab627c01949ac9f20ac7f`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user