-
Notifications
You must be signed in to change notification settings - Fork 8.6k
[Obs AI Assistant] Display warning banner when re-indexing is in progress #222593
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
9edcc50
d96ef41
d2d46c2
6c72e6a
90d4d67
7b2cc38
fe836a4
e273bfe
5d6ff81
e5c0537
17416fa
cbb0246
fd4a2b3
9edbb0f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -35,6 +35,7 @@ import { | |||||
| type Feedback, | ||||||
| aiAssistantSimulatedFunctionCalling, | ||||||
| getElasticManagedLlmConnector, | ||||||
| KnowledgeBaseState, | ||||||
| } from '@kbn/observability-ai-assistant-plugin/public'; | ||||||
| import type { AuthenticatedUser } from '@kbn/security-plugin/common'; | ||||||
| import { findLastIndex } from 'lodash'; | ||||||
|
|
@@ -388,6 +389,11 @@ export function ChatBody({ | |||||
| !conversationCalloutDismissed && | ||||||
| tourCalloutDismissed; | ||||||
|
|
||||||
| const showKnowledgeBaseReIndexingCallout = | ||||||
| knowledgeBase.status.value?.enabled === true && | ||||||
| knowledgeBase.status.value?.kbState === KnowledgeBaseState.READY && | ||||||
| knowledgeBase.status.value?.isReIndexing === true; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit:
Suggested change
|
||||||
|
|
||||||
| const isPublic = conversation.value?.public; | ||||||
| const isArchived = !!conversation.value?.archived; | ||||||
| const showPromptEditor = !isArchived && (!isPublic || isConversationOwnedByCurrentUser); | ||||||
|
|
@@ -529,12 +535,12 @@ export function ChatBody({ | |||||
| ) | ||||||
| } | ||||||
| showElasticLlmCalloutInChat={showElasticLlmCalloutInChat} | ||||||
| showKnowledgeBaseReIndexingCallout={showKnowledgeBaseReIndexingCallout} | ||||||
| /> | ||||||
| ) : ( | ||||||
| <ChatTimeline | ||||||
| conversationId={conversationId} | ||||||
| messages={messages} | ||||||
| knowledgeBase={knowledgeBase} | ||||||
| chatService={chatService} | ||||||
| currentUser={conversationUser} | ||||||
| isConversationOwnedByCurrentUser={isConversationOwnedByCurrentUser} | ||||||
|
|
@@ -556,6 +562,7 @@ export function ChatBody({ | |||||
| onActionClick={handleActionClick} | ||||||
| isArchived={isArchived} | ||||||
| showElasticLlmCalloutInChat={showElasticLlmCalloutInChat} | ||||||
| showKnowledgeBaseReIndexingCallout={showKnowledgeBaseReIndexingCallout} | ||||||
| /> | ||||||
| )} | ||||||
| </EuiPanel> | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,7 @@ | |
|
|
||
| import React, { type ReactNode, useMemo } from 'react'; | ||
| import { css } from '@emotion/css'; | ||
| import { EuiCode, EuiCommentList } from '@elastic/eui'; | ||
| import { EuiCode, EuiCommentList, useEuiTheme } from '@elastic/eui'; | ||
| import type { AuthenticatedUser } from '@kbn/security-plugin/common'; | ||
| import { omit } from 'lodash'; | ||
| import { | ||
|
|
@@ -20,12 +20,12 @@ import { | |
| aiAssistantAnonymizationRules, | ||
| } from '@kbn/observability-ai-assistant-plugin/public'; | ||
| import { AnonymizationRule } from '@kbn/observability-ai-assistant-plugin/common'; | ||
| import type { UseKnowledgeBaseResult } from '../hooks/use_knowledge_base'; | ||
| import { ChatItem } from './chat_item'; | ||
| import { ChatConsolidatedItems } from './chat_consolidated_items'; | ||
| import { getTimelineItemsfromConversation } from '../utils/get_timeline_items_from_conversation'; | ||
| import { useKibana } from '../hooks/use_kibana'; | ||
| import { ElasticLlmConversationCallout } from './elastic_llm_conversation_callout'; | ||
| import { KnowledgeBaseReindexingCallout } from '../knowledge_base/knowledge_base_reindexing_callout'; | ||
|
|
||
| export interface ChatTimelineItem | ||
| extends Pick<Message['message'], 'role' | 'content' | 'function_call'> { | ||
|
|
@@ -53,14 +53,14 @@ export interface ChatTimelineItem | |
| export interface ChatTimelineProps { | ||
| conversationId?: string; | ||
| messages: Message[]; | ||
| knowledgeBase: UseKnowledgeBaseResult; | ||
| chatService: ObservabilityAIAssistantChatService; | ||
| hasConnector: boolean; | ||
| chatState: ChatState; | ||
| isConversationOwnedByCurrentUser: boolean; | ||
| isArchived: boolean; | ||
| currentUser?: Pick<AuthenticatedUser, 'full_name' | 'username'>; | ||
| showElasticLlmCalloutInChat: boolean; | ||
| showKnowledgeBaseReIndexingCallout: boolean; | ||
| onEdit: (message: Message, messageAfterEdit: Message) => void; | ||
| onFeedback: (feedback: Feedback) => void; | ||
| onRegenerate: (message: Message) => void; | ||
|
|
@@ -103,16 +103,11 @@ function highlightContent( | |
| } | ||
| return parts; | ||
| } | ||
|
|
||
| const euiCommentListClassName = css` | ||
| padding-bottom: 32px; | ||
| `; | ||
|
|
||
| const stickyElasticLlmCalloutContainerClassName = css` | ||
| position: sticky; | ||
| top: 0; | ||
| z-index: 1; | ||
| `; | ||
|
|
||
| export function ChatTimeline({ | ||
| conversationId, | ||
| messages, | ||
|
|
@@ -122,6 +117,7 @@ export function ChatTimeline({ | |
| isConversationOwnedByCurrentUser, | ||
| isArchived, | ||
| showElasticLlmCalloutInChat, | ||
| showKnowledgeBaseReIndexingCallout, | ||
| onEdit, | ||
| onFeedback, | ||
| onRegenerate, | ||
|
|
@@ -144,6 +140,17 @@ export function ChatTimeline({ | |
| return { anonymizationEnabled: false }; | ||
| } | ||
| }, [uiSettings]); | ||
| const { euiTheme } = useEuiTheme(); | ||
|
|
||
| const stickyCalloutContainerClassName = css` | ||
| position: sticky; | ||
| top: 0; | ||
| z-index: 1; | ||
| background: ${euiTheme.colors.backgroundBasePlain}; | ||
| &:empty { | ||
| display: none; | ||
| } | ||
|
Comment on lines
+149
to
+152
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Had to add the background here in order to have some padding between the scrollable chat timeline and the callout. And
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Custom CSS tends to break when EUI makes significant changes. I'm not saying we should not do this but we should consider if it's strictly required. Eg if we remove the sticky requirement, can we get rid of the custom css entirely?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Btw. In order to unblock this and get it merged, I'm fine with leaving the custom css in. Let's try to minimize it if possible going forward.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the context @sorenlouv - I agree that this could be made more future-proof for potential EUI changes (hopefully handling of an empty parent within EUI doesn't have another custom behaviour 🤞🏻). |
||
| `; | ||
|
|
||
| const items = useMemo(() => { | ||
| const timelineItems = getTimelineItemsfromConversation({ | ||
|
|
@@ -198,11 +205,10 @@ export function ChatTimeline({ | |
|
|
||
| return ( | ||
| <EuiCommentList className={euiCommentListClassName}> | ||
| {showElasticLlmCalloutInChat ? ( | ||
| <div className={stickyElasticLlmCalloutContainerClassName}> | ||
| <ElasticLlmConversationCallout /> | ||
| </div> | ||
| ) : null} | ||
| <div className={stickyCalloutContainerClassName}> | ||
| {showKnowledgeBaseReIndexingCallout ? <KnowledgeBaseReindexingCallout /> : null} | ||
| {showElasticLlmCalloutInChat ? <ElasticLlmConversationCallout /> : null} | ||
| </div> | ||
| {items.map((item, index) => { | ||
| return Array.isArray(item) ? ( | ||
| <ChatConsolidatedItems | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,144 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License | ||
| * 2.0; you may not use this file except in compliance with the Elastic License | ||
| * 2.0. | ||
| */ | ||
|
|
||
| import React from 'react'; | ||
| import { act, render, screen } from '@testing-library/react'; | ||
| import { KnowledgeBaseState } from '@kbn/observability-ai-assistant-plugin/public'; | ||
| import { WelcomeMessage } from './welcome_message'; | ||
| import type { UseKnowledgeBaseResult } from '../hooks/use_knowledge_base'; | ||
| import type { UseGenAIConnectorsResult } from '../hooks/use_genai_connectors'; | ||
|
|
||
| const mockConnectors: UseGenAIConnectorsResult = { | ||
| connectors: [ | ||
| { | ||
| id: 'test-connector', | ||
| name: 'Test Connector', | ||
| actionTypeId: '.gen-ai', | ||
| isPreconfigured: false, | ||
| isDeprecated: false, | ||
| isSystemAction: false, | ||
| referencedByCount: 0, | ||
| }, | ||
| ], | ||
| loading: false, | ||
| selectedConnector: 'test-connector', | ||
| selectConnector: jest.fn(), | ||
| reloadConnectors: jest.fn(), | ||
| }; | ||
|
|
||
| jest.mock('@kbn/kibana-react-plugin/public', () => ({ | ||
| useKibana: () => ({ | ||
| services: { | ||
| triggersActionsUi: { | ||
| getAddConnectorFlyout: jest.fn(() => null), | ||
| }, | ||
| observabilityAIAssistant: { | ||
| service: { | ||
| getScreenContexts: jest.fn(() => []), | ||
| }, | ||
| useGenAIConnectors: jest.fn(() => mockConnectors), | ||
| }, | ||
| }, | ||
| }), | ||
| })); | ||
|
|
||
| const createMockKnowledgeBase = ( | ||
| partial: Partial<UseKnowledgeBaseResult> = {} | ||
| ): UseKnowledgeBaseResult => ({ | ||
| isInstalling: false, | ||
| isPolling: false, | ||
| install: jest.fn(), | ||
| warmupModel: jest.fn(), | ||
| isWarmingUpModel: false, | ||
| status: { | ||
| value: { | ||
| enabled: true, | ||
| errorMessage: undefined, | ||
| kbState: KnowledgeBaseState.NOT_INSTALLED, | ||
| concreteWriteIndex: undefined, | ||
| currentInferenceId: undefined, | ||
| isReIndexing: false, | ||
| }, | ||
| loading: false, | ||
| error: undefined, | ||
| refresh: jest.fn(), | ||
| }, | ||
| ...partial, | ||
| }); | ||
|
|
||
| describe('WelcomeMessage', () => { | ||
| afterEach(() => { | ||
| jest.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('renders a warning callout while knowledge base is re-indexing', async () => { | ||
| const knowledgeBase = createMockKnowledgeBase({ | ||
| status: { | ||
| value: { | ||
| kbState: KnowledgeBaseState.READY, | ||
| enabled: true, | ||
| concreteWriteIndex: 'my-index', | ||
| currentInferenceId: 'inference_id', | ||
| isReIndexing: true, | ||
| }, | ||
| loading: false, | ||
| error: undefined, | ||
| refresh: jest.fn(), | ||
| }, | ||
| }); | ||
|
|
||
| const { rerender } = render( | ||
| <WelcomeMessage | ||
| knowledgeBase={knowledgeBase} | ||
| connectors={mockConnectors} | ||
| onSelectPrompt={jest.fn()} | ||
| showElasticLlmCalloutInChat={false} | ||
| showKnowledgeBaseReIndexingCallout={true} | ||
| /> | ||
| ); | ||
|
|
||
| // Callout is shown during re-indexing | ||
| expect(screen.queryByText(/Re-indexing in progress/i)).toBeInTheDocument(); | ||
| expect( | ||
| screen.queryByText(/Knowledge base is currently being re-indexed./i) | ||
| ).toBeInTheDocument(); | ||
|
|
||
| // Knowledge base finished re-indexing | ||
| const updatedKnowledgeBase = createMockKnowledgeBase({ | ||
| status: { | ||
| value: { | ||
| kbState: KnowledgeBaseState.READY, | ||
| enabled: true, | ||
| concreteWriteIndex: 'my-index', | ||
| currentInferenceId: 'inference_id', | ||
| isReIndexing: false, | ||
| }, | ||
| loading: false, | ||
| error: undefined, | ||
| refresh: jest.fn(), | ||
| }, | ||
| }); | ||
|
|
||
| await act(async () => { | ||
| rerender( | ||
| <WelcomeMessage | ||
| knowledgeBase={updatedKnowledgeBase} | ||
| connectors={mockConnectors} | ||
| onSelectPrompt={jest.fn()} | ||
| showElasticLlmCalloutInChat={false} | ||
| showKnowledgeBaseReIndexingCallout={false} | ||
| /> | ||
| ); | ||
| }); | ||
|
|
||
| // Callout is no longer shown | ||
| expect(screen.queryByText(/Re-indexing in progress/i)).not.toBeInTheDocument(); | ||
| expect( | ||
| screen.queryByText(/Knowledge base is currently being re-indexed./i) | ||
| ).not.toBeInTheDocument(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License | ||
| * 2.0; you may not use this file except in compliance with the Elastic License | ||
| * 2.0. | ||
| */ | ||
|
|
||
| import React from 'react'; | ||
| import { css } from '@emotion/css'; | ||
| import { EuiCallOut, useEuiTheme } from '@elastic/eui'; | ||
| import { i18n } from '@kbn/i18n'; | ||
|
|
||
| export const KnowledgeBaseReindexingCallout = () => { | ||
| const { euiTheme } = useEuiTheme(); | ||
|
|
||
| const knowledgeBaseReindexingCalloutName = css` | ||
| margin-bottom: ${euiTheme.size.s}; | ||
| width: 100%; | ||
| `; | ||
|
|
||
| return ( | ||
| <EuiCallOut | ||
| className={knowledgeBaseReindexingCalloutName} | ||
| title={i18n.translate('xpack.aiAssistant.knowledgeBase.reindexingCalloutTitle', { | ||
| defaultMessage: 'Re-indexing in progress.', | ||
| })} | ||
| color="warning" | ||
| iconType="alert" | ||
| data-test-subj="knowledgeBaseReindexingCallOut" | ||
| > | ||
| {i18n.translate('xpack.aiAssistant.knowledgeBase.reindexingCalloutBody', { | ||
| defaultMessage: | ||
| 'Knowledge base is currently being re-indexed. If you have entries, some may be unavailable until the operation completes.', | ||
| })} | ||
| </EuiCallOut> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes cause amguous type errors:
Type 'boolean | undefined' is not assignable to type 'boolean'for the flag, so I hope it's all right to leave as is :)