mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
@@ -0,0 +1,185 @@
|
||||
# SPEC-REVIEW-422-r1
|
||||
|
||||
- Issue: https://github.com/Matysh/houseplan-card/issues/422
|
||||
- ТЗ: `docs/specs/422-capture-and-anchor-gates.md`
|
||||
- Материал ревью: ветка `issue/422-capture-and-anchor-gates`, коммит `2bc83024` (`git rev-parse HEAD`, сверено непосредственно перед выводом).
|
||||
- Заход: r1 (первый), возвратов не было, «Закрытие раунда»/«Унаследовано» не применимы.
|
||||
|
||||
## Скоуп проверки
|
||||
|
||||
Полный разбор: заход первый, дельты нет. Читалось: тело issue #422 и оба
|
||||
комментария владельца, `docs/specs/422-capture-and-anchor-gates.md` целиком,
|
||||
`docs/SCOPE.md`, `PROCESS.md` §1, §2.2–2.10, §5, §7.1. Технически —
|
||||
`demo/docs/capture.mjs` (клип-арифметика, режим `--stability`, точка записи
|
||||
кадров), `.github/workflows/docs-screenshots.yml` целиком (порядок шагов),
|
||||
`scripts/review-doc-guard.mjs` (`resolveObjects`, `resolveReachable`,
|
||||
`danglingMaterialRefusal`, `materialAnchorBlock`, `materialAnchorsFrom`),
|
||||
`test/review-doc-guard.test.mjs` (существующие тесты #414) и
|
||||
`scripts/mutation-gate.mjs` (формат записи мутанта, `runMutant`,
|
||||
`runCleanGuards`) — чтобы проверить не только формулировки ТЗ, но и то, что
|
||||
описанные контракты вообще реализуемы в существующей инфраструктуре, а не
|
||||
только на бумаге.
|
||||
|
||||
Класс файлов — только B (`.github/**`, `demo/**`, `scripts/**`, `test/**`),
|
||||
продуктового кода (класс A) задача не касается. Продуктовая рамка `SCOPE.md`
|
||||
к задаче неприменима впрямую (см. отдельное замечание ниже, не блокирующее).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Не просто чтение текста ТЗ — по каждому из двух пунктов сверено соответствие
|
||||
описанного текущему коду и осмысленность контракта:
|
||||
|
||||
1. **Гейт стабильности съёмки.** Прочитан `demo/docs/capture.mjs:261-341`:
|
||||
подтверждено, что в режиме `--stability=N` кадры снимаются в одном процессе,
|
||||
в память (`shots.push(...)`), сравниваются `comparePairs`, и запись файла
|
||||
(`writeFileSync(imagePath, image)`) в этом режиме не выполняется —
|
||||
`continue` на строке 320 её обходит. Значит существующая проверка и
|
||||
предлагаемая кросс-прогонная не пересекаются по побочным эффектам, и вторая
|
||||
действительно требует отдельного полного прогона `capture.mjs`, а не
|
||||
модификации `--stability`.
|
||||
2. Прочитан сам клип-код (`:284-289`): `floor(x)`, `floor(y)`,
|
||||
`ceil(x+width)-floor(x)`, `ceil(y+height)-floor(y)` — расширение наружу,
|
||||
как и описывает АС4. Строки в ТЗ совпадают с реальными.
|
||||
3. Прочитан `.github/workflows/docs-screenshots.yml` целиком: `Capture` —
|
||||
строка 80, шаг «Кадр не плавает…» — строка 99. Комментарий на `:96-100`
|
||||
действительно утверждает «Проверка стоит ПЕРЕД съёмкой набора» — это не
|
||||
так, факт подтверждён, AC9 обоснован.
|
||||
4. Прочитан `scripts/review-doc-guard.mjs:190-320`: подтверждено, что
|
||||
`resolveReachable` (для SHA) фильтрует через
|
||||
`git for-each-ref --contains … refs/remotes/origin refs/tags`, а
|
||||
`resolveObjects` (для якорей) — голый `git cat-file -e`, без привязки к
|
||||
ссылкам. Асимметрия, на которой строится задача, воспроизведена чтением, не
|
||||
только заявлена.
|
||||
5. Прочитан `test/review-doc-guard.test.mjs:181-203`: существующие тесты #414
|
||||
уже внедряют `resolveObjects` как параметр-заглушку (`() => true` /
|
||||
`() => false`) — то есть АС5/АС6 технически исполнимы без переписывания этих
|
||||
тестов, они останутся зелёными при замене реализации на настоящую (AC6
|
||||
прямо это требует).
|
||||
6. Прочитан `scripts/mutation-gate.mjs:4609-4626` (`runMutant`) — формат
|
||||
записи мутанта: один `guard` (шелл-команда), франчворк требует, чтобы она
|
||||
краснела на мутанте и оставалась зелёной без него. Отдельного поля «а эта
|
||||
команда обязана остаться зелёной» нет — но `guard` это произвольная
|
||||
шелл-строка, значит двойное условие («новый гейт красный, старый зелёный»)
|
||||
выражается одной составной командой. Проверил, что план АС2 технически
|
||||
реализуем в существующем формате, а не требует незаявленного изменения
|
||||
`mutation-gate.mjs`.
|
||||
|
||||
Гейты (typecheck/test/build) не гонялись: задача на этапе ревью ТЗ, кода нет,
|
||||
раздел «Гейты» неприменим к этому этапу.
|
||||
|
||||
## Находки
|
||||
|
||||
Ничего блокирующего не найдено. Один Low, снят решением ревьюера (см. ниже),
|
||||
не влияет на вердикт.
|
||||
|
||||
### Low — AC9 не называет способ доказательства (снято)
|
||||
|
||||
**Файл:** `docs/specs/422-capture-and-anchor-gates.md:154-155`.
|
||||
|
||||
Все критерии AC1–AC8 заканчиваются явной строкой «Доказательство: …»; AC9
|
||||
(«Комментарий о порядке шагов… соответствует фактическому порядку») такой
|
||||
строки не имеет — формально нарушает требование §2.5 «у каждого AC указано,
|
||||
чем он доказывается». По существу способ доказательства здесь единственно
|
||||
возможный и очевидный из самого текста критерия — чтение файла и сравнение
|
||||
формулировки с фактическим порядком шагов, — поэтому не возвращаю ТЗ на цикл
|
||||
ради одной строки. Автору стоит дописать «Доказательство: ревью кода (чтение
|
||||
файла)» при следующей правке файла, но это не создаёт нового цикла.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **Оба контракта воспроизведены исполнением**, а не только заявлены:
|
||||
владелец привёл SHA двух прогонов мутированного `capture.mjs`
|
||||
(`6ba168494519a10dd659` / `4142a6ab9558cb481ecc`) и реальный вывод
|
||||
`danglingMaterialRefusal` на подставном недостижимом объекте. Ни одно
|
||||
утверждение о поведении не подано как факт без проверки — граница «догадка
|
||||
vs решение» (о ней прямо просят следить инструкции) не нарушена нигде в
|
||||
тексте; единственные гипотетические места («системный шрифтовой кэш —
|
||||
теоретически да») явно помечены как риск, а не как факт.
|
||||
- **Не-скоуп проведён точно**: #408/#409 (порог кадров-свидетелей), #414
|
||||
(формат машинного блока), #413 (правило достижимости SHA — не меняется),
|
||||
#421 (три непадающих проверки — сосед) — каждый назван и обоснованно
|
||||
оставлен вне задачи.
|
||||
- **AC1↔AC2 — контрастная пара** (положительный + отрицательный прогон)
|
||||
выполнена по правилу «тест обязан уметь падать», причём в самом ТЗ, а не
|
||||
только в будущем коде.
|
||||
- **AC4** описывает уже существующий, но непокрытый тестами код —
|
||||
корректно классифицировано как «юнит без брaузера», строки совпадают с
|
||||
реальным файлом.
|
||||
- **AC5/AC6/AC7** согласованы с уже существующей структурой
|
||||
`danglingMaterialRefusal(text, resolveReachable, headerLines, resolveObjects)`:
|
||||
расширение — замена реализации `resolveObjects`, а не смена контракта
|
||||
функции; существующие тесты #414 переживут это без правки утверждений, как и
|
||||
требует AC6 (см. проверку в п.5 «Как проверялось»).
|
||||
- **Границы явно называются**: §«Чего гейт не умеет» из `review-doc-guard.mjs`
|
||||
прямо унаследована и не пересматривается — задача сознательно не расширяет
|
||||
предмет проверки за пределы наличия→достижимости.
|
||||
- **Release-артефакты** согласованы: `User-Visible: no` обосновано (поведение
|
||||
продукта не меняется), `docs/TESTING.md` заявлен как место для новой
|
||||
локальной команды — соответствует тому, что AC1/мутанты предполагают именно
|
||||
такую команду.
|
||||
- Обязательные разделы §7.1 присутствуют все: сценарий · что человек увидит ·
|
||||
проблема+контракт (объединены, оба пункта покрыты) · скоуп/не-скоуп · UX/
|
||||
модель данных/i18n («не применимо», обоснованно) · AC1…AC9 с доказательством
|
||||
(кроме AC9, см. Low) · план автотестов · риски · откат · release-артефакты.
|
||||
|
||||
## Рассмотрено и отклонено как небеспокоящее (для прозрачности процесса)
|
||||
|
||||
- **Инфраструктурная задача, но идёт полным флоу.** Задача не задевает ни
|
||||
одного файла класса A, то есть по механическому признаку §1 должна была бы
|
||||
идти «вне флоу» (без ТЗ, без ревью ТЗ). Однако issue помечен `infra` и уже
|
||||
прошёл `S2` → `S3` → `S4` с полноценным файлом ТЗ; автор прямо называет
|
||||
прецеденты (#398, #399, #404). Это расхождение с буквой §1, но не с текущей
|
||||
практикой репозитория, а по правилу приоритета источников самого PROCESS.md
|
||||
(«при расхождении документации с GitHub побеждает GitHub») решение о
|
||||
маршруте — не предмет ревью содержания ТЗ. Не понижает вердикт.
|
||||
- **Печатаемая для человека команда в `materialAnchorBlock` использует
|
||||
`--all`, а новый внутренний гейт — `refs/remotes/origin`+теги.** Проверил,
|
||||
не требует ли контракт синхронизировать и текст, который печатается
|
||||
читателю. Прочитан текст контракта в ТЗ («что делает задачу исполнимой»):
|
||||
требование — переиспользовать ту же ФОРМУ команды с другой областью для
|
||||
внутренней проверки, а не переписать шаблон для человека. `--all` — надмножество
|
||||
origin+тегов, поэтому printed-команда не даёт ложноотрицательных результатов
|
||||
относительно решения CI, только более широкий (и осознанно локальный,
|
||||
человеческий) поиск. Не ambiguity, не finding.
|
||||
- **AC9 допускает два технически разных фикса** (переписать комментарий или
|
||||
переставить шаги местами, раз исходный комментарий утверждал «ДО»).
|
||||
Раздел «Скоуп» ограничивает предмет правки явно словом «комментарий», без
|
||||
«порядок шагов» — поэтому чтение AC9 вместе со скоупом однозначно: правится
|
||||
текст, а не переставляются шаги. Не finding.
|
||||
- **Клип-арифметика в `capture.mjs` не вынесена в отдельную экспортируемую
|
||||
функцию** — юнит-тест из AC4 потребует извлечения (рефакторинга) в
|
||||
импортируемую функцию. Это рутинный технический шаг реализации, не
|
||||
продуктовое и не архитектурное решение, требующее фиксации как
|
||||
«принято предположительно» — не finding.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не запускал никакой код: этап — ревью ТЗ, а не код-ревью; кода по задаче ещё
|
||||
нет (текущий коммит — только файл ТЗ).
|
||||
- Не оценивал реальную стоимость второго прогона съёмки (AC8) — это по
|
||||
контракту задача измерения на этапе реализации, а не ревью ТЗ.
|
||||
- Не проверял `docs/USER-GUIDE.ru.md` — задача не меняет видимое поведение
|
||||
продукта, терминология интерфейса не затрагивается.
|
||||
- Не проверял канонические документы подсистем (`SUN.md`, `LIGHT.md` и т.д.) —
|
||||
задача не касается ни одной из этих поверхностей (гейты CI и dev-тулинг).
|
||||
|
||||
## Вердикт
|
||||
|
||||
Зелёный. High: 0, Medium: 0 (единственная находка — Low, снята с записью,
|
||||
бюджет §4 не тратится).
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/422-capture-and-anchor-gates`, коммит `2bc83024fb58` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `a0b3a754b5b0c9fc3ea431de4d60aa1987237690`
|
||||
```
|
||||
git log --all --format='%H %T' | grep a0b3a754b5b0
|
||||
```
|
||||
- ТЗ `docs/specs/422-capture-and-anchor-gates.md`, блоб `b1a2d2affa772ed7a2ae9fc55053d22ca5bcbe75`
|
||||
```
|
||||
git log --all --find-object=b1a2d2affa772ed7a2ae9fc55053d22ca5bcbe75 -- docs/specs/422-capture-and-anchor-gates.md
|
||||
```
|
||||
Reference in New Issue
Block a user