Files
houseplan-card/docs/reviews/SPEC-REVIEW-512-r1.md
2026-09-09 18:52:44 +03:00

16 KiB
Raw Permalink Blame History

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 инвариант:
    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