From 8ed8ecc3fd3768278856505b4851a848e872e53c Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Wed, 2 Sep 2026 16:36:03 +0000 Subject: [PATCH] docs: review document for #422 Issue: #422 User-Visible: no --- docs/reviews/SPEC-REVIEW-422-r1.md | 185 +++++++++++++++++++++++++++++ 1 file changed, 185 insertions(+) create mode 100644 docs/reviews/SPEC-REVIEW-422-r1.md diff --git a/docs/reviews/SPEC-REVIEW-422-r1.md b/docs/reviews/SPEC-REVIEW-422-r1.md new file mode 100644 index 00000000..ab7c5c77 --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-422-r1.md @@ -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 не тратится). + +--- + + + +## Материал раунда + +- Ветка: `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 + ```