Skip to content

fix(timeline): coalesce scrub state updates per frame - #520

Open
arhxam wants to merge 1 commit into
getopenscreen:mainfrom
arhxam:codex/coalesce-timeline-scrub-state
Open

fix(timeline): coalesce scrub state updates per frame#520
arhxam wants to merge 1 commit into
getopenscreen:mainfrom
arhxam:codex/coalesce-timeline-scrub-state

Conversation

@arhxam

@arhxam arhxam commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • keep the scrub playhead's direct DOM movement at pointer-event speed
  • move the optimistic React timecode state into the existing requestAnimationFrame seek
  • publish only the latest pending pointer position once per frame
  • add a React Profiler regression test that measures commits before and after a controlled frame

Root cause

Timeline scrubbing already had two performance protections: direct DOM movement for the playhead and an animation-frame-coalesced store/native seek. setScrubbingTimeSec, however, sat between them and ran on every pointermove, forcing the timeline subtree to re-render at raw pointer frequency.

Pointer-down remains immediate. During a drag, local scrub state and the store/native seek now publish together once per animation frame, while the direct playhead write preserves zero-latency visual tracking.

Related issue

Fixes #407

Type of change

  • Bug fix
  • Feature
  • Enhancement
  • Documentation
  • Refactor / maintenance
  • Performance
  • Security

Release impact

  • Patch
  • Minor
  • Major / breaking change
  • No release note needed

Desktop impact

  • Windows
  • macOS
  • Linux
  • Installer / packaging
  • Not platform-specific

Screenshots / video

No visual change. The timecode still follows the latest pointer position each rendered frame.

Testing

  • TDD red phase: three pre-frame pointer moves produced three extra Profiler commits (8 observed vs 5 expected)
  • the same test now observes no pre-frame commits, one post-frame commit, and one seek to the latest position
  • npx vitest --run src/components/ai-edition/v4/V4Timeline.geometry.test.tsx src/components/ai-edition/v4/V4Timeline.waveform.test.tsx — 17 passed
  • npx tsc --noEmit
  • npx tsc -p tsconfig.test.json --noEmit
  • npm run lint — no errors (14 pre-existing warnings)
  • npm run test — 181 files, 2,164 passed, 2 skipped
  • git diff --check

Summary by CodeRabbit

  • Bug Fixes

    • Improved timeline scrubbing performance by limiting state updates to once per animation frame.
    • Reduced unnecessary rendering during pointer dragging while keeping playhead movement responsive.
  • Tests

    • Added coverage verifying that scrubbing updates are deferred and consolidated correctly during drag interactions.

@arhxam
arhxam requested a review from EtienneLescot as a code owner August 28, 2026 02:22
@coderabbitai

coderabbitai Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b2398f4e-78c9-40d3-a92a-450a9283f39a

📥 Commits

Reviewing files that changed from the base of the PR and between 059f4e8 and 363e716.

📒 Files selected for processing (2)
  • src/components/ai-edition/v4/V4Timeline.geometry.test.tsx
  • src/components/ai-edition/v4/V4Timeline.tsx

Included review availability: Your plan provides up to 8 included reviews per hour; 4 remain after this review.


📝 Walkthrough

Walkthrough

V4Timeline now throttles scrubbing React state updates through requestAnimationFrame. Tests verify that pointer moves do not publish updates before the frame callback and publish one time update afterward.

Changes

Timeline scrubbing

Layer / File(s) Summary
Scrubbing update throttling
src/components/ai-edition/v4/V4Timeline.tsx
seekToClientX updates scrubbingTimeSec immediately only for immediate seeks. Throttled seeks update it with the existing animation-frame callback and setCurrentTime.
Deferred scrubbing test coverage
src/components/ai-edition/v4/V4Timeline.geometry.test.tsx
The test helper supports profiling and returns the time-update mock. The scrubbing test verifies that pointer moves defer updates until the animation frame, then publish one update with 720.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 363e7

Timeline scrubbing now batches React timecode updates once per animation frame while preserving immediate playhead movement and latest-position seeking. No actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: etiennelescot

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: coalescing timeline scrub state updates once per animation frame.
Description check ✅ Passed The description is complete and follows the repository template. It explains the change, links issue #407, identifies the change types and impacts, notes the lack of visual change, and lists testing p…
Linked Issues check ✅ Passed The changes satisfy issue #407. Scrubbing React state updates now occur in the existing requestAnimationFrame flow, while direct DOM playhead updates remain immediate. The regression test verifies one…
Out of Scope Changes check ✅ Passed The changes are within scope. They modify timeline scrubbing behavior and add focused regression coverage for the linked performance issue. No unrelated code changes are identified.
Full details: Description check

Explanation

The description is complete and follows the repository template. It explains the change, links issue #407, identifies the change types and impacts, notes the lack of visual change, and lists testing performed.

Full details: Linked Issues check

Explanation

The changes satisfy issue #407. Scrubbing React state updates now occur in the existing requestAnimationFrame flow, while direct DOM playhead updates remain immediate. The regression test verifies one update per controlled frame.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: scrubbing the timeline fires a React state update per pointermove

1 participant