From 337dfd63905209e37667d53649af03a18957f183 Mon Sep 17 00:00:00 2001 From: Sihan He Date: Wed, 7 Aug 2024 18:02:28 +0800 Subject: [PATCH 01/29] Add message action module Signed-off-by: Sihan He --- .../generate_popover_body.tsx | 73 ++++++++---- public/tabs/chat/messages/message_action.tsx | 110 +++++++++++++++++ public/tabs/chat/messages/message_bubble.tsx | 111 ++++-------------- 3 files changed, 188 insertions(+), 106 deletions(-) create mode 100644 public/tabs/chat/messages/message_action.tsx diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index 2a9db358..7d766a65 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -4,6 +4,7 @@ */ import React, { useState } from 'react'; +import cx from 'classnames'; import { i18n } from '@osd/i18n'; import { EuiButton, @@ -13,7 +14,14 @@ import { EuiPanel, EuiSpacer, EuiText, + EuiAvatar, + EuiButtonIcon, + EuiCopy, + EuiLoadingContent, + EuiLoadingSpinner, } from '@elastic/eui'; +import { MessageActions } from '../../tabs/chat/messages/message_action'; + import { IncontextInsight as IncontextInsightInput } from '../../types'; import { getConfigSchema, getIncontextInsightRegistry, getNotifications } from '../../services'; import { HttpSetup } from '../../../../../src/core/public'; @@ -97,30 +105,55 @@ export const GeneratePopoverBody: React.FC<{ {summary} - {getConfigSchema().chat.enabled && ( + { onChatContinuation()} - grow={false} - paddingSize="none" - style={{ width: '120px', float: 'right' }} + hasShadow={false} + color="plain" > - - - - - - - {i18n.translate('assistantDashboards.incontextInsight.continueInChat', { - defaultMessage: 'Continue in chat', - })} - - - + { + onGenerateSummary( + incontextInsight.suggestions && incontextInsight.suggestions.length > 0 + ? incontextInsight.suggestions[0] + : 'Please summarize the input' + ); + }} + feedbackResult={false} + showFeedback + onFeedback={(correct) => console.log(`Feedback: ${correct}`)} + showTraceIcon={false} + /> + {getConfigSchema().chat.enabled && ( + onChatContinuation()} + grow={false} + paddingSize="none" + style={{ width: '120px', float: 'right' }} + > + + + + + + + {i18n.translate('assistantDashboards.incontextInsight.continueInChat', { + defaultMessage: 'Continue in chat', + })} + + + + + )} - )} + } ) : ( void; + feedbackResult?: boolean; + showFeedback?: boolean; + onFeedback?: (correct: boolean) => void; + showTraceIcon?: boolean; + traceInteractionId?: string; + onViewTrace?: () => void; + shouldActionBarVisibleOnHover?: boolean; + isFullWidth?: boolean; +} + +export const MessageActions: React.FC = ({ + contentToCopy = '', + showRegenerate = false, + onRegenerate, + feedbackResult, + showFeedback = false, + onFeedback, + showTraceIcon = false, + traceInteractionId = null, + onViewTrace = null, + shouldActionBarVisibleOnHover = false, + isFullWidth = false, +}) => { + return ( + + {!isFullWidth && ( + + + {(copy) => ( + + )} + + + )} + {showRegenerate && onRegenerate && ( + + + + )} + {showFeedback && onFeedback && ( + <> + {feedbackResult !== false && ( + + onFeedback(true)} + /> + + )} + {feedbackResult !== true && ( + + onFeedback(false)} + /> + + )} + + )} + {showTraceIcon && traceInteractionId && onViewTrace && ( + + + + )} + + ); +}; diff --git a/public/tabs/chat/messages/message_bubble.tsx b/public/tabs/chat/messages/message_bubble.tsx index 3ffaca74..29b17d18 100644 --- a/public/tabs/chat/messages/message_bubble.tsx +++ b/public/tabs/chat/messages/message_bubble.tsx @@ -18,6 +18,8 @@ import { import React, { useCallback } from 'react'; import { IconType } from '@elastic/eui/src/components/icon/icon'; import cx from 'classnames'; +import { MessageActions } from './message_action'; + // TODO: Replace with getChrome().logos.Chat.url import { useChatActions } from '../../../hooks'; import chatIcon from '../../../assets/chat.svg'; @@ -169,92 +171,29 @@ export const MessageBubble: React.FC = React.memo((props) => {props.showActionBar && ( <> - - {!fullWidth && ( - - - {(copy) => ( - - )} - - - )} - {props.showRegenerate && props.interaction?.interaction_id ? ( - - props.onRegenerate?.(props.interaction?.interaction_id || '')} - title="regenerate message" - color="text" - iconType="refresh" - /> - - ) : null} - {showFeedback && ( - // After feedback, only corresponding thumb icon will be kept and disabled. - <> - {feedbackResult !== false ? ( - - feedbackOutput(true, feedbackResult)} - /> - - ) : null} - {feedbackResult !== true ? ( - - feedbackOutput(false, feedbackResult)} - /> - - ) : null} - - )} - {props.message.interactionId ? ( - - { - const message = props.message as IOutput; - - const viewTraceAction: ISuggestedAction = { - actionType: 'view_trace', - metadata: { - interactionId: message.interactionId || '', - }, - message: 'How was this generated?', - }; - executeAction(viewTraceAction, message); - }} - title="How was this generated?" - color="text" - iconType="iInCircle" - /> - - ) : null} - + props.onRegenerate?.(props.interaction?.interaction_id || '')} + feedbackResult={feedbackResult} + showFeedback={showFeedback} + onFeedback={(correct) => feedbackOutput(correct, feedbackResult)} + showTraceIcon={!!props.message.interactionId} + traceInteractionId={props.message.interactionId || ''} + onViewTrace={() => { + const message = props.message as IOutput; + const viewTraceAction: ISuggestedAction = { + actionType: 'view_trace', + metadata: { + interactionId: message.interactionId || '', + }, + message: 'How was this generated?', + }; + executeAction(viewTraceAction, message); + }} + shouldActionBarVisibleOnHover={props.shouldActionBarVisibleOnHover} + isFullWidth={fullWidth} + /> )} From 7938e5fd77b79695d173745f3729c17d1175440a Mon Sep 17 00:00:00 2001 From: Sihan He Date: Fri, 9 Aug 2024 19:07:26 +0800 Subject: [PATCH 02/29] Finish basic function of feedback buttons Signed-off-by: Sihan He --- public/chat_header_button.tsx | 22 +++++++++ .../generate_popover_body.tsx | 30 +++++++++--- public/hooks/use_feed_back.tsx | 46 +++++++++---------- 3 files changed, 68 insertions(+), 30 deletions(-) diff --git a/public/chat_header_button.tsx b/public/chat_header_button.tsx index de1caf1d..c8340800 100644 --- a/public/chat_header_button.tsx +++ b/public/chat_header_button.tsx @@ -26,6 +26,9 @@ import { import { useCore } from './contexts/core_context'; import { MountPointPortal } from '../../../src/plugins/opensearch_dashboards_react/public'; import { usePatchFixedStyle } from './hooks/use_patch_fixed_style'; +import { IOutput } from '../common/types/chat_saved_object_attributes'; +import { SendFeedbackBody } from '../common/types/chat_saved_object_attributes'; +import { ASSISTANT_API } from '../common/constants/llm'; interface HeaderChatButtonProps { application: ApplicationStart; @@ -214,6 +217,25 @@ export const HeaderChatButton = (props: HeaderChatButtonProps) => { }; }, [appId, flyoutVisible, props.assistantActions, registry]); + useEffect(() => { + const handleFeedback = async (event: { message: IOutput; body: SendFeedbackBody }) => { + console.log('send feedback', event.message); + try { + await core.services.http.put(`${ASSISTANT_API.FEEDBACK}/${event.message.interactionId}`, { + body: JSON.stringify(event.body), + query: core.services.dataSource.getDataSourceQuery(), + }); + console.log(`Feedback sent: ${event.body}`); + } catch (error) { + console.log('send feedback error'); + } + }; + registry.on('onSendFeedback', handleFeedback); + return () => { + registry.off('onSendFeedback', handleFeedback); + }; + }, [core.services.http, core.services.dataSource, registry]); + return ( <>
diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index 7d766a65..0c6036c9 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import React, { useState } from 'react'; +import React, { useCallback, useState } from 'react'; import cx from 'classnames'; import { i18n } from '@osd/i18n'; import { @@ -20,8 +20,9 @@ import { EuiLoadingContent, EuiLoadingSpinner, } from '@elastic/eui'; +import { IMessage, Interaction, IOutput } from 'common/types/chat_saved_object_attributes'; import { MessageActions } from '../../tabs/chat/messages/message_action'; - +import { useFeedback } from '../../hooks/use_feed_back'; import { IncontextInsight as IncontextInsightInput } from '../../types'; import { getConfigSchema, getIncontextInsightRegistry, getNotifications } from '../../services'; import { HttpSetup } from '../../../../../src/core/public'; @@ -36,8 +37,12 @@ export const GeneratePopoverBody: React.FC<{ const [isLoading, setIsLoading] = useState(false); const [summary, setSummary] = useState(''); const [conversationId, setConversationId] = useState(''); + const [interaction, setInteraction] = useState(null); + const [message, setMessage] = useState(null); + const toasts = getNotifications().toasts; const registry = getIncontextInsightRegistry(); + const { sendFeedback, feedbackResult } = useFeedback(interaction, false); const onChatContinuation = () => { registry?.continueInChat(incontextInsight, conversationId); @@ -77,11 +82,13 @@ export const GeneratePopoverBody: React.FC<{ const interactionLength = response.interactions.length; if (interactionLength > 0) { setConversationId(response.interactions[interactionLength - 1].conversation_id); + setInteraction(response.interactions[interactionLength - 1]); } const messageLength = response.messages.length; if (messageLength > 0 && response.messages[messageLength - 1].type === 'output') { setSummary(response.messages[messageLength - 1].content); + setMessage(response.messages[messageLength - 1]); } }) .catch((error) => { @@ -99,6 +106,17 @@ export const GeneratePopoverBody: React.FC<{ return summarize(); }; + const onFeedback = useCallback( + (correct: boolean) => { + if (feedbackResult !== undefined || !message) { + return; + } + console.log('message', message); + sendFeedback(message as IOutput, correct); + }, + [message, sendFeedback] + ); + return summary ? ( <> @@ -116,16 +134,16 @@ export const GeneratePopoverBody: React.FC<{ { - onGenerateSummary( + onRegenerate={async () => { + await onGenerateSummary( incontextInsight.suggestions && incontextInsight.suggestions.length > 0 ? incontextInsight.suggestions[0] : 'Please summarize the input' ); }} - feedbackResult={false} + feedbackResult={feedbackResult} showFeedback - onFeedback={(correct) => console.log(`Feedback: ${correct}`)} + onFeedback={onFeedback} showTraceIcon={false} /> {getConfigSchema().chat.enabled && ( diff --git a/public/hooks/use_feed_back.tsx b/public/hooks/use_feed_back.tsx index 7a37bd37..3200df90 100644 --- a/public/hooks/use_feed_back.tsx +++ b/public/hooks/use_feed_back.tsx @@ -4,46 +4,44 @@ */ import { useState } from 'react'; -import { ASSISTANT_API } from '../../common/constants/llm'; + import { IOutput, Interaction } from '../../common/types/chat_saved_object_attributes'; import { useCore } from '../contexts/core_context'; import { useChatState } from './use_chat_state'; import { SendFeedbackBody } from '../../common/types/chat_saved_object_attributes'; +import { getIncontextInsightRegistry } from '../services'; -export const useFeedback = (interaction?: Interaction | null) => { - const core = useCore(); - const { chatState } = useChatState(); +export const useFeedback = (interaction?: Interaction | null, hasChatState: boolean = true) => { + const registry = getIncontextInsightRegistry(); + const chatStateContext = useChatState(); const [feedbackResult, setFeedbackResult] = useState( interaction?.additional_info?.feedback?.satisfaction ?? undefined ); const sendFeedback = async (message: IOutput, correct: boolean) => { - const outputMessage = message; - // Markdown type output all has interactionId. The interactionId of message is equal to interaction id. - const outputMessageIndex = chatState.messages.findIndex((item) => { - return item.type === 'output' && item.interactionId === message.interactionId; - }); - const inputMessage = chatState.messages - .slice(0, outputMessageIndex) - .reverse() - .find((item) => item.type === 'input'); - if (!inputMessage) { - return; + if (hasChatState && chatStateContext?.chatState) { + const chatState = chatStateContext.chatState; + // Markdown type output all has interactionId. The interactionId of message is equal to interaction id. + const outputMessageIndex = chatState.messages.findIndex((item) => { + return item.type === 'output' && item.interactionId === message.interactionId; + }); + const inputMessage = chatState.messages + .slice(0, outputMessageIndex) + .reverse() + .find((item) => item.type === 'input'); + if (!inputMessage) { + return; + } } const body: SendFeedbackBody = { satisfaction: correct, }; - try { - await core.services.http.put(`${ASSISTANT_API.FEEDBACK}/${message.interactionId}`, { - body: JSON.stringify(body), - query: core.services.dataSource.getDataSourceQuery(), - }); - setFeedbackResult(correct); - } catch (error) { - console.log('send feedback error'); - } + console.log('use message', message); + + registry.emit('onSendFeedback', { message, body }); + setFeedbackResult(correct); }; return { sendFeedback, feedbackResult }; From 82c0fd4566368b6813ebd1ffa1a3b7d7c544dea6 Mon Sep 17 00:00:00 2001 From: Sihan He Date: Mon, 12 Aug 2024 22:10:05 +0800 Subject: [PATCH 03/29] Add test cases and fix bugs Signed-off-by: Sihan He --- public/chat_header_button.tsx | 12 +- .../generate_popover_body.tsx | 25 +---- public/hooks/use_feed_back.test.tsx | 105 ++++++++---------- public/hooks/use_feed_back.tsx | 24 ++-- .../incontext_insight_registry.test.ts | 15 +++ .../incontext_insight_registry.ts | 22 +++- public/tabs/chat/chat_page_content.tsx | 6 +- .../chat/messages/message_action.test.tsx | 82 ++++++++++++++ public/tabs/chat/messages/message_action.tsx | 30 +++-- public/tabs/chat/messages/message_bubble.tsx | 25 +---- 10 files changed, 210 insertions(+), 136 deletions(-) create mode 100644 public/tabs/chat/messages/message_action.test.tsx diff --git a/public/chat_header_button.tsx b/public/chat_header_button.tsx index c8340800..7496cdce 100644 --- a/public/chat_header_button.tsx +++ b/public/chat_header_button.tsx @@ -26,7 +26,6 @@ import { import { useCore } from './contexts/core_context'; import { MountPointPortal } from '../../../src/plugins/opensearch_dashboards_react/public'; import { usePatchFixedStyle } from './hooks/use_patch_fixed_style'; -import { IOutput } from '../common/types/chat_saved_object_attributes'; import { SendFeedbackBody } from '../common/types/chat_saved_object_attributes'; import { ASSISTANT_API } from '../common/constants/llm'; @@ -218,23 +217,22 @@ export const HeaderChatButton = (props: HeaderChatButtonProps) => { }, [appId, flyoutVisible, props.assistantActions, registry]); useEffect(() => { - const handleFeedback = async (event: { message: IOutput; body: SendFeedbackBody }) => { - console.log('send feedback', event.message); + const handleFeedback = async (event: { interactionId: string; body: SendFeedbackBody }) => { try { - await core.services.http.put(`${ASSISTANT_API.FEEDBACK}/${event.message.interactionId}`, { + await core.services.http.put(`${ASSISTANT_API.FEEDBACK}/${event.interactionId}`, { body: JSON.stringify(event.body), query: core.services.dataSource.getDataSourceQuery(), }); - console.log(`Feedback sent: ${event.body}`); + registry.feedbackSuccess(event.interactionId, event.body.satisfaction); } catch (error) { - console.log('send feedback error'); + console.error('send feedback error', error); } }; registry.on('onSendFeedback', handleFeedback); return () => { registry.off('onSendFeedback', handleFeedback); }; - }, [core.services.http, core.services.dataSource, registry]); + }, [core, registry]); return ( <> diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index 0c6036c9..6c8cb544 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -3,8 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import React, { useCallback, useState } from 'react'; -import cx from 'classnames'; +import React, { useState } from 'react'; import { i18n } from '@osd/i18n'; import { EuiButton, @@ -14,15 +13,9 @@ import { EuiPanel, EuiSpacer, EuiText, - EuiAvatar, - EuiButtonIcon, - EuiCopy, - EuiLoadingContent, - EuiLoadingSpinner, } from '@elastic/eui'; import { IMessage, Interaction, IOutput } from 'common/types/chat_saved_object_attributes'; import { MessageActions } from '../../tabs/chat/messages/message_action'; -import { useFeedback } from '../../hooks/use_feed_back'; import { IncontextInsight as IncontextInsightInput } from '../../types'; import { getConfigSchema, getIncontextInsightRegistry, getNotifications } from '../../services'; import { HttpSetup } from '../../../../../src/core/public'; @@ -42,7 +35,6 @@ export const GeneratePopoverBody: React.FC<{ const toasts = getNotifications().toasts; const registry = getIncontextInsightRegistry(); - const { sendFeedback, feedbackResult } = useFeedback(interaction, false); const onChatContinuation = () => { registry?.continueInChat(incontextInsight, conversationId); @@ -106,17 +98,6 @@ export const GeneratePopoverBody: React.FC<{ return summarize(); }; - const onFeedback = useCallback( - (correct: boolean) => { - if (feedbackResult !== undefined || !message) { - return; - } - console.log('message', message); - sendFeedback(message as IOutput, correct); - }, - [message, sendFeedback] - ); - return summary ? ( <> @@ -141,9 +122,9 @@ export const GeneratePopoverBody: React.FC<{ : 'Please summarize the input' ); }} - feedbackResult={feedbackResult} + interaction={interaction} showFeedback - onFeedback={onFeedback} + message={message as IOutput} showTraceIcon={false} /> {getConfigSchema().chat.enabled && ( diff --git a/public/hooks/use_feed_back.test.tsx b/public/hooks/use_feed_back.test.tsx index 943aab2f..421a58ac 100644 --- a/public/hooks/use_feed_back.test.tsx +++ b/public/hooks/use_feed_back.test.tsx @@ -4,36 +4,24 @@ */ import { renderHook, act } from '@testing-library/react-hooks'; - import { useFeedback } from './use_feed_back'; import * as chatStateHookExports from './use_chat_state'; -import * as coreHookExports from '../contexts/core_context'; -import { httpServiceMock } from '../../../../src/core/public/mocks'; -import * as chatContextHookExports from '../contexts/chat_context'; import { Interaction, IOutput, IMessage } from '../../common/types/chat_saved_object_attributes'; -import { ASSISTANT_API } from '../../common/constants/llm'; -import { DataSourceServiceMock } from '../services/data_source_service.mock'; +import { getIncontextInsightRegistry } from '../services'; + +jest.mock('../services'); describe('useFeedback hook', () => { - const httpMock = httpServiceMock.createStartContract(); + let registryMock: unknown; const chatStateDispatchMock = jest.fn(); - const dataSourceMock = new DataSourceServiceMock(); - const chatContextMock = { - rootAgentId: 'root_agent_id_mock', - selectedTabId: 'chat', - setConversationId: jest.fn(), - setTitle: jest.fn(), - currentAccount: { username: 'admin' }, - }; beforeEach(() => { - jest.spyOn(coreHookExports, 'useCore').mockReturnValue({ - services: { - http: httpMock, - dataSource: dataSourceMock, - }, - }); - jest.spyOn(chatContextHookExports, 'useChatContext').mockReturnValue(chatContextMock); + registryMock = { + sendFeedbackRequest: jest.fn(), + on: jest.fn(), + }; + + (getIncontextInsightRegistry as jest.Mock).mockReturnValue(registryMock); jest.spyOn(chatStateHookExports, 'useChatState').mockReturnValue({ chatState: { messages: [], interactions: [], llmResponding: false }, @@ -59,6 +47,9 @@ describe('useFeedback hook', () => { }); it('should call feedback api regularly with passed correct value and set feedback state if call API success', async () => { + const mockInteraction = { + interaction_id: 'interactionId', + } as Interaction; const mockInputMessage = { type: 'input', } as IMessage; @@ -67,32 +58,33 @@ describe('useFeedback hook', () => { interactionId: 'interactionId', } as IOutput; const mockMessages = [mockInputMessage, mockOutputMessage]; + const correct = true; jest.spyOn(chatStateHookExports, 'useChatState').mockReturnValue({ chatState: { messages: mockMessages, interactions: [], llmResponding: false }, chatStateDispatch: chatStateDispatchMock, }); - const { result } = renderHook(() => useFeedback()); + const { result } = renderHook(() => useFeedback(mockInteraction)); expect(result.current.feedbackResult).toBe(undefined); const sendFeedback = result.current.sendFeedback; await act(async () => { - await sendFeedback(mockOutputMessage, true); + await sendFeedback(mockOutputMessage, correct); }); - expect(httpMock.put).toHaveBeenCalledWith( - `${ASSISTANT_API.FEEDBACK}/${mockOutputMessage.interactionId}`, - { - body: JSON.stringify({ - satisfaction: true, - }), - query: dataSourceMock.getDataSourceQuery(), - } + act(() => { + registryMock.on.mock.calls.forEach(([event, handler]) => { + if (event === `feedbackSuccess:${mockOutputMessage.interactionId}`) { + handler({ correct }); + } + }); + }); + expect(registryMock.sendFeedbackRequest).toHaveBeenCalledWith( + mockOutputMessage.interactionId, + correct ); - expect(result.current.feedbackResult).toBe(true); + expect(result.current.feedbackResult).toBe(correct); }); it('should not update feedback state if API fail', async () => { - httpMock.put.mockRejectedValueOnce(new Error('')); - const mockInputMessage = { type: 'input', } as IMessage; @@ -113,43 +105,36 @@ describe('useFeedback hook', () => { await sendFeedback(mockOutputMessage, true); }); - expect(httpMock.put).toHaveBeenCalledWith( - `${ASSISTANT_API.FEEDBACK}/${mockOutputMessage.interactionId}`, - { - body: JSON.stringify({ - satisfaction: true, - }), - query: dataSourceMock.getDataSourceQuery(), - } - ); expect(result.current.feedbackResult).toBe(undefined); }); - it('should not call API to feedback if there is no input message before passed output message', async () => { + it('should call feedback request on registry without checking for input message if hasChatState is false', async () => { const mockOutputMessage = { type: 'output', interactionId: 'interactionId', } as IOutput; - const mockMessages = [mockOutputMessage]; - jest.spyOn(chatStateHookExports, 'useChatState').mockReturnValue({ - chatState: { messages: mockMessages, interactions: [], llmResponding: false }, - chatStateDispatch: chatStateDispatchMock, - }); - const { result } = renderHook(() => useFeedback()); + const mockInteraction = { + interaction_id: 'interactionId', + } as Interaction; + + const { result } = renderHook(() => useFeedback(mockInteraction, false)); expect(result.current.feedbackResult).toBe(undefined); const sendFeedback = result.current.sendFeedback; await act(async () => { - await sendFeedback(mockOutputMessage, true); + await sendFeedback(mockOutputMessage, false); }); - - expect(httpMock.put).not.toHaveBeenCalledWith( - `${ASSISTANT_API.FEEDBACK}/${mockOutputMessage.interactionId}`, - { - body: JSON.stringify({ - satisfaction: true, - }), - } + act(() => { + registryMock.on.mock.calls.forEach(([event, handler]) => { + if (event === `feedbackSuccess:${mockOutputMessage.interactionId}`) { + handler({ correct: false }); + } + }); + }); + expect(registryMock.sendFeedbackRequest).toHaveBeenCalledWith( + mockOutputMessage.interactionId, + false ); + expect(result.current.feedbackResult).toBe(false); }); }); diff --git a/public/hooks/use_feed_back.tsx b/public/hooks/use_feed_back.tsx index 3200df90..28a984e5 100644 --- a/public/hooks/use_feed_back.tsx +++ b/public/hooks/use_feed_back.tsx @@ -3,12 +3,10 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { useState } from 'react'; +import { useState, useEffect } from 'react'; import { IOutput, Interaction } from '../../common/types/chat_saved_object_attributes'; -import { useCore } from '../contexts/core_context'; import { useChatState } from './use_chat_state'; -import { SendFeedbackBody } from '../../common/types/chat_saved_object_attributes'; import { getIncontextInsightRegistry } from '../services'; export const useFeedback = (interaction?: Interaction | null, hasChatState: boolean = true) => { @@ -33,16 +31,20 @@ export const useFeedback = (interaction?: Interaction | null, hasChatState: bool return; } } + registry.sendFeedbackRequest(message.interactionId, correct); + }; - const body: SendFeedbackBody = { - satisfaction: correct, + useEffect(() => { + const successFeedback = (event: { correct: boolean }) => { + setFeedbackResult(event.correct); }; - - console.log('use message', message); - - registry.emit('onSendFeedback', { message, body }); - setFeedbackResult(correct); - }; + if (interaction && interaction.interaction_id) { + registry.on(`feedbackSuccess:${interaction.interaction_id}`, successFeedback); + return () => { + registry.off(`feedbackSuccess:${interaction.interaction_id}`, successFeedback); + }; + } + }, [interaction]); return { sendFeedback, feedbackResult }; }; diff --git a/public/services/__tests__/incontext_insight_registry.test.ts b/public/services/__tests__/incontext_insight_registry.test.ts index b49c3e54..0427c478 100644 --- a/public/services/__tests__/incontext_insight_registry.test.ts +++ b/public/services/__tests__/incontext_insight_registry.test.ts @@ -51,6 +51,21 @@ describe('IncontextInsightRegistry', () => { }); }); + it('emits "onSendFeedback" event when sendFeedbackRequest is called', () => { + const mockFn = jest.fn(); + const correct = true; + registry.on('onSendFeedback', mockFn); + + registry.sendFeedbackRequest('test interactionId', correct); + + expect(mockFn).toHaveBeenCalledWith({ + interactionId: 'test interactionId', + body: { + satisfaction: correct, + }, + }); + }); + it('adds item to registry when register is called with a single item', () => { registry.register(insight); diff --git a/public/services/incontext_insight/incontext_insight_registry.ts b/public/services/incontext_insight/incontext_insight_registry.ts index 1b35a04e..577e846c 100644 --- a/public/services/incontext_insight/incontext_insight_registry.ts +++ b/public/services/incontext_insight/incontext_insight_registry.ts @@ -5,7 +5,12 @@ import EventEmitter from 'events'; import { IncontextInsight, IncontextInsights } from '../../types'; -import { ISuggestedAction, Interaction } from '../../../common/types/chat_saved_object_attributes'; +import { + ISuggestedAction, + Interaction, + IOutput, + SendFeedbackBody, +} from '../../../common/types/chat_saved_object_attributes'; export class IncontextInsightRegistry extends EventEmitter { private registry: IncontextInsights = new Map(); @@ -50,6 +55,21 @@ export class IncontextInsightRegistry extends EventEmitter { }); } + public sendFeedbackRequest(interactionId: string | undefined, correct: boolean) { + if (interactionId === undefined || interactionId === '') return; + const body: SendFeedbackBody = { + satisfaction: correct, + }; + this.emit('onSendFeedback', { + interactionId, + body, + }); + } + + public feedbackSuccess(interactionId: string, correct: boolean) { + this.emit(`feedbackSuccess:${interactionId}`, { correct }); + } + public register(item: IncontextInsight | IncontextInsight[]): void; public register(item: unknown) { if (Array.isArray(item)) { diff --git a/public/tabs/chat/chat_page_content.tsx b/public/tabs/chat/chat_page_content.tsx index 89f518cc..f559c460 100644 --- a/public/tabs/chat/chat_page_content.tsx +++ b/public/tabs/chat/chat_page_content.tsx @@ -14,11 +14,7 @@ import { EuiText, } from '@elastic/eui'; import React, { useLayoutEffect, useRef } from 'react'; -import { - IMessage, - ISuggestedAction, - Interaction, -} from '../../../common/types/chat_saved_object_attributes'; +import { IMessage, Interaction } from '../../../common/types/chat_saved_object_attributes'; import { WelcomeMessage } from '../../components/chat_welcome_message'; import { useChatContext } from '../../contexts'; import { useChatState, useChatActions } from '../../hooks'; diff --git a/public/tabs/chat/messages/message_action.test.tsx b/public/tabs/chat/messages/message_action.test.tsx new file mode 100644 index 00000000..b8442af9 --- /dev/null +++ b/public/tabs/chat/messages/message_action.test.tsx @@ -0,0 +1,82 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import React from 'react'; +import { render, screen, fireEvent } from '@testing-library/react'; +import '@testing-library/jest-dom/extend-expect'; +import { MessageActions } from './message_action'; +import { useFeedback } from '../../../hooks/use_feed_back'; +import { IOutput, Interaction } from '../../../../common/types/chat_saved_object_attributes'; + +jest.mock('../../../hooks/use_feed_back'); + +describe('MessageActions', () => { + let mockUseFeedback: jest.Mock; + + beforeEach(() => { + mockUseFeedback = useFeedback as jest.Mock; + mockUseFeedback.mockReturnValue({ + feedbackResult: undefined, + sendFeedback: jest.fn(), + }); + document.execCommand = jest.fn().mockImplementation(() => true); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should render copy button and call copy function when clicked', () => { + const contentToCopy = 'Test content'; + const { getByLabelText } = render(); + + const copyButton = getByLabelText('copy message'); + fireEvent.click(copyButton); + + expect(document.execCommand).toHaveBeenCalledWith('copy'); + }); + + it('should render regenerate button and call onRegenerate function when clicked', () => { + const onRegenerate = jest.fn(); + render(); + + const regenerateButton = screen.getByLabelText('regenerate message'); + fireEvent.click(regenerateButton); + + expect(onRegenerate).toHaveBeenCalledTimes(1); + }); + + it('should render feedback buttons and call handleFeedback function', () => { + const interaction = { interaction_id: 'interaction1' } as Interaction; + const message = { interactionId: 'interaction1' } as IOutput; + const sendFeedback = jest.fn(); + + mockUseFeedback.mockReturnValue({ + feedbackResult: undefined, + sendFeedback, + }); + + render(); + + const thumbsUpButton = screen.getByLabelText('feedback thumbs up'); + const thumbsDownButton = screen.getByLabelText('feedback thumbs down'); + + fireEvent.click(thumbsUpButton); + expect(sendFeedback).toHaveBeenCalledWith(message, true); + + fireEvent.click(thumbsDownButton); + expect(sendFeedback).toHaveBeenCalledWith(message, false); + }); + + it('should render trace icon and call onViewTrace function when clicked', () => { + const onViewTrace = jest.fn(); + render(); + + const traceButton = screen.getByLabelText('How was this generated?'); + fireEvent.click(traceButton); + + expect(onViewTrace).toHaveBeenCalledTimes(1); + }); +}); diff --git a/public/tabs/chat/messages/message_action.tsx b/public/tabs/chat/messages/message_action.tsx index 80f6a734..61f04ced 100644 --- a/public/tabs/chat/messages/message_action.tsx +++ b/public/tabs/chat/messages/message_action.tsx @@ -3,16 +3,18 @@ * SPDX-License-Identifier: Apache-2.0 */ -import React from 'react'; +import React, { useCallback } from 'react'; import { EuiFlexGroup, EuiFlexItem, EuiButtonIcon, EuiCopy } from '@elastic/eui'; +import { IOutput, Interaction } from '../../../../common/types/chat_saved_object_attributes'; +import { useFeedback } from '../../../hooks/use_feed_back'; interface MessageActionsProps { contentToCopy?: string; showRegenerate?: boolean; onRegenerate?: () => void; - feedbackResult?: boolean; + interaction?: Interaction | null; + message?: IOutput | null; showFeedback?: boolean; - onFeedback?: (correct: boolean) => void; showTraceIcon?: boolean; traceInteractionId?: string; onViewTrace?: () => void; @@ -24,15 +26,27 @@ export const MessageActions: React.FC = ({ contentToCopy = '', showRegenerate = false, onRegenerate, - feedbackResult, + interaction, + message, showFeedback = false, - onFeedback, showTraceIcon = false, traceInteractionId = null, onViewTrace = null, shouldActionBarVisibleOnHover = false, isFullWidth = false, }) => { + const { feedbackResult, sendFeedback } = useFeedback(interaction); + + const handleFeedback = useCallback( + (correct: boolean) => { + if (feedbackResult !== undefined || !message) { + return; + } + sendFeedback(message, correct); + }, + [feedbackResult, message, sendFeedback] + ); + return ( = ({ /> )} - {showFeedback && onFeedback && ( + {showFeedback && ( <> {feedbackResult !== false && ( @@ -77,7 +91,7 @@ export const MessageActions: React.FC = ({ aria-label="feedback thumbs up" color={feedbackResult === true ? 'primary' : 'text'} iconType="thumbsUp" - onClick={() => onFeedback(true)} + onClick={() => handleFeedback(true)} /> )} @@ -87,7 +101,7 @@ export const MessageActions: React.FC = ({ aria-label="feedback thumbs down" color={feedbackResult === false ? 'primary' : 'text'} iconType="thumbsDown" - onClick={() => onFeedback(false)} + onClick={() => handleFeedback(false)} /> )} diff --git a/public/tabs/chat/messages/message_bubble.tsx b/public/tabs/chat/messages/message_bubble.tsx index 29b17d18..a54c6781 100644 --- a/public/tabs/chat/messages/message_bubble.tsx +++ b/public/tabs/chat/messages/message_bubble.tsx @@ -5,8 +5,6 @@ import { EuiAvatar, - EuiButtonIcon, - EuiCopy, EuiFlexGroup, EuiFlexItem, EuiLoadingContent, @@ -15,9 +13,8 @@ import { EuiSpacer, EuiIcon, } from '@elastic/eui'; -import React, { useCallback } from 'react'; +import React from 'react'; import { IconType } from '@elastic/eui/src/components/icon/icon'; -import cx from 'classnames'; import { MessageActions } from './message_action'; // TODO: Replace with getChrome().logos.Chat.url @@ -29,7 +26,6 @@ import { ISuggestedAction, Interaction, } from '../../../../common/types/chat_saved_object_attributes'; -import { useFeedback } from '../../../hooks/use_feed_back'; type MessageBubbleProps = { showActionBar: boolean; @@ -47,10 +43,6 @@ type MessageBubbleProps = { ); export const MessageBubble: React.FC = React.memo((props) => { - const { feedbackResult, sendFeedback } = useFeedback( - 'interaction' in props ? props.interaction : null - ); - const { executeAction } = useChatActions(); // According to the design of the feedback, only markdown type output is supported. @@ -59,17 +51,6 @@ export const MessageBubble: React.FC = React.memo((props) => props.message.type === 'output' && props.message.contentType === 'markdown'; - const feedbackOutput = useCallback( - (correct: boolean, result: boolean | undefined) => { - // No repeated feedback. - if (result !== undefined || !('message' in props)) { - return; - } - sendFeedback(props.message as IOutput, correct); - }, - [props, sendFeedback] - ); - const createAvatar = (iconType?: IconType) => { if (iconType) { return ( @@ -175,9 +156,9 @@ export const MessageBubble: React.FC = React.memo((props) => contentToCopy={props.message.content ?? ''} showRegenerate={props.showRegenerate} onRegenerate={() => props.onRegenerate?.(props.interaction?.interaction_id || '')} - feedbackResult={feedbackResult} + interaction={props.interaction} + message={props.message as IOutput} showFeedback={showFeedback} - onFeedback={(correct) => feedbackOutput(correct, feedbackResult)} showTraceIcon={!!props.message.interactionId} traceInteractionId={props.message.interactionId || ''} onViewTrace={() => { From 19a57492c998210a5778f7826c4cc66295134ca6 Mon Sep 17 00:00:00 2001 From: Sihan He Date: Wed, 14 Aug 2024 13:20:57 +0800 Subject: [PATCH 04/29] Refactor the code to deal with core loss and fix the regenerate bug Signed-off-by: Sihan He --- public/chat_header_button.tsx | 20 ----- .../generate_popover_body.tsx | 90 ++++++++++++------- public/components/incontext_insight/index.tsx | 9 +- public/hooks/use_feed_back.test.tsx | 78 ++++++---------- public/hooks/use_feed_back.tsx | 42 +++++---- public/plugin.tsx | 9 +- .../incontext_insight_registry.test.ts | 15 ---- .../incontext_insight_registry.ts | 22 +---- public/tabs/chat/messages/message_action.tsx | 8 +- public/tabs/chat/messages/message_bubble.tsx | 4 + 10 files changed, 137 insertions(+), 160 deletions(-) diff --git a/public/chat_header_button.tsx b/public/chat_header_button.tsx index 7496cdce..de1caf1d 100644 --- a/public/chat_header_button.tsx +++ b/public/chat_header_button.tsx @@ -26,8 +26,6 @@ import { import { useCore } from './contexts/core_context'; import { MountPointPortal } from '../../../src/plugins/opensearch_dashboards_react/public'; import { usePatchFixedStyle } from './hooks/use_patch_fixed_style'; -import { SendFeedbackBody } from '../common/types/chat_saved_object_attributes'; -import { ASSISTANT_API } from '../common/constants/llm'; interface HeaderChatButtonProps { application: ApplicationStart; @@ -216,24 +214,6 @@ export const HeaderChatButton = (props: HeaderChatButtonProps) => { }; }, [appId, flyoutVisible, props.assistantActions, registry]); - useEffect(() => { - const handleFeedback = async (event: { interactionId: string; body: SendFeedbackBody }) => { - try { - await core.services.http.put(`${ASSISTANT_API.FEEDBACK}/${event.interactionId}`, { - body: JSON.stringify(event.body), - query: core.services.dataSource.getDataSourceQuery(), - }); - registry.feedbackSuccess(event.interactionId, event.body.satisfaction); - } catch (error) { - console.error('send feedback error', error); - } - }; - registry.on('onSendFeedback', handleFeedback); - return () => { - registry.off('onSendFeedback', handleFeedback); - }; - }, [core, registry]); - return ( <>
diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index 6c8cb544..e45d0f2e 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -19,14 +19,16 @@ import { MessageActions } from '../../tabs/chat/messages/message_action'; import { IncontextInsight as IncontextInsightInput } from '../../types'; import { getConfigSchema, getIncontextInsightRegistry, getNotifications } from '../../services'; import { HttpSetup } from '../../../../../src/core/public'; +import { DataSourceService } from '../../services/data_source_service'; import { ASSISTANT_API } from '../../../common/constants/llm'; import { getAssistantRole } from '../../utils/constants'; export const GeneratePopoverBody: React.FC<{ incontextInsight: IncontextInsightInput; httpSetup?: HttpSetup; + dataSourceService?: DataSourceService; closePopover: () => void; -}> = ({ incontextInsight, httpSetup, closePopover }) => { +}> = ({ incontextInsight, httpSetup, dataSourceService, closePopover }) => { const [isLoading, setIsLoading] = useState(false); const [summary, setSummary] = useState(''); const [conversationId, setConversationId] = useState(''); @@ -36,6 +38,21 @@ export const GeneratePopoverBody: React.FC<{ const toasts = getNotifications().toasts; const registry = getIncontextInsightRegistry(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const handleSummaryResponse = (response: any) => { + const interactionLength = response.interactions.length; + if (interactionLength > 0) { + setConversationId(response.interactions[interactionLength - 1].conversation_id); + setInteraction(response.interactions[interactionLength - 1]); + } + + const messageLength = response.messages.length; + if (messageLength > 0 && response.messages[messageLength - 1].type === 'output') { + setSummary(response.messages[messageLength - 1].content); + setMessage(response.messages[messageLength - 1]); + } + }; + const onChatContinuation = () => { registry?.continueInChat(incontextInsight, conversationId); closePopover(); @@ -57,8 +74,8 @@ export const GeneratePopoverBody: React.FC<{ incontextInsightType = incontextInsight.key; } - await httpSetup - ?.post(ASSISTANT_API.SEND_MESSAGE, { + try { + const response = await httpSetup?.post(ASSISTANT_API.SEND_MESSAGE, { body: JSON.stringify({ messages: [], input: { @@ -69,35 +86,46 @@ export const GeneratePopoverBody: React.FC<{ promptPrefix: getAssistantRole(incontextInsightType), }, }), - }) - .then((response) => { - const interactionLength = response.interactions.length; - if (interactionLength > 0) { - setConversationId(response.interactions[interactionLength - 1].conversation_id); - setInteraction(response.interactions[interactionLength - 1]); - } - - const messageLength = response.messages.length; - if (messageLength > 0 && response.messages[messageLength - 1].type === 'output') { - setSummary(response.messages[messageLength - 1].content); - setMessage(response.messages[messageLength - 1]); - } - }) - .catch((error) => { - toasts.addDanger( - i18n.translate('assistantDashboards.incontextInsight.generateSummaryError', { - defaultMessage: 'Generate summary error', - }) - ); - }) - .finally(() => { - setIsLoading(false); }); + handleSummaryResponse(response); + } catch (error) { + toasts.addDanger( + i18n.translate('assistantDashboards.incontextInsight.generateSummaryError', { + defaultMessage: 'Generate summary error', + }) + ); + } finally { + setIsLoading(false); + } }; return summarize(); }; + const onRegenerateSummary = async (interactionId: string) => { + setIsLoading(true); + setSummary(''); + if (conversationId) { + try { + const response = await httpSetup?.put(`${ASSISTANT_API.REGENERATE}`, { + body: JSON.stringify({ + conversationId, + interactionId, + }), + query: dataSourceService?.getDataSourceQuery(), + }); + handleSummaryResponse(response); + } catch (error) { + toasts.addDanger( + i18n.translate('assistantDashboards.incontextInsight.generateSummaryError', { + defaultMessage: 'Regenerate summary error', + }) + ); + } + } + setIsLoading(false); + }; + return summary ? ( <> @@ -115,17 +143,13 @@ export const GeneratePopoverBody: React.FC<{ { - await onGenerateSummary( - incontextInsight.suggestions && incontextInsight.suggestions.length > 0 - ? incontextInsight.suggestions[0] - : 'Please summarize the input' - ); - }} + onRegenerate={async () => onRegenerateSummary(interaction?.interaction_id || '')} interaction={interaction} showFeedback message={message as IOutput} showTraceIcon={false} + httpSetup={httpSetup} + dataSourceService={dataSourceService} /> {getConfigSchema().chat.enabled && ( { +export const IncontextInsight = ({ + children, + httpSetup, + dataSourceService, +}: IncontextInsightProps) => { const anchor = useRef(null); const [isVisible, setIsVisible] = useState(false); @@ -278,6 +284,7 @@ export const IncontextInsight = ({ children, httpSetup }: IncontextInsightProps) ); diff --git a/public/hooks/use_feed_back.test.tsx b/public/hooks/use_feed_back.test.tsx index 421a58ac..95bd54d5 100644 --- a/public/hooks/use_feed_back.test.tsx +++ b/public/hooks/use_feed_back.test.tsx @@ -7,22 +7,23 @@ import { renderHook, act } from '@testing-library/react-hooks'; import { useFeedback } from './use_feed_back'; import * as chatStateHookExports from './use_chat_state'; import { Interaction, IOutput, IMessage } from '../../common/types/chat_saved_object_attributes'; -import { getIncontextInsightRegistry } from '../services'; +import { DataSourceService } from '../services'; +import { HttpSetup } from '../../../../src/core/public'; +import { ASSISTANT_API } from '../../common/constants/llm'; jest.mock('../services'); describe('useFeedback hook', () => { - let registryMock: unknown; + const httpMock: jest.Mocked = ({ + put: jest.fn(), + } as unknown) as jest.Mocked; + + const dataSourceServiceMock = ({ + getDataSourceQuery: jest.fn(), + } as unknown) as DataSourceService; const chatStateDispatchMock = jest.fn(); beforeEach(() => { - registryMock = { - sendFeedbackRequest: jest.fn(), - on: jest.fn(), - }; - - (getIncontextInsightRegistry as jest.Mock).mockReturnValue(registryMock); - jest.spyOn(chatStateHookExports, 'useChatState').mockReturnValue({ chatState: { messages: [], interactions: [], llmResponding: false }, chatStateDispatch: chatStateDispatchMock, @@ -63,28 +64,29 @@ describe('useFeedback hook', () => { chatState: { messages: mockMessages, interactions: [], llmResponding: false }, chatStateDispatch: chatStateDispatchMock, }); - const { result } = renderHook(() => useFeedback(mockInteraction)); + const { result } = renderHook(() => + useFeedback(mockInteraction, httpMock, dataSourceServiceMock) + ); expect(result.current.feedbackResult).toBe(undefined); const sendFeedback = result.current.sendFeedback; await act(async () => { await sendFeedback(mockOutputMessage, correct); }); - act(() => { - registryMock.on.mock.calls.forEach(([event, handler]) => { - if (event === `feedbackSuccess:${mockOutputMessage.interactionId}`) { - handler({ correct }); - } - }); - }); - expect(registryMock.sendFeedbackRequest).toHaveBeenCalledWith( - mockOutputMessage.interactionId, - correct + expect(httpMock.put).toHaveBeenCalledWith( + `${ASSISTANT_API.FEEDBACK}/${mockOutputMessage.interactionId}`, + { + body: JSON.stringify({ satisfaction: correct }), + query: dataSourceServiceMock.getDataSourceQuery(), + } ); expect(result.current.feedbackResult).toBe(correct); }); it('should not update feedback state if API fail', async () => { + const mockInteraction = { + interaction_id: 'interactionId', + } as Interaction; const mockInputMessage = { type: 'input', } as IMessage; @@ -97,7 +99,11 @@ describe('useFeedback hook', () => { chatState: { messages: mockMessages, interactions: [], llmResponding: false }, chatStateDispatch: chatStateDispatchMock, }); - const { result } = renderHook(() => useFeedback()); + + httpMock.put.mockRejectedValueOnce(new Error('API error')); + const { result } = renderHook(() => + useFeedback(mockInteraction, httpMock, dataSourceServiceMock) + ); expect(result.current.feedbackResult).toBe(undefined); const sendFeedback = result.current.sendFeedback; @@ -107,34 +113,4 @@ describe('useFeedback hook', () => { expect(result.current.feedbackResult).toBe(undefined); }); - - it('should call feedback request on registry without checking for input message if hasChatState is false', async () => { - const mockOutputMessage = { - type: 'output', - interactionId: 'interactionId', - } as IOutput; - const mockInteraction = { - interaction_id: 'interactionId', - } as Interaction; - - const { result } = renderHook(() => useFeedback(mockInteraction, false)); - expect(result.current.feedbackResult).toBe(undefined); - - const sendFeedback = result.current.sendFeedback; - await act(async () => { - await sendFeedback(mockOutputMessage, false); - }); - act(() => { - registryMock.on.mock.calls.forEach(([event, handler]) => { - if (event === `feedbackSuccess:${mockOutputMessage.interactionId}`) { - handler({ correct: false }); - } - }); - }); - expect(registryMock.sendFeedbackRequest).toHaveBeenCalledWith( - mockOutputMessage.interactionId, - false - ); - expect(result.current.feedbackResult).toBe(false); - }); }); diff --git a/public/hooks/use_feed_back.tsx b/public/hooks/use_feed_back.tsx index 28a984e5..dde38f83 100644 --- a/public/hooks/use_feed_back.tsx +++ b/public/hooks/use_feed_back.tsx @@ -4,20 +4,28 @@ */ import { useState, useEffect } from 'react'; - -import { IOutput, Interaction } from '../../common/types/chat_saved_object_attributes'; +import { ASSISTANT_API } from '../../common/constants/llm'; +import { + IOutput, + Interaction, + SendFeedbackBody, +} from '../../common/types/chat_saved_object_attributes'; import { useChatState } from './use_chat_state'; -import { getIncontextInsightRegistry } from '../services'; +import { HttpSetup } from '../../../../src/core/public'; +import { DataSourceService } from '../services/data_source_service'; -export const useFeedback = (interaction?: Interaction | null, hasChatState: boolean = true) => { - const registry = getIncontextInsightRegistry(); +export const useFeedback = ( + interaction?: Interaction | null, + httpSetup?: HttpSetup, + dataSourceService?: DataSourceService +) => { const chatStateContext = useChatState(); const [feedbackResult, setFeedbackResult] = useState( interaction?.additional_info?.feedback?.satisfaction ?? undefined ); const sendFeedback = async (message: IOutput, correct: boolean) => { - if (hasChatState && chatStateContext?.chatState) { + if (chatStateContext?.chatState) { const chatState = chatStateContext.chatState; // Markdown type output all has interactionId. The interactionId of message is equal to interaction id. const outputMessageIndex = chatState.messages.findIndex((item) => { @@ -31,20 +39,20 @@ export const useFeedback = (interaction?: Interaction | null, hasChatState: bool return; } } - registry.sendFeedbackRequest(message.interactionId, correct); - }; - useEffect(() => { - const successFeedback = (event: { correct: boolean }) => { - setFeedbackResult(event.correct); + const body: SendFeedbackBody = { + satisfaction: correct, }; - if (interaction && interaction.interaction_id) { - registry.on(`feedbackSuccess:${interaction.interaction_id}`, successFeedback); - return () => { - registry.off(`feedbackSuccess:${interaction.interaction_id}`, successFeedback); - }; + try { + await httpSetup?.put(`${ASSISTANT_API.FEEDBACK}/${message.interactionId}`, { + body: JSON.stringify(body), + query: dataSourceService?.getDataSourceQuery(), + }); + setFeedbackResult(correct); + } catch (error) { + console.error('send feedback error', error); } - }, [interaction]); + }; return { sendFeedback, feedbackResult }; }; diff --git a/public/plugin.tsx b/public/plugin.tsx index a0399e95..3cae59fb 100644 --- a/public/plugin.tsx +++ b/public/plugin.tsx @@ -207,7 +207,14 @@ export class AssistantPlugin renderIncontextInsight: (props: any) => { if (!this.incontextInsightRegistry?.isEnabled()) return
; const httpSetup = core.http; - return ; + const dataSourceService = this.dataSourceService; + return ( + + ); }, }; } diff --git a/public/services/__tests__/incontext_insight_registry.test.ts b/public/services/__tests__/incontext_insight_registry.test.ts index 0427c478..b49c3e54 100644 --- a/public/services/__tests__/incontext_insight_registry.test.ts +++ b/public/services/__tests__/incontext_insight_registry.test.ts @@ -51,21 +51,6 @@ describe('IncontextInsightRegistry', () => { }); }); - it('emits "onSendFeedback" event when sendFeedbackRequest is called', () => { - const mockFn = jest.fn(); - const correct = true; - registry.on('onSendFeedback', mockFn); - - registry.sendFeedbackRequest('test interactionId', correct); - - expect(mockFn).toHaveBeenCalledWith({ - interactionId: 'test interactionId', - body: { - satisfaction: correct, - }, - }); - }); - it('adds item to registry when register is called with a single item', () => { registry.register(insight); diff --git a/public/services/incontext_insight/incontext_insight_registry.ts b/public/services/incontext_insight/incontext_insight_registry.ts index 577e846c..1b35a04e 100644 --- a/public/services/incontext_insight/incontext_insight_registry.ts +++ b/public/services/incontext_insight/incontext_insight_registry.ts @@ -5,12 +5,7 @@ import EventEmitter from 'events'; import { IncontextInsight, IncontextInsights } from '../../types'; -import { - ISuggestedAction, - Interaction, - IOutput, - SendFeedbackBody, -} from '../../../common/types/chat_saved_object_attributes'; +import { ISuggestedAction, Interaction } from '../../../common/types/chat_saved_object_attributes'; export class IncontextInsightRegistry extends EventEmitter { private registry: IncontextInsights = new Map(); @@ -55,21 +50,6 @@ export class IncontextInsightRegistry extends EventEmitter { }); } - public sendFeedbackRequest(interactionId: string | undefined, correct: boolean) { - if (interactionId === undefined || interactionId === '') return; - const body: SendFeedbackBody = { - satisfaction: correct, - }; - this.emit('onSendFeedback', { - interactionId, - body, - }); - } - - public feedbackSuccess(interactionId: string, correct: boolean) { - this.emit(`feedbackSuccess:${interactionId}`, { correct }); - } - public register(item: IncontextInsight | IncontextInsight[]): void; public register(item: unknown) { if (Array.isArray(item)) { diff --git a/public/tabs/chat/messages/message_action.tsx b/public/tabs/chat/messages/message_action.tsx index 61f04ced..c564cda7 100644 --- a/public/tabs/chat/messages/message_action.tsx +++ b/public/tabs/chat/messages/message_action.tsx @@ -7,6 +7,8 @@ import React, { useCallback } from 'react'; import { EuiFlexGroup, EuiFlexItem, EuiButtonIcon, EuiCopy } from '@elastic/eui'; import { IOutput, Interaction } from '../../../../common/types/chat_saved_object_attributes'; import { useFeedback } from '../../../hooks/use_feed_back'; +import { HttpSetup } from '../../../../../../src/core/public'; +import { DataSourceService } from '../../../services/data_source_service'; interface MessageActionsProps { contentToCopy?: string; @@ -20,6 +22,8 @@ interface MessageActionsProps { onViewTrace?: () => void; shouldActionBarVisibleOnHover?: boolean; isFullWidth?: boolean; + httpSetup?: HttpSetup; + dataSourceService?: DataSourceService; } export const MessageActions: React.FC = ({ @@ -34,8 +38,10 @@ export const MessageActions: React.FC = ({ onViewTrace = null, shouldActionBarVisibleOnHover = false, isFullWidth = false, + httpSetup, + dataSourceService, }) => { - const { feedbackResult, sendFeedback } = useFeedback(interaction); + const { feedbackResult, sendFeedback } = useFeedback(interaction, httpSetup, dataSourceService); const handleFeedback = useCallback( (correct: boolean) => { diff --git a/public/tabs/chat/messages/message_bubble.tsx b/public/tabs/chat/messages/message_bubble.tsx index a54c6781..d7157475 100644 --- a/public/tabs/chat/messages/message_bubble.tsx +++ b/public/tabs/chat/messages/message_bubble.tsx @@ -16,6 +16,7 @@ import { import React from 'react'; import { IconType } from '@elastic/eui/src/components/icon/icon'; import { MessageActions } from './message_action'; +import { useCore } from '../../../contexts'; // TODO: Replace with getChrome().logos.Chat.url import { useChatActions } from '../../../hooks'; @@ -44,6 +45,7 @@ type MessageBubbleProps = { export const MessageBubble: React.FC = React.memo((props) => { const { executeAction } = useChatActions(); + const core = useCore(); // According to the design of the feedback, only markdown type output is supported. const showFeedback = @@ -174,6 +176,8 @@ export const MessageBubble: React.FC = React.memo((props) => }} shouldActionBarVisibleOnHover={props.shouldActionBarVisibleOnHover} isFullWidth={fullWidth} + httpSetup={core.services.http} + dataSourceService={core.services.dataSource} /> )} From 15f3d2a74052fc547ec6490909a6638febdcfccc Mon Sep 17 00:00:00 2001 From: Sihan He Date: Mon, 19 Aug 2024 10:48:52 +0800 Subject: [PATCH 05/29] Update CHANGELOG.md Signed-off-by: Sihan He --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b46806e1..041f34de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,4 +24,5 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Reset chat and reload history after data source change ([#194](https://github.com/opensearch-project/dashboards-assistant/pull/194)) - Add experimental feature to support text to visualization ([#218](https://github.com/opensearch-project/dashboards-assistant/pull/218)) - Be compatible with ML configuration index mapping change ([#239](https://github.com/opensearch-project/dashboards-assistant/pull/239)) -- Support context aware alert analysis by reusing incontext insight component([#215](https://github.com/opensearch-project/dashboards-assistant/pull/215)) \ No newline at end of file +- Support context aware alert analysis by reusing incontext insight component([#215](https://github.com/opensearch-project/dashboards-assistant/pull/215)) +- Add the message action bar module and refactor the original code([#253](https://github.com/opensearch-project/dashboards-assistant/pull/253)) \ No newline at end of file From 578d8b82bb71c3234f68d127d705c0c39a2d5b62 Mon Sep 17 00:00:00 2001 From: Sihan He Date: Mon, 19 Aug 2024 11:06:10 +0800 Subject: [PATCH 06/29] Update for smaller buttons Signed-off-by: Sihan He --- public/tabs/chat/messages/message_action.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/public/tabs/chat/messages/message_action.tsx b/public/tabs/chat/messages/message_action.tsx index c564cda7..c880fbfe 100644 --- a/public/tabs/chat/messages/message_action.tsx +++ b/public/tabs/chat/messages/message_action.tsx @@ -4,7 +4,7 @@ */ import React, { useCallback } from 'react'; -import { EuiFlexGroup, EuiFlexItem, EuiButtonIcon, EuiCopy } from '@elastic/eui'; +import { EuiFlexGroup, EuiFlexItem, EuiSmallButtonIcon, EuiCopy } from '@elastic/eui'; import { IOutput, Interaction } from '../../../../common/types/chat_saved_object_attributes'; import { useFeedback } from '../../../hooks/use_feed_back'; import { HttpSetup } from '../../../../../../src/core/public'; @@ -67,7 +67,7 @@ export const MessageActions: React.FC = ({ {(copy) => ( - = ({ )} {showRegenerate && onRegenerate && ( - = ({ <> {feedbackResult !== false && ( - = ({ )} {feedbackResult !== true && ( - = ({ )} {showTraceIcon && traceInteractionId && onViewTrace && ( - Date: Fri, 30 Aug 2024 15:39:01 +0800 Subject: [PATCH 07/29] Update for ui-metric Signed-off-by: Sihan He --- opensearch_dashboards.json | 4 ++- .../generate_popover_body.tsx | 8 ++--- public/components/incontext_insight/index.tsx | 4 +++ public/hooks/use_feed_back.test.tsx | 4 +-- public/hooks/use_feed_back.tsx | 32 ++++++++++++++----- public/plugin.tsx | 3 +- public/tabs/chat/messages/message_action.tsx | 16 +++++++--- public/tabs/chat/messages/message_bubble.tsx | 2 +- public/types.ts | 2 ++ 9 files changed, 54 insertions(+), 21 deletions(-) diff --git a/opensearch_dashboards.json b/opensearch_dashboards.json index 44509c71..b2857036 100644 --- a/opensearch_dashboards.json +++ b/opensearch_dashboards.json @@ -15,7 +15,9 @@ ], "optionalPlugins": [ "dataSource", - "dataSourceManagement" + "dataSourceManagement", + "usageCollection", + "telemetry" ], "requiredBundles": [], "configPath": [ diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index e45d0f2e..d2ebade8 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -22,13 +22,15 @@ import { HttpSetup } from '../../../../../src/core/public'; import { DataSourceService } from '../../services/data_source_service'; import { ASSISTANT_API } from '../../../common/constants/llm'; import { getAssistantRole } from '../../utils/constants'; +import { UsageCollectionSetup } from '../../../../../src/plugins/usage_collection/public'; export const GeneratePopoverBody: React.FC<{ incontextInsight: IncontextInsightInput; httpSetup?: HttpSetup; dataSourceService?: DataSourceService; + usageCollection?: UsageCollectionSetup; closePopover: () => void; -}> = ({ incontextInsight, httpSetup, dataSourceService, closePopover }) => { +}> = ({ incontextInsight, httpSetup, dataSourceService, usageCollection, closePopover }) => { const [isLoading, setIsLoading] = useState(false); const [summary, setSummary] = useState(''); const [conversationId, setConversationId] = useState(''); @@ -146,10 +148,8 @@ export const GeneratePopoverBody: React.FC<{ onRegenerate={async () => onRegenerateSummary(interaction?.interaction_id || '')} interaction={interaction} showFeedback - message={message as IOutput} showTraceIcon={false} - httpSetup={httpSetup} - dataSourceService={dataSourceService} + usageCollection={usageCollection} /> {getConfigSchema().chat.enabled && ( { const anchor = useRef(null); const [isVisible, setIsVisible] = useState(false); @@ -285,6 +288,7 @@ export const IncontextInsight = ({ incontextInsight={input} httpSetup={httpSetup} dataSourceService={dataSourceService} + usageCollection={usageCollection} closePopover={closePopover} /> ); diff --git a/public/hooks/use_feed_back.test.tsx b/public/hooks/use_feed_back.test.tsx index 95bd54d5..6ecab50f 100644 --- a/public/hooks/use_feed_back.test.tsx +++ b/public/hooks/use_feed_back.test.tsx @@ -71,7 +71,7 @@ describe('useFeedback hook', () => { const sendFeedback = result.current.sendFeedback; await act(async () => { - await sendFeedback(mockOutputMessage, correct); + await sendFeedback(correct, mockOutputMessage); }); expect(httpMock.put).toHaveBeenCalledWith( `${ASSISTANT_API.FEEDBACK}/${mockOutputMessage.interactionId}`, @@ -108,7 +108,7 @@ describe('useFeedback hook', () => { const sendFeedback = result.current.sendFeedback; await act(async () => { - await sendFeedback(mockOutputMessage, true); + await sendFeedback(true, mockOutputMessage); }); expect(result.current.feedbackResult).toBe(undefined); diff --git a/public/hooks/use_feed_back.tsx b/public/hooks/use_feed_back.tsx index dde38f83..71e13449 100644 --- a/public/hooks/use_feed_back.tsx +++ b/public/hooks/use_feed_back.tsx @@ -3,7 +3,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { useState, useEffect } from 'react'; +import { useState } from 'react'; +import { v4 as uuidv4 } from 'uuid'; import { ASSISTANT_API } from '../../common/constants/llm'; import { IOutput, @@ -13,23 +14,25 @@ import { import { useChatState } from './use_chat_state'; import { HttpSetup } from '../../../../src/core/public'; import { DataSourceService } from '../services/data_source_service'; +import { UsageCollectionSetup } from '../../../../src/plugins/usage_collection/public'; export const useFeedback = ( interaction?: Interaction | null, httpSetup?: HttpSetup, - dataSourceService?: DataSourceService + dataSourceService?: DataSourceService, + usageCollection?: UsageCollectionSetup ) => { const chatStateContext = useChatState(); const [feedbackResult, setFeedbackResult] = useState( interaction?.additional_info?.feedback?.satisfaction ?? undefined ); - const sendFeedback = async (message: IOutput, correct: boolean) => { + const sendFeedback = async (correct: boolean, message: IOutput | null) => { if (chatStateContext?.chatState) { const chatState = chatStateContext.chatState; // Markdown type output all has interactionId. The interactionId of message is equal to interaction id. const outputMessageIndex = chatState.messages.findIndex((item) => { - return item.type === 'output' && item.interactionId === message.interactionId; + return item.type === 'output' && item.interactionId === message?.interactionId; }); const inputMessage = chatState.messages .slice(0, outputMessageIndex) @@ -40,15 +43,28 @@ export const useFeedback = ( } } + const reportMetric = usageCollection + ? (metric: string) => { + usageCollection.reportUiStats( + `chat`, + usageCollection.METRIC_TYPE.CLICK, + metric + '-' + uuidv4() + ); + } + : () => {}; + const body: SendFeedbackBody = { satisfaction: correct, }; try { - await httpSetup?.put(`${ASSISTANT_API.FEEDBACK}/${message.interactionId}`, { - body: JSON.stringify(body), - query: dataSourceService?.getDataSourceQuery(), - }); + if (message) { + await httpSetup?.put(`${ASSISTANT_API.FEEDBACK}/${message.interactionId}`, { + body: JSON.stringify(body), + query: dataSourceService?.getDataSourceQuery(), + }); + } setFeedbackResult(correct); + reportMetric(correct ? 'thumbup' : 'thumbdown'); } catch (error) { console.error('send feedback error', error); } diff --git a/public/plugin.tsx b/public/plugin.tsx index 3cae59fb..f0b474d0 100644 --- a/public/plugin.tsx +++ b/public/plugin.tsx @@ -149,7 +149,6 @@ export class AssistantPlugin if (this.config.chat.enabled) { const setupChat = async () => { const [coreStart, startDeps] = await core.getStartServices(); - const CoreContext = createOpenSearchDashboardsReactContext({ ...coreStart, setupDeps, @@ -207,12 +206,14 @@ export class AssistantPlugin renderIncontextInsight: (props: any) => { if (!this.incontextInsightRegistry?.isEnabled()) return
; const httpSetup = core.http; + const usageCollection = setupDeps.usageCollection || null; const dataSourceService = this.dataSourceService; return ( ); }, diff --git a/public/tabs/chat/messages/message_action.tsx b/public/tabs/chat/messages/message_action.tsx index c880fbfe..ce312460 100644 --- a/public/tabs/chat/messages/message_action.tsx +++ b/public/tabs/chat/messages/message_action.tsx @@ -9,6 +9,7 @@ import { IOutput, Interaction } from '../../../../common/types/chat_saved_object import { useFeedback } from '../../../hooks/use_feed_back'; import { HttpSetup } from '../../../../../../src/core/public'; import { DataSourceService } from '../../../services/data_source_service'; +import { UsageCollectionSetup } from '../../../../../../src/plugins/usage_collection/public'; interface MessageActionsProps { contentToCopy?: string; @@ -24,6 +25,7 @@ interface MessageActionsProps { isFullWidth?: boolean; httpSetup?: HttpSetup; dataSourceService?: DataSourceService; + usageCollection?: UsageCollectionSetup; } export const MessageActions: React.FC = ({ @@ -31,7 +33,7 @@ export const MessageActions: React.FC = ({ showRegenerate = false, onRegenerate, interaction, - message, + message = null, showFeedback = false, showTraceIcon = false, traceInteractionId = null, @@ -40,15 +42,21 @@ export const MessageActions: React.FC = ({ isFullWidth = false, httpSetup, dataSourceService, + usageCollection, }) => { - const { feedbackResult, sendFeedback } = useFeedback(interaction, httpSetup, dataSourceService); + const { feedbackResult, sendFeedback } = useFeedback( + interaction, + httpSetup, + dataSourceService, + usageCollection + ); const handleFeedback = useCallback( (correct: boolean) => { - if (feedbackResult !== undefined || !message) { + if (feedbackResult !== undefined) { return; } - sendFeedback(message, correct); + sendFeedback(correct, message); }, [feedbackResult, message, sendFeedback] ); diff --git a/public/tabs/chat/messages/message_bubble.tsx b/public/tabs/chat/messages/message_bubble.tsx index d7157475..c27a2480 100644 --- a/public/tabs/chat/messages/message_bubble.tsx +++ b/public/tabs/chat/messages/message_bubble.tsx @@ -46,7 +46,6 @@ type MessageBubbleProps = { export const MessageBubble: React.FC = React.memo((props) => { const { executeAction } = useChatActions(); const core = useCore(); - // According to the design of the feedback, only markdown type output is supported. const showFeedback = 'message' in props && @@ -178,6 +177,7 @@ export const MessageBubble: React.FC = React.memo((props) => isFullWidth={fullWidth} httpSetup={core.services.http} dataSourceService={core.services.dataSource} + usageCollection={core.services.setupDeps.usageCollection} /> )} diff --git a/public/types.ts b/public/types.ts index 538fadff..f541ce64 100644 --- a/public/types.ts +++ b/public/types.ts @@ -16,6 +16,7 @@ import { } from '../../../src/plugins/visualizations/public'; import { DataPublicPluginSetup, DataPublicPluginStart } from '../../../src/plugins/data/public'; import { AppMountParameters, CoreStart } from '../../../src/core/public'; +import { UsageCollectionSetup } from '../../../src/plugins/usage_collection/public'; export interface RenderProps { props: MessageContentProps; @@ -46,6 +47,7 @@ export interface AssistantPluginSetupDependencies { visualizations: VisualizationsSetup; embeddable: EmbeddableSetup; dataSourceManagement?: DataSourceManagementPluginSetup; + usageCollection?: UsageCollectionSetup; } export interface AssistantSetup { From 2e2598554092e5e570c2f6b20089e3a9637948d1 Mon Sep 17 00:00:00 2001 From: Heng Qian Date: Wed, 28 Aug 2024 11:30:47 +0800 Subject: [PATCH 08/29] Support insight with RAG Signed-off-by: Heng Qian --- CHANGELOG.md | 3 +- common/constants/llm.ts | 5 + public/assets/shiny_sparkle.svg | 32 +++ public/assets/sparkle.svg | 11 + .../generate_popover_body.test.tsx | 212 ++++++++------- .../generate_popover_body.tsx | 256 ++++++++++++------ .../components/incontext_insight/index.scss | 67 ++++- public/components/incontext_insight/index.tsx | 90 +++--- public/types.ts | 7 +- server/plugin.ts | 2 + server/routes/get_agent.ts | 30 ++ server/routes/summary_routes.ts | 118 ++++++++ server/types.ts | 14 + 13 files changed, 628 insertions(+), 219 deletions(-) create mode 100644 public/assets/shiny_sparkle.svg create mode 100644 public/assets/sparkle.svg create mode 100644 server/routes/summary_routes.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e74ad30..19d60dbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,4 +25,5 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Add experimental feature to support text to visualization ([#218](https://github.com/opensearch-project/dashboards-assistant/pull/218)) - Be compatible with ML configuration index mapping change ([#239](https://github.com/opensearch-project/dashboards-assistant/pull/239)) - Support context aware alert analysis by reusing incontext insight component([#215](https://github.com/opensearch-project/dashboards-assistant/pull/215)) -Use smaller and compressed variants of buttons and form components ([#250](https://github.com/opensearch-project/dashboards-assistant/pull/250)) \ No newline at end of file +Use smaller and compressed variants of buttons and form components ([#250](https://github.com/opensearch-project/dashboards-assistant/pull/250)) +- Support insight with RAG in alert analysis and refine the UX diff --git a/common/constants/llm.ts b/common/constants/llm.ts index 63ac5a59..bfc623ad 100644 --- a/common/constants/llm.ts +++ b/common/constants/llm.ts @@ -24,6 +24,11 @@ export const TEXT2VIZ_API = { TEXT2VEGA: `${API_BASE}/text2vega`, }; +export const SUMMARY_ASSISTANT_API = { + SUMMARIZE: `${API_BASE}/summary`, + INSIGHT: `${API_BASE}/insight`, +}; + export const NOTEBOOK_API = { CREATE_NOTEBOOK: `${NOTEBOOK_PREFIX}/note`, SET_PARAGRAPH: `${NOTEBOOK_PREFIX}/set_paragraphs/`, diff --git a/public/assets/shiny_sparkle.svg b/public/assets/shiny_sparkle.svg new file mode 100644 index 00000000..1aeadce0 --- /dev/null +++ b/public/assets/shiny_sparkle.svg @@ -0,0 +1,32 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/public/assets/sparkle.svg b/public/assets/sparkle.svg new file mode 100644 index 00000000..885e5c63 --- /dev/null +++ b/public/assets/sparkle.svg @@ -0,0 +1,11 @@ + + + + + + + + + + + diff --git a/public/components/incontext_insight/generate_popover_body.test.tsx b/public/components/incontext_insight/generate_popover_body.test.tsx index 5001e1c7..75a6478f 100644 --- a/public/components/incontext_insight/generate_popover_body.test.tsx +++ b/public/components/incontext_insight/generate_popover_body.test.tsx @@ -8,7 +8,7 @@ import { render, cleanup, fireEvent, waitFor } from '@testing-library/react'; import { getConfigSchema, getNotifications } from '../../services'; import { GeneratePopoverBody } from './generate_popover_body'; import { HttpSetup } from '../../../../../src/core/public'; -import { ASSISTANT_API } from '../../../common/constants/llm'; +import { SUMMARY_ASSISTANT_API } from '../../../common/constants/llm'; jest.mock('../../services'); @@ -42,25 +42,28 @@ describe('GeneratePopoverBody', () => { const closePopoverMock = jest.fn(); - it('renders the generate summary button', () => { - const { getByText } = render( - - ); - - expect(getByText('Generate summary')).toBeInTheDocument(); - }); - - it('calls onGenerateSummary when button is clicked', async () => { - mockPost.mockResolvedValue({ - interactions: [{ conversation_id: 'test-conversation' }], - messages: [{ type: 'output', content: 'Generated summary content' }], + it('auto generates summary and insight', async () => { + mockPost.mockImplementation((path: string, body) => { + let value; + switch (path) { + case SUMMARY_ASSISTANT_API.SUMMARIZE: + value = { + summary: 'Generated summary content', + insightAgentId: 'insight_agent_id', + }; + break; + + case SUMMARY_ASSISTANT_API.INSIGHT: + value = 'Generated insight content'; + break; + + default: + return null; + } + return Promise.resolve(value); }); - const { getByText } = render( + const { getByText, getByLabelText, queryByText, queryByLabelText } = render( { /> ); - const button = getByText('Generate summary'); - fireEvent.click(button); + // 1. Auto generate summary + // title is assistant icon + 'Summary' + expect(getByLabelText('alert-assistant')).toBeInTheDocument(); + expect(getByText('Summary')).toBeInTheDocument(); + // content is loading + expect(getByLabelText('loading_content')).toBeInTheDocument(); // Wait for loading to complete and summary to render await waitFor(() => { expect(getByText('Generated summary content')).toBeInTheDocument(); }); - - expect(mockPost).toHaveBeenCalledWith(ASSISTANT_API.SEND_MESSAGE, expect.any(Object)); + // loading content disappeared + expect(queryByLabelText('loading_content')).toBeNull(); + expect(mockPost).toHaveBeenCalledWith(SUMMARY_ASSISTANT_API.SUMMARIZE, expect.any(Object)); expect(mockToasts.addDanger).not.toHaveBeenCalled(); - }); - - it('shows loading state while generating summary', async () => { - const { getByText } = render( - - ); - const button = getByText('Generate summary'); - fireEvent.click(button); + // insight tip icon is visible + const insightTipIcon = getByLabelText('Insight'); + expect(insightTipIcon).toBeInTheDocument(); - // Wait for loading state to appear - expect(getByText('Generating summary...')).toBeInTheDocument(); - }); - - it('handles error during summary generation', async () => { - mockPost.mockRejectedValue(new Error('Network Error')); - - const { getByText } = render( - - ); - - const button = getByText('Generate summary'); - fireEvent.click(button); + // 2. Click insight tip icon to view insight + fireEvent.click(insightTipIcon); + // title is back button + 'Insight With RAG' + const backButton = getByLabelText('back-to-summary'); + expect(backButton).toBeInTheDocument(); + expect(getByText('Insight With RAG')).toBeInTheDocument(); + // Wait for loading to complete and insight to render await waitFor(() => { - expect(mockToasts.addDanger).toHaveBeenCalledWith('Generate summary error'); + expect(getByText('Generated insight content')).toBeInTheDocument(); }); + expect(queryByText('Generated summary content')).toBeNull(); + + // loading content disappeared + expect(queryByLabelText('loading_content')).toBeNull(); + expect(mockPost).toHaveBeenCalledWith(SUMMARY_ASSISTANT_API.INSIGHT, expect.any(Object)); + expect(mockToasts.addDanger).not.toHaveBeenCalled(); + + // 3. Click back button to view summary + fireEvent.click(backButton); + expect(queryByText('Generated insight content')).toBeNull(); + expect(queryByText('Generated summary content')).toBeInTheDocument(); }); - it('renders the continue in chat button after summary is generated', async () => { - mockPost.mockResolvedValue({ - interactions: [{ conversation_id: 'test-conversation' }], - messages: [{ type: 'output', content: 'Generated summary content' }], + it('auto generates summary without insight agent id', async () => { + mockPost.mockImplementation((path: string, body) => { + let value; + switch (path) { + case SUMMARY_ASSISTANT_API.SUMMARIZE: + value = { + summary: 'Generated summary content', + insightAgentId: undefined, + }; + break; + + case SUMMARY_ASSISTANT_API.INSIGHT: + value = 'Generated insight content'; + break; + + default: + return null; + } + return Promise.resolve(value); }); - const { getByText } = render( + const { getByText, getByLabelText, queryByLabelText } = render( { /> ); - const button = getByText('Generate summary'); - fireEvent.click(button); + // title is assistant icon + 'Summary' + expect(getByLabelText('alert-assistant')).toBeInTheDocument(); + expect(getByText('Summary')).toBeInTheDocument(); + // content is loading + expect(getByLabelText('loading_content')).toBeInTheDocument(); - // Wait for the summary to be displayed + // Wait for loading to complete and summary to render await waitFor(() => { expect(getByText('Generated summary content')).toBeInTheDocument(); }); + // loading content disappeared + expect(queryByLabelText('loading_content')).toBeNull(); + expect(mockPost).toHaveBeenCalledWith(SUMMARY_ASSISTANT_API.SUMMARIZE, expect.any(Object)); + expect(mockToasts.addDanger).not.toHaveBeenCalled(); - // Check for continue in chat button - expect(getByText('Continue in chat')).toBeInTheDocument(); + // insight tip icon is not visible + expect(queryByLabelText('Insight')).toBeNull(); + // Only call http post 1 time. + expect(mockPost).toHaveBeenCalledTimes(1); }); - it('calls onChatContinuation when continue in chat button is clicked', async () => { - mockPost.mockResolvedValue({ - interactions: [{ conversation_id: 'test-conversation' }], - messages: [{ type: 'output', content: 'Generated summary content' }], - }); + it('handles error during summary generation', async () => { + mockPost.mockRejectedValue(new Error('Network Error')); - const { getByText } = render( + const { queryByText } = render( ); - const button = getByText('Generate summary'); - fireEvent.click(button); + // Auto close popover window if error occurs + expect(queryByText('test-generated-popover')).toBeNull(); await waitFor(() => { - expect(getByText('Generated summary content')).toBeInTheDocument(); + expect(mockToasts.addDanger).toHaveBeenCalledWith('Generate summary error'); }); - - const continueButton = getByText('Continue in chat'); - fireEvent.click(continueButton); - - expect(mockPost).toHaveBeenCalledTimes(1); - expect(closePopoverMock).toHaveBeenCalled(); }); - it("continue in chat button doesn't appear when chat is disabled", async () => { - mockPost.mockResolvedValue({ - interactions: [{ conversation_id: 'test-conversation' }], - messages: [{ type: 'output', content: 'Generated summary content' }], - }); - (getConfigSchema as jest.Mock).mockReturnValue({ - chat: { enabled: false }, + it('handles error during insight generation', async () => { + mockPost.mockImplementation((path: string, body) => { + let value; + switch (path) { + case SUMMARY_ASSISTANT_API.SUMMARIZE: + value = { + summary: 'Generated summary content', + insightAgentId: 'insight_agent_id', + }; + break; + + case SUMMARY_ASSISTANT_API.INSIGHT: + return Promise.reject(new Error('Network Error')); + + default: + return null; + } + return Promise.resolve(value); }); - const { getByText, queryByText } = render( + const { getByText, queryByLabelText } = render( ); - const button = getByText('Generate summary'); - fireEvent.click(button); - + expect(getByText('Summary')).toBeInTheDocument(); + // Wait for loading to complete and summary to render await waitFor(() => { - expect(getByText('Generated summary content')).toBeInTheDocument(); + expect(mockToasts.addDanger).toHaveBeenCalledWith('Generate insight error'); }); - - expect(queryByText('Continue in chat')).toBeNull(); - expect(mockPost).toHaveBeenCalledTimes(1); + // Show summary content although insight generation failed + expect(getByText('Generated summary content')).toBeInTheDocument(); + // insight tip icon is not visible for this alert + expect(queryByLabelText('Insight')).toBeNull(); }); }); diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index 2a9db358..22171fe0 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -6,74 +6,87 @@ import React, { useState } from 'react'; import { i18n } from '@osd/i18n'; import { - EuiButton, + EuiBadge, + EuiButtonEmpty, EuiFlexGroup, EuiFlexItem, - EuiIcon, + EuiIconTip, + EuiLoadingContent, + EuiMarkdownFormat, EuiPanel, + EuiPopoverFooter, + EuiPopoverTitle, EuiSpacer, EuiText, } from '@elastic/eui'; +import { useEffectOnce } from 'react-use'; import { IncontextInsight as IncontextInsightInput } from '../../types'; -import { getConfigSchema, getIncontextInsightRegistry, getNotifications } from '../../services'; +import { getNotifications } from '../../services'; import { HttpSetup } from '../../../../../src/core/public'; -import { ASSISTANT_API } from '../../../common/constants/llm'; -import { getAssistantRole } from '../../utils/constants'; +import { SUMMARY_ASSISTANT_API } from '../../../common/constants/llm'; +import shiny_sparkle from '../../assets/shiny_sparkle.svg'; export const GeneratePopoverBody: React.FC<{ incontextInsight: IncontextInsightInput; httpSetup?: HttpSetup; closePopover: () => void; }> = ({ incontextInsight, httpSetup, closePopover }) => { - const [isLoading, setIsLoading] = useState(false); const [summary, setSummary] = useState(''); - const [conversationId, setConversationId] = useState(''); + const [insight, setInsight] = useState(''); + const [insightAvailable, setInsightAvailable] = useState(false); + const [showInsight, setShowInsight] = useState(false); const toasts = getNotifications().toasts; - const registry = getIncontextInsightRegistry(); - const onChatContinuation = () => { - registry?.continueInChat(incontextInsight, conversationId); - closePopover(); - }; + useEffectOnce(() => { + onGenerateSummary( + incontextInsight.suggestions && incontextInsight.suggestions.length > 0 + ? incontextInsight.suggestions[0] + : 'Please summarize the input' + ); + }); const onGenerateSummary = (summarizationQuestion: string) => { - setIsLoading(true); - setSummary(''); - setConversationId(''); const summarize = async () => { - const contextContent = incontextInsight.contextProvider + const contextObj = incontextInsight.contextProvider ? await incontextInsight.contextProvider() - : ''; - let incontextInsightType: string; + : undefined; + const contextContent = contextObj?.context || ''; + let summaryType: string; const endIndex = incontextInsight.key.indexOf('_', 0); if (endIndex !== -1) { - incontextInsightType = incontextInsight.key.substring(0, endIndex); + summaryType = incontextInsight.key.substring(0, endIndex); } else { - incontextInsightType = incontextInsight.key; + summaryType = incontextInsight.key; } + const insightType = + summaryType === 'alerts' + ? contextObj?.additionalInfo.monitorType === 'cluster_metrics_monitor' + ? 'os_insight' + : 'user_insight' + : undefined; await httpSetup - ?.post(ASSISTANT_API.SEND_MESSAGE, { + ?.post(SUMMARY_ASSISTANT_API.SUMMARIZE, { body: JSON.stringify({ - messages: [], - input: { - type: 'input', - content: summarizationQuestion, - contentType: 'text', - context: { content: contextContent, dataSourceId: incontextInsight.datasourceId }, - promptPrefix: getAssistantRole(incontextInsightType), - }, + type: summaryType, + insightType, + question: summarizationQuestion, + context: contextContent, }), }) .then((response) => { - const interactionLength = response.interactions.length; - if (interactionLength > 0) { - setConversationId(response.interactions[interactionLength - 1].conversation_id); - } - - const messageLength = response.messages.length; - if (messageLength > 0 && response.messages[messageLength - 1].type === 'output') { - setSummary(response.messages[messageLength - 1].content); + const summaryContent = response.summary; + setSummary(summaryContent); + const insightAgentIdExists = !!response.insightAgentId; + setInsightAvailable(insightAgentIdExists); + if (insightAgentIdExists) { + onGenerateInsightBasedOnSummary( + response.insightAgentId, + summaryType, + summaryContent, + contextContent, + 'Please provide your insight on this alert to help users understand this alert, find potential causes and give feasible solutions to address this alert' + ); } }) .catch((error) => { @@ -82,65 +95,144 @@ export const GeneratePopoverBody: React.FC<{ defaultMessage: 'Generate summary error', }) ); - }) - .finally(() => { - setIsLoading(false); + closePopover(); }); }; return summarize(); }; - return summary ? ( - <> - - {summary} - - - {getConfigSchema().chat.enabled && ( + const onGenerateInsightBasedOnSummary = ( + insightAgentId: string, + insightType: string, + summaryContent: string, + context: string, + insightQuestion: string + ) => { + const generateInsight = async () => { + httpSetup + ?.post(SUMMARY_ASSISTANT_API.INSIGHT, { + body: JSON.stringify({ + insightAgentId, + insightType, + summary: summaryContent, + context, + question: insightQuestion, + }), + }) + .then((response) => { + setInsight(response); + }) + .catch((error) => { + toasts.addDanger( + i18n.translate('assistantDashboards.incontextInsight.generateSummaryError', { + defaultMessage: 'Generate insight error', + }) + ); + setInsightAvailable(false); + setShowInsight(false); + }); + }; + + return generateInsight(); + }; + + const renderContent = () => { + const content = showInsight && insightAvailable ? insight : summary; + return content ? ( + <> onChatContinuation()} - grow={false} - paddingSize="none" - style={{ width: '120px', float: 'right' }} + color="subdued" > - + + {content} + + + + + ) : ( + + ); + }; + + const renderInnerTitle = () => { + return ( + + {showInsight ? ( + - + { + setShowInsight(false); + }} + iconType="arrowLeft" + iconSide={'left'} + color={'text'} + > + {i18n.translate('assistantDashboards.incontextInsight.InsightWithRAG', { + defaultMessage: 'Insight With RAG', + })} + + + ) : ( + - - {i18n.translate('assistantDashboards.incontextInsight.continueInChat', { - defaultMessage: 'Continue in chat', - })} - +
+ + {i18n.translate('assistantDashboards.incontextInsight.Summary', { + defaultMessage: 'Summary', + })} + +
- - )} + )} + + ); + }; + + const renderInnerFooter = () => { + return ( + + + {insightAvailable && ( + { + setShowInsight(true); + }} + > + + + )} + + + ); + }; + + return ( + <> + {renderInnerTitle()} + {renderContent()} + {renderInnerFooter()} - ) : ( - { - await onGenerateSummary( - incontextInsight.suggestions && incontextInsight.suggestions.length > 0 - ? incontextInsight.suggestions[0] - : 'Please summarize the input' - ); - }} - isLoading={isLoading} - disabled={isLoading} - > - {isLoading - ? i18n.translate('assistantDashboards.incontextInsight.generatingSummary', { - defaultMessage: 'Generating summary...', - }) - : i18n.translate('assistantDashboards.incontextInsight.generateSummary', { - defaultMessage: 'Generate summary', - })} - ); }; diff --git a/public/components/incontext_insight/index.scss b/public/components/incontext_insight/index.scss index 914f9b94..42813465 100644 --- a/public/components/incontext_insight/index.scss +++ b/public/components/incontext_insight/index.scss @@ -107,10 +107,75 @@ } .incontextInsightPopoverBody { - max-width: 400px; + max-width: 486px; + min-width: 486px; width: 100%; } +.incontextInsightGeneratePopoverTitle { + // TODO: Remove this one paddingSize is fixed + padding: 0 !important; + margin-bottom: 0 !important; + min-height: 30px; + max-height: 30px; + border: none; + + :first-child { + border: none; + } + + .euiBadge__text { + text-transform: none; + font-weight: bold; + font-size: 15px; + } + + .euiBadge__icon { + margin: 2px 0 2px 0; + } + + .euiIcon--small { + height: 100%; + width: 22px; + padding: 2px 0; + } + + .euiButtonEmpty{ + padding-inline: 8px; + + .euiButtonEmpty__content { + margin-block: 2px; + padding-block: 2px; + + .euiIcon--small { + height: 100%; + width: 16px; + padding: 2px 0; + } + + .euiButtonEmpty__text { + margin-inline-start: 4px; + text-transform: none; + font-weight: bold; + font-size: 15px; + } + } + } +} + +.incontextInsightGeneratePopoverFooter{ + padding-block: 0 !important; + margin-top: 0 !important; + margin-bottom: 0 !important; + border: none; +} + +.incontextInsightGeneratePopoverContent { + display: flex; + overflow: auto; + max-height: 300px; +} + .incontextInsightSummary { border: $euiBorderThin; border-radius: 4px; diff --git a/public/components/incontext_insight/index.tsx b/public/components/incontext_insight/index.tsx index add28340..feaa87cf 100644 --- a/public/components/incontext_insight/index.tsx +++ b/public/components/incontext_insight/index.tsx @@ -7,29 +7,30 @@ import './index.scss'; import { i18n } from '@osd/i18n'; import { - EuiWrappingPopover, - EuiSmallButton, + EuiBadge, EuiCompressedFieldText, + EuiCompressedFormRow, EuiFlexGroup, EuiFlexItem, - EuiCompressedFormRow, - EuiPopoverTitle, - EuiText, - EuiPopoverFooter, - EuiBadge, - EuiSpacer, + EuiIcon, EuiListGroup, EuiListGroupItem, EuiPanel, - keys, - EuiIcon, + EuiPopoverFooter, + EuiPopoverTitle, + EuiSmallButton, EuiSmallButtonIcon, + EuiSpacer, + EuiText, + EuiWrappingPopover, + keys, } from '@elastic/eui'; import React, { Children, isValidElement, useEffect, useRef, useState } from 'react'; import { IncontextInsight as IncontextInsightInput } from '../../types'; import { getIncontextInsightRegistry, getNotifications } from '../../services'; // TODO: Replace with getChrome().logos.Chat.url import chatIcon from '../../assets/chat.svg'; +import sparkle from '../../assets/sparkle.svg'; import { HttpSetup } from '../../../../../src/core/public'; import { GeneratePopoverBody } from './generate_popover_body'; @@ -260,7 +261,7 @@ export const IncontextInsight = ({ children, httpSetup }: IncontextInsightProps)
- +
@@ -305,37 +306,42 @@ export const IncontextInsight = ({ children, httpSetup }: IncontextInsightProps) offset={6} panelPaddingSize="s" > - - - -
- - {i18n.translate('assistantDashboards.incontextInsight.assistant', { - defaultMessage: 'OpenSearch Assistant', - })} - -
- - -
- -
- - - + { + // For 'generate' type insights, we don't want to show this title but its own inner title + input.type !== 'generate' && ( + + + +
+ + {i18n.translate('assistantDashboards.incontextInsight.assistant', { + defaultMessage: 'OpenSearch Assistant', + })} + +
+ + +
+ +
+ + + + ) + }
{popoverBody()}
); diff --git a/public/types.ts b/public/types.ts index 538fadff..fa1020b8 100644 --- a/public/types.ts +++ b/public/types.ts @@ -84,13 +84,18 @@ export interface ChatConfig { export type IncontextInsights = Map; +export interface ContextObj { + context: string; + additionalInfo: Record; +} + export interface IncontextInsight { key: string; type?: IncontextInsightType; summary?: string; suggestions?: string[]; interactionId?: string; - contextProvider?: () => Promise; + contextProvider?: () => Promise; datasourceId?: string; } diff --git a/server/plugin.ts b/server/plugin.ts index 48777819..c74033a9 100644 --- a/server/plugin.ts +++ b/server/plugin.ts @@ -17,6 +17,7 @@ import { BasicInputOutputParser } from './parsers/basic_input_output_parser'; import { VisualizationCardParser } from './parsers/visualization_card_parser'; import { registerChatRoutes } from './routes/chat_routes'; import { registerText2VizRoutes } from './routes/text2viz_routes'; +import { registerSummaryAssistantRoutes } from './routes/summary_routes'; export class AssistantPlugin implements Plugin { private readonly logger: Logger; @@ -51,6 +52,7 @@ export class AssistantPlugin implements Plugin ({ diff --git a/server/routes/get_agent.ts b/server/routes/get_agent.ts index 76cdd467..76b2bbc1 100644 --- a/server/routes/get_agent.ts +++ b/server/routes/get_agent.ts @@ -26,3 +26,33 @@ export const getAgent = async (id: string, client: OpenSearchClient['transport'] throw new Error(`get agent ${id} failed, reason: ${errorMessage}`); } }; + +export const searchAgentByName = async (name: string, client: OpenSearchClient['transport']) => { + try { + const requestParams = { + query: { + term: { + 'name.keyword': name, + }, + }, + sort: { + created_time: 'desc', + }, + size: 1, + }; + + const response = await client.request({ + method: 'GET', + path: `${ML_COMMONS_BASE_API}/agents/_search`, + body: requestParams, + }); + + if (!response || response.body.hits.total.value === 0) { + throw new Error(`cannot find any agent by name: ${name}`); + } + return response.body.hits.hits[0]._id; + } catch (error) { + const errorMessage = JSON.stringify(error.meta?.body) || error; + throw new Error(`search ${name} agent failed, reason: ` + errorMessage); + } +}; diff --git a/server/routes/summary_routes.ts b/server/routes/summary_routes.ts new file mode 100644 index 00000000..c5a8ea60 --- /dev/null +++ b/server/routes/summary_routes.ts @@ -0,0 +1,118 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { schema } from '@osd/config-schema'; +import { IRouter } from '../../../../src/core/server'; +import { SUMMARY_ASSISTANT_API } from '../../common/constants/llm'; +import { getOpenSearchClientTransport } from '../utils/get_opensearch_client_transport'; +import { getAgent, searchAgentByName } from './get_agent'; +import { ML_COMMONS_BASE_API } from '../utils/constants'; +import { InsightType, SummaryType } from '../types'; + +const SUMMARY_AGENT_CONFIG_ID = 'summary'; + +export function registerSummaryAssistantRoutes(router: IRouter) { + router.post( + { + path: SUMMARY_ASSISTANT_API.SUMMARIZE, + validate: { + body: schema.object({ + type: schema.string(), + insightType: schema.maybe(schema.string()), + question: schema.string(), + context: schema.maybe(schema.string()), + }), + query: schema.object({ + dataSourceId: schema.maybe(schema.string()), + }), + }, + }, + router.handleLegacyErrors(async (context, req, res) => { + const client = await getOpenSearchClientTransport({ + context, + dataSourceId: req.query.dataSourceId, + }); + const agentId = await getAgent(SUMMARY_AGENT_CONFIG_ID, client); + const prompt = SummaryType.find((type) => type.id === req.body.type)?.prompt; + const response = await client.request({ + method: 'POST', + path: `${ML_COMMONS_BASE_API}/agents/${agentId}/_execute`, + body: { + parameters: { + prompt, + context: req.body.context, + question: req.body.question, + }, + }, + }); + let summary; + let insightAgentId; + try { + if (req.body.insightType) { + // We have separate agent for os_insight and user_insight. And for user_insight, we can + // only get it by searching on name since it is not stored in agent config. + if (req.body.insightType === 'os_insight') { + insightAgentId = await getAgent(req.body.insightType, client); + } else if (req.body.insightType === 'user_insight') { + if (req.body.type === 'alerts') { + insightAgentId = await searchAgentByName('KB_For_Alert_Insight', client); + } + } + } + } catch (e) { + console.log(`Cannot find insight agent for ${req.body.insightType}`); + } + try { + summary = response.body.inference_results[0].output[0].result; + return res.ok({ body: { summary, insightAgentId } }); + } catch (e) { + return res.internalError(); + } + }) + ); + router.post( + { + path: SUMMARY_ASSISTANT_API.INSIGHT, + validate: { + body: schema.object({ + insightAgentId: schema.string(), + insightType: schema.string(), + summary: schema.string(), + context: schema.string(), + question: schema.string(), + }), + query: schema.object({ + dataSourceId: schema.maybe(schema.string()), + }), + }, + }, + router.handleLegacyErrors(async (context, req, res) => { + const client = await getOpenSearchClientTransport({ + context, + dataSourceId: req.query.dataSourceId, + }); + const prompt = InsightType.find((type) => type.id === req.body.insightType)?.prompt; + const response = await client.request({ + method: 'POST', + path: `${ML_COMMONS_BASE_API}/agents/${req.body.insightAgentId}/_execute`, + body: { + parameters: { + text: prompt, + context: req.body.context, + summary: req.body.summary, + question: req.body.question, + }, + }, + }); + try { + let result = response.body.inference_results[0].output[0].result; + result = JSON.parse(result).output.text; + return res.ok({ body: result }); + } catch (e) { + return res.internalError(); + } + }) + ); +} diff --git a/server/types.ts b/server/types.ts index a47b42bd..5130c888 100644 --- a/server/types.ts +++ b/server/types.ts @@ -50,3 +50,17 @@ declare module '../../../src/core/server' { }; } } + +export const SummaryType = [ + { + id: 'alerts', + prompt: `You are an OpenSearch Alert Assistant to help summarize the alerts. Here is the detail of alert: $\{parameters.context}; The question is $\{parameters.question}`, + }, +]; + +export const InsightType = [ + { + id: 'alerts', + prompt: `You are an OpenSearch Alert Assistant to provide your insight on this alert. Here is the detail of alert: $\{parameters.context}; The alert summary is $\{parameters.summary}; The question is $\{parameters.question}`, + }, +]; From 21e2278fd94b5a606e76450a6d2da6fbf3e97928 Mon Sep 17 00:00:00 2001 From: Heng Qian Date: Fri, 30 Aug 2024 16:15:58 +0800 Subject: [PATCH 09/29] Add change in CHANGELOG.md Signed-off-by: Heng Qian --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 19d60dbb..e2af8ca0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,4 +26,4 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Be compatible with ML configuration index mapping change ([#239](https://github.com/opensearch-project/dashboards-assistant/pull/239)) - Support context aware alert analysis by reusing incontext insight component([#215](https://github.com/opensearch-project/dashboards-assistant/pull/215)) Use smaller and compressed variants of buttons and form components ([#250](https://github.com/opensearch-project/dashboards-assistant/pull/250)) -- Support insight with RAG in alert analysis and refine the UX +- Support insight with RAG in alert analysis assistant and refine the UX ([#266](https://github.com/opensearch-project/dashboards-assistant/pull/266)) From 662923bd928755edbcf68928c67cae23df7fd698 Mon Sep 17 00:00:00 2001 From: Heng Qian Date: Fri, 30 Aug 2024 17:14:15 +0800 Subject: [PATCH 10/29] Put footer in the same panel with content to match UX design Signed-off-by: Heng Qian --- .../incontext_insight/generate_popover_body.tsx | 14 ++++---------- public/components/incontext_insight/index.scss | 3 +-- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index 22171fe0..7116521e 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -141,18 +141,13 @@ export const GeneratePopoverBody: React.FC<{ const content = showInsight && insightAvailable ? insight : summary; return content ? ( <> - - + + {content} + + {renderInnerFooter()} - ) : ( @@ -232,7 +227,6 @@ export const GeneratePopoverBody: React.FC<{ <> {renderInnerTitle()} {renderContent()} - {renderInnerFooter()} ); }; diff --git a/public/components/incontext_insight/index.scss b/public/components/incontext_insight/index.scss index 42813465..08282529 100644 --- a/public/components/incontext_insight/index.scss +++ b/public/components/incontext_insight/index.scss @@ -165,8 +165,7 @@ .incontextInsightGeneratePopoverFooter{ padding-block: 0 !important; - margin-top: 0 !important; - margin-bottom: 0 !important; + margin: 0 !important; border: none; } From 3ba34824c634edf48484a7f38055988502dae7e0 Mon Sep 17 00:00:00 2001 From: Heng Qian Date: Mon, 2 Sep 2024 10:29:35 +0800 Subject: [PATCH 11/29] Refine alert prompt Signed-off-by: Heng Qian --- public/components/incontext_insight/generate_popover_body.tsx | 2 +- server/types.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index 7116521e..9a255a18 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -85,7 +85,7 @@ export const GeneratePopoverBody: React.FC<{ summaryType, summaryContent, contextContent, - 'Please provide your insight on this alert to help users understand this alert, find potential causes and give feasible solutions to address this alert' + `Please provide your insight on this ${summaryType}.` ); } }) diff --git a/server/types.ts b/server/types.ts index 5130c888..1894728a 100644 --- a/server/types.ts +++ b/server/types.ts @@ -54,13 +54,13 @@ declare module '../../../src/core/server' { export const SummaryType = [ { id: 'alerts', - prompt: `You are an OpenSearch Alert Assistant to help summarize the alerts. Here is the detail of alert: $\{parameters.context}; The question is $\{parameters.question}`, + prompt: `You are an OpenSearch Alert Assistant to help summarize the alerts.\n Here is the detail of alert: $\{parameters.context};\n The question is: $\{parameters.question}`, }, ]; export const InsightType = [ { id: 'alerts', - prompt: `You are an OpenSearch Alert Assistant to provide your insight on this alert. Here is the detail of alert: $\{parameters.context}; The alert summary is $\{parameters.summary}; The question is $\{parameters.question}`, + prompt: `You are an OpenSearch Alert Assistant to provide your insight on this alert to help users understand the alert, find potential causes and give feasible solutions to address it.\n Here is the detail of alert: $\{parameters.context};\n The alert summary is: $\{parameters.summary};\n The question is: $\{parameters.question}`, }, ]; From 1956d4973abedcf0880f7d0810bc77a23c246737 Mon Sep 17 00:00:00 2001 From: Heng Qian Date: Mon, 2 Sep 2024 11:06:50 +0800 Subject: [PATCH 12/29] set CSS scrollbar-width to thin Signed-off-by: Heng Qian --- public/components/incontext_insight/index.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/public/components/incontext_insight/index.scss b/public/components/incontext_insight/index.scss index 08282529..20bc523d 100644 --- a/public/components/incontext_insight/index.scss +++ b/public/components/incontext_insight/index.scss @@ -173,6 +173,7 @@ display: flex; overflow: auto; max-height: 300px; + scrollbar-width: thin; } .incontextInsightSummary { From 14f9030daec1606750474b8651fbcc1ac5892d4f Mon Sep 17 00:00:00 2001 From: Heng Qian Date: Mon, 2 Sep 2024 17:21:05 +0800 Subject: [PATCH 13/29] Hide insight agent id from front-end Signed-off-by: Heng Qian --- .../generate_popover_body.tsx | 8 +++--- server/routes/summary_routes.ts | 28 ++++++++++++++----- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index 9a255a18..f77bd2e6 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -77,12 +77,12 @@ export const GeneratePopoverBody: React.FC<{ .then((response) => { const summaryContent = response.summary; setSummary(summaryContent); - const insightAgentIdExists = !!response.insightAgentId; + const insightAgentIdExists = insightType !== undefined && response.insightAgentIdExists; setInsightAvailable(insightAgentIdExists); if (insightAgentIdExists) { onGenerateInsightBasedOnSummary( - response.insightAgentId, summaryType, + insightType, summaryContent, contextContent, `Please provide your insight on this ${summaryType}.` @@ -103,7 +103,7 @@ export const GeneratePopoverBody: React.FC<{ }; const onGenerateInsightBasedOnSummary = ( - insightAgentId: string, + summaryType: string, insightType: string, summaryContent: string, context: string, @@ -113,7 +113,7 @@ export const GeneratePopoverBody: React.FC<{ httpSetup ?.post(SUMMARY_ASSISTANT_API.INSIGHT, { body: JSON.stringify({ - insightAgentId, + summaryType, insightType, summary: summaryContent, context, diff --git a/server/routes/summary_routes.ts b/server/routes/summary_routes.ts index c5a8ea60..bf69bb77 100644 --- a/server/routes/summary_routes.ts +++ b/server/routes/summary_routes.ts @@ -12,6 +12,9 @@ import { ML_COMMONS_BASE_API } from '../utils/constants'; import { InsightType, SummaryType } from '../types'; const SUMMARY_AGENT_CONFIG_ID = 'summary'; +const OS_INSIGHT_AGENT_CONFIG_ID = 'os_insight'; +let osInsightAgentId: string | undefined; +let userInsightAgentId: string | undefined; export function registerSummaryAssistantRoutes(router: IRouter) { router.post( @@ -48,17 +51,23 @@ export function registerSummaryAssistantRoutes(router: IRouter) { }, }); let summary; - let insightAgentId; + let insightAgentIdExists = false; try { if (req.body.insightType) { // We have separate agent for os_insight and user_insight. And for user_insight, we can // only get it by searching on name since it is not stored in agent config. if (req.body.insightType === 'os_insight') { - insightAgentId = await getAgent(req.body.insightType, client); + if (!osInsightAgentId) { + osInsightAgentId = await getAgent(OS_INSIGHT_AGENT_CONFIG_ID, client); + } + insightAgentIdExists = !!osInsightAgentId; } else if (req.body.insightType === 'user_insight') { if (req.body.type === 'alerts') { - insightAgentId = await searchAgentByName('KB_For_Alert_Insight', client); + if (!userInsightAgentId) { + userInsightAgentId = await searchAgentByName('KB_For_Alert_Insight', client); + } } + insightAgentIdExists = !!userInsightAgentId; } } } catch (e) { @@ -66,7 +75,7 @@ export function registerSummaryAssistantRoutes(router: IRouter) { } try { summary = response.body.inference_results[0].output[0].result; - return res.ok({ body: { summary, insightAgentId } }); + return res.ok({ body: { summary, insightAgentIdExists } }); } catch (e) { return res.internalError(); } @@ -77,7 +86,7 @@ export function registerSummaryAssistantRoutes(router: IRouter) { path: SUMMARY_ASSISTANT_API.INSIGHT, validate: { body: schema.object({ - insightAgentId: schema.string(), + summaryType: schema.string(), insightType: schema.string(), summary: schema.string(), context: schema.string(), @@ -93,10 +102,12 @@ export function registerSummaryAssistantRoutes(router: IRouter) { context, dataSourceId: req.query.dataSourceId, }); - const prompt = InsightType.find((type) => type.id === req.body.insightType)?.prompt; + const prompt = InsightType.find((type) => type.id === req.body.summaryType)?.prompt; + const insightAgentId = + req.body.insightType === 'os_insight' ? osInsightAgentId : userInsightAgentId; const response = await client.request({ method: 'POST', - path: `${ML_COMMONS_BASE_API}/agents/${req.body.insightAgentId}/_execute`, + path: `${ML_COMMONS_BASE_API}/agents/${insightAgentId}/_execute`, body: { parameters: { text: prompt, @@ -112,6 +123,9 @@ export function registerSummaryAssistantRoutes(router: IRouter) { return res.ok({ body: result }); } catch (e) { return res.internalError(); + } finally { + // Reset userInsightAgentId in case users update their insight agent. + userInsightAgentId = undefined; } }) ); From f799e154dbf002f7c28460df4e47cf6d41dd97cc Mon Sep 17 00:00:00 2001 From: Heng Qian Date: Tue, 3 Sep 2024 10:26:15 +0800 Subject: [PATCH 14/29] Change summary agent config id Signed-off-by: Heng Qian --- server/routes/summary_routes.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/routes/summary_routes.ts b/server/routes/summary_routes.ts index bf69bb77..03f26ab8 100644 --- a/server/routes/summary_routes.ts +++ b/server/routes/summary_routes.ts @@ -11,7 +11,7 @@ import { getAgent, searchAgentByName } from './get_agent'; import { ML_COMMONS_BASE_API } from '../utils/constants'; import { InsightType, SummaryType } from '../types'; -const SUMMARY_AGENT_CONFIG_ID = 'summary'; +const SUMMARY_AGENT_CONFIG_ID = 'os_general_llm'; const OS_INSIGHT_AGENT_CONFIG_ID = 'os_insight'; let osInsightAgentId: string | undefined; let userInsightAgentId: string | undefined; From d2278f1bd335f0d93757d91436555f1e33615786 Mon Sep 17 00:00:00 2001 From: Heng Qian Date: Wed, 4 Sep 2024 11:12:48 +0800 Subject: [PATCH 15/29] Address comments Signed-off-by: Heng Qian --- CHANGELOG.md | 2 +- server/routes/get_agent.ts | 3 ++- server/routes/summary_routes.ts | 4 +++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e2af8ca0..94bb2021 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,5 +25,5 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Add experimental feature to support text to visualization ([#218](https://github.com/opensearch-project/dashboards-assistant/pull/218)) - Be compatible with ML configuration index mapping change ([#239](https://github.com/opensearch-project/dashboards-assistant/pull/239)) - Support context aware alert analysis by reusing incontext insight component([#215](https://github.com/opensearch-project/dashboards-assistant/pull/215)) -Use smaller and compressed variants of buttons and form components ([#250](https://github.com/opensearch-project/dashboards-assistant/pull/250)) +- Use smaller and compressed variants of buttons and form components ([#250](https://github.com/opensearch-project/dashboards-assistant/pull/250)) - Support insight with RAG in alert analysis assistant and refine the UX ([#266](https://github.com/opensearch-project/dashboards-assistant/pull/266)) diff --git a/server/routes/get_agent.ts b/server/routes/get_agent.ts index 76b2bbc1..993a0dd7 100644 --- a/server/routes/get_agent.ts +++ b/server/routes/get_agent.ts @@ -35,6 +35,7 @@ export const searchAgentByName = async (name: string, client: OpenSearchClient[' 'name.keyword': name, }, }, + _source: ['_id'], sort: { created_time: 'desc', }, @@ -48,7 +49,7 @@ export const searchAgentByName = async (name: string, client: OpenSearchClient[' }); if (!response || response.body.hits.total.value === 0) { - throw new Error(`cannot find any agent by name: ${name}`); + return undefined; } return response.body.hits.hits[0]._id; } catch (error) { diff --git a/server/routes/summary_routes.ts b/server/routes/summary_routes.ts index 03f26ab8..b98f6ce3 100644 --- a/server/routes/summary_routes.ts +++ b/server/routes/summary_routes.ts @@ -71,7 +71,9 @@ export function registerSummaryAssistantRoutes(router: IRouter) { } } } catch (e) { - console.log(`Cannot find insight agent for ${req.body.insightType}`); + context.assistant_plugin.logger.info( + `Cannot find insight agent for ${req.body.insightType}` + ); } try { summary = response.body.inference_results[0].output[0].result; From f2516bba128a2ec7492373ece0eb078b1c333d30 Mon Sep 17 00:00:00 2001 From: Sihan He Date: Wed, 4 Sep 2024 11:41:29 +0800 Subject: [PATCH 16/29] Incorporate message action buttons into alerting summary Signed-off-by: Sihan He --- .../generate_popover_body.tsx | 55 ++---- public/tabs/chat/messages/message_action.tsx | 165 +++++++++++------- public/tabs/chat/messages/message_bubble.tsx | 62 +++---- 3 files changed, 141 insertions(+), 141 deletions(-) diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index 24de1393..da42d758 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -10,7 +10,6 @@ import { EuiButtonEmpty, EuiFlexGroup, EuiFlexItem, - EuiIconTip, EuiLoadingContent, EuiMarkdownFormat, EuiPanel, @@ -20,7 +19,6 @@ import { EuiText, } from '@elastic/eui'; import { useEffectOnce } from 'react-use'; -import { IMessage, Interaction, IOutput } from 'common/types/chat_saved_object_attributes'; import { MessageActions } from '../../tabs/chat/messages/message_action'; import { IncontextInsight as IncontextInsightInput } from '../../types'; import { getNotifications } from '../../services'; @@ -28,8 +26,6 @@ import { HttpSetup } from '../../../../../src/core/public'; import { SUMMARY_ASSISTANT_API } from '../../../common/constants/llm'; import shiny_sparkle from '../../assets/shiny_sparkle.svg'; import { DataSourceService } from '../../services/data_source_service'; -import { ASSISTANT_API } from '../../../common/constants/llm'; -import { getAssistantRole } from '../../utils/constants'; import { UsageCollectionSetup } from '../../../../../src/plugins/usage_collection/public'; export const GeneratePopoverBody: React.FC<{ @@ -54,21 +50,6 @@ export const GeneratePopoverBody: React.FC<{ ); }); - // // eslint-disable-next-line @typescript-eslint/no-explicit-any - // const handleSummaryResponse = (response: any) => { - // const interactionLength = response.interactions.length; - // if (interactionLength > 0) { - // setConversationId(response.interactions[interactionLength - 1].conversation_id); - // setInteraction(response.interactions[interactionLength - 1]); - // } - - // const messageLength = response.messages.length; - // if (messageLength > 0 && response.messages[messageLength - 1].type === 'output') { - // setSummary(response.messages[messageLength - 1].content); - // setMessage(response.messages[messageLength - 1]); - // } - // }; - const onGenerateSummary = (summarizationQuestion: string) => { const summarize = async () => { const contextObj = incontextInsight.contextProvider @@ -226,32 +207,16 @@ export const GeneratePopoverBody: React.FC<{ const renderInnerFooter = () => { return ( - - {/* onRegenerateSummary(interaction?.interaction_id || '')} - interaction={interaction} - showFeedback - showTraceIcon={false} - usageCollection={usageCollection} - /> */} - {insightAvailable && ( - { - setShowInsight(true); - }} - > - - - )} - + { + setShowInsight(true); + }} + usageCollection={usageCollection} + isOnTrace={showInsight} + /> ); }; diff --git a/public/tabs/chat/messages/message_action.tsx b/public/tabs/chat/messages/message_action.tsx index ce312460..b36c64ed 100644 --- a/public/tabs/chat/messages/message_action.tsx +++ b/public/tabs/chat/messages/message_action.tsx @@ -4,7 +4,8 @@ */ import React, { useCallback } from 'react'; -import { EuiFlexGroup, EuiFlexItem, EuiSmallButtonIcon, EuiCopy } from '@elastic/eui'; +import { EuiFlexGroup, EuiFlexItem, EuiSmallButtonIcon, EuiCopy, EuiToolTip } from '@elastic/eui'; +import { i18n } from '@osd/i18n'; import { IOutput, Interaction } from '../../../../common/types/chat_saved_object_attributes'; import { useFeedback } from '../../../hooks/use_feed_back'; import { HttpSetup } from '../../../../../../src/core/public'; @@ -19,6 +20,7 @@ interface MessageActionsProps { message?: IOutput | null; showFeedback?: boolean; showTraceIcon?: boolean; + isOnTrace?: boolean; traceInteractionId?: string; onViewTrace?: () => void; shouldActionBarVisibleOnHover?: boolean; @@ -26,8 +28,11 @@ interface MessageActionsProps { httpSetup?: HttpSetup; dataSourceService?: DataSourceService; usageCollection?: UsageCollectionSetup; + buttonOrder?: string[]; } +type ButtonKey = 'copy' | 'regenerate' | 'thumbUp' | 'thumbDown' | 'trace'; + export const MessageActions: React.FC = ({ contentToCopy = '', showRegenerate = false, @@ -36,13 +41,15 @@ export const MessageActions: React.FC = ({ message = null, showFeedback = false, showTraceIcon = false, + isOnTrace = false, traceInteractionId = null, - onViewTrace = null, + onViewTrace, shouldActionBarVisibleOnHover = false, isFullWidth = false, httpSetup, dataSourceService, usageCollection, + buttonOrder = ['trace', 'regenerate', 'thumbUp', 'thumbDown', 'copy'], }) => { const { feedbackResult, sendFeedback } = useFeedback( interaction, @@ -61,77 +68,105 @@ export const MessageActions: React.FC = ({ [feedbackResult, message, sendFeedback] ); + const renderButtonWithTooltip = (content: string, button: JSX.Element, key: string) => ( + + {button} + + ); + + const feedbackTip = + 'Thank you for the feedback. Try again by adjusting your prompt so that I have the opportunity to better assist you.'; + + const buttonConfigs = { + copy: { + show: !isFullWidth, + component: renderButtonWithTooltip( + 'Copy to clipboard', + + {(copy) => } + , + 'copy' + ), + }, + regenerate: { + show: showRegenerate && onRegenerate, + component: renderButtonWithTooltip( + 'Regenerate message', + , + 'regenerate' + ), + }, + thumbUp: { + show: showFeedback && feedbackResult !== false, + component: renderButtonWithTooltip( + feedbackResult === true ? feedbackTip : 'Good response', + handleFeedback(true)} + />, + 'thumbUp' + ), + }, + thumbDown: { + show: showFeedback && feedbackResult !== true, + component: renderButtonWithTooltip( + feedbackResult === false ? feedbackTip : 'Bad response', + handleFeedback(false)} + />, + 'thumbDown' + ), + }, + trace: { + show: showTraceIcon && onViewTrace, + component: renderButtonWithTooltip( + 'Insight with RAG', + , + 'trace' + ), + }, + }; + return ( - {!isFullWidth && ( - - - {(copy) => ( - - )} - - - )} - {showRegenerate && onRegenerate && ( - - - - )} - {showFeedback && ( - <> - {feedbackResult !== false && ( - - handleFeedback(true)} - /> - - )} - {feedbackResult !== true && ( - - handleFeedback(false)} - /> + {buttonOrder.map( + (key) => + buttonConfigs[key as ButtonKey].show && ( + + {buttonConfigs[key as ButtonKey].component} - )} - - )} - {showTraceIcon && traceInteractionId && onViewTrace && ( - - - + ) )} ); diff --git a/public/tabs/chat/messages/message_bubble.tsx b/public/tabs/chat/messages/message_bubble.tsx index c27a2480..e3b0144a 100644 --- a/public/tabs/chat/messages/message_bubble.tsx +++ b/public/tabs/chat/messages/message_bubble.tsx @@ -149,38 +149,38 @@ export const MessageBubble: React.FC = React.memo((props) => className="llm-chat-bubble-panel llm-chat-bubble-panel-output" > {props.children} + {props.showActionBar && ( + <> + + props.onRegenerate?.(props.interaction?.interaction_id || '')} + interaction={props.interaction} + message={props.message as IOutput} + showFeedback={showFeedback} + showTraceIcon={!!props.message.interactionId} + traceInteractionId={props.message.interactionId || ''} + onViewTrace={() => { + const message = props.message as IOutput; + const viewTraceAction: ISuggestedAction = { + actionType: 'view_trace', + metadata: { + interactionId: message.interactionId || '', + }, + message: 'How was this generated?', + }; + executeAction(viewTraceAction, message); + }} + shouldActionBarVisibleOnHover={props.shouldActionBarVisibleOnHover} + isFullWidth={fullWidth} + httpSetup={core.services.http} + dataSourceService={core.services.dataSource} + usageCollection={core.services.setupDeps.usageCollection} + /> + + )} - {props.showActionBar && ( - <> - - props.onRegenerate?.(props.interaction?.interaction_id || '')} - interaction={props.interaction} - message={props.message as IOutput} - showFeedback={showFeedback} - showTraceIcon={!!props.message.interactionId} - traceInteractionId={props.message.interactionId || ''} - onViewTrace={() => { - const message = props.message as IOutput; - const viewTraceAction: ISuggestedAction = { - actionType: 'view_trace', - metadata: { - interactionId: message.interactionId || '', - }, - message: 'How was this generated?', - }; - executeAction(viewTraceAction, message); - }} - shouldActionBarVisibleOnHover={props.shouldActionBarVisibleOnHover} - isFullWidth={fullWidth} - httpSetup={core.services.http} - dataSourceService={core.services.dataSource} - usageCollection={core.services.setupDeps.usageCollection} - /> - - )} ); From d0f227d7b0454ab988167de2cd9cc24a7e18f5e4 Mon Sep 17 00:00:00 2001 From: Sihan He Date: Wed, 4 Sep 2024 11:53:03 +0800 Subject: [PATCH 17/29] Add hover tips and rearrange buttons Signed-off-by: Sihan He --- public/tabs/chat/messages/message_action.tsx | 165 +++++++++++-------- public/tabs/chat/messages/message_bubble.tsx | 62 +++---- 2 files changed, 131 insertions(+), 96 deletions(-) diff --git a/public/tabs/chat/messages/message_action.tsx b/public/tabs/chat/messages/message_action.tsx index ce312460..b36c64ed 100644 --- a/public/tabs/chat/messages/message_action.tsx +++ b/public/tabs/chat/messages/message_action.tsx @@ -4,7 +4,8 @@ */ import React, { useCallback } from 'react'; -import { EuiFlexGroup, EuiFlexItem, EuiSmallButtonIcon, EuiCopy } from '@elastic/eui'; +import { EuiFlexGroup, EuiFlexItem, EuiSmallButtonIcon, EuiCopy, EuiToolTip } from '@elastic/eui'; +import { i18n } from '@osd/i18n'; import { IOutput, Interaction } from '../../../../common/types/chat_saved_object_attributes'; import { useFeedback } from '../../../hooks/use_feed_back'; import { HttpSetup } from '../../../../../../src/core/public'; @@ -19,6 +20,7 @@ interface MessageActionsProps { message?: IOutput | null; showFeedback?: boolean; showTraceIcon?: boolean; + isOnTrace?: boolean; traceInteractionId?: string; onViewTrace?: () => void; shouldActionBarVisibleOnHover?: boolean; @@ -26,8 +28,11 @@ interface MessageActionsProps { httpSetup?: HttpSetup; dataSourceService?: DataSourceService; usageCollection?: UsageCollectionSetup; + buttonOrder?: string[]; } +type ButtonKey = 'copy' | 'regenerate' | 'thumbUp' | 'thumbDown' | 'trace'; + export const MessageActions: React.FC = ({ contentToCopy = '', showRegenerate = false, @@ -36,13 +41,15 @@ export const MessageActions: React.FC = ({ message = null, showFeedback = false, showTraceIcon = false, + isOnTrace = false, traceInteractionId = null, - onViewTrace = null, + onViewTrace, shouldActionBarVisibleOnHover = false, isFullWidth = false, httpSetup, dataSourceService, usageCollection, + buttonOrder = ['trace', 'regenerate', 'thumbUp', 'thumbDown', 'copy'], }) => { const { feedbackResult, sendFeedback } = useFeedback( interaction, @@ -61,77 +68,105 @@ export const MessageActions: React.FC = ({ [feedbackResult, message, sendFeedback] ); + const renderButtonWithTooltip = (content: string, button: JSX.Element, key: string) => ( + + {button} + + ); + + const feedbackTip = + 'Thank you for the feedback. Try again by adjusting your prompt so that I have the opportunity to better assist you.'; + + const buttonConfigs = { + copy: { + show: !isFullWidth, + component: renderButtonWithTooltip( + 'Copy to clipboard', + + {(copy) => } + , + 'copy' + ), + }, + regenerate: { + show: showRegenerate && onRegenerate, + component: renderButtonWithTooltip( + 'Regenerate message', + , + 'regenerate' + ), + }, + thumbUp: { + show: showFeedback && feedbackResult !== false, + component: renderButtonWithTooltip( + feedbackResult === true ? feedbackTip : 'Good response', + handleFeedback(true)} + />, + 'thumbUp' + ), + }, + thumbDown: { + show: showFeedback && feedbackResult !== true, + component: renderButtonWithTooltip( + feedbackResult === false ? feedbackTip : 'Bad response', + handleFeedback(false)} + />, + 'thumbDown' + ), + }, + trace: { + show: showTraceIcon && onViewTrace, + component: renderButtonWithTooltip( + 'Insight with RAG', + , + 'trace' + ), + }, + }; + return ( - {!isFullWidth && ( - - - {(copy) => ( - - )} - - - )} - {showRegenerate && onRegenerate && ( - - - - )} - {showFeedback && ( - <> - {feedbackResult !== false && ( - - handleFeedback(true)} - /> - - )} - {feedbackResult !== true && ( - - handleFeedback(false)} - /> + {buttonOrder.map( + (key) => + buttonConfigs[key as ButtonKey].show && ( + + {buttonConfigs[key as ButtonKey].component} - )} - - )} - {showTraceIcon && traceInteractionId && onViewTrace && ( - - - + ) )} ); diff --git a/public/tabs/chat/messages/message_bubble.tsx b/public/tabs/chat/messages/message_bubble.tsx index c27a2480..e3b0144a 100644 --- a/public/tabs/chat/messages/message_bubble.tsx +++ b/public/tabs/chat/messages/message_bubble.tsx @@ -149,38 +149,38 @@ export const MessageBubble: React.FC = React.memo((props) => className="llm-chat-bubble-panel llm-chat-bubble-panel-output" > {props.children} + {props.showActionBar && ( + <> + + props.onRegenerate?.(props.interaction?.interaction_id || '')} + interaction={props.interaction} + message={props.message as IOutput} + showFeedback={showFeedback} + showTraceIcon={!!props.message.interactionId} + traceInteractionId={props.message.interactionId || ''} + onViewTrace={() => { + const message = props.message as IOutput; + const viewTraceAction: ISuggestedAction = { + actionType: 'view_trace', + metadata: { + interactionId: message.interactionId || '', + }, + message: 'How was this generated?', + }; + executeAction(viewTraceAction, message); + }} + shouldActionBarVisibleOnHover={props.shouldActionBarVisibleOnHover} + isFullWidth={fullWidth} + httpSetup={core.services.http} + dataSourceService={core.services.dataSource} + usageCollection={core.services.setupDeps.usageCollection} + /> + + )} - {props.showActionBar && ( - <> - - props.onRegenerate?.(props.interaction?.interaction_id || '')} - interaction={props.interaction} - message={props.message as IOutput} - showFeedback={showFeedback} - showTraceIcon={!!props.message.interactionId} - traceInteractionId={props.message.interactionId || ''} - onViewTrace={() => { - const message = props.message as IOutput; - const viewTraceAction: ISuggestedAction = { - actionType: 'view_trace', - metadata: { - interactionId: message.interactionId || '', - }, - message: 'How was this generated?', - }; - executeAction(viewTraceAction, message); - }} - shouldActionBarVisibleOnHover={props.shouldActionBarVisibleOnHover} - isFullWidth={fullWidth} - httpSetup={core.services.http} - dataSourceService={core.services.dataSource} - usageCollection={core.services.setupDeps.usageCollection} - /> - - )} ); From a0e1a8908f1d196e2b1b0aea24ccae2a1d216e22 Mon Sep 17 00:00:00 2001 From: Heng Qian Date: Wed, 4 Sep 2024 17:21:30 +0800 Subject: [PATCH 18/29] Fix UT Signed-off-by: Heng Qian --- .../incontext_insight/generate_popover_body.test.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/public/components/incontext_insight/generate_popover_body.test.tsx b/public/components/incontext_insight/generate_popover_body.test.tsx index 75a6478f..1085a862 100644 --- a/public/components/incontext_insight/generate_popover_body.test.tsx +++ b/public/components/incontext_insight/generate_popover_body.test.tsx @@ -37,7 +37,7 @@ describe('GeneratePopoverBody', () => { contextProvider: jest.fn(), suggestions: ['Test summarization question'], datasourceId: 'test-datasource', - key: 'test-key', + key: 'alerts', }; const closePopoverMock = jest.fn(); @@ -49,7 +49,7 @@ describe('GeneratePopoverBody', () => { case SUMMARY_ASSISTANT_API.SUMMARIZE: value = { summary: 'Generated summary content', - insightAgentId: 'insight_agent_id', + insightAgentIdExists: true, }; break; @@ -122,7 +122,7 @@ describe('GeneratePopoverBody', () => { case SUMMARY_ASSISTANT_API.SUMMARIZE: value = { summary: 'Generated summary content', - insightAgentId: undefined, + insightAgentIdExists: false, }; break; @@ -192,7 +192,7 @@ describe('GeneratePopoverBody', () => { case SUMMARY_ASSISTANT_API.SUMMARIZE: value = { summary: 'Generated summary content', - insightAgentId: 'insight_agent_id', + insightAgentIdExists: true, }; break; From 600f9f034eb6d9d0225a0f7011adede6d163822a Mon Sep 17 00:00:00 2001 From: Sihan He Date: Thu, 5 Sep 2024 11:47:43 +0800 Subject: [PATCH 19/29] Add ui-metric for alerting summary Signed-off-by: Sihan He --- .../incontext_insight/generate_popover_body.tsx | 14 ++++++++++++++ public/hooks/use_feed_back.tsx | 5 +++-- public/tabs/chat/messages/message_action.tsx | 5 ++++- public/tabs/chat/messages/message_bubble.tsx | 1 + 4 files changed, 22 insertions(+), 3 deletions(-) diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index da42d758..8b22e14a 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -5,6 +5,7 @@ import React, { useState } from 'react'; import { i18n } from '@osd/i18n'; +import { v4 as uuidv4 } from 'uuid'; import { EuiBadge, EuiButtonEmpty, @@ -39,9 +40,20 @@ export const GeneratePopoverBody: React.FC<{ const [insight, setInsight] = useState(''); const [insightAvailable, setInsightAvailable] = useState(false); const [showInsight, setShowInsight] = useState(false); + const metricAppName = 'alertSumm'; const toasts = getNotifications().toasts; + const reportGeneratedMetric = usageCollection + ? (metric: string) => { + usageCollection.reportUiStats( + metricAppName, + usageCollection.METRIC_TYPE.CLICK, + metric + '-' + uuidv4() + ); + } + : () => {}; + useEffectOnce(() => { onGenerateSummary( incontextInsight.suggestions && incontextInsight.suggestions.length > 0 @@ -93,6 +105,7 @@ export const GeneratePopoverBody: React.FC<{ `Please provide your insight on this ${summaryType}.` ); } + reportGeneratedMetric('generated'); }) .catch((error) => { toasts.addDanger( @@ -216,6 +229,7 @@ export const GeneratePopoverBody: React.FC<{ }} usageCollection={usageCollection} isOnTrace={showInsight} + metricAppName={metricAppName} /> ); diff --git a/public/hooks/use_feed_back.tsx b/public/hooks/use_feed_back.tsx index 71e13449..dd78b1dc 100644 --- a/public/hooks/use_feed_back.tsx +++ b/public/hooks/use_feed_back.tsx @@ -20,7 +20,8 @@ export const useFeedback = ( interaction?: Interaction | null, httpSetup?: HttpSetup, dataSourceService?: DataSourceService, - usageCollection?: UsageCollectionSetup + usageCollection?: UsageCollectionSetup, + metricAppName: string = 'chat' ) => { const chatStateContext = useChatState(); const [feedbackResult, setFeedbackResult] = useState( @@ -46,7 +47,7 @@ export const useFeedback = ( const reportMetric = usageCollection ? (metric: string) => { usageCollection.reportUiStats( - `chat`, + metricAppName, usageCollection.METRIC_TYPE.CLICK, metric + '-' + uuidv4() ); diff --git a/public/tabs/chat/messages/message_action.tsx b/public/tabs/chat/messages/message_action.tsx index b36c64ed..be3a235a 100644 --- a/public/tabs/chat/messages/message_action.tsx +++ b/public/tabs/chat/messages/message_action.tsx @@ -28,6 +28,7 @@ interface MessageActionsProps { httpSetup?: HttpSetup; dataSourceService?: DataSourceService; usageCollection?: UsageCollectionSetup; + metricAppName?: string; buttonOrder?: string[]; } @@ -49,13 +50,15 @@ export const MessageActions: React.FC = ({ httpSetup, dataSourceService, usageCollection, + metricAppName = 'chat', buttonOrder = ['trace', 'regenerate', 'thumbUp', 'thumbDown', 'copy'], }) => { const { feedbackResult, sendFeedback } = useFeedback( interaction, httpSetup, dataSourceService, - usageCollection + usageCollection, + metricAppName ); const handleFeedback = useCallback( diff --git a/public/tabs/chat/messages/message_bubble.tsx b/public/tabs/chat/messages/message_bubble.tsx index e3b0144a..d86997b1 100644 --- a/public/tabs/chat/messages/message_bubble.tsx +++ b/public/tabs/chat/messages/message_bubble.tsx @@ -177,6 +177,7 @@ export const MessageBubble: React.FC = React.memo((props) => httpSetup={core.services.http} dataSourceService={core.services.dataSource} usageCollection={core.services.setupDeps.usageCollection} + metricAppName="chat" /> )} From 5f68dfa92c0a8a3baefedeef8f0f4ab04c823f15 Mon Sep 17 00:00:00 2001 From: Sihan He Date: Thu, 5 Sep 2024 12:29:35 +0800 Subject: [PATCH 20/29] fix testing bugs Signed-off-by: Sihan He --- public/tabs/chat/messages/message_action.test.tsx | 4 ++-- public/tabs/chat/messages/message_action.tsx | 9 ++++++++- public/tabs/chat/messages/message_bubble.test.tsx | 14 +++++++------- public/tabs/chat/messages/message_bubble.tsx | 2 +- 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/public/tabs/chat/messages/message_action.test.tsx b/public/tabs/chat/messages/message_action.test.tsx index b8442af9..1c7b8fe5 100644 --- a/public/tabs/chat/messages/message_action.test.tsx +++ b/public/tabs/chat/messages/message_action.test.tsx @@ -64,10 +64,10 @@ describe('MessageActions', () => { const thumbsDownButton = screen.getByLabelText('feedback thumbs down'); fireEvent.click(thumbsUpButton); - expect(sendFeedback).toHaveBeenCalledWith(message, true); + expect(sendFeedback).toHaveBeenCalledWith(true, message); fireEvent.click(thumbsDownButton); - expect(sendFeedback).toHaveBeenCalledWith(message, false); + expect(sendFeedback).toHaveBeenCalledWith(false, message); }); it('should render trace icon and call onViewTrace function when clicked', () => { diff --git a/public/tabs/chat/messages/message_action.tsx b/public/tabs/chat/messages/message_action.tsx index b36c64ed..143540d2 100644 --- a/public/tabs/chat/messages/message_action.tsx +++ b/public/tabs/chat/messages/message_action.tsx @@ -89,7 +89,14 @@ export const MessageActions: React.FC = ({ component: renderButtonWithTooltip( 'Copy to clipboard', - {(copy) => } + {(copy) => ( + + )} , 'copy' ), diff --git a/public/tabs/chat/messages/message_bubble.test.tsx b/public/tabs/chat/messages/message_bubble.test.tsx index 29ebc526..e3723c00 100644 --- a/public/tabs/chat/messages/message_bubble.test.tsx +++ b/public/tabs/chat/messages/message_bubble.test.tsx @@ -102,7 +102,7 @@ describe('', () => { }} /> ); - expect(screen.queryAllByTitle('copy message')).toHaveLength(1); + expect(screen.queryAllByLabelText('copy message')).toHaveLength(1); }); it('should NOT display action(copy message) on non-text output', () => { @@ -117,7 +117,7 @@ describe('', () => { }} /> ); - expect(screen.queryAllByTitle('copy message')).toHaveLength(0); + expect(screen.queryAllByLabelText('copy message')).toHaveLength(0); rerender( ', () => { }} /> ); - expect(screen.queryAllByTitle('copy message')).toHaveLength(0); + expect(screen.queryAllByLabelText('copy message')).toHaveLength(0); }); it('should display action: regenerate message', () => { @@ -152,7 +152,7 @@ describe('', () => { }} /> ); - expect(screen.queryAllByTitle('regenerate message')).toHaveLength(1); + expect(screen.queryAllByLabelText('regenerate message')).toHaveLength(1); }); it('should NOT display action: regenerate message', () => { @@ -167,7 +167,7 @@ describe('', () => { }} /> ); - expect(screen.queryAllByTitle('regenerate message')).toHaveLength(0); + expect(screen.queryAllByLabelText('regenerate message')).toHaveLength(0); }); it('should display actions: thumbs up and thumbs down on markdown output', () => { @@ -208,7 +208,7 @@ describe('', () => { }; render(); fireEvent.click(screen.getByLabelText('feedback thumbs up')); - expect(sendFeedbackMock).toHaveBeenCalledWith(message, true); + expect(sendFeedbackMock).toHaveBeenCalledWith(true, message); }); it('should send thumbs down feedback', () => { @@ -219,7 +219,7 @@ describe('', () => { }; render(); fireEvent.click(screen.getByLabelText('feedback thumbs down')); - expect(sendFeedbackMock).toHaveBeenCalledWith(message, false); + expect(sendFeedbackMock).toHaveBeenCalledWith(false, message); }); it('should not send feedback if message has already rated', () => { diff --git a/public/tabs/chat/messages/message_bubble.tsx b/public/tabs/chat/messages/message_bubble.tsx index e3b0144a..91ae7aa6 100644 --- a/public/tabs/chat/messages/message_bubble.tsx +++ b/public/tabs/chat/messages/message_bubble.tsx @@ -176,7 +176,7 @@ export const MessageBubble: React.FC = React.memo((props) => isFullWidth={fullWidth} httpSetup={core.services.http} dataSourceService={core.services.dataSource} - usageCollection={core.services.setupDeps.usageCollection} + usageCollection={core.services.setupDeps?.usageCollection} /> )} From 15d44c359592146c8eb317faeaaf3e72b48ba9a4 Mon Sep 17 00:00:00 2001 From: Heng Qian Date: Mon, 9 Sep 2024 10:53:22 +0800 Subject: [PATCH 21/29] Change agent execute API Signed-off-by: Heng Qian --- server/plugin.ts | 2 +- server/routes/summary_routes.ts | 45 ++++++++++++++++----------------- 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/server/plugin.ts b/server/plugin.ts index de546344..ee60b04a 100644 --- a/server/plugin.ts +++ b/server/plugin.ts @@ -59,7 +59,7 @@ export class AssistantPlugin implements Plugin ({ diff --git a/server/routes/summary_routes.ts b/server/routes/summary_routes.ts index b98f6ce3..90687bf7 100644 --- a/server/routes/summary_routes.ts +++ b/server/routes/summary_routes.ts @@ -10,13 +10,17 @@ import { getOpenSearchClientTransport } from '../utils/get_opensearch_client_tra import { getAgent, searchAgentByName } from './get_agent'; import { ML_COMMONS_BASE_API } from '../utils/constants'; import { InsightType, SummaryType } from '../types'; +import { AssistantServiceSetup } from '../services/assistant_service'; const SUMMARY_AGENT_CONFIG_ID = 'os_general_llm'; const OS_INSIGHT_AGENT_CONFIG_ID = 'os_insight'; let osInsightAgentId: string | undefined; let userInsightAgentId: string | undefined; -export function registerSummaryAssistantRoutes(router: IRouter) { +export function registerSummaryAssistantRoutes( + router: IRouter, + assistantService: AssistantServiceSetup +) { router.post( { path: SUMMARY_ASSISTANT_API.SUMMARIZE, @@ -37,18 +41,12 @@ export function registerSummaryAssistantRoutes(router: IRouter) { context, dataSourceId: req.query.dataSourceId, }); - const agentId = await getAgent(SUMMARY_AGENT_CONFIG_ID, client); const prompt = SummaryType.find((type) => type.id === req.body.type)?.prompt; - const response = await client.request({ - method: 'POST', - path: `${ML_COMMONS_BASE_API}/agents/${agentId}/_execute`, - body: { - parameters: { - prompt, - context: req.body.context, - question: req.body.question, - }, - }, + const assistantClient = assistantService.getScopedClient(req, context); + const response = await assistantClient.executeAgentByName(SUMMARY_AGENT_CONFIG_ID, { + prompt, + context: req.body.context, + question: req.body.question, }); let summary; let insightAgentIdExists = false; @@ -107,17 +105,18 @@ export function registerSummaryAssistantRoutes(router: IRouter) { const prompt = InsightType.find((type) => type.id === req.body.summaryType)?.prompt; const insightAgentId = req.body.insightType === 'os_insight' ? osInsightAgentId : userInsightAgentId; - const response = await client.request({ - method: 'POST', - path: `${ML_COMMONS_BASE_API}/agents/${insightAgentId}/_execute`, - body: { - parameters: { - text: prompt, - context: req.body.context, - summary: req.body.summary, - question: req.body.question, - }, - }, + if (!insightAgentId) { + context.assistant_plugin.logger.info( + `Cannot find insight agent for ${req.body.insightType}` + ); + return res.internalError(); + } + const assistantClient = assistantService.getScopedClient(req, context); + const response = await assistantClient.executeAgent(insightAgentId, { + text: prompt, + context: req.body.context, + summary: req.body.summary, + question: req.body.question, }); try { let result = response.body.inference_results[0].output[0].result; From 14d6d0bcd009fb831a0e2b003792266ccc68504c Mon Sep 17 00:00:00 2001 From: Heng Qian Date: Mon, 9 Sep 2024 16:58:32 +0800 Subject: [PATCH 22/29] Remove prompt from node JS server Signed-off-by: Heng Qian --- .../incontext_insight/generate_popover_body.tsx | 16 +++++++++++++--- server/routes/summary_routes.ts | 8 +------- server/types.ts | 14 -------------- 3 files changed, 14 insertions(+), 24 deletions(-) diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index f77bd2e6..b346f0a4 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -47,9 +47,19 @@ export const GeneratePopoverBody: React.FC<{ const onGenerateSummary = (summarizationQuestion: string) => { const summarize = async () => { - const contextObj = incontextInsight.contextProvider - ? await incontextInsight.contextProvider() - : undefined; + let contextObj; + try { + contextObj = (await incontextInsight.contextProvider?.()) ?? undefined; + } catch (e) { + console.error('Error executing contextProvider:', e); + toasts.addDanger( + i18n.translate('assistantDashboards.incontextInsight.generateSummaryError', { + defaultMessage: 'Generate summary error', + }) + ); + closePopover(); + return; + } const contextContent = contextObj?.context || ''; let summaryType: string; const endIndex = incontextInsight.key.indexOf('_', 0); diff --git a/server/routes/summary_routes.ts b/server/routes/summary_routes.ts index 90687bf7..dcdc6879 100644 --- a/server/routes/summary_routes.ts +++ b/server/routes/summary_routes.ts @@ -8,11 +8,9 @@ import { IRouter } from '../../../../src/core/server'; import { SUMMARY_ASSISTANT_API } from '../../common/constants/llm'; import { getOpenSearchClientTransport } from '../utils/get_opensearch_client_transport'; import { getAgent, searchAgentByName } from './get_agent'; -import { ML_COMMONS_BASE_API } from '../utils/constants'; -import { InsightType, SummaryType } from '../types'; import { AssistantServiceSetup } from '../services/assistant_service'; -const SUMMARY_AGENT_CONFIG_ID = 'os_general_llm'; +const SUMMARY_AGENT_CONFIG_ID = 'os_summary'; const OS_INSIGHT_AGENT_CONFIG_ID = 'os_insight'; let osInsightAgentId: string | undefined; let userInsightAgentId: string | undefined; @@ -41,10 +39,8 @@ export function registerSummaryAssistantRoutes( context, dataSourceId: req.query.dataSourceId, }); - const prompt = SummaryType.find((type) => type.id === req.body.type)?.prompt; const assistantClient = assistantService.getScopedClient(req, context); const response = await assistantClient.executeAgentByName(SUMMARY_AGENT_CONFIG_ID, { - prompt, context: req.body.context, question: req.body.question, }); @@ -102,7 +98,6 @@ export function registerSummaryAssistantRoutes( context, dataSourceId: req.query.dataSourceId, }); - const prompt = InsightType.find((type) => type.id === req.body.summaryType)?.prompt; const insightAgentId = req.body.insightType === 'os_insight' ? osInsightAgentId : userInsightAgentId; if (!insightAgentId) { @@ -113,7 +108,6 @@ export function registerSummaryAssistantRoutes( } const assistantClient = assistantService.getScopedClient(req, context); const response = await assistantClient.executeAgent(insightAgentId, { - text: prompt, context: req.body.context, summary: req.body.summary, question: req.body.question, diff --git a/server/types.ts b/server/types.ts index 5a5f1455..7f4e509d 100644 --- a/server/types.ts +++ b/server/types.ts @@ -52,17 +52,3 @@ declare module '../../../src/core/server' { }; } } - -export const SummaryType = [ - { - id: 'alerts', - prompt: `You are an OpenSearch Alert Assistant to help summarize the alerts.\n Here is the detail of alert: $\{parameters.context};\n The question is: $\{parameters.question}`, - }, -]; - -export const InsightType = [ - { - id: 'alerts', - prompt: `You are an OpenSearch Alert Assistant to provide your insight on this alert to help users understand the alert, find potential causes and give feasible solutions to address it.\n Here is the detail of alert: $\{parameters.context};\n The alert summary is: $\{parameters.summary};\n The question is: $\{parameters.question}`, - }, -]; From cde988b8400ca8e19e57e9c2a57990cf3d112b1f Mon Sep 17 00:00:00 2001 From: Heng Qian Date: Tue, 10 Sep 2024 14:11:47 +0800 Subject: [PATCH 23/29] Replace CSS with component property Signed-off-by: Heng Qian --- .../generate_popover_body.test.tsx | 3 +- .../generate_popover_body.tsx | 56 ++++++++----------- .../components/incontext_insight/index.scss | 43 +------------- 3 files changed, 26 insertions(+), 76 deletions(-) diff --git a/public/components/incontext_insight/generate_popover_body.test.tsx b/public/components/incontext_insight/generate_popover_body.test.tsx index 1085a862..3fb3c3e7 100644 --- a/public/components/incontext_insight/generate_popover_body.test.tsx +++ b/public/components/incontext_insight/generate_popover_body.test.tsx @@ -94,7 +94,7 @@ describe('GeneratePopoverBody', () => { // 2. Click insight tip icon to view insight fireEvent.click(insightTipIcon); // title is back button + 'Insight With RAG' - const backButton = getByLabelText('back-to-summary'); + let backButton = getByLabelText('back-to-summary'); expect(backButton).toBeInTheDocument(); expect(getByText('Insight With RAG')).toBeInTheDocument(); @@ -110,6 +110,7 @@ describe('GeneratePopoverBody', () => { expect(mockToasts.addDanger).not.toHaveBeenCalled(); // 3. Click back button to view summary + backButton = getByLabelText('back-to-summary'); fireEvent.click(backButton); expect(queryByText('Generated insight content')).toBeNull(); expect(queryByText('Generated summary content')).toBeInTheDocument(); diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index b346f0a4..fca2c10a 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -6,10 +6,9 @@ import React, { useState } from 'react'; import { i18n } from '@osd/i18n'; import { - EuiBadge, - EuiButtonEmpty, EuiFlexGroup, EuiFlexItem, + EuiIcon, EuiIconTip, EuiLoadingContent, EuiMarkdownFormat, @@ -18,6 +17,7 @@ import { EuiPopoverTitle, EuiSpacer, EuiText, + EuiTitle, } from '@elastic/eui'; import { useEffectOnce } from 'react-use'; import { IncontextInsight as IncontextInsightInput } from '../../types'; @@ -167,44 +167,34 @@ export const GeneratePopoverBody: React.FC<{ const renderInnerTitle = () => { return ( - {showInsight ? ( - - - + + {showInsight ? ( + { setShowInsight(false); }} - iconType="arrowLeft" - iconSide={'left'} + type="arrowLeft" color={'text'} - > - {i18n.translate('assistantDashboards.incontextInsight.InsightWithRAG', { - defaultMessage: 'Insight With RAG', - })} - - - - ) : ( - - -
- + /> + ) : ( + + )} + + + + +
{i18n.translate('assistantDashboards.incontextInsight.Summary', { - defaultMessage: 'Summary', + defaultMessage: showInsight ? 'Insight With RAG' : 'Summary', })} - -
- - - )} + + + + + ); }; diff --git a/public/components/incontext_insight/index.scss b/public/components/incontext_insight/index.scss index 20bc523d..4c6ed7a5 100644 --- a/public/components/incontext_insight/index.scss +++ b/public/components/incontext_insight/index.scss @@ -115,52 +115,11 @@ .incontextInsightGeneratePopoverTitle { // TODO: Remove this one paddingSize is fixed padding: 0 !important; + padding-left: 5px !important; margin-bottom: 0 !important; min-height: 30px; max-height: 30px; border: none; - - :first-child { - border: none; - } - - .euiBadge__text { - text-transform: none; - font-weight: bold; - font-size: 15px; - } - - .euiBadge__icon { - margin: 2px 0 2px 0; - } - - .euiIcon--small { - height: 100%; - width: 22px; - padding: 2px 0; - } - - .euiButtonEmpty{ - padding-inline: 8px; - - .euiButtonEmpty__content { - margin-block: 2px; - padding-block: 2px; - - .euiIcon--small { - height: 100%; - width: 16px; - padding: 2px 0; - } - - .euiButtonEmpty__text { - margin-inline-start: 4px; - text-transform: none; - font-weight: bold; - font-size: 15px; - } - } - } } .incontextInsightGeneratePopoverFooter{ From 72aeb88f7362781dfc65f554460f2286a86ac68c Mon Sep 17 00:00:00 2001 From: Sihan He Date: Wed, 11 Sep 2024 13:10:59 +0800 Subject: [PATCH 24/29] Save the status when switch between summary and insight Signed-off-by: Sihan He --- .../generate_popover_body.tsx | 43 +++++++++++++------ public/components/incontext_insight/index.tsx | 1 - 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index 8b22e14a..f4fb18f7 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -26,16 +26,14 @@ import { getNotifications } from '../../services'; import { HttpSetup } from '../../../../../src/core/public'; import { SUMMARY_ASSISTANT_API } from '../../../common/constants/llm'; import shiny_sparkle from '../../assets/shiny_sparkle.svg'; -import { DataSourceService } from '../../services/data_source_service'; import { UsageCollectionSetup } from '../../../../../src/plugins/usage_collection/public'; export const GeneratePopoverBody: React.FC<{ incontextInsight: IncontextInsightInput; httpSetup?: HttpSetup; - dataSourceService?: DataSourceService; usageCollection?: UsageCollectionSetup; closePopover: () => void; -}> = ({ incontextInsight, httpSetup, dataSourceService, usageCollection, closePopover }) => { +}> = ({ incontextInsight, httpSetup, usageCollection, closePopover }) => { const [summary, setSummary] = useState(''); const [insight, setInsight] = useState(''); const [insightAvailable, setInsightAvailable] = useState(false); @@ -220,17 +218,34 @@ export const GeneratePopoverBody: React.FC<{ const renderInnerFooter = () => { return ( - { - setShowInsight(true); - }} - usageCollection={usageCollection} - isOnTrace={showInsight} - metricAppName={metricAppName} - /> + { +
+ { + setShowInsight(true); + }} + usageCollection={usageCollection} + isOnTrace={showInsight} + metricAppName={metricAppName} + /> +
+ } + { +
+ {}} + usageCollection={usageCollection} + isOnTrace={showInsight} + metricAppName={metricAppName} + /> +
+ } ); }; diff --git a/public/components/incontext_insight/index.tsx b/public/components/incontext_insight/index.tsx index 75160663..9129bf8f 100644 --- a/public/components/incontext_insight/index.tsx +++ b/public/components/incontext_insight/index.tsx @@ -288,7 +288,6 @@ export const IncontextInsight = ({ From a35d1474c19e4a92a1910aef967c35213b6ad2e3 Mon Sep 17 00:00:00 2001 From: Sihan He Date: Thu, 12 Sep 2024 14:03:11 +0800 Subject: [PATCH 25/29] Add tests for metrics Signed-off-by: Sihan He --- .../generate_popover_body.test.tsx | 31 +++++++++-- .../generate_popover_body.tsx | 2 +- public/hooks/use_feed_back.test.tsx | 51 +++++++++++++++++++ public/tabs/chat/messages/message_action.tsx | 9 +++- .../chat/messages/message_bubble.test.tsx | 14 +++++ 5 files changed, 100 insertions(+), 7 deletions(-) diff --git a/public/components/incontext_insight/generate_popover_body.test.tsx b/public/components/incontext_insight/generate_popover_body.test.tsx index 3fb3c3e7..68b19520 100644 --- a/public/components/incontext_insight/generate_popover_body.test.tsx +++ b/public/components/incontext_insight/generate_popover_body.test.tsx @@ -4,11 +4,12 @@ */ import React from 'react'; -import { render, cleanup, fireEvent, waitFor } from '@testing-library/react'; +import { render, cleanup, fireEvent, waitFor, screen } from '@testing-library/react'; import { getConfigSchema, getNotifications } from '../../services'; import { GeneratePopoverBody } from './generate_popover_body'; import { HttpSetup } from '../../../../../src/core/public'; import { SUMMARY_ASSISTANT_API } from '../../../common/constants/llm'; +import { UsageCollectionSetup } from '../../../../../src/plugins/usage_collection/public'; jest.mock('../../services'); @@ -26,7 +27,13 @@ beforeEach(() => { }); afterEach(cleanup); - +const reportUiStatsMock = jest.fn(); +const mockUsageCollection: UsageCollectionSetup = { + reportUiStats: reportUiStatsMock, + METRIC_TYPE: { + CLICK: 'click', + }, +}; const mockPost = jest.fn(); const mockHttpSetup: HttpSetup = ({ post: mockPost, @@ -68,6 +75,7 @@ describe('GeneratePopoverBody', () => { incontextInsight={incontextInsightMock} httpSetup={mockHttpSetup} closePopover={closePopoverMock} + usageCollection={mockUsageCollection} /> ); @@ -86,9 +94,15 @@ describe('GeneratePopoverBody', () => { expect(queryByLabelText('loading_content')).toBeNull(); expect(mockPost).toHaveBeenCalledWith(SUMMARY_ASSISTANT_API.SUMMARIZE, expect.any(Object)); expect(mockToasts.addDanger).not.toHaveBeenCalled(); + // generated metric is sent + expect(reportUiStatsMock).toHaveBeenCalledWith( + 'alertSumm', + 'click', + expect.stringMatching(/^generated/) + ); // insight tip icon is visible - const insightTipIcon = getByLabelText('Insight'); + const insightTipIcon = screen.getAllByLabelText('How was this generated?')[0]; expect(insightTipIcon).toBeInTheDocument(); // 2. Click insight tip icon to view insight @@ -142,6 +156,7 @@ describe('GeneratePopoverBody', () => { incontextInsight={incontextInsightMock} httpSetup={mockHttpSetup} closePopover={closePopoverMock} + usageCollection={mockUsageCollection} /> ); @@ -159,9 +174,15 @@ describe('GeneratePopoverBody', () => { expect(queryByLabelText('loading_content')).toBeNull(); expect(mockPost).toHaveBeenCalledWith(SUMMARY_ASSISTANT_API.SUMMARIZE, expect.any(Object)); expect(mockToasts.addDanger).not.toHaveBeenCalled(); + // generated metric is sent + expect(reportUiStatsMock).toHaveBeenCalledWith( + 'alertSumm', + 'click', + expect.stringMatching(/^generated/) + ); // insight tip icon is not visible - expect(queryByLabelText('Insight')).toBeNull(); + expect(screen.queryAllByLabelText('How was this generated?')).toHaveLength(0); // Only call http post 1 time. expect(mockPost).toHaveBeenCalledTimes(1); }); @@ -223,6 +244,6 @@ describe('GeneratePopoverBody', () => { // Show summary content although insight generation failed expect(getByText('Generated summary content')).toBeInTheDocument(); // insight tip icon is not visible for this alert - expect(queryByLabelText('Insight')).toBeNull(); + expect(screen.queryAllByLabelText('How was this generated?')).toHaveLength(0); }); }); diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index 3642ae8f..57bbc211 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -239,7 +239,7 @@ export const GeneratePopoverBody: React.FC<{ {}} usageCollection={usageCollection} isOnTrace={showInsight} diff --git a/public/hooks/use_feed_back.test.tsx b/public/hooks/use_feed_back.test.tsx index 6ecab50f..169d6a1f 100644 --- a/public/hooks/use_feed_back.test.tsx +++ b/public/hooks/use_feed_back.test.tsx @@ -10,6 +10,8 @@ import { Interaction, IOutput, IMessage } from '../../common/types/chat_saved_ob import { DataSourceService } from '../services'; import { HttpSetup } from '../../../../src/core/public'; import { ASSISTANT_API } from '../../common/constants/llm'; +import { UsageCollectionSetup } from '../../../../src/plugins/usage_collection/public'; +import { None } from 'vega'; jest.mock('../services'); @@ -23,6 +25,14 @@ describe('useFeedback hook', () => { } as unknown) as DataSourceService; const chatStateDispatchMock = jest.fn(); + const reportUiStatsMock = jest.fn(); + const mockUsageCollection: UsageCollectionSetup = { + reportUiStats: reportUiStatsMock, + METRIC_TYPE: { + CLICK: 'click', + }, + }; + beforeEach(() => { jest.spyOn(chatStateHookExports, 'useChatState').mockReturnValue({ chatState: { messages: [], interactions: [], llmResponding: false }, @@ -113,4 +123,45 @@ describe('useFeedback hook', () => { expect(result.current.feedbackResult).toBe(undefined); }); + + it('should call reportUiStats when sending feedback', async () => { + const mockInteraction = { + interaction_id: 'interactionId', + } as Interaction; + const mockInputMessage = { + type: 'input', + } as IMessage; + const mockOutputMessage = { + type: 'output', + interactionId: 'interactionId', + } as IOutput; + const mockMessages = [mockInputMessage, mockOutputMessage]; + const correct = true; + jest.spyOn(chatStateHookExports, 'useChatState').mockReturnValue({ + chatState: { messages: mockMessages, interactions: [], llmResponding: false }, + chatStateDispatch: chatStateDispatchMock, + }); + const { result } = renderHook(() => + useFeedback(mockInteraction, httpMock, dataSourceServiceMock, mockUsageCollection, 'chat') + ); + expect(result.current.feedbackResult).toBe(undefined); + + const sendFeedback = result.current.sendFeedback; + await act(async () => { + await sendFeedback(correct, mockOutputMessage); + }); + expect(httpMock.put).toHaveBeenCalledWith( + `${ASSISTANT_API.FEEDBACK}/${mockOutputMessage.interactionId}`, + { + body: JSON.stringify({ satisfaction: correct }), + query: dataSourceServiceMock.getDataSourceQuery(), + } + ); + expect(result.current.feedbackResult).toBe(correct); + expect(reportUiStatsMock).toHaveBeenCalledWith( + 'chat', + 'click', + expect.stringMatching(/^thumbup/) + ); + }); }); diff --git a/public/tabs/chat/messages/message_action.tsx b/public/tabs/chat/messages/message_action.tsx index be3a235a..d76b1484 100644 --- a/public/tabs/chat/messages/message_action.tsx +++ b/public/tabs/chat/messages/message_action.tsx @@ -92,7 +92,14 @@ export const MessageActions: React.FC = ({ component: renderButtonWithTooltip( 'Copy to clipboard', - {(copy) => } + {(copy) => ( + + )} , 'copy' ), diff --git a/public/tabs/chat/messages/message_bubble.test.tsx b/public/tabs/chat/messages/message_bubble.test.tsx index e3723c00..a41274da 100644 --- a/public/tabs/chat/messages/message_bubble.test.tsx +++ b/public/tabs/chat/messages/message_bubble.test.tsx @@ -10,10 +10,12 @@ import { MessageBubble } from './message_bubble'; import { IOutput } from '../../../../common/types/chat_saved_object_attributes'; import * as useFeedbackHookExports from '../../../hooks/use_feed_back'; import * as useChatActionsExports from '../../../hooks/use_chat_actions'; +import * as coreHookExports from '../../../contexts/core_context'; describe('', () => { const sendFeedbackMock = jest.fn(); const executeActionMock = jest.fn(); + const reportUiStatsMock = jest.fn(); beforeEach(() => { jest @@ -28,6 +30,18 @@ describe('', () => { abortAction: jest.fn(), regenerate: jest.fn(), }); + jest.spyOn(coreHookExports, 'useCore').mockReturnValue({ + services: { + setupDeps: { + usageCollection: { + reportUiStats: reportUiStatsMock, + METRIC_TYPE: { + CLICK: 'click', + }, + }, + }, + }, + }); }); afterEach(() => { From e7c2f7f1565a22814130001fc1a8493191b79a81 Mon Sep 17 00:00:00 2001 From: Sihan He Date: Wed, 18 Sep 2024 10:50:44 +0800 Subject: [PATCH 26/29] Use usageCollectionPluginMock in tests Signed-off-by: Sihan He --- CHANGELOG.md | 4 +--- .../generate_popover_body.test.tsx | 15 ++++----------- .../incontext_insight/generate_popover_body.tsx | 2 +- public/components/incontext_insight/index.tsx | 3 --- public/hooks/use_feed_back.test.tsx | 15 ++++----------- public/plugin.tsx | 2 -- public/tabs/chat/messages/message_action.tsx | 7 ++++--- 7 files changed, 14 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c117caeb..e5334f5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,4 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Support insight with RAG in alert analysis assistant and refine the UX ([#266](https://github.com/opensearch-project/dashboards-assistant/pull/266)) - Add assistant enabled capabilities to control rendering component([#267](https://github.com/opensearch-project/dashboards-assistant/pull/267)) - Add data to summary API([#295](https://github.com/opensearch-project/dashboards-assistant/pull/295)) -- Refactor popover to add message action bar and add metrics to thumb-up and thumb-down([#304](https://github.com/opensearch-project/dashboards-assistant/pull/304)) - - +- Refactor popover to add message action bar and add metrics to thumb-up and thumb-down([#304](https://github.com/opensearch-project/dashboards-assistant/pull/304)) \ No newline at end of file diff --git a/public/components/incontext_insight/generate_popover_body.test.tsx b/public/components/incontext_insight/generate_popover_body.test.tsx index 827e6cdb..3be1e416 100644 --- a/public/components/incontext_insight/generate_popover_body.test.tsx +++ b/public/components/incontext_insight/generate_popover_body.test.tsx @@ -9,8 +9,7 @@ import { getConfigSchema, getNotifications } from '../../services'; import { GeneratePopoverBody } from './generate_popover_body'; import { HttpSetup } from '../../../../../src/core/public'; import { SUMMARY_ASSISTANT_API } from '../../../common/constants/llm'; -import { UsageCollectionSetup } from '../../../../../src/plugins/usage_collection/public'; - +import { usageCollectionPluginMock } from '../../../../../src/plugins/usage_collection/public/mocks'; jest.mock('../../services'); @@ -28,13 +27,7 @@ beforeEach(() => { }); afterEach(cleanup); -const reportUiStatsMock = jest.fn(); -const mockUsageCollection: UsageCollectionSetup = { - reportUiStats: reportUiStatsMock, - METRIC_TYPE: { - CLICK: 'click', - }, -}; +const mockUsageCollection = usageCollectionPluginMock.createSetupContract(); const mockPost = jest.fn(); const mockHttpSetup: HttpSetup = ({ post: mockPost, @@ -96,7 +89,7 @@ describe('GeneratePopoverBody', () => { expect(mockPost).toHaveBeenCalledWith(SUMMARY_ASSISTANT_API.SUMMARIZE, expect.any(Object)); expect(mockToasts.addDanger).not.toHaveBeenCalled(); // generated metric is sent - expect(reportUiStatsMock).toHaveBeenCalledWith( + expect(mockUsageCollection.reportUiStats).toHaveBeenCalledWith( 'alertSumm', 'click', expect.stringMatching(/^generated/) @@ -176,7 +169,7 @@ describe('GeneratePopoverBody', () => { expect(mockPost).toHaveBeenCalledWith(SUMMARY_ASSISTANT_API.SUMMARIZE, expect.any(Object)); expect(mockToasts.addDanger).not.toHaveBeenCalled(); // generated metric is sent - expect(reportUiStatsMock).toHaveBeenCalledWith( + expect(mockUsageCollection.reportUiStats).toHaveBeenCalledWith( 'alertSumm', 'click', expect.stringMatching(/^generated/) diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index a92ff32f..57bbc211 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -29,7 +29,6 @@ import { SUMMARY_ASSISTANT_API } from '../../../common/constants/llm'; import shiny_sparkle from '../../assets/shiny_sparkle.svg'; import { UsageCollectionSetup } from '../../../../../src/plugins/usage_collection/public'; - export const GeneratePopoverBody: React.FC<{ incontextInsight: IncontextInsightInput; httpSetup?: HttpSetup; @@ -53,6 +52,7 @@ export const GeneratePopoverBody: React.FC<{ ); } : () => {}; + useEffectOnce(() => { onGenerateSummary( incontextInsight.suggestions && incontextInsight.suggestions.length > 0 diff --git a/public/components/incontext_insight/index.tsx b/public/components/incontext_insight/index.tsx index 9129bf8f..76a1b5a8 100644 --- a/public/components/incontext_insight/index.tsx +++ b/public/components/incontext_insight/index.tsx @@ -32,14 +32,12 @@ import { getIncontextInsightRegistry, getNotifications } from '../../services'; import chatIcon from '../../assets/chat.svg'; import sparkle from '../../assets/sparkle.svg'; import { HttpSetup } from '../../../../../src/core/public'; -import { DataSourceService } from '../../services/data_source_service'; import { GeneratePopoverBody } from './generate_popover_body'; import { UsageCollectionSetup } from '../../../../../src/plugins/usage_collection/public/plugin'; export interface IncontextInsightProps { children?: React.ReactNode; httpSetup?: HttpSetup; - dataSourceService?: DataSourceService; usageCollection?: UsageCollectionSetup; } @@ -47,7 +45,6 @@ export interface IncontextInsightProps { export const IncontextInsight = ({ children, httpSetup, - dataSourceService, usageCollection, }: IncontextInsightProps) => { const anchor = useRef(null); diff --git a/public/hooks/use_feed_back.test.tsx b/public/hooks/use_feed_back.test.tsx index 169d6a1f..3e5ff1ce 100644 --- a/public/hooks/use_feed_back.test.tsx +++ b/public/hooks/use_feed_back.test.tsx @@ -10,8 +10,7 @@ import { Interaction, IOutput, IMessage } from '../../common/types/chat_saved_ob import { DataSourceService } from '../services'; import { HttpSetup } from '../../../../src/core/public'; import { ASSISTANT_API } from '../../common/constants/llm'; -import { UsageCollectionSetup } from '../../../../src/plugins/usage_collection/public'; -import { None } from 'vega'; +import { usageCollectionPluginMock } from '../../../../src/plugins/usage_collection/public/mocks'; jest.mock('../services'); @@ -23,15 +22,9 @@ describe('useFeedback hook', () => { const dataSourceServiceMock = ({ getDataSourceQuery: jest.fn(), } as unknown) as DataSourceService; - const chatStateDispatchMock = jest.fn(); - const reportUiStatsMock = jest.fn(); - const mockUsageCollection: UsageCollectionSetup = { - reportUiStats: reportUiStatsMock, - METRIC_TYPE: { - CLICK: 'click', - }, - }; + const chatStateDispatchMock = jest.fn(); + const mockUsageCollection = usageCollectionPluginMock.createSetupContract(); beforeEach(() => { jest.spyOn(chatStateHookExports, 'useChatState').mockReturnValue({ @@ -158,7 +151,7 @@ describe('useFeedback hook', () => { } ); expect(result.current.feedbackResult).toBe(correct); - expect(reportUiStatsMock).toHaveBeenCalledWith( + expect(mockUsageCollection.reportUiStats).toHaveBeenCalledWith( 'chat', 'click', expect.stringMatching(/^thumbup/) diff --git a/public/plugin.tsx b/public/plugin.tsx index 0be6c54a..0c2058ef 100644 --- a/public/plugin.tsx +++ b/public/plugin.tsx @@ -297,12 +297,10 @@ export class AssistantPlugin if (!this.incontextInsightRegistry?.isEnabled()) return
; const httpSetup = core.http; const usageCollection = setupDeps.usageCollection || null; - const dataSourceService = this.dataSourceService; return ( ); diff --git a/public/tabs/chat/messages/message_action.tsx b/public/tabs/chat/messages/message_action.tsx index d76b1484..ddcf6396 100644 --- a/public/tabs/chat/messages/message_action.tsx +++ b/public/tabs/chat/messages/message_action.tsx @@ -83,9 +83,10 @@ export const MessageActions: React.FC = ({ ); - const feedbackTip = - 'Thank you for the feedback. Try again by adjusting your prompt so that I have the opportunity to better assist you.'; - + const feedbackTip = i18n.translate(`assistantDashboards.messageActions.feedbackTip`, { + defaultMessage: + 'Thank you for the feedback. Try again by adjusting your prompt so that I have the opportunity to better assist you.', + }); const buttonConfigs = { copy: { show: !isFullWidth, From 2cd22d1e4fb37ad9da78f635e40cb043156111cc Mon Sep 17 00:00:00 2001 From: Sihan He Date: Thu, 19 Sep 2024 14:24:30 +0800 Subject: [PATCH 27/29] Refactor reportMetric as util funciton Signed-off-by: Sihan He --- .../generate_popover_body.tsx | 15 ++----------- public/hooks/use_feed_back.tsx | 14 ++----------- public/utils/report_metric.ts | 21 +++++++++++++++++++ 3 files changed, 25 insertions(+), 25 deletions(-) create mode 100644 public/utils/report_metric.ts diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index 57bbc211..0eafa1eb 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -5,12 +5,10 @@ import React, { useState } from 'react'; import { i18n } from '@osd/i18n'; -import { v4 as uuidv4 } from 'uuid'; import { EuiFlexGroup, EuiFlexItem, EuiIcon, - EuiIconTip, EuiLoadingContent, EuiMarkdownFormat, EuiPanel, @@ -28,6 +26,7 @@ import { HttpSetup } from '../../../../../src/core/public'; import { SUMMARY_ASSISTANT_API } from '../../../common/constants/llm'; import shiny_sparkle from '../../assets/shiny_sparkle.svg'; import { UsageCollectionSetup } from '../../../../../src/plugins/usage_collection/public'; +import { reportMetric } from '../../utils/report_metric'; export const GeneratePopoverBody: React.FC<{ incontextInsight: IncontextInsightInput; @@ -43,16 +42,6 @@ export const GeneratePopoverBody: React.FC<{ const toasts = getNotifications().toasts; - const reportGeneratedMetric = usageCollection - ? (metric: string) => { - usageCollection.reportUiStats( - metricAppName, - usageCollection.METRIC_TYPE.CLICK, - metric + '-' + uuidv4() - ); - } - : () => {}; - useEffectOnce(() => { onGenerateSummary( incontextInsight.suggestions && incontextInsight.suggestions.length > 0 @@ -114,7 +103,7 @@ export const GeneratePopoverBody: React.FC<{ `Please provide your insight on this ${summaryType}.` ); } - reportGeneratedMetric('generated'); + reportMetric(usageCollection, metricAppName, 'generated'); }) .catch((error) => { toasts.addDanger( diff --git a/public/hooks/use_feed_back.tsx b/public/hooks/use_feed_back.tsx index dd78b1dc..b220cfb0 100644 --- a/public/hooks/use_feed_back.tsx +++ b/public/hooks/use_feed_back.tsx @@ -4,7 +4,6 @@ */ import { useState } from 'react'; -import { v4 as uuidv4 } from 'uuid'; import { ASSISTANT_API } from '../../common/constants/llm'; import { IOutput, @@ -15,6 +14,7 @@ import { useChatState } from './use_chat_state'; import { HttpSetup } from '../../../../src/core/public'; import { DataSourceService } from '../services/data_source_service'; import { UsageCollectionSetup } from '../../../../src/plugins/usage_collection/public'; +import { reportMetric } from '../utils/report_metric'; export const useFeedback = ( interaction?: Interaction | null, @@ -44,16 +44,6 @@ export const useFeedback = ( } } - const reportMetric = usageCollection - ? (metric: string) => { - usageCollection.reportUiStats( - metricAppName, - usageCollection.METRIC_TYPE.CLICK, - metric + '-' + uuidv4() - ); - } - : () => {}; - const body: SendFeedbackBody = { satisfaction: correct, }; @@ -65,7 +55,7 @@ export const useFeedback = ( }); } setFeedbackResult(correct); - reportMetric(correct ? 'thumbup' : 'thumbdown'); + reportMetric(usageCollection, metricAppName, correct ? 'thumbup' : 'thumbdown'); } catch (error) { console.error('send feedback error', error); } diff --git a/public/utils/report_metric.ts b/public/utils/report_metric.ts new file mode 100644 index 00000000..02740812 --- /dev/null +++ b/public/utils/report_metric.ts @@ -0,0 +1,21 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { v4 as uuidv4 } from 'uuid'; +import { UsageCollectionSetup } from '../../../../src/plugins/usage_collection/public'; + +export const reportMetric = ( + usageCollection?: UsageCollectionSetup, + metricAppName: string = 'app', + metric: string = 'click' +) => { + if (usageCollection) { + usageCollection.reportUiStats( + metricAppName, + usageCollection.METRIC_TYPE.CLICK, + metric + '-' + uuidv4() + ); + } +}; From b8bbda15cfd8a53edd1275384651eaeb4ac1ef1c Mon Sep 17 00:00:00 2001 From: Sihan He Date: Thu, 19 Sep 2024 18:56:16 +0800 Subject: [PATCH 28/29] Remove telementry plugin Signed-off-by: Sihan He --- opensearch_dashboards.json | 3 +-- public/utils/report_metric.ts | 10 ++++------ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/opensearch_dashboards.json b/opensearch_dashboards.json index af7e74b9..3bfa5670 100644 --- a/opensearch_dashboards.json +++ b/opensearch_dashboards.json @@ -18,8 +18,7 @@ "optionalPlugins": [ "dataSource", "dataSourceManagement", - "usageCollection", - "telemetry" + "usageCollection" ], "requiredBundles": [], "configPath": [ diff --git a/public/utils/report_metric.ts b/public/utils/report_metric.ts index 02740812..1595da1f 100644 --- a/public/utils/report_metric.ts +++ b/public/utils/report_metric.ts @@ -4,18 +4,16 @@ */ import { v4 as uuidv4 } from 'uuid'; +import { METRIC_TYPE, UiStatsMetricType } from '@osd/analytics'; import { UsageCollectionSetup } from '../../../../src/plugins/usage_collection/public'; export const reportMetric = ( usageCollection?: UsageCollectionSetup, metricAppName: string = 'app', - metric: string = 'click' + metric: string = 'click', + metricType: UiStatsMetricType = METRIC_TYPE.CLICK ) => { if (usageCollection) { - usageCollection.reportUiStats( - metricAppName, - usageCollection.METRIC_TYPE.CLICK, - metric + '-' + uuidv4() - ); + usageCollection.reportUiStats(metricAppName, metricType, metric + '-' + uuidv4()); } }; From c209f93485cf14b083414e3eb00591f22688e6b3 Mon Sep 17 00:00:00 2001 From: Sihan He Date: Fri, 20 Sep 2024 11:47:32 +0800 Subject: [PATCH 29/29] Fix metric type Signed-off-by: Sihan He --- .../incontext_insight/generate_popover_body.test.tsx | 8 ++++---- .../incontext_insight/generate_popover_body.tsx | 5 +++-- public/plugin.tsx | 3 +-- public/tabs/chat/messages/message_action.tsx | 3 +-- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/public/components/incontext_insight/generate_popover_body.test.tsx b/public/components/incontext_insight/generate_popover_body.test.tsx index 3be1e416..2581d776 100644 --- a/public/components/incontext_insight/generate_popover_body.test.tsx +++ b/public/components/incontext_insight/generate_popover_body.test.tsx @@ -90,8 +90,8 @@ describe('GeneratePopoverBody', () => { expect(mockToasts.addDanger).not.toHaveBeenCalled(); // generated metric is sent expect(mockUsageCollection.reportUiStats).toHaveBeenCalledWith( - 'alertSumm', - 'click', + 'alertSummary', + 'count', expect.stringMatching(/^generated/) ); @@ -170,8 +170,8 @@ describe('GeneratePopoverBody', () => { expect(mockToasts.addDanger).not.toHaveBeenCalled(); // generated metric is sent expect(mockUsageCollection.reportUiStats).toHaveBeenCalledWith( - 'alertSumm', - 'click', + 'alertSummary', + 'count', expect.stringMatching(/^generated/) ); diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index 0eafa1eb..2bb39737 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -19,6 +19,7 @@ import { EuiTitle, } from '@elastic/eui'; import { useEffectOnce } from 'react-use'; +import { METRIC_TYPE } from '@osd/analytics'; import { MessageActions } from '../../tabs/chat/messages/message_action'; import { IncontextInsight as IncontextInsightInput } from '../../types'; import { getNotifications } from '../../services'; @@ -38,7 +39,7 @@ export const GeneratePopoverBody: React.FC<{ const [insight, setInsight] = useState(''); const [insightAvailable, setInsightAvailable] = useState(false); const [showInsight, setShowInsight] = useState(false); - const metricAppName = 'alertSumm'; + const metricAppName = 'alertSummary'; const toasts = getNotifications().toasts; @@ -103,7 +104,7 @@ export const GeneratePopoverBody: React.FC<{ `Please provide your insight on this ${summaryType}.` ); } - reportMetric(usageCollection, metricAppName, 'generated'); + reportMetric(usageCollection, metricAppName, 'generated', METRIC_TYPE.COUNT); }) .catch((error) => { toasts.addDanger( diff --git a/public/plugin.tsx b/public/plugin.tsx index 98f66f20..6d28c37c 100644 --- a/public/plugin.tsx +++ b/public/plugin.tsx @@ -297,12 +297,11 @@ export class AssistantPlugin renderIncontextInsight: (props: any) => { if (!this.incontextInsightRegistry?.isEnabled()) return
; const httpSetup = core.http; - const usageCollection = setupDeps.usageCollection || null; return ( ); }, diff --git a/public/tabs/chat/messages/message_action.tsx b/public/tabs/chat/messages/message_action.tsx index ddcf6396..e2b84b24 100644 --- a/public/tabs/chat/messages/message_action.tsx +++ b/public/tabs/chat/messages/message_action.tsx @@ -84,8 +84,7 @@ export const MessageActions: React.FC = ({ ); const feedbackTip = i18n.translate(`assistantDashboards.messageActions.feedbackTip`, { - defaultMessage: - 'Thank you for the feedback. Try again by adjusting your prompt so that I have the opportunity to better assist you.', + defaultMessage: 'We have successfully received your feedback. Thank you.', }); const buttonConfigs = { copy: {