Conversation
WalkthroughIntroduces a video-based fallback mechanism for device wake-lock persistence during recording. When native wake lock fails on iOS low-battery scenarios, a silent 1-pixel video plays instead. Enhances wake lock hook to expose lifecycle states and improves obtain/release handling with proper cleanup. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
echo/frontend/src/components/participant/ParticipantConversationAudio.tsx(3 hunks)echo/frontend/src/hooks/useVideoWakeLockFallback.ts(1 hunks)echo/frontend/src/hooks/useWakeLock.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
echo/frontend/**/src/**/*.{css,tsx,ts}
📄 CodeRabbit inference engine (echo/frontend/AGENTS.md)
Use CSS variables
var(--app-background)andvar(--app-text)instead of hardcoded colors like#F6F4F1or#2D2D2Cto ensure theme changes propagate
Files:
echo/frontend/src/hooks/useVideoWakeLockFallback.tsecho/frontend/src/components/participant/ParticipantConversationAudio.tsxecho/frontend/src/hooks/useWakeLock.ts
echo/frontend/**/src/{routes,components}/**/*.tsx
📄 CodeRabbit inference engine (echo/frontend/AGENTS.md)
echo/frontend/**/src/{routes,components}/**/*.tsx: Compose Mantine primitives (Stack,Group,ActionIcon, etc.) while layering Tailwind utility classes viaclassName, alongside toast feedback via@/components/common/Toaster
For Tailwind classes that need dynamic values, replace with inlinestyleprops using CSS variables instead of hardcoded hex values
Files:
echo/frontend/src/components/participant/ParticipantConversationAudio.tsx
echo/frontend/**/src/{theme.tsx,components,routes}/**/*.tsx
📄 CodeRabbit inference engine (echo/frontend/AGENTS.md)
When adding new Mantine components, reference the global Mantine theme in
src/theme.tsxwith customprimarycolor palette and component defaults, preferring CSS variables in styles over hardcoded hex values
Files:
echo/frontend/src/components/participant/ParticipantConversationAudio.tsx
echo/frontend/**/src/{components,routes}/**/*.tsx
📄 CodeRabbit inference engine (echo/frontend/AGENTS.md)
UI mutations should surface inline feedback: pair toasts with contextual Mantine
Alertcomponents inside modals/forms for errors or warnings
Files:
echo/frontend/src/components/participant/ParticipantConversationAudio.tsx
🧠 Learnings (3)
📓 Common learnings
Learnt from: ussaama
Repo: Dembrane/echo PR: 205
File: echo/frontend/src/lib/query.ts:1444-1506
Timestamp: 2025-07-10T12:48:20.683Z
Learning: ussaama prefers string concatenation over template literals for simple cases where readability is clearer, even when linting tools suggest template literals. Human readability takes precedence over strict linting rules in straightforward concatenation scenarios.
📚 Learning: 2025-08-06T13:38:30.769Z
Learnt from: ussaama
Repo: Dembrane/echo PR: 256
File: echo/frontend/src/components/participant/MicrophoneTest.tsx:54-54
Timestamp: 2025-08-06T13:38:30.769Z
Learning: In echo/frontend/src/components/participant/MicrophoneTest.tsx, the useDisclosure hook is used where only the `close` function is needed locally (called in handleConfirmMicChange and handleCancelMicChange), while `opened` and `open` are unused because the modal state is managed by a parent component. The `showSecondModal` state variable is used separately to control content switching within the modal between the main UI and confirmation step.
Applied to files:
echo/frontend/src/hooks/useVideoWakeLockFallback.tsecho/frontend/src/components/participant/ParticipantConversationAudio.tsx
📚 Learning: 2025-12-05T00:33:33.409Z
Learnt from: CR
Repo: Dembrane/echo PR: 0
File: echo/frontend/AGENTS.md:0-0
Timestamp: 2025-12-05T00:33:33.409Z
Learning: Applies to echo/frontend/**/src/routes/auth/**/*.tsx : Auth hero uses `/public/video/auth-hero.mp4` with `/public/video/auth-hero-poster.jpg` as poster; keep the bright blur overlay consistent when iterating on onboarding screens
Applied to files:
echo/frontend/src/hooks/useVideoWakeLockFallback.ts
🧬 Code graph analysis (1)
echo/frontend/src/components/participant/ParticipantConversationAudio.tsx (2)
echo/frontend/src/hooks/useWakeLock.ts (1)
useWakeLock(3-91)echo/frontend/src/hooks/useVideoWakeLockFallback.ts (1)
useVideoWakeLockFallback(6-126)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-check-server
🔇 Additional comments (7)
echo/frontend/src/components/participant/ParticipantConversationAudio.tsx (1)
28-29: LGTM - clean integration of the video wake lock fallback.The hook wiring is correct. Passing
wakeLock.isActiveto determine when to engage the fallback is the right call. The side-effect-only usage pattern (not destructuring the return) is totally valid here since you just need the hook to manage the DOM video element lifecycle.Ship it. 🚀
Also applies to: 104-104, 118-122
echo/frontend/src/hooks/useWakeLock.ts (3)
9-22: Solid cleanup pattern - listener removal before release.The
releaseHandlerRefpattern here is 10x. Prevents the classic "stale listener on a released sentinel" footgun. One minor note:wakeLock.current.release()on line 18 returns a Promise that's not awaited. In this sync cleanup context that's acceptable, but if you ever need guaranteed ordering, you'd want to make this async.
24-61: LGTM - proper wakelock lifecycle management.Releasing the old wakelock before requesting a new one (lines 29-40) is the correct approach. Avoids resource leaks and race conditions with multiple concurrent sentinels. The try-catch with
setIsActive(false)on failure is defensive and correct.
63-82: Empty dep array is intentional but verifyobtainWakeLockOnMountbehavior.The lint suppression is fine for "run once on mount" semantics. Just be aware: if a consumer ever passes a dynamic
obtainWakeLockOnMountthat changes after mount, it won't re-evaluate. Given current usage inParticipantConversationAudio.tsxwhere it's hardcoded totrue, this is totally fine.echo/frontend/src/hooks/useVideoWakeLockFallback.ts (3)
3-4: Based inline video data - zero network round trips.Embedding the minimal H.264 video as base64 is the big brain move here. No CDN dependency, no network latency, instant playback. This will work even in offline scenarios which is clutch for recording use cases.
122-125:videoElementreturn value won't update after creation.Refs don't trigger re-renders, so
videoRef.currentwill benullon initial render and consumers won't see the updated value unless they re-render for unrelated reasons. If you need reactive access to the element, you'd need to track it with state instead.That said, looking at the consumer in
ParticipantConversationAudio.tsx, the return values aren't being used - the hook is purely for side effects. So this is fine as-is, but be aware if future consumers rely onvideoElementbeing accurate.
90-99: 5s interval for iOS pause detection is reasonable.iOS can be aggressive about pausing background media. The periodic check + restart pattern is the right workaround. If you ever see issues in the wild, could consider reducing to 2-3s, but 5s should be fine for most cases.
| if (!shouldActivateFallback) { | ||
| // Clean up video element if it exists | ||
| if (videoRef.current) { | ||
| videoRef.current.pause(); | ||
| videoRef.current.src = ""; | ||
| videoRef.current.remove(); | ||
| videoRef.current = null; | ||
| console.log( | ||
| "[VideoWakeLockFallback] Cleaned up video - wakelock is working", | ||
| ); | ||
| } | ||
| if (checkIntervalRef.current) { | ||
| clearInterval(checkIntervalRef.current); | ||
| checkIntervalRef.current = null; | ||
| } | ||
| return; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting cleanup logic to avoid duplication.
The cleanup code at lines 22-34 and lines 109-118 is nearly identical. Not a blocker, but could DRY this up with a helper:
+const cleanupVideo = (
+ videoRef: React.MutableRefObject<HTMLVideoElement | null>,
+ checkIntervalRef: React.MutableRefObject<NodeJS.Timeout | null>,
+) => {
+ if (videoRef.current) {
+ videoRef.current.pause();
+ videoRef.current.src = "";
+ videoRef.current.remove();
+ videoRef.current = null;
+ }
+ if (checkIntervalRef.current) {
+ clearInterval(checkIntervalRef.current);
+ checkIntervalRef.current = null;
+ }
+};Then call cleanupVideo(videoRef, checkIntervalRef) in both places. Optional refactor.
Also applies to: 107-119
🤖 Prompt for AI Agents
In echo/frontend/src/hooks/useVideoWakeLockFallback.ts around lines 20-35 and
107-119, the video cleanup logic is duplicated; extract a small helper function
(e.g., cleanupVideo) that accepts the videoRef and checkIntervalRef, performs
null checks, pauses the video, clears src, removes the element, nulls the
videoRef, clears and nulls the interval ref, and emits the same console log;
replace both duplicated blocks with a single call to cleanupVideo(videoRef,
checkIntervalRef) to keep behavior identical and preserve types and null-safety.
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.