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

Specify the frecency threshold for fetching the top frecent sites #3635

Closed
gabrielluong opened this issue Oct 8, 2020 · 7 comments · Fixed by #3645
Closed

Specify the frecency threshold for fetching the top frecent sites #3635

gabrielluong opened this issue Oct 8, 2020 · 7 comments · Fixed by #3645
Assignees

Comments

@gabrielluong
Copy link
Member

gabrielluong commented Oct 8, 2020

In mozilla-mobile/fenix#14302, we are looking at following iOS and Desktop's frecency threshold to determine which visited sites should be displayed as a visited top site (autogenerated). Currently, we don't expose frecency despite fetching it in the query:

"SELECT h.frecency, h.title, h.url

This issue is to return the frecency value in the TopFrecentSiteInfo so that AC/Fenix can filter the returned top sites by a given frecency threshold.

┆Issue is synchronized with this Jira Task
┆Story Points: 5
┆Epic: a-s consumer issues
┆Sprint: Backlog

@gabrielluong gabrielluong self-assigned this Oct 8, 2020
@mhammond
Copy link
Member

mhammond commented Oct 8, 2020

One concern we've historically had with this is that the value for frecency isn't meaningful as an absolute value - it's only meaningful in its relationship to other values. It also means it will be much trickier to adjust the implementation (eg, see #610) How will you determine what frecency threshold to use? What will you do when there are too few or too many sites for a specific value?

IOW, I'm wondering if there is a better way to implement this than exposing an arbitrary value that we'd prefer to keep as an implementation detail?

@gabrielluong
Copy link
Member Author

One concern we've historically had with this is that the value for frecency isn't meaningful as an absolute value - it's only meaningful in its relationship to other values. It also means it will be much trickier to adjust the implementation (eg, see #610) How will you determine what frecency threshold to use? What will you do when there are too few or too many sites for a specific value?

Ed (mardak) pointed me to the desktop threshold https://searchfox.org/mozilla-central/rev/919607a3610222099fbfb0113c98b77888ebcbfb/browser/components/newtab/lib/TopSitesFeed.jsm#79 with an added note: frecency of a typed in url gets a frecency score of… 2000 vs a link visit gets frecency score of 100

I am still investigating what iOS is doing, but right now if Fenix wanted to conform to desktop. We would simply expose the frecency score and use the same threshold.

@gabrielluong
Copy link
Member Author

gabrielluong commented Oct 8, 2020

Right now, our issue in Fenix is that any visited site is automatically considered a visited top site because we don't use any heuristics to determine the cutoff of what should be considered a visited top site.

@gabrielluong
Copy link
Member Author

I would be open to longer term options. I think the short term plan for Fenix that was conveyed to me was to emulate the iOS and desktop behaviour.

@eoger
Copy link
Contributor

eoger commented Oct 9, 2020

We could also keep the threshold/cut mechanism in the Rust layer so we don't leak the frequency values.

@gabrielluong
Copy link
Member Author

gabrielluong commented Oct 9, 2020

We could also keep the threshold/cut mechanism in the Rust layer so we don't leak the frequency values.

I am still digging around to see what everyone (desktop + iOS) does. Right now, it might be easiest to expose it and let individual consumers decide the threshold and tweak it as appropriate since we're also offering this as a AC component feature (although that might be rare).

@mhammond
Copy link
Member

mhammond commented Oct 9, 2020

Yeah, I actually think checking just the frecency is going to be wrong whoever does it TBH (maybe a threshold of visit counts would be better, with frecency just used for ordering?) but that's a bigger discussion, and it's hard to argue too hard against this given how desktop etc work, so 🤷‍♂️

@gabrielluong gabrielluong changed the title Return the frecency in TopFrecentSiteInfo Specify the frecency threshold for fetching the top frecent sites Oct 23, 2020
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 a pull request may close this issue.

3 participants