Streaming response formatting

[erangi-ar]

No description provided.

[Copilot]

Pull request overview

This PR refactors the streaming response handling for the production LLM testing feature by replacing custom message parsing with a standardized markdown rendering approach and improving the streaming architecture.

Changes:

• Replaces custom message content parsing logic with react-markdown library for better markdown rendering support
• Refactors TestProductionLLM component to use useStreamingResponse hook for real-time message streaming
• Updates notification server URL from port 3005 to 4040 in development environment
• Adds new translation keys for the test production LLM interface in English and Estonian

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
GUI/translations/et/common.json	Adds Estonian translations for test production LLM interface
GUI/translations/en/common.json	Adds English translations for test production LLM interface
GUI/src/pages/TestProductionLLM/index.tsx	Refactors to use streaming hook, removes retry logic, implements token-by-token message building
GUI/src/hooks/useStreamingResponse.tsx	Updates to use environment variable for notification node URL
GUI/src/components/MessageContent/index.tsx	Replaces custom parsing with ReactMarkdown and remark-gfm for GFM support
GUI/src/components/MessageContent/MessageContent.scss	Adds comprehensive markdown styling including code blocks, lists, and links
GUI/package.json	Adds react-markdown ^10.1.0 and remark-gfm ^4.0.1 dependencies
GUI/package-lock.json	Updates lock file with new markdown-related dependencies
GUI/.env.development	Changes notification node URL from port 3005 to 4040

Files not reviewed (1)

• GUI/package-lock.json: Language not supported

Comments suppressed due to low confidence (1)

GUI/src/hooks/useStreamingResponse.tsx:113

• There's a potential race condition in the streaming setup. A 500ms delay is used to allow the SSE connection to establish before triggering the stream, but this is an arbitrary timeout that may not be sufficient in all network conditions. If the POST request (line 110-113) completes before the SSE connection is fully established, the server might start sending events that the client isn't ready to receive. Consider implementing a more robust handshake mechanism, such as waiting for a 'connected' event from the SSE connection before triggering the stream, or having the server return a success response only after verifying the client is connected.

        await new Promise(resolve => setTimeout(resolve, 500));

        // Step 3: POST to trigger streaming
        const postUrl = `${notificationNodeUrl}/channels/${channelId}/orchestrate/stream`;
        console.log('[API] Triggering stream:', postUrl);

        await axios.post(postUrl, {
          message,
          options,
        });

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The node prop is destructured but never used in the custom component renderers. While this doesn't cause runtime issues, it creates unused variable warnings. Consider using rest parameter syntax without explicitly mentioning node, or prefix it with an underscore to indicate it's intentionally unused:

Instead of:

a: ({ node, ...props }) => (...)

Use:

a: ({ ...props }) => (...)

Or if you need to keep the node parameter for future use:

a: ({ node: _node, ...props }) => (...)

Suggested change

	a: ({ node, ...props }) => (
	<a {...props} target="_blank" rel="noopener noreferrer" />
	),
	// Style strong/bold text
	strong: ({ node, ...props }) => (
	<strong {...props} className="markdown-bold" />
	),
	// Style ordered lists
	ol: ({ node, ...props }) => (
	<ol {...props} className="markdown-list" />
	),
	// Style list items
	li: ({ node, ...props }) => (
	a: ({ ...props }) => (
	<a {...props} target="_blank" rel="noopener noreferrer" />
	),
	// Style strong/bold text
	strong: ({ ...props }) => (
	<strong {...props} className="markdown-bold" />
	),
	// Style ordered lists
	ol: ({ ...props }) => (
	<ol {...props} className="markdown-list" />
	),
	// Style list items
	li: ({ ...props }) => (

[Copilot]

The Content Security Policy (CSP) configuration still references the old notification node URL port (http://localhost:3005) in the connect-src directive, but REACT_APP_NOTIFICATION_NODE_URL has been changed to http://localhost:4040. This mismatch will cause CSP violations and prevent the application from connecting to the notification server. Update the CSP to include http://localhost:4040 in the connect-src directive.

[Copilot]

The channel ID is generated once per component mount using useMemo with an empty dependency array. This means the same channel ID will be reused across multiple conversations within the same session. If a user sends multiple messages or clears the chat, they'll continue using the same channel ID. Consider whether this is the intended behavior, or if a new channel ID should be generated for each conversation/session. If channel IDs should be unique per conversation, consider generating a new ID when the chat is cleared or creating a function to regenerate the channel ID.

[Copilot]

When an error occurs during streaming or the component unmounts while streaming is active, the bot message that was partially populated may remain in an incomplete state. The onError callback sets isLoading to false but doesn't clean up the partial bot message from the messages array. Consider adding logic to remove incomplete bot messages when streaming fails, or add an error state to the message to indicate it's incomplete.

[Copilot]

The setIsLoading(false) is called inside the onToken callback when the first token arrives (line 83). However, if no tokens are received before stream completion or an error occurs, isLoading may not be properly reset. The loading state should be managed more consistently - either always set to false in onComplete and onError callbacks, or let the parent component handle it more explicitly. Currently, the loading state management is split between the token callback and error handling, which could lead to inconsistent UI states.

[Copilot]

The conversation history is prepared at line 60-64 by mapping over the existing messages array, but this happens before the user message is added to the messages state (which occurs at line 52). This means the current user message is not included in the conversation history sent to the backend. The comment on line 59 states "exclude the current user message", but this appears to be intentional. However, verify that the backend expects the current message to be sent separately (via the message parameter) and doesn't need it in the conversation history.

[Copilot]

The notificationNodeUrl environment variable is read but there's no validation to ensure it's defined. If the environment variable is missing, the URLs will be constructed with undefined, leading to malformed URLs like "undefined/sse/stream/...". Add a validation check to ensure the environment variable is set before using it, or provide a fallback value with a helpful error message.

Suggested change

	const notificationNodeUrl = import.meta.env.REACT_APP_NOTIFICATION_NODE_URL;
	const getNotificationNodeUrl = (): string => {
	const value = import.meta.env.REACT_APP_NOTIFICATION_NODE_URL;
	if (!value) {
	throw new Error(
	'Environment variable REACT_APP_NOTIFICATION_NODE_URL is not defined. ' +
	'Please set it to the base URL of the notification service to enable streaming responses.'
	);
	}
	return value;
	};
	const notificationNodeUrl = getNotificationNodeUrl();

[Copilot]

The nullish coalescing operator (??"") is used as a fallback for the placeholder translation, but this is unnecessary. The t() function from react-i18next returns the key itself as a string if the translation is missing, so it will never return null or undefined. This fallback can be removed to simplify the code.

Suggested change

	placeholder={t('testProductionLLM.messagePlaceholder')??""}
	placeholder={t('testProductionLLM.messagePlaceholder')}