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

22 KiB
Raw Permalink Blame History

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/** или геометрии; в АС они не названы. Jobs Hassfest, 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 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: 313b4ed375a38af88c5c0c30cb67a7a52090aa2b
    git log --all --format='%H %T' | grep 313b4ed375a3
    
  • Тело issue: f13eeaa6a21cf6847ca3050e8cdf314c293abd2796714243f1800f224b9e5000
  • Вердикт конвейера: green · High 0 · маршрут fix