From bc335b28cd1edc506e5b2732427dc9aab18dd34e Mon Sep 17 00:00:00 2001 From: Debsmita Santra Date: Tue, 27 Jan 2026 13:04:39 +0530 Subject: [PATCH 1/2] fix(lightspeed): ux improvements --- .../lightspeed/src/api/LightspeedApiClient.ts | 30 ++++- .../api/__tests__/LightspeedApiClient.test.ts | 32 ++++- .../lightspeed/src/components/DeleteModal.tsx | 8 +- .../src/components/LightspeedChatBox.tsx | 55 ++++++--- .../components/RenameConversationModal.tsx | 42 +++++-- .../src/components/ToolCallContent.tsx | 69 +++-------- .../components/__tests__/DeleteModal.test.tsx | 2 +- .../RenameConversationModal.test.tsx | 113 +++++++++++++++++- .../__tests__/ToolCallContent.test.tsx | 84 ++----------- .../plugins/lightspeed/src/translations/de.ts | 1 - .../plugins/lightspeed/src/translations/es.ts | 1 - .../plugins/lightspeed/src/translations/fr.ts | 1 - .../plugins/lightspeed/src/translations/it.ts | 1 - .../plugins/lightspeed/src/translations/ja.ts | 1 - .../lightspeed/src/translations/ref.ts | 4 +- .../src/utils/lightspeed-chatbox-utils.tsx | 17 ++- .../lightspeed/src/utils/toolCallMapper.tsx | 3 +- 17 files changed, 272 insertions(+), 192 deletions(-) diff --git a/workspaces/lightspeed/plugins/lightspeed/src/api/LightspeedApiClient.ts b/workspaces/lightspeed/plugins/lightspeed/src/api/LightspeedApiClient.ts index 40a3d222ab..7974730586 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/api/LightspeedApiClient.ts +++ b/workspaces/lightspeed/plugins/lightspeed/src/api/LightspeedApiClient.ts @@ -195,9 +195,18 @@ export class LightspeedApiClient implements LightspeedAPI { ); if (!response.ok) { - throw new Error( - `failed to delete conversation, status ${response.status}: ${response.statusText}`, - ); + let errorMessage = `failed to delete conversation, status ${response.status}: ${response.statusText}`; + + try { + const errorBody = await response.json(); + if (errorBody?.error) { + errorMessage = errorBody.error; + } + } catch (e) { + // If JSON parsing fails, use the default error message + } + + throw new Error(errorMessage); } return { success: true }; } @@ -217,9 +226,18 @@ export class LightspeedApiClient implements LightspeedAPI { ); if (!response.ok) { - throw new Error( - `failed to rename conversation, status ${response.status}: ${response.statusText}`, - ); + let errorMessage = `failed to rename conversation, status ${response.status}: ${response.statusText}`; + + try { + const errorBody = await response.json(); + if (errorBody?.error) { + errorMessage = errorBody.error; + } + } catch (e) { + // If JSON parsing fails, use the default error message + } + + throw new Error(errorMessage); } return { success: true }; } diff --git a/workspaces/lightspeed/plugins/lightspeed/src/api/__tests__/LightspeedApiClient.test.ts b/workspaces/lightspeed/plugins/lightspeed/src/api/__tests__/LightspeedApiClient.test.ts index 0bd63ce59a..d84a04d6ef 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/api/__tests__/LightspeedApiClient.test.ts +++ b/workspaces/lightspeed/plugins/lightspeed/src/api/__tests__/LightspeedApiClient.test.ts @@ -214,14 +214,44 @@ describe('LightspeedApiClient', () => { ok: false, status: 400, statusText: 'Bad Request', + json: jest.fn().mockResolvedValue({ + error: + 'Error from lightspeed-core server: The conversation ID conversationId- has invalid format.', + }), } as unknown as Response); await expect( client.renameConversation('conv-123', 'New Name'), ).rejects.toThrow( - 'failed to rename conversation, status 400: Bad Request', + 'Error from lightspeed-core server: The conversation ID conversationId- has invalid format.', ); }); + + it('should throw error with statusText when JSON parsing fails', async () => { + mockFetchApi.fetch.mockResolvedValue({ + ok: false, + status: 400, + statusText: 'Bad Request', + json: jest.fn().mockRejectedValue(new Error('Invalid JSON')), + } as unknown as Response); + + await expect( + client.renameConversation('conv-123', 'New Name'), + ).rejects.toThrow('Bad Request'); + }); + + it('should throw error with statusText when JSON does not contain error field', async () => { + mockFetchApi.fetch.mockResolvedValue({ + ok: false, + status: 400, + statusText: 'Bad Request', + json: jest.fn().mockResolvedValue({ message: 'Some other error' }), + } as unknown as Response); + + await expect( + client.renameConversation('conv-123', 'New Name'), + ).rejects.toThrow('Bad Request'); + }); }); describe('getFeedbackStatus', () => { diff --git a/workspaces/lightspeed/plugins/lightspeed/src/components/DeleteModal.tsx b/workspaces/lightspeed/plugins/lightspeed/src/components/DeleteModal.tsx index 14e685fc72..7220ea1b73 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/components/DeleteModal.tsx +++ b/workspaces/lightspeed/plugins/lightspeed/src/components/DeleteModal.tsx @@ -99,12 +99,8 @@ export const DeleteModal = ({ {t('conversation.delete.confirm.message')} {isError && ( - - - {t('conversation.action.error' as any, { - error: String(error), - })} - + + {String(error)} )} diff --git a/workspaces/lightspeed/plugins/lightspeed/src/components/LightspeedChatBox.tsx b/workspaces/lightspeed/plugins/lightspeed/src/components/LightspeedChatBox.tsx index 1029fd02b1..2697d87813 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/components/LightspeedChatBox.tsx +++ b/workspaces/lightspeed/plugins/lightspeed/src/components/LightspeedChatBox.tsx @@ -222,7 +222,7 @@ export const LightspeedChatBox = forwardRef( // Map first tool call to PatternFly's toolCall prop const firstToolCall = message.toolCalls?.[0]; const toolCallProp = firstToolCall - ? mapToPatternFlyToolCall(firstToolCall, t) + ? mapToPatternFlyToolCall(firstToolCall, t, message.role) : undefined; // Handle additional tool calls (if any) via extraContent @@ -256,26 +256,48 @@ export const LightspeedChatBox = forwardRef( body: reasoningContent, expandableSectionProps: {}, }; - extraContentParts.beforeMainContent = ( - - ); } } + const allToolCalls: React.ReactNode[] = []; + + // Add first tool call if it exists + if (toolCallProp && firstToolCall) { + allToolCalls.push( +
+ +
, + ); + } + + // Add additional tool calls if (additionalToolCalls && additionalToolCalls.length > 0) { - extraContentParts.afterMainContent = ( + additionalToolCalls.forEach(tc => { + const tcProps = mapToPatternFlyToolCall(tc, t, message.role); + allToolCalls.push( +
+ +
, + ); + }); + } + + // Show Thinking first, then tool calls + if (deepThinking || allToolCalls.length > 0) { + extraContentParts.beforeMainContent = ( <> - {additionalToolCalls.map(tc => { - const tcProps = mapToPatternFlyToolCall(tc, t); - return ( -
- -
- ); - })} + {deepThinking && } + {allToolCalls.length > 0 && ( +
+ {allToolCalls} +
+ )} ); } @@ -295,7 +317,6 @@ export const LightspeedChatBox = forwardRef( return ( diff --git a/workspaces/lightspeed/plugins/lightspeed/src/components/RenameConversationModal.tsx b/workspaces/lightspeed/plugins/lightspeed/src/components/RenameConversationModal.tsx index 500e73b5e3..a6b8efdd7f 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/components/RenameConversationModal.tsx +++ b/workspaces/lightspeed/plugins/lightspeed/src/components/RenameConversationModal.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import { useState } from 'react'; +import { useEffect, useState } from 'react'; import { TextField } from '@material-ui/core'; import CancelOutlinedIcon from '@mui/icons-material/CancelOutlined'; @@ -29,6 +29,7 @@ import DialogTitle from '@mui/material/DialogTitle'; import IconButton from '@mui/material/IconButton'; import Typography from '@mui/material/Typography'; +import { useConversations } from '../hooks/useConversations'; import { useRenameConversation } from '../hooks/useRenameConversation'; import { useTranslation } from '../hooks/useTranslation'; @@ -47,21 +48,41 @@ export const RenameConversationModal = ({ isError, error, } = useRenameConversation(); + const { data: conversations } = useConversations(); const [chatName, setChatName] = useState(''); + const [originalChatName, setOriginalChatName] = useState(''); + + useEffect(() => { + if (isOpen && conversations) { + const conversation = conversations.find( + c => c.conversation_id === conversationId, + ); + if (conversation) { + setChatName(conversation.topic_summary); + setOriginalChatName(conversation.topic_summary); + } else { + setChatName(''); + setOriginalChatName(''); + } + } + }, [isOpen, conversationId, conversations]); const handleChatNameChange = (event: React.ChangeEvent) => { setChatName(event.target.value); }; - const handleRename = () => { - (async () => { + const handleRename = async () => { + try { await renameConversation({ conversation_id: conversationId, newName: chatName, invalidateCache: false, }); onClose(); - })(); + } catch (e) { + // eslint-disable-next-line no-console + console.warn(e); + } }; return ( @@ -123,12 +144,8 @@ export const RenameConversationModal = ({ /> {isError && ( - - - {t('conversation.action.error' as any, { - error: String(error), - })} - + + {String(error)} )} @@ -137,7 +154,10 @@ export const RenameConversationModal = ({ sx={{ textTransform: 'none', }} - disabled={chatName.trim() === ''} + disabled={ + chatName.trim() === '' || + chatName.trim() === originalChatName.trim() + } onClick={handleRename} > {t('conversation.rename.confirm.action')} diff --git a/workspaces/lightspeed/plugins/lightspeed/src/components/ToolCallContent.tsx b/workspaces/lightspeed/plugins/lightspeed/src/components/ToolCallContent.tsx index 50ba4f346a..ab3eaff986 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/components/ToolCallContent.tsx +++ b/workspaces/lightspeed/plugins/lightspeed/src/components/ToolCallContent.tsx @@ -14,10 +14,8 @@ * limitations under the License. */ -import { useState } from 'react'; - +import { Message } from '@patternfly/chatbot'; import { - Button, Content, ContentVariants, DescriptionList, @@ -27,30 +25,27 @@ import { Flex, FlexItem, Label, - Tooltip, } from '@patternfly/react-core'; -import { CopyIcon, WrenchIcon } from '@patternfly/react-icons'; +import { WrenchIcon } from '@patternfly/react-icons'; import { useTranslation } from '../hooks/useTranslation'; import { ToolCall } from '../types'; interface ToolCallContentProps { toolCall: ToolCall; + role?: 'user' | 'bot'; } /** * Lightweight component for rendering tool call expandable content. * Used inside PatternFly's ToolCall component's expandableContent prop. */ -export const ToolCallContent = ({ toolCall }: ToolCallContentProps) => { +export const ToolCallContent = ({ + toolCall, + role = 'bot', +}: ToolCallContentProps) => { const { t } = useTranslation(); - const [isExpanded, setIsExpanded] = useState(false); - - const handleCopy = async () => { - if (toolCall.response) { - await globalThis.navigator.clipboard.writeText(toolCall.response); - } - }; + // const [isExpanded, setIsExpanded] = useState(false); const formatExecutionTime = (seconds?: number): string => { if (seconds === undefined) return ''; @@ -210,16 +205,6 @@ export const ToolCallContent = ({ toolCall }: ToolCallContentProps) => { {t('toolCall.response')} - - - - - )} diff --git a/workspaces/lightspeed/plugins/lightspeed/src/components/__tests__/DeleteModal.test.tsx b/workspaces/lightspeed/plugins/lightspeed/src/components/__tests__/DeleteModal.test.tsx index 7a3bf9935f..2bfd1e069f 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/components/__tests__/DeleteModal.test.tsx +++ b/workspaces/lightspeed/plugins/lightspeed/src/components/__tests__/DeleteModal.test.tsx @@ -142,7 +142,7 @@ describe('DeleteModal', () => { ); expect(screen.getByRole('alert')).toBeInTheDocument(); - expect(screen.getByText(/Error occured/i)).toBeInTheDocument(); + expect(screen.getByText(/Error/i)).toBeInTheDocument(); }); test('should not call onConfirm when delete fails', async () => { diff --git a/workspaces/lightspeed/plugins/lightspeed/src/components/__tests__/RenameConversationModal.test.tsx b/workspaces/lightspeed/plugins/lightspeed/src/components/__tests__/RenameConversationModal.test.tsx index 0eaba182a1..34133efc49 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/components/__tests__/RenameConversationModal.test.tsx +++ b/workspaces/lightspeed/plugins/lightspeed/src/components/__tests__/RenameConversationModal.test.tsx @@ -16,6 +16,7 @@ import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { useConversations } from '../../hooks/useConversations'; import { mockUseTranslation } from '../../test-utils/mockTranslations'; import { RenameConversationModal } from '../RenameConversationModal'; @@ -35,6 +36,22 @@ jest.mock('../../hooks/useRenameConversation', () => ({ useRenameConversation: () => mockUseRenameConversation(), })); +jest.mock('../../hooks/useConversations', () => ({ + useConversations: jest.fn().mockReturnValue({ + data: [ + { + conversation_id: 'test-conversation-id', + topic_summary: 'Old Chat Name', + last_message_timestamp: Date.now(), + }, + ], + isRefetching: false, + isLoading: false, + }), +})); + +const mockUseConversations = useConversations as jest.Mock; + describe('RenameConversationModal', () => { const onClose = jest.fn(); const conversationId = 'test-conversation-id'; @@ -42,9 +59,25 @@ describe('RenameConversationModal', () => { beforeEach(() => { jest.clearAllMocks(); mockMutateAsync.mockResolvedValue({ success: true }); + mockUseConversations.mockReturnValue({ + data: [ + { + conversation_id: 'test-conversation-id', + topic_summary: 'Old Chat Name', + last_message_timestamp: Date.now(), + }, + ], + isRefetching: false, + isLoading: false, + }); + mockUseRenameConversation.mockReturnValue({ + mutateAsync: mockMutateAsync, + isError: false, + error: 'null', + }); }); - test('should render the modal with correct content when open', () => { + test('should render the modal with correct content when open', async () => { render( { ); expect(screen.getByText('Rename chat?')).toBeInTheDocument(); - expect(screen.getByLabelText('Chat name')).toBeInTheDocument(); + const input = screen.getByLabelText('Chat name'); + expect(input).toBeInTheDocument(); expect(screen.getByRole('button', { name: 'Rename' })).toBeInTheDocument(); expect(screen.getByRole('button', { name: 'Cancel' })).toBeInTheDocument(); + + // Wait for the input to be populated with the old chat name + await waitFor(() => { + expect(input).toHaveValue('Old Chat Name'); + }); }); test('should not render when isOpen is false', () => { @@ -98,6 +137,12 @@ describe('RenameConversationModal', () => { ); const input = screen.getByLabelText('Chat name'); + + // Wait for the input to be populated with the old chat name + await waitFor(() => { + expect(input).toHaveValue('Old Chat Name'); + }); + const newName = 'My New Conversation Name'; fireEvent.change(input, { target: { value: newName } }); @@ -116,8 +161,8 @@ describe('RenameConversationModal', () => { }); }); - test('should display error message when rename fails', () => { - mockUseRenameConversation.mockReturnValueOnce({ + test('should display error message when rename fails', async () => { + mockUseRenameConversation.mockReturnValue({ mutateAsync: mockMutateAsync, isError: true, error: new Error('Rename failed') as any, @@ -131,7 +176,63 @@ describe('RenameConversationModal', () => { />, ); - expect(screen.getByRole('alert')).toBeInTheDocument(); - expect(screen.getByText(/Error occured/i)).toBeInTheDocument(); + await waitFor(() => { + expect(screen.getByRole('alert')).toBeInTheDocument(); + }); + + expect(screen.getByText(/Error/i)).toBeInTheDocument(); + expect(screen.getByText(/Rename failed/i)).toBeInTheDocument(); + }); + + test('should handle case when conversation is not found in list', async () => { + mockUseConversations.mockReturnValueOnce({ + data: [ + { + conversation_id: 'other-conversation-id', + topic_summary: 'Other Chat', + last_message_timestamp: Date.now(), + }, + ], + isRefetching: false, + isLoading: false, + }); + + render( + , + ); + + const input = screen.getByLabelText('Chat name'); + + // Input should be empty when conversation is not found + await waitFor(() => { + expect(input).toHaveValue(''); + }); + }); + + test('should handle case when conversations data is undefined', async () => { + mockUseConversations.mockReturnValueOnce({ + data: undefined, + isRefetching: false, + isLoading: false, + }); + + render( + , + ); + + const input = screen.getByLabelText('Chat name'); + + // Input should be empty when conversations data is undefined + await waitFor(() => { + expect(input).toHaveValue(''); + }); }); }); diff --git a/workspaces/lightspeed/plugins/lightspeed/src/components/__tests__/ToolCallContent.test.tsx b/workspaces/lightspeed/plugins/lightspeed/src/components/__tests__/ToolCallContent.test.tsx index 3a1d4450ef..330ae981f9 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/components/__tests__/ToolCallContent.test.tsx +++ b/workspaces/lightspeed/plugins/lightspeed/src/components/__tests__/ToolCallContent.test.tsx @@ -14,23 +14,12 @@ * limitations under the License. */ -import { fireEvent, render, screen, waitFor } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; +import { render, screen } from '@testing-library/react'; import { mockUseTranslation } from '../../test-utils/mockTranslations'; import { ToolCall } from '../../types'; import { ToolCallContent } from '../ToolCallContent'; -// Mock clipboard API -Object.defineProperty(globalThis, 'navigator', { - value: { - clipboard: { - writeText: jest.fn(), - }, - }, - writable: true, -}); - jest.mock('../../hooks/useTranslation', () => ({ useTranslation: jest.fn(() => mockUseTranslation()), })); @@ -90,23 +79,13 @@ describe('ToolCallContent', () => { expect(screen.queryByText('type')).not.toBeInTheDocument(); }); - test('should truncate long responses with show more button', () => { - const longResponse = 'A'.repeat(400); // More than 300 characters - const longResponseToolCall: ToolCall = { - ...baseToolCall, - response: longResponse, - }; - - render(); - - // Should show "show more" button - expect(screen.getByText('show more')).toBeInTheDocument(); - - // Click "show more" - fireEvent.click(screen.getByText('show more')); + test('should render Message component for response', () => { + render(); - // Should now show "show less" - expect(screen.getByText('show less')).toBeInTheDocument(); + // Message component should render the response content + expect( + screen.getByText('Found 5 users in the catalog'), + ).toBeInTheDocument(); }); test('should format execution time correctly in milliseconds', () => { @@ -121,24 +100,6 @@ describe('ToolCallContent', () => { expect(screen.getByText(/Execution time:.*240ms/)).toBeInTheDocument(); }); - test('should handle keyboard navigation for show more button', () => { - const longResponse = 'A'.repeat(400); - const longResponseToolCall: ToolCall = { - ...baseToolCall, - response: longResponse, - }; - - render(); - - const showMoreButton = screen.getByRole('button', { name: 'show more' }); - - // Press Enter to toggle (PatternFly Button handles keyboard events internally) - fireEvent.click(showMoreButton); - - // Should now show "show less" - expect(screen.getByText('show less')).toBeInTheDocument(); - }); - test('should show no parameters section when arguments are empty', () => { const noArgsToolCall: ToolCall = { ...baseToolCall, @@ -193,22 +154,6 @@ describe('ToolCallContent', () => { expect(screen.queryByText('Response')).not.toBeInTheDocument(); }); - test('should show copy button and copy response when clicked', async () => { - render(); - - // Copy button should be visible - const copyButton = screen.getByRole('button', { name: 'Copy response' }); - expect(copyButton).toBeInTheDocument(); - - // Click the copy button - fireEvent.click(copyButton); - - // Verify clipboard was called - expect(globalThis.navigator.clipboard.writeText).toHaveBeenCalledWith( - 'Found 5 users in the catalog', - ); - }); - test('should not show copy button when no response', () => { const noResponseToolCall: ToolCall = { ...baseToolCall, @@ -223,21 +168,6 @@ describe('ToolCallContent', () => { ).not.toBeInTheDocument(); }); - test('should show tooltip on copy button hover', async () => { - const user = userEvent.setup(); - render(); - - const copyButton = screen.getByRole('button', { name: 'Copy response' }); - - // Hover over the copy button - await user.hover(copyButton); - - // Tooltip should appear - await waitFor(() => { - expect(screen.getByRole('tooltip')).toBeInTheDocument(); - }); - }); - test('should render wrench icon in MCP Server header', () => { render(); diff --git a/workspaces/lightspeed/plugins/lightspeed/src/translations/de.ts b/workspaces/lightspeed/plugins/lightspeed/src/translations/de.ts index 2fc40f5c0e..db322d5538 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/translations/de.ts +++ b/workspaces/lightspeed/plugins/lightspeed/src/translations/de.ts @@ -77,7 +77,6 @@ const lightspeedTranslationDe = createTranslationMessages({ 'conversation.rename.confirm.title': 'Chat umbenennen?', 'conversation.rename.confirm.action': 'Umbenennen', 'conversation.rename.placeholder': 'Chat-Name', - 'conversation.action.error': 'Fehler aufgetreten: {{error}}', // Permissions 'permission.required.title': 'Fehlende Berechtigungen', diff --git a/workspaces/lightspeed/plugins/lightspeed/src/translations/es.ts b/workspaces/lightspeed/plugins/lightspeed/src/translations/es.ts index 9e02d857c9..04513634cb 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/translations/es.ts +++ b/workspaces/lightspeed/plugins/lightspeed/src/translations/es.ts @@ -80,7 +80,6 @@ const lightspeedTranslationEs = createTranslationMessages({ 'conversation.rename.confirm.title': '¿Renombrar chat?', 'conversation.rename.confirm.action': 'Renombrar', 'conversation.rename.placeholder': 'Nombre del chat', - 'conversation.action.error': 'Error ocurrido: {{error}}', // Permissions 'permission.required.title': 'Permisos faltantes', diff --git a/workspaces/lightspeed/plugins/lightspeed/src/translations/fr.ts b/workspaces/lightspeed/plugins/lightspeed/src/translations/fr.ts index ed03d81030..39c75fa1f5 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/translations/fr.ts +++ b/workspaces/lightspeed/plugins/lightspeed/src/translations/fr.ts @@ -71,7 +71,6 @@ const lightspeedTranslationFr = createTranslationMessages({ 'conversation.rename.confirm.title': 'Renommer la conversation ?', 'conversation.rename.confirm.action': 'Renommer', 'conversation.rename.placeholder': 'Nom de la conversation', - 'conversation.action.error': 'Erreur: {{error}}', 'permission.required.title': 'Autorisations manquantes', 'permission.required.description': 'Pour afficher le plugin lightspeed, veuillez contacter votre administrateur pour qu’il vous donne les permissionslightspeed.chat.read et lightspeed.chat.create .', diff --git a/workspaces/lightspeed/plugins/lightspeed/src/translations/it.ts b/workspaces/lightspeed/plugins/lightspeed/src/translations/it.ts index f361f5c761..e401c80b16 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/translations/it.ts +++ b/workspaces/lightspeed/plugins/lightspeed/src/translations/it.ts @@ -73,7 +73,6 @@ const lightspeedTranslationIt = createTranslationMessages({ 'conversation.rename.confirm.title': 'Rinominare la chat?', 'conversation.rename.confirm.action': 'Rinomina', 'conversation.rename.placeholder': 'Nome della chat', - 'conversation.action.error': 'Si è verificato un errore: {{error}}', 'permission.required.title': 'Autorizzazioni mancanti', 'permission.required.description': "Per visualizzare il plugin Lightspeed, contattare l'amministratore per ottenere le autorizzazioni lightspeed.chat.read e lightspeed.chat.create.", diff --git a/workspaces/lightspeed/plugins/lightspeed/src/translations/ja.ts b/workspaces/lightspeed/plugins/lightspeed/src/translations/ja.ts index 53767855d5..f75a6aa4d7 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/translations/ja.ts +++ b/workspaces/lightspeed/plugins/lightspeed/src/translations/ja.ts @@ -71,7 +71,6 @@ const lightspeedTranslationJa = createTranslationMessages({ 'conversation.rename.confirm.title': 'チャットの名前を変更しますか?', 'conversation.rename.confirm.action': '名前の変更', 'conversation.rename.placeholder': 'チャット名', - 'conversation.action.error': 'エラーが発生しました: {{error}}', 'permission.required.title': '権限の不足', 'permission.required.description': 'lightspeed プラグインを表示するには、管理者に連絡して lightspeed.chat.read および lightspeed.chat.create 権限を付与してもらうよう依頼してください。', diff --git a/workspaces/lightspeed/plugins/lightspeed/src/translations/ref.ts b/workspaces/lightspeed/plugins/lightspeed/src/translations/ref.ts index 4d8540b614..cbfc7b7e7c 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/translations/ref.ts +++ b/workspaces/lightspeed/plugins/lightspeed/src/translations/ref.ts @@ -74,7 +74,6 @@ export const lightspeedMessages = { 'conversation.rename.confirm.title': 'Rename chat?', 'conversation.rename.confirm.action': 'Rename', 'conversation.rename.placeholder': 'Chat name', - 'conversation.action.error': 'Error occured: {{error}}', // Permissions 'permission.required.title': 'Missing permissions', @@ -116,8 +115,7 @@ export const lightspeedMessages = { 'Adjust your search query and try again. Check your spelling or try a more general term.', 'chatbox.welcome.greeting': 'Hello, {{userName}}', 'chatbox.welcome.description': 'How can I help you today?', - 'chatbox.message.placeholder': - 'Send a message and optionally upload a JSON, YAML, or TXT file...', + 'chatbox.message.placeholder': 'Enter a prompt for Lightspeed', 'chatbox.fileUpload.failed': 'File upload failed', 'chatbox.fileUpload.infoText': 'Supported file types are: .txt, .yaml, and .json. The maximum file size is 25 MB.', diff --git a/workspaces/lightspeed/plugins/lightspeed/src/utils/lightspeed-chatbox-utils.tsx b/workspaces/lightspeed/plugins/lightspeed/src/utils/lightspeed-chatbox-utils.tsx index ee0a520b3c..e9212b5792 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/utils/lightspeed-chatbox-utils.tsx +++ b/workspaces/lightspeed/plugins/lightspeed/src/utils/lightspeed-chatbox-utils.tsx @@ -38,7 +38,7 @@ export const getFootnoteProps = ( popoverProps: { className: additionalClassName ?? '', } as PopoverProps, - title: t?.('footer.accuracy.popover.title') || 'Verify accuracy', + title: '', description: t?.('footer.accuracy.popover.description') || `While Developer Lightspeed strives for accuracy, there's always a possibility of errors. It's a good practice to verify critical information from reliable sources, especially if it's crucial for decision-making or actions.`, @@ -52,10 +52,6 @@ export const getFootnoteProps = ( label: t?.('footer.accuracy.popover.cta.label') || 'Got it', onClick: () => {}, }, - link: { - label: t?.('footer.accuracy.popover.link.label') || 'Learn more', - url: 'https://www.redhat.com/', - }, }, }); @@ -250,7 +246,16 @@ export const getCategorizeMessages = ( if (pinnedChats.includes(c.conversation_id)) { categorizedMessages[pinnedChatsKey].push({ ...message, - icon: , + icon: ( + + ), }); } else { categorizedMessages[recentKey].push(message); diff --git a/workspaces/lightspeed/plugins/lightspeed/src/utils/toolCallMapper.tsx b/workspaces/lightspeed/plugins/lightspeed/src/utils/toolCallMapper.tsx index d824070e32..7d63629458 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/utils/toolCallMapper.tsx +++ b/workspaces/lightspeed/plugins/lightspeed/src/utils/toolCallMapper.tsx @@ -25,13 +25,14 @@ import { ToolCall } from '../types'; export const mapToPatternFlyToolCall = ( toolCall: ToolCall, t: (key: any, options?: any) => string, + role?: 'user' | 'bot', ): ToolCallProps => { return { titleText: t('toolCall.header' as any, { toolName: toolCall.toolName }), isLoading: toolCall.isLoading, loadingText: t('toolCall.executing'), expandableContent: !toolCall.isLoading ? ( - + ) : undefined, actions: [], }; From c3679423d1114eb29430ee9a6f4d602e8f784f69 Mon Sep 17 00:00:00 2001 From: Debsmita Santra Date: Tue, 27 Jan 2026 19:33:40 +0530 Subject: [PATCH 2/2] persist display mode in user settings --- .../.changeset/eleven-candles-unite.md | 5 + .../plugins/lightspeed/report-alpha.api.md | 6 - .../lightspeed/src/api/LightspeedApiClient.ts | 6 +- .../src/components/LightSpeedChat.tsx | 9 +- .../src/components/LightspeedChatBox.tsx | 27 +- .../components/LightspeedDrawerProvider.tsx | 69 +++-- .../LightspeedDrawerStateExposer.tsx | 12 +- .../src/components/ToolCallContent.tsx | 1 - .../LightspeedDrawerProvider.test.tsx | 282 ++++++++++++++++++ .../LightspeedDrawerStateExposer.test.tsx | 22 +- .../__tests__/useDisplayModeSettings.test.ts | 206 +++++++++++++ .../__tests__/usePinnedChatsSettings.test.ts | 10 +- .../hooks/__tests__/useSortSettings.test.ts | 10 +- .../plugins/lightspeed/src/hooks/index.ts | 1 + .../src/hooks/useDisplayModeSettings.ts | 110 +++++++ .../plugins/lightspeed/src/translations/de.ts | 6 - .../plugins/lightspeed/src/translations/es.ts | 7 - .../plugins/lightspeed/src/translations/fr.ts | 7 - .../plugins/lightspeed/src/translations/it.ts | 7 - .../plugins/lightspeed/src/translations/ja.ts | 6 - .../lightspeed/src/translations/ref.ts | 6 - .../utils/__tests__/reasoningParser.test.ts | 49 +-- .../src/utils/lightspeed-chatbox-utils.tsx | 21 -- .../lightspeed/src/utils/reasoningParser.ts | 54 ++-- 24 files changed, 753 insertions(+), 186 deletions(-) create mode 100644 workspaces/lightspeed/.changeset/eleven-candles-unite.md create mode 100644 workspaces/lightspeed/plugins/lightspeed/src/components/__tests__/LightspeedDrawerProvider.test.tsx create mode 100644 workspaces/lightspeed/plugins/lightspeed/src/hooks/__tests__/useDisplayModeSettings.test.ts create mode 100644 workspaces/lightspeed/plugins/lightspeed/src/hooks/useDisplayModeSettings.ts diff --git a/workspaces/lightspeed/.changeset/eleven-candles-unite.md b/workspaces/lightspeed/.changeset/eleven-candles-unite.md new file mode 100644 index 0000000000..50d2c6a1e6 --- /dev/null +++ b/workspaces/lightspeed/.changeset/eleven-candles-unite.md @@ -0,0 +1,5 @@ +--- +'@red-hat-developer-hub/backstage-plugin-lightspeed': patch +--- + +some ux improvements and persisting the display mode preference diff --git a/workspaces/lightspeed/plugins/lightspeed/report-alpha.api.md b/workspaces/lightspeed/plugins/lightspeed/report-alpha.api.md index 4b30d5afe4..47193ee7b7 100644 --- a/workspaces/lightspeed/plugins/lightspeed/report-alpha.api.md +++ b/workspaces/lightspeed/plugins/lightspeed/report-alpha.api.md @@ -43,15 +43,9 @@ readonly "conversation.delete.confirm.action": string; readonly "conversation.rename.confirm.title": string; readonly "conversation.rename.confirm.action": string; readonly "conversation.rename.placeholder": string; -readonly "conversation.action.error": string; readonly "permission.required.title": string; readonly "permission.required.description": string; readonly "footer.accuracy.label": string; -readonly "footer.accuracy.popover.title": string; -readonly "footer.accuracy.popover.description": string; -readonly "footer.accuracy.popover.image.alt": string; -readonly "footer.accuracy.popover.cta.label": string; -readonly "footer.accuracy.popover.link.label": string; readonly "common.cancel": string; readonly "common.close": string; readonly "common.readMore": string; diff --git a/workspaces/lightspeed/plugins/lightspeed/src/api/LightspeedApiClient.ts b/workspaces/lightspeed/plugins/lightspeed/src/api/LightspeedApiClient.ts index 7974730586..06080af4fe 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/api/LightspeedApiClient.ts +++ b/workspaces/lightspeed/plugins/lightspeed/src/api/LightspeedApiClient.ts @@ -203,7 +203,8 @@ export class LightspeedApiClient implements LightspeedAPI { errorMessage = errorBody.error; } } catch (e) { - // If JSON parsing fails, use the default error message + // eslint-disable-next-line no-console + console.warn(e); } throw new Error(errorMessage); @@ -234,7 +235,8 @@ export class LightspeedApiClient implements LightspeedAPI { errorMessage = errorBody.error; } } catch (e) { - // If JSON parsing fails, use the default error message + // eslint-disable-next-line no-console + console.warn(e); } throw new Error(errorMessage); diff --git a/workspaces/lightspeed/plugins/lightspeed/src/components/LightSpeedChat.tsx b/workspaces/lightspeed/plugins/lightspeed/src/components/LightSpeedChat.tsx index 9468345efd..25773dd339 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/components/LightSpeedChat.tsx +++ b/workspaces/lightspeed/plugins/lightspeed/src/components/LightSpeedChat.tsx @@ -122,11 +122,6 @@ const useStyles = makeStyles(theme => ({ maxWidth: 'unset !important', }, }, - footerPopover: { - '& img': { - maxWidth: '100%', - }, - }, sortDropdown: { padding: 0, margin: 0, @@ -849,9 +844,7 @@ export const LightspeedChat = ({ onAttachRejected={onAttachRejected} placeholder={t('chatbox.message.placeholder')} /> - + } diff --git a/workspaces/lightspeed/plugins/lightspeed/src/components/LightspeedChatBox.tsx b/workspaces/lightspeed/plugins/lightspeed/src/components/LightspeedChatBox.tsx index 2697d87813..0e37ac08c8 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/components/LightspeedChatBox.tsx +++ b/workspaces/lightspeed/plugins/lightspeed/src/components/LightspeedChatBox.tsx @@ -70,6 +70,21 @@ const useStyles = makeStyles(theme => ({ }, }, }, + deepThinking: { + animation: '$deepThinking 1.6s ease-in-out infinite', + }, + + '@keyframes deepThinking': { + '0%': { + opacity: 0.65, + }, + '50%': { + opacity: 1, + }, + '100%': { + opacity: 0.65, + }, + }, })); // Extended message type that includes tool calls @@ -239,22 +254,20 @@ export const LightspeedChatBox = forwardRef( parsedReasoning.isReasoningInProgress || parsedReasoning.hasReasoning ) { - const reasoningContent = - parsedReasoning.reasoning || - (() => { - const reasoningMatch = messageContent.match(/(.*?)$/s); - return reasoningMatch ? reasoningMatch[1].trim() : ''; - })(); + const reasoningContent = parsedReasoning.reasoning; if (reasoningContent) { deepThinking = { cardBodyProps: { id: `deep-thinking-${index}`, style: { whiteSpace: 'pre-line' }, + className: parsedReasoning.isReasoningInProgress + ? classes.deepThinking + : undefined, }, toggleContent: t('reasoning.thinking'), body: reasoningContent, - expandableSectionProps: {}, + isDefaultExpanded: false, }; } } diff --git a/workspaces/lightspeed/plugins/lightspeed/src/components/LightspeedDrawerProvider.tsx b/workspaces/lightspeed/plugins/lightspeed/src/components/LightspeedDrawerProvider.tsx index 534bfdf8ac..82c904e4d8 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/components/LightspeedDrawerProvider.tsx +++ b/workspaces/lightspeed/plugins/lightspeed/src/components/LightspeedDrawerProvider.tsx @@ -27,6 +27,7 @@ import { useLocation, useMatch, useNavigate } from 'react-router-dom'; import { makeStyles } from '@mui/styles'; import { ChatbotDisplayMode, ChatbotModal } from '@patternfly/chatbot'; +import { useBackstageUserIdentity, useDisplayModeSettings } from '../hooks'; import { FileContent } from '../types'; import { LightspeedChatContainer } from './LightspeedChatContainer'; import { LightspeedDrawerContext } from './LightspeedDrawerContext'; @@ -47,10 +48,14 @@ export const LightspeedDrawerProvider = ({ children }: PropsWithChildren) => { const classes = useStyles(); const navigate = useNavigate(); const location = useLocation(); + const user = useBackstageUserIdentity(); + const { + displayMode: persistedDisplayMode, + setDisplayMode: setPersistedDisplayMode, + } = useDisplayModeSettings(user, ChatbotDisplayMode.default); - const [displayModeState, setDisplayModeState] = useState( - ChatbotDisplayMode.default, - ); + const [displayModeState, setDisplayModeState] = + useState(persistedDisplayMode); const [isOpen, setIsOpen] = useState(false); const [drawerWidth, setDrawerWidth] = useState(400); const [currentConversationIdState, setCurrentConversationIdState] = useState< @@ -89,38 +94,59 @@ export const LightspeedDrawerProvider = ({ children }: PropsWithChildren) => { } // Update this to fullscreen only when it is not already in the docked mode setDisplayModeState(prev => { - if (prev === ChatbotDisplayMode.docked) { - return prev; // Don't override docked mode + if ( + prev === ChatbotDisplayMode.docked || + persistedDisplayMode === ChatbotDisplayMode.docked + ) { + return ChatbotDisplayMode.docked; // Don't override docked mode preference } return ChatbotDisplayMode.embedded; }); + // When opening via URL + if (persistedDisplayMode !== ChatbotDisplayMode.embedded) { + setPersistedDisplayMode(ChatbotDisplayMode.embedded); + } setIsOpen(true); } else { - // When leaving lightspeed route, update this only when the current mode is fullscreen - setDisplayModeState(prev => { - if (prev === ChatbotDisplayMode.embedded) { - return ChatbotDisplayMode.default; - } - return prev; - }); + // When leaving lightspeed route, restore the persisted display mode + // If persisted mode is embedded, reset to default + if (persistedDisplayMode === ChatbotDisplayMode.embedded) { + setDisplayModeState(ChatbotDisplayMode.default); + } else { + setDisplayModeState(persistedDisplayMode); + } } - }, [conversationId, isLightspeedRoute]); + }, [ + conversationId, + isLightspeedRoute, + persistedDisplayMode, + setPersistedDisplayMode, + ]); - // Open chatbot in overlay mode + // Open chatbot using the persisted display mode preference const openChatbot = useCallback(() => { openedViaFABRef.current = true; - setDisplayModeState(ChatbotDisplayMode.default); + + const modeToUse = persistedDisplayMode || ChatbotDisplayMode.default; + setDisplayModeState(modeToUse); + + if (modeToUse === ChatbotDisplayMode.embedded) { + const convId = currentConversationIdState; + const path = convId + ? `/lightspeed/conversation/${convId}` + : '/lightspeed'; + navigate(path); + } + setIsOpen(true); - }, []); + }, [persistedDisplayMode, currentConversationIdState, navigate]); - // Close chatbot const closeChatbot = useCallback(() => { // If in embedded mode on the lightspeed route, navigate back if (displayModeState === ChatbotDisplayMode.embedded && isLightspeedRoute) { navigateBackOrGoToCatalog(); } setIsOpen(false); - setDisplayModeState(ChatbotDisplayMode.default); }, [displayModeState, isLightspeedRoute, navigateBackOrGoToCatalog]); const toggleChatbot = useCallback(() => { @@ -135,7 +161,6 @@ export const LightspeedDrawerProvider = ({ children }: PropsWithChildren) => { (id: string | undefined) => { setCurrentConversationIdState(id); - // Update route if in embedded mode if ( displayModeState === ChatbotDisplayMode.embedded && isLightspeedRoute @@ -164,6 +189,11 @@ export const LightspeedDrawerProvider = ({ children }: PropsWithChildren) => { setDisplayModeState(mode); + // Persist the display mode preference when user explicitly changes it, but not for route-based changes to embedded mode + if (mode !== ChatbotDisplayMode.embedded || !isLightspeedRoute) { + setPersistedDisplayMode(mode); + } + // Navigate to fullscreen route with conversation ID if available if (mode === ChatbotDisplayMode.embedded) { const convId = conversationIdParam ?? currentConversationIdState; @@ -185,6 +215,7 @@ export const LightspeedDrawerProvider = ({ children }: PropsWithChildren) => { currentConversationIdState, displayModeState, navigateBackOrGoToCatalog, + setPersistedDisplayMode, ], ); diff --git a/workspaces/lightspeed/plugins/lightspeed/src/components/LightspeedDrawerStateExposer.tsx b/workspaces/lightspeed/plugins/lightspeed/src/components/LightspeedDrawerStateExposer.tsx index 3d50f25a46..a41cc30d88 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/components/LightspeedDrawerStateExposer.tsx +++ b/workspaces/lightspeed/plugins/lightspeed/src/components/LightspeedDrawerStateExposer.tsx @@ -54,10 +54,16 @@ export type DrawerStateExposerProps = { export const LightspeedDrawerStateExposer = ({ onStateChange, }: DrawerStateExposerProps) => { - const { displayMode, drawerWidth, setDrawerWidth, toggleChatbot } = - useLightspeedDrawerContext(); + const { + displayMode, + drawerWidth, + setDrawerWidth, + toggleChatbot, + isChatbotActive, + } = useLightspeedDrawerContext(); - const isDrawerOpen = displayMode === ChatbotDisplayMode.docked; + const isDrawerOpen = + displayMode === ChatbotDisplayMode.docked && isChatbotActive; const toggleChatbotRef = useRef(toggleChatbot); toggleChatbotRef.current = toggleChatbot; diff --git a/workspaces/lightspeed/plugins/lightspeed/src/components/ToolCallContent.tsx b/workspaces/lightspeed/plugins/lightspeed/src/components/ToolCallContent.tsx index ab3eaff986..8b56fd38f8 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/components/ToolCallContent.tsx +++ b/workspaces/lightspeed/plugins/lightspeed/src/components/ToolCallContent.tsx @@ -45,7 +45,6 @@ export const ToolCallContent = ({ role = 'bot', }: ToolCallContentProps) => { const { t } = useTranslation(); - // const [isExpanded, setIsExpanded] = useState(false); const formatExecutionTime = (seconds?: number): string => { if (seconds === undefined) return ''; diff --git a/workspaces/lightspeed/plugins/lightspeed/src/components/__tests__/LightspeedDrawerProvider.test.tsx b/workspaces/lightspeed/plugins/lightspeed/src/components/__tests__/LightspeedDrawerProvider.test.tsx new file mode 100644 index 0000000000..c8e79c6856 --- /dev/null +++ b/workspaces/lightspeed/plugins/lightspeed/src/components/__tests__/LightspeedDrawerProvider.test.tsx @@ -0,0 +1,282 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { MemoryRouter } from 'react-router-dom'; + +import { ChatbotDisplayMode } from '@patternfly/chatbot'; +import { render, screen, waitFor } from '@testing-library/react'; + +import { useLightspeedDrawerContext } from '../../hooks/useLightspeedDrawerContext'; +import { LightspeedDrawerProvider } from '../LightspeedDrawerProvider'; + +const mockSetPersistedDisplayMode = jest.fn(); +const mockUseDisplayModeSettings = jest.fn(() => ({ + displayMode: ChatbotDisplayMode.default, + setDisplayMode: mockSetPersistedDisplayMode, +})); + +const mockUser = 'user:default/test'; +const mockUseBackstageUserIdentity = jest.fn(() => mockUser); + +jest.mock('../../hooks/useDisplayModeSettings', () => ({ + useDisplayModeSettings: () => mockUseDisplayModeSettings(), +})); + +jest.mock('../../hooks/useBackstageUserIdentity', () => ({ + useBackstageUserIdentity: () => mockUseBackstageUserIdentity(), +})); + +// Mock LightspeedChatContainer to avoid complex dependencies +jest.mock('../LightspeedChatContainer', () => ({ + LightspeedChatContainer: () => ( +
Chat Container
+ ), +})); + +describe('LightspeedDrawerProvider', () => { + const TestComponent = () => { + const context = useLightspeedDrawerContext(); + return ( +
+
{context.displayMode}
+
+ {context.isChatbotActive ? 'open' : 'closed'} +
+ + +
+ ); + }; + + beforeEach(() => { + jest.clearAllMocks(); + mockUseDisplayModeSettings.mockReturnValue({ + displayMode: ChatbotDisplayMode.default, + setDisplayMode: mockSetPersistedDisplayMode, + }); + }); + + const renderWithRouter = (initialEntries = ['/']) => { + return render( + + + + + , + ); + }; + + describe('initialization', () => { + it('should initialize with persisted display mode', () => { + mockUseDisplayModeSettings.mockReturnValue({ + displayMode: ChatbotDisplayMode.docked, + setDisplayMode: mockSetPersistedDisplayMode, + }); + + renderWithRouter(); + + expect(screen.getByTestId('display-mode')).toHaveTextContent( + ChatbotDisplayMode.docked, + ); + }); + + it('should initialize with default mode for first-time users', () => { + mockUseDisplayModeSettings.mockReturnValue({ + displayMode: ChatbotDisplayMode.default, + setDisplayMode: mockSetPersistedDisplayMode, + }); + + renderWithRouter(); + + expect(screen.getByTestId('display-mode')).toHaveTextContent( + ChatbotDisplayMode.default, + ); + }); + + it('should start with chatbot closed', () => { + renderWithRouter(); + + expect(screen.getByTestId('is-open')).toHaveTextContent('closed'); + }); + }); + + describe('opening chatbot', () => { + it('should open chatbot in persisted overlay mode', async () => { + mockUseDisplayModeSettings.mockReturnValue({ + displayMode: ChatbotDisplayMode.default, + setDisplayMode: mockSetPersistedDisplayMode, + }); + + renderWithRouter(); + + const toggleButton = screen.getByTestId('toggle-button'); + toggleButton.click(); + + await waitFor(() => { + expect(screen.getByTestId('is-open')).toHaveTextContent('open'); + }); + + expect(screen.getByTestId('display-mode')).toHaveTextContent( + ChatbotDisplayMode.default, + ); + }); + + it('should open chatbot in persisted docked mode', async () => { + mockUseDisplayModeSettings.mockReturnValue({ + displayMode: ChatbotDisplayMode.docked, + setDisplayMode: mockSetPersistedDisplayMode, + }); + + renderWithRouter(); + + const toggleButton = screen.getByTestId('toggle-button'); + toggleButton.click(); + + await waitFor(() => { + expect(screen.getByTestId('is-open')).toHaveTextContent('open'); + }); + + expect(screen.getByTestId('display-mode')).toHaveTextContent( + ChatbotDisplayMode.docked, + ); + }); + }); + + describe('closing chatbot', () => { + it('should close chatbot without resetting display mode', async () => { + mockUseDisplayModeSettings.mockReturnValue({ + displayMode: ChatbotDisplayMode.docked, + setDisplayMode: mockSetPersistedDisplayMode, + }); + + renderWithRouter(); + + const toggleButton = screen.getByTestId('toggle-button'); + toggleButton.click(); + + await waitFor(() => { + expect(screen.getByTestId('is-open')).toHaveTextContent('open'); + }); + + toggleButton.click(); + + await waitFor(() => { + expect(screen.getByTestId('is-open')).toHaveTextContent('closed'); + }); + + expect(screen.getByTestId('display-mode')).toHaveTextContent( + ChatbotDisplayMode.docked, + ); + }); + }); + + describe('setting display mode', () => { + it('should persist display mode when user changes it', async () => { + renderWithRouter(); + + const setModeButton = screen.getByTestId('set-mode-button'); + setModeButton.click(); + + await waitFor(() => { + expect(mockSetPersistedDisplayMode).toHaveBeenCalledWith( + ChatbotDisplayMode.docked, + ); + }); + }); + + it('should update display mode in context', async () => { + renderWithRouter(); + + const setModeButton = screen.getByTestId('set-mode-button'); + setModeButton.click(); + + await waitFor(() => { + expect(screen.getByTestId('display-mode')).toHaveTextContent( + ChatbotDisplayMode.docked, + ); + }); + }); + }); + + describe('lightspeed route handling', () => { + it('should set embedded mode when on /lightspeed route', async () => { + mockUseDisplayModeSettings.mockReturnValue({ + displayMode: ChatbotDisplayMode.default, + setDisplayMode: mockSetPersistedDisplayMode, + }); + + renderWithRouter(['/lightspeed']); + + await waitFor(() => { + expect(screen.getByTestId('display-mode')).toHaveTextContent( + ChatbotDisplayMode.embedded, + ); + expect(screen.getByTestId('is-open')).toHaveTextContent('open'); + }); + }); + + it('should preserve docked mode when on /lightspeed route if persisted', async () => { + mockUseDisplayModeSettings.mockReturnValue({ + displayMode: ChatbotDisplayMode.docked, + setDisplayMode: mockSetPersistedDisplayMode, + }); + + renderWithRouter(['/lightspeed']); + + await waitFor(() => { + expect(screen.getByTestId('display-mode')).toHaveTextContent( + ChatbotDisplayMode.docked, + ); + }); + }); + + it('should restore persisted mode when leaving /lightspeed route', async () => { + mockUseDisplayModeSettings.mockReturnValue({ + displayMode: ChatbotDisplayMode.docked, + setDisplayMode: mockSetPersistedDisplayMode, + }); + + const { rerender } = renderWithRouter(['/lightspeed']); + + await waitFor(() => { + expect(screen.getByTestId('display-mode')).toHaveTextContent( + ChatbotDisplayMode.docked, + ); + }); + + rerender( + + + + + , + ); + + await waitFor(() => { + expect(screen.getByTestId('display-mode')).toHaveTextContent( + ChatbotDisplayMode.docked, + ); + }); + }); + }); +}); diff --git a/workspaces/lightspeed/plugins/lightspeed/src/components/__tests__/LightspeedDrawerStateExposer.test.tsx b/workspaces/lightspeed/plugins/lightspeed/src/components/__tests__/LightspeedDrawerStateExposer.test.tsx index 08c0bbc182..cd28acf4a9 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/components/__tests__/LightspeedDrawerStateExposer.test.tsx +++ b/workspaces/lightspeed/plugins/lightspeed/src/components/__tests__/LightspeedDrawerStateExposer.test.tsx @@ -72,10 +72,11 @@ describe('LightspeedDrawerStateExposer', () => { }); }); - it('should set isDrawerOpen to true when displayMode is docked', async () => { + it('should set isDrawerOpen to true when displayMode is docked AND chatbot is active', async () => { renderWithContext( createContextValue({ displayMode: ChatbotDisplayMode.docked, + isChatbotActive: true, }), ); @@ -88,6 +89,23 @@ describe('LightspeedDrawerStateExposer', () => { }); }); + it('should set isDrawerOpen to false when displayMode is docked but chatbot is not active', async () => { + renderWithContext( + createContextValue({ + displayMode: ChatbotDisplayMode.docked, + isChatbotActive: false, + }), + ); + + await waitFor(() => { + expect(mockOnStateChange).toHaveBeenCalledWith( + expect.objectContaining({ + isDrawerOpen: false, + }), + ); + }); + }); + it('should set isDrawerOpen to false when displayMode is overlay', async () => { renderWithContext( createContextValue({ @@ -170,6 +188,7 @@ describe('LightspeedDrawerStateExposer', () => { const { rerender } = renderWithContext( createContextValue({ displayMode: ChatbotDisplayMode.default, + isChatbotActive: false, }), ); @@ -185,6 +204,7 @@ describe('LightspeedDrawerStateExposer', () => { diff --git a/workspaces/lightspeed/plugins/lightspeed/src/hooks/__tests__/useDisplayModeSettings.test.ts b/workspaces/lightspeed/plugins/lightspeed/src/hooks/__tests__/useDisplayModeSettings.test.ts new file mode 100644 index 0000000000..ee0f30386b --- /dev/null +++ b/workspaces/lightspeed/plugins/lightspeed/src/hooks/__tests__/useDisplayModeSettings.test.ts @@ -0,0 +1,206 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { createElement, ReactNode } from 'react'; + +import { StorageApi, storageApiRef } from '@backstage/core-plugin-api'; +import { mockApis, TestApiProvider } from '@backstage/test-utils'; + +import { ChatbotDisplayMode } from '@patternfly/chatbot'; +import { act, renderHook, waitFor } from '@testing-library/react'; + +import { useDisplayModeSettings } from '../useDisplayModeSettings'; + +describe('useDisplayModeSettings', () => { + // Use a non-guest user for most tests (guest users don't persist) + const mockUser = 'user:default/john'; + const guestUser = 'user:default/guest'; + let mockStorageApi: StorageApi; + + const createWrapper = (storageApi: StorageApi) => { + return ({ children }: { children: ReactNode }) => + createElement(TestApiProvider, { + apis: [[storageApiRef, storageApi]], + children, + }); + }; + + beforeEach(() => { + mockStorageApi = mockApis.storage(); + }); + + describe('initialization', () => { + it('should initialize with default value when user is undefined', () => { + const { result } = renderHook( + () => useDisplayModeSettings(undefined, ChatbotDisplayMode.default), + { + wrapper: createWrapper(mockStorageApi), + }, + ); + + expect(result.current.displayMode).toBe(ChatbotDisplayMode.default); + }); + + it('should initialize with default value for a new user', () => { + const { result } = renderHook( + () => useDisplayModeSettings(mockUser, ChatbotDisplayMode.default), + { + wrapper: createWrapper(mockStorageApi), + }, + ); + + expect(result.current.displayMode).toBe(ChatbotDisplayMode.default); + }); + + it('should initialize with default for guest user without loading from storage', async () => { + const { result } = renderHook( + () => useDisplayModeSettings(guestUser, ChatbotDisplayMode.default), + { + wrapper: createWrapper(mockStorageApi), + }, + ); + + // Guest users should always get default values + expect(result.current.displayMode).toBe(ChatbotDisplayMode.default); + }); + + it('should load persisted value from storage', async () => { + // Set a value in storage first + const bucket = mockStorageApi.forBucket('lightspeed'); + await bucket.set('displayMode', ChatbotDisplayMode.docked); + + const { result } = renderHook( + () => useDisplayModeSettings(mockUser, ChatbotDisplayMode.default), + { + wrapper: createWrapper(mockStorageApi), + }, + ); + + await waitFor(() => { + expect(result.current.displayMode).toBe(ChatbotDisplayMode.docked); + }); + }); + }); + + describe('setDisplayMode', () => { + it('should change display mode to docked', async () => { + const { result } = renderHook( + () => useDisplayModeSettings(mockUser, ChatbotDisplayMode.default), + { + wrapper: createWrapper(mockStorageApi), + }, + ); + + expect(result.current.displayMode).toBe(ChatbotDisplayMode.default); + + act(() => { + result.current.setDisplayMode(ChatbotDisplayMode.docked); + }); + + await waitFor(() => { + expect(result.current.displayMode).toBe(ChatbotDisplayMode.docked); + }); + + // Verify it was persisted + const bucket = mockStorageApi.forBucket('lightspeed'); + const snapshot = bucket.snapshot('displayMode'); + expect(snapshot.value).toBe(ChatbotDisplayMode.docked); + }); + + it('should change display mode to embedded', async () => { + const { result } = renderHook( + () => useDisplayModeSettings(mockUser, ChatbotDisplayMode.default), + { + wrapper: createWrapper(mockStorageApi), + }, + ); + + act(() => { + result.current.setDisplayMode(ChatbotDisplayMode.embedded); + }); + + await waitFor(() => { + expect(result.current.displayMode).toBe(ChatbotDisplayMode.embedded); + }); + + // Verify it was persisted + const bucket = mockStorageApi.forBucket('lightspeed'); + const snapshot = bucket.snapshot('displayMode'); + expect(snapshot.value).toBe(ChatbotDisplayMode.embedded); + }); + + it('should not persist for guest users', async () => { + const { result } = renderHook( + () => useDisplayModeSettings(guestUser, ChatbotDisplayMode.default), + { + wrapper: createWrapper(mockStorageApi), + }, + ); + + act(() => { + result.current.setDisplayMode(ChatbotDisplayMode.docked); + }); + + await waitFor(() => { + // State should change locally + expect(result.current.displayMode).toBe(ChatbotDisplayMode.docked); + }); + + // But should not be persisted + const bucket = mockStorageApi.forBucket('lightspeed'); + const snapshot = bucket.snapshot('displayMode'); + expect(snapshot.value).toBeUndefined(); + }); + + it('should not change mode when user is undefined', async () => { + const { result } = renderHook( + () => useDisplayModeSettings(undefined, ChatbotDisplayMode.default), + { + wrapper: createWrapper(mockStorageApi), + }, + ); + + const initialMode = result.current.displayMode; + + act(() => { + result.current.setDisplayMode(ChatbotDisplayMode.docked); + }); + + // Should remain unchanged + expect(result.current.displayMode).toBe(initialMode); + }); + }); + + describe('cross-tab synchronization', () => { + it('should update when storage changes (simulating cross-tab sync)', async () => { + const { result } = renderHook( + () => useDisplayModeSettings(mockUser, ChatbotDisplayMode.default), + { + wrapper: createWrapper(mockStorageApi), + }, + ); + + expect(result.current.displayMode).toBe(ChatbotDisplayMode.default); + + // Simulate storage change from another tab + const bucket = mockStorageApi.forBucket('lightspeed'); + await bucket.set('displayMode', ChatbotDisplayMode.docked); + + await waitFor(() => { + expect(result.current.displayMode).toBe(ChatbotDisplayMode.docked); + }); + }); + }); +}); diff --git a/workspaces/lightspeed/plugins/lightspeed/src/hooks/__tests__/usePinnedChatsSettings.test.ts b/workspaces/lightspeed/plugins/lightspeed/src/hooks/__tests__/usePinnedChatsSettings.test.ts index a862c1b8bd..b445d20350 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/hooks/__tests__/usePinnedChatsSettings.test.ts +++ b/workspaces/lightspeed/plugins/lightspeed/src/hooks/__tests__/usePinnedChatsSettings.test.ts @@ -15,8 +15,8 @@ */ import { createElement, ReactNode } from 'react'; -import { storageApiRef } from '@backstage/core-plugin-api'; -import { MockStorageApi, TestApiProvider } from '@backstage/test-utils'; +import { StorageApi, storageApiRef } from '@backstage/core-plugin-api'; +import { mockApis, TestApiProvider } from '@backstage/test-utils'; import { act, renderHook, waitFor } from '@testing-library/react'; @@ -26,9 +26,9 @@ describe('usePinnedChatsSettings', () => { // Use a non-guest user for most tests (guest users don't persist) const mockUser = 'user:default/john'; const guestUser = 'user:default/guest'; - let mockStorageApi: MockStorageApi; + let mockStorageApi: StorageApi; - const createWrapper = (storageApi: MockStorageApi) => { + const createWrapper = (storageApi: StorageApi) => { return ({ children }: { children: ReactNode }) => createElement(TestApiProvider, { apis: [[storageApiRef, storageApi]], @@ -37,7 +37,7 @@ describe('usePinnedChatsSettings', () => { }; beforeEach(() => { - mockStorageApi = MockStorageApi.create(); + mockStorageApi = mockApis.storage(); }); describe('initialization', () => { diff --git a/workspaces/lightspeed/plugins/lightspeed/src/hooks/__tests__/useSortSettings.test.ts b/workspaces/lightspeed/plugins/lightspeed/src/hooks/__tests__/useSortSettings.test.ts index 9710e6def8..7a552ba154 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/hooks/__tests__/useSortSettings.test.ts +++ b/workspaces/lightspeed/plugins/lightspeed/src/hooks/__tests__/useSortSettings.test.ts @@ -15,8 +15,8 @@ */ import { createElement, ReactNode } from 'react'; -import { storageApiRef } from '@backstage/core-plugin-api'; -import { MockStorageApi, TestApiProvider } from '@backstage/test-utils'; +import { StorageApi, storageApiRef } from '@backstage/core-plugin-api'; +import { mockApis, TestApiProvider } from '@backstage/test-utils'; import { act, renderHook, waitFor } from '@testing-library/react'; @@ -26,9 +26,9 @@ describe('useSortSettings', () => { // Use a non-guest user for most tests (guest users don't persist) const mockUser = 'user:default/john'; const guestUser = 'user:default/guest'; - let mockStorageApi: MockStorageApi; + let mockStorageApi: StorageApi; - const createWrapper = (storageApi: MockStorageApi) => { + const createWrapper = (storageApi: StorageApi) => { return ({ children }: { children: ReactNode }) => createElement(TestApiProvider, { apis: [[storageApiRef, storageApi]], @@ -37,7 +37,7 @@ describe('useSortSettings', () => { }; beforeEach(() => { - mockStorageApi = MockStorageApi.create(); + mockStorageApi = mockApis.storage(); }); describe('initialization', () => { diff --git a/workspaces/lightspeed/plugins/lightspeed/src/hooks/index.ts b/workspaces/lightspeed/plugins/lightspeed/src/hooks/index.ts index 4bbe4124cd..bdeaa1408f 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/hooks/index.ts +++ b/workspaces/lightspeed/plugins/lightspeed/src/hooks/index.ts @@ -23,6 +23,7 @@ export * from './useIsMobile'; export * from './useLastOpenedConversation'; export * from './useLightspeedDeletePermission'; export * from './useLightspeedViewPermission'; +export * from './useDisplayModeSettings'; export * from './usePinnedChatsSettings'; export * from './useSortSettings'; export * from './useTranslation'; diff --git a/workspaces/lightspeed/plugins/lightspeed/src/hooks/useDisplayModeSettings.ts b/workspaces/lightspeed/plugins/lightspeed/src/hooks/useDisplayModeSettings.ts new file mode 100644 index 0000000000..fb223f7a82 --- /dev/null +++ b/workspaces/lightspeed/plugins/lightspeed/src/hooks/useDisplayModeSettings.ts @@ -0,0 +1,110 @@ +/* + * Copyright Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { useCallback, useEffect, useState } from 'react'; + +import { storageApiRef, useApi } from '@backstage/core-plugin-api'; + +import { ChatbotDisplayMode } from '@patternfly/chatbot'; + +import { isGuestUser } from '../utils/user-utils'; + +const BUCKET_NAME = 'lightspeed'; +const DISPLAY_MODE_KEY = 'displayMode'; + +type UseDisplayModeSettingsReturn = { + displayMode: ChatbotDisplayMode; + setDisplayMode: (mode: ChatbotDisplayMode) => void; +}; + +/** + * Hook to manage display mode settings with persistence using Backstage StorageApi. + * + * @param user - The user entity ref (e.g., "user:default/john") + * @param defaultMode - The default display mode to use if no preference is stored + * @returns Object containing display mode state and management function + */ +export const useDisplayModeSettings = ( + user: string | undefined, + defaultMode: ChatbotDisplayMode = ChatbotDisplayMode.default, +): UseDisplayModeSettingsReturn => { + const storageApi = useApi(storageApiRef); + const bucket = storageApi.forBucket(BUCKET_NAME); + + const [displayModeState, setDisplayModeState] = + useState(defaultMode); + + const shouldPersist = !isGuestUser(user); + + useEffect(() => { + if (!user) { + setDisplayModeState(defaultMode); + return undefined; + } + + if (isGuestUser(user)) { + setDisplayModeState(defaultMode); + return undefined; + } + + try { + const modeSnapshot = + bucket.snapshot(DISPLAY_MODE_KEY); + setDisplayModeState(modeSnapshot.value ?? defaultMode); + } catch (error) { + // eslint-disable-next-line no-console + console.error('Error reading display mode from storage:', error); + } + + const modeSubscription = bucket + .observe$(DISPLAY_MODE_KEY) + .subscribe({ + next: snapshot => { + setDisplayModeState(snapshot.value ?? defaultMode); + }, + error: error => { + // eslint-disable-next-line no-console + console.error('Error observing displayMode:', error); + }, + }); + + return () => { + modeSubscription.unsubscribe(); + }; + }, [bucket, user, defaultMode]); + + const setDisplayMode = useCallback( + (mode: ChatbotDisplayMode) => { + if (!user) return; + + setDisplayModeState(mode); + + if (shouldPersist) { + try { + bucket.set(DISPLAY_MODE_KEY, mode); + } catch (error) { + // eslint-disable-next-line no-console + console.error('Error saving display mode:', error); + } + } + }, + [bucket, user, shouldPersist], + ); + + return { + displayMode: displayModeState, + setDisplayMode, + }; +}; diff --git a/workspaces/lightspeed/plugins/lightspeed/src/translations/de.ts b/workspaces/lightspeed/plugins/lightspeed/src/translations/de.ts index db322d5538..a00f1de4c6 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/translations/de.ts +++ b/workspaces/lightspeed/plugins/lightspeed/src/translations/de.ts @@ -92,12 +92,6 @@ const lightspeedTranslationDe = createTranslationMessages({ // Footer and feedback 'footer.accuracy.label': 'Überprüfen Sie KI-generierte Inhalte immer vor der Verwendung.', - 'footer.accuracy.popover.title': 'Genauigkeit überprüfen', - 'footer.accuracy.popover.description': - 'Obwohl Developer Lightspeed sich um Genauigkeit bemüht, besteht immer die Möglichkeit von Fehlern. Es ist eine gute Praxis, kritische Informationen aus zuverlässigen Quellen zu überprüfen, besonders wenn sie für Entscheidungsfindung oder Handlungen entscheidend sind.', - 'footer.accuracy.popover.image.alt': 'Beispielbild für Fußnoten-Popover', - 'footer.accuracy.popover.cta.label': 'Verstanden', - 'footer.accuracy.popover.link.label': 'Mehr erfahren', // Common actions 'common.cancel': 'Abbrechen', diff --git a/workspaces/lightspeed/plugins/lightspeed/src/translations/es.ts b/workspaces/lightspeed/plugins/lightspeed/src/translations/es.ts index 04513634cb..6148edc15c 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/translations/es.ts +++ b/workspaces/lightspeed/plugins/lightspeed/src/translations/es.ts @@ -95,13 +95,6 @@ const lightspeedTranslationEs = createTranslationMessages({ // Footer and feedback 'footer.accuracy.label': 'Siempre revisa el contenido generado por IA antes de usarlo.', - 'footer.accuracy.popover.title': 'Verificar precisión', - 'footer.accuracy.popover.description': - 'Si bien Developer Lightspeed se esfuerza por la precisión, siempre existe la posibilidad de errores. Es una buena práctica verificar información crítica de fuentes confiables, especialmente si es crucial para la toma de decisiones o acciones.', - 'footer.accuracy.popover.image.alt': - 'Imagen de ejemplo para el popover de nota al pie', - 'footer.accuracy.popover.cta.label': 'Entendido', - 'footer.accuracy.popover.link.label': 'Aprende más', // Common actions 'common.cancel': 'Cancelar', diff --git a/workspaces/lightspeed/plugins/lightspeed/src/translations/fr.ts b/workspaces/lightspeed/plugins/lightspeed/src/translations/fr.ts index 39c75fa1f5..624b3af0c2 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/translations/fr.ts +++ b/workspaces/lightspeed/plugins/lightspeed/src/translations/fr.ts @@ -80,13 +80,6 @@ const lightspeedTranslationFr = createTranslationMessages({ 'Cette fonctionnalité utilise la technologie AI. Ne pas inclure d’informations personnelles ou toute autre information sensible dans vos entrées de données. Des interactions pourront être utilisées pour améliorer les produits ou services de Red Hat.', 'footer.accuracy.label': 'Toujours vérifier le contenu AI généré avant utilisation.', - 'footer.accuracy.popover.title': 'Vérifier l’exactitude', - 'footer.accuracy.popover.description': - 'Bien que Developer Lightspeed soit orienté sur l’exactitude, il y a toujours possibilité d’erreurs. Il est toujours bon de vérifier les informations critiques à partir de sources de confiance, surtout si c’est crucial pour prendre des décisions ou entreprendre des actions.', - 'footer.accuracy.popover.image.alt': - 'Exemple d’image de note de bas de page popover', - 'footer.accuracy.popover.cta.label': "J'ai compris!", - 'footer.accuracy.popover.link.label': 'En savoir plus', 'common.cancel': 'Annuler', 'common.close': 'Fermer', 'common.readMore': 'En savoir plus', diff --git a/workspaces/lightspeed/plugins/lightspeed/src/translations/it.ts b/workspaces/lightspeed/plugins/lightspeed/src/translations/it.ts index e401c80b16..bac84607b4 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/translations/it.ts +++ b/workspaces/lightspeed/plugins/lightspeed/src/translations/it.ts @@ -82,13 +82,6 @@ const lightspeedTranslationIt = createTranslationMessages({ 'Questa funzione utilizza una tecnologia AI. Non includere nei dati immessi informazioni personali o altre informazioni sensibili. Le interazioni possono essere utilizzate per migliorare i prodotti o i servizi Red Hat.', 'footer.accuracy.label': "Esaminare sempre i contenuti generati dall'intelligenza artificiale prima di utilizzarli.", - 'footer.accuracy.popover.title': "Verificare l'accuratezza", - 'footer.accuracy.popover.description': - "Nonostante l'impegno di Developer Lightspeed a garantire la massima precisione, esiste sempre un margine di errore. È buona norma verificare le informazioni critiche confrontandole con fonti affidabili, soprattutto se sono essenziali per prendere decisioni o intraprendere azioni.", - 'footer.accuracy.popover.image.alt': - 'Immagine di esempio per il popover del piè di pagina', - 'footer.accuracy.popover.cta.label': 'Ho capito', - 'footer.accuracy.popover.link.label': 'Per saperne di più', 'common.cancel': 'Cancella', 'common.close': 'Chiudi', 'common.readMore': 'Per saperne di più', diff --git a/workspaces/lightspeed/plugins/lightspeed/src/translations/ja.ts b/workspaces/lightspeed/plugins/lightspeed/src/translations/ja.ts index f75a6aa4d7..ae5358ba5f 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/translations/ja.ts +++ b/workspaces/lightspeed/plugins/lightspeed/src/translations/ja.ts @@ -80,12 +80,6 @@ const lightspeedTranslationJa = createTranslationMessages({ 'この機能は AI テクノロジーを使用します。入力内容に個人情報やその他の機密情報を含めないでください。やり取りの内容は、Red Hat の製品やサービスを改善するために使用される場合があります。', 'footer.accuracy.label': 'AI によって生成されたコンテンツは、使用する前に必ず確認してください。', - 'footer.accuracy.popover.title': '正確性の確認', - 'footer.accuracy.popover.description': - 'Developer Lightspeed は正確性を期すよう努めておりますが、誤りが生じる可能性は常にあります。特に意思決定や行動に関わる重要な情報については、信頼できる情報源でその情報を確認することを推奨します。', - 'footer.accuracy.popover.image.alt': '脚注ポップオーバーのサンプル画像', - 'footer.accuracy.popover.cta.label': '了解しました', - 'footer.accuracy.popover.link.label': '詳細', 'common.cancel': 'キャンセル', 'common.close': '閉じる', 'common.readMore': 'さらに表示する', diff --git a/workspaces/lightspeed/plugins/lightspeed/src/translations/ref.ts b/workspaces/lightspeed/plugins/lightspeed/src/translations/ref.ts index cbfc7b7e7c..4dc5d19087 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/translations/ref.ts +++ b/workspaces/lightspeed/plugins/lightspeed/src/translations/ref.ts @@ -88,12 +88,6 @@ export const lightspeedMessages = { // Footer and feedback 'footer.accuracy.label': 'Always review AI generated content prior to use.', - 'footer.accuracy.popover.title': 'Verify accuracy', - 'footer.accuracy.popover.description': - "While Developer Lightspeed strives for accuracy, there's always a possibility of errors. It's a good practice to verify critical information from reliable sources, especially if it's crucial for decision-making or actions.", - 'footer.accuracy.popover.image.alt': 'Example image for footnote popover', - 'footer.accuracy.popover.cta.label': 'Got it', - 'footer.accuracy.popover.link.label': 'Learn more', // Common actions 'common.cancel': 'Cancel', diff --git a/workspaces/lightspeed/plugins/lightspeed/src/utils/__tests__/reasoningParser.test.ts b/workspaces/lightspeed/plugins/lightspeed/src/utils/__tests__/reasoningParser.test.ts index 1f1d9473e6..353f7c611b 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/utils/__tests__/reasoningParser.test.ts +++ b/workspaces/lightspeed/plugins/lightspeed/src/utils/__tests__/reasoningParser.test.ts @@ -14,31 +14,9 @@ * limitations under the License. */ -import { isReasoningInProgress, parseReasoning } from '../reasoningParser'; +import { parseReasoning } from '../reasoningParser'; describe('reasoningParser', () => { - describe('isReasoningInProgress', () => { - it('should return false for empty content', () => { - expect(isReasoningInProgress('')).toBe(false); - }); - - it('should return false when no reasoning tags are present', () => { - expect(isReasoningInProgress('This is regular content')).toBe(false); - }); - - it('should return false when both opening and closing tags are present', () => { - expect( - isReasoningInProgress('Some reasoningMain content'), - ).toBe(false); - }); - - it('should return true when only opening tag is present', () => { - expect(isReasoningInProgress('Some reasoning in progress')).toBe( - true, - ); - }); - }); - describe('parseReasoning', () => { describe('empty or null content', () => { it('should return empty result for empty string', () => { @@ -98,7 +76,9 @@ describe('reasoningParser', () => { 'First reasoning blockSome content Second reasoning blockMore content'; const result = parseReasoning(content); // Should extract the first complete block - expect(result.reasoning).toBe('First reasoning block'); + expect(result.reasoning).toBe( + 'First reasoning block\n\nSecond reasoning block', + ); expect(result.mainContent).toContain('Some content'); expect(result.hasReasoning).toBe(true); }); @@ -136,9 +116,9 @@ describe('reasoningParser', () => { it('should handle empty reasoning block', () => { const content = 'Main content'; const result = parseReasoning(content); - expect(result.reasoning).toBe(''); + expect(result.reasoning).toBe(null); expect(result.mainContent).toBe('Main content'); - expect(result.hasReasoning).toBe(true); + expect(result.hasReasoning).toBe(false); }); }); @@ -146,16 +126,16 @@ describe('reasoningParser', () => { it('should detect reasoning in progress when only opening tag exists', () => { const content = 'Reasoning that is still streaming'; const result = parseReasoning(content); - expect(result.reasoning).toBe(null); + expect(result.reasoning).toBe('Reasoning that is still streaming'); expect(result.mainContent).toBe(''); - expect(result.hasReasoning).toBe(false); + expect(result.hasReasoning).toBe(true); expect(result.isReasoningInProgress).toBe(true); }); it('should handle reasoning in progress with content before tag', () => { const content = 'Some content Reasoning in progress'; const result = parseReasoning(content); - expect(result.reasoning).toBe(null); + expect(result.reasoning).toBe('Reasoning in progress'); expect(result.mainContent).toBe(''); expect(result.isReasoningInProgress).toBe(true); }); @@ -173,9 +153,7 @@ describe('reasoningParser', () => { const content = 'First reasoningMain content Second reasoningMore content'; const result = parseReasoning(content); - // Should extract the first complete block - expect(result.reasoning).toBe('First reasoning'); - // Should remove the first block but may leave second if not handled + expect(result.reasoning).toBe('First reasoning\n\nSecond reasoning'); expect(result.mainContent).toContain('Main content'); expect(result.hasReasoning).toBe(true); }); @@ -184,10 +162,10 @@ describe('reasoningParser', () => { const content = 'Complete reasoningMain content In progress'; const result = parseReasoning(content); - expect(result.reasoning).toBe('Complete reasoning'); - expect(result.mainContent).toContain('Main content'); + expect(result.reasoning).toBe('Complete reasoning\n\nIn progress'); + expect(result.mainContent).toContain(''); expect(result.hasReasoning).toBe(true); - expect(result.isReasoningInProgress).toBe(false); + expect(result.isReasoningInProgress).toBe(true); }); }); @@ -195,7 +173,6 @@ describe('reasoningParser', () => { it('should handle content with similar but not matching tags', () => { const content = 'Not a tagRegular content'; const result = parseReasoning(content); - // Should match since pattern uses as closing tag expect(result.hasReasoning).toBe(true); expect(result.reasoning).toBe('Not a tag'); }); diff --git a/workspaces/lightspeed/plugins/lightspeed/src/utils/lightspeed-chatbox-utils.tsx b/workspaces/lightspeed/plugins/lightspeed/src/utils/lightspeed-chatbox-utils.tsx index e9212b5792..aeef450a89 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/utils/lightspeed-chatbox-utils.tsx +++ b/workspaces/lightspeed/plugins/lightspeed/src/utils/lightspeed-chatbox-utils.tsx @@ -15,7 +15,6 @@ */ import PushPinIcon from '@mui/icons-material/PushPin'; import { Conversation, SourcesCardProps } from '@patternfly/chatbot'; -import { PopoverProps } from '@patternfly/react-core'; import { BaseMessage, @@ -28,31 +27,11 @@ import { } from '../types'; export const getFootnoteProps = ( - additionalClassName: string, t?: (key: string, params?: any) => string, ) => ({ label: t?.('footer.accuracy.label') || 'Always review AI generated content prior to use.', - popover: { - popoverProps: { - className: additionalClassName ?? '', - } as PopoverProps, - title: '', - description: - t?.('footer.accuracy.popover.description') || - `While Developer Lightspeed strives for accuracy, there's always a possibility of errors. It's a good practice to verify critical information from reliable sources, especially if it's crucial for decision-making or actions.`, - bannerImage: { - src: 'https://cdn.dribbble.com/userupload/10651749/file/original-8a07b8e39d9e8bf002358c66fce1223e.gif', - alt: - t?.('footer.accuracy.popover.image.alt') || - 'Example image for footnote popover', - }, - cta: { - label: t?.('footer.accuracy.popover.cta.label') || 'Got it', - onClick: () => {}, - }, - }, }); export const getTimestampVariablesString = (v: number) => { diff --git a/workspaces/lightspeed/plugins/lightspeed/src/utils/reasoningParser.ts b/workspaces/lightspeed/plugins/lightspeed/src/utils/reasoningParser.ts index 8990bbd53c..c01a0a3e49 100644 --- a/workspaces/lightspeed/plugins/lightspeed/src/utils/reasoningParser.ts +++ b/workspaces/lightspeed/plugins/lightspeed/src/utils/reasoningParser.ts @@ -29,15 +29,6 @@ export interface ParsedReasoning { isReasoningInProgress: boolean; } -export const isReasoningInProgress = (content: string): boolean => { - if (!content) return false; - - const hasOpeningTag = content.includes(''); - const hasClosingTag = content.includes(''); - - return hasOpeningTag && !hasClosingTag; -}; - export const parseReasoning = (content: string): ParsedReasoning => { if (!content) { return { @@ -48,36 +39,33 @@ export const parseReasoning = (content: string): ParsedReasoning => { }; } - const reasoningInProgress = isReasoningInProgress(content); + const lastOpenIndex = content.lastIndexOf(''); + const lastCloseIndex = content.lastIndexOf(''); - const reasoningPattern = /(.*?)<\/think>/s; - const match = content.match(reasoningPattern); + const isReasoningInProgress = + lastOpenIndex !== -1 && lastOpenIndex > lastCloseIndex; - if (match) { - const reasoning = (match[1] || '').trim(); - const mainContent = content.replace(reasoningPattern, '').trim(); + const reasoningPattern = /(.*?)<\/think>/gs; + const matches = Array.from(content.matchAll(reasoningPattern)); - return { - reasoning, - mainContent, - hasReasoning: true, - isReasoningInProgress: false, - }; - } + const extractedReasoning = matches + .map(m => (m[1] || '').trim()) + .filter(Boolean); - if (reasoningInProgress) { - return { - reasoning: null, - mainContent: '', - hasReasoning: false, - isReasoningInProgress: true, - }; + if (isReasoningInProgress && lastOpenIndex !== -1) { + const partial = content.substring(lastOpenIndex + ''.length).trim(); + if (partial) { + extractedReasoning.push(partial); + } } + const mainContent = content.replace(reasoningPattern, '').trim(); + return { - reasoning: null, - mainContent: content, - hasReasoning: false, - isReasoningInProgress: false, + reasoning: + extractedReasoning.length > 0 ? extractedReasoning.join('\n\n') : null, + mainContent: isReasoningInProgress ? '' : mainContent, + hasReasoning: extractedReasoning.length > 0, + isReasoningInProgress, }; };