-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Obs AI Assistant] Instructions & Claude improvements #181058
Changes from all commits
b73f6cd
f2475ab
661520f
4744879
31a2139
0849046
02bba6d
b0713a6
9ebd207
3eaf7f9
8040437
b62414c
847a248
ecf5f15
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 |
---|---|---|
|
@@ -5,7 +5,7 @@ | |
* 2.0. | ||
*/ | ||
|
||
import { concat, last, mergeMap, Observable, shareReplay, withLatestFrom } from 'rxjs'; | ||
import { concat, from, last, mergeMap, Observable, shareReplay, withLatestFrom } from 'rxjs'; | ||
import { | ||
ChatCompletionChunkEvent, | ||
MessageAddEvent, | ||
|
@@ -16,8 +16,32 @@ import { | |
ConcatenatedMessage, | ||
} from './concatenate_chat_completion_chunks'; | ||
|
||
type ConcatenateMessageCallback = ( | ||
concatenatedMessage: ConcatenatedMessage | ||
) => Promise<ConcatenatedMessage>; | ||
|
||
function mergeWithEditedMessage( | ||
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. unrelated change, I'm not sure if mergeMap is supposed to take an async callback, and what happens when the promise is rejected, so I've opted to make it more explicit by using RxJS's |
||
originalMessage: ConcatenatedMessage, | ||
chunkEvent: ChatCompletionChunkEvent, | ||
callback?: ConcatenateMessageCallback | ||
): Observable<MessageAddEvent> { | ||
return from( | ||
(callback ? callback(originalMessage) : Promise.resolve(originalMessage)).then((message) => { | ||
const next: MessageAddEvent = { | ||
type: StreamingChatResponseEventType.MessageAdd as const, | ||
id: chunkEvent.id, | ||
message: { | ||
'@timestamp': new Date().toISOString(), | ||
...message, | ||
}, | ||
}; | ||
return next; | ||
}) | ||
); | ||
} | ||
|
||
export function emitWithConcatenatedMessage( | ||
callback?: (concatenatedMessage: ConcatenatedMessage) => Promise<ConcatenatedMessage> | ||
callback?: ConcatenateMessageCallback | ||
): ( | ||
source$: Observable<ChatCompletionChunkEvent> | ||
) => Observable<ChatCompletionChunkEvent | MessageAddEvent> { | ||
|
@@ -30,17 +54,8 @@ export function emitWithConcatenatedMessage( | |
concatenateChatCompletionChunks(), | ||
last(), | ||
withLatestFrom(source$), | ||
mergeMap(async ([message, chunkEvent]) => { | ||
const next: MessageAddEvent = { | ||
type: StreamingChatResponseEventType.MessageAdd as const, | ||
id: chunkEvent.id, | ||
message: { | ||
'@timestamp': new Date().toISOString(), | ||
...(callback ? await callback(message) : message), | ||
}, | ||
}; | ||
|
||
return next; | ||
mergeMap(([message, chunkEvent]) => { | ||
return mergeWithEditedMessage(message, chunkEvent, callback); | ||
}) | ||
) | ||
); | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,20 +5,21 @@ | |
* 2.0. | ||
*/ | ||
|
||
import { filter, Observable, tap } from 'rxjs'; | ||
import { filter, OperatorFunction, tap } from 'rxjs'; | ||
import { | ||
ChatCompletionError, | ||
ChatCompletionErrorCode, | ||
type StreamingChatResponseEvent, | ||
StreamingChatResponseEventType, | ||
type ChatCompletionErrorEvent, | ||
BufferFlushEvent, | ||
} from '../conversation_complete'; | ||
|
||
export function throwSerializedChatCompletionErrors() { | ||
return <T extends StreamingChatResponseEvent>( | ||
source$: Observable<StreamingChatResponseEvent> | ||
): Observable<Exclude<T, ChatCompletionErrorEvent>> => { | ||
return source$.pipe( | ||
export function throwSerializedChatCompletionErrors< | ||
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. Simplifying the type |
||
T extends StreamingChatResponseEvent | BufferFlushEvent | ||
>(): OperatorFunction<T, Exclude<T, ChatCompletionErrorEvent>> { | ||
return (source$) => | ||
source$.pipe( | ||
tap((event) => { | ||
// de-serialise error | ||
if (event.type === StreamingChatResponseEventType.ChatCompletionError) { | ||
|
@@ -33,5 +34,4 @@ export function throwSerializedChatCompletionErrors() { | |
event.type !== StreamingChatResponseEventType.ChatCompletionError | ||
) | ||
); | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,13 +10,55 @@ import { | |
EuiFlexGroup, | ||
EuiFlexItem, | ||
EuiPanel, | ||
UseEuiTheme, | ||
useEuiTheme, | ||
} from '@elastic/eui'; | ||
import { css } from '@emotion/css'; | ||
import { i18n } from '@kbn/i18n'; | ||
import React from 'react'; | ||
import { ChatActionClickHandler, ChatActionClickType } from '../chat/types'; | ||
|
||
const getCodeBlockClassName = (theme: UseEuiTheme) => css` | ||
background-color: ${theme.euiTheme.colors.lightestShade}; | ||
.euiCodeBlock__pre { | ||
margin-bottom: 0; | ||
padding: ${theme.euiTheme.size.m}; | ||
min-block-size: 48px; | ||
} | ||
.euiCodeBlock__controls { | ||
inset-block-start: ${theme.euiTheme.size.m}; | ||
inset-inline-end: ${theme.euiTheme.size.m}; | ||
} | ||
`; | ||
Comment on lines
+21
to
+32
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. I always get suspicious when I see custom css. Is this an oversight from EUI or is our use case different? 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. |
||
|
||
function CodeBlockWrapper({ children }: { children: React.ReactNode }) { | ||
const theme = useEuiTheme(); | ||
return ( | ||
<EuiPanel | ||
hasShadow={false} | ||
hasBorder={false} | ||
paddingSize="s" | ||
className={getCodeBlockClassName(theme)} | ||
> | ||
{children} | ||
</EuiPanel> | ||
); | ||
} | ||
|
||
export function CodeBlock({ children }: { children: React.ReactNode }) { | ||
return ( | ||
<CodeBlockWrapper> | ||
<EuiFlexGroup direction="column" gutterSize="xs"> | ||
<EuiFlexItem grow={false}> | ||
<EuiCodeBlock isCopyable fontSize="m"> | ||
{children} | ||
</EuiCodeBlock> | ||
</EuiFlexItem> | ||
</EuiFlexGroup> | ||
</CodeBlockWrapper> | ||
); | ||
} | ||
|
||
export function EsqlCodeBlock({ | ||
value, | ||
actionsDisabled, | ||
|
@@ -26,26 +68,8 @@ export function EsqlCodeBlock({ | |
actionsDisabled: boolean; | ||
onActionClick: ChatActionClickHandler; | ||
}) { | ||
const theme = useEuiTheme(); | ||
|
||
return ( | ||
<EuiPanel | ||
hasShadow={false} | ||
hasBorder={false} | ||
paddingSize="s" | ||
className={css` | ||
background-color: ${theme.euiTheme.colors.lightestShade}; | ||
.euiCodeBlock__pre { | ||
margin-bottom: 0; | ||
padding: ${theme.euiTheme.size.m}; | ||
min-block-size: 48px; | ||
} | ||
.euiCodeBlock__controls { | ||
inset-block-start: ${theme.euiTheme.size.m}; | ||
inset-inline-end: ${theme.euiTheme.size.m}; | ||
} | ||
`} | ||
> | ||
<CodeBlockWrapper> | ||
<EuiFlexGroup direction="column" gutterSize="xs"> | ||
<EuiFlexItem grow={false}> | ||
<EuiCodeBlock isCopyable fontSize="m"> | ||
|
@@ -87,6 +111,6 @@ export function EsqlCodeBlock({ | |
</EuiFlexGroup> | ||
</EuiFlexItem> | ||
</EuiFlexGroup> | ||
</EuiPanel> | ||
</CodeBlockWrapper> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,7 +21,7 @@ import type { Code, InlineCode, Parent, Text } from 'mdast'; | |
import React, { useMemo, useRef } from 'react'; | ||
import type { Node } from 'unist'; | ||
import { ChatActionClickHandler } from '../chat/types'; | ||
import { EsqlCodeBlock } from './esql_code_block'; | ||
import { CodeBlock, EsqlCodeBlock } from './esql_code_block'; | ||
|
||
interface Props { | ||
content: string; | ||
|
@@ -104,6 +104,9 @@ const esqlLanguagePlugin = () => { | |
|
||
if (node.type === 'code' && node.lang === 'esql') { | ||
node.type = 'esql'; | ||
} else if (node.type === 'code') { | ||
// switch to type that allows us to control rendering | ||
node.type = 'codeBlock'; | ||
} | ||
}; | ||
|
||
|
@@ -131,6 +134,14 @@ export function MessageText({ loading, content, onActionClick }: Props) { | |
processingPlugins[1][1].components = { | ||
...components, | ||
cursor: Cursor, | ||
codeBlock: (props) => { | ||
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. this ensures we can also render things like |
||
return ( | ||
<> | ||
<CodeBlock>{props.value}</CodeBlock> | ||
<EuiSpacer size="m" /> | ||
</> | ||
); | ||
}, | ||
esql: (props) => { | ||
return ( | ||
<> | ||
|
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.
it was hallucinating
*
here.