mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-30 19:58:50 +00:00
claude[bot]
@@ -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 → в задаче
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `8afba7e6edb5` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `2acfb9ec47f68cd0c479f34c772597bb4569e96a`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 2acfb9ec47f6
|
||||
```
|
||||
- Тело issue: `12e66588332b94c78100156f01e4832881b8163fb6f8fdca83acfcb8e90f4545`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user