Repository navigation
Enable multiple items per content object for Elastic service - #148340
DonalEvans merged 2 commits into
Conversation
Also rename supportsImageEmbeddingContent() to supportsNonTextEmbeddingContent() since content may be audio, video or PDF, not just image.
|
Pinging @elastic/search-inference-team (Team:Search - Inference) |
|
Hi @DonalEvans, I've created a changelog YAML for you. |
🔍 Preview links for changed docs⏳ Building and deploying preview... View progress This comment will be updated with preview links when the build is complete. |
ℹ️ Important: Docs version tagging👋 Thanks for updating the docs! Just a friendly reminder that our docs are now cumulative. This means all 9.x versions are documented on the same page and published off of the main branch, instead of creating separate pages for each minor version. We use applies_to tags to mark version-specific features and changes. Expand for a quick overviewWhen to use applies_to tags:✅ At the page level to indicate which products/deployments the content applies to (mandatory) What NOT to do:❌ Don't remove or replace information that applies to an older version 🤔 Need help?
|
ApprovabilityVerdict: Needs human review This PR enables a new capability (multiple items per content object) for ElasticInferenceService, changing which requests pass validation vs. fail. Combined with an unresolved comment about potential validation bypass in JinaAIService, human review is warranted. You can customize Macroscope's approvability policy. Learn more. |
| @Override | ||
| protected boolean supportsImageEmbeddingContent() { | ||
| protected boolean supportsNonTextEmbeddingContent() { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
🟢 Low jinaai/JinaAIService.java:212
supportsNonTextEmbeddingContent() returning true causes non-image non-text content to bypass the validation in doEmbeddingInfer(...). Since JinaAIActionCreator only handles text and image embeddings, other content types will trigger downstream failures instead of being rejected early with a clear error. Consider restricting this to supportsImageEmbeddingContent() so only image content is allowed through.
@Override
- protected boolean supportsNonTextEmbeddingContent() {
- return true;
+ protected boolean supportsImageEmbeddingContent() {
+ return true;
}🤖 Copy this AI Prompt to have your agent fix this:
In file x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/jinaai/JinaAIService.java around lines 212-215:
`supportsNonTextEmbeddingContent()` returning `true` causes non-image non-text content to bypass the validation in `doEmbeddingInfer(...)`. Since `JinaAIActionCreator` only handles text and image embeddings, other content types will trigger downstream failures instead of being rejected early with a clear error. Consider restricting this to `supportsImageEmbeddingContent()` so only image content is allowed through.
Evidence trail:
server/src/main/java/org/elasticsearch/inference/DataType.java (enum values TEXT, IMAGE, AUDIO, VIDEO, PDF), server/src/main/java/org/elasticsearch/inference/InferenceString.java:131-132 (isNonText = !isText, so IMAGE/AUDIO/VIDEO/PDF are all non-text), x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/SenderService.java:295 (bypass when supportsNonTextEmbeddingContent() is true), x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/jinaai/JinaAIService.java:199-200 (only rejects non-text for non-multimodal models), x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/jinaai/request/JinaAIEmbeddingsRequestEntity.java:96-109 (writeInputs only handles isText and isImage, producing empty object for AUDIO/VIDEO/PDF)
…#148340) Also rename supportsImageEmbeddingContent() to supportsNonTextEmbeddingContent() since content may be audio, video or PDF, not just image.
…#148340) Also rename supportsImageEmbeddingContent() to supportsNonTextEmbeddingContent() since content may be audio, video or PDF, not just image.
Also rename supportsImageEmbeddingContent() to
supportsNonTextEmbeddingContent() since content may be audio, video or PDF, not just image.