fix(export): keep speed segments non-overlapping

[arhxam]

Summary

• clamp overlapping speed regions to the monotonic output cursor
• preserve the documented earliest-starting-region-wins behavior
• drop later speed regions that are fully covered, preventing duplicate decoding and output-time drift

Related issue

No linked issue; found with overlapping speed-region property cases.

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

Not applicable; export timeline segmentation logic.

Testing

• npx vitest --run src/lib/exporter/timelineSegments.test.ts (18 passed)
• npx vitest --run --exclude electron/recording/webm-seek-index.test.ts (1,532 passed)
• npx tsc --noEmit
• npx tsc -p tsconfig.test.json --noEmit
• npm run lint (passes with 11 existing warnings)
• npm run docs:check
• npm run i18n:check
• npx vite build

Summary by CodeRabbit

• Bug Fixes

  • Improved speed-based timeline segmentation when speed regions overlap or are nested.
  • Prevented duplicate, backward, or incorrectly ordered timeline segments.
  • Ensured normal-speed gaps are included only when they contain actual content.
  • Improved the consistency and reliability of exported timeline segments.

• Tests

  • Added coverage for overlapping and fully contained speed regions.

[coderabbitai]

Warning

Review limit reached

@EtienneLescot, you've reached your PR review limit, so we couldn't start this review.

Next review available in:5 seconds

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c09ea1c1-6ddc-4613-b13d-25cf0e86fe45

📥 Commits

Reviewing files that changed from the base of the PR and between 3557be4 and 5a38403.

📒 Files selected for processing (2)

• src/lib/exporter/timelineSegments.test.ts
• src/lib/exporter/timelineSegments.ts

📝 Walkthrough

Walkthrough

splitBySpeed now produces disjoint timeline segments for overlapping and nested speed regions. Tests verify earlier-region precedence, ignored contained regions, and correct 1x gap segments.

Changes

Speed region handling

Layer / File(s)	Summary
Disjoint speed segment generation src/lib/exporter/timelineSegments.ts, src/lib/exporter/timelineSegments.test.ts	splitBySpeed clamps overlapping regions, skips covered regions, and emits only non-empty 1x gaps. Tests cover overlapping and fully contained regions.

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

Possibly related PRs

• getopenscreen/openscreen#80: Introduced the splitBySpeed implementation that this PR updates for overlapping and nested regions.

Suggested reviewers:etiennelescot

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: preventing speed segments from overlapping in the export logic.
Description check	✅ Passed	The description includes all required sections: summary, issue reference, type of change, release impact, desktop impact, and testing. All sections are appropriately filled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[EtienneLescot]

The clamp is right, and the two tests pin exactly the cases that were broken. if (srEnd <= cursor) continue plus effectiveStart = Math.max(srStart, cursor) is the same shape the native side already uses in speed_segments_for_window (crates/compositor/src/regions.rs), so this brings the TS path into agreement with Rust rather than inventing a third rule, and it honours the existing docstring's "the earliest-starting one wins" instead of quietly changing the contract.

One thing to be straight about: splitBySpeed is dormant right now. Its only consumers are StreamingVideoDecoder.decodeAll and getExportMetrics in src/lib/exporter/streamingDecoder.ts, and nothing in the repo calls either — every export goes through buildSceneDescription + exportMultiNative/exportGifNative from ExportDialog.tsx. So no user is hitting the duplicated output today. That doesn't argue against merging: it removes a divergence from the compositor and leaves the web decoder correct for whenever it's wired back up.

Also worth knowing, since it landed after your merge base: overlapping speed regions are now trivial to create. addSpeed in useTimeline.ts does no overlap check on create, and new pills are sized by zoom — roughly 3m45s per click at full zoom-out on a 30-minute recording. Two clicks reproduce your first test case exactly.

Merging.

[EtienneLescot]

Docstring and test note landed in 5a38403, CI green, threads resolved. Approving.

For the record on priority: splitBySpeed has no live caller today — every export goes through the native compositor — so nothing user-facing was broken. What this buys is that the TS helper and speed_segments_for_window now implement the same rule, which matters because the toolbar makes overlapping speed regions easy to create and the next person to wire up the web decoder should not rediscover this.