Simplified linear and RRF retrievers #128633

Mikep86 · 2025-05-29T18:39:03Z

Adds simplified syntaxes for the linear and rrf retrievers:

GET my-index/_search
{
  "retriever": {
    "linear": {
      "fields": ["*"],
      "query": "my awesome query",
      "normalizer": "minmax"
    }
  }
}

GET my-index/_search
{
  "retriever": {
    "rrf": {
      "fields": ["*"],
      "query": "my awesome query"
    }
  }
}

This PR isn't quite complete as YAML tests for the rrf retriever haven't been added yet. But everything else should be in a reviewable state.

elasticsearchmachine · 2025-05-29T18:39:56Z

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

- Do not allow querying remote indices - Handle when index wildcard pattern does not match any indices

Mikep86 · 2025-06-09T16:02:30Z

...-rrf/src/yamlRestTest/resources/rest-api-spec/test/linear/20_linear_retriever_simplified.yml

+  - do:
+      catch: bad_request
+      search:
+        index: test-index
+        body:
+          retriever:
+            linear:
+              fields: [ "text_1", "text_2" ]
+              query: ""
+
+  - contains: { error.root_cause.0.reason: "[linear] [query] cannot be empty" }


Maybe we should handle this by rewriting to a MatchNoneQueryBuilder instead?

It would be interesting to think about whether we want to do a match_none here or even a match_all if the query is not provided. Or even perhaps an exists query if fields are provided. We could look at how match_phrase supports a zero terms query.

However, I think all of that could be done as followup work and for this PR throwing is probably fine.

kderusso

First pass - looks really good, nice work! 👏 I left a few commends and questions!

x-pack/plugin/inference/build.gradle

...-rrf/src/yamlRestTest/resources/rest-api-spec/test/linear/20_linear_retriever_simplified.yml

kderusso · 2025-06-09T18:32:03Z

...-rrf/src/yamlRestTest/resources/rest-api-spec/test/linear/20_linear_retriever_simplified.yml

+  - do:
+      catch: bad_request
+      search:
+        index: test-index
+        body:
+          retriever:
+            linear:
+              fields: [ "text_1", "text_2" ]
+              query: ""
+
+  - contains: { error.root_cause.0.reason: "[linear] [query] cannot be empty" }


It would be interesting to think about whether we want to do a match_none here or even a match_all if the query is not provided. Or even perhaps an exists query if fields are provided. We could look at how match_phrase supports a zero terms query.

However, I think all of that could be done as followup work and for this PR throwing is probably fine.

...-rrf/src/yamlRestTest/resources/rest-api-spec/test/linear/20_linear_retriever_simplified.yml

.../rank-rrf/src/test/java/org/elasticsearch/xpack/rank/linear/LinearRetrieverBuilderTests.java

kderusso · 2025-06-09T18:43:04Z

.../rank-rrf/src/test/java/org/elasticsearch/xpack/rank/linear/LinearRetrieverBuilderTests.java

+        assertEquals("[linear] does not support the simplified query format when querying remote indices", iae.getMessage());
+    }
+
+    private static ResolvedIndices createMockResolvedIndices(


There's some duplication between this and the RRF tests, could that be cleaned up a bit?

100%, there is duplication here. We could probably refactor this into a base class with extensions for linear & rrf. I didn't focus on this immediately in the interest of getting a complete PR out first. Time permitting, we can refcator in this PR or save it for a quick follow-up after feature freeze.

I'm fine with it as a followup post FF

kderusso · 2025-06-09T18:44:56Z

x-pack/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilder.java

+            var localIndicesMetadata = resolvedIndices.getConcreteLocalIndicesMetadata();
+            if (localIndicesMetadata.size() > 1) {
+                throw new IllegalArgumentException(
+                    "[" + NAME + "] does not support the simplified query format when querying multiple indices"


Same nitpick here about error messages. Also, could some more of this type of checking be shared with the linear retriever instead of duplicated?

kderusso · 2025-06-09T18:47:23Z

...rank-rrf/src/test/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilderParsingTests.java

-            + "    }"
-            + "  }"
-            + "}";
+        String restContent = """


Thank you, much more readable 😁

jimczi

Can you split this pull request in two? One for the simplified linear retriever and one for RRF.
I wouldn't expect RRF and linear to implement the same logic and I find it difficult to read since we don't really describe how it works in the description.

Mikep86 · 2025-06-10T13:52:55Z

Closing this in favor of separate PRs for linear & RRF. Linear retriever PR is first: #129200

Mikep86 added 18 commits May 20, 2025 15:32

Add simplified inner retriever parser

a10e92d

Integrate simplified handling into RRF

07e368f

Refactoring

3fc2567

Added doRewrite method

822edac

Add fields and query instance vars

4b06636

RRF retriever parsing updates

2212167

Added custom rewrite logic for RRF retriever

4059894

Remove unused method

1c8dab3

Use a most_fields multi-match query

a217e52

Added TODO

11d1445

Added support for wildcards

7d3e78e

Handle when only query is supplied by the user

c6bf013

Added fields, query, and normalizer params to linear retriever

510fe54

Added custom rewrite logic for linear retriever

187758f

Updated doRewrite method to return rewritten retriever

51a3dbe

Fix NPE

e33cebf

Temporary fix for minmax bug

a777601

Validation improvements

34250dc

Mikep86 added >enhancement auto-backport Automatically create backport pull requests when merged :SearchOrg/Relevance Label for the Search (solution/org) Relevance team :Search Relevance/Search Catch all for Search Relevance v8.19.0 v9.1.0 labels May 29, 2025

Update docs/changelog/128633.yaml

551e7f5

Mikep86 added 4 commits May 29, 2025 14:47

Fix changelog

7267b59

Merge branch 'main' into simplified-linear-and-rrf-retrievers

22594a5

Updated RRFRetrieverBuilderParsingTests

397a1b8

_almost_ working version of rewrite tests

1703f1f

Mikep86 added 18 commits June 4, 2025 16:29

Fix inappropriate reference to RRFRankPlugin.NAME

2915332

Resolve TODO

dc54673

Merge branch 'main' into simplified-linear-and-rrf-retrievers

e5db2e4

Added linear retriever YAML tests

7adb091

Adjust test

0655c1c

Fix inference API calls in YAML tests

54b67c2

Update comment

c9f7a84

Linear boosting test development

e3f7a54

Formatting

1240aa1

Check linear retriever rank window size propagation

1cd8797

Check RRF retriever rank window size and rank constant propagation

f8923c3

Improve edge case handling:

2dc0b44

- Do not allow querying remote indices - Handle when index wildcard pattern does not match any indices

Added more linear YAML tests

6d8eb0b

Added raw vector field query test

ffe7374

Add more linear retriever YAML tests

966b641

Added remote index search test for linear retriever

a84eac4

Added remote index search test for rrf retriever

5632821

Added field boost combination test

f906d41

Mikep86 requested review from jimczi, kderusso and pmpailis June 9, 2025 15:52

Mikep86 commented Jun 9, 2025

View reviewed changes

Merge branch 'main' into simplified-linear-and-rrf-retrievers

42c10c6

kderusso reviewed Jun 9, 2025

View reviewed changes

Added semantic text multi-match query test

7c2dc4d

jimczi reviewed Jun 10, 2025

View reviewed changes

Mikep86 mentioned this pull request Jun 10, 2025

Simplified Linear Retriever #129200

Merged

Mikep86 closed this Jun 10, 2025

Mikep86 added a commit to Mikep86/elasticsearch that referenced this pull request Jun 18, 2025

Apply RRF retriever changes from elastic#128633

34040c9

Simplified linear and RRF retrievers #128633

Simplified linear and RRF retrievers #128633

Uh oh!

Conversation

Mikep86 commented May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented May 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kderusso left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

jimczi left a comment

Choose a reason for hiding this comment

Uh oh!

Mikep86 commented Jun 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Mikep86 commented May 29, 2025 •

edited

Loading