mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-03 21:28:59 +00:00
claude[bot]
@@ -0,0 +1,185 @@
|
||||
# Код-ревью #424 — заход r2
|
||||
|
||||
Вердикт: зелёный · заход r2 · блокирующих циклов 1/4 · High: 0 · Medium: 0
|
||||
|
||||
## Почему разбор полный, а не по дельте
|
||||
|
||||
Вердикт r1 (жёлтый, Medium: ТЗ отсутствует в HEAD) получен на ветке с
|
||||
коммитами `9ef559bd` (правка) / `8003f3da` (приёмка кадров). Проверка:
|
||||
|
||||
```
|
||||
$ git cat-file -t 8003f3da
|
||||
fatal: Not a valid object name 8003f3da
|
||||
```
|
||||
|
||||
SHA r1 не резолвится вообще — он не просто не предок текущего `HEAD`, он не
|
||||
существует в текущей истории репозитория. Это подтверждает то, что написано в
|
||||
переписке issue: между r1 и этим заходом ветка пережила **второй** ребейз
|
||||
(комментарий «Ревью не запускалось» на конфликте `docs/images/06-device-editor.png`
|
||||
+ `screenshots.json`, прогон `33670260214`) — тот же класс события, что уже
|
||||
приводил к потере ТЗ в первом ребейзе. После такого ребейза это, по формуле
|
||||
§7.2, другой код: связность коммитов, на которых была доказана каждая находка
|
||||
r1, не сохранилась. Поэтому весь материал разобран заново, а не только
|
||||
Medium-находка r1.
|
||||
|
||||
Текущий диапазон: `origin/dev..HEAD`, 3 коммита:
|
||||
|
||||
```
|
||||
09c41000 fix: pin the compositor so a frame depends only on the commit (#424)
|
||||
8de9ea00 docs: restore the capture determinism spec lost in the rebase (#424)
|
||||
921d0f4c docs: accept the screenshots taken with the pinned compositor (#424)
|
||||
```
|
||||
|
||||
Трейлеры `Issue: #424` / `User-Visible: no` на месте во всех трёх; changelog
|
||||
не требуется и не тронут — согласовано.
|
||||
|
||||
## Скоуп
|
||||
|
||||
14 файлов, класс B (`demo/**`, `test/**`) + C/D (`docs/images/**`,
|
||||
`docs/specs/**`, `docs/TESTING.md`) + правка `scripts/mutation-gate.mjs`:
|
||||
|
||||
- `demo/docs/browser-args.mjs` — новый чистый модуль, `DETERMINISTIC_ARGS` с
|
||||
пятью флагами (три старых + `--disable-partial-raster` +
|
||||
`--run-all-compositor-stages-before-draw`).
|
||||
- `demo/docs/capture.mjs` — импортирует набор из модуля вместо локальной
|
||||
константы.
|
||||
- `test/capture-determinism-args.test.mjs` — 4 юнит-теста.
|
||||
- `scripts/mutation-gate.mjs` — 2 новых мутанта.
|
||||
- `docs/TESTING.md` — раздел про флаги.
|
||||
- `docs/specs/424-capture-determinism.md` — восстановленное ТЗ (198 строк).
|
||||
- `docs/images/*.png` (7 файлов) + `screenshots.json` — пересъёмка и приёмка.
|
||||
|
||||
Продуктовый код (`src/**`) не тронут — это подтверждено и `smoke-select.mjs`
|
||||
(ниже), и самим диффом.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Зелёного Validate на `921d0f4c` нет, поэтому все дешёвые гейты прогнаны лично:
|
||||
|
||||
| Гейт | Результат |
|
||||
|---|---|
|
||||
| `npx tsc --noEmit` | чисто |
|
||||
| `npm test` | 1753 pass / 0 fail / 1 skipped (1754 всего) |
|
||||
| `npm run build` | успешно |
|
||||
| `npm run bundle:sync` | синхронизировано, `git status` после — чисто (дрейфа между `dist`, `custom_components/houseplan/frontend`, `demo/srv/assets` нет) |
|
||||
| `node scripts/check-docs.mjs` | «Documentation checks passed (7 files, 10 external links)» |
|
||||
| `node scripts/process-gate.mjs --issues` | пройден; 1 warning — «инфраструктурный диапазон (#118): файлов класса A нет, статусная метка issue не требуется» (ожидаемо для чистого B/C/D диапазона) |
|
||||
| `node --test test/capture-determinism-args.test.mjs` | 4/4 pass |
|
||||
| `node scripts/mutation-gate.mjs --id=capture-allows-partial-raster` | чистый прогон ok → мутант пойман, «тест покраснел, как обязан» |
|
||||
| `node scripts/mutation-gate.mjs --id=capture-draws-before-compositor-settles` | то же, поймано 1 из 1 |
|
||||
| `node scripts/smoke-select.mjs --base origin/dev --head HEAD` | «Исполняемого frontend-диффа нет (`src/**/*.ts` не тронут)», browser-smoke не выбираются |
|
||||
|
||||
Дисциплина «тест умеет падать» проверена лично на обоих новых мутантах, а не
|
||||
принята со слов автора: `--id=<mutant>` без `--check` реально накладывает патч
|
||||
и перезапускает guard.
|
||||
|
||||
Дополнительно, для AC4 (приёмка кадров), сверил манифест с диском построчно
|
||||
(sha256 каждого из 10 файлов в `docs/images/*.png` против `imageSha256` в
|
||||
`screenshots.json`) — совпадение побайтовое по всем десяти, включая 7
|
||||
изменившихся и 3 нетронутых.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **Medium r1 закрыт по существу.** `docs/specs/424-capture-determinism.md`
|
||||
присутствует в HEAD (коммит `8de9ea00`), 198 строк, содержимое — финальная
|
||||
r2-редакция спек-ревью: AC5 несёт числовой порог 15% с предварительными
|
||||
замерами (11711/11595 мс без флагов vs 11589/11567 мс с флагами, строки
|
||||
131–138), план тестов описывает вынос в `browser-args.mjs` вместо
|
||||
`readFileSync`-разбора (строки 144–152). Это то же содержимое, которое
|
||||
спек-ревью r2 объявило зелёным — не пересобрано по памяти заново, проверено
|
||||
чтением файла, а не по слову автора.
|
||||
- **AC2 (решение закреплено тестом).** `test/capture-determinism-args.test.mjs`
|
||||
проверяет оба новых флага, сохранность трёх старых (#410) и то, что
|
||||
`capture.mjs` берёт набор из модуля, а не объявляет свой (`readFileSync` +
|
||||
regex по исходнику `capture.mjs`, без импорта самого скрипта — ровно то
|
||||
решение, которое r1 спек-ревью потребовало явно назвать).
|
||||
- **AC3 (отрицательный прогон).** Оба мутанта существуют, у каждого свой
|
||||
guard (`node --test test/capture-determinism-args.test.mjs`) и текст
|
||||
`because`, оба лично прогнаны и поймали свою мутацию.
|
||||
- **AC4 (приёмка).** 7 изменившихся файлов (`01,02,03,04,05,06-editor,
|
||||
06-display-preview`) точно совпадают со списком `acceptance.declared` (7
|
||||
записей); `witnesses: 3 ≥ floor: 1` — порог не обойдён. Хеши в манифесте
|
||||
побайтово совпадают с файлами на диске (проверено sha256 по всем 10
|
||||
сценариям).
|
||||
- **AC5 (перф, запись в репозитории).** Порог 15% и числа записаны теперь и в
|
||||
ТЗ, и неявно подкреплены `docs/TESTING.md`. Сам замер не переснимал — числа
|
||||
не зависят от кода этой ветки (они про Chromium/композитор), а порог
|
||||
проверке подлежит только при регрессии; здесь регрессии нет и сам факт
|
||||
фиксации числа в репозитории (то, чего не хватало в r1) подтверждён чтением.
|
||||
- **AC6 (golden не затронут).** `grep` по `demo/golden/**` на упоминания
|
||||
`capture.mjs` / `browser-args.mjs` — ноль совпадений; `demo/golden/**` в
|
||||
диффе не участвует вообще (`git diff --stat` подтверждает). Структурной
|
||||
связи нет, полный прогон 153 сценариев не потребовался — проверено чтением,
|
||||
не исполнением, тем же методом, каким это подтвердило r1 (там факт был
|
||||
установлен уже безотносительно ребейза, ребейз кода golden не касался).
|
||||
- **Комментарий в `browser-args.mjs`** соответствует коду: называет причину
|
||||
каждого флага и ссылку на #424/#410, не расходится с телом ТЗ.
|
||||
- **`capture.mjs`** — старый комментарий-обоснование трёх флагов #410
|
||||
сохранён, добавлена ссылка на новый модуль вместо задвоения текста; код
|
||||
использует `DETERMINISTIC_ARGS` в единственном месте (`launch(...)`, строка
|
||||
245–246).
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка r1 | Чем закрыта | Где это видно |
|
||||
|---|---|---|
|
||||
| Medium: `docs/specs/424-capture-determinism.md` отсутствовал в HEAD (`8003f3da`), утверждённая r2-редакция ТЗ нигде не резолвилась в дереве | Коммит `8de9ea00 docs: restore the capture determinism spec lost in the rebase (#424)` возвращает файл целиком | `docs/specs/424-capture-determinism.md` присутствует в HEAD, 198 строк, содержит числовой порог AC5 (15%, строки 131–138) и абзац про `browser-args.mjs` в плане тестов (строки 144–152) — то есть это подтверждённо r2-, а не r1-редакция |
|
||||
|
||||
Других находок в r1 не было (High: 0, Medium: 1 — единственная, закрыта выше).
|
||||
|
||||
## Унаследовано из r1
|
||||
|
||||
Формально ничего не унаследовано без повторной проверки: ребейз, случившийся
|
||||
между r1 и этим заходом, делает исходный SHA r1 нерезолвящимся (см. раздел
|
||||
выше), поэтому весь материал в этом документе проверен заново в этом раунде, а
|
||||
не принят на основании вывода r1. Содержательные выводы r1 (AC2/AC3/AC4/AC6
|
||||
реализованы корректно) совпали с результатом независимой повторной проверки —
|
||||
это не наследование, а воспроизведённый результат.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- **AC1 (кросс-прогонный гейт, восемь идентичных прогонов).** Гейт
|
||||
`scripts/capture-determinism.mjs` живёт в ветке #422 и в этом диапазоне
|
||||
(`origin/dev..HEAD`) физически отсутствует — ТЗ прямо относит его к
|
||||
«не-скоупу», объединяемому при мерже обеих задач. Проверить AC1 в изоляции
|
||||
этой ветки нечем; это ограничение задачи, а не пропуск с моей стороны.
|
||||
- **`npm run golden:verify` (153 сценария) исполнением.** Не запускал — diff
|
||||
не содержит правок `demo/golden/**`, и структурной связи с изменённым кодом
|
||||
нет (см. AC6 выше, проверено чтением/grep). Полный прогон непропорционален
|
||||
диапазону, где `demo/golden/**` не изменился ни байтом.
|
||||
- **Повторный замер производительности AC5.** Не переснимал 11.7с/11.6с
|
||||
замеры — они не зависят от диффа этой ветки (флаги и код те же, что были
|
||||
измерены автором), а порог 15% далёк от границы. Число теперь зафиксировано
|
||||
в репозитории, чего и требовала находка r1.
|
||||
- **`npm run invariants`.** Diff не трогает геометрию модели (комнаты, стены,
|
||||
`layout`, `marker.space`, `open_spans`) — инструмент неприменим к этой
|
||||
задаче.
|
||||
- **`python -m pytest tests_backend -q`.** `custom_components/**/*.py` в
|
||||
диффе не участвует.
|
||||
- **Полный набор `demo/smoke_*.mjs`.** `smoke-select.mjs` подтвердил: диапазон
|
||||
не трогает `src/**`, browser-smoke этим диффом не выбираются — прогон всего
|
||||
набора не пропорционален задаче.
|
||||
|
||||
## Итог
|
||||
|
||||
Единственная Medium-находка r1 закрыта содержательно, подтверждено чтением
|
||||
файла, а не заявлением автора. Полный (не только по находке) разбор,
|
||||
вызванный вторым ребейзом ветки, не выявил новых High/Medium. Все дешёвые
|
||||
гейты прогнаны лично и зелёные, целевые тесты и оба мутанта проверены
|
||||
исполнением с патчем.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/424-capture-determinism`, коммит `921d0f4c0d83` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `17a366e81754945c9041705a22246f55f0d33a85`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 17a366e81754
|
||||
```
|
||||
- ТЗ `docs/specs/424-capture-determinism.md`, блоб `9eab318dfa8306d74763c292fa7db70e6621fdab`
|
||||
```
|
||||
git log --all --find-object=9eab318dfa8306d74763c292fa7db70e6621fdab -- docs/specs/424-capture-determinism.md
|
||||
```
|
||||
Reference in New Issue
Block a user