Conversation
📝 WalkthroughWalkthroughImplements a graceful disconnect grace period system allowing users to reconnect within 60 seconds while preserving session state. Adds per-user timers, reconnection detection, new session lifecycle events, and frontend message preservation. Transcription manager adds post-final silence grace period; infrastructure allocations increased. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client<br/>(ChatInterface)
participant Server as Server<br/>(MentraAI)
participant SessionMgr as SessionManager
participant API as Chat API
rect rgba(100, 150, 200, 0.5)
Note over Client,API: Graceful Disconnect Flow
Client->>Server: Disconnect / Network Loss
Server->>SessionMgr: softRemove(userId, 60000ms)
SessionMgr->>SessionMgr: Start Grace Timer
SessionMgr->>API: broadcastChatEvent('session_reconnecting')
API->>Client: SSE: session_reconnecting
Client->>Client: Disable Session, Preserve Messages
end
rect rgba(100, 200, 100, 0.5)
Note over Client,API: Reconnect Within Grace Period
Client->>Server: Reconnect
Server->>SessionMgr: cancelRemoval(userId)
SessionMgr->>SessionMgr: Clear Grace Timer
SessionMgr->>API: broadcastChatEvent('session_reconnected')
API->>Client: SSE: session_reconnected
Client->>Client: Re-enable Session, Restore Context
end
rect rgba(200, 100, 100, 0.5)
Note over SessionMgr,API: Grace Period Expires
SessionMgr->>API: broadcastChatEvent('session_ended')
SessionMgr->>SessionMgr: clearPendingEvents()
SessionMgr->>SessionMgr: Hard Remove User
API->>Client: SSE: session_ended
Client->>Client: Clear Messages, Reset State
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
issues/graceful-disconnect/issue.md (1)
11-25: Minor: Add language specifiers to fenced code blocks.Several code blocks lack language identifiers, which affects syntax highlighting and fails markdown linting. This applies to lines 11, 51, 65, 196, and 357.
🔧 Example fix for the first occurrence
-``` +```text glasses disconnect (even for 1 second) → onStop fires🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@issues/graceful-disconnect/issue.md` around lines 11 - 25, Several fenced code blocks in the markdown (e.g., the block starting with "glasses disconnect (even for 1 second)" and the other blocks at the review locations) lack language specifiers; update each fenced block by adding an appropriate language label (for example ```text, ```diff, or ```bash) to enable syntax highlighting and satisfy markdown linting, ensuring you update the blocks referenced in the review (the ones around the "glasses disconnect..." snippet and the other occurrences called out).src/server/manager/TranscriptionManager.ts (1)
125-129: Speaker diarization disabled — verify this is intentional.Speaker diarization is commented out with "disabled for testing". If this is meant to be temporary, consider adding a TODO or feature flag. If permanent, the dead code should be removed to avoid confusion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server/manager/TranscriptionManager.ts` around lines 125 - 129, The commented-out diarization check inside TranscriptionManager (the block referencing this.isListening, this.activeSpeakerId, speakerId, and text) should either be restored behind a clear feature flag or removed with a TODO; update the code so the intent is explicit: if diarization is temporary, reintroduce the guard as a conditional using a new boolean flag (e.g., speakerDiarizationEnabled or config.speakerDiarization) and log the dropped transcripts with context, otherwise delete the dead commented block and add a one-line TODO comment stating why diarization is disabled and where/when to re-enable it; ensure you touch the same symbols (this.isListening, this.activeSpeakerId) so behavior remains consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/server/manager/SessionManager.ts`:
- Line 16: The field pendingRemovals in SessionManager is typed as the undefined
Timer type; change its declaration to use the concrete browser/Node timer type
by replacing Timer with ReturnType<typeof setTimeout> (i.e., declare private
pendingRemovals: Map<string, ReturnType<typeof setTimeout>> = new Map()). Update
any related usages (clearTimeout calls) if necessary to ensure the stored values
are passed directly to clearTimeout without casting.
In `@src/server/manager/TranscriptionManager.ts`:
- Around line 61-62: The constant comments for SILENCE_TIMEOUT_MS and
FINAL_SILENCE_TIMEOUT_MS are inconsistent with their numeric values; update them
to be consistent by either setting SILENCE_TIMEOUT_MS = 1500 and
FINAL_SILENCE_TIMEOUT_MS = 5000 to match the inline comments (1.5s and 5s), or
keep the current numeric values (2000/2000) and change the inline comments to
reflect "2s" for both; adjust the definitions/comments in
TranscriptionManager.ts (symbols: SILENCE_TIMEOUT_MS, FINAL_SILENCE_TIMEOUT_MS)
accordingly and ensure any related logic/tests are still correct.
- Around line 182-186: The early return inside TranscriptionManager when
this.currentSilenceMs > this.SILENCE_TIMEOUT_MS prevents the live transcription
HUD from being refreshed; before returning, invoke the same HUD/display update
logic used later in the method (the block that updates the live transcription
HUD/display) so interim text is shown even during the post-final grace period,
but still do NOT reset the silence timer. Locate the branch checking
currentSilenceMs and move or duplicate the HUD update call (the live
transcription HUD update block) to run just before the return in that branch
while leaving timer-reset logic unchanged.
---
Nitpick comments:
In `@issues/graceful-disconnect/issue.md`:
- Around line 11-25: Several fenced code blocks in the markdown (e.g., the block
starting with "glasses disconnect (even for 1 second)" and the other blocks at
the review locations) lack language specifiers; update each fenced block by
adding an appropriate language label (for example ```text, ```diff, or ```bash)
to enable syntax highlighting and satisfy markdown linting, ensuring you update
the blocks referenced in the review (the ones around the "glasses disconnect..."
snippet and the other occurrences called out).
In `@src/server/manager/TranscriptionManager.ts`:
- Around line 125-129: The commented-out diarization check inside
TranscriptionManager (the block referencing this.isListening,
this.activeSpeakerId, speakerId, and text) should either be restored behind a
clear feature flag or removed with a TODO; update the code so the intent is
explicit: if diarization is temporary, reintroduce the guard as a conditional
using a new boolean flag (e.g., speakerDiarizationEnabled or
config.speakerDiarization) and log the dropped transcripts with context,
otherwise delete the dead commented block and add a one-line TODO comment
stating why diarization is disabled and where/when to re-enable it; ensure you
touch the same symbols (this.isListening, this.activeSpeakerId) so behavior
remains consistent.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
issues/graceful-disconnect/issue.mdporter.yamlsrc/frontend/pages/ChatInterface.tsxsrc/server/MentraAI.tssrc/server/api/chat.tssrc/server/api/debug.tssrc/server/manager/SessionManager.tssrc/server/manager/TranscriptionManager.ts
| */ | ||
| export class SessionManager { | ||
| private users: Map<string, User> = new Map(); | ||
| private pendingRemovals: Map<string, Timer> = new Map(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if Timer type is defined in the codebase or dependencies
rg -n "type Timer\b|interface Timer\b" --type tsRepository: Mentra-Community/Mentra-AI-2
Length of output: 54
🏁 Script executed:
# Check TypeScript config for lib settings that might provide Timer
cat tsconfig.json 2>/dev/null | head -20Repository: Mentra-Community/Mentra-AI-2
Length of output: 527
🏁 Script executed:
# Check the actual imports and Timer usage in SessionManager.ts
cat -n src/server/manager/SessionManager.ts | head -100Repository: Mentra-Community/Mentra-AI-2
Length of output: 4082
🏁 Script executed:
# Search for Timer usage throughout the codebase
rg -n "Timer\b" --type ts src/Repository: Mentra-Community/Mentra-AI-2
Length of output: 1706
Replace Timer type with ReturnType<typeof setTimeout> for type safety.
The type Timer on line 16 is undefined in your TypeScript configuration. While skipLibCheck may suppress the error at build time, it's a compilation error that prevents type checking. The codebase already uses ReturnType<typeof setTimeout> elsewhere (ChatInterface.tsx, HomePage.tsx) for timer storage.
Fix
- private pendingRemovals: Map<string, Timer> = new Map();
+ private pendingRemovals: Map<string, ReturnType<typeof setTimeout>> = new Map();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private pendingRemovals: Map<string, Timer> = new Map(); | |
| private pendingRemovals: Map<string, ReturnType<typeof setTimeout>> = new Map(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/manager/SessionManager.ts` at line 16, The field pendingRemovals
in SessionManager is typed as the undefined Timer type; change its declaration
to use the concrete browser/Node timer type by replacing Timer with
ReturnType<typeof setTimeout> (i.e., declare private pendingRemovals:
Map<string, ReturnType<typeof setTimeout>> = new Map()). Update any related
usages (clearTimeout calls) if necessary to ensure the stored values are passed
directly to clearTimeout without casting.
Comment on lines
+61
to
+62
| private readonly SILENCE_TIMEOUT_MS = 2000; // 1.5s silence = query complete for interim transcriptions | ||
| private readonly FINAL_SILENCE_TIMEOUT_MS = 2000; // 5s after isFinal — gives user time to add another sentence |
There was a problem hiding this comment.
Comment/code mismatch for timeout constants.
The inline comments don't match the actual values:
- Line 61: Comment says "1.5s silence" but value is
2000(2s) - Line 62: Comment says "5s after isFinal" but value is
2000(2s)
This will cause confusion for future maintainers. Either update the comments to match the values, or adjust the values to match the intended behavior.
🔧 Proposed fix
- private readonly SILENCE_TIMEOUT_MS = 2000; // 1.5s silence = query complete for interim transcriptions
- private readonly FINAL_SILENCE_TIMEOUT_MS = 2000; // 5s after isFinal — gives user time to add another sentence
+ private readonly SILENCE_TIMEOUT_MS = 2000; // 2s silence = query complete for interim transcriptions
+ private readonly FINAL_SILENCE_TIMEOUT_MS = 2000; // 2s after isFinal — gives user time to add another sentenceOr if 5s was the intended grace period:
- private readonly FINAL_SILENCE_TIMEOUT_MS = 2000; // 5s after isFinal — gives user time to add another sentence
+ private readonly FINAL_SILENCE_TIMEOUT_MS = 5000; // 5s after isFinal — gives user time to add another sentence📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private readonly SILENCE_TIMEOUT_MS = 2000; // 1.5s silence = query complete for interim transcriptions | |
| private readonly FINAL_SILENCE_TIMEOUT_MS = 2000; // 5s after isFinal — gives user time to add another sentence | |
| private readonly SILENCE_TIMEOUT_MS = 2000; // 2s silence = query complete for interim transcriptions | |
| private readonly FINAL_SILENCE_TIMEOUT_MS = 2000; // 2s after isFinal — gives user time to add another sentence |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/manager/TranscriptionManager.ts` around lines 61 - 62, The
constant comments for SILENCE_TIMEOUT_MS and FINAL_SILENCE_TIMEOUT_MS are
inconsistent with their numeric values; update them to be consistent by either
setting SILENCE_TIMEOUT_MS = 1500 and FINAL_SILENCE_TIMEOUT_MS = 5000 to match
the inline comments (1.5s and 5s), or keep the current numeric values
(2000/2000) and change the inline comments to reflect "2s" for both; adjust the
definitions/comments in TranscriptionManager.ts (symbols: SILENCE_TIMEOUT_MS,
FINAL_SILENCE_TIMEOUT_MS) accordingly and ensure any related logic/tests are
still correct.
Comment on lines
+182
to
+186
| } else if (this.currentSilenceMs > this.SILENCE_TIMEOUT_MS) { | ||
| // We're in a post-final grace period and an interim arrived — keep the longer timer | ||
| // but DON'T reset it (the user is speaking, which is good — let the existing timer run) | ||
| // console.log(`⏱️ [TIMER] Interim arrived during post-final grace period, NOT resetting (${this.currentSilenceMs}ms remaining) | text="${text.slice(0, 50)}"`); | ||
| return; |
There was a problem hiding this comment.
Early return skips HUD display update during post-final grace period.
When an interim transcription arrives during the post-final grace period (lines 182-186), the function returns early. This means the live transcription HUD display (lines 194-200) won't update, potentially leaving stale text on display glasses while the user continues speaking.
Consider whether the HUD should still update even if the timer isn't reset:
🔧 Proposed fix to update HUD before returning
} else if (this.currentSilenceMs > this.SILENCE_TIMEOUT_MS) {
// We're in a post-final grace period and an interim arrived — keep the longer timer
// but DON'T reset it (the user is speaking, which is good — let the existing timer run)
// console.log(`⏱️ [TIMER] Interim arrived during post-final grace period, NOT resetting (${this.currentSilenceMs}ms remaining) | text="${text.slice(0, 50)}"`);
+
+ // Still update the HUD display so user sees their ongoing speech
+ if (this.isListening && this.user.appSession?.capabilities?.hasDisplay) {
+ this.user.appSession.layouts.showTextWall(
+ `Listening...\n\n${this.getFullTranscript()}`,
+ { durationMs: 5000 }
+ );
+ }
return;
} else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/manager/TranscriptionManager.ts` around lines 182 - 186, The early
return inside TranscriptionManager when this.currentSilenceMs >
this.SILENCE_TIMEOUT_MS prevents the live transcription HUD from being
refreshed; before returning, invoke the same HUD/display update logic used later
in the method (the block that updates the live transcription HUD/display) so
interim text is shown even during the post-final grace period, but still do NOT
reset the silence timer. Locate the branch checking currentSilenceMs and move or
duplicate the HUD update call (the live transcription HUD update block) to run
just before the return in that branch while leaving timer-reset logic unchanged.
Member
|
good job! 😎👍 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
This PR introduces a graceful disconnect flow with a 1-minute reconnect grace period to prevent full session teardown on brief network interruptions, while also removing speaker diarization from the transcription pipeline to improve reliability and avoid dropped queries.
Problem
Previously, any short disconnect (network blip, SDK hiccup, signal loss) immediately triggered
onStop, fully destroying theUsersession and wiping conversation history, photos, location data, and all managers. This caused users to lose context and restart conversations after even a 1–2 second interruption. Additionally, unstable speaker diarization caused valid transcriptions to be dropped, further degrading reliability.Solution
Instead of immediately performing a hard cleanup, the system now performs a soft disconnect that detaches the glasses session while keeping the
Usersingleton alive for 60 seconds. If the user reconnects within this window, the existing session is seamlessly restored with all state intact. If no reconnect occurs, a delayed hard cleanup runs after the grace period expires. Speaker diarization has also been disabled so all incoming transcriptions are accepted consistently.Key Changes
softRemove()andcancelRemoval()inSessionManagersession_reconnectingandsession_reconnecteddestroyedflag was not reset on reconnectUser Experience Impact
Summary
Together, these changes significantly improve session stability, transcription reliability, and overall user experience by making the system resilient to transient disconnects while ensuring transcription continues to function correctly after reconnects.
Summary by CodeRabbit
New Features
Improvements