mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,227 @@
|
||||
# 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:
|
||||
|
||||
1. Гейт стабильности съёмки документации мерил три снимка **внутри одного
|
||||
процесса** и был бы зелёным на дефекте #410 (обрезка кадра плавала
|
||||
**между** прогонами). Добавлен кросс-прогонный гейт
|
||||
`scripts/capture-determinism.mjs` — снимает набор дважды в разных
|
||||
процессах и сравнивает хеши; старая проверка `--stability=3` сохранена.
|
||||
Целочисленная обрезка (`demo/docs/clip.mjs`) вынесена в чистую функцию и
|
||||
покрыта юнитом.
|
||||
2. Живость якоря материала ревью-документа (`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](https://github.com/Matysh/houseplan-card/actions/runs/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](https://github.com/Matysh/houseplan-card/actions/runs/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](https://github.com/Matysh/houseplan-card/actions/runs/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.
|
||||
|
||||
## Материал раунда
|
||||
|
||||
<!-- material-anchors: заполняется шагом публикации -->
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/422-capture-and-anchor-gates`, коммит `653b9a763294` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `f0b4c84fbe45cfa5d0e7be19af9387b0d3a79e62`
|
||||
```
|
||||
git log --all --format='%H %T' | grep f0b4c84fbe45
|
||||
```
|
||||
- ТЗ `docs/specs/422-capture-and-anchor-gates.md`, блоб `797df838584f268fb2f1e99a41b5dad15088d698`
|
||||
```
|
||||
git log --all --find-object=797df838584f268fb2f1e99a41b5dad15088d698 -- docs/specs/422-capture-and-anchor-gates.md
|
||||
```
|
||||
Reference in New Issue
Block a user