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.

Sign up for GitHub

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

Jump to bottom

Hashtag ordering kinda sucks #35

Closed
deanveloper opened this issue Jan 8, 2023 · 22 comments · Fixed by #49
Closed

Hashtag ordering kinda sucks #35

deanveloper opened this issue Jan 8, 2023 · 22 comments · Fixed by #49
Labels
bug Something isn't working

Comments

@deanveloper
Copy link
Collaborator

Steps to reproduce the problem

Not sure if I should be listing this as an enhancement or a bug. I think we'll call it a bug.

  1. Type "#genshin"
  2. notice that weird hashtags that aren't used much get suggested first
  3. "#GenshinImpact" is at the bottom even though it's the most used hashtag of the suggestions

image

Expected behaviour

We should prioritize hashtags that are used more often

Actual behaviour

We don't seem to? I'm a bit confused here.

Detailed description

No response

Specifications

N/A
@deanveloper deanveloper added the bug Something isn't working label Jan 8, 2023
@neatchee
Copy link
Owner

I think this is an easy fix; it looks like a change was made in 2019 that changed the sort order, but then it was changed back, and the field name to sort on was changed, and it was never updated again to sort with the new field name. Testing a fix now...

@neatchee
Copy link
Owner

Well, this is rather complicated...

The sorting function is set up to sort from values in the tags sql table, which does not store the actual occurrence count. The "uses this week" value is an aggregate value generated at runtime from recent tag history. Sorting was set up to use a "score" which is built over time, and is now recorded as "max_score"

So we could easily redo the sorting to use the max_score but that does NOT produce a sorted list by "uses this week"; for example "mastodogs" appears above "mastodon" because at some point mastodogs got super popular and bursted above the average usage of 'mastodon'.

I've got a version deployed that feels a bit better but still isn't great. It'd be a substantive refactor to actually sort by "uses this week" unfortunately.

@neatchee
Copy link
Owner

neatchee commented Jan 12, 2023 •
edited
Loading

This is the tags table schema:

---------------------+-----------------------------
 id                  | bigint
 name                | character varying
 created_at          | timestamp without time zone 
 updated_at          | timestamp without time zone 
 usable              | boolean 
 trendable           | boolean
 listable            | boolean
 reviewed_at         | timestamp without time zone
 requested_review_at | timestamp without time zone
 last_status_at      | timestamp without time zone
 max_score           | double precision
 max_score_at        | timestamp without time zone
 display_name        | character varying

And this is the current deployed sorting code for Urusai as of now:

def search_for(term, limit = 5, offset = 0, options = {})
    stripped_term = term.strip

    query = Tag.listable.matches_name(stripped_term)
    query = query.merge(matching_name(stripped_term).or(where.not(reviewed_at: nil))) if options[:exclude_unreviewed]

    query.order(Arel.sql('max_score ASC, name ASC'))
         .limit(limit)
         .offset(offset)
  end

Currently checked in upstream is:

query.order(Arel.sql(length(name) ASC, name ASC'))
         .limit(limit)
         .offset(offset)

@neatchee
Copy link
Owner

Well, the real answer here is to just do what's intended to get better autosuggestion/search: set up elasticsearch integration.

So that's what I'm doing :D

We can try to make this better too, if we want, but IMO it's low prio since there's a preferred solution

@neatchee neatchee closed this as not planned Won't fix, can't repro, duplicate, stale Jan 13, 2023
@deanveloper
Copy link
Collaborator Author

deanveloper commented Jan 13, 2023 •
edited
Loading

Think this is worth reopening, doesn't seem fixed, even with elasticsearch :/

image

@deanveloper deanveloper reopened this Jan 13, 2023
@neatchee
Copy link
Owner

neatchee commented Jan 13, 2023 via email

@deanveloper
Copy link
Collaborator Author

deanveloper commented Jan 13, 2023 •
edited
Loading

Oh just looked at the code snippet, you probably want to order by score DESC right? ASC would sort it least-to-most

@neatchee
Copy link
Owner

neatchee commented Jan 13, 2023 via email

@deanveloper
Copy link
Collaborator Author

deanveloper commented Jan 13, 2023 •
edited
Loading

Hmm... I'll see if I can mess around with it and see if I can figure something out. Seems weird because the current behavior seems correct on other hashtags, like this is exactly what I would expect:

image
image

@neatchee
Copy link
Owner

neatchee commented Jan 13, 2023 via email

@deanveloper
Copy link
Collaborator Author

Hashtag ordering seems to work fine on Megalodon... I wonder if this is just some weird edge case in the browser

@neatchee
Copy link
Owner

neatchee commented Feb 11, 2023 via email

@deanveloper
Copy link
Collaborator Author

deanveloper commented Feb 11, 2023 •
edited
Loading

Nope, it looks like the frontend is doing something weird... The request seems to return the hashtags in the right order. Wonder what's happening.

image

However... when redux gets updated, looks like they're out of order. Weird...

image

@neatchee
Copy link
Owner

neatchee commented Feb 11, 2023 via email

@deanveloper
Copy link
Collaborator Author

deanveloper commented Feb 11, 2023 •
edited
Loading

It looks like some kind of transformation gets applied before it gets put in redux, trying to figure out where this happens (which seems a bit weird, not like suggestions really need to be stored in global state...)

EDIT - Guessing it has something to do with this line. (app/javascript/flavours/glitch/reducers/compose.js)

return mergeLocalHashtagResults(sortHashtagsByUse(state, tags.map(item => ({ ...item, type: 'hashtag' }))), token.slice(1), state.get('tagHistory'));

@deanveloper
Copy link
Collaborator Author

deanveloper commented Feb 11, 2023 •
edited
Loading

This seems so weird... I can't seem to replicate the behavior in Node, even if I copy the behavior nearly line-for-line

Browser
image

Node
image

The browser one starts correctly, but then ends incorrectly, and I have no clue why

@neatchee
Copy link
Owner

neatchee commented Feb 11, 2023 via email

@deanveloper
Copy link
Collaborator Author

Yup, reproduces in all browsers. Although I've got it reproducing in node now, so that's good! Maybe I was just copying it over incorrectly.

@deanveloper
Copy link
Collaborator Author

Oh man, I figured it out. I'll file this bug with mastodon as well.

It only occurs if:

  1. You have used the hashtag in this browser, which puts it in your tagHistory.
  2. The tag in your tagHistory matches (case sensitive) one of the suggested hashtags.

If both of these cases happen, it's supposed to promote it to the top, as it's a suggested hashtag which you have used before. However, the sorting algorithm is reversed, and case-sensitive, making it (inconsistently) demote it to the bottom.

@neatchee
Copy link
Owner

neatchee commented Feb 11, 2023 via email

@deanveloper
Copy link
Collaborator Author

Ahh, the bug appears to have been introduced in glitch-soc, I assume mastodon fixed it but glitch-soc never brought the fix over?

@neatchee
Copy link
Owner

neatchee commented Feb 11, 2023 via email

This was referenced Feb 11, 2023
Closed
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix hashtag ordering deanveloper/urusai.social
2 participants
@deanveloper @neatchee