mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
claude[bot]
@@ -0,0 +1,165 @@
|
||||
# SPEC-REVIEW-524-r1
|
||||
|
||||
**Issue:** [#524](https://github.com/Matysh/houseplan-card/issues/524) — «План тормозит в Firefox (низкий FPS), в Chromium тот же план идёт ровно»
|
||||
**Этап:** S4-spec-review · заход r1 · блокирующих циклов израсходовано 0 из 4
|
||||
**Ревьюер:** Claude (ревьюер ТЗ) · **Автор ТЗ:** Codex — разные роли, разные модели, требование §6 соблюдено
|
||||
**Материал:** раздел `## ТЗ` в теле issue #524, ревизия на момент ревью
|
||||
**SHA256(нормализованное тело раздела `## ТЗ`):** `a8169e9d730a1339235136f18a2a0071d900eb6a415cd9ba8a5388f87ff065a4`
|
||||
**Рабочее дерево репозитория на момент ревью:** `8ba87b3ff70f41e387dc0422ab53c0eced49ef19` (`dev`) — использовалось только для сверки утверждений ТЗ с текущим кодом, продуктовый код не менялся
|
||||
|
||||
## Вердикт
|
||||
|
||||
**Зелёный.** High: 0. Medium: 0. Low: 1 (снят с записью, см. ниже).
|
||||
|
||||
## Скоуп проверки
|
||||
|
||||
Задача — деградация FPS в Firefox на плане с 61 маркером устройства. S2-аналитика
|
||||
(два раунда, оба от владельца/аналитика в комментариях issue) довела причину до
|
||||
профиля живой машины: `--device-shell-shadow` и производные от неё
|
||||
`--device-ring-*` выражены через `--dev-size`, которая происходит от контейнерной
|
||||
единицы (`--icon-size, 2.5cqw`). Любой пересчёт контейнерных запросов меняет
|
||||
вычисленный `box-shadow` у каждого маркера и — поскольку он в списке `transition`
|
||||
— запускает на каждом некомпозируемый 150-мс переход. У владельца это 61
|
||||
одновременный переход, Gecko с этим объёмом перерисовки падает до 9,4 к/с.
|
||||
ТЗ фиксирует минимальную правку: убрать `box-shadow` из списка `transition` на
|
||||
двух узлах маркера, сохранив сами значения тени и переходы `border-color`/`opacity`/`background`/`color`.
|
||||
|
||||
Проверял:
|
||||
1. Каждое числовое и текстовое утверждение ТЗ против текущего кода на
|
||||
`8ba87b3f` (не правил код, только читал).
|
||||
2. Полноту и проверяемость AC1…AC6.
|
||||
3. Отсутствие невидимых предположений, выданных за факт (§7.1).
|
||||
4. Соответствие обязательным разделам ТЗ (PROCESS.md §7.1) и выбору полного
|
||||
трека (§5).
|
||||
5. Не расширяет ли ТЗ скоуп сверх заявленного и не оставляет ли открытых
|
||||
продуктовых вопросов, которые следовало адресовать владельцу.
|
||||
|
||||
## Находки
|
||||
|
||||
Находок, блокирующих переход, нет. Одна находка низкой серьёзности снята
|
||||
решением ревьюера (см. ниже) — фиксирую только для протокола, доработки не
|
||||
требует.
|
||||
|
||||
### Low — ТЗ не выделяет отдельными разделами «сценарий» и «что человек увидит до/после» (снято)
|
||||
|
||||
PROCESS.md §7.1 требует эти два раздела первыми в ТЗ. В теле `## ТЗ` их нет как
|
||||
отдельных заголовков: текст сразу переходит к «Причина установлена…» и
|
||||
«Контракту». Содержательно ответ на оба вопроса есть — но размазан по
|
||||
преамбуле issue («Симптом») и по S2-комментариям, а не сформулирован в самом
|
||||
ТЗ.
|
||||
|
||||
**Почему снимаю, а не возвращаю на правки:** AC4 (golden + `docs:accept
|
||||
--identical`) прямо гарантирует нулевую визуальную дельту — то есть «что человек
|
||||
увидит после» тривиально: то же самое, что и до, только без подвисания при
|
||||
наведении/панораме в Firefox. Формальный заголовок ничего не добавил бы к уже
|
||||
проверяемому контракту, а задача целиком описывается одним предложением без
|
||||
риска расхождения трактовок. Дописать один абзац дёшево, но отсутствие его не
|
||||
создаёт неоднозначности ни для одного AC — в отличие от типичного случая,
|
||||
когда пропуск этого раздела прячет продуктовое решение под видом технического.
|
||||
|
||||
## Что проверено и подтверждено соответствующим коду
|
||||
|
||||
Сверял построчно с `src/styles/devices.styles.ts` на `8ba87b3f`:
|
||||
|
||||
- `--device-shell-shadow` (строки 128–131) и её копии в `.theme-light`/`.theme-dark`
|
||||
(155–159, 166–169) действительно выражены через `calc(var(--dev-size) * …)`,
|
||||
а `--dev-size` (строка 114) — через `--device-base-size, 2.25cqw` (контейнерная
|
||||
единица). Цепочка зависимости от `cqw`, на которой строится вся причина,
|
||||
подтверждена, а не принята на слово.
|
||||
- `.device-shell-frame` (200) действительно имеет `box-shadow: var(--device-shell-shadow)`
|
||||
(211) и `transition: border-color .15s, box-shadow .15s, opacity .2s` (212) —
|
||||
точное совпадение с адресом из К1.
|
||||
- `.device-core` (247) действительно имеет `transition: background .15s, color
|
||||
.15s, box-shadow .15s, opacity .2s` (264) — точное совпадение со вторым
|
||||
адресом К1.
|
||||
- **Проверил экстенсивностью правки:** `grep -n "transition:.*box-shadow"` по
|
||||
всему `src/**` даёт ровно эти два совпадения. ТЗ не пропускает третье место,
|
||||
которое требовало бы такой же правки, и не описывает несуществующий рефакторинг
|
||||
— весь `box-shadow`-анимационный сюрфейс проекта — это два правила, оба
|
||||
названы.
|
||||
- `.vacpuck` (498–524) тоже несёт статичный `box-shadow`, но он не в `--dev-size`
|
||||
и не в списке `transition` — ТЗ справедливо его не касается.
|
||||
- К3 (кольцо выделения/фокуса): `.dev.sel`/`.dev.focus` (402–408) действительно
|
||||
задают `--device-ring-color`/`--device-ring-width` (последняя тоже через
|
||||
`--dev-size`), и они попадают в `box-shadow` `.device-core` (260–262) как
|
||||
дочерний узел `.dev` (`device-face.ts:116–118` — `.device-shell-frame` и
|
||||
`.device-core` рендерятся внутри `.dev`, custom properties наследуются).
|
||||
После снятия `box-shadow` из transition (К1) появление кольца станет
|
||||
мгновенным, как и заявляет К3 и AC3 — логика согласована с кодом, а не
|
||||
постулирована.
|
||||
- Assumption #2 («переход запускается одинаково в обоих движках, разница — в
|
||||
цене перерисовки») дословно опирается на измерение из S2-комментария
|
||||
(«1 px ширины → по два перехода на маркер и там, и там, 1220 переходов за
|
||||
2 секунды в обоих браузерах»); это не техническая догадка автора ТЗ, а перенос
|
||||
уже полученного экспериментального факта — граница между «предположением» и
|
||||
«фактом» в блоке соблюдена корректно.
|
||||
- AC1–AC3 используют паттерн (`transitionrun` на теневом корне,
|
||||
`getAnimations()`), уже проверенный в проекте на другой задаче —
|
||||
`demo/smoke_space_switch_transitions.mjs` (#525) обходит вложенные shadow
|
||||
root тем же приёмом. Метод не гипотетический, он воспроизводим существующим
|
||||
кодом смоков.
|
||||
- AC5 (мутант): возврат `box-shadow` в `transition` `.device-shell-frame`
|
||||
действительно ломает именно то, что проверяет AC1 (событие `transitionrun`
|
||||
с `propertyName === 'box-shadow'` появится) — свидетель назван верно и
|
||||
однозначно, соответствует формату §2.7/#435 (заранее, для будущего код-ревью).
|
||||
- Touch: markers показываются и в View/kiosk, не только в редакторах: правильно,
|
||||
что раздел Touch это учитывает, а не ограничивается редакторами. Блокирующих
|
||||
гарантий `docs/TOUCH-SUPPORT.md` правка не касается — это верно, поскольку
|
||||
AC4 гарантирует нулевую визуальную дельту в статике, а мгновенное вместо
|
||||
анимированного кольцо не входит в перечень touch-гарантий.
|
||||
- Трек: назван нарушенный критерий §5 явно («нет влияния на производительность»)
|
||||
— соответствует требованию не обосновывать «обычный трек» ощущением.
|
||||
- Предположения (раздел «Принятые технические предположения») промаркированы
|
||||
как предположения, а не выданы за факт — ровно то, что требует §7.1.
|
||||
- Откат, риски, i18n/миграция/бэкенд, release-артефакты — присутствуют,
|
||||
сформулированы конкретно и проверяемо (файлы, а не «обновить документацию»).
|
||||
|
||||
## Побочное наблюдение (не находка, не блокирует)
|
||||
|
||||
`src/styles/plan.styles.ts:522–533` несёт комментарий из #525, поясняющий, что
|
||||
`.device-shell-frame` `box-shadow` нужно было ключевать в списках, потому что
|
||||
он анимируется — после правки #524 это конкретное свойство перестаёт
|
||||
анимироваться на этом узле, и цитата в комментарии («device markers
|
||||
(.device-shell-frame, box-shadow) with this issue») станет частично неточной
|
||||
(сам принцип «анимируемое в списке требует ключа» останется в силе через
|
||||
`border-color`/`opacity`). Это правка в чужом файле вне «Затронутых файлов»
|
||||
ТЗ #524, чинить её здесь было бы расширением скоупа (§9 правил). Оставляю как
|
||||
наблюдение для код-ревью — если исполнитель поправит формулировку по пути,
|
||||
это уместно; если нет, для AC #524 это не значимо.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- Не запускал никакой код и не выполнял скрипты гейтов — на этапе ревью ТЗ
|
||||
продуктового кода ещё нет, задача проверить формулировку и её выполнимость по
|
||||
существующему коду, что и сделано чтением.
|
||||
- Не проверял глубже вопрос «почему ширина контейнера пересчитывается 138 раз
|
||||
за 11 секунд» — К5 прямо выносит его за периметр этой задачи, и это
|
||||
корректное решение объёма, не пробел.
|
||||
- Не оценивал целесообразность отдельной Firefox-полосы в CI — К5 и S2-комментарий
|
||||
сами откладывают это отдельным issue, в периметр ревью ТЗ #524 не входит.
|
||||
|
||||
## Унаследовано / раунды
|
||||
|
||||
Не применимо — это первый заход (r1) по этой задаче.
|
||||
|
||||
## Рекомендация
|
||||
|
||||
Перевести issue в `S5-ready`. AC пронумерованы, каждый называет способ
|
||||
доказательства и заведомо испытан существующим в проекте паттерном смока;
|
||||
причина подтверждена профилем живой машины и перепроверена по коду;
|
||||
предположения промаркированы; откат и риски названы. Единственная находка —
|
||||
Low, снята с запиской, доработки не требует.
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `dev`, коммит `8ba87b3ff70f` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `5d55bc4762834d175d8dc16f5d453a0d84ae3ad5`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 5d55bc476283
|
||||
```
|
||||
- Тело issue: `4021e888ec225919431efb04e4f8a06976a0d5cc12089679550b50ac80fce4bf`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user