mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-03 21:28:59 +00:00
claude[bot]
@@ -0,0 +1,196 @@
|
||||
# SPEC-REVIEW-473-r2
|
||||
|
||||
- Issue: https://github.com/Matysh/houseplan-card/issues/473
|
||||
- Этап: spec (ревью ТЗ, PROCESS.md §2.4)
|
||||
- Заход: r2 · блокирующих циклов израсходовано 1 из 4
|
||||
- ТЗ: `docs/specs/473-iso-perf-witnesses-and-smoke.md`, SHA `ab0463de7881c64f8b42bed1f007c7ce4e434cf9`
|
||||
- Вердикт: **зелёный**
|
||||
|
||||
## Скоуп ревью (по дельте, PROCESS.md §2.10)
|
||||
|
||||
Предыдущий раунд: `SPEC-REVIEW-473-r1.md`, вердикт жёлтый, материал —
|
||||
`2cedcc6221e40ea6ead2f48ff7fb37ddc96a51ca` (= HEAD на момент r1). SHA
|
||||
резолвится напрямую, ребейза не было — линейная история
|
||||
`2cedcc62 → 7e4b0e69 (review doc) → ab0463de (fix)`.
|
||||
|
||||
Дельта раунда: `git diff 2cedcc62..ab0463de -- docs/specs/473-iso-perf-witnesses-and-smoke.md`
|
||||
— 41 добавленная / 5 удалённых строк, единственный файл. Полный диапазон
|
||||
`git diff 2cedcc62..ab0463de --stat` дополнительно содержит только сам
|
||||
документ r1 (`docs/reviews/SPEC-REVIEW-473-r1.md`, публикация предыдущего
|
||||
раунда) — продуктового и тестового кода как не было, так и нет.
|
||||
|
||||
Дельта локальна: правки только внутри ТЗ, автор не тронул код, не было
|
||||
ребейза, подсистема не менялась, объём (41 строка) на порядок меньше объёма
|
||||
исходного ТЗ (125 строк). Условия «разбор остаётся полным» (§2.10) не
|
||||
выполнены — разбор ведётся по дельте: перепроверка трёх находок r1 плюс
|
||||
верификация всех новых технических утверждений, которые дельта внесла.
|
||||
AC, не задетые дельтой (AC1–AC6), наследуются без повторной проверки.
|
||||
|
||||
## Закрытие раунда r1
|
||||
|
||||
| Находка r1 | Чем закрыта | Где это видно |
|
||||
|---|---|---|
|
||||
| Medium-1 — AC7 недоказуем предписанным способом (диффозависимый триггер не сработает на ветке самой задачи, `workflow_dispatch` у `validate.yml` нет) | AC7 переписан на ручное доказательство, как AC6: «вручную при реализации... команды и числа в issue», с явным объяснением почему автоматический путь недостижим на этой ветке. Добавлен AC8, доказывающий саму диффозависимость контрактным тестом на функцию классификации (без запуска реального Validate) | `docs/specs/473-iso-perf-witnesses-and-smoke.md:123-124` |
|
||||
| Medium-2 — `--variants=60` не поддерживается `demo/benchmark_large_house.mjs` | Флаг убран из §5, формулировка заменена на факт: только `--samples=3 --warmups=1`, с явной сноской, что `--variants` принадлежит только `benchmark_glow.mjs`, и точной ссылкой на сигнатуру `demo/benchmark_large_house.mjs:17-18` | `docs/specs/473-iso-perf-witnesses-and-smoke.md:92-95` |
|
||||
| Medium-3 — отсутствуют разделы §7.1 (Сценарий, Что человек увидит, UX/данные/i18n, Риски) без пометки «не применимо» | Добавлены `## 1.1. Сценарий`, `## 1.2. Что человек увидит до и после`, `## 7.1. UX, модель данных, i18n` («не применимо», с объяснением) и `## 7.2. Риски и меры` (таблица из четырёх рисков с мерами), по образцу `404-smoke-exception-guard.md` | `docs/specs/473-iso-perf-witnesses-and-smoke.md:29-44, 126-138` |
|
||||
|
||||
Проверка не ограничилась фактом наличия текста — каждое техническое
|
||||
утверждение новых строк сверено с деревом (см. ниже).
|
||||
|
||||
## Как проверялось
|
||||
|
||||
Гейты `typecheck`/`test`/`build` неприменимы и на этом раунде: дифф — один
|
||||
документный файл, продуктового и тестового кода по-прежнему нет. Ссылка на
|
||||
зелёный Validate (`ab0463de`, job `process-gate` и др.) в задании адресована
|
||||
дешёвым гейтам будущей реализации, не этому диффу — здесь нечего было ломать.
|
||||
|
||||
Построчная проверка каждого нового технического утверждения:
|
||||
|
||||
- `demo/benchmark_large_house.mjs:17-22` прочитан целиком — принимает ровно
|
||||
`samples`, `warmups`, `target-root`, `profile`, `output`; `--variants` там
|
||||
действительно не читается. Подтверждает правку Medium-2 буквально.
|
||||
- `demo/benchmark_glow.mjs:27` — `valueArg('variants')` существует только
|
||||
здесь, подтверждает, что убранный флаг относился к другому скрипту.
|
||||
- `package.json` — `benchmark:large-house-isometric` (`--profile=large-house-isometric-v1`)
|
||||
и `benchmark:large-house-interaction` (`--profile=large-house-interaction-v1`)
|
||||
существуют и совпадают с профилями AC7/§5.
|
||||
- `.github/workflows/validate.yml:279-281` — job `changes` классифицирует
|
||||
диапазон именно через inline-shell `has() { grep -qE "$1" ... }`, что
|
||||
подтверждает предпосылку AC8 (классификация живёт в inline-shell, вынос в
|
||||
отдельный скрипт или тестируемые regex-шаблоны — реальная, не выдуманная
|
||||
работа) и отсутствие `workflow_dispatch` у `validate.yml`, на чём стоит
|
||||
переформулированный AC7.
|
||||
- `.github/workflows/validate.yml:492,720` — `timeout-minutes: 20` у
|
||||
соответствующих job, подтверждает цифру «20-минутный лимit» в §7.2.
|
||||
- `demo/performance/*.json` — уже существующая пара «smoke-бюджет как
|
||||
урезанное подмножество полного» (`budgets-glow-smoke.json` рядом с
|
||||
`budgets-large-house-glow-overlay.json`) — прецедент для новых
|
||||
`budgets-isometric-smoke.json`/`budgets-interaction-smoke.json` не
|
||||
выдуман, а назван по образцу.
|
||||
- `test/validate-workflow.test.mjs`, `test/performance-budget.test.mjs` —
|
||||
существуют (7140 и 19956 байт), формат уже содержит контрактные проверки
|
||||
над текстом YAML/JSON — AC8 в этом стиле реализуем.
|
||||
- Фраза «правило после #426» (§7.2, третья строка) — не находка в
|
||||
строгом смысле, но проверена: собственно ревью #426 такого правила не
|
||||
формулирует, однако тот же оборот с тем же смыслом («мутант на каждый
|
||||
защитный контракт... для каждой новой проверки, которая обязана
|
||||
краснеть, обязателен прогон отрицательного мутанта») уже используется тем
|
||||
же автором в `docs/specs/162-vacuum-map-space-routing.md:126-129` —
|
||||
внутренне согласованная, не изолированная ссылка, и совпадает с духом
|
||||
PROCESS.md §2.7. Не блокирует.
|
||||
|
||||
## Находки
|
||||
|
||||
Одна остаточная, оценена как Low в скоупе Medium-3 из r1.
|
||||
|
||||
### Low-1 — риск «хрупкость мутантов к рефакторингу подписи кэша» не вошёл в таблицу §7.2
|
||||
|
||||
**Файл:** `docs/specs/473-iso-perf-witnesses-and-smoke.md:131-138`
|
||||
|
||||
Находка r1 (Medium-3) перечисляла два конкретных риска, отсутствовавших в
|
||||
документе дословно: «шумность абсолютного порога на 3 образцах на
|
||||
разделяемом раннере» и «хрупкость четырёх новых мутантов к будущим
|
||||
рефакторингам подписи кэша». Новая таблица §7.2 закрывает первый (строка 2:
|
||||
«Абсолютный smoke-потолок на 3 образцах шумит на общем раннере и красит
|
||||
честные ветки»), но не содержит второго: ни в таблице, ни в §10 нет строки о
|
||||
том, что новые свидетели §4 читают конкретные поля подписи кэша
|
||||
(`shapeSignature`/`signature` — `anchor, plate, room, wallHeight, offset,
|
||||
shadows, selected, unitsPerPixel`) и что переименование или реструктуризация
|
||||
этих полей при будущем рефакторинге кэша способна тихо обнулить мутант, не
|
||||
роняя его при этом (мутант перестаёт целиться в существующее поле, а не
|
||||
перестаёт проходить).
|
||||
|
||||
Ремонт дешёвый — одна строка в таблице §7.2, например: «Рефакторинг состава
|
||||
подписи кэша (`shapeSignature`) переименует или уберёт поле, на которое
|
||||
целится мутант, и мутант перестанет быть репрезентативным без единого
|
||||
падения теста» → мера «имя поля упомянуто в тексте гарда/теста, ревью кода
|
||||
любой правки `shapeSignature` обязано свериться со списком мутантов §4».
|
||||
|
||||
Это Low, а не Medium: структурный пробел, из-за которого Medium-3 был
|
||||
выставлен (полное отсутствие разделов), закрыт — раздел «Риски» есть,
|
||||
таблица содержит по существу верные и проверенные пункты, а недостающая
|
||||
строка не влияет на достижимость ни одного AC и не меняет контракт свидетелей
|
||||
§4. Снимаю решением ревьюера с записью, а не возвращаю в новый цикл: цена
|
||||
пятого гипотетического упоминания риска ниже цены ещё одного раунда ревью на
|
||||
документ, который уже прошёл структурную проверку. Автор волен добавить
|
||||
строку при реализации без нового цикла ревью ТЗ.
|
||||
|
||||
## Что проверено и корректно (сверх унаследованного из r1)
|
||||
|
||||
- AC7 и AC8 вместе закрывают ровно то, что не закрывал старый AC7: AC7
|
||||
доказывает актуальные потолки вручную (как AC6), AC8 доказывает сам
|
||||
механизм диффозависимости без обращения к недостижимому CI-прогону этой
|
||||
ветки — пробел в цепочке доказательств не остался.
|
||||
- §1.1/§1.2 отвечают на оба продуктовых вопроса §7.1 PROCESS.md таким,
|
||||
каким они должны быть для инфраструктурной задачи: персона — автор+ревьюер
|
||||
задач отрисовки (не пользователь продукта — обосновано в r1 как
|
||||
legitimate для этого класса задач, прецедент #404/#399/#422), «что видит»
|
||||
сформулировано без терминов реализации («job краснеет с абсолютным
|
||||
потолком... до того, как ревью началось»).
|
||||
- §7.1 (UX/данные/i18n) корректно и минималистично: «не применимо» с
|
||||
причиной, не пустая формальность.
|
||||
- Новая нумерация разделов (1.1, 1.2, 7.1, 7.2) не создала коллизий с
|
||||
существующими §2–§10 и таблицей АC — сверено построчно, разделы идут в
|
||||
логическом порядке рядом с тем, что они дополняют.
|
||||
|
||||
## Унаследовано из r1 (без повторной проверки)
|
||||
|
||||
Документ: `docs/reviews/SPEC-REVIEW-473-r1.md`, материал: SHA
|
||||
`2cedcc6221e40ea6ead2f48ff7fb37ddc96a51ca` (дерево
|
||||
`41b642d61cff58dea0718d19efa6209b897a5a33`, блоб ТЗ
|
||||
`03c7d24cb503bea7ac083ed9a1ffc12a77b3c0e0`). Принято без повторной сверки,
|
||||
дельта не касается:
|
||||
|
||||
- обоснование полного трека (две поверхности, сложность 3) — критерий §5
|
||||
PROCESS.md для лёгкого трека нарушен корректно и без изменений;
|
||||
- точность построчных ссылок §4 на `iso-scene-render.ts` (783, 786, 805,
|
||||
821) и `iso-overlays.ts` (342-345), включая имена полей подписи кэша;
|
||||
- реализуемость AC1 (мутанты) и AC2 (envelope-юнит) — не задеты дельтой;
|
||||
- реализуемость AC3/AC5 как контрактных тестов над текстом
|
||||
`validate.yml` (regex-стиль `test/validate-workflow.test.mjs`);
|
||||
- реализуемость AC4 (smoke-бюджеты как урезанное подмножество полных
|
||||
профилей) — прецедент `budgets-glow-smoke.json`;
|
||||
- воспроизводимость AC6 на `de215578` (9870 мс против потолка 3500);
|
||||
- корректность строки `docs/specs/README.md` (не задета дельтой r2);
|
||||
- не-скоуп (§3) — не редактировался.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- `typecheck`/`test`/`build` — дифф раунда не содержит ни продуктового, ни
|
||||
тестового кода; станет предметом код-ревью.
|
||||
- Фактическая укладываемость новых профилей в 20-минутный лимит
|
||||
`performance_smoke` и фактическая шумность абсолютных порогов на общем
|
||||
раннере — не проверяемо на этапе ТЗ, задача сама называет это
|
||||
предположением/риском с планом отхода.
|
||||
- Не читал 122+ существующих мутанта `scripts/mutation-gate.mjs`
|
||||
построчно — не задето дельтой.
|
||||
- Не проверял, действительно ли фикстура «плита у стены» в существующих
|
||||
тестах даёт именно touching, а не overlap для мутанта AABB — ТЗ само
|
||||
помечает это открытым риском (§7.2) с планом добавить фикстуру касания;
|
||||
это станет предметом код-ревью через отрицательный прогон.
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- SHA ветки: `ab0463de7881c64f8b42bed1f007c7ce4e434cf9` (= HEAD на момент
|
||||
ревью, `git rev-parse HEAD` сверен непосредственно перед выводом)
|
||||
- Дерево ТЗ: `docs/specs/473-iso-perf-witnesses-and-smoke.md` на этом SHA
|
||||
- Базовый SHA дельты: `2cedcc6221e40ea6ead2f48ff7fb37ddc96a51ca` (материал r1)
|
||||
- `git diff 2cedcc62..ab0463de --stat`: `docs/reviews/SPEC-REVIEW-473-r1.md`
|
||||
(new, 236 lines — публикация r1), `docs/specs/473-iso-perf-witnesses-and-smoke.md`
|
||||
(+41/−5)
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/473-iso-perf-witnesses`, коммит `ab0463de7881` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `d4bba359ffb6d71f9283be0c3144eba6cffa154e`
|
||||
```
|
||||
git log --all --format='%H %T' | grep d4bba359ffb6
|
||||
```
|
||||
- ТЗ `docs/specs/473-iso-perf-witnesses-and-smoke.md`, блоб `5e2068d0dcab862fdf8964a3ec6966f1e73c2431`
|
||||
```
|
||||
git log --all --find-object=5e2068d0dcab862fdf8964a3ec6966f1e73c2431 -- docs/specs/473-iso-perf-witnesses-and-smoke.md
|
||||
```
|
||||
Reference in New Issue
Block a user