fix(timeline): make virtual lookup order-independent

[arhxam]

Summary

• derive virtual duration from the greatest clip end instead of the final array element
• resolve virtual positions against timeline order while preserving the caller's original clip index
• select the chronologically earliest next kept segment even when playback segments arrive unordered
• cover reversed and unordered clip collections with regression tests

Related issue

No linked issue; found while auditing the virtual-preview ordering contract.

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; this is timeline math with no visual changes.

Testing

• npm exec -- vitest run src/lib/ai-edition/timeline/virtual-preview.test.ts
• npm exec -- biome check src/lib/ai-edition/timeline/virtual-preview.ts src/lib/ai-edition/timeline/virtual-preview.test.ts
• npm run build-vite
• npm run wb:typecheck

Summary by CodeRabbit

• Bug Fixes

  • Improved virtual timeline calculations when clips are provided in an unexpected order.
  • Corrected duration reporting to account for the furthest timeline endpoint.
  • Improved clip and segment selection for more consistent preview positioning and playback behavior.

• Tests

  • Added coverage for reversed and unordered clip sequences.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 39 minutes

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: 46d86dac-7184-4576-a895-56fda061d5c6

📥 Commits

Reviewing files that changed from the base of the PR and between 3543390 and 947dab6.

📒 Files selected for processing (2)

• src/lib/ai-edition/timeline/virtual-preview.test.ts
• src/lib/ai-edition/timeline/virtual-preview.ts

📝 Walkthrough

Walkthrough

Virtual preview timeline calculations now remain correct when clips are provided in a different order. Tests cover duration, position lookup, and next-segment selection for unordered clips.

Changes

Virtual preview ordering

Layer / File(s)	Summary
Order-independent duration and position lookup src/lib/ai-edition/timeline/virtual-preview.ts, src/lib/ai-edition/timeline/virtual-preview.test.ts	totalVirtualDuration uses the greatest timeline end. locateVirtualPosition sorts clips before resolving positions and fallback values. Tests cover reversed clips.
Earliest next segment selection src/lib/ai-edition/timeline/virtual-preview.ts, src/lib/ai-edition/timeline/virtual-preview.test.ts	findNextKeptSegment selects the earliest qualifying raw-timeline segment before applying the active-clip fallback. Tests cover unordered playback clips.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• getopenscreen/openscreen#221: Both changes update virtual-preview.ts and address clip-order-independent segment resolution.

Suggested reviewers: etiennelescot

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: making virtual timeline lookups independent of clip order.
Description check	✅ Passed	The description includes all required sections and clearly documents the fix, scope, testing, and release impact.
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

---

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.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/ai-edition/timeline/virtual-preview.ts`:
- Around line 113-116: The sort in findNextKeptSegment() needs a deterministic
secondary ordering when candidates share the same rawStart. Update the .sort()
comparator to apply the playback contract’s tie-breaker after comparing
rawStart, and add a regression test using tied candidates with permuted
playbackClips order to verify the selected segment remains consistent.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2b8edd8e-e647-4e3e-8029-b16efecc4601

📥 Commits

Reviewing files that changed from the base of the PR and between 545043d and 3543390.

📒 Files selected for processing (2)

• src/lib/ai-edition/timeline/virtual-preview.test.ts
• src/lib/ai-edition/timeline/virtual-preview.ts

[EtienneLescot]

Closing this one, and I want to give you the actual reason rather than leaving it to rot.

The premise does not hold. resequenceClips (src/lib/ai-edition/document/timeline.ts) re-lays clips back-to-back from t=0 in array order, and every mutator funnels through it — so array index order is ascending timelineStartSec, always. playbackClips, the only input to findNextKeptSegment, comes from resolvePlaybackSegments, which sorts explicitly before rebuilding with a monotonic cursor. I went looking for a path that delivers unordered clips and there is not one.

So the sorting is defending against a state the document model cannot produce — and it is not free: locateVirtualPosition is called every frame from WebcamOverlay's rAF loop, and findNextKeptSegment every frame from VirtualPreview's. This turns an allocation-free findIndex into a map + sort at 60fps, twice.

What made me close it rather than ask for changes is that the one thing the PR does change behaviourally is not the thing in the title. findNextKeptSegment scans segments in ruler order and takes the first match on either the raw-ruler rule or the source-clock fallback; this runs rule (a) globally first, then rule (b). That is a precedence inversion, it is not mentioned in the description, no test covers it, and the 17-line comment above the function still describes the old interleaving. That comment is the only surviving record of the playback-loop bug it was written to prevent — playing into a trim answered clip_1, so playback jumped to the top of the timeline and looped there.

If you think the precedence should change, that is a real conversation and worth its own PR: state the new rule, update the comment that guards it, and pin it with a test that fails on the old order. As it stands the change is invisible.

For context on where the rest landed: #236, #237, #238, #239, #242 and #235 are all merged. #242 in particular found a data-integrity bug I had not seen — deleting the last clip left orphaned modifiers that got resurrected onto new footage on the next open. That one was worth the whole batch.