Update IRealtimeHook

[hchen2020]

PR Type

Enhancement

---

Description

• Enhanced IRealtimeHook interface with default implementations

• Modified ShouldReconnect method to accept message parameter

• Improved reconnection logic in RealtimeHub service

• Moved reconnection check to function call context

---

Diagram Walkthrough

flowchart LR
  A["IRealtimeHook Interface"] --> B["Default Method Implementations"]
  A --> C["ShouldReconnect Method Update"]
  C --> D["RealtimeHub Service"]
  D --> E["Function Call Context"]
  E --> F["Reconnection Logic"]

Loading

File Walkthrough

Relevant files

Enhancement

IRealtimeHook.cs Add default implementations to IRealtimeHook interface      src/Infrastructure/BotSharp.Abstraction/Realtime/IRealtimeHook.cs Added default implementations for all interface methods Modified ShouldReconnect to accept RoleDialogModel message parameter Used expression-bodied members for cleaner syntax Changed empty array syntax to [] +11/-4    RealtimeHub.cs Refactor reconnection logic in RealtimeHub service              src/Infrastructure/BotSharp.Core.Realtime/Services/RealtimeHub.cs Moved reconnection logic from general context to function call context Updated ShouldReconnect call to pass message parameter Added logging for reconnection events Removed duplicate reconnection check code +11/-13

---

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Logic Change The reconnection logic has been moved from general context to function call context only. This means reconnection will only be triggered after function calls, not in other scenarios where it might have been triggered before. This behavioral change should be validated to ensure it doesn't break existing functionality. var hooks = _services.GetHooks<IRealtimeHook>(_conn.CurrentAgentId); foreach (var hook in hooks) { if (await hook.ShouldReconnect(_conn, message)) { await _completer.Reconnect(_conn); _logger.LogWarning("Reconnecting to model due to function call: {FunctionName}", message.FunctionName); break; } } Breaking Change The ShouldReconnect method signature has changed to include a RoleDialogModel message parameter. This is a breaking change that will require all existing implementations to be updated to match the new signature. Task<bool> ShouldReconnect(RealtimeHubConnection conn, RoleDialogModel message) => Task.FromResult(false);

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Reconnection logic moved to wrong context The reconnection logic was moved from general message processing to only function call context, which means reconnection will no longer be triggered for non-function messages. This could break existing reconnection behavior for other message types and reduce the robustness of the realtime connection management. Examples: src/Infrastructure/BotSharp.Core.Realtime/Services/RealtimeHub.cs [102-114] await routing.InvokeFunction(message.FunctionName, message, options: new() { From = InvokeSource.Llm }); var hooks = _services.GetHooks<IRealtimeHook>(_conn.CurrentAgentId); foreach (var hook in hooks) { if (await hook.ShouldReconnect(_conn, message)) { await _completer.Reconnect(_conn); _logger.LogWarning("Reconnecting to model due to function call: {FunctionName}", message.FunctionName); break; ... (clipped 3 lines) Solution Walkthrough: Before: if (message.FunctionName != null) { // ... function call logic // Reconnection check is ONLY performed for function calls. var hooks = _services.GetHooks<IRealtimeHook>(...); foreach (var hook in hooks) { if (await hook.ShouldReconnect(_conn, message)) { await _completer.Reconnect(_conn); break; } } } After: if (message.FunctionName != null) { // ... function call logic } else { // ... regular message logic } // Reconnection check is performed for ALL messages. var hooks = _services.GetHooks<IRealtimeHook>(...); foreach (var hook in hooks) { if (await hook.ShouldReconnect(_conn, message)) { await _completer.Reconnect(_conn); break; } } Suggestion importance[1-10]: 9 __ Why: This suggestion correctly identifies a critical regression where the reconnection logic is now incorrectly scoped, potentially breaking reconnection for all non-function-call messages.	High
Possible issue	Maintain backward compatibility The method signature change breaks backward compatibility for existing implementations. Consider adding an overload instead of changing the existing method signature to maintain compatibility with existing code. src/Infrastructure/BotSharp.Abstraction/Realtime/IRealtimeHook.cs [18-19] +Task<bool> ShouldReconnect(RealtimeHubConnection conn) => Task.FromResult(false); Task<bool> ShouldReconnect(RealtimeHubConnection conn, RoleDialogModel message) => Task.FromResult(false); Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that changing an interface method signature is a breaking change for consumers of the library, and proposing to add an overload is a valid strategy to maintain backward compatibility.	Medium
General	Add exception handling for reconnection The reconnection logic should handle potential exceptions during the reconnection process. Add try-catch around the reconnection call to prevent unhandled exceptions from breaking the flow. src/Infrastructure/BotSharp.Core.Realtime/Services/RealtimeHub.cs [104-113] var hooks = _services.GetHooks<IRealtimeHook>(_conn.CurrentAgentId); foreach (var hook in hooks) { if (await hook.ShouldReconnect(_conn, message)) { - await _completer.Reconnect(_conn); - _logger.LogWarning("Reconnecting to model due to function call: {FunctionName}", message.FunctionName); + try + { + await _completer.Reconnect(_conn); + _logger.LogWarning("Reconnecting to model due to function call: {FunctionName}", message.FunctionName); + } + catch (Exception ex) + { + _logger.LogError(ex, "Failed to reconnect to model for function call: {FunctionName}", message.FunctionName); + } break; } } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out that the reconnection logic could fail and proposes adding a try-catch block with logging, which is a good practice for improving the code's robustness.	Medium
More