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

Sort Talks by ranking and Events by year in Spotlight Search #486

Merged
merged 5 commits into from
Dec 7, 2024

Conversation

aamfahim
Copy link
Contributor

@aamfahim aamfahim commented Dec 4, 2024

Resolves #219

Not sure how it was before but after #407 I noticed on the search modal the talks being shown are over a decade old. I think users would prefer to see the newer talks first.

@marcoroth Also mentioned the same issue with the events as well. So I fixed that as well.

Talks Before

before.mp4

Talks After

after.mp4

Events Before

before.mp4

Events After

after.mp4

@aamfahim aamfahim changed the title feat: order talks by newest first for spotlight search Sort Talks by publication year in Spotlight Search Dec 4, 2024
Copy link
Collaborator

@marcoroth marcoroth left a comment

Choose a reason for hiding this comment

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

Nice one, I also noticed this today and I think this makes sense. The same for the events too!

Thanks for this @aamfahim! 🙏🏼

@aamfahim
Copy link
Contributor Author

aamfahim commented Dec 4, 2024

Thanks @marcoroth. I was just recording before and after for the PR lol. I'll update the events as well!

@aamfahim aamfahim marked this pull request as ready for review December 4, 2024 02:03
@adrienpoly
Copy link
Owner

I agree with the implementation for Event.

However, I have some reservations about the Talk model. Currently, the Talk model uses a fuzzy full-text search on the title, description, and speaker names via the Talk::Searchable concern. This concern includes a ranked scope that sorts results based on relevance, assigning more weight to search terms found in the title compared to the description.

That said, I notice that the ranked scope isn’t being applied in the current implementation. I recommend incorporating it to improve result accuracy. Here's an example:

@talks = @talks.ft_search(search_query).ranked if search_query.present?

Using a date-based ordering might lead to issues. For example, it could prioritize newer talks where the keyword appears only once in the description over older talks where the keyword appears in the title. This could result in less relevant results being ranked higher, which may not be ideal for the user's search experience.

suggestions to test

for the same bm25 score talks are sorted by dates

scope :ranked, -> do
  order(Arel.sql("bm25(talks_search_index, 10.0, 1.0, 5.0) ASC, talks.date DESC"))
end

We weight the bm25 score with a number to give more weight for recent talks, the 0.0001 value might needs to be adjusted

scope :ranked, -> do
  order(Arel.sql("bm25(talks_search_index, 10.0, 1.0, 5.0) + (strftime('%s', 'now') - strftime('%s', talks.date)) * 0.0001 ASC"))
end

@aamfahim
Copy link
Contributor Author

aamfahim commented Dec 5, 2024

@adrienpoly I agree that it makes more sense to show relevant talks (aka ranked) rather than just the most recent 5 talks.

I've tested the first two suggested scopes. Here are my findings

scope :ranked, -> do
  order(Arel.sql("bm25(talks_search_index, 10.0, 1.0, 5.0) ASC, talks.date DESC"))
end

With this modification, results aren't much difference from the current implementation of ranked scope BUT it is working, when searching for active record there is a difference. So incase of same bm25 score, date is being used as the tie breaker.

scope :ranked, -> do
  order(Arel.sql("bm25(talks_search_index, 10.0, 1.0, 5.0) + (strftime('%s', 'now') - strftime('%s', talks.date)) * 0.0001 ASC"))
end

With this modification, I'm only getting the most recent 5.

Keywords tested: testing, ruby, active record.

@adrienpoly adrienpoly mentioned this pull request Dec 6, 2024
@adrienpoly
Copy link
Owner

@aamfahim I played a bit with the weight in #497 hopefully this can help you refine the best ranking strategy

@aamfahim aamfahim changed the title Sort Talks by publication year in Spotlight Search Sort Talks by ranking and Events by year in Spotlight Search Dec 7, 2024
@adrienpoly adrienpoly merged commit 6971a99 into adrienpoly:main Dec 7, 2024
4 checks passed
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.

Feature Request: Sort Talks Menu search by publication year
3 participants