Add streaming to AI Insights#247644
Conversation
| DefaultClientOptions | ||
| >({ http }); | ||
| const fetchInsight = async (signal?: AbortSignal) => { | ||
| const response = await http.fetch('/internal/observability_agent_builder/ai_insights/alert', { |
There was a problem hiding this comment.
Why was the use of the API client reverted. We want to keep using the typed API client from server-route-repository, ideally.
| </EuiButtonEmpty> | ||
| </ButtonSection> | ||
| ) : wasStopped ? ( | ||
| <ButtonSection> |
There was a problem hiding this comment.
- The regenerate button next to the "Start Conversation" (similar to contextual insights) might look a bit better I feel.
- Also can you add a "loading animation/cursor" similar to contextual insights
Screen.Recording.2025-12-30.at.12.55.57.PM.mov
| </EuiFlexGroup> | ||
| </> | ||
| {isLoading ? ( | ||
| <ButtonSection> |
There was a problem hiding this comment.
When the buttons are outside of the EuiPanel it looks a bit odd, can we move them inside the panel similar to contextual insights?
fcb4247 to
be99599
Compare
| const fetchInsight = async () => { | ||
| const response = await apiClient.fetch( | ||
| const fetchInsight = async (signal?: AbortSignal) => { | ||
| const response = (await apiClient.fetch( |
There was a problem hiding this comment.
Can you make apiClient a hook while we're in here as we're repeating some code in the insight components. Something like:
export function useApiClient() {
const {
services: { http },
} = useKibana();
return createRepositoryClient<
ObservabilityAgentBuilderServerRouteRepository,
DefaultClientOptions
>({ http });
}
There was a problem hiding this comment.
Thank you, @neptunian ! It's a good suggestion, I created the hook use_api_client
viduni94
left a comment
There was a problem hiding this comment.
Thanks for the changes @yuliia-fryshko
Added some comments.
Can you also add tests for the observability_agent_builder/public/components/ai_insight/ai_insight.tsx component and use_streaming_ai_insight hook?
| import { AbortError } from '@kbn/kibana-utils-plugin/common'; | ||
| import { httpResponseIntoObservable } from '@kbn/sse-utils-client'; | ||
|
|
||
| interface InsightState { |
There was a problem hiding this comment.
Should this be
| interface InsightState { | |
| interface InsightResponse { |
| }, | ||
| { summary: '', context: '' } | ||
| ), | ||
| takeUntil(abort$) |
There was a problem hiding this comment.
Instead of having setIsLoading(false); in line 36, you probably could do:
| takeUntil(abort$) | |
| takeUntil(abort$), | |
| finalize(() => setIsLoading(false)) |
This guarantees that setIsLoading(false) runs regardless of how the stream ends (complete, error, or unsubscribe)
| takeUntil(abort$) | ||
| ); | ||
|
|
||
| const subscription = observable$.subscribe({ |
There was a problem hiding this comment.
| const subscription = observable$.subscribe({ | |
| subscriptionRef.current = observable$.subscribe({ |
| }, | ||
| }); | ||
|
|
||
| subscriptionRef.current = subscription; |
There was a problem hiding this comment.
| subscriptionRef.current = subscription; |
| asResponse: true, | ||
| rawResponse: true, | ||
| } | ||
| ); | ||
| return response; | ||
| )) as unknown as { response: Response }; |
There was a problem hiding this comment.
Can we avoid manually adding asResponse and rawResponse to each endpoint and also can we avoid as unknown as?
Instead, you can use the inbuilt stream method from the API client:
const createStream = useCallback(
(signal: AbortSignal) =>
apiClient.stream('POST /internal/observability_agent_builder/ai_insights/log', {
signal,
params: {
body: {
alertId,
},
},
}),
[apiClient, alertId]
);
Note: This may require some changes in the use_streaming_ai_insight hook
There was a problem hiding this comment.
Thank you, Viduni! For pointing to it, I wanted refactor it as well a bit .
Replaced manual asResponse/rawResponse flags and type casting with apiClient.stream(), updated useStreamingAiInsight to accept Observable<StreamEvent> instead of Promise<Response>
| asResponse: true, | ||
| rawResponse: true, | ||
| } | ||
| ); | ||
| return { | ||
| summary: response.summary ?? '', | ||
| context: response.context ?? '', | ||
| }; | ||
| )) as unknown as { response: Response }; |
There was a problem hiding this comment.
| asResponse: true, | ||
| rawResponse: true, | ||
| } | ||
| ); | ||
| return { | ||
| summary: response.summary, | ||
| context: response.context, | ||
| }; | ||
| )) as unknown as { response: Response }; |
There was a problem hiding this comment.
| onClick={regenerate} | ||
| > | ||
| {i18n.translate('xpack.observabilityAgentBuilder.aiInsight.regenerateButton', { | ||
| defaultMessage: 'Regenerate', |
| </EuiFlexItem> | ||
| </EuiFlexGroup> | ||
| </> | ||
| {isLoading ? ( |
| try { | ||
| const observable$ = createStream(abortController.signal).pipe( | ||
| filter( | ||
| (event: InsightStreamEvent): event is InsightStreamEvent => |
There was a problem hiding this comment.
This type guard seems redundant as the input is already typed as InsightStreamEvent
There was a problem hiding this comment.
Thank you, Viduni! I removed alsp the condition checks for 'context' | 'chatCompletionChunk' | 'chatCompletionMessage', which are the only possible values of InsightStreamEvent.
| const observable$ = createStream(abortController.signal).pipe( | ||
| filter( | ||
| (event: InsightStreamEvent): event is InsightStreamEvent => | ||
| event.type === 'context' || | ||
| event.type === 'chatCompletionChunk' || | ||
| event.type === 'chatCompletionMessage' | ||
| ), | ||
| map((event: InsightStreamEvent): ParsedEvent => { | ||
| if (event.type === 'context') { | ||
| return { type: 'context', context: event.context }; | ||
| } | ||
| if (event.type === 'chatCompletionChunk') { | ||
| return { type: 'chunk', content: event.content }; | ||
| } | ||
| return { type: 'message', content: event.content }; | ||
| }), | ||
| scan<ParsedEvent, InsightResponse>( | ||
| (acc, event) => { | ||
| if (event.type === 'context') { | ||
| return { ...acc, context: event.context || '' }; | ||
| } | ||
| if (event.type === 'chunk') { | ||
| return { ...acc, summary: acc.summary + (event.content || '') }; | ||
| } | ||
| if (event.type === 'message') { | ||
| return { ...acc, summary: event.content || '' }; | ||
| } | ||
| return acc; | ||
| }, | ||
| { summary: '', context: '' } | ||
| ), |
There was a problem hiding this comment.
Do we need these intermediate mapping steps?
Can this be simplified as:
| const observable$ = createStream(abortController.signal).pipe( | |
| filter( | |
| (event: InsightStreamEvent): event is InsightStreamEvent => | |
| event.type === 'context' || | |
| event.type === 'chatCompletionChunk' || | |
| event.type === 'chatCompletionMessage' | |
| ), | |
| map((event: InsightStreamEvent): ParsedEvent => { | |
| if (event.type === 'context') { | |
| return { type: 'context', context: event.context }; | |
| } | |
| if (event.type === 'chatCompletionChunk') { | |
| return { type: 'chunk', content: event.content }; | |
| } | |
| return { type: 'message', content: event.content }; | |
| }), | |
| scan<ParsedEvent, InsightResponse>( | |
| (acc, event) => { | |
| if (event.type === 'context') { | |
| return { ...acc, context: event.context || '' }; | |
| } | |
| if (event.type === 'chunk') { | |
| return { ...acc, summary: acc.summary + (event.content || '') }; | |
| } | |
| if (event.type === 'message') { | |
| return { ...acc, summary: event.content || '' }; | |
| } | |
| return acc; | |
| }, | |
| { summary: '', context: '' } | |
| ), | |
| const observable$ = createStream(abortController.signal).pipe( | |
| scan<InsightStreamEvent, InsightResponse>( | |
| (acc, event) => { | |
| if (event.type === 'context') { | |
| return { ...acc, context: event.context }; | |
| } | |
| if (event.type === 'chatCompletionChunk') { | |
| return { ...acc, summary: acc.summary + event.content }; | |
| } | |
| if (event.type === 'chatCompletionMessage') { | |
| return { ...acc, summary: event.content }; | |
| } | |
| return acc; | |
| }, | |
| { summary: '', context: '' } | |
| ), |
There was a problem hiding this comment.
Btw, I didn't test the above suggestion. Feel free to make any adjustments after testing.
| expect(result.current.error).toBe('Boom'); | ||
| }); | ||
| unmount(); | ||
| }); |
There was a problem hiding this comment.
Can you add a test that tests calling stop() aborts the stream?
There was a problem hiding this comment.
Good suggestion , I checked the hook and also added a test that verifies the regenerate button appears after stopping the stream
| interface ParsedEvent { | ||
| type: 'context' | 'chunk' | 'message'; | ||
| context?: string; | ||
| content?: string; | ||
| } |
There was a problem hiding this comment.
See comment - https://github.com/elastic/kibana/pull/247644/changes#r2705477560
| interface ParsedEvent { | |
| type: 'context' | 'chunk' | 'message'; | |
| context?: string; | |
| content?: string; | |
| } |
| abortController.signal.addEventListener('abort', () => { | ||
| subscriber.next(); | ||
| subscriber.complete(); | ||
| }); |
There was a problem hiding this comment.
This event listener is added and not removed and can cause a memory leak.
| abortController.signal.addEventListener('abort', () => { | |
| subscriber.next(); | |
| subscriber.complete(); | |
| }); | |
| const handler = () => { | |
| subscriber.next(); | |
| subscriber.complete(); | |
| }; | |
| abortController.signal.addEventListener('abort', handler); | |
| return () => abortController.signal.removeEventListener('abort', handler); |
| if (typeof index !== 'string' || typeof id !== 'string') { | ||
| throw new Error('Index and id must be strings'); | ||
| } |
There was a problem hiding this comment.
Why do we need to throw an error here?
There is a check for this already in line 63 and there is no way createStream can be called if the conditions of this check are satisfied yeah?
Instead you can do a type assertion for the params in line 54 and 55
index: index as string,
id: id as string,
| unmount(); | ||
| }); | ||
|
|
||
| it('shows error banner and retries on click', () => { |
There was a problem hiding this comment.
| it('shows error banner and retries on click', () => { | |
| it('displays an error banner with a retry button when an error occurs that refetches insights when clicked', () => { |
There was a problem hiding this comment.
OR you could consider splitting this into 2 tests and wrap in a describe block.
Example:
describe('when an error occurs', () => {
it('displays an error banner', () => { ... });
it('refetches insights when retry button is clicked', () => { ... });
});
| unmount(); | ||
| }); | ||
|
|
||
| it('renders regenerate and start conversation actions when summary exists', () => { |
There was a problem hiding this comment.
This test is testing too many things in a single test. Consider splitting is as follows:
Example:
describe('when a summary has been generated', () => {
it('displays regenerate and start conversation buttons', () => {
// Just check buttons exist
});
it('calls regenerate when regenerate button is clicked', () => {
// Test regenerate click
});
it('opens the conversation flyout with correct attachments when start conversation is clicked', () => {
// Test start conversation click
});
});
| <EuiButtonEmpty | ||
| data-test-subj="observabilityAgentBuilderRegenerateButton" | ||
| size="s" | ||
| iconType="sparkles" | ||
| onClick={regenerate} | ||
| > |
There was a problem hiding this comment.
We decided not to show the Regenerate button if there is no error yeah?
(Because we can't edit the prompt, there is no point in regenerating as the response will most likely be the same. Ideally it should only be shown if the stream was stopped mid response - and not show when the stream completes)
Did that decision change? (Maybe I missed something)
There was a problem hiding this comment.
I’m fine with removing it. my understanding was that we wouldn’t want to add it as a separate ticket, because it would make sense to handle it within the prompt edit. But since we’re implementing the stop functionality, it wouldn’t hurt to keep it there anyway - we know that the LLM can give slightly different answers each time we call it.
Sorry if this was my misunderstanding. I will remove it, and thank you for pointing it out.
There was a problem hiding this comment.
we know that the LLM can give slightly different answers each time we call it
I don't think a slight difference adds any value without being able to edit the prompt. If we decide to add the prompt editing functionality, we can introduce the Regenerate button at that point. For now, I think it's sufficient to show only when the stream is stopped mid-generation.
There was a problem hiding this comment.
Yes, I removed the Regenerate btn in case we have received a full response
|
Merging |
| expect(buildAttachments).toHaveBeenCalledWith('Hello world', 'context'); | ||
| expect(mockOpenConversationFlyout).toHaveBeenCalledWith({ | ||
| newConversation: true, | ||
| attachments: [{ type: 'test', data: {} }], |
There was a problem hiding this comment.
Can you check for the agentId for OBSERVABILITY_AGENT_ID in here too?
| height: 16px; | ||
| margin-left: 2px; | ||
| vertical-align: middle; | ||
| background-color: ${euiTheme.colors.darkShade}; |
| }); | ||
|
|
||
| describe('when an error occurs', () => { | ||
| it('displays an error banner', () => { |
There was a problem hiding this comment.
The test says "displays an error banner" but doesn't actually verify the error banner and message. We should verify that as well.
| <EuiSpacer size="s" /> | ||
| <EuiFlexGroup justifyContent="flexEnd" gutterSize="s" responsive={false}> | ||
| <EuiFlexItem grow={false}> | ||
| <StartConversationButton onClick={handleStartConversation} /> |
There was a problem hiding this comment.
I can start a conversation when the summary is emtpy
- Click on the insight
- Immediately click on
Stop Generation--> The content is empty - "Start conversation" button is shown --> Click that and continue the conversation
Notice how an empty string is passed as the summary
Screen.Recording.2026-01-21.at.5.22.07.PM.mov
We shouldn't allow starting a conversation when the summary is empty.
There was a problem hiding this comment.
Good catch, @viduni94 ! So, when the user clicked "stop conversation" we should show only "Regenerate " btn (without "Start conversation"), right?
There was a problem hiding this comment.
We should definitely not show "Start conversation" when the summary is empty.
When there's a partial result:
- Old contextual results show the "Start conversation" button
- However, I'm not sure whether there's any value in starting a conversation with a partial summary - maybe there is.
Maybe worth checking with @isaclfreire
There was a problem hiding this comment.
Hmm, I definitely think that if the summary is empty, it doesn’t make sense to start the conversation. But when the summary is partial, I think it depends on which parts are available. I don’t have a strong opinion on this.
I’d propose removing the “Start conversation” button when the summary is empty, and keeping it when the summary is partial. That said, I’m flexible and happy to change this now or later.
There was a problem hiding this comment.
Screen.Recording.2026-01-22.at.20.02.37.mov
There was a problem hiding this comment.
Sounds good! Let's leave it this way and update later if it comes up.
| logger, | ||
| signal: getRequestAbortedSignal(request), | ||
| }), | ||
| }) as IKibanaResponse; |
There was a problem hiding this comment.
as IKibanaResponse is only included in 2 out of the 3 routes.
Can we remove it in all?
| logger, | ||
| signal: getRequestAbortedSignal(request), | ||
| }), | ||
| }) as IKibanaResponse; |
There was a problem hiding this comment.
as IKibanaResponse is only included in 2 out of the 3 routes.
Can we remove it in all?
|
Thanks for the changes @yuliia-fryshko |
viduni94
left a comment
There was a problem hiding this comment.
LGTM
Left a few comments about readability, cursor in dark mode and API client memoizing.
Thanks for all the changes @yuliia-fryshko
| {isLoading ? ( | ||
| <> | ||
| <EuiSpacer size="m" /> | ||
| <EuiHorizontalRule margin="none" /> | ||
| <EuiSpacer size="s" /> | ||
| <EuiFlexGroup justifyContent="flexStart" gutterSize="s" responsive={false}> | ||
| <EuiFlexItem grow={false}> | ||
| <EuiButtonEmpty | ||
| data-test-subj="observabilityAgentBuilderStopGeneratingButton" | ||
| color="text" | ||
| iconType="stop" | ||
| size="s" | ||
| onClick={stop} | ||
| > | ||
| {i18n.translate( | ||
| 'xpack.observabilityAgentBuilder.aiInsight.stopGeneratingButton', | ||
| { | ||
| defaultMessage: 'Stop generating', | ||
| } | ||
| )} | ||
| </EuiButtonEmpty> | ||
| </EuiFlexItem> | ||
| </EuiFlexGroup> | ||
| </> | ||
| ) : wasStopped ? ( | ||
| <> | ||
| <EuiSpacer size="m" /> | ||
| <EuiHorizontalRule margin="none" /> | ||
| <EuiSpacer size="s" /> | ||
| <EuiFlexGroup justifyContent="flexEnd" gutterSize="s" responsive={false}> | ||
| <EuiFlexItem grow={false}> | ||
| <EuiButtonEmpty | ||
| data-test-subj="observabilityAgentBuilderRegenerateButton" | ||
| size="s" | ||
| iconType="sparkles" | ||
| onClick={regenerate} | ||
| > | ||
| {i18n.translate('xpack.observabilityAgentBuilder.aiInsight.regenerateButton', { | ||
| defaultMessage: 'Regenerate', | ||
| })} | ||
| </EuiButtonEmpty> | ||
| </EuiFlexItem> | ||
| {Boolean(summary && summary.trim()) && ( | ||
| <EuiFlexItem grow={false}> | ||
| <StartConversationButton onClick={handleStartConversation} /> | ||
| </EuiFlexItem> | ||
| )} | ||
| </EuiFlexGroup> | ||
| </> | ||
| ) : Boolean(summary && summary.trim()) ? ( | ||
| <> | ||
| <EuiSpacer size="m" /> | ||
| <EuiHorizontalRule margin="none" /> | ||
| <EuiSpacer size="s" /> | ||
| <EuiFlexGroup justifyContent="flexEnd" gutterSize="s" responsive={false}> | ||
| <EuiFlexItem grow={false}> |
There was a problem hiding this comment.
All the conditions here makes it very hard to read this component.
Can we improve it with something like:
const hasSummary = Boolean(summary?.trim());
const renderFooter = () => {
if (isLoading) {
return (
<EuiFlexGroup justifyContent="flexStart" gutterSize="s" responsive={false}>
<EuiFlexItem grow={false}>
<EuiButtonEmpty
data-test-subj="observabilityAgentBuilderStopGeneratingButton"
color="text"
iconType="stop"
size="s"
onClick={stop}
>
{i18n.translate('xpack.observabilityAgentBuilder.aiInsight.stopGeneratingButton', {
defaultMessage: 'Stop generating',
})}
</EuiButtonEmpty>
</EuiFlexItem>
</EuiFlexGroup>
);
}
if (wasStopped) {
return (
<EuiFlexGroup justifyContent="flexEnd" gutterSize="s" responsive={false}>
<EuiFlexItem grow={false}>
<EuiButtonEmpty
data-test-subj="observabilityAgentBuilderRegenerateButton"
size="s"
iconType="sparkles"
onClick={regenerate}
>
{i18n.translate('xpack.observabilityAgentBuilder.aiInsight.regenerateButton', {
defaultMessage: 'Regenerate',
})}
</EuiButtonEmpty>
</EuiFlexItem>
{hasSummary && (
<EuiFlexItem grow={false}>
<StartConversationButton onClick={handleStartConversation} />
</EuiFlexItem>
)}
</EuiFlexGroup>
);
}
if (hasSummary) {
return (
<EuiFlexGroup justifyContent="flexEnd" gutterSize="s" responsive={false}>
<EuiFlexItem grow={false}>
<StartConversationButton onClick={handleStartConversation} />
</EuiFlexItem>
</EuiFlexGroup>
);
}
return null;
};
// already exists
if (
!hasConnectors ||
!agentBuilder ||
!isAgentChatExperienceEnabled ||
!hasAgentBuilderAccess ||
!hasEnterpriseLicense
) {
return null;
}
const footer = renderFooter();
Then in the JSX:
<EuiPanel color="subdued">
{error ? (
<AiInsightErrorBanner error={error} onRetry={fetch} />
) : (
<EuiText size="s">
<EuiMarkdownFormat textSize="s">{summary}</EuiMarkdownFormat>
{isLoading && <LoadingCursor />}
</EuiText>
)}
{footer && (
<>
<EuiSpacer size="m" />
<EuiHorizontalRule margin="none" />
<EuiSpacer size="s" />
{footer}
</>
)}
</EuiPanel>
WDYT?
| height: 16px; | ||
| margin-left: 2px; | ||
| vertical-align: middle; | ||
| background: rgba(0, 0, 0, 0.25); |
| ObservabilityAgentBuilderServerRouteRepository, | ||
| DefaultClientOptions | ||
| >({ http }); | ||
| const apiClient = useApiClient(); |
There was a problem hiding this comment.
This re-creates the API client object on every render. Consider memoizing it in use_api_client.
import { useMemo } from 'react';
export function useApiClient(): RouteRepositoryClient<...> {
const {
services: { http },
} = useKibana();
return useMemo(
() => createRepositoryClient<
ObservabilityAgentBuilderServerRouteRepository,
DefaultClientOptions
>({ http }),
[http]
);
}
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Public APIs missing exports
Page load bundle
History
|





Close #440
This PR adds streaming support to all AI insights (log, error, and alert insights) in the Observability Agent Builder.
Changes in the PR:
getLogAiInsights(),generateErrorAiInsight(),getAlertAiInsight()Stop generatingbutton during streamingExamples:
Screen.Recording.2026-01-08.at.12.39.52.mov
Screen.Recording.2026-01-08.at.12.43.17.mov
Screen.Recording.2026-01-08.at.12.44.35.mov