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

Update model to return correct model for CHAT_COMPLETION task type #120326

Merged
merged 12 commits into from
Jan 17, 2025

Conversation

jaybcee
Copy link
Member

@jaybcee jaybcee commented Jan 16, 2025

Description

CHAT_COMPLETION was introduced as a new task type, but the corresponding model for the task type was not changed. Changing it.

It's a bug, but it's not released yet and is a recent change.

Relates to: https://github.com/elastic/search-team/issues/8457

@jaybcee jaybcee added auto-backport Automatically create backport pull requests when merged v8.18.0 labels Jan 16, 2025
@jaybcee jaybcee marked this pull request as ready for review January 16, 2025 20:56
@jonathan-buttner jonathan-buttner added :ml Machine learning Team:ML Meta label for the ML team Feature:GenAI Features around GenAI labels Jan 16, 2025
Copy link
Contributor

@jonathan-buttner jonathan-buttner left a comment

Choose a reason for hiding this comment

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

Thanks for catching this!

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/ml-core (Team:ML)

@jaybcee jaybcee added the >bug label Jan 16, 2025
@elasticsearchmachine
Copy link
Collaborator

Hi @jaybcee, I've created a changelog YAML for you.

@jaybcee jaybcee added >non-issue and removed >bug labels Jan 16, 2025
@jaybcee jaybcee enabled auto-merge (squash) January 16, 2025 21:30
@jaybcee jaybcee disabled auto-merge January 16, 2025 22:01
timgrein

This comment was marked as duplicate.

Copy link
Contributor

@timgrein timgrein left a comment

Choose a reason for hiding this comment

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

Nice catch! 🎣

/**
* This class uses the unified chat completion method.
*/
public class SimpleChatCompletionServiceIntegrationValidator implements ServiceIntegrationValidator {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we've a test class SimpleChatCompletionServiceIntegrationValidatorTests for this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea, let me take care of that 👍

Copy link
Contributor

@timgrein timgrein left a comment

Choose a reason for hiding this comment

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

Thanks for adding the tests! LGTM :shipit:

@jonathan-buttner jonathan-buttner merged commit 408f473 into main Jan 17, 2025
17 checks passed
@jonathan-buttner jonathan-buttner deleted the jbc-bugfix branch January 17, 2025 15:52
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.x

elasticsearchmachine pushed a commit that referenced this pull request Jan 17, 2025
…120326) (#120404)

* Update model to return correct model for CHAT_COMPLETION task type

* Update docs/changelog/120326.yaml

* Delete docs/changelog/120326.yaml

* Fixing chat completion functionality

* Fixing tests

* naming

* Fixing tests

* Adding more tests

---------

Co-authored-by: Jonathan Buttner <[email protected]>
Co-authored-by: Jonathan Buttner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged Feature:GenAI Features around GenAI :ml Machine learning >non-issue Team:ML Meta label for the ML team v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants