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

[v1.3] [EXPERIMENTAL] Display ranking details at search #399

Closed
3 tasks
brunoocasali opened this issue Aug 8, 2023 · 1 comment
Closed
3 tasks

[v1.3] [EXPERIMENTAL] Display ranking details at search #399

brunoocasali opened this issue Aug 8, 2023 · 1 comment
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@brunoocasali
Copy link
Member

⚠️ This issue is generated, it means the examples and the namings do not necessarily correspond to the language of this repository.
Also, if you are a maintainer, please add any clarification and instructions about this issue.

Sorry if this is already wholly/partially implemented. Feel free to let me know about the state of this issue in the repo.

Related to meilisearch/integration-guides#280


This issue is divided into two sections, first, you need to make the implementation, and second, you must update the code-samples (no one likes outdated docs, right?).

New implementation

Related to:

This feature aims to return ranking details for each document to understand and tweak the score of the documents more easily.

Ensure the SDKs can handle the new search parameter showRankingScoreDetails. Also ensure the SDK can handle the _rankingScoreDetails attributes in the matched hits.

⚠️ This feature is enabled by querying PATCH /experimental-features with { "scoreDetails": true }

Extra: Add inline documentation for the method, explaining the availability of this feature only for Meilisearch v1.3 and newer. And that is also an experimental feature that must be manually opt-in using the /experimental-features meilisearch/meilisearch#3857 endpoint.

TODO:

  • Add the ability receive a new param in the search method called showRankingScoreDetails.
  • Add the ability handle the new _rankingScoreDetails key/value in the search hits' response.
  • Add integration tests (don't forget to enable the experimental feature)
@brunoocasali brunoocasali added good first issue Good for newcomers enhancement New feature or request labels Aug 8, 2023
meili-bors bot added a commit that referenced this issue Sep 27, 2023
413: Add support for search ranking score r=curquiza a=Sherlouk

# Pull Request

## Related issue
Fixes #398

## What does this PR do?
- Add the ability receive a new param in the search method called showRankingScore.
- Add the ability handle the new _rankingScore key/value in the search hits' response.
- Add integration tests
- Update the code-samples accordingly

⚠️  **This is a breaking change**.

I'd love to hear some thoughts from others on how we may be able to avoid this kind of breaking change, but with #398 and #399 new values are being added to the "hit" object. The hit object is currently a generic provided by the user of the library, and as such it's impossible for us to add new attributes to that.

In this PR I've tried to get around this by creating our own encapsulation called "SearchHit" which itself holds a reference to the actual result but also any additional attributes that Meilisearch itself returns. In order to minimise the impact on users of the library I have implemented a "dynamic member lookup" with a keypath, this continues to support (in a completely type safe and compiler safe way) dot notation to variables within the hit result. Which means `hits[0].title` would still return the title on the hit, even though architecturally it's address is `hits[0].result.title`. While this does work for the vast majority of use cases, we are unable to support the case where a user tries to cast a hit to a type, you can see this in our tests where you can no longer cast `hits[0]` to a "Book", rather it is now a "SearchHit<Book>", but again... you can access variables the same and no other changes are needed to the code.

Importantly though this workaround with dynamic member lookups is limited to the capabilities of keypaths, which as documented officially by Apple [here](https://github.com/apple/swift/blob/main/test/attr/attr_dynamic_member_lookup.swift#L671-L672) does not include methods. As such where users have functions within their models, they would have to call that through `hits[0].result.someFunction()`.

I do personally think this is the best compromise. It is a breaking change, but most will not be impacted in any meaningful way. If others have a better idea though I'd love to hear it. This PR lays up the foundations for #399 which would be significantly easier once this is merged. But let's discuss.

## PR checklist
Please check if your PR fulfills the following requirements:
- [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
- [x] Have you read the contributing guidelines?
- [x] Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!


Co-authored-by: James Sherlock <[email protected]>
Co-authored-by: Clémentine U. - curqui <[email protected]>
@curquiza
Copy link
Member

Replaced by #439 since the feature has been stabilized in Meilisearch v1.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants