Use different counts for denominator in scaled rank for posts

[andrewmoise]

In the discussion about a better "Scaled" sort in bug #5210, it was recommended that I make a new sort option to implement the different sorting I described.

Looking at the change, I don't think applying this PR is the right way. I think the complexity and UX issues are such that it would be better to just amend the "Scaled" sort option in-place instead of adding a new sort option. But, I'm submitting this for discussion and to move the process forward. Comments are welcome. Having the option visible for purposes of comparison may be useful, for purposes of evaluating the change.

To actually work, this PR needs to go in alongside some other changes in other repos:

• Add a translation for "Balanced" in crates/utils/translations/translations/*.json
• Add an option for the "Balanced" sort in src/shared/components/common/sort-select.tsx in lemmy-ui, around line 141

Comments welcome.

[Nutomic]

So the only difference compared to scaled rank is that it uses SUM(comments + upvotes + downvotes) instead of users_active_month in the same formula? Then I agree that its better to modify the existing sort instead of adding a new one. I dont think scaled sort is working well now so a change will likely be beneficial. This way we also avoid adding another option for users which would be too complicated. If there are any problems they can be found during beta testing.

[andrewmoise]

Sounds good to me. Here's the simpler option, just updating what metric we use.

I generally like "Scaled" the best out of all the sorts that are available; it just has that one failure mode if the denominator users_active_month is tiny, which this fixes.

[Nutomic]

Somehow CI is not running on this pull request, it seems to happen because its based on the release/v0.19 branch which uses a deprecated CI config. Can you change it to target main and put your changes on top of the latest main branch?

[andrewmoise]

Okay, done. I haven't tested it on main, since I don't have a main dev instance as of now, but I built it and it compiles at least.

[andrewmoise]

Federation tests are failing because it can't connect to git.asonix.dog. Is that me? That feels unrelated to this change.

[dessalines]

I don't like that you're replacing an existing sort instead adding a new one. Now there's no way to compare how they both will function in practice. We can take guesses, but we won't know.

I also don't like the name balanced, as that doesn't give any hint as to what it is. Maybe LowInteractions, LowContent, Inactive, LessActive, something like that.

[andrewmoise]

Tell you what. You can have both options:

• https://github.com/andrewmoise/lemmy/tree/balanced-sort is the current code, which changes the meaning of 'Scaled'
• https://github.com/andrewmoise/lemmy/tree/balanced-sort-2 adds a new sorting option with new semantics, leaving 'Scaled' alone.

I've implemented both now, so you can take your pick. Or, apply the latter to a dev instance for purpose of comparing the sorts. It's probably not for me to get involved with the decision of which approach you want to put in, since it's not my project, but I did the separate option when you requested it, and the change-Scaled option when nutomic requested it, so now you have both.

Note that there are trivial changes to lemmy-ui and translations which need to happen for the separate 'Balanced' option to work; see the beginning of this PR comments.

Likewise I think you can pick the name.

[Nutomic]

@dessalines We already have 8 different ranking options, plus a dozen more 'top x' variants. It doesnt make sense to add another option with such a tiny difference, it would be very confusing for users. And the way to compare this to the previous scaled sort logic is by deploying a beta version and testing it, just like any other code change.

[dessalines]

Lots of clients /apps have already added the scaled sort, and removing that option isn't a good idea, unless they're both tested on production instances alongside each other. Testing it on test-servers wouldn't be easy for something that is based on activity / interactions, and wouldn't give you a feel for it in practice.

If in the future in turns out this one is way better than scaled, then we can remove scaled, but IMO we shouldn't be removing sorts that have already been implemented by apps without some consideration.

@andrewmoise please open up a PR for your 2nd one if you would.

[andrewmoise]

@dessalines You're asking me to open a second PR for the alternate approach, to exist in parallel, instead of the two of you just getting together and deciding together what the single plan is?

@dessalines @Nutomic I think just get together, figure out the answer, and then we can do that one. I have to say I do tend to lean towards Nutomic's opinion for the long term. It's really the exact same sort algorithm, we're just changing one parameter in it to be a more reliable measurement of what we were trying to measure in the first place. That said, I get the concern about not taking stuff away or making changes without examination in practice, so I'm happy to do either one. Just let me know as a unified plan so it isn't just indefinitely flipping back and forth between them depending on which person I'm talking to. I don't think it's a good use of time to continue to flip the code back and forth.

[dessalines]

Alright then, I think we can merge this as is. I'm fairly sure that someone will be angry that we yanked their favorite sort out from under them tho.

[dessalines]

Review needed from @dullbananas @Nutomic

[dullbananas]

Add an interactions_month column to the community_aggregates table using a migration, and use it here just like how users_active_month was used, which will be more efficient. In utils.sql, add a variant of the r.community_aggregates_activity function that does the sum, and use it to update the new column in the active_counts function in this file.

To prevent a compile error, you also need to add the new column in crates/db_schema/src/aggregates/structs.rs at the end of the struct, and it should probably have the #[serde(skip)] attribute.