20 KiB
CODE-REVIEW-422-r1
- Issue: https://github.com/Matysh/houseplan-card/issues/422
- Ветка:
issue/422-capture-and-anchor-gates - SHA материала (сверен
git rev-parse HEADперед выводом итогов):653b9a763294cf3c88537bf129e0d8f8f4e6e9c4 - Заход: r1 (первый код-ревью раунда, разбор полный — дельта по §2.10 не применяется)
- Класс изменений: только B (
.github/**,demo/**,scripts/**,test/**) — продуктового кода (src/**,custom_components/**/*.py) нет - ТЗ:
docs/specs/422-capture-and-anchor-gates.md, ревью ТЗ — зелёное,docs/reviews/SPEC-REVIEW-422-r1.md
Скоуп
Две независимые правки гейтов, обе из аудита v1.70.0:
- Гейт стабильности съёмки документации мерил три снимка внутри одного
процесса и был бы зелёным на дефекте #410 (обрезка кадра плавала
между прогонами). Добавлен кросс-прогонный гейт
scripts/capture-determinism.mjs— снимает набор дважды в разных процессах и сравнивает хеши; старая проверка--stability=3сохранена. Целочисленная обрезка (demo/docs/clip.mjs) вынесена в чистую функцию и покрыта юнитом. - Живость якоря материала ревью-документа (
scripts/review-doc-guard.mjs) проверяласьgit cat-file -e— наличием объекта в локальной базе, а не достижимостью. Заменено наanchorLiveness(): достижимость отrefs/remotes/originи тегов, той же командой, что печатается читателю.
Ветка ребейзнута на dev после мержа #424 (починка компоситора, была
блокирующей зависимостью — конвейер съёмки не мог быть введён красным по
известной причине). После ребейза дерево пересъёмки дало 10/10 совпадений,
изменился только отпечаток скрипта в манифесте.
Как проверялось
| Гейт | Команда | Результат |
|---|---|---|
| Typecheck | npx tsc --noEmit |
чисто, без вывода |
| Unit-тесты | npm test |
1765 pass / 0 fail / 1 skipped (1766 всего) — совпадает с заявленным автором числом |
| Build + сверка бандлов | npm run build && npm run bundle:sync, cmp трёх копий |
dist = custom_components/houseplan/frontend = demo/srv/assets, побайтово |
check-docs.mjs |
node scripts/check-docs.mjs |
КРАСНЫЙ — см. находку H1. На origin/dev тот же скрипт зелёный (проверено отдельным git worktree) |
| Новые юниты | node --test test/capture-clip.test.mjs, node --test test/review-doc-guard.test.mjs |
5/5 и 24/24 pass |
| Оба мутанта (умеют падать) | node scripts/mutation-gate.mjs --id=capture-drifts-between-runs, --id=anchor-liveness-ignores-reachability |
оба «покраснел, как обязан», поймано 1 из 1 |
| Реестр мутантов согласован с кодом | node scripts/mutation-gate.mjs --check |
оба новых якоря (capture-drifts-between-runs, anchor-liveness-ignores-reachability) применились чисто, FAIL нет |
| Новый кросс-прогонный гейт целиком, вживую | node scripts/capture-determinism.mjs (реальный Chromium, реальная съёмка дважды) |
зелёный: «съёмка воспроизводима: 10 кадров совпали между прогонами» — рабочее дерево после этого возвращено git checkout -- docs/images/ |
| CI на этом SHA | — | нет зелёного прогона; реальный workflow-run 33672132518 на этом же SHA — failure, job «Предполётные проверки» падает на шаге check-docs.mjs --external с той же ошибкой, что я воспроизвёл локально |
Не прогонялось и почему:
- Браузерные
demo/smoke_*.mjs— diff не трогаетsrc/**;node scripts/smoke-select.mjs --base origin/dev --head HEADподтверждает: «Исполняемого frontend-диффа нет… Browser-smoke этим диффом не выбираются». npm run golden:verify— визуальный результат карточки не меняется (diff не трогаетsrc/**, скриншоты документации — не golden-эталоны продукта).python -m pytest tests_backend -q— Python не тронут.node scripts/model-invariants.mjs— геометрия/layout/толщина стен/marker.space/open_spansне затронуты.- Полный
docs-screenshots.ymlв CI (workflow_dispatch) — недоступен из ревью-сессии; вместо этого прогнан эквивалент локально (node scripts/capture-determinism.mjsвживую) и сверен с реальным прогоном автора на CI (см. таблицу выше).
Находки
High H1 — коммит 653b9a76 кладёт в манифест неверный captureScriptSha256, гейт docs красный на этом SHA
docs/images/screenshots.json после коммита «docs: refresh the capture
fingerprint after the clip extraction» хранит
captureScriptSha256: 12eb99bc095f…. Реальный хеш файла demo/docs/capture.mjs
на этом дереве — cadb8e1bcab9… (git show HEAD:demo/docs/capture.mjs | sha256sum).
Значение 12eb99bc… — это хеш capture.mjs до выноса wholePixelClip в
clip.mjs (совпадает с версией файла на коммитах 70805c9c/8ed8ecc3,
предшествующих рефакторингу в d59ceee9). То есть коммит, который по
сообщению должен был «обновить отпечаток после выноса обрезки», записал
отпечаток до него — похоже на пересъёмку со старой копии скрипта до
финального ребейза, зафиксированную посмертно.
Воспроизведено дважды, независимо:
node scripts/check-docs.mjs
→ ERROR screenshot capture script changed; run npm run build && node demo/docs/capture.mjs
и тем же образом в живом CI на этом самом SHA — прогон 33672132518, job «Предполётные проверки: документация, провенанс, процесс», шаг «Документация: гайды, ченджлоги, скриншот-индекс»:
ERROR screenshot capture script changed; run npm run build && node demo/docs/capture.mjs
##[error]Process completed with exit code 1.
Контрольная проверка: тот же check-docs.mjs на origin/dev (отдельный
git worktree) — зелёный, «Documentation checks passed». Дефект вносится
именно этим диффом, не окружением.
Сами PNG и их imageSha256 в манифесте совпадают с байтами на диске — порча
только в одном поле (captureScriptSha256). Это docs — один из
обязательных job'ов validate.yml (AGENTS.md, список #191); красный он не
может уйти в S8-merged по прецеденту #230/#234.
Чем чинится: пересъёмка на финальном дереве, npm run build && node demo/docs/capture.mjs, коммит обновлённого docs/images/screenshots.json
(и, если изменятся байты кадров — самих PNG).
Medium M1 (в скоупе) — AC8 не доказан: замера времени конвейера нет ни в issue, ни в коммитах
ТЗ явно требует: «Конвейер не стал заметно дольше… Доказательство: замер до и
после, разница названа в issue числом». Ни в одном комментарии issue #422, ни
в сообщениях коммитов такого числа нет — только общее «оба гейта готовы» и
последующий разбор дефекта #424. Единственная цифра, которая нашлась вживую —
общая длительность одного CI-job'а без разбивки на «до» и «после»
(Съёмка и сверка скриншот-индекса, 76 c, прогон
33657466605) —
это не сравнение, а единственная точка. Риск в самом ТЗ («второй прогон
удваивает время шага») признан явно и там же названо смягчение («звести к
нетривиальным сценариям, если удвоение дорого») — но без цифры невозможно
понять, нужно ли смягчение вообще.
Medium M2 (в скоупе) — комитченный мутант AC1/AC2 не проверяет сам новый гейт; frameHashes/driftBetweenRuns без юнитов
AC1 требует доказательства, что кросс-прогонный гейт красный на мутации,
«моделирующей #410» (сдвиг обрезки, постоянный в процессе, разный между
прогонами); AC2 — что та же мутация оставляет старый --stability=3
зелёным, и оба факта доказываются «мутантом в scripts/mutation-gate.mjs,
прогнанным штатным раннером».
Комитченный мутант capture-drifts-between-runs
(scripts/mutation-gate.mjs:960-971) патчит demo/docs/clip.mjs
(Math.floor → Math.round) и охраняется test/capture-clip.test.mjs —
это прямое попадание в AC4 (целочисленная обрезка), но не в AC1/AC2: он не
трогает и не запускает ни scripts/capture-determinism.mjs, ни
--stability=3. У самого нового гейта (frameHashes, driftBetweenRuns в
scripts/capture-determinism.mjs) нет ни одного прямого юнита и ни одного
мутанта — единственная проверка их логики это полный браузерный прогон,
которого в цикле реализации не бывает (§8).
Живое AC1-доказательство есть — реальный дефект #424 (не синтетическая
мутация) на этой же ветке дал кросс-прогонный гейт красным на CI, теми же
хешами что и в песочнице автора (прогон 33657466605). Но AC2 (что старый
--stability=3 в этот момент остаётся зелёным) не проверено даже так: в
том прогоне шаг --stability=3 стоял после упавшего кросс-прогонного и
был помечен skipped, а не success. Разрыв «--stability стабильно зелёный
на этом классе дефектов» держится только на историческом знании из #410, не
на прогоне в этой задаче.
Предложение (не обязывающее, автор волен решить иначе): либо добавить прямой
юнит на frameHashes/driftBetweenRuns (это чистые функции — дёшево), либо
переименовать/дополнить формулировку AC1/AC2 «доказательство», раз реальное
доказательство идёт через живой инцидент #424, а не через мутант.
Low L1 (снято с записью) — комментарий в docs-screenshots.yml про «третий прогон»
Комментарий у шага --stability=3 («снимать набор третий раз ради порядка
строк в логе незачем») предполагает, что читатель уже понимает, что перед ним
второй набор кадров, а не третий прогон капчура в буквальном смысле счёта
запусков (capture-determinism.mjs = 2 прогона, затем --stability = ещё 3
внутрипроцессных снимка на сценарий). Формально AC9 («комментарий
соответствует фактическому порядку шагов») выполнен — порядок шагов описан
верно, спор только о том, что «третий» считает по-разному в двух местах
одного комментария. Не блокирует, автор может уточнить при следующей правке
файла или оставить как есть.
Проверено и корректно
- AC3 (проверка «не плавает внутри одного состояния» сохранена) —
проверено чтением
demo/docs/capture.mjs: код веткиSTABILITY_SHOTS(сравнение черезcomparePairs,continueдо записи файла) не тронут диффом, только вынесен комментарий. - AC4 (целочисленная обрезка) — доказано автотестом
(
test/capture-clip.test.mjs, 5/5), тест умеет падать: мутантcapture-drifts-between-runsкрасит именно его. - AC5/AC6/AC7 (достижимость якоря, а не наличие; старое поведение #414 не
сломано; дерево/блоб/тип через
cat-file -t) — доказано автотестом (test/review-doc-guard.test.mjs, новые 8 тестов из 24, все зелёные), тест умеет падать (мутантanchor-liveness-ignores-reachability). Дополнительно прочитан кодanchorLiveness()и вручную проверены обе git-команды (--find-objectбез пути,--format=%T) на текущем репозитории — обе ведут себя так, как описано в коде. - Область поиска —
refs/remotes/origin+теги, не--all— отдельный тест подтверждает («областью поиска служат origin и теги, а не --all»); расхождение с командой, которую видит человек вmaterialAnchorBlock(там всё ещё--all), уже разобрано и принято ревьюером ТЗ в r1 (--all— надмножество, ложноотрицательных не даёт) — не пересматриваю, дельта диффа этого места не касается. - AC1 (позитив) — доказано исполнением реального браузерного гейта дважды: на исправленном дереве (10/10 кадров совпали) и, по историческим данным CI этой же ветки, на неисправленном (до #424) — гейт был красным с тем же «отпечатком» дефекта, что и исходный #410 (пиксели на сглаженных границах).
- AC9 — проверено чтением
.github/workflows/docs-screenshots.yml: фактический порядок —capture-determinism.mjs→--stability=3→ «Хеши кадров» → «Вердикт»; комментарий над каждым шагом соответствует (кроме формулировки L1). - Трейлеры всех четырёх коммитов (
Issue: #422,User-Visible: no) на месте; changelog не тронут — верно, видимого поведения продукта нет. - Класс файлов — только B, инфраструктурный маршрут с файлом ТЗ уже принят ревью ТЗ r1 со ссылкой на прецеденты (#398, #399, #404) — не пересматриваю.
- Три копии бандла (
dist,custom_components/houseplan/frontend,demo/srv/assets) синхронны побайтово.
Чего не проверял
- Полный
docs-screenshots.ymlчерезworkflow_dispatchна этом SHA — нет прав на диспетчеризацию из ревью-сессии; опираюсь на прогон автора (33657466605, до фикса #424) и собственный локальный прогон эквивалентного шага. python -m pytest tests_backend— diff не касаетсяcustom_components/**, пропущено по правилу «нужно только если тронут бэкенд».- Полный
npm run golden:verify— не запускал: diff не меняет визуальный результат карточки (только инструмент съёмки документационных скриншотов), а golden-эталоны продукта отдельны отdocs/images/screenshots.json. - Производительность съёмки как таковая — числа нет ни у автора, ни у меня; см. M1.
Материал раунда
Материал раунда
- Ветка:
issue/422-capture-and-anchor-gates, коммит653b9a763294— ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. - Дерево материала:
f0b4c84fbe45cfa5d0e7be19af9387b0d3a79e62git log --all --format='%H %T' | grep f0b4c84fbe45 - ТЗ
docs/specs/422-capture-and-anchor-gates.md, блоб797df838584f268fb2f1e99a41b5dad15088d698git log --all --find-object=797df838584f268fb2f1e99a41b5dad15088d698 -- docs/specs/422-capture-and-anchor-gates.md