22 KiB
CODE-REVIEW #748 · заход r2
Материал. origin/dev..HEAD, диапазон из двух коммитов на вершине
395210117e242b5b16118cdfd1985f9a3d620cef:
945377aa—fix(process): reconcile canon and hints with the pipeline after #707–#730 (#748), содержательный коммит задачи;39521011—docs: review document for #748, публикация документаCODE-REVIEW-748-r1.md(шаг конвейера после r1).
Трейлеры обоих — Issue: #748, User-Visible: no; оба верны (ни один не
меняет видимое пользователю продукта поведение, changelog не трогается).
Трек: show. Рабочая копия уже стоит на этом SHA.
Validate на 39521011 зелёный:
https://github.com/Matysh/houseplan-card/actions/runs/36884081445
(headSha сверен напрямую: gh run view 36884081445 --json headSha →
39521011, conclusion: success).
Скоуп
Пять точечных расхождений канона/подсказок/кода с уже влитым поведением
(#517, #707–#730), плюс попутная правка таблицы
docs/testing-notes/mutation-browser-guards.md. Содержание задачи с r1 не
менялось — это тот же код, доехавший до dev после ребейза (см. «Материал
раунда r1» ниже). Полный разбор диффа и АС проведён заново, а не ограничен
дельтой, по правилу §2.10 («ребейз на ушедший вперёд dev» — довод для
полного объёма, а не для сокращённого).
Что произошло между r1 и r2
- r1 вынес зелёный вердикт на коммите
c099c4dda0a0(материал — веткаissue/748-canon-reconcile, дерево3f4ae17e4935…). - Пока шло ревью,
devушёл вперёд на 16 коммитов. Слияние кандидата упёрлось не в конфликт, а в право токена конвейера на файл.github/workflows/(GitHub отклонил push безworkflow-scope, #705) — отдельная причина, не связанная с качеством кода. - Автор сам сделал
git rebase origin/devи запушил результат без конфликтов:c099c4dd→945377aaна новой вершине14ea2661. Содержимое коммита не менялось, только базовая точка. - Конвейер опубликовал документ r1 отдельным коммитом
39521011поверх ребейзнутой ветки (это нормальный шаг для код-ревью — индексdocs/reviews/INDEX.mdв ветке задачи при этом не пересобирается, §2.10, и действительно не тронут). - Подняться на
S7-code-reviewвторой раз пришлось из-за дефекта пробуждения раунда (маркер прошлого прогона ждал старый SHA) — технический сбой конвейера, не относится к содержанию задачи; автор завёл его отдельно.
SHA c099c4dda0a0b5be8aec99b31e24d3c41b951559 и дерево 3f4ae17e4935… из
документа r1 в текущем репозитории не резолвятся (git cat-file -t → «Not a
valid object name», git log --all --format='%H %T' | grep 3f4ae17e4935 —
пусто). Это обычный случай §2.10 («SHA, который ребейз осиротил»), а не
находка: SHA не был мёртв в момент публикации r1 (ребейз случился позже, что
видно по времени его собственного комментария), и документ r1 прямо
предупреждал об этом в блоке «Материал раунда». Поэтому тождественность
содержимого между r1 и r2 проверена не по хешу дерева, а прямым построчным
чтением всего диффа origin/dev...HEAD (см. ниже) — и оно совпадает с тем,
что описывает документ r1 и тело issue.
Как проверялось
- Прочитан весь диапазон
git diff origin/dev...HEAD(15 файлов, +259/-33) файл за файлом:scripts/process-gate.mjs,scripts/task-packet.mjs,test/process-gate.test.mjs,test/task-packet.test.mjs,test/process-track.test.mjs,test/ship-review.test.mjs,test/publish-push-refusal.test.mjs,PROCESS.md,AGENTS.md,docs/process/AUTHOR.md,docs/process/REVIEWER.md,.github/workflows/_process.yml,.github/workflows/_ship-review.yml,docs/testing-notes/mutation-browser-guards.md. - Прочитано тело issue #748 (ТЗ, АС1–3, «Принято предположительно», «Кандидаты», «Риски») и все комментарии (оценка, взятие в работу, «Сделано», вердикт r1, два сообщения о ребейзе/перезапуске S7).
- AC1, доказательство «тест умеет падать». Независимо от r1: откатил
scripts/process-gate.mjsиscripts/task-packet.mjsкHEAD~2(14ea2661, вершинаdevдо задачи, тесты и прочий код — текущие);node --test test/process-gate.test.mjs test/task-packet.test.mjsдал ровно 3 красных теста (#748 AC1: номера RULES…,#748 AC1: мёртвого трейлера…,#748 AC1: подсказка S3…), 73 зелёных. Файлы возвращены кHEAD(git checkout HEAD -- …),git status --shortпуст. - Тот же прогон на текущем
HEAD(без отката) — 165/165 зелёных:process-gate,task-packet,process-track,ship-review,publish-push-refusal(включая bash-тест AC2 на настоящем git/bash- сэндбоксе). node --test test/process-digests.test.mjs test/entry-cost.test.mjs— 10/10 зелёных (сверка ссылок REVIEWER.md/AUTHOR.md на PROCESS.md не разошлась после правки формулировок).node scripts/mutation-gate.mjs --check— exit 0,browser guards: 206/200(WARN, не FAIL, подтверждает правку "ориентир, не стена" из #699).node scripts/smoke-select.mjs --base origin/dev --head HEAD— «исполняемого frontend-диффа нет, тронуто файлов: 15» — браузерные смоки этим диффом не выбираются.- Дешёвые гейты (
tsc --noEmit,npm test,npm run build+ bundle-policy) не перегонял отдельно — Validate на точном SHA материала (39521011) зелёный, подтверждено напрямую черезgh run view.
Закрытие раунда r1
| Находка r1 | Чем закрыта | Где это видно |
|---|---|---|
Low: scripts/process-gate.mjs:183 — комментарий-заголовок над evaluateCommit всё ещё перечисляет снятое правило 9 (// --- проверки по одному коммиту: 1, 4, 5, 6, 9 ---) |
Не закрыта кодом — r1 сам снял находку с записью («блокировать зелёный вердикт из-за одной цифры в комментарии несоразмерно объёму задачи», §8), без требования фикса | Строка не изменилась: grep -n "проверки по одному коммиту" scripts/process-gate.mjs → та же строка :183 с «, 9» на конце. Это ожидаемо, а не регресс — находка была явно waived, не возвращена автору |
Других находок в r1 не было (0 High, 0 Medium).
Унаследовано из r1
Без повторной проверки приняты выводы r1 о соответствии содержимого диффа
тексту ТЗ п.1–5 и о классификации риска/маршрута (документ
docs/reviews/CODE-REVIEW-748-r1.md, материал c099c4dd / дерево
3f4ae17e4935…, которые в текущем дереве не резолвятся — см. «Что произошло
между r1 и r2» выше про штатность этого случая):
- формулировка подсказки
S3вtask-packet.mjsдословно совпадает с ТЗ; - сверка нумерации
RULESс PROCESS.md §10.2 (реализовано 1–8,10; «Не реализовано»: 9) и контрактный тест, который берёт пункты из самого канона регулярным разбором, а не хардкодом; - разбор критериев §5 маршрута (сложность/риск, одна поверхность, отсутствие
миграции/UX-контракта/влияния на perf-touch, поведение уже
задокументировано) — маршрут
fixподтверждён повторно по тем же основаниям (см. «Как проверялось» выше — я читал диффы независимо и пришёл к тому же выводу).
Это не слепое доверие: каждый из этих пунктов я перепроверил собственным чтением текущего (ребейзнутого) диффа — содержание не разошлось с тем, что описывает r1, в чём и состоит смысл «унаследовано», а не «принято на веру».
Находки
Low — таблица docs/testing-notes/mutation-browser-guards.md разошлась с реестром на 1 (не по вине задачи)
Задача правит эту таблицу как попутную: 204/200 → 205/200, lifecycle 89
→ 90, под заявлением «счётчики приведены к реестру» (коммит 945377aa).
На момент этого коммита это было верно. Но живой node scripts/mutation- gate.mjs --check на текущем HEAD печатает browser guards: 206/200, и то
же самое воспроизводится на голом origin/dev без единого изменения из
#748 (git checkout origin/dev -- . && node scripts/mutation-gate.mjs --check → 206/200). То есть реестр вырос на 1 гвард уже после того, как
автор посчитал число, за счёт не относящихся к #748 коммитов, вошедших в
dev за время ожидания ревью, а механический ребейз текстовую строку с
количеством не трогает (git это не конфликт — просто неактуальная цифра).
Почему не Medium. Автоматический контракт test/mutation-gate.test.mjs
(«#659/#699: browser guard inventory is reviewed and exact») сверяет не эти
суммарные числа, а только что каждый id браузерного гварда из реестра
(scripts/mutation-registry.mjs) присутствует в таблице как строка
- `id` и что в таблице нет гвардов-призраков (missingReasons,
staleReasons — оба пустые). Этот тест зелёный: расхождение — только в
человеко-читаемой сводной строке и клетке категории, не в самом перечне, и
--check остаётся WARN (не FAIL) что до, что после расхождения (ориентир — не
стена, #699). Поведения это не меняет, пользователь это не видит, задача
#748 не создала это расхождение — это дрейф времени ожидания ревью. Ровно та
же категория, что собственная Low-находка r1 (строка-комментарий с мёртвым
номером правила): дешёвое, безвредное, инструментальное несоответствие.
Снимаю с записью, фикса не требую: число актуализирует следующая задача, которая тронет этот файл (как и было с r1). Блокировать зелёный вердикт из-за одной цифры несоразмерно объёму задачи (§8) — тот же довод, которым r1 закрыл свою находку.
Что проверено и корректно
- AC1.
rightsFor('S3-spec')→ подсказка без «push ветки», сS4-spec-reviewи ссылкой на §11.8 — текст совпадает с ТЗ дословно.RULESвprocess-gate.mjs— ключи{0,1,2,3,4,5,6,7,8,10}; полеgatesи обе проверки трейлераGates: lightудалены изmakeCommit/evaluateCommitцеликом (git grep -n "Gates" scripts test docs .github PROCESS.md AGENTS.md— только в тексте контрактного теста и в архивных ТЗ/комментариях, как и ожидается). Контрактный тест привязан к тексту самогоPROCESS.md, а не к хардкоду номеров — переименование пункта канона без синхронной правкиRULESуронит его. - AC2. Шаг «Опубликовать документ»
_ship-review.ymlветвится поMODE:nightly→ заголовокdocs: nightly ship review <база>-dev-<sha12>, тело «Ночное пакетное ревью…»,Issue: #727; иначе — бета-текст иIssue: #696без изменений. Оба собираютсяecho-построчно в файл, новый heredoc не добавлен. Тест#748 AC2вpublish-push-refusal.test.mjsгоняет это на настоящем bash+git-сэндбоксе и сверяет итоговое сообщение коммита вdevпобайтово — зелёный. - AC3. PROCESS.md §10.4 п.4 переписан текстом, который описывает
фактический код (построчный
echoв файл для сообщений коммитов, текст из скрипта для комментариев/сводок, heredoc только в непеределанных старых шагах, ссылки на #723/#730). Четыре места теперь называют оба документа ship-ревью — ночной и бета; строки перепроверены напрямую:grep -n "пакетное ревью ship — ночью" .github/workflows/_process.yml→:1747,grep -n "SHIP-REVIEW-<база>-dev" docs/process/AUTHOR.md docs/process/REVIEWER.md AGENTS.md→AUTHOR.md:249,REVIEWER.md:183,AGENTS.md(в составе единого абзаца §5 описанияship). Это синхронизация текста с уже реализованным поведением #727 (scripts/ship-review.mjs:NIGHTLY_TAG,nightlyDocPath, ветвлениеmode === 'nightly'), а не описание чего-то нового — проверено чтением исходного кодаship-review.mjs, который #748 не трогает. Машинные маркерыhp:ship-merge/hp:ship-riskне тронуты; единственный оставшийся heredoc шага решения по вердикту (test/process-track.test.mjs:1001) — prior art вне скоупа задачи, как и зафиксировано в ТЗ. - Коммит один содержательный (
945377aa), трейлерыIssue: #748,User-Visible: noна месте; changelog не тронут и не должен быть — правка не видна пользователю продукта. Второй коммит (39521011) — публикация документа r1, тоже с корректными трейлерами. - Критерии §5 маршрута: сложность/риск низкие, одна поверхность (конвейер
ревью/процесса), миграции конфигов нет, нового UX-контракта нет,
производительности и touch не касается, ожидаемое поведение уже
зафиксировано в PROCESS.md/AGENTS.md и коде #517/#723/#727/#729/#730 —
маршрут
fix. - Ребейз был декларативно «без конфликтов» (сообщение автора) — подтверждено
косвенно: единственный файл, где могло возникнуть тихое (не конфликтное)
расхождение чисел при текстовом ребейзе —
mutation-browser-guards.md— и ровно там расхождение нашлось (см. находку выше); во всех остальных файлах диффа текст и код согласованы без следов драфта.
Чего не проверял
npx tsc --noEmit,npm test,npm run buildотдельно не гонял — Validate на точном SHA материала (39521011) зелёный, сверено напрямую (gh run view 36884081445 --json headSha,conclusion). Частично перепроверено независимо через точечныеnode --testпрогоны выше.actionlintпо изменённым workflow-файлам (_process.yml,_ship-review.yml) не гонял локально (инструмент не установлен в этом окружении) — полагаюсь на отчёт автора («actionlint чистый») и на то, что job «Предполёт» Validate не упал на этом SHA.- Браузерные смоки,
golden:verify,python -m pytest,npm run invariants— не гонял:smoke-selectподтвердил отсутствие исполняемого frontend-диффа, diff не содержит ни одного файлаsrc/**,custom_components/houseplan/**или геометрии; в АС они не названы. JobsHassfest,HACS,Бэкенд: pytest,Геометрия: parity,Перф-смок,Смоки в браузере,Goldenв самом Validate-прогоне пропущены (не запускались) — диффа, который бы их включил, нет. - Performance-профили — не названы в АС, не гонял.
- Дефект пробуждения раунда (маркер прошлого прогона ждал старый SHA, упомянутый автором 2026-10-01 15:56) — вне материала этого ревью: это сбой конвейера, не код задачи; автор сообщил, что заведёт отдельно, я это не перепроверял.
- Аннотация Validate про «запас бюджета 184 Б меньше порога» — не относится
к #748: диффа в
src/**/бандле в задаче нет, это фоновый долг #367/#474.
Критерии §5 (route)
Все шесть критериев пройдены: сложность/риск низкие (пять текстовых правок и
снятие мёртвой проверки), одна поверхность (конвейер ревью/процесса), без
миграции конфигов, без нового UX-контракта, без влияния на perf/touch,
ожидаемое поведение уже зафиксировано в PROCESS.md/коде #517/#723/#727/#729/
#730. route: fix.
Вердикт: зелёный. Единственная находка раунда — Low, принятая и снятая самим ревьюером с записью (та же категория, что и Low r1), цикла не открывает.
Материал раунда
- Ветка:
issue/748-canon-reconcile, коммит395210117e24— ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. - Дерево материала:
313b4ed375a38af88c5c0c30cb67a7a52090aa2bgit log --all --format='%H %T' | grep 313b4ed375a3 - Тело issue:
f13eeaa6a21cf6847ca3050e8cdf314c293abd2796714243f1800f224b9e5000 - Вердикт конвейера:
green· High 0 · маршрутfix