mirror of
https://github.com/Matysh/houseplan-card
synced 2026-10-01 04:09:17 +00:00
claude[bot]
@@ -0,0 +1,82 @@
|
||||
# CODE-REVIEW-524-r1
|
||||
|
||||
Issue: #524 — «План тормозит в Firefox (низкий FPS), в Chromium тот же план идёт ровно»
|
||||
Этап: code · заход r1 · блокирующих циклов израсходовано 0 из 4
|
||||
Материал: ветка `issue/524-marker-shadow-transitions`, SHA `9ad8b1f71061df452252666abea102924389997b` (рабочая копия проверена на нём, `git status` чист на протяжении всего ревью)
|
||||
|
||||
## Скоуп
|
||||
|
||||
Четыре коммита поверх `f4625dd4` (уже на dev — документ спек-ревью #524 r1):
|
||||
|
||||
| коммит | что |
|
||||
|---|---|
|
||||
| `6b648016` | `box-shadow` убран из `transition` у `.device-shell-frame` и `.device-core` (`src/styles/devices.styles.ts`); новый смок `demo/smoke_marker_shadow_transitions.mjs`; мутант `marker-shadow-animates-again` в `scripts/mutation-gate.mjs`; записи в `docs/CHANGELOG.md`/`.ru.md`; правило в `docs/DEVELOPMENT.md`; три копии бандла |
|
||||
| `cd47a59d` | отпечаток скриншотов документации после правки |
|
||||
| `b59e245e` | пояснение в CSS-шаблоне сокращено (комментарии внутри `css\`\`` едут в браузер несжатыми минификатором) |
|
||||
| `9ad8b1f7` | отпечаток скриншотов после сокращения |
|
||||
|
||||
ТЗ — полный трек, критерий §5 не пройден намеренно («нет влияния на производительность» — задача целиком про кадр), spec-ревью r1 уже зелёное (документ `docs/reviews/SPEC-REVIEW-524-r1.md`, вердикт в комментарии issue).
|
||||
|
||||
Правка ограничена CSS: убирает анимацию `box-shadow` у маркера устройства (тень выражена в `cqw`-производных единицах через `--dev-size`, поэтому любой пересчёт контейнерных запросов — подсказка, скроллбар, поворот — перезапускал некомпозируемый переход разом на всех маркерах; у владельца в Firefox это 61 переход одновременно и 9,4 к/с). Геометрия, i18n, бэкенд, конфиг не затронуты — подтверждено чтением диффа (`git diff origin/dev...HEAD --stat`): единственный код-файл — `src/styles/devices.styles.ts`, остальное — реестр мутантов, документация и синхронизированные копии бандла.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
**Унаследовано от зелёного Validate на этом SHA** (https://github.com/Matysh/houseplan-card/actions/runs/34566652393, job «Фронтенд: типы, юниты, мутанты, синхрон бандла» и «Предполётные проверки»): `npx tsc --noEmit`, `npm test`, `npm run build` со сверкой бандла, `bundle:budget`, `no-new-any`, `node scripts/check-docs.mjs`, реестровые (дешёвые) тесты мутации, `mutation-gate` по диффу (6/6 мутантов, включая наш новый). Не перегонял — прогон подтверждён ссылкой, дерево не менялось.
|
||||
|
||||
Job «Смоки в браузере» и «Golden» на этом прогоне **skipped** — не баг реюза, а штатное поведение `scripts/classify-changes.mjs::heavyGatesRequested`: тяжёлые job на обычном push не запускаются, они предназначены для PR/кандидата беты/ночного прогона (см. комментарий в файле, #479/#510). На этой стадии их прогон — моя обязанность.
|
||||
|
||||
Прогнал сам:
|
||||
- **`node demo/smoke_marker_shadow_transitions.mjs`** — после `npm run bundle:sync` (демо-стенд `demo/srv/assets` пуст в свежем чекауте, гитигнорится; пересборка дала побайтово тот же бандл, что и закоммиченный — `git status` остался чист). Результат: `OK`.
|
||||
- **Дисциплина «тест умеет падать»**: вручную вернул `box-shadow` в `transition` `.device-shell-frame` (мутация `marker-shadow-animates-again`), пересобрал, перезапустил тот же смок — упал ожидаемо:
|
||||
```
|
||||
FAILED (3):
|
||||
- containerResizeStartsNoShadowTransition: expected true, got false
|
||||
- shadowTransitionsOnResize: expected 0, got 20
|
||||
- resizeStartsNothingOnMarkers: expected 0, got 20
|
||||
```
|
||||
Откатил правку (`git checkout -- src/styles/devices.styles.ts`), пересобрал — дерево снова чисто.
|
||||
- **`node scripts/smoke-select.mjs --base f4625dd4 --head 9ad8b1f7`** — вернул НЕОПРЕДЕЛЁННОСТЬ (правка чисто CSS-шная, инструмент не находит символа для связывания). Решение: помимо выделенного нового смока других браузерных смоков не прогонял — правка не трогает ни один JS-символ, задевает только два CSS-правила одного файла, оба целиком покрыты новым свидетелем; вероятность, что какой-то из 240 остальных смоков наблюдает именно транзишен `box-shadow` маркеров, оцениваю как исчезающую.
|
||||
- **`npm run golden:verify`**, полная матрица 169/169 сцен (baselines: `ls demo/golden/baselines/*.png | wc -l` = 169) — все `passed`, код выхода 0. Автор смог прогнать только 158/169 в песочнице (обрыв по времени); я прогнал всю матрицу без обрыва. Закрывает AC4 без остатка неопределённости.
|
||||
|
||||
Не прогонял и почему:
|
||||
- **`npm run invariants`** — diff не трогает рёбра комнат, `layout`, `marker.space`, `open_spans`; правка не меняет ни одной геометрической величины, только CSS-анимацию. Не требуется.
|
||||
- **`python -m pytest tests_backend`** — `custom_components/**/*.py` не затронут.
|
||||
- **Performance-профили** (`benchmark_*`) — не названы в AC; AC1 сознательно судит причину (число стартов `transitionrun`), а не время кадра, поэтому перф-гейт не нужен и в ТЗ помечен как ненужный отдельно.
|
||||
- **«Одно число — один источник»** — неприменимо: правка не добавляет и не меняет ни одной видимой пользователю величины, только удаляет анимацию перехода уже существующей тени.
|
||||
|
||||
## Проверка AC
|
||||
|
||||
- **AC1** (смоки, п. выше) — подтверждено самостоятельным прогоном и повторной проверкой на возвращённой мутации (тест красится, как обязан).
|
||||
- **AC2** — тот же прогон: `hoverAnimatesBorder` (по длинным формам `border-*-color`, а не по короткой `border-color` — браузер возвращает длинную форму, смок это учитывает) и `hoverStartsNoShadow` — оба `true`, входят в `checkAll` без исключений (`demo/serve.mjs:75-78`: любой ключ результата, не перечисленный в `expected`, обязан быть `true`).
|
||||
- **AC3** — там же: `selectionRingAppearsInOneFrame` и `selectionRingDoesNotAnimate` — оба `true`. По коду: `.dev.sel`/`.dev:focus-visible` меняют только `--device-ring-color`/`--device-ring-width` (`devices.styles.ts:404-410`), которые входят в `box-shadow` `.device-core` (`:261-263`), а транзишен `.device-core` (`:267`) `box-shadow` больше не содержит — новое значение обязано применяться мгновенно. Проверено и чтением, и исполнением.
|
||||
- **AC4** — `docs:accept --identical` (11 кадров документации, коммит `cd47a59d`) плюс мой полный прогон `golden:verify` (169/169 `passed`, exit 0). Ожидаемо: golden снимает кадры с `animations: 'disabled'` (`demo/golden/run.mjs`), удаление перехода не может сдвинуть финальный пиксель.
|
||||
- **AC5** — мутант `marker-shadow-animates-again` анкорится ровно один раз (подтверждено и unit-тестом реестра в зелёном `npm test`, и тем, что я применил его патч руками — текст совпал побайтово), гард — `demo/smoke_marker_shadow_transitions.mjs`, воспроизведено падение вручную (см. выше). Мутация нацелена только на `.device-shell-frame`, не на `.device-core` — это ровно объём, заявленный в ТЗ (АС5 «красит свидетеля AC1», без требования второго мутанта на ядро), не пробел код-ревью.
|
||||
- **AC6** — записи в `docs/CHANGELOG.md`/`docs/CHANGELOG.ru.md` и правило в `docs/DEVELOPMENT.md` — все три в одном коммите `6b648016` (`User-Visible: yes`, `Issue: #524`), проверено `git show --stat`.
|
||||
|
||||
## Проверено чтением и корректно
|
||||
|
||||
- Оба места правки (`devices.styles.ts:213`, `:267`) — единственные во всём `src/**` вхождения `transition: … box-shadow …` (`grep -n "transition:.*box-shadow" -r src/styles/` — пусто) и единственные оставшиеся упоминания `box-shadow` в `devices.styles.ts` вне них — это `box-shadow: none`, статичное значение `.device-core`/`.vacpuck`-тени и `@keyframes vacpulse` (использует фиксированные `px`, не `cqw`-производные — правило К4 на него не распространяется, это не находка).
|
||||
- К2 (значения теней не изменились) — `--device-shell-shadow`, `--device-core-inset-shadow`, `--device-ring-*` не тронуты диффом (только строки `transition:`).
|
||||
- Комментарии-объяснения в CSS сокращены до одной строки на правило (`b59e245e`) — с обоснованной причиной (комментарии в `css\`\`` шаблоне не срезаются минификатором и едут в бандл; измерено: 300 577 Б против 300 111 Б). Общая проблема (128 таких комментариев в шаблонах) заведена отдельно как #526 — корректно, не эта задача.
|
||||
|
||||
## Находки
|
||||
|
||||
Нет. Ни одной находки уровня High или Medium; расхождений между ТЗ, кодом, тестами и документацией не обнаружено.
|
||||
|
||||
## Вердикт
|
||||
|
||||
Зелёный. Контракт К1–К5 выполнен точно по тексту ТЗ, все шесть AC доказаны — частью автотестами (пересмотренными мной и лично проверенными на падение под мутацией), частью самостоятельным исполнением непрогнанных в CI тяжёлых гейтов (смок, полная golden-матрица).
|
||||
|
||||
---
|
||||
|
||||
<!-- material-anchors: сгенерировано конвейером (#414) -->
|
||||
|
||||
## Материал раунда
|
||||
|
||||
- Ветка: `issue/524-marker-shadow-transitions`, коммит `9ad8b1f71061` — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
|
||||
- Дерево материала: `77965278be6c5a537b749e6a865e000b0e3c73d9`
|
||||
```
|
||||
git log --all --format='%H %T' | grep 77965278be6c
|
||||
```
|
||||
- Тело issue: `4021e888ec225919431efb04e4f8a06976a0d5cc12089679550b50ac80fce4bf`
|
||||
- Вердикт конвейера: `green` · High 0
|
||||
Reference in New Issue
Block a user