Files
houseplan-card/docs/reviews/CODE-REVIEW-524-r1.md
2026-09-11 05:54:09 +00:00

13 KiB
Raw Permalink Blame History

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-матрица).


Материал раунда

  • Ветка: issue/524-marker-shadow-transitions, коммит 9ad8b1f71061 — ребейз его осиротит, и это нормально: ниже якоря, которые ребейз не меняет.
  • Дерево материала: 77965278be6c5a537b749e6a865e000b0e3c73d9
    git log --all --format='%H %T' | grep 77965278be6c
    
  • Тело issue: 4021e888ec225919431efb04e4f8a06976a0d5cc12089679550b50ac80fce4bf
  • Вердикт конвейера: green · High 0