From 3f402a348142c7b9c8e9dfe18be1ff95261576e8 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Wed, 9 Sep 2026 14:37:11 +0000 Subject: [PATCH] docs: review document for #512 Issue: #512 User-Visible: no --- docs/reviews/SPEC-REVIEW-512-r1.md | 85 ++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-512-r1.md diff --git a/docs/reviews/SPEC-REVIEW-512-r1.md b/docs/reviews/SPEC-REVIEW-512-r1.md new file mode 100644 index 00000000..84aacac6 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-512-r1.md @@ -0,0 +1,85 @@ +# SPEC-REVIEW-512-r1 + +- **Issue:** #512 — «Golden: текст версии через seam вне кадров; `docs:accept --identical` по попиксельной идентичности» +- **Этап:** ревью ТЗ (PROCESS.md §2.4) +- **Материал:** `docs/specs/512-golden-version-seam-and-docs-identical-accept.md` (полный трек, обоснование — «две поверхности», критерий §5 «одна поверхность» не пройден — обоснование корректно) + тело issue #512 + комментарий S2-аналитики (Codex, 09.09) на HEAD `e606b808`. +- **Заход:** r1 · блокирующих циклов до этого раунда: 0/4. + +## Скоуп ревью + +Оцениваю: обязательные разделы ТЗ (PROCESS §7.1), однозначность и доказуемость AC1…AC6, отсутствие непомеченных догадок, продуктовые вопросы (их не нашлось — задача invisible-to-user, `User-Visible: no`, что подтверждено кодом), техническую состоятельность контракта seam и обоих инструментов приёмки. + +## Как проверялось + +Ревью ТЗ не запускает гейты кода (правки src/scripts ещё не внесены) — проверка велась чтением ТЗ и **сверкой каждого фактического утверждения ТЗ с текущим деревом на `e606b808`**, а не на веру: + +- прочитаны `docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md` (§1, §2.2–§2.5, §4, §5, §6, §7.1, §7.2), `docs/specs/README.md`; +- сверены все точки чтения `CARD_VERSION` в `src/houseplan-card.ts` и `src/houseplan-editor-runtime.ts` (grep + чтение контекста каждой строки) против перечня §4 ТЗ — совпадение построчное; +- прочитаны `scripts/release-contract.mjs`, `scripts/source-fingerprint.mjs`, `scripts/check-docs.mjs`, `scripts/docs-accept.mjs`, `scripts/docs-acceptance.mjs`, `scripts/capture-environment.mjs`, `demo/docs/capture.mjs`, `demo/golden/harness.mjs`, `demo/golden/matrix.mjs` (участки, задействованные ТЗ); +- вычислен фактический `sha256` текущего `demo/docs/capture.mjs` и сверен с закоммиченным `docs/images/screenshots.json.captureScriptSha256` — совпадают (здоровая базовая линия, см. находку H1); +- проверено, где именно `--screenshots=strict` реально запускается в CI (`publish-prerelease.yml:73`, `validate.yml:83` через `$mode`). + +Гейты кода (`typecheck`/`test`/`build`) не прогонялись — на этапе ревью ТЗ они неприменимы: изменений в `src/**`/`scripts/**` ещё нет, менять нечего. + +## Находки + +### H1 — ТЗ не учитывает, что правка `demo/docs/capture.mjs` ломает `captureScriptSha256` и красит `docs` в `check-docs --screenshots=strict` — собственный AC3 не исполним без незаявленного шага + +**Где:** `docs/specs/512-golden-version-seam-and-docs-identical-accept.md` §6 (п.1: «новый флаг `--out`»), §6.3 (список полей, берущихся «из закоммиченного»), §11 («Затронутые файлы»), AC3. + +**Воспроизведение / доказательство:** + +1. `scripts/check-docs.mjs:211-213` держит **отдельный** от `sourceFingerprint` инвариант: + ```js + const scriptPath = resolve(ROOT, 'demo/docs/capture.mjs'); + if (manifest.captureScriptSha256 !== sha256(readFileSync(scriptPath))) + freshness.push('screenshot capture script changed; run npm run docs:capture and accept …'); + ``` + Проверил на HEAD `e606b808`: закоммиченный `docs/images/screenshots.json.captureScriptSha256` и фактический `sha256(demo/docs/capture.mjs)` **совпадают** — это здоровая текущая база, а не случайность. +2. `scripts/source-fingerprint.mjs:64-75` (`fingerprintFiles`) сканирует только `src/**`, `demo/fixtures/**/*.mjs`, `demo/golden/**/*.mjs` (минус `POST_CAPTURE_INPUTS`) и `BUILD_INPUTS`. **`demo/docs/**` в этот корпус не входит вовсе** — поэтому `sourceFingerprint` НЕ реагирует на правку `capture.mjs`; единственный сторож — отдельно хранимый `captureScriptSha256`. +3. Комментарий в самом `scripts/capture-environment.mjs` описывает ровно этот класс дефекта на примере другого файла (`accept.mjs`/`policy.mjs` для golden, #334): «правка инструмента приёмки объявляла устаревшими сразу три вещи… Проверено на себе: первая редакция … правку сделала, и гейт документации сразу покраснел». Проектная архитектура намеренно вынесла `accept.mjs`/`policy.mjs` в `POST_CAPTURE_INPUTS`, потому что их правка не может сдвинуть пиксель. `demo/docs/capture.mjs`, наоборот, **рендерит кадр** — он не может быть исключён из инварианта «эта правка требует пересъёмки», и ТЗ сознательно правит именно его (`--out`), а не обёртку снаружи (параметризовать вывод без правки самого файла нельзя — сам параметр это и есть правка байтов файла). +4. `--screenshots=strict` реально исполняется в `publish-prerelease.yml:73` (гейт кандидата беты) и в `validate.yml:83` в heavy-режиме — то есть первый же релиз-кандидат после слияния #512 получит красный `docs`, если манифест не переприняли. +5. ТЗ §6.3 перечисляет явно, какие поля манифеста при `--identical` берутся «из закоммиченного» (`imageSha256`, `chromium`, `oxipng`, `acceptance`), а какие — «из кандидата» (`sourceFingerprint`, `scenarios[*].sourceSha256`). **`captureScriptSha256` не назван ни в одном из списков** — неясно, обновляется ли он вообще при `--identical`-приёмке. +6. Симметричный случай (переприёмка golden после смены версии) в ТЗ решён явно — §7 «Переприёмка golden» с `Baseline-Reviewed:`. Для docs-скриншотов, у которых this самое ТЗ правит `capture.mjs`, аналогичного шага нет вовсе ни в §6, ни в §11 «Затронутые файлы» (там `docs/images/screenshots.json` не упомянут). + +**Почему это блокирует, а не наблюдение:** это ровно тот класс инцидента, который проект уже дважды оплатил по другому триггеру (#230, #234 — красный `docs` до отдельной задачи #237) и один раз по этому же файлу-классу (#334). AC3 ТЗ требует «`check-docs --screenshots=strict` зелёный» как доказательство, но по факту сразу после правки `capture.mjs` этот гейт покраснеет по причине, не имеющей отношения к попиксельной логике `--identical`, — независимо от того, различаются кадры или нет. Наивная реализация по тексту ТЗ (сделать `--out`, реализовать компаратор, дважды прогнать unit) оставит эту причину нетронутой: `--identical` в лучшем случае чинит `sourceFingerprint`, но не обязан чинить `captureScriptSha256`, и в ТЗ не назван ни один коммит/шаг, который бы это сделал. + +**Что нужно в правке ТЗ** (техническое решение, owner не нужен — put in the assumptions block): +- явно решить, обновляет ли `--identical`-приёмка `captureScriptSha256` из кандидата (раз кандидат снят актуальным `capture.mjs`, это выглядит естественным выбором и делает исправление автоматическим при первом запуске инструмента после слияния); +- либо, если решение другое — добавить симметричный §7-подобный пункт «Переприёмка docs-скриншотов»: одна пересъёмка+приёмка `docs/images/**` **в том же PR**, что и правка `capture.mjs`, чтобы `dev` не остался с красным `docs` до следующей задачи; +- в любом случае — назвать это явным шагом/строкой AC, а не оставлять читателю ТЗ реконструировать зависимость самому. + +## Что проверено и корректно + +- Трек и его обоснование (§5 «одна поверхность» действительно не проходит — семь мест продукта плюс отдельный инструмент приёмки). +- §4 «Seam версии»: список точек чтения `CARD_VERSION` в обоих файлах построчно совпадает с реальным кодом (`houseplan-card.ts:2130,10732`; `houseplan-editor-runtime.ts:8979,9197,9342,9370,9615`); исключения (`hp_retry`, `console.info`) верно перечислены и подтверждены построчно; `release-contract.mjs` действительно продолжает читать константы, а не seam. +- §5 «Golden-харнес»: `card._haIntegrationVersion = cardVersion` в `harness.mjs:1923` — единственное реальное использование переменной `cardVersion`, читаемой из `package.json` (кроме передачи в `page.evaluate`) — утверждение «таких проверок больше нет» подтверждено. `integrationVersion: '0.0.0-golden-backend'` в `matrix.mjs` уже существует — «сохраняют» в ТЗ означает не создание нового значения, а сохранение поведения. +- Заявление «отпечаток бандла в `baselines-index.json` уже нормализует строку версии (#481)» подтверждено — `scripts/source-fingerprint.mjs` содержит `foreignFingerprintNormalizer`, заменяющий версию продукта плейсхолдером до хеширования. +- Мутанты §8 — конкретны и ловятся названными юнит-тестами; аналог такого рода source-text теста уже есть в проекте (`test/houseplan-source.mjs` + несколько `*-source.test.mjs`), паттерн реализуем без изобретения нового механизма. +- Playwright — уже прод-зависимость (`package.json`), риск §10.1 о его доступности снят корректно. +- Раздел «принятые предположения» (§12) — обе записи технические, продуктового вопроса владельцу здесь нет и не должно быть. +- Продуктовое отсутствие эффекта: единственное DOM-видимое использование `gs.about_version` (строка 9197) находится внутри `support-dialog`, а не в отдельном экране «about»; сам i18n-ключ и текст не меняются (AC6) — противоречия со SCOPE/USER-GUIDE не нашёл, задача не расширяет и не меняет видимое поведение продукта. + +## Чего не проверял + +- Не прогонял `npm run typecheck`/`test`/`build` — на этапе ревью ТЗ они неприменимы, кода ещё нет. +- Не проверял осуществимость `createImageBitmap`/`OffscreenCanvas.getImageData` в headless Chromium построчно (это deталь реализации компаратора, будет предметом код-ревью и его AC-доказательства через `test/png-identical.test.mjs`). +- Не проверял `docs/USER-GUIDE.ru.md` на терминологию — задача не меняет видимое поведение (`User-Visible: no`), пользовательский словарь не затрагивается. +- Низкоприоритетное наблюдение, не поднятое до отдельной находки: `docs:accept --identical` реально достигает 0 отличающихся пикселей только в среде, воспроизводящей растеризацию шрифтов закоммиченных кадров (Linux/WSL — тот же канон, что и у `docs:capture`/`golden:capture`, см. `scripts/capture-environment.mjs`), поскольку сравнение идёт по декодированным пикселям, а не по байтам PNG. §6 сознательно не гейтит `--identical` через `assertCaptureEnvironment`, и это безопасно (несовпадение просто вернёт код 1 и штатный путь), но сценарий §1.1 обещает «минуту локально» без этой оговорки. Не поднимаю до находки: инструмент не даёт неверного результата, ни один AC от этого не ломается, а сама конвенция «локально = WSL для съёмки» в проекте уже установлена (`docs/DEVELOPMENT.md`, `AGENTS.md`). Стоит на всякий случай явно упомянуть в будущей правке `docs/DEVELOPMENT.md` (уже в списке затронутых файлов), чтобы разработчик на голом Windows не тратил время на заведомо бесполезные попытки. + +--- + + + +## Материал раунда + +- Ветка: `issue/512-golden-version-seam-docs-identical`, коммит `e606b808d775` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `c30a6bcb25c50fdeff8d1dd61b7405409a96dc9a` + ``` + git log --all --format='%H %T' | grep c30a6bcb25c5 + ``` +- ТЗ `docs/specs/512-golden-version-seam-and-docs-identical-accept.md`, блоб `efb22d3a13a227c80bf0b4389d8d4f8d0f2ea71c` + ``` + git log --all --find-object=efb22d3a13a227c80bf0b4389d8d4f8d0f2ea71c -- docs/specs/512-golden-version-seam-and-docs-identical-accept.md + ``` +- Вердикт конвейера: `red` · High 1