fix(recording): preserve mic companion audio in editor/export#280
Conversation
📝 WalkthroughWalkthroughThis change adds comprehensive test coverage for Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
electron/ipc/recording/diagnostics.test.ts (1)
70-134: Consider adding coverage for the "embedded audio, no mic companion" branch.The two tests nicely cover the happy paths, but the new branch at
diagnostics.tslines 135–137 — where embedded audio is detected and the only companion is.system.wav(no mic), causing the function to return[]— is not exercised. Given that this branch silently drops the system sidecar, a regression test would make the contract explicit and guard against future re-introductions of the original bug.🧪 Suggested additional test sketch
it("returns an empty array when embedded audio exists but no mic companion is present", async () => { const videoPath = path.join(tempRoot, "recording.mp4"); const systemPath = path.join(tempRoot, "recording.system.wav"); await Promise.all([ fs.writeFile(videoPath, "video"), fs.writeFile(systemPath, "system"), ]); execFileMock.mockImplementation((_f, _a, _o, callback: ExecFileCallback) => { const error = new Error("ffmpeg probe found embedded audio") as Error & { stderr?: string }; error.stderr = "Stream `#0`:1: Audio: aac"; callback(error, "", error.stderr); }); const { getCompanionAudioFallbackPaths } = await import("./diagnostics"); await expect(getCompanionAudioFallbackPaths(videoPath)).resolves.toEqual([]); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/ipc/recording/diagnostics.test.ts` around lines 70 - 134, Add a new test exercising the branch in getCompanionAudioFallbackPaths where embedded audio is detected but only the .system.wav companion exists: create video and system sidecar files (use tempRoot), mock execFileMock to call back with stderr "Stream `#0`:1: Audio: aac" (to simulate embedded audio), import getCompanionAudioFallbackPaths from "./diagnostics", and assert the call resolves to an empty array; name it something like "returns an empty array when embedded audio exists but no mic companion is present" so it clearly covers the diagnostics.ts embedded-audio-without-mic branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@electron/ipc/recording/diagnostics.test.ts`:
- Around line 70-134: Add a new test exercising the branch in
getCompanionAudioFallbackPaths where embedded audio is detected but only the
.system.wav companion exists: create video and system sidecar files (use
tempRoot), mock execFileMock to call back with stderr "Stream `#0`:1: Audio: aac"
(to simulate embedded audio), import getCompanionAudioFallbackPaths from
"./diagnostics", and assert the call resolves to an empty array; name it
something like "returns an empty array when embedded audio exists but no mic
companion is present" so it clearly covers the diagnostics.ts
embedded-audio-without-mic branch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 54aae8a6-88e4-468a-a634-331ab952fdec
📒 Files selected for processing (2)
electron/ipc/recording/diagnostics.test.tselectron/ipc/recording/diagnostics.ts
Pull Request Template
Description
Fix recordings where the source video already contains audio but the microphone is saved as a companion sidecar file. The editor and export pipeline were dropping the mic track in that case, so microphone audio never appeared in playback or final exports.
Motivation
Issue #250 reports that the microphone companion file exists on disk but is ignored by the editor/export pipeline. The companion-audio resolver returned no fallback sources whenever the video had any embedded audio stream, which hid separated microphone tracks from native and browser recording layouts.
Type of Change
Related Issue(s)
Screenshots / Video
N/A (backend audio pipeline fix)
Testing Guide
npx vitest --run electron/ipc/recording/diagnostics.test.ts src/lib/mediaTiming.test.tsChecklist
Summary by CodeRabbit
Tests
Bug Fixes