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

Eliminate duplication that can happen after clicking More when ranking changes over time #147

Open
cointastical opened this issue May 2, 2022 · 7 comments · May be fixed by #1091
Open
Labels
backburner postpone consideration of ticket bug difficulty:hard

Comments

@cointastical
Copy link

cointastical commented May 2, 2022

After an initial load of the front page, an item might move from a position in the top 21 into a lower position before the More button is pressed. Then when More is pressed, that item likely will appear again in the newly added results.

A solution is to exclude from the results for pressing more any items that already appeared in the results above that existed before More was pressed.

@huumn
Copy link
Member

huumn commented May 2, 2022

Do you have an example of this occurring? Afaik this shouldn't happen (I have a cursor), but I might've broken it at some point.

@cointastical
Copy link
Author

What I had seen was when I pressed More, the item that was in position at 21 was then also showing in position 22 as well.

I don't know how long it was between when the page was first loaded, and when I pressed More. Might have been more than 10 minutes, maybe 30 minutes, I wasn't watching closely. A cursor is held over that long of a duration?

Or, perhaps this is an off-by-one bug (since this occurred at the end of the set)?

@huumn
Copy link
Member

huumn commented May 4, 2022

A cursor is held client side so shouldn't be affected by the time you come back.

The cursor encodes the time the page's query was loaded. All pages (ie more) are scoped to the time the page is loaded - new items/votes shouldn't be considered.

Again, there might be a bug in the implementation.

@huumn
Copy link
Member

huumn commented May 18, 2022

Now that we've denormalized ranking scores, this is not going to be possible (if ranking changes there's no way to use ranking at time of page load) but IMO not a huge deal. Closing to clear up issues. We can re-open if this gets bad.

@huumn huumn closed this as completed May 18, 2022
@cointastical
Copy link
Author

Now it has two rows at 21.

Screenshot at 2022-07-13 07-35-57

@huumn huumn reopened this Jul 13, 2022
@ekzyis
Copy link
Member

ekzyis commented May 19, 2023

Yeah I also noticed this occasionally and recently

@ekzyis ekzyis added the bug label May 19, 2023
@huumn
Copy link
Member

huumn commented May 19, 2023

The only way to really eliminate this given the way we rank (denormalized values) is expanding the cursor to encode the items in the list that we already have, so that we can explicitly exclude them on the next page.

@huumn huumn added backburner postpone consideration of ticket difficulty:hard labels Feb 5, 2024
crodas added a commit to crodas/stacker.news that referenced this issue Apr 18, 2024
Fixes stackernews#147

To avoid duplicated results push some state towards the cursor.

The cursor uses a Bloom Filter[1], a probabilistic data structure. In this data
structure, False positive matches are possible, but false negatives are not –
in other words, a query returns either "possibly in set" or "definitely not in
set". Elements can be added to the set, but not removed. The more items added,
the larger the probability of false positives.

[1] https://en.wikipedia.org/wiki/Bloom_filter
@crodas crodas linked a pull request Apr 18, 2024 that will close this issue
crodas added a commit to crodas/stacker.news that referenced this issue Apr 19, 2024
A client side approach to fix stackernews#147
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backburner postpone consideration of ticket bug difficulty:hard
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants