[Security Assistant] Fix error handling on new chat #195507
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stephmilovic
added
release_note:skip
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
stephmilovic
commented
Oct 8, 2024
| `${LLM_TYPE}: ${actionResult?.message} - ${actionResult?.serviceMessage}` | ||
| ); | ||
| if (actionResult?.serviceMessage) { | ||
| error.name = actionResult?.serviceMessage; |
There was a problem hiding this comment.
generateChatTitle will first look for error.name and if not fall back to error.message, in order to deliver a terse title. Therefore I'm putting the serviceMessage (comes from LLM directly) on error.name in each of these chat models.
Flaky Test Runner Stats🎉 All tests passed! - kibana-flaky-test-suite-runner#7111[✅] Security Solution AI Assistant - Cypress: 50/50 tests passed. |
|
@elasticmachine merge upstream |
andrew-goldstein
approved these changes
Oct 14, 2024
There was a problem hiding this comment.
Thanks for this fix @stephmilovic!
✅ Desk tested locally with OpenAI, Bedrock, and Gemini connectors, with new and valid -> invalid conversations
LGTM 🚀
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
|
Starting backport for target branches: 8.x |
💚 Build Succeeded
Metrics [docs]
History
|
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Oct 14, 2024
(cherry picked from commit a15940d)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
kibanamachine
added a commit
that referenced
this pull request
Oct 14, 2024
…196215) # Backport This will backport the following commits from `main` to `8.x`: - [[Security Assistant] Fix error handling on new chat (#195507)](#195507) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Steph Milovic","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-14T22:10:43Z","message":"[Security Assistant] Fix error handling on new chat (#195507)","sha":"a15940d9b939dbf29f74dbde28a2a543b8849cc1","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team: SecuritySolution","backport:prev-minor","Team:Security Generative AI","v8.16.0"],"title":"[Security Assistant] Fix error handling on new chat","number":195507,"url":"https://github.com/elastic/kibana/pull/195507","mergeCommit":{"message":"[Security Assistant] Fix error handling on new chat (#195507)","sha":"a15940d9b939dbf29f74dbde28a2a543b8849cc1"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195507","number":195507,"mergeCommit":{"message":"[Security Assistant] Fix error handling on new chat (#195507)","sha":"a15940d9b939dbf29f74dbde28a2a543b8849cc1"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Steph Milovic <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Resolves #191025
Fixes a bug in Security Assistant where the user's message disappears when a New chat is started with a bad connector.
Since the connector is bad and the first LLM call is to create the title, the error was thrown, the chain ended, and
handleLllmResponsewas called with the error message to save as an assistant message. Because the chain exited early, the title was not created, and the user's message was not persisted. By handling the error on the chat title and titling the conversation, the chain continues, saves the user's message, hits the LLM error again when it tries to send the conversation message, andhandleLllmResponseis called with the error message.Previous behavior
a. Title is "New chat"
b. User's message disappears
c. Assistant error message persists
New behavior
a. Title is "Error name else error message, shortened to 60 characters"
b. User's message persists
c. Assistant error message persists
Cypress test
There was a cypress test that led me to discover this bug in the first place that I am now unskipping. I've got a successful Flakey Test Runner build to show you!