Skip to content

fix(editor): place added audio on the first free track#324

Open
meiiie wants to merge 2 commits intowebadderallorg:mainfrom
meiiie:fix/editor-audio-track-placement
Open

fix(editor): place added audio on the first free track#324
meiiie wants to merge 2 commits intowebadderallorg:mainfrom
meiiie:fix/editor-audio-track-placement

Conversation

@meiiie
Copy link
Copy Markdown
Collaborator

@meiiie meiiie commented Apr 23, 2026

Description

Fix the editor add-audio flow so a new audio region is placed on the first non-overlapping audio track instead of being rejected immediately when the playhead is already inside another audio region.

Motivation

Issue #322 comes from the editor already supporting audio track rows internally, but the current Add Audio flow still blocks insertion as soon as the current timestamp overlaps any existing audio region. That makes it impossible to add a second overlapping audio source on a separate track.

Type of Change

  • Bug Fix
  • New Feature
  • Refactor / Code Cleanup
  • Documentation Update
  • Other (please specify)

Related Issue(s)

Changes Made

  • add a small pure helper to find the first audio track that is free at the current playhead position
  • keep using the existing track 0 behavior when that row is already free
  • move overlapping inserts onto the next available audio track instead of showing an immediate placement error
  • keep the existing duration clamping behavior per chosen track
  • add focused unit coverage for the new placement logic

Testing Guide

  1. Open the editor with an existing audio region already covering the current playhead time.
  2. Click Add Audio and choose another audio file.
  3. Verify the new region is added on a separate audio track instead of being blocked.
  4. Verify non-overlapping inserts on track 0 still behave the same as before.

Checklist

  • I have performed a self-review of my code.
  • I have tested the changes locally.
  • I have added focused regression coverage for the new behavior.
  • I have linked screenshots or videos where helpful.

Local checks run:

  • vitest run src/components/video-editor/timeline/audioTrackPlacement.test.ts src/components/video-editor/types.test.ts
  • tsc --noEmit
  • biome check src/components/video-editor/timeline/audioTrackPlacement.ts src/components/video-editor/timeline/audioTrackPlacement.test.ts src/components/video-editor/timeline/TimelineEditor.tsx

Runtime A/B smoke on Windows (same local project state in both runs):

  • baseline origin/main: one existing audio region at 0ms on track 0, then Add Audio at 0ms leaves the editor with only that original region
  • patched branch: the same Add Audio action adds a second region at 0ms on trackIndex: 1
  • patched UI result: existing-track stays on the first audio row and added-track appears on the next row

Summary by CodeRabbit

  • Refactor

    • Improved audio placement logic in the timeline editor to better determine track assignment and available space for audio segments.
  • Tests

    • Added comprehensive test suite to validate audio track placement behavior and ensure correct track assignment based on existing clips.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 23, 2026

📝 Walkthrough

Walkthrough

The changes extract audio region placement logic from TimelineEditor.tsx into a new centralized utility function findAudioTrackPlacement that determines suitable track assignment and available duration for incoming audio segments. A comprehensive test suite validates the function's track selection, overlap detection, and gap calculation behavior.

Changes

Cohort / File(s) Summary
Audio Track Placement Utility
src/components/video-editor/timeline/audioTrackPlacement.ts
New utility module introducing AudioTrackPlacement interface and findAudioTrackPlacement function that computes track assignment and available duration by iterating through tracks, checking for overlaps, and computing gaps to the next region or timeline end.
TimelineEditor Integration
src/components/video-editor/timeline/TimelineEditor.tsx
Refactored handleAddAudio to delegate track placement logic to findAudioTrackPlacement, replacing manual track sorting, gap calculation, and overlap checking with calls to the new utility and passing resolved placement.trackIndex to onAudioAdded.
Track Placement Tests
src/components/video-editor/timeline/audioTrackPlacement.test.ts
New test suite validating findAudioTrackPlacement behavior across scenarios: first available track selection, overlap detection triggering track advancement, new track creation, and null returns for invalid inputs or unavailable timeline space.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Suggested labels

Checked

Poem

🐰 Hoppy tracks now find their place,
With gap-detection at a pace,
No overlap shall block the way,
Audio flows another day!
Tests confirm each bound and bound,
Placement logic safe and sound. 🎵

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. 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 summarizes the main change: placing added audio on the first free track instead of blocking it when overlapping.
Description check ✅ Passed The description fully addresses all required template sections with detailed context, motivation, testing guide, and comprehensive checklist completion.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/components/video-editor/timeline/TimelineEditor.tsx (1)

1275-1276: ⚠️ Potential issue | 🟠 Major

Make audio overlap validation track-aware.

The add flow can now create valid overlapping audio on different tracks, but this still checks against every audio region. Dragging/resizing an audio item can be blocked by a region on another audio track.

🐛 Proposed fix
 				if (isAudioItem) {
-					return checkOverlap(audioRegions);
+					const currentAudio = audioRegions.find((region) => region.id === excludeId);
+					const trackIndex = currentAudio?.trackIndex ?? 0;
+					return checkOverlap(
+						audioRegions.filter((region) => (region.trackIndex ?? 0) === trackIndex),
+					);
 				}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/video-editor/timeline/TimelineEditor.tsx` around lines 1275 -
1276, The current audio overlap check in TimelineEditor.tsx returns
checkOverlap(audioRegions) for isAudioItem, which compares against all audio
regions across tracks; update the logic so audio overlap validation is
track-aware by filtering audioRegions to only those with the same track
identifier as the item being added/dragged/resized (e.g., region.trackId or
region.track) and then call checkOverlap(filteredAudioRegions). Alternatively,
extend checkOverlap to accept a trackId parameter and perform the same-track
filter internally; apply this change for add/drag/resize flows (where
isAudioItem is used) to avoid blocking operations due to regions on other audio
tracks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/components/video-editor/timeline/TimelineEditor.tsx`:
- Around line 1275-1276: The current audio overlap check in TimelineEditor.tsx
returns checkOverlap(audioRegions) for isAudioItem, which compares against all
audio regions across tracks; update the logic so audio overlap validation is
track-aware by filtering audioRegions to only those with the same track
identifier as the item being added/dragged/resized (e.g., region.trackId or
region.track) and then call checkOverlap(filteredAudioRegions). Alternatively,
extend checkOverlap to accept a trackId parameter and perform the same-track
filter internally; apply this change for add/drag/resize flows (where
isAudioItem is used) to avoid blocking operations due to regions on other audio
tracks.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f1d05c2e-b75a-409c-b848-92334bd9c4f1

📥 Commits

Reviewing files that changed from the base of the PR and between e70eabc and 818a723.

📒 Files selected for processing (3)
  • src/components/video-editor/timeline/TimelineEditor.tsx
  • src/components/video-editor/timeline/audioTrackPlacement.test.ts
  • src/components/video-editor/timeline/audioTrackPlacement.ts

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant