mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,231 @@
|
|||||||
|
# CODE-REVIEW-632-r1
|
||||||
|
|
||||||
|
Issue: #632 · этап: код-ревью · заход r1 · блокирующих циклов израсходовано 0 из 4
|
||||||
|
Трек: инфраструктурный (ускоренный вход, AGENTS.md §1 / PROCESS.md §1) — весь
|
||||||
|
дифф класса B, ни одного файла класса A.
|
||||||
|
Материал: `git log --oneline origin/dev..HEAD` = один коммит `a775080a`
|
||||||
|
(«task-packet: track follows issue status and history, not a class-A-free
|
||||||
|
diff»), рабочая копия уже на нём (detached HEAD). `git diff origin/dev...HEAD`:
|
||||||
|
`scripts/task-packet.mjs`, `test/task-packet.test.mjs`,
|
||||||
|
`scripts/mutation-registry.mjs` — 3 файла, +122/-6.
|
||||||
|
|
||||||
|
## Скоуп
|
||||||
|
|
||||||
|
Баг: `scripts/task-packet.mjs` классифицировал ветку продуктовой S6-задачи как
|
||||||
|
«инфраструктурную» и печатал «файлы класса A трогать НЕЛЬЗЯ», если текущий
|
||||||
|
diff к `origin/dev` ещё не содержал ни одного файла класса A — реальный кейс
|
||||||
|
#607, чья ветка в момент воспроизведения несла только
|
||||||
|
`docs/reviews/SPEC-REVIEW-607-r1.md`. Ожидаемое поведение по телу issue:
|
||||||
|
«продуктовый S-flow и статус S5/S6/S7 должны сохранять право менять class A
|
||||||
|
до появления первого такого файла».
|
||||||
|
|
||||||
|
Правка вводит `branchIsInfrastructure()` (документы `docs/reviews/**` больше
|
||||||
|
не считаются материалом ветки) и `productFlowEvidence()` (статус S1–S5,
|
||||||
|
раздел `## ТЗ` в теле issue, файл `docs/specs/NN-*`, документ или вердикт
|
||||||
|
ревью ТЗ) — при любом из этих признаков эвристика «diff без класса A →
|
||||||
|
инфраструктура» не применяется.
|
||||||
|
|
||||||
|
## Как проверялось
|
||||||
|
|
||||||
|
Дешёвые гейты подтверждены зелёным Validate на этом же SHA `a775080a`
|
||||||
|
(https://github.com/Matysh/houseplan-card/actions/runs/35941025607) — по
|
||||||
|
инструкции раунда не перегонялись повторно: `npx tsc --noEmit`, `npm test`,
|
||||||
|
`npm run build` + сверка бандла. Diff не касается `src/**`, геометрии,
|
||||||
|
`open_spans`, `marker.space` → `check-docs.mjs` и `model-invariants.mjs` не
|
||||||
|
применимы. Смоки/golden/pytest/perf — diff их не касается (чистый
|
||||||
|
`scripts/**`+`test/**`, `src/**` не менялся).
|
||||||
|
|
||||||
|
Что прогнал сам, целенаправленно по AC и по защитным свойствам:
|
||||||
|
|
||||||
|
| Команда | Результат |
|
||||||
|
|---|---|
|
||||||
|
| `node --test test/task-packet.test.mjs` | 13/13 ok |
|
||||||
|
| `node scripts/mutation-gate.mjs --check` | `task-packet-product-flow-overrides-diff` ok, `task-packet-review-docs-not-material` ok, `task-packet-s6-s7-alone-not-product-flow` ok (анкоры валидны) |
|
||||||
|
| `node scripts/mutation-gate.mjs --id=task-packet-product-flow-overrides-diff` | поймано 1 из 1 |
|
||||||
|
| `node scripts/mutation-gate.mjs --id=task-packet-review-docs-not-material` | поймано 1 из 1 |
|
||||||
|
| `node scripts/mutation-gate.mjs --id=task-packet-s6-s7-alone-not-product-flow` | поймано 1 из 1 |
|
||||||
|
|
||||||
|
Три мутанта реально красят ровно заявленный тест и не красят посторонние —
|
||||||
|
защитные AC1–AC3 доказаны по правилу «чем краснеет» (§2.7), не только
|
||||||
|
заявлением автора.
|
||||||
|
|
||||||
|
Дополнительно (ручной разбор по коду, не мутацией — проверка гипотезы о
|
||||||
|
непокрытой ветке трека): вызвал `buildPacket()` напрямую с входами,
|
||||||
|
имитирующими реальный **`trivial`**-трек (§5.1) продуктовой задачи —
|
||||||
|
- синтетический кейс: `labels: ['bug','P2','S6-in-progress','trivial']`,
|
||||||
|
`branch.infrastructure: true`, тело issue с обычными `AC1./AC2.` без
|
||||||
|
заголовка `## ТЗ` (trivial-трек ТЗ не пишет вовсе, §5.1) → `track` остаётся
|
||||||
|
`'инфраструктурный'`, `rights` содержит «файлы класса A трогать НЕЛЬЗЯ»;
|
||||||
|
- то же самое воспроизвёл на **реальном** закрытом trivial-issue #612 (метки
|
||||||
|
`bug, P2, trivial`, тело — `## Дефект 1` / `## Дефект 2` / `## AC`, без
|
||||||
|
`## ТЗ`, без `docs/specs` файла, без `SPEC-REVIEW` документа/вердикта) —
|
||||||
|
`buildPacket` с тем же телом и `S6-in-progress` даёт тот же ложный
|
||||||
|
инфраструктурный запрет.
|
||||||
|
|
||||||
|
## Находки
|
||||||
|
|
||||||
|
### Medium (в скоупе задачи) — `trivial`-трек не входит в признаки продуктового потока
|
||||||
|
|
||||||
|
**Файл:** `scripts/task-packet.mjs:127-143` (`PRE_CODE_STATUSES`,
|
||||||
|
`productFlowEvidence`)
|
||||||
|
|
||||||
|
**Симптом:** `productFlowEvidence()` ищет ровно четыре признака: статус
|
||||||
|
S1–S5, заголовок `## ТЗ` в теле issue, файл `docs/specs/NN-*`, документ или
|
||||||
|
вердикт ревью ТЗ. Все четыре предполагают, что задача писала спецификацию.
|
||||||
|
Короткий трек (`trivial`, PROCESS.md §5.1) сознательно эту стадию пропускает
|
||||||
|
целиком: «Маршрут: `S1-new` → `S2-analysis` → `S5-ready` → `S6-in-progress`
|
||||||
|
→ …, минуя `S3-spec` и `S4-spec-review`» и «AC пишет автор в теле issue при
|
||||||
|
переводе в `S5-ready`» — без раздела `## ТЗ`, без файла в `docs/specs`, без
|
||||||
|
документа ревью ТЗ. Для такой задачи, как только она проходит `S5-ready` (то
|
||||||
|
есть как раз в `S6-in-progress`/`S7-code-review`, где и предполагается вызов
|
||||||
|
`task-packet`), `productFlowEvidence` возвращает пустой список тем же
|
||||||
|
образом, что и для настоящей ускоренной инфраструктурной задачи — их
|
||||||
|
неотличить. Итог: `branchIsInfrastructure(...)===true` (диффа пока нет
|
||||||
|
файлов класса A — обычная фаза «тесты/фикстуры раньше src») снова печатает
|
||||||
|
«файлы класса A трогать НЕЛЬЗЯ» для реальной продуктовой trivial-задачи —
|
||||||
|
это в точности симптом #607, воспроизведённый для целого документированного
|
||||||
|
трека, который в тестах задачи (`AC1…AC4` из хендоффа) не участвует ни разу:
|
||||||
|
`AC3` намеренно проверяет случай «инфраструктурная задача без ТЗ» теми же
|
||||||
|
метками `['infra', 'S6-in-progress']` / `['infra', 'S7-code-review']` —
|
||||||
|
без метки `trivial`, поэтому конфликт между «настоящая инфраструктурная
|
||||||
|
задача без ТЗ» и «настоящая trivial-задача без ТЗ» тестами не различается и
|
||||||
|
не покрыт.
|
||||||
|
|
||||||
|
**Воспроизведение (проверено чтением + прямым вызовом `buildPacket`, не
|
||||||
|
исполнением всего скрипта — `gh` в песочнице недоступен, как и у автора):**
|
||||||
|
|
||||||
|
```js
|
||||||
|
const packet = buildPacket({
|
||||||
|
issue: { number: 612, title: '…', state: 'CLOSED', url: 'u',
|
||||||
|
body: '## Дефект 1\n…\n## Дефект 2\n…\n## AC\n- AC1. …\n- AC2. …\n- AC3. …' },
|
||||||
|
labels: ['bug', 'P2', 'trivial', 'S6-in-progress'],
|
||||||
|
branch: { name: 'issue/612-x', tip: 'e'.repeat(40), base: 'f'.repeat(40),
|
||||||
|
ahead: 1, behind: 0, treeWithoutReviews: null, infrastructure: true },
|
||||||
|
});
|
||||||
|
// packet.track === 'инфраструктурный'
|
||||||
|
// packet.rights содержит 'файлы класса A трогать НЕЛЬЗЯ; …'
|
||||||
|
```
|
||||||
|
|
||||||
|
#612 — реальный, закрытый trivial-баг (не синтетика): его тело подтверждает,
|
||||||
|
что trivial-задачи не несут `## ТЗ`.
|
||||||
|
|
||||||
|
**Почему не High:** `task-packet.mjs` — советующий инструмент («пакет ничего
|
||||||
|
не пишет и ничего не решает», комментарий в шапке файла), а не гейт CI;
|
||||||
|
ошибочная строка вводит в заблуждение читающего агента, но не блокирует и не
|
||||||
|
портит продуктовый код напрямую. Окно срабатывания уже (нужен push
|
||||||
|
trivial-ветки, чей текущий diff ещё не содержит класса A) из-за WIP-лимита 1
|
||||||
|
и «push один раз по готовности», но метки `trivial` подтверждено
|
||||||
|
существуют и используются на практике (нашёл 10 issue с меткой `trivial`,
|
||||||
|
#612 — предметный пример) и не являются нишевым — это второй из двух
|
||||||
|
регулярных продуктовых треков.
|
||||||
|
|
||||||
|
**Почему в скоупе задачи, а не отдельный issue:** тот же файл, та же функция
|
||||||
|
(`productFlowEvidence`), тот же класс дефекта, который #632 и должен закрыть
|
||||||
|
— «продуктовый S-flow… должен сохранять право менять class A» из тела
|
||||||
|
issue не содержит оговорки «кроме trivial». Решение владельца 2026-08-19
|
||||||
|
(#202): Medium в скоупе чинится в этой же задаче.
|
||||||
|
|
||||||
|
**Предлагаемое (не обязывающее автора) направление:** добавить
|
||||||
|
`labels.includes('trivial')` (по аналогии — стоит проверить и не помешает ли
|
||||||
|
это тому, что `trivial`+`infra` тематическая метка одновременно
|
||||||
|
встречается на реальных issue вроде #538/#467-470 — там `trivial` явно
|
||||||
|
описывает продуктовый short-track, а не ускоренный infra-вход, что я
|
||||||
|
проверил чтением PROCESS.md §1 и §5.1: ускоренный infra-вход не имеет
|
||||||
|
понятия трека `trivial`/`small` вовсе, это метки исключительно продуктового
|
||||||
|
S-flow) в список признаков `productFlowEvidence`, плюс тест на кейс
|
||||||
|
`['trivial', 'S6-in-progress']` и мутант, ловящий откат.
|
||||||
|
|
||||||
|
### Что проверено и корректно
|
||||||
|
|
||||||
|
- **AC1** (продуктовая S6-задача с ТЗ/ревью ТЗ сохраняет право класса A) —
|
||||||
|
доказано `test/task-packet.test.mjs` › `#632: product S6 issue keeps class
|
||||||
|
A rights…`, мутант `task-packet-product-flow-overrides-diff` красит именно
|
||||||
|
этот тест (проверено прогоном).
|
||||||
|
- **AC2** (`docs/reviews/**` не в материале ветки) — доказано `#632: review
|
||||||
|
documents never classify a branch as infrastructure`, мутант
|
||||||
|
`task-packet-review-docs-not-material` (проверено прогоном). Отдельно
|
||||||
|
проверил чтением: `branchIsInfrastructure` фильтрует по префиксу
|
||||||
|
`docs/reviews/` до вызова `classify`, поэтому вложенные пути
|
||||||
|
(`docs/reviews/legacy/…`, если появятся) тоже безопасны.
|
||||||
|
По исходному репро #607 (ветка содержит **только** `SPEC-REVIEW-607-r1.md`)
|
||||||
|
`branchIsInfrastructure` возвращает `false` уже на уровне `material.length
|
||||||
|
=== 0` — сам факт, что #632 закрывается даже без `productFlowEvidence` для
|
||||||
|
этого конкретного случая; `productFlowEvidence` расширяет защиту на менее
|
||||||
|
тривиальные ситуации (например, ветка уже содержит один файл класса B —
|
||||||
|
тест/фикстуру — раньше первого файла класса A).
|
||||||
|
- **AC3** (настоящая инфраструктурная задача без ТЗ, включая возврат в S6 и
|
||||||
|
стояние на S7, сохраняет запрет) — доказано `#632: statusless or returned
|
||||||
|
infra issue without spec keeps the class A ban`, мутант
|
||||||
|
`task-packet-s6-s7-alone-not-product-flow` (проверено прогоном). Но
|
||||||
|
покрывает только `['bug','infra','process']` / `['infra','S6-in-progress']`
|
||||||
|
/ `['infra','S7-code-review']` — не `trivial`, см. находку выше.
|
||||||
|
- **AC4** (каждый признак самодостаточен; `## ТЗшка` не считается разделом
|
||||||
|
ТЗ) — проверено чтением: `(?![\p{L}\p{N}_])` корректно отсекает
|
||||||
|
`ТЗшка`, поскольку `\b` в JS не различает кириллицу как «словесный»
|
||||||
|
символ (голый `\b` посчитал бы границу сразу после `З` и ложно совпал бы) —
|
||||||
|
логика верна, подтверждено прогоном теста `#632: product S6 issue keeps
|
||||||
|
class A rights…`, который явно проверяет и позитивный, и негативный случаи
|
||||||
|
заголовка.
|
||||||
|
- Реальный кейс #607 (единственный файл на ветке — `SPEC-REVIEW-607-r1.md`)
|
||||||
|
и обратный кейс (единственный файл — `scripts/task-packet.mjs`,
|
||||||
|
собственная ветка #632) оба воспроизведены тестами и не про регрессируют
|
||||||
|
друг друга.
|
||||||
|
- Трейлеры коммита: `Issue: #632`, `User-Visible: no` — корректно,
|
||||||
|
продуктового поведения нет, changelog не требуется.
|
||||||
|
- Класс файлов: `scripts/task-packet.mjs`, `test/task-packet.test.mjs`,
|
||||||
|
`scripts/mutation-registry.mjs` — весь дифф класса B, ни одного файла
|
||||||
|
класса A; трек «инфраструктурный (ускоренный вход)» issue #632 применён
|
||||||
|
корректно (метки: `bug, infra, S7-code-review, process`, без предыдущих
|
||||||
|
`S1…S6`).
|
||||||
|
- Формат нового мутанта в `scripts/mutation-registry.mjs` соответствует
|
||||||
|
соседним записям (`id`, `guard`, `because`, `patches[].find/replace`);
|
||||||
|
`find`-строки совпадают с реальными строками файла посимвольно (иначе
|
||||||
|
`--check` не прошёл бы анкоры — прогнано и подтверждено).
|
||||||
|
|
||||||
|
### Чего не проверял
|
||||||
|
|
||||||
|
- Живой `node scripts/task-packet.mjs --issue 607` с реальным `gh` — не
|
||||||
|
запускал по той же причине, что и автор (`gh`/`api.github.com`
|
||||||
|
недоступны из песочницы ревьюера). Проверено на уровне `buildPacket`
|
||||||
|
напрямую, что эквивалентно проверке решающей логики — сборка входов
|
||||||
|
(`collectInputs`) в этой правке не менялась содержательно (только вызов
|
||||||
|
вынесенной `branchIsInfrastructure`).
|
||||||
|
`check-inputs.mjs --coverage`, `no-new-any.mjs` — не перегонял, diff не
|
||||||
|
добавляет новых скриптов/тестов вне уже показанных и не содержит TS-типов
|
||||||
|
(`any` неприменим к `.mjs`); полагаюсь на зелёный Validate на этом SHA.
|
||||||
|
- `process-gate.mjs` целиком — не перегонял; трейлеры проверил вручную
|
||||||
|
(`git log -1 --format=full`), формат совпадает с §1 таблицей классов.
|
||||||
|
|
||||||
|
## Материал раунда
|
||||||
|
```
|
||||||
|
tree 7cd2f2bf4d1c876b39a41d53e489ec1bbe1019cf
|
||||||
|
blob e033c1014d283942b33fe4819a7def6ec710172d scripts/task-packet.mjs
|
||||||
|
blob 5cbca549226a4905c093671f6c55b394c17c0168 test/task-packet.test.mjs
|
||||||
|
blob a0ff104344554102d33db3e18a06cfa9d2932ec7 scripts/mutation-registry.mjs
|
||||||
|
SHA материала ревью: a775080ae231bf0e1524ab13fda85ee303234280
|
||||||
|
```
|
||||||
|
|
||||||
|
## Вердикт
|
||||||
|
|
||||||
|
**Жёлтый.** AC1/AC2/AC3/AC4 из хендоффа доказаны и воспроизведены (мутанты
|
||||||
|
красят заявленные тесты, тесты падать умеют). Один Medium **в скоупе**:
|
||||||
|
`productFlowEvidence` не покрывает `trivial`-трек продуктовых задач (§5.1),
|
||||||
|
из-за чего реальный продуктовый bug fix на этом треке в `S6-in-progress`/
|
||||||
|
`S7-code-review` может получить тот же ложный «класс A запрещён», что и
|
||||||
|
исходный баг #607 — просто по другому репро. High нет. Возврат автору для
|
||||||
|
правки в этом же issue (решение владельца 2026-08-19, #202) — отдельный
|
||||||
|
issue не заводится.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||||
|
|
||||||
|
## Материал раунда
|
||||||
|
|
||||||
|
- Ветка: `issue/632-task-packet-track`, коммит `a775080ae231` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||||
|
- Дерево материала: `ea023197549dc674c02fae226d9324246cf522fe`
|
||||||
|
```
|
||||||
|
git log --all --format='%H %T' | grep ea023197549d
|
||||||
|
```
|
||||||
|
- Тело issue: `85577dba6802882fd2cb7cc293044cd8318bc67981c0f743e27b5e77294bdece`
|
||||||
|
- Вердикт конвейера: `yellow` · High 0
|
||||||
@@ -19,6 +19,7 @@
|
|||||||
| #635 | [CODE-REVIEW-635-r3.md](CODE-REVIEW-635-r3.md) | code · r3 | 🟢 зелёный | 0 | 0 | firstParagraph: ветка нет\b в фильтре мёртвая из-за ASCII-only \b в JS-регэкспах, расхо… | `scripts/reviews-index.mjs` |
|
| #635 | [CODE-REVIEW-635-r3.md](CODE-REVIEW-635-r3.md) | code · r3 | 🟢 зелёный | 0 | 0 | firstParagraph: ветка нет\b в фильтре мёртвая из-за ASCII-only \b в JS-регэкспах, расхо… | `scripts/reviews-index.mjs` |
|
||||||
| #634 | [CODE-REVIEW-634-r1.md](CODE-REVIEW-634-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
| #634 | [CODE-REVIEW-634-r1.md](CODE-REVIEW-634-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||||
| #633 | [CODE-REVIEW-633-r1.md](CODE-REVIEW-633-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
| #633 | [CODE-REVIEW-633-r1.md](CODE-REVIEW-633-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||||
|
| #632 | [CODE-REVIEW-632-r1.md](CODE-REVIEW-632-r1.md) | code · r1 | 🟡 жёлтый | 0 | 1 | trivial-трек не входит в признаки продуктового потока | `scripts/task-packet.mjs` `task-packet.mjs` |
|
||||||
| #630 | [CODE-REVIEW-630-r1.md](CODE-REVIEW-630-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
| #630 | [CODE-REVIEW-630-r1.md](CODE-REVIEW-630-r1.md) | code · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||||
| #629 | [SPEC-REVIEW-629-r1.md](SPEC-REVIEW-629-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
| #629 | [SPEC-REVIEW-629-r1.md](SPEC-REVIEW-629-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | — | — |
|
||||||
| #627 | [SPEC-REVIEW-627-r1.md](SPEC-REVIEW-627-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | избыточное (не противоречивое) условие в AC2; влияние на touch не названо явным пунктом | `docs/TOUCH-SUPPORT.md` |
|
| #627 | [SPEC-REVIEW-627-r1.md](SPEC-REVIEW-627-r1.md) | spec · r1 | 🟢 зелёный | 0 | 0 | избыточное (не противоречивое) условие в AC2; влияние на touch не названо явным пунктом | `docs/TOUCH-SUPPORT.md` |
|
||||||
|
|||||||
Reference in New Issue
Block a user