mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -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 не тратил время на заведомо бесполезные попытки.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `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
|
||||
Reference in New Issue
Block a user