docs: review document for #473

Issue: #473
User-Visible: no

docs/reviews/SPEC-REVIEW-473-r1.md

@@ -0,0 +1,236 @@
+# SPEC-REVIEW-473-r1
+
+- Issue: https://github.com/Matysh/houseplan-card/issues/473
+- Этап: spec (ревью ТЗ, PROCESS.md §2.4)
+- Заход: r1 · блокирующих циклов израсходовано 0 из 4
+- ТЗ: `docs/specs/473-iso-perf-witnesses-and-smoke.md`, SHA `2cedcc6221e40ea6ead2f48ff7fb37ddc96a51ca`
+- Вердикт: **жёлтый**
+
+## Скоуп ревью
+
+Материал раунда — весь диапазон HEAD..HEAD (первый заход, дельты по
+предыдущему раунду нет): два файла, `docs/specs/473-iso-perf-witnesses-and-smoke.md`
+(новый, 125 строк) и одна строка в `docs/specs/README.md`. Продуктового и
+тестового кода нет — задача ещё в статусе ТЗ.
+
+Задача — процессная/инфраструктурная (постфактум-ревью perf-дельты #160 плюс
+диффозависимый перф-смок в Validate), класс файлов будущей реализации — только
+B (`test/**`, `scripts/mutation-gate.mjs`, `.github/workflows/validate.yml`,
+`demo/performance/*.json`, `PROCESS.md`). `docs/SCOPE.md` прямо не применяется:
+задача не меняет ни один Core user job, а укрепляет процесс, которым продукт
+разрабатывается — это соответствует прецеденту #404/#399/#422 (те же
+инфраструктурные ТЗ полного трека).
+
+## Как проверялось
+
+Гейты (`typecheck`/`test`/`build`) на этом раунде неприменимы: дифф — два
+документных файла, продуктового и тестового кода ещё нет. Прогонять их
+незачем, и ссылка в задании на зелёный Validate (`2cedcc62`) сюда не
+адресуется — там просто нечего было ломать.
+
+Разбор шёл построчной сверкой каждого технического утверждения ТЗ с текущим
+деревом (`2cedcc62`, тот же SHA что HEAD):
+
+- построчные ссылки §4 (`iso-scene-render.ts:783,786,805,821`,
+  `iso-overlays.ts:342-345`) — прочитаны и сверены с исходником;
+- существование и сигнатуры скриптов, упомянутых в §5 (`demo/benchmark_large_house.mjs`,
+  `demo/benchmark_glow.mjs`, `demo/performance/compare.mjs`, `evaluate.mjs`) —
+  прочитаны целиком в интересующих местах (`valueArg`, `--variants`,
+  `--absolute-only`, `minimumSamples`);
+- существующие бюджеты (`budgets-glow-smoke.json`, `budgets-large-house-isometric.json`
+  и др.) — сверены на предмет прецедента «smoke-бюджет как урезанное
+  подмножество полного» и совпадения `hardMaxMs` с цифрами регрессии из
+  Проблемы;
+- `.github/workflows/validate.yml` — job `changes`, `reuse`, `performance_smoke`
+  прочитаны целиком, чтобы понять, как диффозависимый триггер (§5) сочетается
+  с механизмом переиспользования по содержимому (`scripts/gate-reuse.mjs`) и
+  есть ли ручной способ форсировать путь (`workflow_dispatch` — проверено по
+  всем workflow, в `validate.yml` его нет);
+- `test/validate-workflow.test.mjs`, `test/performance-budget.test.mjs` —
+  существуют, стиль контрактных тестов (regex по тексту YAML) подтверждает
+  реализуемость AC3/AC4/AC5 как статических проверок;
+- `de215578` — коммит существует в истории, воспроизведение AC6 технически
+  доступно;
+- `docs/specs/README.md` — формат строки таблицы сверен с соседними записями.
+
+## Находки
+
+Все три — Medium, в скоупе задачи: без High это жёлтый вердикт, правки вносятся
+в тот же файл ТЗ и проходят повторный цикл (PROCESS.md §2.4, #202). Отдельный
+issue не заводится.
+
+### Medium-1 — AC7 недоказуем предписанным способом
+
+**Файл:** `docs/specs/473-iso-perf-witnesses-and-smoke.md:103`
+
+> AC7 | На текущем `dev` оба новых профиля укладываются в потолки за 3 образца | прогон в CI этой ветки
+
+§5 (той же строкой 68-71) делает триггер профилей диффозависимым от
+ПРОДУКТОВЫХ путей: `perf_iso` — если дифф задел `src/iso-*`, `perf_interaction`
+— если дифф задел `src/live-*`, `src/render-*`,
+`src/houseplan-render-lifecycle.ts`, `src/houseplan-card.ts`. Это выходы job
+`changes`, которая судит **дифф ветки против последнего доказанно зелёного
+предка** (`validate.yml:270-284`), а не содержимое дерева.
+
+Сама реализация этой задачи, согласно §6, «продуктовый код не меняет» —
+дифф ветки состоит из `test/**`, `scripts/mutation-gate.mjs`,
+`.github/workflows/validate.yml`, `demo/performance/*.json`. Ни один из этих
+путей не матчит шаблоны `src/iso-*` / `src/live-*` / `src/render-*` /
+`houseplan-card.ts`. Значит на пуше этой самой ветки `perf_iso` и
+`perf_interaction` резолвятся в `false`, новые шаги смока не выполняются
+вовсе, и «прогон в CI этой ветки» не производит того свидетельства, которое
+AC7 требует.
+
+**Проверено, что обхода через ручной триггер нет**: `validate.yml` не имеет
+`workflow_dispatch` (в отличие от `performance.yml`, `mutation-gate.yml`,
+`docs-screenshots.yml`, у которых он есть) — форсировать классификацию
+нечем.
+
+Способ существует (ровно как для AC6 — разовый замер `npm run
+benchmark:large-house-isometric -- --samples=3 --warmups=1` +
+`benchmark:compare --absolute-only --budgets=...`, команда и результат
+записываются в issue), но ТЗ называет другой, автоматический и, как показано
+выше, недостижимый на этой ветке. Либо AC7 меняет способ доказательства на
+ручной (аналогично AC6), либо явно описывает временный форс-коммит в
+продуктовый файл для проверки с последующим откатом перед мержем — сейчас в
+тексте нет ни того, ни другого.
+
+### Medium-2 — `--variants=60` не поддерживается скриптом, на который распространён контракт
+
+**Файл:** `docs/specs/473-iso-perf-witnesses-and-smoke.md:74-79`
+
+> Профили идут с `--variants=60 --samples=3 --warmups=1 --absolute-only` и
+> своими smoke-бюджетами…
+
+Флаг `--variants` относится к сценариям, где он читается: `demo/benchmark_glow.mjs:27`
+(`requestedVariants = valueArg('variants')?.split(',')...`) — там это число
+одновременных обновлений состояния (`[1, 10, 30, 60]`). Оба новых профиля,
+`large-house-isometric-v1` и `large-house-interaction-v1`, идут через
+`demo/benchmark_large_house.mjs` (`package.json:22,25`), который вообще не
+читает `--variants` — там только `samples`, `warmups`, `target-root`,
+`profile`, `output` (`demo/benchmark_large_house.mjs:17-22`). Формулировка §5
+одной строкой распространяет флаг glow-скрипта на все три профиля — фактическая
+ошибка в записанном как готовый контракт наборе CLI-аргументов.
+
+Эффект сегодня безвреден (неизвестный `--variants=` молча игнорируется
+`valueArg`), но это ровно тот класс утверждения, который ТЗ обязано не
+допускать: техническая деталь подана как проверенный факт, не будучи
+сверенной с кодом инструмента. Правка — убрать `--variants=60` из вызовов
+`benchmark:large-house-isometric`/`-interaction`, оставить только у
+`benchmark:glow`.
+
+### Medium-3 — четыре обязательных раздела §7.1 отсутствуют без пометки «не применимо»
+
+**Файл:** `docs/specs/473-iso-perf-witnesses-and-smoke.md` (весь документ, разделы 1-10)
+
+PROCESS.md §7.1 перечисляет обязательные разделы ТЗ: сценарий · что человек
+увидит до и после · проблема · скоуп и не-скоуп · контракт поведения · UX ·
+модель данных и миграция · i18n · критерии приёмки · план автотестов ·
+риски · откат · release-артефакты. В документе есть Проблема (§1),
+Скоуп/Не-скоуп (§2-3), контракт (§4-5), откат (внутри §6), критерии приёмки
+(§7), release-артефакты (§8) — но полностью отсутствуют как разделы:
+**Сценарий**, **Что человек увидит до и после**, **UX/модель
+данных/i18n**, **Риски**. Не встречается даже фраза «не применимо» —
+разделы просто не написаны.
+
+Прямой прецедент — `docs/specs/404-smoke-exception-guard.md` (та же
+инфраструктурная категория, тот же полный трек, тот же автор): там есть
+`## Сценарий` (кто и когда столкнётся — разработчик и CI, не пользователь),
+`## Что человек увидит до и после` (явно «видимого поведения продукта
+задача не меняет»), `## UX, модель данных, i18n` (одной строкой «не
+применимо»), и отдельный `## Риски` с тремя названными рисками и
+смягчениями. Здесь этот минимум не выполнен.
+
+Содержательно часть материала для «Рисков» в тексте есть, но рассыпана по
+другим разделам без общего заголовка: рост времени job (§10, как
+предположение с планом отката, а не как риск), достижимость мутации AABB
+(§10). Не названы вовсе: шумность абсолютного порога на 3 образцах на
+разделяемом раннере (ложный красный на чужой, легитимной правке — именно то,
+против чего заведена вся задача #473, доведённая до абсурда: диффозависимый
+смок может начать шуметь на легитимных изменениях и стать тем самым «гейтом
+не там, где нужно», о котором вторая половина Проблемы), и хрупкость четырёх
+новых мутантов к будущим рефакторингам подписи кэша. Раздел «Риски» обязателен
+не как формальность, а чтобы этот перечень стал видимым до, а не после
+инцидента.
+
+## Что проверено и корректно
+
+- Числа и ссылка на прогон в §1 совпадают дословно с телом issue #473.
+- Все построчные ссылки §4 на `iso-scene-render.ts` (783, 786, 805, 821) и
+  `iso-overlays.ts` (342-345) точны на текущем дереве.
+- Мутация AABB (`boundsNear` без `gap` и без равенства) описывает
+  реальный код: `boundsNear` использует нестрогие `<=`/`>=` с допуском `gap`
+  — мутация в строгие операторы без допуска валидна и достижима.
+- Предметы кэширования (WeakMap по `input.wallSilhouettes`, подпись
+  `shapeSignature`/`signature`, гард `!previous.nearWallBefore || …`) —
+  описаны верно и совпадают с кодом дословно, вплоть до имён полей.
+- `budgets-glow-smoke.json` / `budgets-space-glow-smoke.json` — уже
+  существующий прецедент «smoke-бюджет как урезанное подмножество полного»,
+  подтверждает реализуемость §5 без нового механизма.
+- `hardMaxMs: 3500` в `budgets-large-house-isometric.json`
+  (`firstStableRenderMs`) против измеренных 9870 мс в Проблеме — AC6
+  действительно ловится с одним образцом, как заявлено.
+- `de215578` существует в истории — AC6 воспроизводимо.
+- `test/validate-workflow.test.mjs`, `test/performance-budget.test.mjs`
+  существуют и уже проверяют YAML/JSON текстовыми утверждениями — AC3-AC5
+  реализуемы заявленным способом (статический контракт, не запуск CI).
+- `scripts/gate-reuse.mjs`: ключ переиспользования — хеш содержимого
+  (`sourceFingerprint` + оснастка job), не зависит от диапазона диффа;
+  добавление признака набора профилей в ключ (AC5) технически сводится к
+  добавлению строки в `key:` кэша в `validate.yml`, без изменения самого
+  скрипта — реализуемо, хотя способ (какая именно строка) оставлен автору
+  как техническая деталь (правомерно, не продуктовый вопрос).
+- Пути `src/live-*`, `src/render-*`, `houseplan-render-lifecycle.ts`,
+  `houseplan-card.ts`, `src/iso-*` — все существуют, шаблоны `has()` из
+  Проблемы issue корректны.
+- Обоснование полного трека (не `small`): две поверхности (тесты
+  продуктового кода + workflow), перф — часть контракта задачи — корректно
+  дисквалифицирует лёгкий трек по критерию §5 PROCESS.md.
+- `docs/specs/README.md`: строка добавлена в верном месте таблицы, формат
+  совпадает с соседними записями.
+- Не-скоуп (§3) дословно соответствует «Не входит» issue: пороги полных
+  профилей и §11.4 не переоткрываются.
+
+## Чего не проверял
+
+- Не прогонялись `typecheck`/`test`/`build`/`bundle:sync` — дифф раунда не
+  содержит ни продуктового, ни тестового кода, гонять нечего; это станет
+  предметом код-ревью.
+- Не проверялась фактическая укладываемость трёх новых профилей в
+  20-минутный лимит `performance_smoke` (§10, «Принятые предположения») —
+  это требует реального прогона CI, которого на этапе ТЗ ещё нет; ТЗ само
+  помечает это как предположение с планом отхода («лимит поднимается»), что
+  корректно по формату §7.1.
+- Не проверялась фактическая шумность абсолютных порогов на 3 образцах на
+  разделяемых GitHub-раннерах (см. Medium-3) — это открытый риск, а не
+  проверяемый факт на этой стадии.
+- Не читались все 122+ существующих мутанта в `scripts/mutation-gate.mjs`
+  построчно — только формат записи и подтверждение, что регистрация новых
+  идёт тем же путём.
+- Не проверялась точная фикстура «три комнаты, одна стена, плита у стены и
+  плита вдали» на предмет буквального совпадения с какой-то одной
+  существующей — это техническая деталь реализации, не продуктовый вопрос,
+  и ревьюер вправе спросить о ней при коде, если фикстура не даст нужных
+  условий.
+
+## Материал раунда
+
+- SHA ветки: `2cedcc6221e40ea6ead2f48ff7fb37ddc96a51ca` (= HEAD на момент ревью)
+- Дерево ТЗ: `docs/specs/473-iso-perf-witnesses-and-smoke.md` на этом SHA
+- `git show 2cedcc62 --stat`: `docs/specs/473-iso-perf-witnesses-and-smoke.md` (new, 125 lines), `docs/specs/README.md` (+1 line)
+
+---
+
+<!-- material-anchors: сгенерировано конвейером (#414) -->
+
+## Материал раунда
+
+- Ветка: `issue/473-iso-perf-witnesses`, коммит `2cedcc6221e4` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
+- Дерево материала: `41b642d61cff58dea0718d19efa6209b897a5a33`
+  ```
+  git log --all --format='%H %T' | grep 41b642d61cff
+  ```
+- ТЗ `docs/specs/473-iso-perf-witnesses-and-smoke.md`, блоб `03c7d24cb503bea7ac083ed9a1ffc12a77b3c0e0`
+  ```
+  git log --all --find-object=03c7d24cb503bea7ac083ed9a1ffc12a77b3c0e0 -- docs/specs/473-iso-perf-witnesses-and-smoke.md
+  ```