Skip to content

New recording pulse#1519

Merged
duckduckhero merged 3 commits intomainfrom
new-recording-pulse
Oct 1, 2025
Merged

New recording pulse#1519
duckduckhero merged 3 commits intomainfrom
new-recording-pulse

Conversation

@duckduckhero
Copy link
Contributor

DON'T MERGE THIS YET

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 1, 2025

📝 Walkthrough

Walkthrough

Updates the Transcript Viewer’s activity logic to be session-specific, determining inactivity per session using ongoingSessionId and ongoingStatus. Adjusts the Tab Header’s recording indicator visuals by replacing a single pulsing dot with a composed animated indicator. No data model changes; rendering and conditional UI logic are updated.

Changes

Cohort / File(s) Summary
Transcript per-session activity logic
apps/desktop/src/components/editor-area/transcript-viewer.tsx
Replaces global “inactive” check with per-session computation using ongoingSessionId and ongoingStatus ("running_active"); inactive is now derived per session and drives conditional rendering (e.g., speaker selector).
Recording indicator visual update
apps/desktop/src/components/editor-area/note-header/tab-header.tsx
Replaces single pulsing dot with a container showing a semi-transparent red circle and inner pulsing circle when isCurrentlyRecording is true; rendering-only change.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant TranscriptViewer
  participant SessionState as OngoingSession State
  participant UI as SpeakerSelector UI

  User->>TranscriptViewer: Open Transcript (sessionId)
  TranscriptViewer->>SessionState: Get ongoingSessionId, ongoingStatus
  Note over TranscriptViewer: Compute isThisSessionActive = (sessionId == ongoingSessionId) && (ongoingStatus == "running_active")
  alt Session inactive (per-session)
    TranscriptViewer->>UI: Render speaker selector (inactive = true)
  else Session active
    TranscriptViewer-->>User: Skip inactive UI (early return path)
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The description “DON’T MERGE THIS YET” does not describe any part of the changeset and is unrelated to the actual code changes. Please replace the placeholder note with a concise summary of the changes made, such as the new animated recording indicator and session-specific inactive logic.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title “New recording pulse” succinctly captures the primary change of replacing the recording indicator with a pulsing visual and directly relates to the main visual update described in the changeset.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch new-recording-pulse

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
apps/desktop/src/components/editor-area/transcript-viewer.tsx (1)

185-189: Add explicit guard for valid session IDs in the comparison.

The comparison ongoingSessionId === sessionId on line 188 could incorrectly evaluate to true when both values are undefined (e.g., no ongoing session and invalid route). While line 216 guards against !sessionId, making the logic more explicit would prevent potential edge-case bugs if the guard is moved or removed.

Consider this more defensive approach:

-  // Check if THIS SPECIFIC session is inactive, not global status
-  const ongoingSessionId = useOngoingSession(s => s.sessionId);
-  const ongoingStatus = useOngoingSession(s => s.status);
-  const isThisSessionActive = ongoingStatus === "running_active" && ongoingSessionId === sessionId;
-  const inactive = !isThisSessionActive;
+  // Check if THIS SPECIFIC session is inactive, not global status
+  const ongoingSessionId = useOngoingSession(s => s.sessionId);
+  const ongoingStatus = useOngoingSession(s => s.status);
+  const isThisSessionActive = 
+    sessionId != null && 
+    ongoingSessionId === sessionId && 
+    ongoingStatus === "running_active";
+  const inactive = !isThisSessionActive;
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 54c3f84 and 6f5a16f.

📒 Files selected for processing (2)
  • apps/desktop/src/components/editor-area/note-header/tab-header.tsx (1 hunks)
  • apps/desktop/src/components/editor-area/transcript-viewer.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}

⚙️ CodeRabbit configuration file

**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".

Files:

  • apps/desktop/src/components/editor-area/note-header/tab-header.tsx
  • apps/desktop/src/components/editor-area/transcript-viewer.tsx
🧬 Code graph analysis (1)
apps/desktop/src/components/editor-area/transcript-viewer.tsx (1)
packages/utils/src/contexts/ongoing-session.tsx (1)
  • useOngoingSession (32-46)
⏰ 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). (2)
  • GitHub Check: ci (windows, windows-latest)
  • GitHub Check: ci (macos, macos-14)
🔇 Additional comments (1)
apps/desktop/src/components/editor-area/note-header/tab-header.tsx (1)

129-134: LGTM! Clean visual upgrade for the recording indicator.

The new composed indicator with a semi-transparent outer circle and pulsing inner circle provides better visual feedback than the previous single pulsing dot. Both circles correctly use absolute inset-0 to fill the parent container, creating the intended layered effect.

@duckduckhero duckduckhero merged commit 70d037d into main Oct 1, 2025
9 checks passed
@ComputelessComputer ComputelessComputer deleted the new-recording-pulse branch December 14, 2025 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant