houseplan-card/docs/reviews/CODE-REVIEW-752-r1.md

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 не требует от ревьюера повторения авторской мутационной проверки, а точечная проверка по тестовым файлам и чтению кода совпадений с авторским описанием не выявила расхождений.

---

Материал раунда

• Ветка: issue/752-metrics-volume-reasons, коммит 1bd7435c9d20 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: fba4442b8b85bf3ba1ac7d06c04e1d9eb104fe1c

  git log --all --format='%H %T' | grep fba4442b8b85
  

• Тело issue: 0f72ec0637dcb254d7af7ca5f978581fd8cb3d9206198b0dd0f6666b1d1008c3
• Вердикт конвейера: green · High 0 · маршрут fix