-
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 AI] Bedrock prompt tuning and inference corrections #209011
Conversation
@@ -155,8 +159,6 @@ export const streamGraph = async ({ | |||
const chunk = data?.chunk; | |||
const msg = chunk.message; | |||
if (msg?.tool_call_chunks && msg?.tool_call_chunks.length > 0) { | |||
// I don't think we hit this anymore because of our check for AGENT_NODE_TAG |
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.
this is actually an important param for OpenAI, removing my comment
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.
Looks great! I was able to see the issues before this PR and confirm they are now resolved. The only thing I was unable to test was this section of the test guide:
To confirm the eval fix, run the ESQL eval with Inference connector selected. Previously, an error showed on the securityAiAssistantManagement?tab=evaluation view after hitting "Perform evaluation..."
because the inference connector did not appear on the evaluation page.
This commit ensures inference shows on eval page |
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Public APIs missing comments
Async chunks
History
|
Starting backport for target branches: 8.18, 8.x, 9.0 |
💔 Some backports could not be created
Note: Successful backport PRs will be merged automatically after passing CI. Manual backportTo create the backport manually run:
Questions ?Please refer to the Backport tool documentation |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…209011) (#209191) # Backport This will backport the following commits from `main` to `9.0`: - [[Security AI] Bedrock prompt tuning and inference corrections (#209011)](#209011) <!--- 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":"2025-01-31T23:14:34Z","message":"[Security AI] Bedrock prompt tuning and inference corrections (#209011)","sha":"0d415a6d3a09200dad48a58851d89d81ef897b81","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.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.18.0","v9.1.0"],"title":"[Security AI] Bedrock prompt tuning and inference corrections","number":209011,"url":"https://github.com/elastic/kibana/pull/209011","mergeCommit":{"message":"[Security AI] Bedrock prompt tuning and inference corrections (#209011)","sha":"0d415a6d3a09200dad48a58851d89d81ef897b81"}},"sourceBranch":"main","suggestedTargetBranches":["9.0","8.18"],"targetPullRequestStates":[{"branch":"9.0","label":"v9.0.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.18","label":"v8.18.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/209011","number":209011,"mergeCommit":{"message":"[Security AI] Bedrock prompt tuning and inference corrections (#209011)","sha":"0d415a6d3a09200dad48a58851d89d81ef897b81"}}]}] BACKPORT--> Co-authored-by: Steph Milovic <[email protected]>
…209011) (#209189) # Backport This will backport the following commits from `main` to `8.x`: - [[Security AI] Bedrock prompt tuning and inference corrections (#209011)](#209011) <!--- 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":"2025-01-31T23:14:34Z","message":"[Security AI] Bedrock prompt tuning and inference corrections (#209011)","sha":"0d415a6d3a09200dad48a58851d89d81ef897b81","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.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.18.0","v9.1.0"],"title":"[Security AI] Bedrock prompt tuning and inference corrections","number":209011,"url":"https://github.com/elastic/kibana/pull/209011","mergeCommit":{"message":"[Security AI] Bedrock prompt tuning and inference corrections (#209011)","sha":"0d415a6d3a09200dad48a58851d89d81ef897b81"}},"sourceBranch":"main","suggestedTargetBranches":["9.0","8.18"],"targetPullRequestStates":[{"branch":"9.0","label":"v9.0.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.18","label":"v8.18.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/209011","number":209011,"mergeCommit":{"message":"[Security AI] Bedrock prompt tuning and inference corrections (#209011)","sha":"0d415a6d3a09200dad48a58851d89d81ef897b81"}}]}] BACKPORT--> Co-authored-by: Steph Milovic <[email protected]>
Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync. |
Summary
@jamesspi noticed some issues when invoking inference with an ESQL query, specifically:
1. Bedrock needs a Respond step
We noticed the model starts streaming back the conversation with the tool. Meaning, we saw this "Certainly..." line coming into the stream:
To address this, I needed to add the same "Respond" step we have for Bedrock. There is no distinction in Bedrock when the stream is communicating with the tool or with the final answer, which is why we needed the "Respond" step. Since inference is currently using different providers behind the scene, I added a check for the provider and if it's Bedrock we need the "Respond" step.
2. The ESQL query was not in the response
Despite the system prompt including
Always return value from NaturalLanguageESQLTool as is.
, this is not the behavior we were seeing with both Bedrock and Inference. Updating the prompt to be even more strict and specific fixed the issue for both Bedrock and Inference. This is the new portion of the prompt that enforces the results from the ESQL tool:ALWAYS return the exact response from NaturalLanguageESQLTool verbatim in the final response, without adding further description.
3. Evals not configured for inference
I tried to run evals to confirm this change and realized that the
post_evaluate
route was not properly configured for inference. To correct this, I updated the agent from the deprecatedcreateOpenAIFunctionsAgent
tocreateOpenAIToolsAgent
and added a condition to ensurellmType === 'inference'
uses the tool calling agent. This update was already made in the other place we have the agent selection logic,callAssistantGraph
.Running the evals after the prompt change improved the accuracy from 94% to 97%
Testing
Testing prompt:
securityAiAssistantManagement?tab=evaluation
view after hitting "Perform evaluation..."