Files
houseplan-card/docs/reviews/CODE-REVIEW-517-r1.md
2026-09-10 07:36:17 +00:00

15 KiB
Raw Permalink Blame History

CODE-REVIEW-517-r1

Issue: #517 · этап: code · заход r1 · блокирующих циклов израсходовано 0 из 4 Материал: 156835c048a70497d7f4116c60f4c60457a05107 (ветка issue/517-spec-in-issue-body, один коммит на origin/dev)

Скоуп

Класс B/C: .github/workflows/process.yml, PROCESS.md, AGENTS.md, docs/specs/README.md, scripts/mutation-gate.mjs, scripts/process-gate.mjs, scripts/review-doc-guard.mjs, scripts/task-packet.mjs, test/process-gate.test.mjs, test/review-doc-guard.test.mjs, test/task-packet.test.mjs. Ни одного файла класса A — User-Visible: no корректен, changelog не тронут и не должен быть.

Задача переносит доказуемость «ТЗ полного трека вынесено на этом тексте» из файла docs/specs/NN-slug.md в хеш тела issue, снимаемый конвейером, и замораживает каталог docs/specs/ как архив. Решение владельца зафиксировано в самом issue (2026-09-10).

Это первый заход код-ревью (S6→S7); дельты к прошлому раунду нет — раздел «Унаследовано из r» не применяется, разбор ниже полный, как и требуется на r1.

Как проверялось

Прочитано. docs/SCOPE.md (задача — процессная, персона не описана в SCOPE, что верно отмечено в самом ТЗ), PROCESS.md целиком (§1–14), AGENTS.md, тело issue #517 и все пять комментариев (S2-аналитика, вердикт ревью ТЗ r1 жёлтый, правки автора, вердикт ревью ТЗ r2 зелёный, хендофф S6→S7). Канонические доки подсистем (SUN/LIGHT/…) не относятся — задача не трогает продукт.

Гейты.

Гейт Результат
npx tsc --noEmit чисто
npm test 2469 tests, 2468 pass, 1 skipped, 0 fail
npm run build собран; cmp dist/houseplan-card.js custom_components/houseplan/frontend/houseplan-card.js — идентичны
node scripts/smoke-select.mjs --base origin/dev --head HEAD «Исполняемого frontend-диффа нет — браузер-смоки не выбираются» (src/** не тронут)
node scripts/check-docs.mjs «Documentation checks passed (7 files, 12 external links)» — не обязателен (src/** не тронут), прогнан для очистки совести
node scripts/process-gate.mjs --issues «гейт пройден, предупреждений 1» (WARN п.8: инфраструктурный диапазон, класса A нет — статус issue не требуется)
4 названных мутанта, node scripts/mutation-gate.mjs --id=<name> все 4 «поймано 1 из 1» (ниже)

Не прогонялись и почему: golden:verify (нет визуальных изменений), pytest tests_backend (Python не тронут), no-new-any.mjs (правки в .mjs, не в src/** TS), инварианты модели (геометрия не тронута), performance-профили (не названы в AC и не задеты).

Мутанты — таблица «чем краснеет» (§2.7):

AC Чем доказан Чем краснеет
AC1 (хеш тела в якорях) test/review-doc-guard.test.mjs + мутант review-anchor-drops-issue-body строка - Тело issue: не печатается → тест красный (проверено: node scripts/mutation-gate.mjs --id=review-anchor-drops-issue-body → «тест покраснел, как обязан», поймано 1/1)
AC2 (находка «ТЗ менялось») тот же файл + мутант review-ignores-changed-spec-body issueBodyChanged всегда возвращает null → красный (проверено, поймано 1/1)
AC6 (reuse видит правку тела) тот же файл + мутант reuse-ignores-changed-issue-body сравнение хеша в reusableGreenVerdict снято → красный (проверено, поймано 1/1)
AC3 (гейт без файла ТЗ) test/process-gate.test.mjs + мутант process-gate-requires-spec-file проверка текста заменена на «только файл» → красный (проверено, поймано 1/1)
AC4 (документы без требования файла) test/review-doc-guard.test.mjs, текстовые ассерты по PROCESS.md/AGENTS.md/README сравнение прочтением: строки-требования файла ТЗ нет нигде (grep ниже)
AC5 (AC из тела первым) test/task-packet.test.mjs — прогнан в составе npm test, три сценария (тело/файл/ничего) явно не воспроизводил мутацию отдельно — чистая функция, покрытие достаточное по трём веткам условия; риск низкий

Живая проверка issueBodyDigest на реальном теле issue #517 (снятом gh issue view 517 --json body) пересчитана мной независимо: d7258d4432fab7757039bc05e1ea932f9e023f61449bdf667da69002a06906c2 — совпадает с указанным в хендоффе побайтово. Не заявление автора, а мой собственный прогон.

Проверка AC по существу (чтение + тесты)

  • AC1. materialAnchorBlock пишет - Тело issue: \`только когда переданissueBody (иначе строки нет — старые документы не ломаются, тест это утверждает). Единый шаг «Опубликовать документ ревью» (process.yml:912-988) вызывает --anchorс--issue-body="$MATERIAL_ISSUE_BODY"**на обоих этапах** (spec и code) — не только для code, как можно было бы прочитать из плана поверхностно; проверено чтением шага целиком, условиеif:не сужает его до одного этапа. Хеш снимается в шагеmaterialчерезgh issue view --json body— не изgithub.event.issue.body` (У1 выполнен буквально, а не только продекларирован).
  • AC2. Шаг spec_body (process.yml:652-684) сравнивает текущий хеш с записью последнего зелёного SPEC-REVIEW-NN-r*.md через issueBodyChanged, включён в промпт ревьюера форматированной строкой и запускается только на этапе code и только когда reuse не сработал — что корректно, потому что reuse=true уже само по себе гарантирует неизменность тела (см. AC6). Условие «нет зелёного документа или в нём нет записи → не находка» проверено и тестом, и чтением (issueBodyChanged возвращает null).
  • AC6. reusableGreenVerdict(docs, differs, issueBodyDigest) — третий параметр сравнивается с anchorIssueBodyFrom(latest.text); документ без записи (весь бэклог) судится по дереву, как раньше — проверено тестом с «legacy»-документом без строки. Единственное отступление от буквы AC6 не найдено.
  • AC3. checkSpecs теперь судит текст тела (## ТЗ заголовок или токен AC1), архивный файл всё ещё принимается; офлайн — молчание, --issues — предупреждение, никогда не отказ. Соответствует и ТЗ, и записанному в PROCESS.md §8/§10.5 п.3. checkFrozenSpecs — новая, отдельная, тёплое предупреждение на новый файл в docs/specs/** (README.md исключён явно и по regex, и потому что это правка, не добавление — git diff-filter=A его не найдёт).
  • AC4. Прочитаны PROCESS.md (§2.3, §5, §7.1, §7.3 п.1, §8 правило 3, §13 п.2), AGENTS.md (раздел Specs) и docs/specs/README.md целиком: ни один не требует файла ТЗ для полного трека; grep -n "docs/specs/NN\|docs/specs/<NN>" не находит ни одного места, где файл называется обязательным артефактом — только «архивный, доживает». Ссылка на раздел «Обязательные release-артефакты» в README сохранена и корректна.
  • AC5. task-packet.mjs:buildPacket — fromBody = extractAcceptanceCriteria(issue.body); если непусто или файлов ТЗ нет — источник тело, иначе файл. Тест на трёх фикстурах (тело есть/тела нет-файл есть/ни одного) проходит. Небольшая избыточность — тело парсится дважды (fromBody и затем acSource === issue.body снова через extractAcceptanceCriteria(acSource)), но это чистая функция без побочных эффектов и без наблюдаемой разницы в поведении — не нахожу это дефектом.

Находки

Ни одной находки уровня High или Medium.

Low-1 (снимаю записью, без правки). В reusableGreenVerdict(docs, differs, issueBodyDigest = null) имя третьего параметра совпадает с именем экспортируемой функции issueBodyDigest, объявленной выше в том же модуле (scripts/review-doc-guard.mjs). Внутри reusableGreenVerdict эта функция не вызывается, поэтому побочного эффекта нет — подтверждено чтением и тем, что все тесты и мутанты зелёные/красные штатно. Чисто стилистическая путаница для читателя («это хеш-значение, а не вызов функции»). Ниже порога, который стоило бы возвращать автору отдельным циклом; исправление тривиально, если автор решит его сделать при следующей правке этого файла.

Что проверено и корректно

  • Обратная совместимость: документы без строки Тело issue: (весь бэклог) продолжают читаться как раньше во всех трёх местах (anchorIssueBodyFrom → null, issueBodyChanged → null, reusableGreenVerdict пропускает новую проверку) — проверено тестами и чтением.
  • --specs формат не тронут, страховка #414 и reuse #499 не задеты для старых документов.
  • Трейлеры коммита: Issue: #517, User-Visible: no — оба корректны для этого диффа.
  • process-gate --issues на самом этом диапазоне — предупреждение п.8, ожидаемое для инфраструктурного диапазона без класса A (сам гейт это и объясняет).
  • docs/specs/README.md переписан по AC4, ссылка на раздел release-артефактов сохранена; check-docs.mjs зелёный.
  • Порядок шагов workflow (material → reuse → spec_body → Review) — проверено и тестом на текст process.yml, и чтением: хеш снимается до вызова модели, находка «ТЗ менялось» готова до промпта.
  • Live-проверка issueBodyDigest независимо воспроизведена мной на актуальном теле issue #517 и совпала с заявленной автором.

Чего не проверял

  • Живой прогон конвейера end-to-end (событие labeled → шаг material → анкор в реальном PR-документе) — невозможен до слияния этой же задачи (курица и яйцо, честно названо автором как «живая проверка на первой задаче после слияния»). Проверено вместо этого модульно: все явные и неявные ветки покрыты тестами, я перепрогнал их и мутанты сам, а не поверил отчёту.
  • golden, pytest tests_backend, no-new-any, инварианты модели, performance — не прогонялись, диф их не касается (см. таблицу гейтов выше).
  • Стилистическая придирка Low-1 не проверялась на предмет «а что если где-то ещё в файле есть похожий шэдоуинг» — искал только по этому диффу.

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

  • SHA: 156835c048a70497d7f4116c60f4c60457a05107
  • Ветка: issue/517-spec-in-issue-body
  • Дерево материала: git rev-parse HEAD^{tree} на указанном SHA
  • Документы этапа spec: docs/reviews/SPEC-REVIEW-517-r1.md (жёлтый), docs/reviews/SPEC-REVIEW-517-r2.md (зелёный)
  • Это первый документ этапа code — «Унаследовано из r» не применяется.

Вердикт: зелёный · заход r1 · блокирующих циклов 0/4 · High: 0 · Medium: 0 → в задаче


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

  • Ветка: issue/517-spec-in-issue-body, коммит 156835c048a7 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: 7a703201a159d0e3a288e467a652af3a618a499e
    git log --all --format='%H %T' | grep 7a703201a159
    
  • Вердикт конвейера: green · High 0