Skip to content

Conversation

@iceljc
Copy link
Collaborator

@iceljc iceljc commented Aug 6, 2025

PR Type

Bug fix


Description

  • Parse JSON strings in SignalR message handlers

  • Fix missing semicolon and duplicate dialog grouping


Diagram Walkthrough

flowchart LR
  A["SignalR Messages"] -- "JSON.parse()" --> B["Parsed Objects"]
  B --> C["Message Handlers"]
  D["Dialog Refresh"] --> E["Fixed Grouping Logic"]
Loading

File Walkthrough

Relevant files
Bug fix
signalr-service.js
Parse JSON strings in SignalR handlers                                     

src/lib/services/signalr-service.js

  • Add JSON.parse() to all SignalR message handlers
  • Parse incoming string messages before processing
  • Update conversation ID comparisons to use parsed objects
+49/-34 
chat-box.svelte
Fix syntax and dialog grouping                                                     

src/routes/chat/[agentId]/[conversationId]/chat-box.svelte

  • Add missing semicolon after assignMessageDisclaimer call
  • Add groupedDialogs = groupDialogs(dialogs) to refresh function
+2/-1     

@iceljc iceljc merged commit 04e1e6d into SciSharp:main Aug 6, 2025
1 of 2 checks passed
@qodo-merge-pro
Copy link

qodo-merge-pro bot commented Aug 6, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Error Handling

JSON.parse() calls lack error handling for malformed JSON strings, which could cause runtime exceptions and break SignalR message processing

  const obj = JSON.parse(conversation);
  if (conversationId === obj?.id) {
    console.log(`[OnConversationInitFromClient] ${obj.id}: ${obj.title}`);
    this.onConversationInitFromClient(obj);
  }
});

// register handlers for the hub methods
connection.on('OnMessageReceivedFromClient', (message) => {
  // do something when receiving a message, such as updating the UI or showing a notification
  const obj = JSON.parse(message);
  if (conversationId === obj?.conversation_id) {
    console.log(`[OnMessageReceivedFromClient] ${obj.sender.role}: ${obj.text}`);
    this.onMessageReceivedFromClient(obj);
  }
});

connection.on('OnMessageReceivedFromCsr', (message) => {
  // do something when receiving a message, such as updating the UI or showing a notification
  const obj = JSON.parse(message);
  if (conversationId === obj?.conversation_id) {
    console.log(`[OnMessageReceivedFromCsr] ${obj.role}: ${obj.content}`);
    this.onMessageReceivedFromCsr(obj);
  }
});

connection.on('OnMessageReceivedFromAssistant', (message) => {
  // do something when receiving a message, such as updating the UI or showing a notification
  const obj = JSON.parse(message);
  if (conversationId === obj?.conversation_id) {
    console.log(`[OnMessageReceivedFromAssistant] ${obj.sender.role}: ${obj.text}`);
    this.onMessageReceivedFromAssistant(obj);
  }
});

connection.on('OnNotificationGenerated', (message) => {
  const obj = JSON.parse(message);
  if (conversationId === obj?.conversation_id) {
    this.onNotificationGenerated(obj);
  }
});

connection.on('OnConversationContentLogGenerated', (log) => {
  const obj = JSON.parse(log);
  if (conversationId === obj?.conversation_id) {
    this.onConversationContentLogGenerated(obj);
  }
});

connection.on('OnConversateStateLogGenerated', (log) => {
  const obj = JSON.parse(log);
  if (conversationId === obj?.conversation_id) {
    this.onConversationStateLogGenerated(obj);
  }
});

connection.on('OnStateChangeGenerated', (log) => {
  const obj = JSON.parse(log);
  if (conversationId === obj?.conversation_id) {
    this.onStateChangeGenerated(obj);
  }
});

connection.on('OnAgentQueueChanged', (log) => {
  const obj = JSON.parse(log);
  if (conversationId === obj?.conversation_id) {
    this.onAgentQueueChanged(obj);
  }
});

connection.on('OnSenderActionGenerated', (data) => {
  const obj = JSON.parse(data);
  if (conversationId === obj?.conversation_id) {
    this.onSenderActionGenerated(obj);
  }
});

connection.on('OnMessageDeleted', (data) => {
  const obj = JSON.parse(data);
  if (conversationId === obj?.conversation_id) {
    this.onConversationMessageDeleted(obj);
  }
});

connection.on('BeforeReceiveLlmStreamMessage', data => {
  const obj = JSON.parse(data);
  if (conversationId === obj?.conversation_id) {
    this.beforeReceiveLlmStreamMessage(obj);
  }
});

connection.on('OnReceiveLlmStreamMessage', data => {
  const obj = JSON.parse(data);
  if (conversationId === obj?.conversation_id) {
    this.onReceiveLlmStreamMessage(obj);
  }
});

connection.on('AfterReceiveLlmStreamMessage', data => {
  const obj = JSON.parse(data);
  if (conversationId === obj?.conversation_id) {
    this.afterReceiveLlmStreamMessage(obj);
  }
});

connection.on('OnIndicationReceived', data => {
  const obj = JSON.parse(data);
  if (conversationId === obj?.conversation_id) {
    this.onIndicationReceived(obj);
  }
Duplicate Logic

The groupedDialogs assignment is duplicated in both refresh() and refreshDialogs() functions, potentially causing unnecessary recomputation

		groupedDialogs = groupDialogs(dialogs);
		return dialogs;
    }

	async function refresh() {
		// trigger UI render
		dialogs = await refreshDialogs();
		lastBotMsg = null;
		await tick();
		lastBotMsg = findLastBotMessage(dialogs);
		lastMsg = dialogs.slice(-1)[0];
		assignMessageDisclaimer(dialogs);
		groupedDialogs = groupDialogs(dialogs);

@qodo-merge-pro
Copy link

qodo-merge-pro bot commented Aug 6, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Add JSON parsing error handling

Add error handling around JSON.parse() calls to prevent crashes when receiving
malformed JSON. The current implementation will throw an exception if the
incoming message is not valid JSON, potentially breaking the SignalR connection.

src/lib/services/signalr-service.js [77-81]

-const obj = JSON.parse(conversation);
-if (conversationId === obj?.id) {
-  console.log(`[OnConversationInitFromClient] ${obj.id}: ${obj.title}`);
-  this.onConversationInitFromClient(obj);
+try {
+  const obj = JSON.parse(conversation);
+  if (conversationId === obj?.id) {
+    console.log(`[OnConversationInitFromClient] ${obj.id}: ${obj.title}`);
+    this.onConversationInitFromClient(obj);
+  }
+} catch (error) {
+  console.error('Failed to parse conversation data:', error);
 }
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that the new JSON.parse() call lacks error handling, which could cause a crash if the received data is not valid JSON, thus improving the application's robustness.

Medium
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant