Skip to content

Commit e01713e

Browse files
committed
resolve conflict for bidirectional streaming restart feature
1 parent 4d32bba commit e01713e

File tree

6 files changed

+190
-13
lines changed

6 files changed

+190
-13
lines changed

src/app/components/chat-panel/chat-panel.component.scss

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,8 @@ button.audio-rec-btn,
382382
button.video-rec-btn {
383383
background-color: var(--chat-card-background-color);
384384
&.recording {
385-
background-color: var(--chat-panel-eval-fail-color);
385+
background-color: var(--chat-panel-eval-fail-color) !important;
386+
color: white !important;
386387
}
387388
}
388389

src/app/components/chat/chat.component.spec.ts

Lines changed: 84 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -806,15 +806,92 @@ describe('ChatComponent', () => {
806806

807807
describe('when bidi streaming is restarted', () => {
808808
beforeEach(() => {
809-
component.sessionHasUsedBidi.add(component.sessionId);
809+
component.startAudioRecording();
810+
component.stopAudioRecording();
810811
component.startAudioRecording();
811812
});
812-
it('should show snackbar', () => {
813-
expect(mockSnackBar.open)
814-
.toHaveBeenCalledWith(
815-
'Restarting bidirectional streaming is not currently supported. Please refresh the page or start a new session.',
816-
OK_BUTTON_TEXT,
817-
);
813+
it('should allow restart without error', () => {
814+
expect(component.isAudioRecording).toBe(true);
815+
expect(mockStreamChatService.startAudioChat).toHaveBeenCalledTimes(2);
816+
});
817+
});
818+
819+
describe('when audio recording is stopped and restarted', () => {
820+
beforeEach(() => {
821+
component.startAudioRecording();
822+
expect(component.sessionHasUsedBidi.has(component.sessionId)).toBe(true);
823+
component.stopAudioRecording();
824+
});
825+
it('should remove session from sessionHasUsedBidi set', () => {
826+
expect(component.sessionHasUsedBidi.has(component.sessionId)).toBe(false);
827+
});
828+
829+
it('should allow restarting audio recording', () => {
830+
component.startAudioRecording();
831+
expect(mockSnackBar.open).not.toHaveBeenCalled();
832+
expect(component.isAudioRecording).toBe(true);
833+
});
834+
});
835+
836+
describe('when video recording is stopped and restarted', () => {
837+
beforeEach(() => {
838+
component.startVideoRecording();
839+
expect(component.sessionHasUsedBidi.has(component.sessionId)).toBe(true);
840+
component.stopVideoRecording();
841+
});
842+
843+
it('should remove session from sessionHasUsedBidi set', () => {
844+
expect(component.sessionHasUsedBidi.has(component.sessionId)).toBe(false);
845+
});
846+
847+
it('should allow restarting video recording', () => {
848+
component.startVideoRecording();
849+
expect(mockSnackBar.open).not.toHaveBeenCalled();
850+
expect(component.isVideoRecording).toBe(true);
851+
});
852+
});
853+
854+
describe('when trying to start concurrent bidi streams', () => {
855+
it('should prevent starting audio while already recording', () => {
856+
component.startAudioRecording();
857+
expect(component.isAudioRecording).toBe(true);
858+
859+
component.startAudioRecording();
860+
861+
expect(mockSnackBar.open).toHaveBeenCalledWith(
862+
'Another streaming request is already in progress. Please stop it before starting a new one.',
863+
'OK'
864+
);
865+
expect(mockStreamChatService.startAudioChat).toHaveBeenCalledTimes(1);
866+
});
867+
868+
it('should prevent starting video while already recording', () => {
869+
component.startVideoRecording();
870+
expect(component.isVideoRecording).toBe(true);
871+
872+
component.startVideoRecording();
873+
874+
expect(mockSnackBar.open).toHaveBeenCalledWith(
875+
'Another streaming request is already in progress. Please stop it before starting a new one.',
876+
'OK'
877+
);
878+
expect(mockStreamChatService.startVideoChat).toHaveBeenCalledTimes(1);
879+
});
880+
});
881+
882+
describe('when stopping video recording without videoContainer', () => {
883+
it('should still cleanup sessionHasUsedBidi', () => {
884+
component.startVideoRecording();
885+
expect(component.sessionHasUsedBidi.has(component.sessionId)).toBe(true);
886+
887+
spyOn(component, 'chatPanel').and.returnValue({
888+
videoContainer: undefined
889+
} as any);
890+
891+
component.stopVideoRecording();
892+
893+
expect(component.sessionHasUsedBidi.has(component.sessionId)).toBe(false);
894+
expect(component.isVideoRecording).toBe(false);
818895
});
819896
});
820897
});

src/app/components/chat/chat.component.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ class CustomPaginatorIntl extends MatPaginatorIntl {
113113
}
114114

115115
const BIDI_STREAMING_RESTART_WARNING =
116-
'Restarting bidirectional streaming is not currently supported. Please refresh the page or start a new session.';
116+
'Another streaming request is already in progress. Please stop it before starting a new one.';
117117

118118
@Component({
119119
selector: 'app-chat',
@@ -213,7 +213,6 @@ export class ChatComponent implements OnInit, AfterViewInit, OnDestroy {
213213
private readonly isModelThinkingSubject = new BehaviorSubject(false);
214214
protected readonly canEditSession = signal(true);
215215

216-
// TODO: Remove this once backend supports restarting bidi streaming.
217216
sessionHasUsedBidi = new Set<string>();
218217

219218
eventData = new Map<string, any>();
@@ -1018,11 +1017,14 @@ export class ChatComponent implements OnInit, AfterViewInit, OnDestroy {
10181017
{role: 'bot', text: 'Speaking...'},
10191018
]);
10201019
this.sessionHasUsedBidi.add(this.sessionId);
1020+
this.changeDetectorRef.detectChanges();
10211021
}
10221022

10231023
stopAudioRecording() {
10241024
this.streamChatService.stopAudioChat();
10251025
this.isAudioRecording = false;
1026+
this.sessionHasUsedBidi.delete(this.sessionId);
1027+
this.changeDetectorRef.detectChanges();
10261028
}
10271029

10281030
toggleVideoRecording() {
@@ -1049,15 +1051,17 @@ export class ChatComponent implements OnInit, AfterViewInit, OnDestroy {
10491051
this.messages.update(
10501052
messages => [...messages, {role: 'user', text: 'Speaking...'}]);
10511053
this.sessionHasUsedBidi.add(this.sessionId);
1054+
this.changeDetectorRef.detectChanges();
10521055
}
10531056

10541057
stopVideoRecording() {
10551058
const videoContainer = this.chatPanel()?.videoContainer;
1056-
if (!videoContainer) {
1057-
return;
1059+
if (videoContainer) {
1060+
this.streamChatService.stopVideoChat(videoContainer);
10581061
}
1059-
this.streamChatService.stopVideoChat(videoContainer);
10601062
this.isVideoRecording = false;
1063+
this.sessionHasUsedBidi.delete(this.sessionId);
1064+
this.changeDetectorRef.detectChanges();
10611065
}
10621066

10631067
private getAsyncFunctionsFromParts(

src/app/core/services/stream-chat.service.spec.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,4 +263,64 @@ describe('StreamChatService', () => {
263263
expect(mockWebSocketService.sendMessage).toHaveBeenCalledTimes(2);
264264
}));
265265
});
266+
267+
describe('restart audio chat', () => {
268+
it('should allow restarting audio chat after stopping', async () => {
269+
mockAudioRecordingService.getCombinedAudioBuffer.and.returnValue(
270+
Uint8Array.of());
271+
272+
await service.startAudioChat({
273+
appName: 'fake-app-name',
274+
userId: 'fake-user-id',
275+
sessionId: 'fake-session-id'
276+
});
277+
expect(mockWebSocketService.connect).toHaveBeenCalledTimes(1);
278+
expect(mockAudioRecordingService.startRecording).toHaveBeenCalledTimes(1);
279+
280+
service.stopAudioChat();
281+
expect(mockAudioRecordingService.stopRecording).toHaveBeenCalledTimes(1);
282+
expect(mockWebSocketService.closeConnection).toHaveBeenCalledTimes(1);
283+
284+
await service.startAudioChat({
285+
appName: 'fake-app-name',
286+
userId: 'fake-user-id',
287+
sessionId: 'fake-session-id'
288+
});
289+
expect(mockWebSocketService.connect).toHaveBeenCalledTimes(2);
290+
expect(mockAudioRecordingService.startRecording).toHaveBeenCalledTimes(2);
291+
});
292+
});
293+
294+
describe('restart video chat', () => {
295+
it('should allow restarting video chat after stopping', async () => {
296+
mockAudioRecordingService.getCombinedAudioBuffer.and.returnValue(
297+
Uint8Array.of());
298+
mockVideoService.getCapturedFrame.and.resolveTo(Uint8Array.of());
299+
300+
await service.startVideoChat({
301+
appName: 'fake-app-name',
302+
userId: 'fake-user-id',
303+
sessionId: 'fake-session-id',
304+
videoContainer
305+
});
306+
expect(mockWebSocketService.connect).toHaveBeenCalledTimes(1);
307+
expect(mockAudioRecordingService.startRecording).toHaveBeenCalledTimes(1);
308+
expect(mockVideoService.startRecording).toHaveBeenCalledTimes(1);
309+
310+
service.stopVideoChat(videoContainer);
311+
expect(mockAudioRecordingService.stopRecording).toHaveBeenCalledTimes(1);
312+
expect(mockVideoService.stopRecording).toHaveBeenCalledTimes(1);
313+
expect(mockWebSocketService.closeConnection).toHaveBeenCalledTimes(1);
314+
315+
await service.startVideoChat({
316+
appName: 'fake-app-name',
317+
userId: 'fake-user-id',
318+
sessionId: 'fake-session-id',
319+
videoContainer
320+
});
321+
expect(mockWebSocketService.connect).toHaveBeenCalledTimes(2);
322+
expect(mockAudioRecordingService.startRecording).toHaveBeenCalledTimes(2);
323+
expect(mockVideoService.startRecording).toHaveBeenCalledTimes(2);
324+
});
325+
});
266326
});

src/app/core/services/websocket.service.spec.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,4 +55,33 @@ describe('WebSocketService', () => {
5555
expect(service.urlSafeBase64ToBase64('abcd')).toEqual('abcd');
5656
});
5757
});
58+
59+
describe('connection restart', () => {
60+
it('should reset audio buffer when reconnecting', () => {
61+
service.connect('ws://test1');
62+
63+
(service as any).audioBuffer = [new Uint8Array([1, 2, 3])];
64+
65+
service.connect('ws://test2');
66+
expect((service as any).audioBuffer).toEqual([]);
67+
});
68+
69+
it('should close previous connection when reconnecting', () => {
70+
service.connect('ws://test1');
71+
const firstSocket = (service as any).socket$;
72+
spyOn(firstSocket, 'complete');
73+
74+
service.connect('ws://test2');
75+
expect(firstSocket.complete).toHaveBeenCalled();
76+
});
77+
78+
it('should clear audio interval when closing connection', () => {
79+
service.connect('ws://test');
80+
const intervalId = (service as any).audioIntervalId;
81+
expect(intervalId).not.toBeNull();
82+
83+
service.closeConnection();
84+
expect((service as any).audioIntervalId).toBeNull();
85+
});
86+
});
5887
});

src/app/core/services/websocket.service.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,12 @@ export class WebSocketService implements WebSocketServiceInterface {
3838
private closeReasonSubject = new Subject<string>();
3939

4040
connect(serverUrl: string) {
41+
// Clean up previous connection if exists
42+
this.closeConnection();
43+
44+
// Reset audio buffer for new connection
45+
this.audioBuffer = [];
46+
4147
this.socket$ = new WebSocketSubject({
4248
url: serverUrl,
4349
serializer: (msg) => JSON.stringify(msg),

0 commit comments

Comments
 (0)