fix(bench): измерять полный handoff в каждом sample - #298
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (3)
Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour. 📝 WalkthroughWalkthroughДобавлена фабрика lifecycle-sample для проверки compositor-to-live handoff. Benchmark создаёт отдельный контроллер для каждой итерации. Тесты проверяют counts, уничтожение live-контроллера и отказ при повторном использовании. ChangesЖизненный цикл handoff
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change makes each compositor-to-live handoff benchmark sample use a fresh controller and verifies the expected lifecycle effects. No merge-blocking risk remains. Sequence Diagram(s)sequenceDiagram
participant Benchmark
participant CompositorSpring
participant LiveMotionValue
participant LifecycleSample
Benchmark->>LifecycleSample: create independent sample
LifecycleSample->>CompositorSpring: create and start controller
Benchmark->>CompositorSpring: handoffToLive()
CompositorSpring->>LiveMotionValue: return live controller
Benchmark->>LifecycleSample: verify(live)
LifecycleSample->>LifecycleSample: validate counts and final values
Benchmark->>LiveMotionValue: destroy()
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (8 passed)
Full details: тестыExplanation Тесты не доказывают целевое исправление стенда. Resolution Добавьте тест, который проходит через фактическую оркестрацию benchmark:
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Новые guards на30a851444f5bdb8614f15ea8a866d58df40e2832: независимо сверяются serialized donor position/velocity, реальные clock/currentTime reads, phase-specific effects; неподдерживаемые keyframe/timing modifiers отклоняются, cleanup выполняется даже при исключении phase observer. Фактические counterexamples steps(1)/неверные конечные числа/пропущенное чтение/частичный start/observer failure превращены в RED-регрессии. Focused44PASS; static/full suite4014PASS/21skip. Собранный controller прошёл200 lifecycle samples, не performance admission. Новый exact-head CI/browser и свежий независимый donor-contract reviewer запущены; прежний PASS не перенесён на новый SHA. |
|
@coderabbitai review |
|
lemone112
left a comment
lemone112
left a comment
There was a problem hiding this comment.
Автоматизация из ChatGPT от агента GPT-5.6 Sol
Вердикт: merge-blocker не доказан на exact head a13f214c.
Validity envelope: base 867254c9; changed surface — benchmark/test only; production src/exports/dependencies не затронуты. Exact-head GitHub Actions: browser Chromium/Firefox/WebKit и consumer/CI checks завершены SUCCESS. CodeRabbit содержательно покрывал старый 0dac03c; его прежний pre-merge finding про отсутствие orchestration-теста на финальном head SUPERSEDED: текущий diff экспортирует общий measureLatency, вводит createCompositorHandoffLatencyScenario/measureCompositorHandoffLatency и тестирует передачу setup→op→verify→teardown, включая failure/cleanup paths.
Proof obligations:
- BENCH-298-01 fresh owner per sample / отсутствие старого no-op пути — NO COUNTEREXAMPLE FOUND. CHECK=YES для exact-head remote suite; source inspection подтверждает, что CLI и lifecycle tests используют один runner/scenario. Это эмпирическое доказательство, не формальная теорема.
- BENCH-298-02 cleanup при op/observer/setup failure — NO COUNTEREXAMPLE FOUND. CHECK=YES: в diff есть адресные failure-path tests; exact-head CI зелёный.
- BENCH-298-03 donor oracle не должен импортировать production-reader как истину — FACT, CHECK=YES по diff: donor execution разбирается в benchmark support самостоятельно.
Evidence ledger: FACT — production runtime не изменён; FACT — exact-head remote gates зелёные; FACT — старый CodeRabbit test-theater finding устранён на более новом snapshot. ASSUMPTION — GitHub-hosted CI достаточно репрезентативен для заявленного benchmark-contract; PR не заявляет абсолютные performance numbers, поэтому это допущение не расширяет контракт.
Независимый контрпример, переживший текущие tests/gates, не найден. Autofix: НЕ БЫЛО.
|
Steward disposition: SUPERSEDED / close, не merge candidate. FACT: current Поэтому исторические receipts #298 остаются evidence, но старую 8-commit ветку не rebasing и не переносим в main. Если в #302/#300 при current-main пересборке обнаружится конкретное уникальное свойство #298, переносится только этот bounded counterexample, а не carrier целиком. Invalidation trigger: новый current plan снова назначит #298/его exact benchmark отдельным owner. |
1 participant
Пользовательский результат
Handoff-бенчмарк больше не измеряет повторно использованный live-controller как быстрый no-op и не принимает формально похожие, но неверные donor-планы.
Контракт
Каждый sample получает нового владельца. Setup, timed handoff, проверка и cleanup разделены. Вне тайминга проверяются реальные animate/cancel/requestFrame и чтения часов, а значение/скорость live сверяются с независимым снимком замороженного фактически переданного donor-плана на 16 мс.
Поддерживаются две конечные формы локального emitter: CSS linear() и explicit linear keyframes. Это проверка сохранения фактической позиции/скорости при передаче, не повторная проверка физического ОДУ.
Доказательство
Архитектура
Production src/exports/dependencies не меняются. Эталон принадлежит стенду и читает фактический donor-план, не импортирует production-reader как определение правильности. Cleanup выполняется и при сбоях observer/частичного setup.
Производительность
Прежние 100/100/100 нс no-op не являются доказательством производительности. Этот PR чинит валидность измерения, не заявляет speedup, tail latency или отсутствие регрессий во всех средах.
Риски и не-цели
Не общий CSS-парсер и не proof произвольного browser rendering. Accessor/Proxy adversaries и будущие WebIDL fields не покрыты. Browser residency/paint и физическая плавность требуют отдельных проверок #300/#301.
Документация и выпуск
Динамические результаты находятся в PR, версия/CHANGELOG не меняются. Merge/release/deploy НЕ БЫЛО. Откат после возможного merge: revert этого PR.
Гейты
Связано с #295; стек поверх #295. Не включает #296/#297.