diff --git a/docs/reviews/SPEC-REVIEW-532-r2.md b/docs/reviews/SPEC-REVIEW-532-r2.md new file mode 100644 index 00000000..65f2969c --- /dev/null +++ b/docs/reviews/SPEC-REVIEW-532-r2.md @@ -0,0 +1,194 @@ +# SPEC-REVIEW-532-r2 + +Issue: #532 · Этап: ТЗ на ревью (S4-spec-review) · Заход r2 · блокирующих циклов израсходовано 0/4 (r1 — зелёный, бюджет не тратит, #227) + +## Скоуп ревью + +Материал — тело issue #532, раздел `## ТЗ`, редакция r2 (комментарий автора +«ТЗ r2 — на ревью», 2026-09-11T15:56:10Z). Это не первый заход, но контракт +изменился по существу (К2 переписан, AC3 переписан), поэтому по PROCESS.md +§2.10 разбор веду **полным**, а не только по находкам r1 — причина: «смена +контракта поведения», один из явных критериев, требующих полного разбора. +Основание: r1 получил зелёный вердикт на К2 «golden останется зелёным» +(комментарий ревьюера от 2026-09-11T15:31:18Z), затем автор сам вернул задачу +в `S3-spec`, потому что реализация это обещание опровергла (комментарий +«Возврат в `S3-spec`: К2 опровергнут замером», 15:54:23Z). Это не находка +ревью — обещание опровергла сама реализация, автор зафиксировал это честно и +переписал контракт, не подгоняя факт под старый текст. + +Точную дельту r1→r2 получил не из пересказа, а из истории редактирования тела +issue (`gh api graphql` → `Issue.userContentEdits`, три снимка: до появления +`## ТЗ`, редакция r1, редакция r2) и построил `diff -u` между снимками r1 и r2. + +## Как проверялось + +Читал в порядке из инструкции: `docs/SCOPE.md`, `PROCESS.md` (§1–§10.3), +`AGENTS.md`, тело issue #532 целиком (симптом → замер → аналитика → ТЗ) и все +5 комментариев, `docs/SUN.md` (канонический документ фона, раздел «Current +four-phase background»). Технические утверждения ТЗ сверял с кодом на дереве +`dev`@`8afba7e6` (рабочая копия): + +- `src/styles/plan.styles.ts:89-96` — селектор и стопка `drop-shadow` + дословно совпадают с К1/К2 (`.stage.daycycle .hp-paperg`, + `.hp-static-stage.daycycle .hp-paperg`); правки ещё нет, дерево в + состоянии «до» задачи — ожидаемо для стадии ТЗ; +- `demo/golden/matrix.mjs:6` — пороги сцены `stage`: `maxChannelDelta: 10`, + `maxDiffRatio: 0.0005`, ровно те, о которые разбивается К2 (3.43 % на + `day-cycle-night-dark` — превышение в ~68 раз, как и пишет автор); +- `demo/golden/matrix.mjs:458-472` — ровно 4 сцены с `bgMode: 'daynight'` во + всей матрице (`day-cycle-{dawn,day,dusk,night}-dark`), других нет; отсюда + утверждение К2 «никакой другой кадр матрицы измениться не имеет права» + проверяемо и заявление АС3 «ровно четыре» структурно верно — играть могут + только эти четыре записи; +- `src/glow-blend.ts:61` — `svgScreenBlendSupported` действительно рантайм- + проба, не читает `bg_mode` — подтверждает К4 кодом, а не только измерением + (унаследовано из r1, но перепроверил сам, это дёшево); +- `docs/SUN.md:59-86` — раздел «Current four-phase background» пока не + содержит упоминания композиционного слоя контура: правка AC5 добавляет + новую строку, а не противоречит существующей; стиль совпадает с уже + имеющимися архитектурными утверждениями раздела (например, про группировку + контура «outside the grouped plan-paper footprint»), так что место для + этого факта в каноне — правильное; +- `scripts/smoke-select.mjs:1-45` — `isExecutableFrontend` считает `.ts`-файл + исполняемым, значит `plan.styles.ts` инструмент разберёт; но извлекает он + символы, а изменённая строка — чистая CSS-декларация (`will-change: + filter;`) без идентификаторов проекта. Заявление риска 5 «селектор честно + отвечает «неопределённость»» правдоподобно по механике инструмента (нет + символа для точного совпадения, а «filter» как слово слишком частый, чтобы + пройти `BROAD_SHARE`); автор не выдаёт это за факт, а обещает прогнать и + назвать результат в хендоффе — это ровно то, что требует процесс; +- проверил, что `demo/smoke_daycycle_raster.mjs` и + `demo/benchmark_daycycle_raster.mjs` в дереве действительно отсутствуют — + ТЗ корректно называет свидетеля «создаётся», а не существующим. + +Не гонял ни одного гейта: на этапе ТЗ материал — текст, а не код; гейты +исполнимого кода к этому раунду не относятся (кода ещё нет — реализация, +проваленная в предыдущей попытке, не смержена, `git log` подтверждает: на +`dev` нет коммита #532). + +## Находки + +Нет. Ни одной блокирующей или требующей правки в скоупе задачи. + +Рассмотренные и снятые как небеспроблемные кандидаты (перечисляю, потому что +процесс требует не прятать сомнение, а не потому что они стали находками): + +1. **Порог бюджета 2.0 против переизмеренного «здорового» отношения 0.82** — + запас всего ~2.44×, у′же, чем на «около восьмикратный» из r1 (там был + 0.26). Автор сам скорректировал формулировку риска 4 (убрал «восьмикратный + запас», оставил «большой запас») — то есть не скрыл сужение. Порог отмечен + как «принято предположительно, менять свободно» — ревьюер вправе спорить, + и я не вижу оснований спорить: 2.0 всё ещё далеко и от здорового (0.82), и + от больного (15.06) значения, а фактический шум CI будет виден на реальном + прогоне свидетеля в код-ревью. +2. **AC3 экстраполирует порог «сдвиг среднего цвета ≤0.1/255» на все четыре + кадра, хотя пиксельно автор разобрал только `day-cycle-night-dark` + (сдвиг 0.07)** — для остальных трёх есть только «все четыре разошлись» без + разбора. Это не догадка, выданная за факт: у AC3 явный красный триггер + («пятый разошедшийся кадр либо сдвиг среднего цвета — находка ревью»), + значит фактическую проверку для всех четырёх кадров сделает код-ревью на + реальном артефакте golden, а не эта редакция ТЗ. Контракт проверяем, а не + голословен. +3. **`.hp-static-stage.daycycle .hp-paperg` (houseplan-space-card) получает + ту же подсказку, но ни одна сцена матрицы её не использует с + `bgMode: daynight`** — по коду это существующий пробел покрытия golden, + не появившийся из-за этой задачи и не расширяемый ею;К1 намеренно + трогает оба селектора одним правилом, а не только `.stage`. Не блокирует. +4. **Отложенная пересъёмка эталонов оставляет `dev`/nightly `golden` красным + до коммита на кандидате беты.** Это не самодеятельность автора: тот же + механизм (`Release:` + `Baseline-Reviewed:`, `golden:accept --reviewed`) + уже используется в проекте буквально соседним коммитом + (`f65d07e3 Принять три PDF-эталона, которые изменил #530`). Риск назван в + ТЗ явно (риск 2), цена не спрятана. + +## Закрытие раунда r1 + +r1 (`docs/reviews/SPEC-REVIEW-532-r1.md`, зелёный, материал — тело issue до +правки 2026-09-11T15:55:49Z) не вернул находок — цикл не был потрачен. +Возврат в `S3-spec` произошёл не по вердикту ревью, а по решению автора после +того, как собственная реализация опровергла обещание К2. Формально закрывать +нечего, но фиксирую, чем именно старое К2 расходится с новым и почему это не +недоработка автора, а корректная реакция на новый факт: + +| Что было в r1 | Что показала реализация | Чем закрыто в r2 | +|---|---|---| +| К2: «вид не меняется ни на пиксель сверх порога»; golden обязан остаться зелёным при `maxChannelDelta: 10` | 4 кадра `day-cycle-*-dark` разошлись, `day-cycle-night-dark` — 3.43 % пикселей, превышение `maxDiffRatio` в ~68 раз | К2 переписан: вид не меняется **содержательно** (ореол снаружи плана байт-в-байт, средний цвет кадра 176.59→176.66), но 4 эталона пересматриваются на кандидате беты | +| AC3: «четыре golden-кадра не изменились», проверка — `golden:verify` на полной матрице | Тот же прогон красный | AC3 переписан: «изменились ровно четыре, и вот чем» — список кадров + срез ореола + средний цвет; пятый кадр или больший сдвиг — красный | +| Риск 3 (r1): «Golden. Порог сцены 10, кадры обязаны остаться зелёными» | Не подтвердилось | Риск заменён на «Пересъёмка эталонов стоит руки владельца» — цена, а не гарантия | +| Порог бюджета 2.0 на числах 7.9/0.26 (демо-стенд, ховер) | Реальный смок на панораме дал другие абсолютные числа (15.06/0.82) | Числа в «Принято предположительно» обновлены на измеренные смоком, порог 2.0 оставлен — по-прежнему с запасом в обе стороны | + +## Унаследовано из r1 + +Без повторной проверки принято на слово из r1 то, чего дельта не касается, но +я всё же перепроверил дешёвые из этих утверждений сам (см. «Как проверялось») +вместо слепого наследования, так как они стоили пары `grep`: + +- **К1** (правило `will-change: filter` на связке двух селекторов, + ограничено классом `daycycle`) — не менялось между r1 и r2 текстуально; + перепроверил код (`plan.styles.ts:89-96`) сам, а не унаследовал. +- **К3** (четыре слоя окружения вне скоупа) и **К4** (экранный блендинг вне + скоупа, `svgScreenBlendSupported` — рантайм-проба) — не менялись; К4 + перепроверил кодом сам. +- **К5** (бюджет — отношение, а не абсолютные мс) — не менялся текстуально. +- **AC1, AC4** (вычисленный стиль `.hp-paperg` с/без `will-change` в двух + режимах фона) — не менялись по существу (только словесная правка «новый + смок» → «смок» и точное число в столбце AC2 вместо оценки «около 8»). +- **AC5** (документация `docs/SUN.md` + оба changelog) — не менялся. +- **Продуктовая рамка, скоуп/не-скоуп, откат (кроме добавленной фразы про + эталоны)** — не менялись. +- Источник: `docs/reviews/SPEC-REVIEW-532-r1.md`, материал — тело issue на + снимке до 2026-09-11T15:55:49Z (см. `userContentEdits`, узел 0, дерево + `dev`@`116cfd5f` на момент r1). + +## Что проверено и корректно + +- Полнота разделов по PROCESS.md §7.1: сценарий и «до/после» — первые в блоке + ТЗ; проблема разобрана выше `## ТЗ` (симптом, решающий опыт, замер) и ТЗ на + неё явно ссылается; скоуп/не-скоуп — «Чего задача не трогает»; контракт — + К1–К5; i18n, миграция, touch — закрыты одним перечислением «не трогает» + (`bg_mode` и его миграции, i18n, touch-контракт); план автотестов и AC — + таблица AC1–AC5 с «чем краснеет» у каждого; риски — 5 штук, включая новый; + откат — есть; release-артефакты — changelog RU+EN, `docs/SUN.md`, отдельный + коммит эталонов на кандидате беты. +- Трек: полный, критерий §5, который задача не проходит, назван явно («нет + влияния на производительность»). +- Ни одного продуктового вопроса владельцу не поднято и не нужно: все + открытые места (порог 2.0, `will-change: filter` vs `transform`, permanent + vs on-gesture, движок свидетеля) — технические, помечены «принято + предположительно, менять свободно», и я как ревьюер с ними согласен по + существу (см. «Как проверялось»). +- Не найдено ни одной догадки, выданной за факт: там, где вывод не доказан + на 100 % (антиалиасинг штриховки как причина, а не что-то ещё), автор дал + контрольный эксперимент (тот же эффект с `transform` вместо `filter`) и + явный красный триггер в AC3, а не декларацию «это точно так». + +## Чего не проверял + +- Не выполнял ни одного гейта (`typecheck`/`test`/`build`/golden) — на этой + стадии кода нет, гонять нечего; дерево `dev`@`8afba7e6` не содержит правки + #532 (реализация из провалившейся попытки не смержена). +- Не перепроверял сам сырые числа профиля Firefox с машины владельца + (объективно недоступны) — принимаю их как входные данные симптома, не как + предмет этого ревью. +- Не проверял, действительно ли `will-change` в реальном Gecko владельца + будет принят движком (риск 1) — это заявлено и ТЗ, и мной как + непроверяемая до реализации/профиля неопределённость, а не блокирующий + пробел контракта. + +## Вердикт + +Вердикт: зелёный · заход r2 · блокирующих циклов 0/4 · High: 0 · Medium: 0 → в задаче + +--- + + + +## Материал раунда + +- Ветка: `dev`, коммит `8afba7e6edb5` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. +- Дерево материала: `2acfb9ec47f68cd0c479f34c772597bb4569e96a` + ``` + git log --all --format='%H %T' | grep 2acfb9ec47f6 + ``` +- Тело issue: `12e66588332b94c78100156f01e4832881b8163fb6f8fdca83acfcb8e90f4545` +- Вердикт конвейера: `green` · High 0