Skip to content

Conversation

kderusso
Copy link
Member

Adds documentation for the query rules retriever introduced in #114855

Copy link
Contributor

Documentation preview:

@kderusso kderusso added >docs General docs changes :SearchOrg/Relevance Label for the Search (solution/org) Relevance team v8.17.0 labels Oct 29, 2024
@kderusso
Copy link
Member Author

@elasticmachine update branch

@kderusso kderusso marked this pull request as ready for review October 29, 2024 20:32
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-docs (Team:Docs)

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/search-eng (Team:SearchOrg)

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/search-relevance (Team:Search - Relevance)

@kderusso kderusso requested review from a team and leemthompo October 29, 2024 20:33
Copy link
Contributor

@leemthompo leemthompo left a comment

Choose a reason for hiding this comment

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

This is looking great, a few nits from me, many of which edit preexisting text.

[WARNING]
====
The `rule` retriever will apply rules to any documents returned from its defined `retriever` or any of its sub-retrievers.
This means that for the best results, the `rule` retriever should be the outermost defined retriever.
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we could hammer this home with an annotation on the example?

results of a nested retriever to its parent. In this scenario, we'll make use of all 4 (currently) available retrievers, i.e. `standard`, `knn`, `text_similarity_reranker` and `rrf`.
The following example demonstrates the powerful queries that we can now compose, and how retrievers simplify this process.
We can use any combination of retrievers we want, propagating the results of a nested retriever to its parent.
In this scenario, we'll make use of all 4 (currently) available retrievers, i.e. `standard`, `knn`, `text_similarity_reranker` and `rrf`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to update this too?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good question. I think no for now - the reasoning being that rules are a niche use case, and as we have plans to add more retrievers it will soon become unwieldy to have one monster example. Instead, I'll reword to say that we're using some retrievers and that others are available.

@kderusso
Copy link
Member Author

kderusso commented Nov 1, 2024

@elasticmachine run docs-build

@kderusso
Copy link
Member Author

kderusso commented Nov 1, 2024

@elasticmachine merge upstream

Copy link
Contributor

@Mikep86 Mikep86 left a comment

Choose a reason for hiding this comment

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

Looks good! I left a few minor comments.

@kderusso kderusso requested a review from leemthompo November 4, 2024 20:54
@kderusso
Copy link
Member Author

kderusso commented Nov 4, 2024

@elasticmachine merge upstream

Copy link
Contributor

@leemthompo leemthompo left a comment

Choose a reason for hiding this comment

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

The docs-build-pr build is failing because of an id collision.

INFO:build_docs:asciidoctor: WARNING: redirects.asciidoc: line 1610: id assigned to section already in use: _parameters_8

Smells like an edge case! 👃

You haven't explicitly set this in any of the files you changed. However, you did add a ==== Parameters headings without explicit ids and I'm wondering if there isn't some dynamic auto-generation going on behind the scenes that's causing the collision.

I've made a couple of suggestion to add explicit ids to these to see if that changes anything...

* Upload a model to {es} with {eland-docs}/machine-learning.html#ml-nlp-pytorch[Eland] using the `text_similarity` NLP task type.
** Then set up an <<inference-example-eland,{es} service inference endpoint>> with the `rerank` task type
** Refer to the <<text-similarity-reranker-retriever-example-eland,example>> on this page for a step-by-step guide.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
[discrete]
[[text-similarity-reranker-retriever-parameters]]

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the other suggestion is the important one, so committed that first to see 👁️

@leemthompo
Copy link
Contributor

The build is fixed 🎉 , but there are some formatting issues (misaligned subheadings) in the preview that need to be fixed :)

@kderusso
Copy link
Member Author

kderusso commented Nov 6, 2024

@leemthompo I think I've fixed the formatting issues, would love one more 👀 before merging. Thanks!

@kderusso kderusso requested a review from leemthompo November 6, 2024 19:07
Copy link
Contributor

@leemthompo leemthompo left a comment

Choose a reason for hiding this comment

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

Nice thank you! 🚀

@kderusso kderusso merged commit 14a7b8f into elastic:main Nov 6, 2024
5 checks passed
@kderusso kderusso added the auto-backport Automatically create backport pull requests when merged label Nov 6, 2024
@leemthompo
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Questions ?

Please refer to the Backport tool documentation

leemthompo added a commit to leemthompo/elasticsearch that referenced this pull request Nov 7, 2024
* Add initial query rules retriever docs

* Add docs tests

* Apply suggestions from code review

Co-authored-by: Liam Thompson <[email protected]>

* PR feedback

* Make query rules guide retriever-first

* Add warning to DSL doc

* Update docs/reference/search/retriever.asciidoc

Co-authored-by: Mike Pellegrini <[email protected]>

* Update docs/reference/search/retriever.asciidoc

Co-authored-by: Mike Pellegrini <[email protected]>

* Apply suggestions from code review

Co-authored-by: Mike Pellegrini <[email protected]>

* Give parameters subheading an explicit id

* Fix formatting

---------

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Liam Thompson <[email protected]>
Co-authored-by: Mike Pellegrini <[email protected]>
(cherry picked from commit 14a7b8f)
jozala pushed a commit that referenced this pull request Nov 13, 2024
* Add initial query rules retriever docs

* Add docs tests

* Apply suggestions from code review

Co-authored-by: Liam Thompson <[email protected]>

* PR feedback

* Make query rules guide retriever-first

* Add warning to DSL doc

* Update docs/reference/search/retriever.asciidoc

Co-authored-by: Mike Pellegrini <[email protected]>

* Update docs/reference/search/retriever.asciidoc

Co-authored-by: Mike Pellegrini <[email protected]>

* Apply suggestions from code review

Co-authored-by: Mike Pellegrini <[email protected]>

* Give parameters subheading an explicit id

* Fix formatting

---------

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Liam Thompson <[email protected]>
Co-authored-by: Mike Pellegrini <[email protected]>
alexey-ivanov-es pushed a commit to alexey-ivanov-es/elasticsearch that referenced this pull request Nov 28, 2024
* Add initial query rules retriever docs

* Add docs tests

* Apply suggestions from code review

Co-authored-by: Liam Thompson <[email protected]>

* PR feedback

* Make query rules guide retriever-first

* Add warning to DSL doc

* Update docs/reference/search/retriever.asciidoc

Co-authored-by: Mike Pellegrini <[email protected]>

* Update docs/reference/search/retriever.asciidoc

Co-authored-by: Mike Pellegrini <[email protected]>

* Apply suggestions from code review

Co-authored-by: Mike Pellegrini <[email protected]>

* Give parameters subheading an explicit id

* Fix formatting

---------

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: Liam Thompson <[email protected]>
Co-authored-by: Mike Pellegrini <[email protected]>
@kderusso kderusso deleted the kderusso/query-rules-retrievers-docs branch January 24, 2025 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged >docs General docs changes :SearchOrg/Relevance Label for the Search (solution/org) Relevance team v8.17.0 v9.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants