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

[Security Assistant] Migrates to LangGraph and adds KB Tools #184554

Merged
merged 26 commits into from
Jun 14, 2024

Conversation

spong
Copy link
Member

@spong spong commented May 30, 2024

Summary

Migrates our existing RAG pipeline to use LangGraph, and adds tools for Knowledge Base retrieval/storage.

When the assistantKnowledgeBaseByDefault FF is enabled, a new branch, callAssistantGraph(), is taken in postActionsConnectorExecuteRoute that exercises the LangGraph implementation. This is a drop-in replacement for the existing callAgentExecutor() in effort to keep adoption as clean and easy as possible.

The new control flow is as follows:

postActionsConnectorExecuteRoute -> callAssistantGraph() -> getDefaultAssistantGraph() -> isStreamingEnabled ? streamGraph() : invokeGraph()

Graph creation is isolated to getDefaultAssistantGraph(), and execution (streaming or not) has been extracted to streamGraph() and invokeGraph() respectively. Note: Streaming currently only works with ChatOpenAI models, but SimpleChatModelStreaming was de-risked and just need to discuss potential solutions with @stephmilovic. See comment here.

DefaultAssistantGraph

To start with a predictable and piecemeal migration, our existing agentExecutor pipeline has been recreated in LangGraph. It consists of a single agent node, either OpenAIFunctionsAgent, or StructuredChatAgent (depending on the backing LLM), a tool executing node, and a conditional edge that routes between the two nodes until there are no more function calls chosen by the agent. This varies from our initial implementation in that multiple tool calls are now supported, so a user could ask about their alerts AND retrieve additional knowledge base information in the same response.

Note

While chat_history has been plumbed into the graph, after discussing with @YulNaumenko we decided to wait to plumb the rest of persistence into the graph until #184485 is merged. I had already plumbed through the chatTitleGeneration node (here), and so will just need to include initial conversation creation and append/update operations.

Knowledge History & KB Tools

Knowledge History is now always added in the initial prompt for any KB documents marked as required, and two new tools were added for creating and recalling KB entries from within the conversation, KnowledgeBaseWriteTool and KnowledgeBaseRetrievalTool respectively. All three methods of storing and retrieving KB content use the kbDataClient for access, and scopes all requests to the authenticatedUser that made the initial request.

Additional Notes:

  • LangChain dependencies have been updated, and a new dependency on LangGraph has been added.

Checklist

Delete any items that are not applicable to this PR.

@spong spong added release_note:skip Skip the PR/issue when compiling release notes Feature:Security Assistant Security Assistant Team:Security Generative AI Security Generative AI v8.15.0 labels May 30, 2024
@spong spong self-assigned this May 30, 2024
@spong spong marked this pull request as ready for review June 4, 2024 06:48
@spong spong requested review from a team as code owners June 4, 2024 06:48
@spong spong requested a review from a team as a code owner June 6, 2024 20:33

const userFilter = [
{
nested: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should extend the filter to support empty("shared") KBs, which are available for all users in the space.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm working through adding the different permutations of kb mock data today for #184974 and will use that to start crafting tests to cover these scenarios.

I want to take a moment and rethink how we're categorizing/namespacing content using kbResource though to see if there will be any issues here. For now I've kept things matching the original KB implementation, and just introduced the user kbResource for all user created entries. We will also need to capture entryType somewhere to differentiate between raw text content and index-backed entries. Would be nice to have a catch-all tags for organization and labeling too.

I will start thinking through more of this early next week as I get back to plumbing through the remainder of the kbDataClient methods and REST API's, but if you have any thoughts here I would love to hear them 🙂

@patrykkopycinski patrykkopycinski requested a review from a team as a code owner June 12, 2024 21:04
@spong
Copy link
Member Author

spong commented Jun 13, 2024

@elasticmachine merge upstream

Copy link
Contributor

@patrykkopycinski patrykkopycinski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you Garrett! 🙇

@spong spong removed the request for review from a team June 13, 2024 22:52
@spong spong enabled auto-merge (squash) June 14, 2024 04:06
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #1 / CustomTimelineDataGridBody should render exactly as snapshots

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
elasticAssistant 32 34 +2

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
elasticAssistant 0 1 +1
Unknown metric groups

API count

id before after diff
elasticAssistant 46 48 +2

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @spong

@spong spong merged commit 199eb64 into elastic:main Jun 14, 2024
36 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jun 14, 2024
@spong spong deleted the kb-tools branch June 14, 2024 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Security Assistant Security Assistant release_note:skip Skip the PR/issue when compiling release notes Team:Security Generative AI Security Generative AI v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants