Skip to content
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] error when opening old conversation #176299

Closed
klacabane opened this issue Feb 6, 2024 · 5 comments · Fixed by #197745
Closed

[Obs AI Assistant] error when opening old conversation #176299

klacabane opened this issue Feb 6, 2024 · 5 comments · Fixed by #197745
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Team:Obs AI Assistant Observability AI Assistant

Comments

@klacabane
Copy link
Contributor

klacabane commented Feb 6, 2024

Summary

An error is thrown when picking an old conversation from the chat panel. I was only able to trigger this (on main) when opening the chat from a contextual insight

Steps to reproduce:

error_old_conversation.mov
Uncaught Error: Cannot set initialMessages if initialConversationId is set
    useConversation use_conversation.ts:49
    ChatBody chat_body.tsx:97
    React 12
    unstable_runWithPriority scheduler.development.js:468
    React 6
@klacabane klacabane added bug Fixes for quality problems that affect the customer experience Team:Obs AI Assistant Observability AI Assistant Team:obs-knowledge Observability Experience Knowledge team labels Feb 6, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-knowledge-team (Team:obs-knowledge)

@klacabane klacabane changed the title [observability ai assistant] error when opening old conversation [Obs AI Assistant] error when opening old conversation Feb 6, 2024
@dgieselaar dgieselaar self-assigned this Mar 28, 2024
@dgieselaar dgieselaar removed their assignment Apr 23, 2024
@emma-raffenne emma-raffenne removed the Team:obs-knowledge Observability Experience Knowledge team label Aug 8, 2024
@viduni94 viduni94 self-assigned this Oct 22, 2024
@dgieselaar
Copy link
Member

@viduni94 fwiw, this is a warning that is logged in the console, and it happens because the conversationId changes during the lifecycle of the component, which we don't support because of the complexity of creating a new conversation and then updating the url/conversation id. If you manage to reproduce, I would try to figure out if there is actually a bug, and if not, maybe there's a way that we can prevent the warning.

@viduni94
Copy link
Contributor

viduni94 commented Oct 23, 2024

@viduni94 fwiw, this is a warning that is logged in the console, and it happens because the conversationId changes during the lifecycle of the component, which we don't support because of the complexity of creating a new conversation and then updating the url/conversation id. If you manage to reproduce, I would try to figure out if there is actually a bug, and if not, maybe there's a way that we can prevent the warning.

Hi @dgieselaar
Thanks a lot for the information.
I was able to reproduce the issue.. when I reproduced it, this error crashes the AI assistant and it gets closed automatically (video attached at the end of this comment).
The root cause I found was, when an old conversation is clicked in this context, this condition get's triggered because both initialMessages and initialConversationId are set.
The initialMessages come from the temp conversation triggered from the Contextual Insights (which doesn't have a conversation ID)

Image

I tried fixing the error by not passing initialMessages. This did fix the error and I was able to open an old conversation afterwards.

const { conversation, messages, next, state, stop, saveTitle } = useConversation({
    initialConversationId,
    ...(!initialConversationId ? { initialMessages } : {}),
    initialTitle,
    chatService,
    connectorId: connectors.selectedConnector,
    onConversationUpdate,
  });

However, I realized that as long as initialMessages are set, clicking on "New Chat" will show those messages instead of opening an empty conversation. I've been trying different implementations to figure out how to clear the initialMessages via setMessages([]), but haven't found a reliable method to do it because useConversation is used in the ChatBody and not in the ChatFlyout.

Let me know if you have any thoughts on this. Thank you


Video of reproducing the error:
https://github.com/user-attachments/assets/3346652a-d290-4bc0-b799-baa8811647de

@dgieselaar
Copy link
Member

@viduni94 This code is quite a mess, sorry about that 😄 I think what you should be able to do is to use the conversationService to create a new conversation. That will rotate the key used for the ChatFlyout and it will reset initialMessages as well. Here's an example. If you only do this when it's a new conversation, I think it should be fine - but I haven't tried it.

@viduni94
Copy link
Contributor

viduni94 commented Oct 24, 2024

@viduni94 This code is quite a mess, sorry about that 😄 I think what you should be able to do is to use the conversationService to create a new conversation. That will rotate the key used for the ChatFlyout and it will reset initialMessages as well. Here's an example. If you only do this when it's a new conversation, I think it should be fine - but I haven't tried it.

kibana/x-pack/plugins/observability_solution/observability_ai_assistant/public/components/insight/insight.tsx

Line 128 in ee341d5

service.conversations.openNewConversation({

Hi @dgieselaar
Thanks a lot for the info. And no worries about the code 😄

I did try this solution yesterday, however when I open a new conversation using above method, the flyout closes and reopens instead of remaining open and showing a blank conversation. Hence why I was looking for a way to clear the initial messages instead.

I'll check whether I can use service.conversations.openNewConversation and still persist the open state of the flyout.

Update: service.conversations.openNewConversation always closes the flyout and reopens. Therefore, I'm back to looking for a way to reset the initialMessages

viduni94 added a commit to viduni94/kibana that referenced this issue Oct 29, 2024
viduni94 added a commit to viduni94/kibana that referenced this issue Oct 29, 2024
viduni94 added a commit to viduni94/kibana that referenced this issue Oct 29, 2024
viduni94 added a commit to viduni94/kibana that referenced this issue Oct 31, 2024
viduni94 added a commit to viduni94/kibana that referenced this issue Oct 31, 2024
viduni94 added a commit to viduni94/kibana that referenced this issue Oct 31, 2024
viduni94 added a commit to viduni94/kibana that referenced this issue Oct 31, 2024
viduni94 added a commit to viduni94/kibana that referenced this issue Oct 31, 2024
viduni94 added a commit to viduni94/kibana that referenced this issue Oct 31, 2024
viduni94 added a commit to viduni94/kibana that referenced this issue Oct 31, 2024
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Oct 31, 2024
…c#197745)

Closes elastic#176299

## Summary

### Problem

When a chat is started from contextual insights, `initialMessages` are
being populated from the temp conversation created from the error.
Because the `initialMessages` are present, when an old conversation is
clicked, the conversation ID of that conversation and the
`initalMessages` from the temp chat are passed to the conversation. This
throws an error, as there is a condition which restricts both of these
being passed to `useConversation` which is used by the `ChatBody`.
- Only one of these should be passed (not both)

### Solution
With the current implementation, even though not passing
`initialMessages` allows the user to open previous conversations
successfully, it doesn't allow the user to start a new chat because as
long as initial messages are set, they will be loaded when the New Chat
button is clicked instead of showing a blank conversation. Clearing
initial messages requires re-architecting how the Chat uses the
conversation.

Therefore, as the solution, when a chat is opened from contextual
insights, the ConversationList will be hidden. The user can interact
with the AI Assistant on the opened chat with initial messages from
contextual insights.

(cherry picked from commit 287c1ec)
nreese pushed a commit to nreese/kibana that referenced this issue Nov 1, 2024
…c#197745)

Closes elastic#176299

## Summary

### Problem

When a chat is started from contextual insights, `initialMessages` are
being populated from the temp conversation created from the error.
Because the `initialMessages` are present, when an old conversation is
clicked, the conversation ID of that conversation and the
`initalMessages` from the temp chat are passed to the conversation. This
throws an error, as there is a condition which restricts both of these
being passed to `useConversation` which is used by the `ChatBody`.
- Only one of these should be passed (not both)

### Solution
With the current implementation, even though not passing
`initialMessages` allows the user to open previous conversations
successfully, it doesn't allow the user to start a new chat because as
long as initial messages are set, they will be loaded when the New Chat
button is clicked instead of showing a blank conversation. Clearing
initial messages requires re-architecting how the Chat uses the
conversation.

Therefore, as the solution, when a chat is opened from contextual
insights, the ConversationList will be hidden. The user can interact
with the AI Assistant on the opened chat with initial messages from
contextual insights.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Team:Obs AI Assistant Observability AI Assistant
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants