Skip to content

[Inference] Fixes some OpenAI models not accepting temperature for Inference service#218887

Merged
qn895 merged 9 commits intoelastic:mainfrom
qn895:ai-fix-o3-temperature
May 1, 2025
Merged

[Inference] Fixes some OpenAI models not accepting temperature for Inference service#218887
qn895 merged 9 commits intoelastic:mainfrom
qn895:ai-fix-o3-temperature

Conversation

@qn895
Copy link
Member

@qn895 qn895 commented Apr 22, 2025

Summary

This PR fixes #213652 by adding logic to exclude the temperature param from the body request for those models.

Observability AI Assistant

Before

Screenshot 2025-04-22 at 16 05 28

After
Screenshot 2025-04-22 at 16 03 29

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_note:* 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.

@qn895 qn895 added :ml Team:AI Infra Platform AppEx AI Infrastructure Team t// labels Apr 22, 2025
@qn895 qn895 self-assigned this Apr 22, 2025
: functionCalling === 'simulated';

let request: OpenAIRequest;
const modelName = _modelName ?? connector?.name;
Copy link
Contributor

Choose a reason for hiding this comment

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

connector.name is the name of the stack connector, not the name of the default model the connector is using. You'll have to read the connector's config to retrieve the model name (connector.config.defaultModel for openAI).

Also I think it would probably make sense to have that logic being done inside the getTemperatureIfValid helper (defaulting to checking the connector's default model name if the modelName parameter is unspecified I mean)

Copy link
Member Author

@qn895 qn895 Apr 23, 2025

Choose a reason for hiding this comment

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

Thanks for the hint! Updated here 7afcb5d

@qn895
Copy link
Member Author

qn895 commented Apr 23, 2025

/ci

@qn895 qn895 force-pushed the ai-fix-o3-temperature branch from 2dbc065 to 7afcb5d Compare April 23, 2025 20:47
@qn895
Copy link
Member Author

qn895 commented Apr 23, 2025

/ci

@qn895 qn895 marked this pull request as ready for review April 23, 2025 23:13
@qn895 qn895 requested review from a team as code owners April 23, 2025 23:13
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@elasticmachine
Copy link
Contributor

Pinging @elastic/appex-ai-infra (Team:AI Infra)

@qn895 qn895 added release_note:enhancement backport:version Backport to applied version labels labels Apr 24, 2025
if (connector?.type === InferenceConnectorType.OpenAI && model) {
const normalizedModelName = model.toLowerCase();
const shouldExcludeTemperature = OPENAI_MODELS_WITHOUT_TEMPERATURE.some((m) =>
normalizedModelName.includes(m)
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this be more accurate if we kept a list of regexes to match the model? We could ensure the string is matching the start of the model name and it'd also mean it doesn't need to manually lowercased first.

Something like:

const OPENAI_MODELS_WITHOUT_TEMPERATURE = [/^o1/i, /^o3/i];

....

const shouldExcludeTemperature = OPENAI_MODELS_WITHOUT_TEMPERATURE.some((m) => m.test(model));
Copy link
Member Author

Choose a reason for hiding this comment

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

Looking at the list of official models from OpenAI, I think testing normalizedModelName.startsWith is probably sufficient here. The naming convention with other prefixes starts with gpt-4 so we should be good. Updated in 3afd43a (#218887)

@qn895
Copy link
Member Author

qn895 commented Apr 28, 2025

@elasticmachine merge upstream

@prodsecmachine
Copy link
Collaborator

prodsecmachine commented May 1, 2025

🎉 Snyk checks have passed. No issues have been found so far.

security/snyk check is complete. No issues have been found. (View Details)

license/snyk check is complete. No issues have been found. (View Details)

Copy link
Member

@jgowdyelastic jgowdyelastic left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Tested (with o3-mini) and LGTM.

@qn895 qn895 changed the title [AI Assistant] Fix some OpenAI models not accepting temperature for Inference service May 1, 2025
@qn895 qn895 enabled auto-merge (squash) May 1, 2025 14:59
@qn895 qn895 merged commit 7aabcc6 into elastic:main May 1, 2025
10 checks passed
@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

✅ unchanged

History

cc @qn895

@qn895
Copy link
Member Author

qn895 commented May 2, 2025

💚 All backports created successfully

Status Branch Result
8.19

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

Questions ?

Please refer to the Backport tool documentation

qn895 added a commit to qn895/kibana that referenced this pull request May 2, 2025
…nference service (elastic#218887)

## Summary

This PR fixes elastic#213652 by adding
logic to exclude the `temperature` param from the body request for those
models.

**Observability AI Assistant**

**Before**

<img width="955" alt="Screenshot 2025-04-22 at 16 05 28"
src="https://github.com/user-attachments/assets/496e41ac-926e-41c2-814a-af12ddc17e95"
/>

**After**
<img width="955" alt="Screenshot 2025-04-22 at 16 03 29"
src="https://github.com/user-attachments/assets/1d3ab290-2101-40a8-8f6b-59064e772291"
/>

### 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/src/platform/packages/shared/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_note:*` 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)
- [ ] ...

---------

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
(cherry picked from commit 7aabcc6)
kapral18 added a commit to kapral18/kibana that referenced this pull request May 4, 2025
…ends-crash

* main: (111 commits)
  [ResponseOps][Rules] Cases action title length too long (elastic#219226)
  [main] Sync bundled packages with Package Storage (elastic#219839)
  Fix ignored dynamic templates (elastic#219875)
  Enforce dependency review by kibana-security workflow (elastic#219262)
  [Security Solution] [Detections] Removes tech preview text from eql seq suppression ui (elastic#219870)
  [Security Solution] Fix alerts table potentially not applying alert assignees (elastic#219460)
  fix(slo): alert deletion (elastic#219876)
  [AI4DSOC] fix styling to address cutoff when screen is narrow (elastic#219306)
  [Security Solution][Endpoint] Response action create and history log API updates in of space awareness (elastic#218674)
  Update publish_oas_docs.sh to deploy Kibana Serverless API docs (elastic#219867)
  feat(slo): lock resource installation (elastic#219747)
  [AI4DSOC] Alert flyout code cleanup (elastic#219810)
  [fleet] fixing `isAgentlessDefault` config usage and readability improvements to `isAgentlessSetupDefault` (elastic#219423)
  feat(slo): Bulk delete UI (elastic#219634)
  m1 demo prep (elastic#219588)
  [Security Solution] Replace sourcerer in EQL tab with dataview picker (elastic#218897)
  [AI4DSOC] Attack discovery widget follow up follow up (elastic#219849)
  [AI Assistant] Fix some OpenAI models not accepting temperature for Inference service (elastic#218887)
  Update dependency msw to ~2.7.5 (main) (elastic#219289)
  Use new client URLs in doc link service (elastic#219600)
  ...
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label May 5, 2025
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.
cc: @qn895

qn895 added a commit that referenced this pull request May 6, 2025
…e for Inference service (#218887) (#219962)

# Backport

This will backport the following commits from `main` to `8.19`:
- [[AI Assistant] Fix some OpenAI models not accepting temperature for
Inference service
(#218887)](#218887)

<!--- Backport version: 9.6.6 -->

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

<!--BACKPORT [{"author":{"name":"Quynh Nguyen
(Quinn)","email":"43350163+qn895@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-05-01T16:23:27Z","message":"[AI
Assistant] Fix some OpenAI models not accepting temperature for
Inference service (#218887)\n\n## Summary\n\nThis PR fixes
#213652 by adding\nlogic to
exclude the `temperature` param from the body request for
those\nmodels.\n\n\n**Observability AI Assistant**\n\n**Before**\n\n<img
width=\"955\" alt=\"Screenshot 2025-04-22 at 16 05
28\"\nsrc=\"https://github.com/user-attachments/assets/496e41ac-926e-41c2-814a-af12ddc17e95\"\n/>\n\n**After**\n<img
width=\"955\" alt=\"Screenshot 2025-04-22 at 16 03
29\"\nsrc=\"https://github.com/user-attachments/assets/1d3ab290-2101-40a8-8f6b-59064e772291\"\n/>\n\n\n###
Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers
should verify this PR satisfies this list as well.\n\n- [ ] Any text
added follows [EUI's
writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\nsentence case text and includes
[i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n-
[
]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas
added for features that require explanation or tutorials\n- [ ] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common scenarios\n- [ ] If a plugin
configuration key changed, check if it needs to be\nallowlisted in the
cloud and added to the
[docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n-
[ ] This was checked for breaking HTTP API changes, and any
breaking\nchanges have been approved by the breaking-change committee.
The\n`release_note:breaking` label should be applied in these
situations.\n- [ ] [Flaky
Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\nused on any tests changed\n- [ ] The PR description includes the
appropriate Release Notes section,\nand the correct `release_note:*`
label is applied per
the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n\n###
Identify risks\n\nDoes this PR introduce any risks? For example,
consider risks like hard\nto test bugs, performance regression,
potential of data loss.\n\nDescribe the risk, its severity, and
mitigation for each identified\nrisk. Invite stakeholders and evaluate
how to proceed before merging.\n\n- [ ] [See some
risk\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)\n-
[ ] ...\n\n---------\n\nCo-authored-by: Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"7aabcc6f5ecaeca63e1df4b6cd0940414f6f37bb","branchLabelMapping":{"^v9.1.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement",":ml","backport:version","Team:AI
Infra","v9.1.0","v8.19.0"],"title":"[Inference] Fix some OpenAI models
not accepting temperature for Inference
service","number":218887,"url":"https://github.com/elastic/kibana/pull/218887","mergeCommit":{"message":"[AI
Assistant] Fix some OpenAI models not accepting temperature for
Inference service (#218887)\n\n## Summary\n\nThis PR fixes
#213652 by adding\nlogic to
exclude the `temperature` param from the body request for
those\nmodels.\n\n\n**Observability AI Assistant**\n\n**Before**\n\n<img
width=\"955\" alt=\"Screenshot 2025-04-22 at 16 05
28\"\nsrc=\"https://github.com/user-attachments/assets/496e41ac-926e-41c2-814a-af12ddc17e95\"\n/>\n\n**After**\n<img
width=\"955\" alt=\"Screenshot 2025-04-22 at 16 03
29\"\nsrc=\"https://github.com/user-attachments/assets/1d3ab290-2101-40a8-8f6b-59064e772291\"\n/>\n\n\n###
Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers
should verify this PR satisfies this list as well.\n\n- [ ] Any text
added follows [EUI's
writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\nsentence case text and includes
[i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n-
[
]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas
added for features that require explanation or tutorials\n- [ ] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common scenarios\n- [ ] If a plugin
configuration key changed, check if it needs to be\nallowlisted in the
cloud and added to the
[docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n-
[ ] This was checked for breaking HTTP API changes, and any
breaking\nchanges have been approved by the breaking-change committee.
The\n`release_note:breaking` label should be applied in these
situations.\n- [ ] [Flaky
Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\nused on any tests changed\n- [ ] The PR description includes the
appropriate Release Notes section,\nand the correct `release_note:*`
label is applied per
the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n\n###
Identify risks\n\nDoes this PR introduce any risks? For example,
consider risks like hard\nto test bugs, performance regression,
potential of data loss.\n\nDescribe the risk, its severity, and
mitigation for each identified\nrisk. Invite stakeholders and evaluate
how to proceed before merging.\n\n- [ ] [See some
risk\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)\n-
[ ] ...\n\n---------\n\nCo-authored-by: Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"7aabcc6f5ecaeca63e1df4b6cd0940414f6f37bb"}},"sourceBranch":"main","suggestedTargetBranches":["8.19"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/218887","number":218887,"mergeCommit":{"message":"[AI
Assistant] Fix some OpenAI models not accepting temperature for
Inference service (#218887)\n\n## Summary\n\nThis PR fixes
#213652 by adding\nlogic to
exclude the `temperature` param from the body request for
those\nmodels.\n\n\n**Observability AI Assistant**\n\n**Before**\n\n<img
width=\"955\" alt=\"Screenshot 2025-04-22 at 16 05
28\"\nsrc=\"https://github.com/user-attachments/assets/496e41ac-926e-41c2-814a-af12ddc17e95\"\n/>\n\n**After**\n<img
width=\"955\" alt=\"Screenshot 2025-04-22 at 16 03
29\"\nsrc=\"https://github.com/user-attachments/assets/1d3ab290-2101-40a8-8f6b-59064e772291\"\n/>\n\n\n###
Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers
should verify this PR satisfies this list as well.\n\n- [ ] Any text
added follows [EUI's
writing\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\nsentence case text and includes
[i18n\nsupport](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)\n-
[
]\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\nwas
added for features that require explanation or tutorials\n- [ ] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common scenarios\n- [ ] If a plugin
configuration key changed, check if it needs to be\nallowlisted in the
cloud and added to the
[docker\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\n-
[ ] This was checked for breaking HTTP API changes, and any
breaking\nchanges have been approved by the breaking-change committee.
The\n`release_note:breaking` label should be applied in these
situations.\n- [ ] [Flaky
Test\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\nused on any tests changed\n- [ ] The PR description includes the
appropriate Release Notes section,\nand the correct `release_note:*`
label is applied per
the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\n\n###
Identify risks\n\nDoes this PR introduce any risks? For example,
consider risks like hard\nto test bugs, performance regression,
potential of data loss.\n\nDescribe the risk, its severity, and
mitigation for each identified\nrisk. Invite stakeholders and evaluate
how to proceed before merging.\n\n- [ ] [See some
risk\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)\n-
[ ] ...\n\n---------\n\nCo-authored-by: Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"7aabcc6f5ecaeca63e1df4b6cd0940414f6f37bb"}},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label May 6, 2025
akowalska622 pushed a commit to akowalska622/kibana that referenced this pull request May 29, 2025
…nference service (elastic#218887)

## Summary

This PR fixes elastic#213652 by adding
logic to exclude the `temperature` param from the body request for those
models.


**Observability AI Assistant**

**Before**

<img width="955" alt="Screenshot 2025-04-22 at 16 05 28"
src="https://github.com/user-attachments/assets/496e41ac-926e-41c2-814a-af12ddc17e95"
/>

**After**
<img width="955" alt="Screenshot 2025-04-22 at 16 03 29"
src="https://github.com/user-attachments/assets/1d3ab290-2101-40a8-8f6b-59064e772291"
/>


### 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/src/platform/packages/shared/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_note:*` 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)
- [ ] ...

---------

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@nastasha-solomon nastasha-solomon changed the title [Inference] Fix some OpenAI models not accepting temperature for Inference service Jun 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:version Backport to applied version labels :ml release_note:enhancement Team:AI Infra Platform AppEx AI Infrastructure Team t// v8.19.0 v9.1.0

7 participants