From 9811674195755a54eb193ded2c706cb23a8dde4d Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:07:55 +0000 Subject: [PATCH] docs: review document for #424 Issue: #424 User-Visible: no --- docs/reviews/CODE-REVIEW-424-r2.md | 185 +++++++++++++++++++++++++++++ 1 file changed, 185 insertions(+) create mode 100644 docs/reviews/CODE-REVIEW-424-r2.md diff --git a/docs/reviews/CODE-REVIEW-424-r2.md b/docs/reviews/CODE-REVIEW-424-r2.md new file mode 100644 index 00000000..1a465be5 --- /dev/null +++ b/docs/reviews/CODE-REVIEW-424-r2.md @@ -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=` без `--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. Все дешёвые +гейты прогнаны лично и зелёные, целевые тесты и оба мутанта проверены +исполнением с патчем. + +--- + + + +## Материал раунда + +- Ветка: `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 + ```