-
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
[Security Solution] Adds support for custom Security Assistant SystemPrompts and Conversations #159365
[Security Solution] Adds support for custom Security Assistant SystemPrompts and Conversations #159365
Conversation
…ant-system-prompt-crud
…ant-system-prompt-crud
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Module Count
Public APIs missing comments
Async chunks
Unknown metric groupsAPI count
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @spong |
setTimeout(() => { | ||
deleteConversation(cId); | ||
}, 0); | ||
// onSystemPromptDeleted(cId); |
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.
Plz delete 🙂
(newOptions: ConversationSelectorOption[]) => { | ||
if (newOptions.length === 0) { | ||
setSelectedOptions([]); | ||
// handleSelectionChange([]); |
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.
Plz delete 🙂
} else if (conversationOptions.findIndex((o) => o.label === newOptions?.[0].label) !== -1) { | ||
setSelectedConversationId(newOptions?.[0].label); | ||
} | ||
// setSelectedConversationId(value ?? DEFAULT_CONVERSATION_TITLE); |
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.
Plz delete 🙂
/> | ||
</EuiFormRow> | ||
|
||
{provider === OpenAiProviderType.OpenAi && <></>} |
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.
Remove or add back specific conversation/api settings
</EuiFlexItem> | ||
<EuiFlexItem grow={false}> | ||
{/* Right offset to compensate for 'selected' icon of EuiSuperSelect since native footers aren't supported*/} | ||
<div style={{ width: '24px' }} /> |
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.
nit: consider creating a const
for the style
export const ADD_SYSTEM_PROMPT = i18n.translate( | ||
'xpack.elasticAssistant.assistant.promptEditor.systemPrompt.systemPromptModal.addSystemPromptTitle', | ||
{ | ||
defaultMessage: 'Add system prompt...', |
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.
Could be caps since proper noun/feature
export const ADD_NEW_SYSTEM_PROMPT = i18n.translate( | ||
'xpack.elasticAssistant.assistant.firstPromptEditor.addNewSystemPrompt', | ||
{ | ||
defaultMessage: 'Add new system prompt...', |
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.
Could be caps b/c proper noun/feature
allQuickPrompts: localStorageQuickPrompts ?? [], | ||
allSystemPrompts: localStorageSystemPrompts ?? [], |
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.
Verify referential integrity WRT re-renders
Pinging @elastic/security-solution (Team: SecuritySolution) |
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.
Thanks for these features @spong! 🙏
Also thanks for paring on the review & desk testing
LGTM 🚀
Summary
Adds the following new abilities to the Security Assistant:
Name
,Prompt
,Default Conversations
, andDefault for New Conversations
System Prompt
setting withinConversation Settings
baseSystemPrompts
so they can be provided to the AssistantContextProvider on a per solution basis. The consolidates assistant dependency defaults to thex-pack/plugins/security_solution/public/assistant/content
andx-pack/packages/kbn-elastic-assistant/impl/content
directories respectively.BASE_SECURITY_SYSTEM_PROMPTS
BASE_SECURITY_CONVERSATIONS
See epic https://github.com/elastic/security-team/issues/6775 (internal) for additional details.
Checklist
Delete any items that are not applicable to this PR.