Files
2026-10-01 16:49:55 +00:00

19 KiB
Raw Permalink Blame History

CODE-REVIEW-748-r3

Issue: #748 · этап: code · трек: show · заход: r3 · блокирующих циклов израсходовано 0 из 2 Материал: 40fa1d8505bfbf07cf7300eda040a692c051e437 (рабочая копия на нём, git rev-parse HEAD сверен)

Скоуп

Пять мелких расхождений канона/подсказок/кода, найденных при реализации #707–#730, сведённых в одну инфраструктурную задачу (класс B, трек show, подтверждён владельцем: «Оценка… трек: show»):

  1. подсказка S3 в task-packet.mjs звала пушить ветку ТЗ — убрано (§2.3, #517, §11.8);
  2. номер 9 в RULES (process-gate.mjs) держал мёртвую проверку «Gates: light», пока §10.2 п.9 — нереализованное; проверка снята, добавлен контракт нумерации;
  3. §10.4 п.4 канона требовал heredoc в run:, а код и тесты #723/#730 требуют обратного — формулировка приведена в соответствие с кодом;
  4. четыре места («ship merge» комментарий, AUTHOR.md, REVIEWER.md, AGENTS.md) называли только добетовый пакетный обзор ship, хотя с #727 ночь читает код первой — тексты обновлены;
  5. сообщение коммита ночной публикации ship-ревью несло бета-трейлер #696 — у ночи теперь свой заголовок/тело/Issue: #727.

Это третий заход ревью этой же задачи. Оба предыдущих (r1 на c099c4dd, r2 на 39521011) — зелёные, без правок по существу. Между раундами содержимое не менялось: оба раза механизм слияния show отказывал, потому что токен конвейера (HP_PROCESS_TOKEN) не имеет права workflow, а кандидат меняет .github/workflows/_process.yml и _ship-review.yml — GitHub отклоняет push такого файла без этого скоупа (не находка материала, #312: это отказ GitHub по праву токена, а не расхождение ветки). Оба раза автор ребейзил ветку на ушедший вперёд dev своими правами и возвращал S7-code-review; дельта между r2 и r3 — второй такой ребейз (на dev после слияния #749) плюс пересборка docs/reviews/INDEX.md. Содержательный коммит переименовался с 39521011→ 617dc140 (хеш меняется при ребейзе), но дерево то же: подтверждено прямым чтением всего диффа origin/dev...HEAD в этом раунде (старые SHA c099c4dd, 39521011, 945377aa в этом дереве не резолвятся — обычное дело после ребейза, не находка, §2.10).

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

Полный разбор, не сокращённый по дельте: предмет повторного раунда — ребейз на ушедший вперёд dev, для которого §2.10 прямо требует полный объём («ребейз на ушедший вперёд dev» — один из перечисленных случаев), а старый материал не резолвится, так что сверять «только изменившееся» не от чего технически — сверено построчно с ТЗ и с обоими прежними отчётами.

  • git diff origin/dev...HEAD прочитан файл за файлом: task-packet.mjs, process-gate.mjs, PROCESS.md §10.4 п.4, .github/workflows/_process.yml, .github/workflows/_ship-review.yml, AGENTS.md, docs/process/AUTHOR.md, docs/process/REVIEWER.md, docs/testing-notes/mutation-browser-guards.md, все пять тестовых файлов, docs/reviews/INDEX.md.
  • AC1 (п.1, п.2): прочитан новый контракт в test/process-gate.test.mjs — парсит PROCESS.md §10.2, делит пункты на реализованные/«Не реализовано» и сверяет оба множества с ключами RULES и с номерами находок в коде (fail(N/warn(N). Проверено чтением регэкспов и ручной сверкой с текстом §10.2 (разделы ### 10.2 / ### 10.3, блок «Не реализовано и остаётся долгом: 9. …») — границы совпадают, тест не захватывает лишних пунктов. Тест на мёртвый трейлер (Gates: light is refused… → переименован, ждёт [] вместо [9]) и тест подсказки task-packet (doesNotMatch(/push ветки/), match(/S4-spec-review/), match(/§11\.8/)) прочитаны и соответствуют коду.
  • AC2 (п.5): прочитан диф _ship-review.yml — ветвление по MODE формирует subject/lead/issue до сборки $msg тем же построчным echo в файл (без heredoc), бета-ветка не тронута. Тест test/publish-push-refusal.test.mjs («#748 AC2 … ночная публикация») гоняет настоящий bash/git-сэндбокс и утверждает точный текст коммита в dev, включая Issue: #727; соседний бета-тест (:401–403 старый) не менялся.
  • AC3 (п.3, п.4, гейт): §10.4 п.4 прочитан целиком — новая формулировка прямо говорит «строкой не пишется», даёт обе причины (обрыв YAML и отсутствие проверки текста из YAML тестом) и ссылается на #723/#730. Все четыре места п.4 (_process.yml:1766 комментарий слияния, AGENTS.md, AUTHOR.md, REVIEWER.md) называют и ночной, и бета-документ — сверено построчно с текстом ТЗ «Меняется: …». test/process-track.test.mjs подтверждает оба фрагмента в теле комментария слияния на настоящем bash-прогоне шага «Решение по вердикту». test/ship-review.test.mjs добавляет issue-727.json и комментарий об отсеве #727 как не-ship — проверено чтением, не исполнением (фикстура для gh issue view, сам прогон теста не запускал повторно: Validate уже исполнил его на этом SHA).
  • Доказательство «тест умеет падать» для AC1/AC2 я не переисполнял откатом патча в этом раунде — это уже сделали оба прежних раунда на содержательно той же дельте (r1: откат process-gate.mjs/task-packet.mjs к HEAD^ на c099c4dd; r2: откат к HEAD~2 на 39521011, в обоих — ровно три новых теста краснеют). Дерево между раундами не менялось (см. «Скоуп»), а сами тестовые файлы в r3 идентичны тому, что проверяли r1/r2 — унаследовано.
  • node scripts/smoke-select.mjs --base origin/dev --head HEAD — прогнал сам: «Исполняемого frontend-диффа нет», браузерные смоки не выбираются — скоуп задачи не касается src/**.
  • node scripts/mutation-gate.mjs --check — прогнал сам: 206/200 (WARN, не FAIL) — подтверждает правку mutation-browser-guards.md как вторичную (ориентир, не стена, #699) и воспроизводит расхождение «205 в таблице / 206 в реестре», уже разобранное и снятое в r2 (см. ниже).

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

  • Все пять правок ТЗ (п.1–п.5) на месте, слово в слово с разделом «Меняется» тела issue; машинные маркеры (hp:ship-merge, hp:ship-risk) не тронуты, в _process.yml остаётся ровно один heredoc этого шага — как и требует новая формулировка п.4 («старые переводятся при переработке шага», этот шаг не переработан).
  • Трейлеры на всех коммитах класса, кроме C, корректны: 617dc140 — Issue: #748, User-Visible: no; обе публикации документов ревью и регенерация INDEX.md — тот же набор. Ни один коммит не трогает docs/CHANGELOG*.md — согласуется с User-Visible: no (правка не меняет видимое пользователю поведение продукта, только процесс/документацию).
  • Единственное изменённое число, видимое и пользователю задачи (разработчику как читателю документации), — счётчики mutation-browser-guards.md (205/200, lifecycle 90) — источник один: scripts/mutation-registry.mjs, mutation-gate --check печатает то же число оттуда же. Расхождение «205 в таблице» vs «206 в живом реестре сейчас» — рост реестра за счёт посторонних коммитов dev после фиксации текста задачи, не двойной источник (см. находки ниже, унаследовано из r2).
  • docs/reviews/INDEX.md — механическая регенерация (класс D), прибавляет ровно две новые строки (CODE-REVIEW-748-r1/r2) и счётчики «документов: 269, issue: 137»; не редактировался руками.
  • Контракт нумерации RULES действительно привязывает код к канону (прочитан регэксп и выполнены границы §10.2 вручную) — находка на будущее «номер правила разошёлся с каноном» (повод всей задачи) теперь ловится тестом, а не только внимательностью ревьюера.
  • route: fix. Критерий §5 не нарушен ни по одному пункту: complexity — пять текстовых синхронизаций и снятие мёртвой проверки, без нового решения; surfaces — один контур (согласованность пайплайна процесса — подсказки, гейт, канон, workflow-комментарии); migration — ни новое поле конфига, ни compatibility-флаг не добавлены; ux-contract — задача не трогает src/**, никакого пользовательского UX; perf-touch — вне скоупа полностью; undocumented — поведение уже зафиксировано кодом #723/#727/#729/#730, задача лишь согласует тексты с ним.

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

  • npx tsc --noEmit, npm test, npm run build + сверку трёх копий бандла — не перегонял: Validate на точном SHA материала (40fa1d85) зелёный (https://github.com/Matysh/houseplan-card/actions/runs/36893848393), достаточно сослаться (#343).
  • actionlint — не перегонял отдельно; инструмент недоступен офлайн в этой среде (npx actionlint не разрешился). Риск низкий: правки в _process.yml/_ship-review.yml — только внутри run:/heredoc-тела и текста markdown-комментария, без изменения структуры YAML (ключи, отступы, условия if: не тронуты), а Validate уже исполнил оба изменённых шага на этом SHA — синтаксическая ошибка YAML оборвала бы прогон, а не просто предупреждение линтера.
  • Браузерные смоки, golden:verify, pytest tests_backend, инварианты модели, performance — не гонял: smoke-select подтвердил отсутствие исполняемого frontend-диффа, Python и геометрия не тронуты, задача не называет performance в AC.
  • Повторный откат патча для демонстрации «тест умеет падать» в этом раунде — не переисполнял; унаследовано из r1/r2 (дерево между раундами не менялось, см. «Как проверялось»).

Закрытие раунда r2

Находка r2 Чем закрыта Где это видно
Low: таблица mutation-browser-guards.md («205/200») на 1 отстала от живого реестра («206/200») Не по вине задачи (реестр вырос за время ожидания ревью на посторонних коммитах dev); r2 сама сняла находку с записью и указала адрес — отдельная задача #767, где этот счётчик предлагается поставить под тест Текст вердикта r2 в issue: «Low r2 (таблица гвардов 205 против 206 в реестре) относится к #767»; воспроизведено и в r3: mutation-gate --check печатает 206/200 сейчас, таблица в дереве — «205/200»

Других находок у r2 не было (High 0, Medium 0) — закрывать по существу нечего; между r2 и r3 нет контентной дельты (см. «Скоуп»), только два ребейза и регенерация индекса.

Унаследовано из r2 (и из r1)

  • AC1, AC2, AC3 — приняты без повторного «убедился, что тест умеет падать» (откат патча): r1 проверила это на c099c4dd (HEAD^), r2 — на 39521011 (HEAD~2), оба раза откат даёт ровно три новых падающих теста; дерево задачи между r1, r2 и r3 не менялось по содержанию (только ребейз поверх ушедшего вперёд dev + публикация документов ревью), подтверждено прямым построчным чтением всего git diff origin/dev...HEAD в этом раунде.
  • Low r1 (scripts/process-gate.mjs:183 — заголовок-комментарий «проверки по одному коммиту: 1, 4, 5, 6, 9» всё ещё называет снятое правило 9) — r1 сняла её с записью («поведения и тестов не касается»); строка в дереве 40fa1d85 действительно не исправлена (проверено чтением: grep -n "проверки по одному коммиту" scripts/process-gate.mjs → строка 183 цела) — остаётся Low, цикла не открывает, трек show судит только дефекты поведения, видимые пользователю, или невыполненные AC (docs/process/REVIEWER.md «Трек show»); здесь ни того, ни другого.
  • Low r2 (счётчики mutation-browser-guards.md) — см. таблицу выше.
  • Отказ GitHub по push workflow-файла без права workflow и оба последующих ребейза (c099c4dd→39521011→40fa1d85) — не находки материала (§312), а эксплуатационный факт конвейера; приняты как есть.

Находки

Нет. High: 0, Medium: 0.

Вердикт

Зелёный. AC1–AC3 доказаны (контрактными/сэндбокс-тестами, которые умеют падать — унаследовано из r1/r2, независимо перепроверено чтением в этом раунде) и разобраны чтением там, где код уже раньше исполнялся Validate. Единственные находки — два Low из прежних раундов, оба намеренно оставлены ревьюерами с записью и не относятся к этой задаче по существу (мёртвый текст комментария без поведенческого эффекта; счётчик реестра, уехавший за время ожидания и адресованный в #767). Цикл не открывается.


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

  • Ветка: issue/748-canon-reconcile, коммит 40fa1d8505bf — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: 11b2d9c1446ac2797ccb4c8295eedc42a666bfa7
    git log --all --format='%H %T' | grep 11b2d9c1446a
    
  • Тело issue: f13eeaa6a21cf6847ca3050e8cdf314c293abd2796714243f1800f224b9e5000
  • Вердикт конвейера: green · High 0 · маршрут fix