13 KiB
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 20git 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/169passed, 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-матрица).
Материал раунда
- Ветка:
issue/524-marker-shadow-transitions, коммит9ad8b1f71061— ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет. - Дерево материала:
77965278be6c5a537b749e6a865e000b0e3c73d9git log --all --format='%H %T' | grep 77965278be6c - Тело issue:
4021e888ec225919431efb04e4f8a06976a0d5cc12089679550b50ac80fce4bf - Вердикт конвейера:
green· High 0