Skip to content

Prep for user-group autocomplete #1745

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

chrisbobbe
Copy link
Collaborator

These three commits add a rank field to MentionAutocompleteResult, simply choosing 0 for wildcard results and 1 for user results, and a bucketSort on that new field. This is to prepare for more complicated ranking, with more ranks:

Discussion: #mobile > user-group mentions #F233 @ 💬

@gnprice, I'm curious how you'd like the tests to change, to support that more complicated ranking. Would you be up for demonstrating that by writing a few commits? I think you could do that by converting the compareByBotStatus logic into the new bucket-sort-by-rank logic. If we're following web (sort_recipients in web/src/typeahead_helper.ts), then some user-group results will need to be interleaved between human- and bot-user results, which calls for humans and bots to be in different buckets. As a first step, I think we'd go from two ranks (wildcard then users) to three ranks (wildcard then humans then bots); is that right?

Like we do in emoji autocomplete.

This commit doesn't make any changes to the results ordering;
bucketSort sorts stably, and the input is still just the wildcard
results followed by the user results.

Soon, though, we'd like to rank by match quality and add user-group
results (for zulip#233) interleaved with user results. Bucket sorting
will help us do this without making many intermediate copies of
lists of results; see discussion:
  https://chat.zulip.org/#narrow/channel/48-mobile/topic/user-group.20mentions.20.23F233/near/2216353
@chrisbobbe chrisbobbe requested a review from gnprice July 24, 2025 20:43
@chrisbobbe chrisbobbe added the integration review Added by maintainers when PR may be ready for integration label Jul 24, 2025
Copy link
Member

@gnprice gnprice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, that plan sounds good. I'll take a look.

Comment on lines +775 to +779
/// A measure of a wildcard result's quality in the context of the query,
/// from 0 (best) to one less than [_numResultRanks].
///
/// See also [_rankUserResult].
static const _rankWildcardResult = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name is a verb phrase (well, it could be extra elliptical and mean "the rank for a wildcard result", but I want to read it as a verb phrase: "rank a wildcard result"), so it's a bit odd for it to be a constant value.

Maybe go ahead and make it a function that returns a constant? That'll better demonstrate the future structure you intend anyway.

@gnprice
Copy link
Member

gnprice commented Jul 25, 2025

Would you be up for demonstrating that by writing a few commits? I think you could do that by converting the compareByBotStatus logic into the new bucket-sort-by-rank logic.

Sent as #1748 — please take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration review Added by maintainers when PR may be ready for integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants