[Security Assistant] V2 Knowledge Base Settings feedback and fixes by spong · Pull Request #194354 · elastic/kibana

spong · 2024-09-27T21:54:45Z

Summary

This PR is a follow up to #192665 and addresses a bunch of feedback and fixes including:

Adds support for updating/editing entries
Fixes initial loading experience of the KB Settings Setup/Table
Fixes two bugs where semantic_text and text must be declared for IndexEntries to work
Add new Settings Context Menu items for KB and Alerts
Add support for required entries in initial prompt
- See this trace for included knowledge. Note that the KnowledgeBaseRetrievalTool was not selected.
- Note: All prompts were updated to include the {knowledge_history} placeholder, and not behind the feature flag, as this will just be the empty case until the feature flag is enabled.

TODO (in this or follow-up PR):

Add suggestions to index and fields inputs
Adds URL deeplinking to securityAssistantManagement
Fix bug where updating entry does not re-create embeddings (see comment)
Fix loading indicators when adding/editing entries
API integration tests for update API (@e40pud)

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
- Docs being tracked in [Request] 8.16 Security Assistant Custom Knowledge Bases security-docs#5337 for when feature flag is enabled
Unit or functional tests were updated or added to match the most common scenarios

…elds for indexEntries, and plumbs dataViews for autocomplete

…ish-kb-settings

spong · 2024-10-03T15:35:25Z

...kages/kbn-elastic-assistant/impl/knowledge_base/knowledge_base_settings_management/index.tsx

+  const { data: kbStatus, isFetched } = useKnowledgeBaseStatus({ http, resource: ESQL_RESOURCE });
+  const isKbSetup = isKnowledgeBaseSetup(kbStatus);


@patrykkopycinski, this may need to be updated once #184885 is merged

spong · 2024-10-03T15:38:07Z

x-pack/plugins/elastic_assistant/server/lib/langchain/graphs/default_assistant_graph/prompts.ts

 export const formatPrompt = (prompt: string, additionalPrompt?: string) =>
  ChatPromptTemplate.fromMessages([
    ['system', additionalPrompt ? `${prompt}\n\n${additionalPrompt}` : prompt],
+    ['placeholder', '{knowledge_history}'],


This is not behind the feature flag as it is fine if we return no knowledge history before this feature is fully enabled. Behavior will be just the same as if there is no knowledge history once enabled.

spong · 2024-10-03T15:40:33Z

...ins/elastic_assistant/server/lib/langchain/graphs/default_assistant_graph/nodes/run_agent.ts

+  const knowledgeHistory = await kbDataClient?.getRequiredKnowledgeBaseDocumentEntries();
+
  const agentOutcome = await agentRunnable.withConfig({ tags: [AGENT_NODE_TAG] }).invoke(
    {
      ...state,
+      knowledge_history: `${KNOWLEDGE_HISTORY_PREFIX}\n${
+        knowledgeHistory?.length
+          ? JSON.stringify(knowledgeHistory.map((e) => e.text))
+          : NO_KNOWLEDGE_HISTORY
+      }`,


nit: I had considered pulling this out to it's own AddKnowledgeHistory node (similar to how we fetch chat_history and add to graph state). This would probably be good cleanup

spong · 2024-10-03T15:42:44Z

x-pack/plugins/elastic_assistant/server/ai_assistant_data_clients/knowledge_base/index.ts

        sortField: 'created_at',
        sortOrder: 'asc',
-        filter: `${userFilter}${` AND type:index`}`, // TODO: Support global tools (no user filter), and filter by space as well
+        filter: `${userFilter} AND type:index`,


Note: Removed TODO as the new userFilter covers this case (and it always only queries for the current space).

spong · 2024-10-03T15:43:53Z

x-pack/plugins/elastic_assistant/server/ai_assistant_data_clients/knowledge_base/index.ts

+          `getKnowledgeBaseDocuments() - Similarity Search returned [${JSON.stringify(
+            results.length
+          )}] results`


Just cleaning up console noise

spong · 2024-10-03T15:55:18Z

x-pack/plugins/elastic_assistant/server/lib/data_stream/documents_data_writer.ts

+          // TODO: Plumb pipeline for update operations so that embeddings are regenerated on update
+          // Changing to `final_pipeline` should work as well according to the docs, see `pipeline` param:
+          // https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-update-by-query.html#docs-update-by-query-api-query-params
+          pipeline: '.kibana-elastic-ai-assistant-ingest-pipeline-knowledge-base',


Note: Removing this as it did not work as a fix. Passing pipeline here didn't regenerate embeddings, which should've worked. Perhaps our painless scripting for managing patch operations is getting in the way here? O11y has this working with just an index() operation, so we may have to do that.

Also note that using a final_pipeline didn't work, and caused ELSER/mapping errors on KB initialization, so that's not a solution either.

kibana-ci · 2024-10-04T00:15:05Z

💚 Build Succeeded

Buildkite Build
Commit: 0a22ada

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`integrationAssistant`	562	563	+1
`securitySolution`	5920	5921	+1
total			+2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`securitySolution`	20.5MB	20.6MB	+4.5KB

History

💔 Build #239201 failed ece41b2
💔 Build #238195 failed 59fdf0c

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

cc @spong

# Conflicts: # x-pack/packages/kbn-elastic-assistant/impl/knowledge_base/knowledge_base_settings_management/use_knowledge_base_table.tsx # x-pack/plugins/elastic_assistant/server/lib/langchain/graphs/default_assistant_graph/nodes/run_agent.ts # x-pack/plugins/elastic_assistant/server/lib/langchain/graphs/default_assistant_graph/nodes/translations.ts # x-pack/plugins/elastic_assistant/server/lib/langchain/graphs/default_assistant_graph/prompts.ts

elasticmachine · 2024-10-09T00:23:21Z

💚 Build Succeeded

Buildkite Build
Commit: 1d11ca6

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`integrationAssistant`	563	564	+1
`securitySolution`	5927	5928	+1
total			+2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`securitySolution`	20.6MB	20.6MB	+4.6KB

History

💔 Build #240470 failed 4871398

cc @spong

kibanamachine · 2024-10-09T16:18:07Z

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11258988002

@e40pud

…lastic#194354) ## Summary This PR is a follow up to elastic#192665 and addresses a bunch of feedback and fixes including: - [X] Adds support for updating/editing entries - [X] Fixes initial loading experience of the KB Settings Setup/Table - [X] Fixes two bugs where `semantic_text` and `text` must be declared for `IndexEntries` to work - [X] Add new Settings Context Menu items for KB and Alerts - [X] Add support for `required` entries in initial prompt * See [this trace](https://smith.langchain.com/public/84a17a31-8ce8-4bd9-911e-38a854484dd8/r) for included knowledge. Note that the KnowledgeBaseRetrievalTool was not selected. * Note: All prompts were updated to include the `{knowledge_history}` placeholder, and _not behind the feature flag_, as this will just be the empty case until the feature flag is enabled. TODO (in this or follow-up PR): - [ ] Add suggestions to `index` and `fields` inputs - [ ] Adds URL deeplinking to securityAssistantManagement - [ ] Fix bug where updating entry does not re-create embeddings (see [comment](elastic#194354 (comment))) - [ ] Fix loading indicators when adding/editing entries - [ ] API integration tests for update API (@e40pud) ### Checklist Delete any items that are not applicable to this PR. - [X] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials * Docs being tracked in elastic/security-docs#5337 for when feature flag is enabled - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Patryk Kopycinski <contact@patrykkopycinski.com> (cherry picked from commit 7df3672)

kibanamachine · 2024-10-09T16:22:37Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

@e40pud

…xes (#194354) (#195644) # Backport This will backport the following commits from `main` to `8.x`: - [[Security Assistant] V2 Knowledge Base Settings feedback and fixes (#194354)](#194354)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Garrett Spong <spong@users.noreply.github.com>

Adds bulk_update API, fixes initial loading experience, adds outputfi…

6c0dbc8

…elds for indexEntries, and plumbs dataViews for autocomplete

spong added release_note:skip Skip the PR/issue when compiling release notes v9.0.0 Feature:Security Assistant Security Assistant Team:Security Generative AI Security Generative AI v8.16.0 backport:version Backport to applied version labels labels Sep 27, 2024

spong self-assigned this Sep 27, 2024

Merge branch 'main' of github.com:elastic/kibana into polish-kb-settings

72f4c1c

andrew-goldstein marked this pull request as ready for review September 30, 2024 20:20

andrew-goldstein requested a review from a team as a code owner September 30, 2024 20:20

[CI] Auto-commit changed files from 'node scripts/notice'

59fdf0c

spong marked this pull request as draft September 30, 2024 21:55

spong added 6 commits September 30, 2024 15:56

Merge branch 'polish-kb-settings' of github.com:spong/kibana into pol…

a817b8f

…ish-kb-settings

Adds new context menu for deep linking to stack management settings

a530cfe

Merge branch 'main' of github.com:elastic/kibana into polish-kb-settings

e21b3cf

Adds support for required KB entries

52944c9

Fix test mocks

628ff34

Merge branch 'main' of github.com:elastic/kibana into polish-kb-settings

8a7470e

spong commented Oct 3, 2024

View reviewed changes

spong added 2 commits October 3, 2024 09:56

Remove pipeline fix that didn't work

2d6c7f0

Merge branch 'main' of github.com:elastic/kibana into polish-kb-settings

ece41b2

spong marked this pull request as ready for review October 3, 2024 15:58

spong added 2 commits October 3, 2024 16:32

Fix checks

1d68e40

Merge branch 'main' of github.com:elastic/kibana into polish-kb-settings

0a22ada

patrykkopycinski added 3 commits October 8, 2024 22:40

fix showing index entries

4871398

Merge branch 'main' into polish-kb-settings

1d11ca6

patrykkopycinski approved these changes Oct 9, 2024

View reviewed changes

patrykkopycinski merged commit 7df3672 into elastic:main Oct 9, 2024

kibanamachine mentioned this pull request Oct 9, 2024

[8.x] [Security Assistant] V2 Knowledge Base Settings feedback and fixes (#194354) #195644

Merged

kibanamachine mentioned this pull request Oct 9, 2024

[Security Assistant] Knowledge base conflict fix #195659

Merged

spong deleted the polish-kb-settings branch October 9, 2024 18:14

patrykkopycinski mentioned this pull request Oct 14, 2024

Kb settings followup #195733

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Assistant] V2 Knowledge Base Settings feedback and fixes#194354

[Security Assistant] V2 Knowledge Base Settings feedback and fixes#194354
patrykkopycinski merged 16 commits intoelastic:mainfrom
spong:polish-kb-settings

spong commented Sep 27, 2024 •

edited by kibanamachine

Loading

Uh oh!

spong Oct 3, 2024

Uh oh!

spong Oct 3, 2024

Uh oh!

spong Oct 3, 2024

Uh oh!

spong Oct 3, 2024 •

edited

Loading

Uh oh!

spong Oct 3, 2024

Uh oh!

spong Oct 3, 2024

Uh oh!

kibana-ci commented Oct 4, 2024

Uh oh!

elasticmachine commented Oct 9, 2024

Uh oh!

kibanamachine commented Oct 9, 2024

Uh oh!

kibanamachine commented Oct 9, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		const { data: kbStatus, isFetched } = useKnowledgeBaseStatus({ http, resource: ESQL_RESOURCE });
		const isKbSetup = isKnowledgeBaseSetup(kbStatus);

Conversation

spong commented Sep 27, 2024 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

spong Oct 3, 2024

Choose a reason for hiding this comment

Uh oh!

spong Oct 3, 2024

Choose a reason for hiding this comment

Uh oh!

spong Oct 3, 2024

Choose a reason for hiding this comment

Uh oh!

spong Oct 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spong Oct 3, 2024

Choose a reason for hiding this comment

Uh oh!

spong Oct 3, 2024

Choose a reason for hiding this comment

Uh oh!

kibana-ci commented Oct 4, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

Uh oh!

elasticmachine commented Oct 9, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

Uh oh!

kibanamachine commented Oct 9, 2024

Uh oh!

kibanamachine commented Oct 9, 2024

💚 All backports created successfully

Questions ?

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

spong commented Sep 27, 2024 •

edited by kibanamachine

Loading

spong Oct 3, 2024 •

edited

Loading