mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
perf: make review scope and ceremony fit the size of the task
The owner's report: the process works but every stage takes a long time even on simple bugs. Two causes, and neither was the one that first comes to mind. The reviewer ran everything regardless. On #89 it installed Chromium, ran all 127 smoke files and a full golden capture — right for a task rated 10/10 for complexity, absurd for a bug about a room divider. Full suites are the pre-beta gate; the review now runs typecheck, unit and build always, and smokes, golden, pytest or performance only where the diff and the AC call for them. The price of narrowing it is honesty: the reviewer must list which gates it ran, which it did not, and why, so a skipped gate is a visible decision rather than a silent one. The reviewer also built its own environment out of model turns, with no npm cache and no browser cache, paid for from the same forty-five minutes. The workflow now installs dependencies and Chromium as ordinary cached steps, after switching to the task branch so the lockfile is the branch's own. Second, ceremony did not scale down. The light track makes a spec cheap; the new trivial track does without one — S2-analysis straight to S5-ready, no spec review, AC in the issue body. It is deliberately hard to qualify for: a bug on one surface, no new UX contract, no migration, no i18n, no perf or touch effect, three checkable AC at most, and expected behaviour already on record. Nothing left to decide is the criterion that holds the whole thing up, and it cannot be met by feeling sure. Code review is never skipped on either track. It is what stands in for testing here, so it is the one stage speed may not buy. Issue: #127 Issue: #128 User-Visible: no
This commit is contained in:
@@ -48,6 +48,7 @@ jobs:
|
|||||||
BLOCKED: ${{ contains(github.event.issue.labels.*.name, 'blocked') }}
|
BLOCKED: ${{ contains(github.event.issue.labels.*.name, 'blocked') }}
|
||||||
EXHAUSTED: ${{ contains(github.event.issue.labels.*.name, 'review-4') }}
|
EXHAUSTED: ${{ contains(github.event.issue.labels.*.name, 'review-4') }}
|
||||||
SMALL: ${{ contains(github.event.issue.labels.*.name, 'small') }}
|
SMALL: ${{ contains(github.event.issue.labels.*.name, 'small') }}
|
||||||
|
TRIVIAL: ${{ contains(github.event.issue.labels.*.name, 'trivial') }}
|
||||||
NUM: ${{ github.event.issue.number }}
|
NUM: ${{ github.event.issue.number }}
|
||||||
run: |
|
run: |
|
||||||
# Этап определяется первым: от него зависит, какие вердикты считать.
|
# Этап определяется первым: от него зависит, какие вердикты считать.
|
||||||
@@ -58,8 +59,9 @@ jobs:
|
|||||||
*) echo "метка $LABEL конвейер не запускает" ;;
|
*) echo "метка $LABEL конвейер не запускает" ;;
|
||||||
esac
|
esac
|
||||||
|
|
||||||
# Лимит циклов: 4 обычный, 2 на лёгком треке (PROCESS.md §4).
|
# Лимит циклов: 4 обычный, 2 на лёгком и коротком треке (PROCESS.md §4).
|
||||||
limit=4; [ "$SMALL" = "true" ] && limit=2
|
limit=4
|
||||||
|
if [ "$SMALL" = "true" ] || [ "$TRIVIAL" = "true" ]; then limit=2; fi
|
||||||
|
|
||||||
# Счётчик считает вердикты ТОЛЬКО своего этапа. Раньше он брал все
|
# Счётчик считает вердикты ТОЛЬКО своего этапа. Раньше он брал все
|
||||||
# подряд, и вердикт по ТЗ съедал цикл из бюджета код-ревью: на #89
|
# подряд, и вердикт по ТЗ съедал цикл из бюджета код-ревью: на #89
|
||||||
@@ -134,8 +136,15 @@ jobs:
|
|||||||
fetch-depth: 0
|
fetch-depth: 0
|
||||||
ref: dev
|
ref: dev
|
||||||
|
|
||||||
|
# Окружение готовит workflow, а не модель своими ходами. Раньше промпт
|
||||||
|
# велел ревьюеру самому выполнить `npm ci`: минуты уходили на установку без
|
||||||
|
# кэша, платились из бюджета 45 минут и из лимитов подписки, а ходы модели
|
||||||
|
# тратились на работу инфраструктуры. В validate.yml кэш стоит на всех
|
||||||
|
# тяжёлых job, здесь его не было.
|
||||||
- uses: actions/setup-node@v4
|
- uses: actions/setup-node@v4
|
||||||
with: { node-version: 22 }
|
with:
|
||||||
|
node-version: 22
|
||||||
|
cache: npm
|
||||||
|
|
||||||
# Материал ревью живёт в ветке задачи: ТЗ в docs/specs/ и код коммитятся
|
# Материал ревью живёт в ветке задачи: ТЗ в docs/specs/ и код коммитятся
|
||||||
# в issue/<NN>-slug. Если ветка запушена — переключаемся на неё, иначе
|
# в issue/<NN>-slug. Если ветка запушена — переключаемся на неё, иначе
|
||||||
@@ -156,6 +165,24 @@ jobs:
|
|||||||
echo "МАТЕРИАЛ НЕ ЗАПУШЕН" >> "$GITHUB_STEP_SUMMARY"
|
echo "МАТЕРИАЛ НЕ ЗАПУШЕН" >> "$GITHUB_STEP_SUMMARY"
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
# Зависимости ставятся ПОСЛЕ переключения на ветку задачи: lockfile мог
|
||||||
|
# измениться именно в ней, и установка по копии из dev дала бы не то дерево.
|
||||||
|
- name: Установить зависимости
|
||||||
|
run: npm ci
|
||||||
|
|
||||||
|
# Браузер нужен не всякому ревью (см. правило выбора гейтов в промпте),
|
||||||
|
# но когда нужен — качать его заново дороже, чем держать в кэше.
|
||||||
|
- name: Кэш браузеров Playwright
|
||||||
|
id: pw
|
||||||
|
uses: actions/cache@v4
|
||||||
|
with:
|
||||||
|
path: ~/.cache/ms-playwright
|
||||||
|
key: playwright-${{ runner.os }}-${{ hashFiles('package-lock.json') }}
|
||||||
|
|
||||||
|
- name: Установить Chromium
|
||||||
|
if: steps.pw.outputs.cache-hit != 'true'
|
||||||
|
run: npx playwright install --with-deps chromium
|
||||||
|
|
||||||
- name: Review
|
- name: Review
|
||||||
id: review
|
id: review
|
||||||
uses: anthropics/claude-code-action@v1
|
uses: anthropics/claude-code-action@v1
|
||||||
@@ -205,10 +232,38 @@ jobs:
|
|||||||
По каждому AC: либо он доказан автотестом и ты убедился, что тест
|
По каждому AC: либо он доказан автотестом и ты убедился, что тест
|
||||||
умеет падать, либо разобран по коду с явной записью «проверено
|
умеет падать, либо разобран по коду с явной записью «проверено
|
||||||
чтением, не исполнением». «Verified» без названной команды и её
|
чтением, не исполнением». «Verified» без названной команды и её
|
||||||
результата доказательством не является. Зависимостей в рабочей
|
результата доказательством не является. Зависимости уже установлены
|
||||||
копии нет: перед гейтами выполни `npm ci`. Проверь трейлеры Issue и
|
workflow, Chromium тоже — `npm ci` выполнять не нужно. Проверь
|
||||||
User-Visible, при User-Visible: yes — правки в оба changelog в том же
|
трейлеры Issue и User-Visible, при User-Visible: yes — правки в оба
|
||||||
коммите.
|
changelog в том же коммите.
|
||||||
|
|
||||||
|
**Объём гейтов соразмерен задаче.** Прогонять весь набор на каждой
|
||||||
|
правке — не тщательность, а потеря времени: полные наборы это
|
||||||
|
предрелизный гейт (PROCESS.md §8), а не гейт ревью.
|
||||||
|
|
||||||
|
Всегда, они дешёвые:
|
||||||
|
`npx tsc --noEmit`, `npm test`, `npm run build` со сверкой трёх
|
||||||
|
копий бандла.
|
||||||
|
|
||||||
|
По необходимости, и «необходимость» определяется diff'ом и AC:
|
||||||
|
- браузерные смоки `demo/smoke_*.mjs` — названные в AC плюс
|
||||||
|
относящиеся к тронутым поверхностям. Их 127; прогон всех уместен
|
||||||
|
только когда задача действительно задевает всё;
|
||||||
|
- `npm run golden:verify` — если diff может изменить видимый
|
||||||
|
результат: рендер, геометрия, стили, слои;
|
||||||
|
- `python -m pytest tests_backend -q` — если тронут
|
||||||
|
`custom_components/**/*.py`;
|
||||||
|
- performance-профили — если названы в AC либо тронуты
|
||||||
|
чувствительные к перфу пути.
|
||||||
|
|
||||||
|
Дисциплина «тест должен уметь падать» не отменяется, но применяется к
|
||||||
|
тем тестам, которые ты прогонял.
|
||||||
|
|
||||||
|
**В комментарии обязателен перечень: какие гейты прогнал, какие нет и
|
||||||
|
почему.** Это условие честности такого сужения: непрогнанный гейт
|
||||||
|
становится видимым решением, а не молчаливым пропуском. Раздел «чего
|
||||||
|
не проверял» в документе ревью — не формальность, а главный его
|
||||||
|
раздел на коротких задачах.
|
||||||
|
|
||||||
Ты НЕ правишь ни ТЗ, ни продуктовый код. Только оцениваешь.
|
Ты НЕ правишь ни ТЗ, ни продуктовый код. Только оцениваешь.
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user