diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 19d54289..6c7e55f5 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -6,11 +6,9 @@ ### Check List - [ ] New functionality includes testing. - - [ ] All tests pass, including unit test, integration test and doctest -- [ ] New functionality has been documented. - - [ ] New functionality has javadoc added - - [ ] New functionality has user manual doc added -- [ ] Commits are signed per the DCO using --signoff + - [ ] All tests pass, including unit test, integration test. +- [ ] New functionality has user manual doc added. +- [ ] Commits are signed per the DCO using --signoff. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check [here](https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin). diff --git a/common/types/chat_saved_object_attributes.ts b/common/types/chat_saved_object_attributes.ts index 608f83ff..c836f3b1 100644 --- a/common/types/chat_saved_object_attributes.ts +++ b/common/types/chat_saved_object_attributes.ts @@ -43,11 +43,11 @@ export interface IOutput { type: 'output'; traceId?: string; // used for tracing agent calls toolsUsed?: string[]; - // TODO: ppl_visualization type may need to be removed in the PR which replaces ppl query render from visualization to data grid. @suzhou - contentType: 'error' | 'markdown' | 'visualization' | 'ppl_visualization' | 'ppl_data_grid'; + contentType: 'error' | 'markdown' | 'visualization' | string; content: string; suggestedActions?: ISuggestedAction[]; messageId?: string; + fullWidth?: boolean; } export type IMessage = IInput | IOutput; diff --git a/public/chat_header_button.test.tsx b/public/chat_header_button.test.tsx index ecf38337..f7c3c773 100644 --- a/public/chat_header_button.test.tsx +++ b/public/chat_header_button.test.tsx @@ -60,7 +60,7 @@ describe('', () => { ', () => { ', () => { ', () => { ', () => { ', () => { ; + messageRenderers: Record; actionExecutors: Record; assistantActions: AssistantActions; currentAccount: UserAccount; @@ -70,7 +70,7 @@ export const HeaderChatButton = (props: HeaderChatButtonProps) => { setFlyoutVisible, setFlyoutComponent, userHasAccess: props.userHasAccess, - contentRenderers: props.contentRenderers, + messageRenderers: props.messageRenderers, actionExecutors: props.actionExecutors, currentAccount: props.currentAccount, title, @@ -86,7 +86,7 @@ export const HeaderChatButton = (props: HeaderChatButtonProps) => { selectedTabId, preSelectedTabId, props.userHasAccess, - props.contentRenderers, + props.messageRenderers, props.actionExecutors, props.currentAccount, title, diff --git a/public/components/__snapshots__/agent_framework_traces.test.tsx.snap b/public/components/__snapshots__/agent_framework_traces.test.tsx.snap index e543776a..fc74dc36 100644 --- a/public/components/__snapshots__/agent_framework_traces.test.tsx.snap +++ b/public/components/__snapshots__/agent_framework_traces.test.tsx.snap @@ -7,17 +7,23 @@ HTMLCollection [ class="euiMarkdownFormat" >
-

+

How was this generated

-

+

Question

-

+

Result

@@ -66,7 +72,11 @@ HTMLCollection [ viewBox="0 0 16 16" width="16" xmlns="http://www.w3.org/2000/svg" - /> + > + + { setFlyoutVisible: jest.fn(), setFlyoutComponent: jest.fn(), userHasAccess: true, - contentRenderers: {}, + messageRenderers: {}, actionExecutors: {}, currentAccount: { username: 'foo', tenant: '' }, setTitle: jest.fn(), diff --git a/public/contexts/chat_context.tsx b/public/contexts/chat_context.tsx index 04df189a..9b8a29dc 100644 --- a/public/contexts/chat_context.tsx +++ b/public/contexts/chat_context.tsx @@ -4,7 +4,7 @@ */ import React, { useContext } from 'react'; -import { ActionExecutor, ContentRenderer, UserAccount, TabId } from '../types'; +import { ActionExecutor, UserAccount, TabId, MessageRenderer } from '../types'; export interface IChatContext { appId?: string; @@ -18,7 +18,7 @@ export interface IChatContext { setFlyoutVisible: React.Dispatch>; setFlyoutComponent: React.Dispatch>; userHasAccess: boolean; - contentRenderers: Record; + messageRenderers: Record; actionExecutors: Record; currentAccount: UserAccount; title?: string; diff --git a/public/hooks/use_chat_actions.test.tsx b/public/hooks/use_chat_actions.test.tsx index 4b23c8e2..513cab4e 100644 --- a/public/hooks/use_chat_actions.test.tsx +++ b/public/hooks/use_chat_actions.test.tsx @@ -75,7 +75,7 @@ describe('useChatActions hook', () => { flyoutVisible: false, flyoutFullScreen: false, userHasAccess: false, - contentRenderers: {}, + messageRenderers: {}, currentAccount: { username: '', tenant: '', diff --git a/public/index.ts b/public/index.ts index cdce4ade..a16d3c12 100644 --- a/public/index.ts +++ b/public/index.ts @@ -12,4 +12,5 @@ export function plugin(initializerContext: PluginInitializerContext) { return new AssistantPlugin(initializerContext); } -export { AssistantSetup } from './types'; +export { AssistantSetup, RenderProps } from './types'; +export { IMessage } from '../common/types/chat_saved_object_attributes'; diff --git a/public/plugin.tsx b/public/plugin.tsx index 7aa4424e..8e335939 100644 --- a/public/plugin.tsx +++ b/public/plugin.tsx @@ -20,7 +20,7 @@ import { AssistantActions, AssistantSetup, AssistantStart, - ContentRenderer, + MessageRenderer, SetupDependencies, } from './types'; @@ -49,7 +49,7 @@ export class AssistantPlugin core: CoreSetup, setupDeps: SetupDependencies ): AssistantSetup { - const contentRenderers: Record = {}; + const messageRenderers: Record = {}; const actionExecutors: Record = {}; const assistantActions: AssistantActions = {} as AssistantActions; /** @@ -95,7 +95,7 @@ export class AssistantPlugin { - if (contentType in contentRenderers) + registerMessageRenderer: (contentType, render) => { + if (contentType in messageRenderers) console.warn(`Content renderer type ${contentType} is already registered.`); - contentRenderers[contentType] = render; + messageRenderers[contentType] = render; }, registerActionExecutor: (actionType, execute) => { if (actionType in actionExecutors) diff --git a/public/tabs/chat/messages/message_bubble.test.tsx b/public/tabs/chat/messages/message_bubble.test.tsx index a836f478..629aa137 100644 --- a/public/tabs/chat/messages/message_bubble.test.tsx +++ b/public/tabs/chat/messages/message_bubble.test.tsx @@ -102,6 +102,7 @@ describe('', () => { type: 'output', contentType: 'visualization', content: 'vis_id_mock', + fullWidth: true, }} /> ); @@ -114,6 +115,7 @@ describe('', () => { type: 'output', contentType: 'ppl_visualization', content: 'vis_id_mock', + fullWidth: true, }} /> ); diff --git a/public/tabs/chat/messages/message_bubble.tsx b/public/tabs/chat/messages/message_bubble.tsx index 8a704e2a..a23b2404 100644 --- a/public/tabs/chat/messages/message_bubble.tsx +++ b/public/tabs/chat/messages/message_bubble.tsx @@ -133,13 +133,7 @@ export const MessageBubble: React.FC = React.memo((props) => ); } - // if (['visualization', 'ppl_visualization'].includes(props.contentType)) { - // return <>{props.children}; - // } - - const isVisualization = ['visualization', 'ppl_visualization'].includes( - props.message.contentType - ); + const fullWidth = props.message.fullWidth; return ( = React.memo((props) => = React.memo((props) => justifyContent="flexStart" style={{ paddingLeft: 10 }} > - {!isVisualization && ( + {!fullWidth && ( {(copy) => ( diff --git a/public/tabs/chat/messages/message_content.test.tsx b/public/tabs/chat/messages/message_content.test.tsx index 5f02a6bb..832701ad 100644 --- a/public/tabs/chat/messages/message_content.test.tsx +++ b/public/tabs/chat/messages/message_content.test.tsx @@ -4,7 +4,7 @@ */ import React from 'react'; -import { render, screen, fireEvent } from '@testing-library/react'; +import { render, screen } from '@testing-library/react'; import { MessageContent } from './message_content'; import * as chatContextExports from '../../../contexts/chat_context'; @@ -15,13 +15,11 @@ jest.mock('../../../components/core_visualization', () => { }); describe('', () => { - const pplVisualizationRenderMock = jest.fn(); const customizedRenderMock = jest.fn(); beforeEach(() => { jest.spyOn(chatContextExports, 'useChatContext').mockReturnValue({ - contentRenderers: { - ppl_visualization: pplVisualizationRenderMock, + messageRenderers: { customized_content_type: customizedRenderMock, }, }); @@ -79,19 +77,6 @@ describe('', () => { expect(screen.queryAllByText('title')).toHaveLength(1); }); - it('should render ppl visualization', () => { - render( - - ); - expect(pplVisualizationRenderMock).toHaveBeenCalledWith({ query: 'mock ppl query' }); - }); - it('should render customized render content', () => { render( ', () => { }} /> ); - expect(customizedRenderMock).toHaveBeenCalledWith('mock customized content'); + expect(customizedRenderMock.mock.calls[0][0]).toMatchInlineSnapshot(` + Object { + "content": "mock customized content", + "contentType": "customized_content_type", + "type": "output", + } + `); + expect(customizedRenderMock.mock.calls[0][1].props).toMatchInlineSnapshot(` + Object { + "message": Object { + "content": "mock customized content", + "contentType": "customized_content_type", + "type": "output", + }, + } + `); + expect(customizedRenderMock.mock.calls[0][1].chatContext).not.toBeUndefined(); }); }); diff --git a/public/tabs/chat/messages/message_content.tsx b/public/tabs/chat/messages/message_content.tsx index cb857524..5b0be173 100644 --- a/public/tabs/chat/messages/message_content.tsx +++ b/public/tabs/chat/messages/message_content.tsx @@ -9,7 +9,7 @@ import { IMessage } from '../../../../common/types/chat_saved_object_attributes' import { CoreVisualization } from '../../../components/core_visualization'; import { useChatContext } from '../../../contexts/chat_context'; -interface MessageContentProps { +export interface MessageContentProps { message: IMessage; } @@ -37,18 +37,15 @@ export const MessageContent: React.FC = React.memo((props)
); - case 'ppl_visualization': { - const render = chatContext.contentRenderers[props.message.contentType]; - if (!render) return null; - return ( -
{render({ query: props.message.content })}
- ); - } - // content types registered by plugins unknown to assistant default: { const message = props.message as IMessage; - return chatContext.contentRenderers[message.contentType]?.(message.content) ?? null; + return ( + chatContext.messageRenderers[message.contentType]?.(message, { + props, + chatContext, + }) ?? null + ); } } }); diff --git a/public/types.ts b/public/types.ts index ba4a90d6..d0698774 100644 --- a/public/types.ts +++ b/public/types.ts @@ -6,9 +6,16 @@ import { DashboardStart } from '../../../src/plugins/dashboard/public'; import { EmbeddableSetup, EmbeddableStart } from '../../../src/plugins/embeddable/public'; import { IMessage, ISuggestedAction } from '../common/types/chat_saved_object_attributes'; +import { IChatContext } from './contexts/chat_context'; +import { MessageContentProps } from './tabs/chat/messages/message_content'; + +export interface RenderProps { + props: MessageContentProps; + chatContext: IChatContext; +} // TODO should pair with server side registered output parser -export type ContentRenderer = (content: unknown) => React.ReactElement; +export type MessageRenderer = (message: IMessage, renderProps: RenderProps) => React.ReactElement; export type ActionExecutor = (params: Record) => void; export interface AssistantActions { send: (input: IMessage) => Promise; @@ -30,7 +37,7 @@ export interface SetupDependencies { } export interface AssistantSetup { - registerContentRenderer: (contentType: string, render: ContentRenderer) => void; + registerMessageRenderer: (contentType: string, render: MessageRenderer) => void; registerActionExecutor: (actionType: string, execute: ActionExecutor) => void; /** * Returns true if chat UI is enabled. diff --git a/public/utils/notebook.ts b/public/utils/notebook.ts index f67d092d..c9f4f54d 100644 --- a/public/utils/notebook.ts +++ b/public/utils/notebook.ts @@ -92,6 +92,9 @@ export const convertMessagesToParagraphs = (messages: IMessage[], username: stri }); break; + /** + * TODO migrate ppl_data_grid logic to dashboards-observerability + */ case 'ppl_data_grid': const queryText = message.content; Object.assign(paragraph, { @@ -121,7 +124,7 @@ export const convertMessagesToParagraphs = (messages: IMessage[], username: stri }); break; - // error and ppl_visualization contentType will not be handled currently. + // error will not be handled currently. default: break; } diff --git a/server/parsers/visualization_card_parser.test.ts b/server/parsers/visualization_card_parser.test.ts index 32aa0757..7284c373 100644 --- a/server/parsers/visualization_card_parser.test.ts +++ b/server/parsers/visualization_card_parser.test.ts @@ -29,18 +29,21 @@ describe('VisualizationCardParser', () => { contentType: 'visualization', suggestedActions: [{ actionType: 'view_in_dashboards', message: 'View in Visualize' }], type: 'output', + fullWidth: true, }, { content: 'id2', contentType: 'visualization', suggestedActions: [{ actionType: 'view_in_dashboards', message: 'View in Visualize' }], type: 'output', + fullWidth: true, }, { content: 'id3', contentType: 'visualization', suggestedActions: [{ actionType: 'view_in_dashboards', message: 'View in Visualize' }], type: 'output', + fullWidth: true, }, ]); }); @@ -66,12 +69,14 @@ describe('VisualizationCardParser', () => { contentType: 'visualization', suggestedActions: [{ actionType: 'view_in_dashboards', message: 'View in Visualize' }], type: 'output', + fullWidth: true, }, { content: 'id2', contentType: 'visualization', suggestedActions: [{ actionType: 'view_in_dashboards', message: 'View in Visualize' }], type: 'output', + fullWidth: true, }, ]); }); @@ -110,6 +115,7 @@ describe('VisualizationCardParser', () => { contentType: 'visualization', suggestedActions: [{ actionType: 'view_in_dashboards', message: 'View in Visualize' }], type: 'output', + fullWidth: true, }, ]); }); @@ -137,12 +143,14 @@ describe('VisualizationCardParser', () => { contentType: 'visualization', suggestedActions: [{ actionType: 'view_in_dashboards', message: 'View in Visualize' }], type: 'output', + fullWidth: true, }, { content: 'id2', contentType: 'visualization', suggestedActions: [{ actionType: 'view_in_dashboards', message: 'View in Visualize' }], type: 'output', + fullWidth: true, }, ]); }); diff --git a/server/parsers/visualization_card_parser.ts b/server/parsers/visualization_card_parser.ts index 7b4c9898..ebe9b651 100644 --- a/server/parsers/visualization_card_parser.ts +++ b/server/parsers/visualization_card_parser.ts @@ -35,6 +35,7 @@ export const VisualizationCardParser = { type: 'output', content: id, contentType: 'visualization', + fullWidth: true, suggestedActions: [ { message: 'View in Visualize',