internetarchive / internetarchive/openlibrary
Enhance Default Carousel Randomness for Unique Display Across Multiple Carousels
- Dominant language
- Python
- Stars
- 6.7k
- Forks
- 2k
- Avg merge
- 2d 19h
- Merged PRs (30d)
- 138
Description
### Describe the problem that you'd like solved
This issue is a follow-up to [#8966](https://github.com/internetarchive/openlibrary/issues/8966) and addresses the need for unique randomness in carousel displays when multiple carousels are present on the same page. The current implementation, which uses `sort=random.hourly`, results in a shared random order across all carousels. This can lead to repetitive content display, especially when carousels share overlapping content.
#### Scenario
Consider a scenario where two carousels on the same page display books with some overlap:
- Carousel 1: A, B, C, D, E
- Carousel 2: A, B, F, G, H
Using `sort=random.hourly` results in a shared random order across both carousels, which can lead to identical content appearing in the same order in both carousels. For example:
- Carousel 1 might display: B, A, E
- Carousel 2 might display: B, A, G
This issue arises because the random seed is shared across all carousels, leading to shared content order.
#### Current Behavior
The current behavior does not allow for unique randomness within the same hour for each carousel. A workaround involves manually appending a custom seed to the sort parameter (e.g., `sort=random.hourly_1` for Carousel 1 and `sort=random.hourly_2` for Carousel 2), but this approach is counter-intuitive and requires manual intervention for each carousel.
#### Desired Behavior
The desired behavior is to have a default randomness that is unique to each carousel on the same page. This means that when `sort=random.hourly` is used, each carousel should display its content in a different order, ensuring unique content display across carousels. If a specific seed is desired for shared randomness, it should be explicitly specified (e.g., `sort=random.hourly_1` for shared randomness).
### Proposal & Constraints
A proposed solution involves modifying the sorting logic to automatically generate a unique seed for each carousel when `sort=random.hourly` is used. This can be achieved by appending a hash of the carousel parameters to the sort parameter, ensuring that each carousel has a unique random order. The code snippet below illustrates this approach:
```python
if not sort.contains('_'):
json_params_str = json.dumps(carousel_params, sort=true)
md5_hash = hash_function(json_params_str)
sort += f'_{md5_hash[:3]}' # Use only a few letters of the hash to prevent excessively large seed space
else:
# Continue with the existing method for processing custom seeds
```
This solution aims to simplify the process for librarians and patrons by eliminating the need for manual seed specification for unique randomness. This should lead to a more intuitive experience for librarians and as such better patron experience.
#### Constraints
When implementing this, we should prioritize keeping the code as simple as possible. We might need to refactor a few methods (words/editions/authors) to ensure that they can all accept the params to use as a seed.
Additional requirements:
- [ ] Write unit tests
- [ ] Performance testing
### Additional context
For further context see: https://github.com/internetarchive/openlibrary/pull/8966#issuecomment-2023233475
### Stakeholders
@cdrini
Contributor guide
Research direction
Start by tracing the existing `sort=random.hourly` sorting logic and how the words, editions, and authors methods receive carousel parameters. Review the context from issue #8966 and its linked comment before deciding how seeds should be derived. Done means unique default ordering across carousels, preserved explicit seeds, unit tests, and performance testing.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend, search
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100