-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Security assistant] Conversation pagination patch #196782
Open
stephmilovic
wants to merge
13
commits into
elastic:main
Choose a base branch
from
stephmilovic:pagination_patch
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
+343
−189
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:fix
v9.0.0
Team: SecuritySolution
Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc.
backport:prev-minor
Backport to (8.x) the previous minor version (i.e. one version back from main)
Team:Security Generative AI
Security Generative AI
v8.16.0
labels
Oct 17, 2024
Pinging @elastic/security-solution (Team: SecuritySolution) |
stephmilovic
added
ci:cloud-deploy
Create or update a Cloud deployment
ci:project-deploy-security
Create a Security Serverless Project
labels
Oct 18, 2024
⏳ Build in-progress
History
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
backport:prev-minor
Backport to (8.x) the previous minor version (i.e. one version back from main)
ci:cloud-deploy
Create or update a Cloud deployment
ci:project-deploy-security
Create a Security Serverless Project
release_note:fix
Team:Security Generative AI
Security Generative AI
Team: SecuritySolution
Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc.
v8.16.0
v9.0.0
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
Users with over
20
conversations will experience this error:In the long term we need to implement pagination for the Assistant. Unfortunately, much of our conversation update logic on the frontend requires fetching all conversations. While a larger refactor is needed, this patch increases the fetch limit from 20 to 5000 conversations as a temporary fix.
UI query optimizations made to support 5000 conversations
useFetchCurrentUserConversations
Hook:Updated to accept
fields
andfilter
arguments. We're now retrieving only 3-4 fields per conversation and excluding themessages
field to reduce data load.useAssistantOverlay
Update:Previously, the hook fetched all conversations and mapped them to check for a title match. Now, it filters by
title: ${title}
, retrieving only the relevant conversation instead of fetching up to 5000.Optimization in
security_solution/public/assistant/provider.tsx
:Instead of fetching all conversations to check if any exist,
getUserConversations
was replaced withgetUserConversationsExist
, optimizing the query to return a count only, reducing unnecessary data fetching.Remove conversations fetch in in
security_solution/public/assistant/stack_management/management_settings.tsx
:I discovered a call to fetch all conversations only to locate the Welcome conversation, which was then passed to
ConversationSettingsManagement
inkbn-elastic-assistant
, where all conversations were fetched again. This redundant call was removed, and the export fromkbn-elastic-assistant
was eliminated, as the references are now entirely internal.Extra protection against Welcome conversation exists error
In the case the user has more than 5000 conversations, to prevent the "Welcome conversation exists" error and similar issues, I've added a
sortField
ofis_default
. This ensures that default conversations are returned first, followed by a secondary sort onupdated_at
. In the conversation selector side nav, conversations remain sorted byupdated_at
.Previously,
conversation.isDefault
could be undefined, but it now defaults tofalse
. This change ensures that conversations withoutisDefault
are sorted correctly—without this, new conversations could be pushed to the bottom, even if they were recently updated. We rely on theupdated_at
field to display the most recent conversations at the top.I did not modify the Elasticsearch type to require
is_default
, as legacy conversations won’t have this field. Instead, the value is defaulted tofalse
in the API.