houseplan-card/docs/reviews/CODE-REVIEW-571-r1.md

CODE-REVIEW — issue #571, заход r1

Материал: f77bdaf52eb1f5b8cec205d55eaf58e17e3ba288, ветка issue/571-golden-capture-provenance, два коммита на origin/dev (3f95dac4, f77bdaf5). Инфраструктурный трек: класс A (src/**, custom_components/houseplan/**/*.py, манифесты) не тронут ни строкой — проверено git diff origin/dev...HEAD --name-only с фильтром по этим путям, список пуст.

Скоуп

Проблема из аудита: demo/golden/accept.mjs не знал, на какой платформе сняты кадры, и записывал в baselines-index.json свою платформу под именем platform — что и дало на ad4000f9 ложь "platform": "win32" у кадров Linux-прогона 34853080375. Задача переносит провенанс съёмки в отчёт и разводит capturedOn/ acceptedOn в индексе. Продуктового поведения не меняет, docs/CHANGELOG* не трогает — согласуется с User-Visible: no в обоих коммитах.

Как проверялось

1. Прочитан весь diff (git diff origin/dev...HEAD) по файлам: scripts/capture-environment.mjs, demo/golden/policy.mjs, demo/golden/accept.mjs, demo/golden/run.mjs, scripts/mutation-registry.mjs, test/capture-environment.test.mjs, test/golden-capture-provenance.test.mjs, docs/images/screenshots.json, бандл (dist/**, custom_components/houseplan/frontend/**).
2. Прослежена логика accept.mjs построчно для всех пяти сценариев приёмки из тела issue (см. таблицу ниже) — доказательство чтением плюс сверка с автотестами, которые эти же сценарии исполняют.
3. Тесты не просто прочитаны — прогнаны лично: node --test --test-name-pattern="#571" test/golden-capture-provenance.test.mjs → 7 pass / 0 fail. Тесты запускают настоящий accept.mjs в изолированном каталоге эталонов (--baselines=<tmp>), не подменяют модуль.
4. Дисциплина «тест умеет падать» проверена сама, не со слов автора. Вручную внесены обе мутации из scripts/mutation-registry.mjs (сохранял и восстанавливал файлы, рабочее дерево осталось чистым — git status --short пуст после проверки):
   • demo/golden/accept.mjs: capturedOn: capturedOn, → capturedOn: acceptance.platform, — упали 2 из 7 тестов (AC1 и «чужая среда с причиной»);
   • scripts/capture-environment.mjs: fail-closed throw в reportCaptureProvenance заменён на return { provenance: null, legacy: true } — упал целевой тест #571 схема 2 fail-closed…. Оба мутанта убиты, ровно теми тестами, что названы в guard соответствующей записи реестра.
5. Сверены неизменные инварианты: imageSha256 во всех 11 записях docs/images/screenshots.json не менялся (git diff ... | grep -E '^[+-].*imageSha256' пусто) — изменился только sourceFingerprint/sourceSha256, что подтверждает заявление «пересъёмка не потребовалась, картинки байт-в-байт совпали». Аналогично не менялись demo/golden/baselines/** (нет ни одного файла в диффе) — PNG эталоны не трогались, поэтому трейлеры Release:/Baseline-Reviewed: не требуются.
6. node scripts/smoke-select.mjs --base origin/dev --head HEAD → «Исполняемого frontend-диффа нет… Browser-smoke этим диффом не выбираются». Смоки не прогонялись — выбирать нечего, диффа в src/** нет.
7. Проверено, что docs-accept.mjs (приёмка скриншотов документации) не задета и не несёт аналогичного дефекта: там нет поля capturedOn, есть только acceptedOn (платформа приёмщика, честно так и названная) — предположение автора о возможном родственном разрыве не подтвердилось, отдельный issue не нужен.
8. Проверены трейлеры обоих коммитов (git log): Issue: #571, User-Visible: no — оба присутствуют, changelog не тронут, что для no корректно.

Сценарии приёмки из issue — прослежены по коду

#	Сценарий	Где в коде	Вердикт
1	Linux-отчёт принят на Windows с причиной: capturedOn=linux, acceptedOn=win32, причина в манифесте	accept.mjs:45-75, цикл по [capturedOn, acceptance.platform] + foreignAllowed	подтверждено тестом #571 AC1: чужая среда съёмки с причиной
2	Тот же сценарий без причины — отказ до записи	тот же цикл, throw до mkdirSync(baselineRoot)/writeFileSync	подтверждено тестом #571 AC2, включая проверку «индекс не тронут»
3	Same-platform без override, обе стороны корректны	foreignAllowed вычисляется как null, когда обе платформы — канон	подтверждено тестом #571 AC1: артефакт Linux принимается
4	Тампер report/PNG и неполный артефакт — fail-closed	не менялось (sourceFingerprint-проверка и goldenAcceptanceRefusal)	подтверждено тестом #571 подмена PNG и неполный артефакт
5	npm test; адресный тест policy/acceptance	см. п.3 выше плюс общий Validate	зелёный прогон на этом SHA (см. ниже)

Гейты — что прогнано, что нет и почему

• Не прогонял заново npx tsc --noEmit, npm test (полный), npm run build со сверкой бандла: Validate на f77bdaf5 уже зелёный (run 35270925670) — сошлись на этом прогоне как источнике для этих трёх гейтов.
• Прогнал сам, адресно: node --test --test-name-pattern="#571" test/golden-capture-provenance.test.mjs (7/7 pass) плюс обе ручные мутации (см. выше) — потому что дисциплина «тест умеет падать» касается именно тех тестов, на которые опирается вердикт, а не всего набора.
• node scripts/check-docs.mjs --strict — не перепрогонял: diff не трогает src/** (условие явно снимает обязательность), и та же проверка уже часть зелёного Validate на f77bdaf5. Косвенно подтверждено чтением: imageSha256 не изменился ни для одной из 11 сцен, значит check-docs не мог покраснеть из-за визуальных данных.
• Инварианты модели (npm run invariants) — не запускал: diff не трогает геометрию, layout, marker.space, open_spans или толщину стен. Неприменимо.
• Browser-smokes (demo/smoke_*.mjs) — не прогонял: smoke-select.mjs подтвердил отсутствие исполняемого frontend-диффа (src/** не тронут).
• npm run golden:verify / golden-джоба — не прогонял и не требовалось: PNG-эталоны не менялись, изменения — в контракте приёмки, покрытом собственными юнит-тестами, реально исполняющими accept.mjs. Автор корректно отметил, что реальная приёмка после мержа сама запишет честный индекс схемы 2.
• python -m pytest tests_backend — не прогонял: custom_components/**/*.py не тронут.
• Perf-профили — неприменимо, в AC не названы и код не тронут.
• mutation-gate --check — не перепрогонял целиком (автор указал 0 FAIL в хендоффе), но сами два новых мутанта проверил вручную (см. выше) — это и есть содержательная часть гейта для этой задачи.

Проверено и корректно

• Провенанс собирается там, где снимаются кадры (captureProvenance в scripts/capture-environment.mjs, вызывается из demo/golden/run.mjs), а не выводится приёмщиком из своего process.platform — ровно то, что требует AC2 issue.
• reportCaptureProvenance — честный fail-closed: схема ≥2 без capture или без capture.platform бросает исключение с явной причиной; схема <2 — отдельная явная ветка legacy: true, capturedOn уезжает null, а не угадывается. Мутационно проверено.
• Отказ по чужой среде происходит до любой записи в индекс — throw расположен раньше mkdirSync(baselineRoot) и writeFileSync(manifestPath) по тексту файла; тест #571 AC2 дополнительно сверяет байт-в-байт, что индекс не изменился.
• Причина осознанного обхода (HP_ALLOW_FOREIGN_CAPTURE) едет в foreignCapture.reason индекса, а не только в stdout — закрывает второй провал из аудита («причина осталась в stdout»).
• indexCapturedOn() читает обе схемы одним правилом и не путает platform (схема 1) с capturedOn (схема 2) — юнит-тест перебирает все четыре случая, включая «схема 2 без capturedOn» → null, а не откат к чужому полю.
• Отмена запрета «run.mjs не трогать» (#455) внесена и оплачена в той же ветке: бандл пересобран (класс D, dist/** и custom_components/houseplan/frontend/** в диффе), индекс скриншотов документации переснят вторым коммитом с сохранением всех 11 imageSha256 — не переложено на будущую задачу.
• Свидетели гоняют настоящий accept.mjs, а не мок: --baselines=<dir> сделан ровно для того, чтобы «отказ до записи» была исполнимым утверждением, а не декларацией.
• Трейлеры Issue:/User-Visible: no на обоих коммитах корректны, changelog не тронут, что для no ожидаемо.

Чего не проверял (и почему это не блокирует)

• Полный npm test / tsc / build лично не гонял — Validate на этом SHA уже зелёный, дублирование было бы потерей времени без нового сигнала.
• Реальную golden-приёмку в CI (запись индекса схемы 2 «в бою») не выполнял — она в скоуп задачи и не входит, и юнит-тесты покрывают тот же код путём, включая изоляцию от рабочего дерева.
• Интеграционный путь witnessCheck.refusal → environmentNote в accept.mjs (строки с previousIndex/indexCapturedOn(previousIndex)) проверен только чтением, не исполнением: специального теста, гоняющего именно отказ по свидетелям с последующей запиской о среде, в диффе нет. Логика — прямой перенос имён (environment.platform → capturedOn, manifest.platform → indexCapturedOn(previousIndex)) без изменения формы; читается корректно, но это не доказательство исполнением. Не выношу как находку: ветка не новая (существовала и до задачи в похожем виде), риск регрессии низкий, а полноценный тест этого пути стоил бы отдельной инфраструктуры (два последовательных прогона приёмки с разными средами) непропорционально размеру задачи.

Находки

Нет ни High, ни Medium. Задача решает ровно проблему из аудита, не расширяет и не сужает скоуп, все AC подтверждены автотестами, исполненными лично, две мутации проверены руками поверх заявления автора.

Вердикт

Зелёный.

---

Материал раунда

• Ветка: issue/571-golden-capture-provenance, коммит f77bdaf52eb1 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
• Дерево материала: 8c131f1d70153bc7634264d5f83dab0b656088c4

  git log --all --format='%H %T' | grep 8c131f1d7015
  

• Тело issue: 3ca7e6df6ede172521144a672fefa1a946c0af10242125f62e6b835a399babe0
• Вердикт конвейера: green · High 0