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

[Inference] Image content #205371

Merged
merged 4 commits into from
Jan 7, 2025
Merged

Conversation

dgieselaar
Copy link
Member

@dgieselaar dgieselaar commented Jan 2, 2025

Adds support for image content parts in the Inference plugin. Only base64 encoded images are supported, as this capability is shared across all three LLM providers.

@dgieselaar dgieselaar added release_note:skip Skip the PR/issue when compiling release notes v9.0.0 v8.18.0 labels Jan 2, 2025
@dgieselaar dgieselaar self-assigned this Jan 2, 2025
@dgieselaar dgieselaar marked this pull request as ready for review January 2, 2025 11:28
@dgieselaar dgieselaar requested a review from a team as a code owner January 2, 2025 11:28
@dgieselaar dgieselaar added the backport:version Backport to applied version labels label Jan 2, 2025
@pgayvallet
Copy link
Contributor

/ci

Copy link
Contributor

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

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

LGTM.

Would you mind adding an entry in the plugin's README too?

@@ -153,7 +153,24 @@ const messagesToBedrock = (messages: Message[]): BedRockMessage[] => {
case MessageRole.User:
return {
role: 'user' as const,
rawContent: [{ type: 'text' as const, text: message.content }],
rawContent: (typeof message.content === 'string'
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT:

rawContent: typeof message.content === 'string' 
  ? [{ text: message.content, type: 'text' } satisfies BedRockTextPart]
  : message.content.map((contentPart) => {
    if (contentPart.type === 'text') {
      return { text: contentPart.text, type: 'text' } satisfies BedRockTextPart;
    } else {
      return {
        type: 'image',
        source: {
          data: contentPart.source.data,
          mediaType: contentPart.source.mimeType,
          type: 'base64',
        },
      } satisfies BedRockImagePart;
    }
}),

Copy link
Member Author

Choose a reason for hiding this comment

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

wdyt about castArray() instead?

Comment on lines +199 to +213
parts: (typeof message.content === 'string' ? [message.content] : message.content).map(
(contentPart) => {
if (typeof contentPart === 'string') {
return { text: contentPart } satisfies Gemini.TextPart;
} else if (contentPart.type === 'text') {
return { text: contentPart.text } satisfies Gemini.TextPart;
}
return {
inlineData: {
data: contentPart.source.data,
mimeType: contentPart.source.mimeType,
},
} satisfies Gemini.InlineDataPart;
}
),
Copy link
Contributor

Choose a reason for hiding this comment

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

Same NIT as for claude, but that's probably a personal preference at this point.

@dgieselaar
Copy link
Member Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/inference-common 46 53 +7
Unknown metric groups

API count

id before after diff
@kbn/inference-common 150 157 +7

History

cc @dgieselaar

@dgieselaar dgieselaar merged commit 2cfc167 into elastic:main Jan 7, 2025
9 checks passed
@dgieselaar dgieselaar deleted the inference-image-content branch January 7, 2025 11:18
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jan 7, 2025
Adds support for image content parts in the Inference plugin. Only
base64 encoded images are supported, as this capability is shared across
all three LLM providers.

---------

Co-authored-by: Elastic Machine <[email protected]>
(cherry picked from commit 2cfc167)
@kibanamachine
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

kibanamachine added a commit that referenced this pull request Jan 7, 2025
# Backport

This will backport the following commits from `main` to `8.x`:
- [[Inference] Image content
(#205371)](#205371)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT [{"author":{"name":"Dario
Gieselaar","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-07T11:17:56Z","message":"[Inference]
Image content (#205371)\n\nAdds support for image content parts in the
Inference plugin. Only\nbase64 encoded images are supported, as this
capability is shared across\nall three LLM
providers.\n\n---------\n\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"2cfc16709d9af1f1b9a7d094f596e0c88bba0179","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","v8.18.0"],"title":"[Inference]
Image
content","number":205371,"url":"https://github.com/elastic/kibana/pull/205371","mergeCommit":{"message":"[Inference]
Image content (#205371)\n\nAdds support for image content parts in the
Inference plugin. Only\nbase64 encoded images are supported, as this
capability is shared across\nall three LLM
providers.\n\n---------\n\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"2cfc16709d9af1f1b9a7d094f596e0c88bba0179"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/205371","number":205371,"mergeCommit":{"message":"[Inference]
Image content (#205371)\n\nAdds support for image content parts in the
Inference plugin. Only\nbase64 encoded images are supported, as this
capability is shared across\nall three LLM
providers.\n\n---------\n\nCo-authored-by: Elastic Machine
<[email protected]>","sha":"2cfc16709d9af1f1b9a7d094f596e0c88bba0179"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Dario Gieselaar <[email protected]>
kowalczyk-krzysztof pushed a commit to kowalczyk-krzysztof/kibana that referenced this pull request Jan 7, 2025
Adds support for image content parts in the Inference plugin. Only
base64 encoded images are supported, as this capability is shared across
all three LLM providers.

---------

Co-authored-by: Elastic Machine <[email protected]>
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 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.

4 participants