Skip to content

Conversation

@iceljc
Copy link
Collaborator

@iceljc iceljc commented Sep 18, 2025

PR Type

Enhancement


Description

  • Add intermediate message handling for chat conversations

  • Widen knowledge base collection dropdown interface

  • Improve message flow with dummy message tracking

  • Enhance UI layout for knowledge base components


Diagram Walkthrough

flowchart LR
  A["SignalR Service"] -- "new handler" --> B["Intermediate Messages"]
  B --> C["Chat Box"]
  C --> D["Message Queue"]
  E["Knowledge Base UI"] -- "wider dropdown" --> F["Collection Selection"]
Loading

File Walkthrough

Relevant files
Enhancement
chat-box.svelte
Add intermediate message handling to chat                               

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

  • Add onIntermediateMessageReceivedFromAssistant handler for new message
    type
  • Implement logic to insert intermediate messages before dummy messages
  • Add is_dummy and is_appended flags to message objects
  • Update message queue handling to use is_dummy flag instead of message
    ID comparison
+30/-3   
+page.svelte
Restructure knowledge base documents layout                           

src/routes/page/knowledge-base/documents/+page.svelte

  • Split collection dropdown container into separate dropdown and add
    containers
  • Restructure HTML layout for better component organization
+2/-0     
+page.svelte
Restructure knowledge base Q&A layout                                       

src/routes/page/knowledge-base/question-answer/+page.svelte

  • Split collection dropdown container into separate dropdown and add
    containers
  • Restructure HTML layout for better component organization
+2/-0     
_knowledgebase.scss
Widen knowledge base dropdown styling                                       

src/lib/scss/custom/pages/_knowledgebase.scss

  • Add minimum width of 300px to collection dropdown container
  • Create new .collection-add-container class for add button styling
+7/-0     
conversationTypes.js
Add new message type properties                                                   

src/lib/helpers/types/conversationTypes.js

  • Add is_dummy and is_appended boolean properties to message type
    definitions
+2/-0     
signalr-service.js
Add intermediate message SignalR support                                 

src/lib/services/signalr-service.js

  • Add onIntermediateMessageReceivedFromAssistant callback property
  • Register new SignalR event handler for intermediate messages
+12/-0   

@qodo-merge-pro
Copy link

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

Ordering Logic

Intermediate assistant messages are inserted before the last dummy message by finding the last item with is_dummy; verify this does not break chronological order when multiple dummies exist or when the dummy is removed/updated during streaming.

    function onIntermediateMessageReceivedFromAssistant(message) {
		const idx = dialogs.findLastIndex(x => x.is_dummy);
		if (idx >= 0) {
			dialogs.splice(idx, 0, {
				...message,
				is_chat_message: true,
				is_appended: true
			});
		} else {
			dialogs.push({
				...message,
				is_chat_message: true,
				is_appended: true
			});
		}

		refresh();

		if (isFrame) {
			window.parent.postMessage(message, "*");
		}
    }
State Flags Consistency

New flags is_dummy and is_appended drive several branches; ensure these flags are consistently set/cleared when a dummy is finalized to avoid stale flags causing UI or queue handling errors.

		refresh();

		if (isFrame) {
			window.parent.postMessage(message, "*");
		}
    }

	/** @param {import('$conversationTypes').ChatResponseModel} message */
	function beforeReceiveLlmStreamMessage(message) {
		isStreaming = true;
		if (dialogs[dialogs.length - 1]?.message_id !== message.message_id
			|| dialogs[dialogs.length - 1]?.sender?.role !== UserRole.Assistant
		) {
			dialogs.push({
				...message,
				is_chat_message: false,
				is_dummy: true
			});
		}
		refresh();
	}


	/** @param {import('$conversationTypes').ChatResponseModel} message */
	function onReceiveLlmStreamMessage(message) {
		isThinking = false;
		isStreaming = true;

		if (!USE_MESSAGE_QUEUE) {
			if (lastMsg?.sender?.role === UserRole.Assistant
				&& lastMsg?.is_dummy
			) {
Event Handling

New SignalR handler for intermediate messages lacks throttling/debouncing; confirm high-frequency events won’t cause excessive re-renders or memory growth in dialogs.

  // 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('OnIntermediateMessageReceivedFromAssistant', (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(`[OnIntermediateMessageReceivedFromAssistant] ${obj.sender.role}: ${obj.text}`);
    this.onIntermediateMessageReceivedFromAssistant(obj);
  }
});

@qodo-merge-pro
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Use a separate UI for intermediate messages

Instead of inserting intermediate messages into the main chat log using complex
flags like is_dummy, display them in a separate UI element. This would simplify
chat state management and improve code maintainability.

Examples:

src/routes/chat/[agentId]/[conversationId]/chat-box.svelte [550-571]
    function onIntermediateMessageReceivedFromAssistant(message) {
		const idx = dialogs.findLastIndex(x => x.is_dummy);
		if (idx >= 0) {
			dialogs.splice(idx, 0, {
				...message,
				is_chat_message: true,
				is_appended: true
			});
		} else {
			dialogs.push({

 ... (clipped 12 lines)
src/lib/helpers/types/conversationTypes.js [171-172]
 * @property {boolean} is_dummy
 * @property {boolean} is_appended

Solution Walkthrough:

Before:

// In chat-box.svelte
let dialogs = []; // Contains final, dummy, and intermediate messages

function onIntermediateMessageReceivedFromAssistant(message) {
    const dummyMessageIndex = dialogs.findLastIndex(x => x.is_dummy);
    if (dummyMessageIndex >= 0) {
        // Insert the intermediate message before the dummy placeholder
        dialogs.splice(dummyMessageIndex, 0, {
            ...message,
            is_appended: true
        });
    } else {
        dialogs.push({ ...message, is_appended: true });
    }
    refresh();
}

function beforeReceiveLlmStreamMessage(message) {
    // Add a dummy placeholder for the upcoming streaming message
    dialogs.push({ ...message, is_dummy: true });
}

After:

// In chat-box.svelte
let dialogs = []; // Contains only final chat messages
let intermediateMessages = []; // State for a separate UI component

function onIntermediateMessageReceivedFromAssistant(message) {
    // Add message to a separate list for a dedicated UI
    intermediateMessages.push(message);
    // Update the separate UI, not the main dialogs array
}

function onMessageReceivedFromAssistant(message) {
    // When the final message arrives, clear the intermediate ones
    intermediateMessages = [];
    // And add the final message to the main chat history
    dialogs.push(message);
    refresh();
}
Suggestion importance[1-10]: 9

__

Why: This is a critical design suggestion that correctly identifies how the new flags (is_dummy, is_appended) and array splicing logic significantly complicate chat state management, proposing a robust architectural alternative that would enhance maintainability.

High
Possible issue
Locate message to replace by flag

In onMessageReceivedFromAssistant, locate the message to be replaced by
searching for the is_dummy flag instead of assuming it's the last item in the
dialogs array. This prevents a bug where intermediate messages could alter the
array order.

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

 function onMessageReceivedFromAssistant(message) {
 		if (!message.is_streaming) {
-			if (dialogs[dialogs.length - 1]?.message_id === message.message_id
-				&& dialogs[dialogs.length - 1]?.sender?.role === UserRole.Assistant
-				&& !dialogs[dialogs.length - 1]?.is_appended
-			) {
-				dialogs[dialogs.length - 1] = {
+			const idx = dialogs.findLastIndex(x => x.is_dummy && x.message_id === message.message_id);
+			if (idx >= 0) {
+				dialogs[idx] = {
 					...message,
 					is_chat_message: true

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a significant bug introduced in the PR where the final assistant message would fail to replace its placeholder if an intermediate message arrived first. The proposed fix is accurate and necessary for the feature to work correctly.

High
  • More

@iceljc iceljc merged commit 6fecedc into SciSharp:main Sep 18, 2025
1 of 2 checks passed
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