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

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)

---

Материал раунда

• Ветка: 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