mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-02 04:38:55 +00:00
claude[bot]
@@ -0,0 +1,226 @@
|
||||
# CODE-REVIEW-752-r1
|
||||
|
||||
Issue: #752 · Трек: show · Заход: r1 · Блокирующих циклов использовано 0 из 2
|
||||
Материал: `1bd7435c9d206aa980df0d53711f2063bac9d1d4` (`origin/dev..HEAD`, один коммит)
|
||||
|
||||
## Скоуп
|
||||
|
||||
Issue #752 — три уточнения к еженедельному отчёту процесса (`process-metrics.mjs`),
|
||||
найденные при реализации #728 и ТЗ #737/#729:
|
||||
|
||||
1. **Объём задачи (П.1).** Объём К7 теперь — `+/−` строк только классов A и B
|
||||
(продукт, тесты, инструменты); документация, перенос архива (`docs/reviews` →
|
||||
`legacy/reviews`) и бандл в объём не входят. Коммиты задачи с трейлером
|
||||
`Release:` (приёмка эталонов, перепривязка тестов к бете) входят в объём;
|
||||
не входят только коммит кандидата беты/релиза и коммит бота `beta-derived`
|
||||
(признак — тот же предикат, что в `bundle-policy.mjs`, #657, а не патч-набор
|
||||
#727, который отбрасывал все `Release:`-коммиты). `isInfra` теперь судит по
|
||||
файлам классов A/B/C без `docs/reviews/**` — задача только с документом
|
||||
ревью признака инфраструктуры не получает.
|
||||
2. **Причины возвратов (П.2).** `merge-candidate.mjs` экспортирует `OUTCOME_SIGNS`/
|
||||
`outcomeOf` — регэксп-признак заголовка на каждый исход `commentFor`/
|
||||
`describePushRefusal`, кроме успешного слияния; текст сбоя шага (`action=error`)
|
||||
перенесён в `commentFor` байт-в-байт. `process-metrics.mjs` импортирует эти
|
||||
признаки: исходы стадии слияния после зелёного вердикта → `merge`, отказ push
|
||||
на стадии ребейза (до ревью) → новая причина `push-refused`. `rereview`
|
||||
причины не даёт (задача уходит в `S7`, а не автору).
|
||||
3. **Черновик ТЗ (П.4, #729).** Новый раздел «Черновик ТЗ (#729)» после «По
|
||||
трекам»: эпохи `S4-spec-review` на треке `ask` (эпоха — от постановки `S4`
|
||||
до следующей статусной метки, повторная `S4` эпоху не начинает), доля с
|
||||
комментарием «Черновик:» и их исход (`S5-ready` — в дело, `S3-spec` —
|
||||
выброшен), коммиты `Spec-Draft:` в `dev` за окно, и медиана `S5 → S7` по
|
||||
трекам с черновиком и без (`n < 3` — «мало данных»). `fetchSnapshot` теперь
|
||||
тянет таймлайны и для задач в `S5-ready`…`S7-code-review`, не только `S8`/
|
||||
закрытых — иначе раздел не видел бы задачи, ещё не домёрженные.
|
||||
|
||||
П.3 (токены за окно) по тексту ТЗ прямо вынесен в отдельный issue (#761,
|
||||
зависит от #737) и в материале отсутствует — это ожидаемо, не находка.
|
||||
|
||||
`User-Visible: no` — отчёт не меняет поведения карточки/интеграции, только
|
||||
инструмент мейнтейнера; трейлеры `Issue: #752` и `User-Visible: no` на
|
||||
коммите на месте, changelog не тронут — верно.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Прочитан полный дифф (`scripts/merge-candidate.mjs`, `scripts/process-metrics.mjs`,
|
||||
оба тестовых файла), тело issue #752 и все 4 комментария (включая уточнение ТЗ
|
||||
от 2026-10-01T13:36 и финальный отчёт разработчика). Логика каждой новой функции
|
||||
(`isBetaCommit`, `countsToVolume`, `isTaskFile`, `changeVolume`, `specEpochs`,
|
||||
`draftSection`, `outcomeOf`/`outcomeReason`) прослежена вручную по тестовым
|
||||
фикстурам построчно — не только прочитана, но пересчитана на конкретных входах
|
||||
теста, чтобы убедиться, что ожидаемые числа в `assert` получаются именно этим
|
||||
кодом, а не совпадение. Отдельно сверены регэкспы `OUTCOME_SIGNS` против
|
||||
реальных строк, которые возвращает каждый `case` в `commentFor` — совпадение
|
||||
подтверждено построчно, не на глаз.
|
||||
|
||||
### Гейты
|
||||
|
||||
| Гейт | Статус | Комментарий |
|
||||
|---|---|---|
|
||||
| `npx tsc --noEmit`, `npm test`, `npm run build` + bundle-policy verify | **не перегонял** | Validate на этом SHA зелёный (ссылка в постановке задачи) — дешёвые гейты подтверждены, бюджет раунда не тратится повторно |
|
||||
| `node --test test/process-metrics.test.mjs test/merge-candidate.test.mjs` | **прогнал сам** | 64 + 34 = 98 тестов, все `ok`, 0 fail — дополнительная проверка поверх Validate, т.к. это основной тестовый материал задачи |
|
||||
| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | **прогнал** | «Исполняемого frontend-диффа нет (`src/**/*.ts` не тронут)» — смоки не выбираются, выбирать нечего |
|
||||
| `golden:verify` | не прогонял | нет метки `ci:golden`, дифф не рендер |
|
||||
| `pytest tests_backend` | не прогонял | Python не тронут |
|
||||
| `npm run invariants` | не прогонял | геометрия/модель не тронуты |
|
||||
| performance-профили | не прогонял | не названы в AC |
|
||||
| мутанты по реестру | не прогонял и не требуется | трек `show`: мутанты в разработке не гоняются ни на каком треке (#709), ревьюер их тоже не применяет |
|
||||
|
||||
## Проверка AC
|
||||
|
||||
**AC1 (объём).** Доказан `test/process-metrics.test.mjs`: `#752 AC1 объём: классы
|
||||
A и B…` и `#752 AC1 объём: Release:-коммиты задачи входят…`. Пересчитал вручную
|
||||
обе фикстуры:
|
||||
- первая — `src/a.ts +10` (A, считается) + `docs/x.md +5000` (C, не считается) +
|
||||
перенос `docs/reviews/R.md → legacy/reviews/v1/R.md` (не считается) → объём
|
||||
`751` = 10, корзина `≤30`; задача `#7` только с документом ревью → `isInfra`
|
||||
ложь, `changeVolume` = `null`. Добавление `test/a.test.mjs +40` (B) даёт 50 —
|
||||
тесты входят.
|
||||
- вторая — «приёмка эталонов» (`demo/golden/attestation.json`, класс B, 6 строк)
|
||||
и «перепривязка тестов» (`test/b.test.mjs`, 4 строки) входят в объём `751`
|
||||
(10+6+4=20); кадры `demo/golden/baselines/*.png` — класс D, не входят.
|
||||
Кандидат беты (подпись `Release v1.0.0-beta.2 candidate`), его рефреш без
|
||||
пересборки бандла (подпись кандидата без изменённого бандла — тоже
|
||||
`committedBundleMustMatch` true по подписи), промоушен (`release: promote…`,
|
||||
бандл в файлах) и бот `beta-derived` (по подписи из `_beta-derived.yml`,
|
||||
сверено отдельным контрактным тестом) — все четыре правильно отфильтрованы
|
||||
`isBetaCommit`, подтверждено `assert.deepEqual(commits.filter(isBetaCommit)…)`
|
||||
с точным списком subject'ов. Отдельно проверен краевой случай: текст
|
||||
«Release v1.0.0 candidate notes» в теле, где `Release: notes wrap here` не
|
||||
является трейлером (строка перед терминальным блоком, не в нём) —
|
||||
`isBetaCommit` корректно возвращает `false`; прочитал `releaseTrailers()` и
|
||||
убедился, что парсинг идёт строго с конца сообщения до первой несовпадающей
|
||||
строки, то есть этот тест действительно проверяет границу терминального
|
||||
блока, а не дублирует предыдущий. **Чем краснеет:** со старым `countsToVolume`
|
||||
(класс ≠ D и не `docs/reviews/**`) первая фикстура дала бы `751` = 5010
|
||||
(документация в объёме) — именно то число, которое ТЗ называет как
|
||||
регрессионный маркер; подтверждено чтением, тест этого не исполняет заново со
|
||||
старым кодом, но разница однозначно видна по диффу `countsToVolume`.
|
||||
- Старый тест AC7 (`#728 сравнение: объём из git без Release:, dist/** и
|
||||
docs/reviews/**…`) не менялся и остаётся зелёным — пересчитал его фикстуру
|
||||
вручную под новой логикой (класс кандидата по подписи вместо голого
|
||||
`Release:`-фильтра) и убедился, что оба коммита (`701`, `702`) дают те же
|
||||
числа объёма и инфраструктуры, что и раньше.
|
||||
|
||||
**AC2 (причины возвратов).** Доказан `test/merge-candidate.test.mjs` (`#752 AC2:
|
||||
каждый шаблон исхода…`) и `test/process-metrics.test.mjs` (`#752 AC2 причины…`).
|
||||
Сверил каждый из 11 `OUTCOME_SIGNS` построчно с текстом соответствующего `case`
|
||||
в `commentFor`/`describePushRefusal` — совпадение есть и оно единственное
|
||||
(тест `merge-candidate.test.mjs` проверяет это явно: `OUTCOME_SIGNS.filter(...)`
|
||||
даёт ровно один признак на каждый текст). Тест `process-metrics.test.mjs`
|
||||
прогоняет сценарий «зелёный вердикт → 6 разных исходов слияния → `merge`,
|
||||
отказ push на ребейзе → `push-refused`, возврат без комментария → `unknown`»
|
||||
через полный `issueTrackMetrics`/`trackSection`/`renderMarkdown` — то есть
|
||||
проверена не только чистая функция, но и интеграция с существующей таблицей
|
||||
«По трекам» (строка `| show | 8 | 0 | 0 | 0 | 0 | 1 | 6 | 0 | 0 | 1 |` сверена
|
||||
построчно с входными данными). Также изменены (не ослаблены, а приведены к
|
||||
новому определению) три утверждения теста #728: фикстура `Release:`-коммита в
|
||||
тесте `trackAt` получила подпись кандидата (иначе по новому правилу это был бы
|
||||
коммит задачи, а не беты — старая фикстура со старым намерением перестала бы
|
||||
значить то же самое); `returnSignal` для отказа push на ребейзе теперь ждёт
|
||||
`'push-refused'` вместо `null`; агрегат по треку `show` в `returnReason`
|
||||
аналогично. Это ожидаемое следствие самого фикса, а не ослабление проверки —
|
||||
логика до изменения доказуемо отличалась (`sig(refused)` была `null`, стала
|
||||
`'push-refused'`, и это ровно то поведение, которое чинит AC2). **Чем
|
||||
краснеет:** тест называет это явно — «на нынешнем коде все пять первых —
|
||||
unknown» (с кодом до правки `sig()` возвращает `null`/`'unknown'` вместо
|
||||
`'merge'`/`'push-refused'`).
|
||||
|
||||
**AC3 (черновики).** Доказан двумя тестами `#752 AC3 черновик ТЗ…`. Прошёл
|
||||
`specEpochs` вручную по фикстуре (повторная постановка `S4-spec-review` не
|
||||
открывает новую эпоху — геометрия событий и результат `[['S3-spec',2],
|
||||
['S5-ready',1]]` сошлись). Прошёл `draftSection` вручную по обеим веткам
|
||||
(черновик выброшен в `S3`, черновик пошёл в `S5`) — числа `{total, withDraft,
|
||||
used, thrown, other}` совпадают с ручным пересчётом. Второй тест проверяет
|
||||
`s5ToS7` по трекам: пересчитал вручную для `ask` (3 задачи с черновиком,
|
||||
медиана 2ч, `n=3` — «данных достаточно»; 2 задачи без черновика, медиана 20ч,
|
||||
`n=2 < 3` — «мало данных») и для `ship` (без эпох `S4` вовсе — `draftedS5`
|
||||
остаётся `false` по умолчанию, что корректно для трека, не проходящего `S4`).
|
||||
Задача, у которой `S7` вне окна (`noS7`), и коммит с `Spec-Draft:` вне окна
|
||||
или без трейлера — оба случая исключены корректно.
|
||||
|
||||
Отдельно нашёл место, не покрытое прямым тестом: расширение `fetchSnapshot`
|
||||
(`allIssues` теперь включает задачи с метками `S5-ready`…`S7-code-review`, не
|
||||
только `S8-merged`/закрытые) — необходимое условие, чтобы `draftSection`
|
||||
вообще увидел ещё не домёрженные `ask`-задачи в проде. Единственный тест на
|
||||
`fetchSnapshot` (`#728 fetchSnapshot…`) не тронут и проверяет попадание задачи
|
||||
`#703` (метка `S6-in-progress`) в `allIssues` — но она туда попадает и без
|
||||
нового условия, через существующий путь `runIssues` (задача упомянута в
|
||||
`display_title` прогона конвейера). То есть новая ветка фильтра в этом тесте
|
||||
не отличима от старой. Прочитал код `fetchSnapshot` (строка с
|
||||
`['S5-ready', 'S6-in-progress', 'S7-code-review', 'S8-merged'].includes(...)`)
|
||||
и убедился, что она синтаксически верна и соответствует описанию в ТЗ и в
|
||||
комментарии над функцией — **проверено чтением, не исполнением**. Это разрыв в
|
||||
покрытии интеграционного пути (AC3 доказан на уровне чистой функции
|
||||
`draftSection`, но не на уровне «фактически попадёт ли нужная задача в tolist
|
||||
при реальном вызове `fetchSnapshot`»), не найденный мной дефект поведения —
|
||||
отмечаю как Low, не как Medium, потому что сама проверка кода не вскрыла
|
||||
несоответствия.
|
||||
|
||||
## Находки
|
||||
|
||||
**Low.** `fetchSnapshot`: новый фильтр `allIssues` (задачи `S5-ready`…
|
||||
`S7-code-review`) не покрыт отдельным тестом — существующий тест на эту
|
||||
функцию проходит через параллельный путь (`runIssues`) независимо от нового
|
||||
условия. Код прочитан и признан корректным; это пробел в доказательстве
|
||||
интеграции, не поведенческий дефект. Трек `show`: бухгалтерия такого рода —
|
||||
Low, цикла не открывает (REVIEWER.md, «Трек show»).
|
||||
|
||||
Больше находок нет: High — 0, Medium — 0.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- Классификация коммитов беты/кандидата/бота (`isBetaCommit`) — все пять
|
||||
категорий из уточнения ТЗ (кандидат по подписи, кандидат по бандлу,
|
||||
промоушен, бот, ложный текст «Release:» не в трейлере) разобраны и
|
||||
пересчитаны вручную, совпадают с тестом.
|
||||
- Классы файлов для объёма (A+B) и признака инфраструктуры (A+B+C без
|
||||
`docs/reviews/**`) соответствуют единой таблице `change-classes.mjs`, не
|
||||
переопределяются локально.
|
||||
- `OUTCOME_SIGNS`/`outcomeOf`/`outcomeReason` — однозначное соответствие
|
||||
признак → исход → причина отчёта, включая не-причину `rereview`; защитный
|
||||
`throw` при отсутствии `rereview` в реестре — на месте.
|
||||
- `specEpochs`/`draftSection` — границы эпох, повторная метка, попадание в
|
||||
окно по первому `S5-ready`, `MIN_COHORT=3` — все пересчитаны вручную на
|
||||
тестовых таймлайнах.
|
||||
- Единственное число — единственный источник: объём/корзина берутся из одного
|
||||
`changeVolume`/`volumeBucket`, используемых и в «По трекам», и в «До и
|
||||
после»; причина возврата — из одного `returnSignal`.
|
||||
- Трейлеры коммита (`Issue: #752`, `User-Visible: no`) корректны, changelog не
|
||||
тронут — соответствует `User-Visible: no`.
|
||||
- Отклонения, заявленные автором (предикат #657 вместо патч-набора #727; три
|
||||
скорректированных утверждения теста #728), проверены по существу и
|
||||
признаны обоснованными, а не прикрытием дефекта.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Полный прогон `npx tsc --noEmit` / `npm test` (весь набор) / `npm run build`
|
||||
— зелёный Validate на этом SHA уже их подтвердил (ссылка в постановке).
|
||||
Сам прогнал только два изменённых тестовых файла (98 тестов) как
|
||||
дополнительную проверку.
|
||||
- `golden:verify`, `pytest tests_backend`, `npm run invariants`,
|
||||
performance-профили — не применимы к этому диффу (нет меток/файлов,
|
||||
требующих эти гейты); `smoke-select.mjs` подтвердил, что frontend-диффа нет.
|
||||
- Мутанты по реестру — не прогонял и не обязан: трек `show`, их ловит ночной
|
||||
прогон (#709).
|
||||
- Реальный GitHub API / реальные issue конвейера — интеграция `fetchSnapshot`
|
||||
с реальными данными репозитория не воспроизводилась; см. находку Low выше.
|
||||
- «16 ручных поломок», заявленные автором в комментарии — не переигрывал
|
||||
заново; трек `show` не требует от ревьюера повторения авторской мутационной
|
||||
проверки, а точечная проверка по тестовым файлам и чтению кода совпадений с
|
||||
авторским описанием не выявила расхождений.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/752-metrics-volume-reasons`, коммит `1bd7435c9d20` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `fba4442b8b85bf3ba1ac7d06c04e1d9eb104fe1c`
|
||||
```
|
||||
git log --all --format='%H %T' | grep fba4442b8b85
|
||||
```
|
||||
- Тело issue: `0f72ec0637dcb254d7af7ca5f978581fd8cb3d9206198b0dd0f6666b1d1008c3`
|
||||
- Вердикт конвейера: `green` · High 0 · маршрут `fix`
|
||||
<!-- hp:usage input_tokens=5179 output_tokens=43592 cache_creation_input_tokens=134275 cache_read_input_tokens=4044567 num_turns=45 -->
|
||||
Reference in New Issue
Block a user