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

[AI Connector] Change completion subAction schema to be OpenAI compatible #200249

Merged

Conversation

YulNaumenko
Copy link
Contributor

@YulNaumenko YulNaumenko commented Nov 14, 2024

Summary

Summarize your PR. If it involves visual changes include a screenshot or gif.

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

  • 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
  • Unit or functional tests were updated or added to match the most common scenarios
  • If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
  • This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.
  • Flaky Test Runner was used on any tests changed
  • The PR description includes the appropriate Release Notes section, and the correct release_node:* label is applied per the guidelines

Identify risks

Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging.

* `none` is the default when no tools are present. `auto` is the default if tools
* are present.
*/
tool_choice: schema.maybe(
Copy link
Contributor

Choose a reason for hiding this comment

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

Am I supposed to do a transform on the OpenAI function_call to make it into tool_choice?

function_call: schema.maybe(
schema.oneOf([
schema.literal('none'),
schema.literal('auto'),
schema.object(
{
name: schema.string(),
},
{ unknowns: 'ignore' }
),
])
),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

correct, we should stop using the deprecated way of function_call usage and migrate to tool_choice and tools

@YulNaumenko YulNaumenko marked this pull request as ready for review December 12, 2024 16:06
@YulNaumenko YulNaumenko requested review from a team as code owners December 12, 2024 16:06
@YulNaumenko YulNaumenko self-assigned this Dec 16, 2024
@YulNaumenko YulNaumenko added 8.18 candidate v8.18.0 v9.0.0 release_note:skip Skip the PR/issue when compiling release notes backport:version Backport to applied version labels labels Dec 16, 2024
…:YulNaumenko/kibana into ai-connector-inference-completion-openai
@YulNaumenko YulNaumenko requested a review from a team as a code owner December 16, 2024 05:45
).rejects.toThrow('API Error');
});

it('responds with a readable stream', async () => {
const response = await connector.performApiCompletionStream({
input: 'What is Elastic?',
const response = await connector.performApiUnifiedCompletionStream({
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please add tests for the async iterator method as well?

Copy link
Contributor

@ymao1 ymao1 left a comment

Choose a reason for hiding this comment

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

response ops changes LGTM. looks like all schema changes are to non-persisted schemas so no intermediate release issues.

Copy link
Contributor

@stephmilovic stephmilovic left a comment

Choose a reason for hiding this comment

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

I have a draft PR to test Security AI Assistant and Attack Discovery. Both are working with the inference connector configured with OpenAI. Nice work Yuliia! LGTM

@YulNaumenko YulNaumenko enabled auto-merge (squash) December 18, 2024 03:45
@YulNaumenko YulNaumenko merged commit 6eaa1d0 into elastic:main Dec 18, 2024
8 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
stackConnectors 659.5KB 660.5KB +1.0KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
stackConnectors 58.5KB 59.0KB +481.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
stackConnectors 128 129 +1

Total ESLint disabled count

id before after diff
stackConnectors 134 135 +1

History

cc @YulNaumenko

@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.x Backport failed because of merge conflicts

You might need to backport the following PRs to 8.x:
- [Inference Connector] Changed UI/UX due to the new RFC for the _inference/_service (#203363)
- [Inference Connector] Modified getProvider to use _inference/_services ES API instead of hardcoded values. (#199047)

Manual backport

To create the backport manually run:

node scripts/backport --pr 200249

Questions ?

Please refer to the Backport tool documentation

pgayvallet pushed a commit to pgayvallet/kibana that referenced this pull request Dec 18, 2024
…ible (elastic#200249)

…

## Summary

Summarize your PR. If it involves visual changes include a screenshot or
gif.

### Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

- [ ] 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
- [ ] [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
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] The PR description includes the appropriate Release Notes section,
and the correct `release_node:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

### Identify risks

Does this PR introduce any risks? For example, consider risks like hard
to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified
risk. Invite stakeholders and evaluate how to proceed before merging.

- [ ] [See some risk
examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
- [ ] ...

(cherry picked from commit 6eaa1d0)
@pgayvallet
Copy link
Contributor

💚 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

pgayvallet added a commit that referenced this pull request Dec 18, 2024
…compatible (#200249) (#204701)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[AI Connector] Change completion subAction schema to be OpenAI
compatible (#200249)](#200249)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Yuliia
Naumenko","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-18T05:13:10Z","message":"[AI
Connector] Change completion subAction schema to be OpenAI compatible
(#200249)\n\n…\r\n\r\n## Summary\r\n\r\nSummarize your PR. If it
involves visual changes include a screenshot or\r\ngif.\r\n\r\n\r\n###
Checklist\r\n\r\nCheck the PR satisfies following conditions.
\r\n\r\nReviewers should verify this PR satisfies this list as
well.\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] If a plugin
configuration key changed, check if it needs to be\r\nallowlisted in the
cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This was checked for breaking HTTP API changes, and any
breaking\r\nchanges have been approved by the breaking-change committee.
The\r\n`release_note:breaking` label should be applied in these
situations.\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] The PR description includes
the appropriate Release Notes section,\r\nand the correct
`release_node:*` label is applied per
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n###
Identify risks\r\n\r\nDoes this PR introduce any risks? For example,
consider risks like hard\r\nto test bugs, performance regression,
potential of data loss.\r\n\r\nDescribe the risk, its severity, and
mitigation for each identified\r\nrisk. Invite stakeholders and evaluate
how to proceed before merging.\r\n\r\n- [ ] [See some
risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)\r\n-
[ ]
...","sha":"6eaa1d0633f20c9c45cff2b758da5881c0922f3e","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:version","8.18
candidate","v8.18.0"],"number":200249,"url":"https://github.com/elastic/kibana/pull/200249","mergeCommit":{"message":"[AI
Connector] Change completion subAction schema to be OpenAI compatible
(#200249)\n\n…\r\n\r\n## Summary\r\n\r\nSummarize your PR. If it
involves visual changes include a screenshot or\r\ngif.\r\n\r\n\r\n###
Checklist\r\n\r\nCheck the PR satisfies following conditions.
\r\n\r\nReviewers should verify this PR satisfies this list as
well.\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] If a plugin
configuration key changed, check if it needs to be\r\nallowlisted in the
cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This was checked for breaking HTTP API changes, and any
breaking\r\nchanges have been approved by the breaking-change committee.
The\r\n`release_note:breaking` label should be applied in these
situations.\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] The PR description includes
the appropriate Release Notes section,\r\nand the correct
`release_node:*` label is applied per
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n###
Identify risks\r\n\r\nDoes this PR introduce any risks? For example,
consider risks like hard\r\nto test bugs, performance regression,
potential of data loss.\r\n\r\nDescribe the risk, its severity, and
mitigation for each identified\r\nrisk. Invite stakeholders and evaluate
how to proceed before merging.\r\n\r\n- [ ] [See some
risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)\r\n-
[ ]
...","sha":"6eaa1d0633f20c9c45cff2b758da5881c0922f3e"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/200249","number":200249,"mergeCommit":{"message":"[AI
Connector] Change completion subAction schema to be OpenAI compatible
(#200249)\n\n…\r\n\r\n## Summary\r\n\r\nSummarize your PR. If it
involves visual changes include a screenshot or\r\ngif.\r\n\r\n\r\n###
Checklist\r\n\r\nCheck the PR satisfies following conditions.
\r\n\r\nReviewers should verify this PR satisfies this list as
well.\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [ ] If a plugin
configuration key changed, check if it needs to be\r\nallowlisted in the
cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This was checked for breaking HTTP API changes, and any
breaking\r\nchanges have been approved by the breaking-change committee.
The\r\n`release_note:breaking` label should be applied in these
situations.\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] The PR description includes
the appropriate Release Notes section,\r\nand the correct
`release_node:*` label is applied per
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n###
Identify risks\r\n\r\nDoes this PR introduce any risks? For example,
consider risks like hard\r\nto test bugs, performance regression,
potential of data loss.\r\n\r\nDescribe the risk, its severity, and
mitigation for each identified\r\nrisk. Invite stakeholders and evaluate
how to proceed before merging.\r\n\r\n- [ ] [See some
risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)\r\n-
[ ]
...","sha":"6eaa1d0633f20c9c45cff2b758da5881c0922f3e"}},{"branch":"8.x","label":"v8.18.0","labelRegex":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

---------

Co-authored-by: Yuliia Naumenko <[email protected]>
JoseLuisGJ pushed a commit to JoseLuisGJ/kibana that referenced this pull request Dec 19, 2024
…ible (elastic#200249)

…

## Summary

Summarize your PR. If it involves visual changes include a screenshot or
gif.


### Checklist

Check the PR satisfies following conditions. 

Reviewers should verify this PR satisfies this list as well.

- [ ] 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
- [ ] [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
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] The PR description includes the appropriate Release Notes section,
and the correct `release_node:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

### Identify risks

Does this PR introduce any risks? For example, consider risks like hard
to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified
risk. Invite stakeholders and evaluate how to proceed before merging.

- [ ] [See some risk
examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
- [ ] ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.18 candidate backport:version Backport to applied version labels release_note:skip Skip the PR/issue when compiling release notes v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants