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

[back][ext] feat: recommendations API can now return random entities #1876

Merged
merged 17 commits into from
Jan 15, 2024

Conversation

GresilleSiffle
Copy link
Collaborator

@GresilleSiffle GresilleSiffle commented Jan 8, 2024

Description

This PR adds two features:

(1) A new API polls/{name}/recommendations/random/ that returns random recommendations, designed to be more efficient than the regular recommendations route.

(2) The browser extension now uses this new API to improve the loading time of recommendations and their diversity.

context

This PR aims to reduce the number of latency alerts triggered by the API recommendations, by providing API clients, such as the browser extension, an alternative and efficient way to request random recommended entities.

Those latency alerts are currently triggered by the browser extension, when fetching the top 250 of all time recommendations to perform a random selection. Thanks to the new API, the extension doesn't need to fetch a big amount of entities to display a diverse bundle of entities.

Note that the new API is more efficient because it uses a simple SQL query that provides less features than the regular recommendations route:

  • no search by text
  • no search by weighted criteria score
  • not possible to return unsafe recommendations

ℹ️ consequences and important changes

This PR significantly changes the strategy used by browser extension to display diverse entities.

(a) Before, only the top 250 of all time recommendations could be displayed. Now all recommended videos have a chance to be displayed, even those with a Tournesol score of 20.1, like on the web site.

(b) The number of cached recommendation results requested by the extension will increase from 2 every 10 minutes (for a given set of languages) to ~ 40, up to a lot more depending on how many time the users refresh their recommendations with the extension refresh button. This may impact our infrastructure.

to-do

back end - /recommendations/

  • remove the random logic

back end - /recommendations/random/

  • add a /recommendations/random/ view
  • create a new serializer
    • and the random parameter in the metadata?
  • document the view
  • test the view

extension

  • request random recommendations
    • should the recommendation be in the top X ?
  • randomly initialize the query parameter random for each instance of TournesolRecommendations
  • make the query parameter random vary for each refresh
  • increase the limit parameters for more randomness
    • actually 5 for old videos
    • actually 7 for new videos
  • connect the extension to the new route

Checklist

  • I added the related issue(s) id in the related issues section (if any)
    • if not, delete the related issues section
  • I described my changes and my decisions in the PR description
  • I read the development guidelines of the CONTRIBUTING.md
  • The tests pass and have been updated if relevant
  • The code quality check pass

@GresilleSiffle GresilleSiffle changed the title [back] feat: recommendations API can now return random entities WIP / [back][ext] feat: recommendations API can now return random entities Jan 9, 2024
@GresilleSiffle GresilleSiffle added Backend Back-end code of Tournesol Extension Development of the browser extension labels Jan 9, 2024
@GresilleSiffle
Copy link
Collaborator Author

GresilleSiffle commented Jan 10, 2024

I've been able to significantly improve the performance by removing the total_score from the SQL query, and the related left outer join tournesol_entitycriteriascore. I think this should our first optimization.

@GresilleSiffle GresilleSiffle changed the title WIP / [back][ext] feat: recommendations API can now return random entities [back][ext] feat: recommendations API can now return random entities Jan 11, 2024
@GresilleSiffle GresilleSiffle marked this pull request as ready for review January 11, 2024 18:02
@GresilleSiffle GresilleSiffle self-assigned this Jan 11, 2024
@GresilleSiffle
Copy link
Collaborator Author

The check run end-to-end tests / e2e-browser-extension (pull_request) is failing because the browser extension uses the route that comes with this PR (not deployed in production yet).

backend/tournesol/serializers/poll.py Outdated Show resolved Hide resolved
backend/tournesol/views/polls.py Show resolved Hide resolved
@@ -18,4 +18,5 @@
COMPARISON_MAX = 10.0

# Default weight for a criteria in the recommendations
# FIXME: the default weight used by the front end is 50, not 10
Copy link
Member

Choose a reason for hiding this comment

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

They don't necessarily need to be the same. The backend can assume 10 when nothing is specified & the frontend can initialize a user's search by default with a value of 50.

I vaguely remember discussing with @amatissart to make it 0 by default. I don't remember well what makes a criteria included or not in the scores used for ranking.

browser-extension/src/background.js Outdated Show resolved Hide resolved
browser-extension/src/background.js Outdated Show resolved Hide resolved
backend/tournesol/views/polls_reco_random.py Outdated Show resolved Hide resolved
... in the /recommendations/random/ API. This parameter is currently
not supported by the view, so it's useless to keep using it to build
the cache key.
@GresilleSiffle GresilleSiffle merged commit f659203 into main Jan 15, 2024
7 of 8 checks passed
@GresilleSiffle GresilleSiffle deleted the back-add_random_recommendations branch January 15, 2024 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backend Back-end code of Tournesol Extension Development of the browser extension
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants