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

Fix(core): remove sorting on recommendations retrieval #77

Closed
wants to merge 4 commits into from

Conversation

marialungu
Copy link
Collaborator

Recommendations were being sorted and this affects the behaviour of Recommend rules (order is given from the API).
Removed filtering based on 'objectID' as it is no longer needed.

@francoischalifour
Copy link
Member

This was necessary when providing multiple objectIDs like recommendations in a cart. Doesn't this break this behavior?

@marialungu
Copy link
Collaborator Author

HI @francoischalifour,

@jojva confirmed that they don't return duplicated objectIDs in a previous PR, so I think we can safely remove this logic.

@francoischalifour
Copy link
Member

@jojva jojva self-requested a review April 5, 2022 17:14
@jojva
Copy link
Contributor

jojva commented Apr 6, 2022

Hi @francoischalifour,

Right, but do they sort the merged hits now?

That's the part I don't understand. When we say "multiple indices", does this mean a recommendation request with several sub-queries, each targeting an index? And if so, I wonder why the library needs to re-sort those, discarding the origin of the index?

Results do not necessarily return a recommendation score, as we now have recommend rules, that are able to pin items. And secondly, recommendation scores are not necessarily the only basis for the ranking. It can also be affected by boost and bury (new recommend rules options), as well as the rest of the ranking formula.

I don't think we can or should mix hits coming from several indices. I'd like to understand the idea behind this and how we can solve the issue.

@francoischalifour
Copy link
Member

@jojva I'm out of the loop with new Recommend features like rules and pins. How about we sync together this week? DMing you.

@marialungu
Copy link
Collaborator Author

Closing as this issue needs to be discussed further given its impact on the recommend flows.

@marialungu marialungu closed this Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants