Skip to content

Commit f717d9b

Browse files
reakaleekgithub-code-quality[bot]github-advanced-security[bot]
authored
Add AskAI message feedback endpoint (#2297)
* Add AskAI message feedback endpoint * Adjust otel naming * Add UI onclick handler and other adjustments * Add log sanitization * Use spans and create tests * Remove all control characters as suggested by CodeQL * Validate input * Refactor * Persist reaction selection * Add debounce if user quickly changes selection * Fix other CodeQL errors regarind log sanitization * Fix formatting * Fix test * Dispose correctly * Change scope of services * Potential fix for pull request finding 'Missing Dispose call on local IDisposable' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * Potential fix for pull request finding 'Missing Dispose call on local IDisposable' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * Format * Fix logging format for conversation_id in AgentBuilder * Fix tests * Potential fix for code scanning alert no. 38: Log entries created from user input Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * Re-add fast path for common cases to avoid string allocation * Potential fix for code scanning alert no. 35: Log entries created from user input Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * Potential fix for code scanning alert no. 39: Log entries created from user input Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * Potential fix for code scanning alert no. 43: Log entries created from user input Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * Also remove newlines * Remove user input from logs * Remove this troublesome log for now * Remove logsanitizer * Fix spacing and colors * Convert `ConversationId` to Guid * Potential fix for code scanning alert no. 47: Log entries created from user input Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --------- Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
1 parent d4a6dd2 commit f717d9b

19 files changed

+485
-52
lines changed

‎src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/Chat.test.tsx‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,28 @@ import { cooldownStore } from '../cooldown.store'
22
import { modalStore } from '../modal.store'
33
import { Chat } from './Chat'
44
import { chatStore } from './chat.store'
5+
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
56
import { render, screen, waitFor } from '@testing-library/react'
67
import userEvent from '@testing-library/user-event'
78
import * as React from 'react'
89

10+
// Create a fresh QueryClient for each test
11+
const createTestQueryClient = () =>
12+
new QueryClient({
13+
defaultOptions: {
14+
queries: { retry: false },
15+
mutations: { retry: false },
16+
},
17+
})
18+
19+
// Wrapper component for tests that need React Query
20+
const renderWithQueryClient = (ui: React.ReactElement) => {
21+
const testQueryClient = createTestQueryClient()
22+
return render(
23+
<QueryClientProvider client={testQueryClient}>{ui}</QueryClientProvider>
24+
)
25+
}
26+
927
// Mock only external HTTP calls - fetchEventSource makes the actual API request
1028
jest.mock('@microsoft/fetch-event-source', () => ({
1129
fetchEventSource: jest.fn(),
@@ -103,7 +121,7 @@ describe('Chat Component', () => {
103121
setupMessages()
104122

105123
// Act
106-
render(<Chat />)
124+
renderWithQueryClient(<Chat />)
107125

108126
// Assert - real messages should be rendered
109127
expect(
@@ -124,7 +142,7 @@ describe('Chat Component', () => {
124142
setupMessages()
125143

126144
// Act
127-
render(<Chat />)
145+
renderWithQueryClient(<Chat />)
128146

129147
// Assert
130148
expect(
@@ -138,7 +156,7 @@ describe('Chat Component', () => {
138156
const user = userEvent.setup()
139157

140158
// Act
141-
render(<Chat />)
159+
renderWithQueryClient(<Chat />)
142160
await user.click(
143161
screen.getByRole('button', { name: /clear conversation/i })
144162
)

‎src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/ChatMessage.test.tsx‎

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,27 @@ import { cooldownStore } from '../cooldown.store'
22
import { ApiError } from '../errorHandling'
33
import { ChatMessage } from './ChatMessage'
44
import { ChatMessage as ChatMessageType } from './chat.store'
5+
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
56
import { render, screen } from '@testing-library/react'
67
import * as React from 'react'
78

9+
// Create a fresh QueryClient for each test
10+
const createTestQueryClient = () =>
11+
new QueryClient({
12+
defaultOptions: {
13+
queries: { retry: false },
14+
mutations: { retry: false },
15+
},
16+
})
17+
18+
// Wrapper component for tests that need React Query
19+
const renderWithQueryClient = (ui: React.ReactElement) => {
20+
const testQueryClient = createTestQueryClient()
21+
return render(
22+
<QueryClientProvider client={testQueryClient}>{ui}</QueryClientProvider>
23+
)
24+
}
25+
826
// Reset cooldown store between tests
927
const resetStores = () => {
1028
cooldownStore.setState({
@@ -63,7 +81,7 @@ describe('ChatMessage Component', () => {
6381

6482
it('should render AI message with correct content', () => {
6583
// Act
66-
render(<ChatMessage message={aiMessage} />)
84+
renderWithQueryClient(<ChatMessage message={aiMessage} />)
6785

6886
// Assert
6987
expect(
@@ -75,7 +93,7 @@ describe('ChatMessage Component', () => {
7593

7694
it('should show feedback buttons when complete', () => {
7795
// Act
78-
render(<ChatMessage message={aiMessage} />)
96+
renderWithQueryClient(<ChatMessage message={aiMessage} />)
7997

8098
// Assert
8199
expect(
@@ -92,7 +110,7 @@ describe('ChatMessage Component', () => {
92110

93111
it('should have correct data attributes', () => {
94112
// Act
95-
render(<ChatMessage message={aiMessage} />)
113+
renderWithQueryClient(<ChatMessage message={aiMessage} />)
96114

97115
// Assert
98116
const messageElement = screen
@@ -117,7 +135,7 @@ describe('ChatMessage Component', () => {
117135

118136
it('should render streaming content', () => {
119137
// Act
120-
render(<ChatMessage message={streamingMessage} />)
138+
renderWithQueryClient(<ChatMessage message={streamingMessage} />)
121139

122140
// Assert
123141
const messageElement = screen
@@ -128,7 +146,7 @@ describe('ChatMessage Component', () => {
128146

129147
it('should not show feedback buttons when streaming', () => {
130148
// Act
131-
render(<ChatMessage message={streamingMessage} />)
149+
renderWithQueryClient(<ChatMessage message={streamingMessage} />)
132150

133151
// Assert
134152
expect(
@@ -158,19 +176,23 @@ describe('ChatMessage Component', () => {
158176

159177
it('should show error callout when status is error', () => {
160178
// Act
161-
render(<ChatMessage message={createErrorMessage()} />)
179+
renderWithQueryClient(
180+
<ChatMessage message={createErrorMessage()} />
181+
)
162182

163183
// Assert - error callout should be visible with title
164184
expect(
165185
screen.getByText(/sorry, there was an error/i)
166186
).toBeInTheDocument()
167187
})
168188

169-
it('should display error guidance for 5xx errors', () => {
189+
it('should display error message for 5xx errors', () => {
170190
// Act
171-
render(<ChatMessage message={createErrorMessage()} />)
191+
renderWithQueryClient(
192+
<ChatMessage message={createErrorMessage()} />
193+
)
172194

173-
// Assert - 5xx errors show generic guidance
195+
// Assert - error guidance should be displayed
174196
expect(
175197
screen.getByText(/We are unable to process your request/i)
176198
).toBeInTheDocument()
@@ -189,7 +211,7 @@ describe('ChatMessage Component', () => {
189211

190212
it('should render markdown as HTML', () => {
191213
// Act
192-
render(<ChatMessage message={messageWithMarkdown} />)
214+
renderWithQueryClient(<ChatMessage message={messageWithMarkdown} />)
193215

194216
// Assert - Bold text should be rendered
195217
expect(screen.getByText(/Bold text/)).toBeInTheDocument()

‎src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/ChatMessage.tsx‎

Lines changed: 34 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ import { ApiError } from '../errorHandling'
66
import { AskAiEvent, ChunkEvent, EventTypes } from './AskAiEvent'
77
import { GeneratingStatus } from './GeneratingStatus'
88
import { References } from './RelatedResources'
9-
import { ChatMessage as ChatMessageType } from './chat.store'
9+
import { ChatMessage as ChatMessageType, useConversationId } from './chat.store'
10+
import { useMessageFeedback } from './useMessageFeedback'
1011
import { useStatusMinDisplay } from './useStatusMinDisplay'
1112
import {
1213
EuiButtonIcon,
@@ -197,43 +198,55 @@ const computeAiStatus = (
197198
// Action bar for complete AI messages
198199
const ActionBar = ({
199200
content,
201+
messageId,
200202
onRetry,
201203
}: {
202204
content: string
205+
messageId: string
203206
onRetry?: () => void
204-
}) => (
205-
<EuiFlexGroup
206-
responsive={false}
207-
component="span"
208-
gutterSize="none"
209-
direction="rowReverse"
210-
>
211-
<EuiFlexItem grow={false}>
207+
}) => {
208+
const { euiTheme } = useEuiTheme()
209+
const conversationId = useConversationId()
210+
const { selectedReaction, submitFeedback } = useMessageFeedback(
211+
messageId,
212+
conversationId
213+
)
214+
return (
215+
<div
216+
css={css`
217+
display: flex;
218+
gap: ${euiTheme.size.xxs};
219+
align-items: center;
220+
flex-direction: row-reverse;
221+
`}
222+
>
212223
<EuiToolTip content="Not helpful">
213224
<EuiButtonIcon
214225
aria-label="This answer was not helpful"
215226
iconType="thumbDown"
216227
color="danger"
217228
size="xs"
218229
iconSize="s"
230+
display={
231+
selectedReaction === 'thumbsDown' ? 'base' : 'empty'
232+
}
233+
onClick={() => submitFeedback('thumbsDown')}
219234
/>
220235
</EuiToolTip>
221-
</EuiFlexItem>
222-
<EuiFlexItem grow={false}>
223236
<EuiToolTip content="Mark as helpful">
224237
<EuiButtonIcon
225238
aria-label="This answer was helpful"
226239
iconType="thumbUp"
227240
color="success"
228241
size="xs"
229242
iconSize="s"
243+
display={selectedReaction === 'thumbsUp' ? 'base' : 'empty'}
244+
onClick={() => submitFeedback('thumbsUp')}
230245
/>
231246
</EuiToolTip>
232-
</EuiFlexItem>
233-
<EuiFlexItem grow={false}>
234247
<EuiCopy
235248
textToCopy={content}
236-
beforeMessage="Copy as markdown"
249+
beforeMessage="Copy markdown"
237250
afterMessage="Copied!"
238251
>
239252
{(copy) => (
@@ -242,13 +255,12 @@ const ActionBar = ({
242255
iconType="copy"
243256
size="xs"
244257
iconSize="s"
258+
color="text"
245259
onClick={copy}
246260
/>
247261
)}
248262
</EuiCopy>
249-
</EuiFlexItem>
250-
{onRetry && (
251-
<EuiFlexItem grow={false}>
263+
{onRetry && (
252264
<EuiToolTip content="Request a new answer">
253265
<EuiButtonIcon
254266
aria-label="Request a new answer"
@@ -258,10 +270,10 @@ const ActionBar = ({
258270
iconSize="s"
259271
/>
260272
</EuiToolTip>
261-
</EuiFlexItem>
262-
)}
263-
</EuiFlexGroup>
264-
)
273+
)}
274+
</div>
275+
)
276+
}
265277

266278
export const ChatMessage = ({
267279
message,
@@ -443,6 +455,7 @@ export const ChatMessage = ({
443455
<EuiSpacer size="m" />
444456
<ActionBar
445457
content={mainContent}
458+
messageId={message.id}
446459
onRetry={onRetry}
447460
/>
448461
</>

‎src/Elastic.Documentation.Site/Assets/web-components/SearchOrAskAi/AskAi/chat.store.ts‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { v4 as uuidv4 } from 'uuid'
1717
import { create } from 'zustand/react'
1818

1919
export type AiProvider = 'AgentBuilder' | 'LlmGateway'
20+
export type Reaction = 'thumbsUp' | 'thumbsDown'
2021

2122
export interface ChatMessage {
2223
id: string
@@ -53,10 +54,13 @@ async function computeSHA256(data: string): Promise<string> {
5354
return hashArray.map((b) => ('0' + b.toString(16)).slice(-2)).join('')
5455
}
5556

57+
const sentAiMessageIds = new Set<string>()
58+
5659
interface ChatState {
5760
chatMessages: ChatMessage[]
5861
conversationId: string | null
5962
aiProvider: AiProvider
63+
messageFeedback: Record<string, Reaction> // messageId -> reaction
6064
scrollPosition: number
6165
actions: {
6266
submitQuestion: (question: string) => void
@@ -72,6 +76,7 @@ interface ChatState {
7276
clearChat: () => void
7377
clearNon429Errors: () => void
7478
cancelStreaming: () => void
79+
setMessageFeedback: (messageId: string, reaction: Reaction) => void
7580
abortMessage: (messageId: string) => void
7681
isStreaming: (messageId: string) => boolean
7782
setScrollPosition: (position: number) => void
@@ -80,8 +85,9 @@ interface ChatState {
8085

8186
export const chatStore = create<ChatState>((set, get) => ({
8287
chatMessages: [],
83-
conversationId: null,
84-
aiProvider: 'LlmGateway',
88+
conversationId: null, // Start with null - will be set by backend on first request
89+
aiProvider: 'LlmGateway', // Default to LLM Gateway
90+
messageFeedback: {},
8591
scrollPosition: 0,
8692
actions: {
8793
submitQuestion: (question: string) => {
@@ -152,8 +158,9 @@ export const chatStore = create<ChatState>((set, get) => ({
152158
stream.throttler.clear()
153159
stream.controller.abort()
154160
}
161+
sentAiMessageIds.clear()
155162
activeStreams.clear()
156-
set({ chatMessages: [], conversationId: null })
163+
set({ chatMessages: [], conversationId: null, messageFeedback: {} })
157164
},
158165

159166
clearNon429Errors: () => {
@@ -197,6 +204,15 @@ export const chatStore = create<ChatState>((set, get) => ({
197204
}))
198205
},
199206

207+
setMessageFeedback: (messageId: string, reaction: Reaction) => {
208+
set((state) => ({
209+
messageFeedback: {
210+
...state.messageFeedback,
211+
[messageId]: reaction,
212+
},
213+
}))
214+
},
215+
200216
abortMessage: (messageId) => {
201217
const stream = activeStreams.get(messageId)
202218
if (stream) {
@@ -410,6 +426,8 @@ export const useAiProvider = () => chatStore((state) => state.aiProvider)
410426
export const useChatScrollPosition = () =>
411427
chatStore((state) => state.scrollPosition)
412428
export const useChatActions = () => chatStore((state) => state.actions)
429+
export const useMessageReaction = (messageId: string) =>
430+
chatStore((state) => state.messageFeedback[messageId] ?? null)
413431
export const useIsStreaming = () =>
414432
chatStore((state) => {
415433
const last = state.chatMessages[state.chatMessages.length - 1]

0 commit comments

Comments
 (0)