Test/add realtime timeout

[iceljc]

PR Type

Enhancement, Tests

---

Description

• Add session reconnect callback to real-time completion interface

  • Update IRealTimeCompletion.Connect to accept onSessionReconnect
  • Propagate callback through GoogleAI and OpenAI providers

• Implement model response timeout handling in OpenAI real-time provider

  • Add ModelResponseTimeout and ModelResponseTimeoutEndEvent to settings
  • Trigger model inference if timeout reached
  • Track user message start time for timeout logic

• Refactor and internalize OpenAI real-time session classes

  • Rename and restrict visibility of session-related classes

• Update tests to support new Connect signature

---

Changes walkthrough 📝

	Relevant files
Enhancement	10 files IRealTimeCompletion.cs Add session reconnect callback to interface                            +5/-2      RealtimeModelSettings.cs Add timeout and end event to model settings                            +2/-1      RealtimeHub.cs Pass session reconnect callback to completer                          +6/-1      RealTimeCompletionProvider.cs Support session reconnect callback in Google provider        +33/-30  ChatSessionUpdate.cs Rename and internalize session update class                            +2/-2      RealTimeCompletionProvider.cs Add timeout logic and propagate reconnect callback              +59/-32  AiWebsocketPipelineResponse.cs Internalize websocket pipeline response class                        +1/-1      AsyncWebsocketDataCollectionResult.cs Internalize async websocket data collection result              +1/-1      AsyncWebsocketDataResultEnumerator.cs Internalize async websocket data result enumerator              +1/-1      RealtimeChatSession.cs Internalize and update session class for new update type  +4/-4
Formatting	1 files ConversationItemCreated.cs Minor formatting update to conversation item                          +1/-0
Tests	1 files GoogleRealTimeTests.cs Update test to use new Connect signature                                  +2/-1

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[GGHansome]

Auto Review Result:

代码审查总结

变更概述:
The code changes involve enhancements to the real-time communication capabilities of various classes implementing the IRealTimeCompletion interface. A new session reconnect feature was added, model response timeout handling was improved, and internal visibility changes were made to optimize encapsulation of certain classes.

发现的问题

问题1: [Code Structure and Consistency]

• 描述: The parameter _conn was renamed to _conn for consistency, but not all usages have been updated. This can cause confusion and potential runtime errors.
• 建议: Ensure all instances of the connection parameter are consistently named _conn.
• 示例:

  // Modification
  private RealtimeHubConnection _conn; // Renamed from "conn"
  // Usage
  await _completer.Connect(_conn, ...); // Ensure all references are updated
  

问题2: [Task Management]

• 描述: The async keyword is frequently used without any await expressions in the Connect methods when using lambda expressions for the callbacks, which leads to misleading async usage.
• 建议: Remove unnecessary async keyword where there is no await, or refactor the code to make proper await calls.
• 示例:

  // Before
  onModelReady: async () => {} // Async without await
  // After
  onModelReady: () => {}

问题3: [Nullability and Default Values]

• 描述: The optional properties like ModelResponseTimeout and ModelResponseTimeoutEndEvent are being changed from non-nullable to nullable, which could cause issues if not properly initialized or handled.
• 建议: Ensure that these properties have default assignments or are properly checked for nullability before use.
• 示例:

  settings.ModelResponseTimeout ??= defaultTimeoutValue;

总体评价

The code changes provide valuable enhancements for session handling and improve the flexibility of real-time models by adding reconnect features and timeout control. However, care must be taken to ensure consistency in naming and proper use of asynchronous constructs to align with best practices. Improvements in nullability handling add robustness to the code.

[qodo-code-review]

Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here.

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Timeout Logic The model response timeout implementation resets startTime to null after triggering model inference, but doesn't check if the timeout event has already been processed, which could lead to multiple timeout triggers if responses continue to arrive. if (_settings?.ModelResponseTimeout > 0 && !string.IsNullOrWhiteSpace(_settings?.ModelResponseTimeoutEndEvent) && startTime.HasValue && (DateTime.UtcNow - startTime.Value).TotalSeconds >= _settings.ModelResponseTimeout && response.Type != _settings.ModelResponseTimeoutEndEvent) { startTime = null; await TriggerModelInference("Responsd to user immediately"); continue; } Error Handling The string interpolation for logging the error message uses ToString() on an object that might be null, which could cause a NullReferenceException if StatusDetails is null. _logger.LogError($"{data.StatusDetails.ToString()}"); return [];

[qodo-code-review]

Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here.

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Avoid null reference on session dispose Check if _session is not null before calling Dispose() to prevent a possible NullReferenceException. src/Plugins/BotSharp.Plugin.OpenAI/Providers/Realtime/RealTimeCompletionProvider.cs [246] -_session.Dispose(); +_session?.Dispose(); Apply this suggestion Suggestion importance[1-10]: 6 __ Why: Adding a null-conditional operator before calling Dispose() is a good defensive practice to prevent a possible NullReferenceException if _session is unexpectedly null. This is a minor but useful improvement for code robustness.	Low
	Prevent null reference in timeout logic Move the startTime.HasValue check before calculating the time difference to avoid a possible exception if startTime is null. src/Plugins/BotSharp.Plugin.OpenAI/Providers/Realtime/RealTimeCompletionProvider.cs [154-163] if (_settings?.ModelResponseTimeout > 0 && !string.IsNullOrWhiteSpace(_settings?.ModelResponseTimeoutEndEvent) && startTime.HasValue - && (DateTime.UtcNow - startTime.Value).TotalSeconds >= _settings.ModelResponseTimeout + && (startTime.HasValue && (DateTime.UtcNow - startTime.Value).TotalSeconds >= _settings.ModelResponseTimeout) && response.Type != _settings.ModelResponseTimeoutEndEvent) { startTime = null; await TriggerModelInference("Responsd to user immediately"); continue; } Apply this suggestion Suggestion importance[1-10]: 3 __ Why: The suggestion adds a redundant check for startTime.HasValue, which is already present in the condition. While it does not harm correctness, it does not provide any additional safety or improvement, as the original code already prevents a null reference. The impact is minimal.	Low
Update

[GGHansome]

Auto Review Result:

Code Review Summary

Change Summary: The code changes primarily focus on improving clarity and maintainability. These changes include breaking long method signatures into multiple lines, converting some public classes to internal, and renaming variables for better consistency. The aim is to enhance the code’s readability and conformity to conventions, particularly around asynchronous operations handling and class accessibility.

Identified Issues

Issue 1: Naming Inconsistency

• Description: There are changes made from "SessionConversationUpdate" to "ChatSessionUpdate" and other related names without a clear note on why this was changed. This may cause confusion without proper documentation.
• Suggestion: Ensure that such changes are documented or commented in the code to maintain clarity on why these specific changes were made.
• Example:

  // Before
  public class SessionConversationUpdate
  // After
  internal class ChatSessionUpdate

Issue 2: Non-Intuitive Use of Optional Parameters

• Description: Optional parameters or nullable types (e.g., int? ModelResponseTimeout) are introduced but may lead to unexpected null reference errors if not properly checked before use.
• Suggestion: Ensure proper null checks or provide default values where these optional parameters are used to prevent runtime exceptions.
• Example:

  // Possible null check
  if (_settings?.ModelResponseTimeout != null) 
  {
      // Use the setting
  }

Overall Evaluation

The code modifications improve readability and maintainability by reorganizing method parameters, redefining class access levels, and renaming classes and variables. However, these might lead to potential complications if not carefully documented, especially for team members unfamiliar with these changes. Proper documentation, especially for renaming changes and nullable types, is essential to avoid misunderstandings and maintain consistent code quality.