diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f4b4759..b9b1d51e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,4 +36,5 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - 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)) - 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)) \ No newline at end of file +- 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)) \ No newline at end of file diff --git a/opensearch_dashboards.json b/opensearch_dashboards.json index f8bdf688..3bfa5670 100644 --- a/opensearch_dashboards.json +++ b/opensearch_dashboards.json @@ -17,7 +17,8 @@ ], "optionalPlugins": [ "dataSource", - "dataSourceManagement" + "dataSourceManagement", + "usageCollection" ], "requiredBundles": [], "configPath": [ diff --git a/public/components/incontext_insight/generate_popover_body.test.tsx b/public/components/incontext_insight/generate_popover_body.test.tsx index 3fb3c3e7..2581d776 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 { usageCollectionPluginMock } from '../../../../../src/plugins/usage_collection/public/mocks'; jest.mock('../../services'); @@ -26,7 +27,7 @@ beforeEach(() => { }); afterEach(cleanup); - +const mockUsageCollection = usageCollectionPluginMock.createSetupContract(); const mockPost = jest.fn(); const mockHttpSetup: HttpSetup = ({ post: mockPost, @@ -68,6 +69,7 @@ describe('GeneratePopoverBody', () => { incontextInsight={incontextInsightMock} httpSetup={mockHttpSetup} closePopover={closePopoverMock} + usageCollection={mockUsageCollection} /> ); @@ -86,9 +88,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(mockUsageCollection.reportUiStats).toHaveBeenCalledWith( + 'alertSummary', + 'count', + 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 +150,7 @@ describe('GeneratePopoverBody', () => { incontextInsight={incontextInsightMock} httpSetup={mockHttpSetup} closePopover={closePopoverMock} + usageCollection={mockUsageCollection} /> ); @@ -159,9 +168,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(mockUsageCollection.reportUiStats).toHaveBeenCalledWith( + 'alertSummary', + 'count', + 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 +238,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 76db7007..8a56f2c9 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -9,7 +9,6 @@ import { EuiFlexGroup, EuiFlexItem, EuiIcon, - EuiIconTip, EuiLoadingContent, EuiMarkdownFormat, EuiPanel, @@ -20,21 +19,28 @@ 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 { ContextObj, IncontextInsight as IncontextInsightInput } from '../../types'; 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 { UsageCollectionSetup } from '../../../../../src/plugins/usage_collection/public'; +import { reportMetric } from '../../utils/report_metric'; export const GeneratePopoverBody: React.FC<{ incontextInsight: IncontextInsightInput; httpSetup?: HttpSetup; + usageCollection?: UsageCollectionSetup; closePopover: () => void; -}> = ({ incontextInsight, httpSetup, closePopover }) => { +}> = ({ incontextInsight, httpSetup, usageCollection, closePopover }) => { const [summary, setSummary] = useState(''); const [insight, setInsight] = useState(''); const [insightAvailable, setInsightAvailable] = useState(false); const [showInsight, setShowInsight] = useState(false); + const metricAppName = 'alertSummary'; + const toasts = getNotifications().toasts; useEffectOnce(() => { @@ -102,6 +108,7 @@ export const GeneratePopoverBody: React.FC<{ `Please provide your insight on this ${summaryType}.` ); } + reportMetric(usageCollection, metricAppName, 'generated', METRIC_TYPE.COUNT); }) .catch((error) => { toasts.addDanger( @@ -208,23 +215,34 @@ export const GeneratePopoverBody: React.FC<{ const renderInnerFooter = () => { return ( - - {insightAvailable && ( - { + { +
+ { 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 feaa87cf..76a1b5a8 100644 --- a/public/components/incontext_insight/index.tsx +++ b/public/components/incontext_insight/index.tsx @@ -33,14 +33,20 @@ import chatIcon from '../../assets/chat.svg'; import sparkle from '../../assets/sparkle.svg'; import { HttpSetup } from '../../../../../src/core/public'; import { GeneratePopoverBody } from './generate_popover_body'; +import { UsageCollectionSetup } from '../../../../../src/plugins/usage_collection/public/plugin'; export interface IncontextInsightProps { children?: React.ReactNode; httpSetup?: HttpSetup; + usageCollection?: UsageCollectionSetup; } // TODO: add saved objects / config to store seed suggestions -export const IncontextInsight = ({ children, httpSetup }: IncontextInsightProps) => { +export const IncontextInsight = ({ + children, + httpSetup, + usageCollection, +}: IncontextInsightProps) => { const anchor = useRef(null); const [isVisible, setIsVisible] = useState(false); @@ -279,6 +285,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 943aab2f..3e5ff1ce 100644 --- a/public/hooks/use_feed_back.test.tsx +++ b/public/hooks/use_feed_back.test.tsx @@ -4,37 +4,29 @@ */ 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 { DataSourceService } from '../services'; +import { HttpSetup } from '../../../../src/core/public'; import { ASSISTANT_API } from '../../common/constants/llm'; -import { DataSourceServiceMock } from '../services/data_source_service.mock'; +import { usageCollectionPluginMock } from '../../../../src/plugins/usage_collection/public/mocks'; + +jest.mock('../services'); describe('useFeedback hook', () => { - const httpMock = httpServiceMock.createStartContract(); + 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(); - const dataSourceMock = new DataSourceServiceMock(); - const chatContextMock = { - rootAgentId: 'root_agent_id_mock', - selectedTabId: 'chat', - setConversationId: jest.fn(), - setTitle: jest.fn(), - currentAccount: { username: 'admin' }, - }; + const mockUsageCollection = usageCollectionPluginMock.createSetupContract(); beforeEach(() => { - jest.spyOn(coreHookExports, 'useCore').mockReturnValue({ - services: { - http: httpMock, - dataSource: dataSourceMock, - }, - }); - jest.spyOn(chatContextHookExports, 'useChatContext').mockReturnValue(chatContextMock); - jest.spyOn(chatStateHookExports, 'useChatState').mockReturnValue({ chatState: { messages: [], interactions: [], llmResponding: false }, chatStateDispatch: chatStateDispatchMock, @@ -59,6 +51,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 +62,34 @@ 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, httpMock, dataSourceServiceMock) + ); expect(result.current.feedbackResult).toBe(undefined); const sendFeedback = result.current.sendFeedback; await act(async () => { - await sendFeedback(mockOutputMessage, true); + await sendFeedback(correct, mockOutputMessage); }); expect(httpMock.put).toHaveBeenCalledWith( `${ASSISTANT_API.FEEDBACK}/${mockOutputMessage.interactionId}`, { - body: JSON.stringify({ - satisfaction: true, - }), - query: dataSourceMock.getDataSourceQuery(), + body: JSON.stringify({ satisfaction: correct }), + query: dataSourceServiceMock.getDataSourceQuery(), } ); - 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 mockInteraction = { + interaction_id: 'interactionId', + } as Interaction; const mockInputMessage = { type: 'input', } as IMessage; @@ -105,51 +102,59 @@ 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; await act(async () => { - await sendFeedback(mockOutputMessage, true); + await sendFeedback(true, mockOutputMessage); }); - 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 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 = [mockOutputMessage]; + 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, httpMock, dataSourceServiceMock, mockUsageCollection, 'chat') + ); expect(result.current.feedbackResult).toBe(undefined); const sendFeedback = result.current.sendFeedback; await act(async () => { - await sendFeedback(mockOutputMessage, true); + await sendFeedback(correct, mockOutputMessage); }); - - expect(httpMock.put).not.toHaveBeenCalledWith( + expect(httpMock.put).toHaveBeenCalledWith( `${ASSISTANT_API.FEEDBACK}/${mockOutputMessage.interactionId}`, { - body: JSON.stringify({ - satisfaction: true, - }), + body: JSON.stringify({ satisfaction: correct }), + query: dataSourceServiceMock.getDataSourceQuery(), } ); + expect(result.current.feedbackResult).toBe(correct); + expect(mockUsageCollection.reportUiStats).toHaveBeenCalledWith( + 'chat', + 'click', + expect.stringMatching(/^thumbup/) + ); }); }); diff --git a/public/hooks/use_feed_back.tsx b/public/hooks/use_feed_back.tsx index 7a37bd37..b220cfb0 100644 --- a/public/hooks/use_feed_back.tsx +++ b/public/hooks/use_feed_back.tsx @@ -5,44 +5,59 @@ 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 { + IOutput, + Interaction, + SendFeedbackBody, +} from '../../common/types/chat_saved_object_attributes'; import { useChatState } from './use_chat_state'; -import { SendFeedbackBody } from '../../common/types/chat_saved_object_attributes'; +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) => { - const core = useCore(); - const { chatState } = useChatState(); +export const useFeedback = ( + interaction?: Interaction | null, + httpSetup?: HttpSetup, + dataSourceService?: DataSourceService, + usageCollection?: UsageCollectionSetup, + metricAppName: string = 'chat' +) => { + 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; + 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; + }); + 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(), - }); + if (message) { + await httpSetup?.put(`${ASSISTANT_API.FEEDBACK}/${message.interactionId}`, { + body: JSON.stringify(body), + query: dataSourceService?.getDataSourceQuery(), + }); + } setFeedbackResult(correct); + reportMetric(usageCollection, metricAppName, correct ? 'thumbup' : 'thumbdown'); } catch (error) { - console.log('send feedback error'); + console.error('send feedback error', error); } }; diff --git a/public/plugin.tsx b/public/plugin.tsx index 8bf25378..6d28c37c 100644 --- a/public/plugin.tsx +++ b/public/plugin.tsx @@ -217,7 +217,6 @@ export class AssistantPlugin if (this.config.chat.enabled) { const setupChat = async () => { const [coreStart, startDeps] = await core.getStartServices(); - const CoreContext = createOpenSearchDashboardsReactContext({ ...coreStart, setupDeps, @@ -298,7 +297,13 @@ export class AssistantPlugin renderIncontextInsight: (props: any) => { if (!this.incontextInsightRegistry?.isEnabled()) return
; const httpSetup = core.http; - return ; + return ( + + ); }, }; } diff --git a/public/tabs/chat/chat_page_content.tsx b/public/tabs/chat/chat_page_content.tsx index 253a52da..bfe6de52 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..1c7b8fe5 --- /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(true, message); + + fireEvent.click(thumbsDownButton); + expect(sendFeedback).toHaveBeenCalledWith(false, message); + }); + + 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 new file mode 100644 index 00000000..e2b84b24 --- /dev/null +++ b/public/tabs/chat/messages/message_action.tsx @@ -0,0 +1,183 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import React, { useCallback } from 'react'; +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'; +import { DataSourceService } from '../../../services/data_source_service'; +import { UsageCollectionSetup } from '../../../../../../src/plugins/usage_collection/public'; + +interface MessageActionsProps { + contentToCopy?: string; + showRegenerate?: boolean; + onRegenerate?: () => void; + interaction?: Interaction | null; + message?: IOutput | null; + showFeedback?: boolean; + showTraceIcon?: boolean; + isOnTrace?: boolean; + traceInteractionId?: string; + onViewTrace?: () => void; + shouldActionBarVisibleOnHover?: boolean; + isFullWidth?: boolean; + httpSetup?: HttpSetup; + dataSourceService?: DataSourceService; + usageCollection?: UsageCollectionSetup; + metricAppName?: string; + buttonOrder?: string[]; +} + +type ButtonKey = 'copy' | 'regenerate' | 'thumbUp' | 'thumbDown' | 'trace'; + +export const MessageActions: React.FC = ({ + contentToCopy = '', + showRegenerate = false, + onRegenerate, + interaction, + message = null, + showFeedback = false, + showTraceIcon = false, + isOnTrace = false, + traceInteractionId = null, + onViewTrace, + shouldActionBarVisibleOnHover = false, + isFullWidth = false, + httpSetup, + dataSourceService, + usageCollection, + metricAppName = 'chat', + buttonOrder = ['trace', 'regenerate', 'thumbUp', 'thumbDown', 'copy'], +}) => { + const { feedbackResult, sendFeedback } = useFeedback( + interaction, + httpSetup, + dataSourceService, + usageCollection, + metricAppName + ); + + const handleFeedback = useCallback( + (correct: boolean) => { + if (feedbackResult !== undefined) { + return; + } + sendFeedback(correct, message); + }, + [feedbackResult, message, sendFeedback] + ); + + const renderButtonWithTooltip = (content: string, button: JSX.Element, key: string) => ( + + {button} + + ); + + const feedbackTip = i18n.translate(`assistantDashboards.messageActions.feedbackTip`, { + defaultMessage: 'We have successfully received your feedback. Thank 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 ( + + {buttonOrder.map( + (key) => + buttonConfigs[key as ButtonKey].show && ( + + {buttonConfigs[key as ButtonKey].component} + + ) + )} + + ); +}; diff --git a/public/tabs/chat/messages/message_bubble.test.tsx b/public/tabs/chat/messages/message_bubble.test.tsx index 29ebc526..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(() => { @@ -102,7 +116,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 +131,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 +166,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 +181,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 +222,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 +233,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 25a67f8f..d86997b1 100644 --- a/public/tabs/chat/messages/message_bubble.tsx +++ b/public/tabs/chat/messages/message_bubble.tsx @@ -5,8 +5,6 @@ import { EuiAvatar, - EuiSmallButtonIcon, - EuiCopy, EuiFlexGroup, EuiFlexItem, EuiLoadingContent, @@ -15,9 +13,11 @@ 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'; +import { useCore } from '../../../contexts'; + // TODO: Replace with getChrome().logos.Chat.url import { useChatActions } from '../../../hooks'; import chatIcon from '../../../assets/chat.svg'; @@ -27,7 +27,6 @@ import { ISuggestedAction, Interaction, } from '../../../../common/types/chat_saved_object_attributes'; -import { useFeedback } from '../../../hooks/use_feed_back'; type MessageBubbleProps = { showActionBar: boolean; @@ -45,29 +44,14 @@ type MessageBubbleProps = { ); export const MessageBubble: React.FC = React.memo((props) => { - const { feedbackResult, sendFeedback } = useFeedback( - 'interaction' in props ? props.interaction : null - ); - const { executeAction } = useChatActions(); - + const core = useCore(); // According to the design of the feedback, only markdown type output is supported. const showFeedback = 'message' in 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 ( @@ -165,98 +149,39 @@ 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} + metricAppName="chat" + /> + + )} - {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} - - - )} ); diff --git a/public/types.ts b/public/types.ts index 57803b4f..f6c279ae 100644 --- a/public/types.ts +++ b/public/types.ts @@ -21,6 +21,8 @@ import { UiActionsSetup, UiActionsStart } from '../../../src/plugins/ui_actions/ import { ExpressionsSetup, ExpressionsStart } from '../../../src/plugins/expressions/public'; import { SavedObjectsStart } from '../../../src/plugins/saved_objects/public'; +import { UsageCollectionSetup } from '../../../src/plugins/usage_collection/public'; + export interface RenderProps { props: MessageContentProps; chatContext: IChatContext; @@ -53,6 +55,7 @@ export interface AssistantPluginSetupDependencies { visualizations: VisualizationsSetup; embeddable: EmbeddableSetup; dataSourceManagement?: DataSourceManagementPluginSetup; + usageCollection?: UsageCollectionSetup; uiActions: UiActionsSetup; expressions: ExpressionsSetup; } diff --git a/public/utils/report_metric.ts b/public/utils/report_metric.ts new file mode 100644 index 00000000..1595da1f --- /dev/null +++ b/public/utils/report_metric.ts @@ -0,0 +1,19 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +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', + metricType: UiStatsMetricType = METRIC_TYPE.CLICK +) => { + if (usageCollection) { + usageCollection.reportUiStats(metricAppName, metricType, metric + '-' + uuidv4()); + } +};