[ML] Adding response parsers for custom service #127179

jonathan-buttner · 2025-04-22T18:43:02Z

This PR adds functionality for parsing responses for the upcoming custom service: #125679

The response parsers leverage the MapPathExtractor to retrieve fields from a json response converted into a map.

jonathan-buttner · 2025-04-22T18:43:35Z

...rence/src/main/java/org/elasticsearch/xpack/inference/external/http/retry/ErrorResponse.java

    }
+
+    @Override
+    public boolean equals(Object o) {


Needed these so the assertThat calls in the error parser tests can succeed.

Alternatively could ErrorResponse be a record rather than an class

Yeah that would make it cleaner. At the moment ErrorResponse is a base class and extended in a few places so we'd have to switch that to composition (I don't think we can extend a record 🤔 ).

jonathan-buttner · 2025-04-22T18:44:01Z

...e/src/main/java/org/elasticsearch/xpack/inference/services/custom/CustomServiceSettings.java

+
+package org.elasticsearch.xpack.inference.services.custom;
+
+public class CustomServiceSettings {


Not all of these values are referenced but they will be once I pull in the rest of the custom service changes in future PRs.

jonathan-buttner · 2025-04-22T18:47:44Z

...in/java/org/elasticsearch/xpack/inference/services/custom/response/CustomResponseParser.java

+
+import java.io.IOException;
+
+public interface CustomResponseParser extends ToXContentFragment, NamedWriteable {


The response parsers need to be a named writeable because the service settings will use a factory to construct the appropriate response parser. The response parser is chosen based on the task type of the service which is provided in the PUT request.

I'll register these in a future PR

How do we plan to handle streaming, since HttpResult and StreamingHttpResult are different? We could maybe have CustomerResponseParser<T> and change the current implementation to BaseCustomResponseParser<T> implements CustomResponseParser<HttpResult> so we can make a StreamingBaseCustomResponseParser<T> implements CustomResponseParser<StreamingHttpResult>?

Ah good point, I hadn't really thought about that. I think you're suggestion will work well though.

elasticsearchmachine · 2025-04-22T20:01:01Z

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

prwhelan · 2025-04-23T20:25:26Z

...in/java/org/elasticsearch/xpack/inference/services/custom/response/CustomResponseParser.java

+
+import java.io.IOException;
+
+public interface CustomResponseParser extends ToXContentFragment, NamedWriteable {


How do we plan to handle streaming, since HttpResult and StreamingHttpResult are different? We could maybe have CustomerResponseParser<T> and change the current implementation to BaseCustomResponseParser<T> implements CustomResponseParser<HttpResult> so we can make a StreamingBaseCustomResponseParser<T> implements CustomResponseParser<StreamingHttpResult>?

prwhelan · 2025-04-23T20:36:09Z

...ava/org/elasticsearch/xpack/inference/services/custom/response/ErrorResponseParserTests.java

+
+    private static HttpResult getMockResult(String jsonString) throws IOException {
+        var response = mock(HttpResponse.class);
+        return new HttpResult(response, Strings.toUTF8Bytes(XContentHelper.stripWhitespace(jsonString)));


Ah, stripWhitespace, that's what I need to go from a multi-line pretty-print JSON string to a testable string...

davidkyle

LGTM

davidkyle · 2025-04-23T11:36:33Z

...rence/src/main/java/org/elasticsearch/xpack/inference/external/http/retry/ErrorResponse.java

    }
+
+    @Override
+    public boolean equals(Object o) {


Alternatively could ErrorResponse be a record rather than an class

davidkyle · 2025-04-23T11:48:14Z

...ava/org/elasticsearch/xpack/inference/services/custom/response/BaseCustomResponseParser.java

+
+        if (obj instanceof List<?> == false) {
+            throw new IllegalArgumentException(
+                Strings.format("Extracted field is an invalid type, expected a list but received [%s]", obj.getClass().getSimpleName())


It would be useful to include the name of the field in this error and the others e.g

Extracted field [embedding] is an invalid type, expected a list but received [%s]

...ain/java/org/elasticsearch/xpack/inference/services/custom/response/ErrorResponseParser.java

davidkyle · 2025-04-23T12:23:25Z

...in/java/org/elasticsearch/xpack/inference/services/custom/response/RerankResponseParser.java

+        var rerankIndex = extractOptionalString(responseParserMap, RERANK_PARSER_INDEX, JSON_PARSER, validationException);
+        var documentText = extractOptionalString(responseParserMap, RERANK_PARSER_DOCUMENT_TEXT, JSON_PARSER, validationException);
+
+        if (relevanceScore == null) {


Suggested change

if (relevanceScore == null) {

if (validationException.validationErrors().size() > 0) {

…inference/services/custom/response/ErrorResponseParser.java Co-authored-by: David Kyle <[email protected]>

…m-model-response-parsers

* Adding response parsers for custom service * [CI] Auto commit changes from spotless * Update x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/custom/response/ErrorResponseParser.java Co-authored-by: David Kyle <[email protected]> * Refactoring to include field names in exceptions * Adding list entry index to error message and field names * Addressing feedback for validation exception --------- Co-authored-by: elasticsearchmachine <[email protected]> Co-authored-by: David Kyle <[email protected]>

elasticsearchmachine · 2025-04-28T17:16:57Z

💚 Backport successful

Status	Branch	Result
✅	8.19

* Adding response parsers for custom service * [CI] Auto commit changes from spotless * Update x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/custom/response/ErrorResponseParser.java * Refactoring to include field names in exceptions * Adding list entry index to error message and field names * Addressing feedback for validation exception --------- Co-authored-by: elasticsearchmachine <[email protected]> Co-authored-by: David Kyle <[email protected]>

Adding response parsers for custom service

53af9be

jonathan-buttner added >non-issue :ml Machine learning Team:ML Meta label for the ML team auto-backport Automatically create backport pull requests when merged v8.19.0 v9.1.0 labels Apr 22, 2025

jonathan-buttner commented Apr 22, 2025

View reviewed changes

[CI] Auto commit changes from spotless

5b8c573

jonathan-buttner marked this pull request as ready for review April 22, 2025 20:00

prwhelan approved these changes Apr 23, 2025

View reviewed changes

davidkyle approved these changes Apr 24, 2025

View reviewed changes

jonathan-buttner and others added 5 commits April 24, 2025 09:44

Update x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/…

27d2a70

…inference/services/custom/response/ErrorResponseParser.java Co-authored-by: David Kyle <[email protected]>

Refactoring to include field names in exceptions

fa1dd88

Adding list entry index to error message and field names

90d2320

Addressing feedback for validation exception

8685ddd

Merge branch 'main' of github.com:elastic/elasticsearch into ml-custo…

69c00ab

…m-model-response-parsers

jonathan-buttner merged commit 677ab35 into elastic:main Apr 28, 2025
17 checks passed

jonathan-buttner deleted the ml-custom-model-response-parsers branch April 28, 2025 17:15

jonathan-buttner mentioned this pull request Apr 28, 2025

[8.19] [ML] Adding response parsers for custom service (#127179) #127478

Merged


		package org.elasticsearch.xpack.inference.services.custom;

		public class CustomServiceSettings {


		import java.io.IOException;

		public interface CustomResponseParser extends ToXContentFragment, NamedWriteable {

	if (relevanceScore == null) {
	if (validationException.validationErrors().size() > 0) {

[ML] Adding response parsers for custom service #127179

[ML] Adding response parsers for custom service #127179

Uh oh!

Conversation

jonathan-buttner commented Apr 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonathan-buttner Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticsearchmachine commented Apr 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidkyle left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

elasticsearchmachine commented Apr 28, 2025

💚 Backport successful

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jonathan-buttner Apr 22, 2025 •

edited

Loading