autocomplete: Add "recent senders criterion" to user-mention autocomplete

lib/model/autocomplete.dart

@@ -187,9 +187,95 @@ class MentionAutocompleteView extends ChangeNotifier {
     required PerAccountStore store,
     required Narrow narrow,
   }) {
-    assert(narrow is! CombinedFeedNarrow);
     return store.users.values.toList()
-      ..sort((userA, userB) => compareByDms(userA, userB, store: store));
+      ..sort(_comparator(store: store, narrow: narrow));
+  }
+
+  /// Compare the users the same way they would be sorted as
+  /// autocomplete candidates.
+  ///
+  /// This behaves the same as the comparator used for sorting in
+  /// [_usersByRelevance], but calling this for each comparison would be a bit
+  /// less efficient because some of the logic is independent of the users and
+  /// can be precomputed.
+  ///
+  /// This is useful for tests in order to distinguish "A comes before B"
+  /// from "A ranks equal to B, and the sort happened to put A before B",
+  /// particularly because [List.sort] makes no guarantees about the order
+  /// of items that compare equal.
+  int debugCompareUsers(User userA, User userB) {
+    return _comparator(store: store, narrow: narrow)(userA, userB);
+  }
+
+  static int Function(User, User) _comparator({
+    required PerAccountStore store,
+    required Narrow narrow,
+  }) {
+    int? streamId;
+    String? topic;
+    switch (narrow) {
+      case StreamNarrow():
+        streamId = narrow.streamId;
+      case TopicNarrow():
+        streamId = narrow.streamId;
+        topic = narrow.topic;
+      case DmNarrow():
+        break;
+      case CombinedFeedNarrow():
+        assert(false, 'No compose box, thus no autocomplete is available in ${narrow.runtimeType}.');
+    }
+    return (userA, userB) => _compareByRelevance(userA, userB,
+      streamId: streamId, topic: topic,
+      store: store);
+  }
+
+  static int _compareByRelevance(User userA, User userB, {
+    required int? streamId,
+    required String? topic,
+    required PerAccountStore store,
+  }) {
+    if (streamId != null) {
+      final result = compareByRecency(userA, userB,
+        streamId: streamId,
+        topic: topic,
+        store: store);
+      if (result != 0) return result;
+    }
+    return compareByDms(userA, userB, store: store);
+  }
+
+  /// Determines which of the two users has more recent activity (messages sent
+  /// recently) in the topic/stream.
+  ///
+  /// First checks for the activity in [topic] if provided.
+  ///
+  /// If no [topic] is provided, or there is no activity in the topic at all,
+  /// then checks for the activity in the stream with [streamId].
+  ///
+  /// Returns a negative number if [userA] has more recent activity than [userB],
+  /// returns a positive number if [userB] has more recent activity than [userA],
+  /// and returns `0` if both [userA] and [userB] have no activity at all.
+  @visibleForTesting
+  static int compareByRecency(User userA, User userB, {
+    required int streamId,
+    required String? topic,
+    required PerAccountStore store,
+  }) {
+    final recentSenders = store.recentSenders;
+    if (topic != null) {
+      final result = -compareRecentMessageIds(
+        recentSenders.latestMessageIdOfSenderInTopic(
+          streamId: streamId, topic: topic, senderId: userA.userId),
+        recentSenders.latestMessageIdOfSenderInTopic(
+          streamId: streamId, topic: topic, senderId: userB.userId));
+      if (result != 0) return result;
+    }
+
+    return -compareRecentMessageIds(
+      recentSenders.latestMessageIdOfSenderInStream(
+        streamId: streamId, senderId: userA.userId),
+      recentSenders.latestMessageIdOfSenderInStream(
+        streamId: streamId, senderId: userB.userId));
   }
 
   /// Determines which of the two users is more recent in DM conversations.
@@ -204,10 +290,21 @@ class MentionAutocompleteView extends ChangeNotifier {
     final aLatestMessageId = recentDms.latestMessagesByRecipient[userA.userId];
     final bLatestMessageId = recentDms.latestMessagesByRecipient[userB.userId];
 
-    return switch((aLatestMessageId, bLatestMessageId)) {
-      (int a, int b) => -a.compareTo(b),
-      (int(),     _) => -1,
-      (_,     int()) => 1,
+    return -compareRecentMessageIds(aLatestMessageId, bLatestMessageId);
+  }
+
+  /// Compares [a] to [b], with null less than all integers.
+  ///
+  /// The values should represent the most recent message ID in each of two
+  /// sets of messages, with null meaning the set is empty.
+  ///
+  /// Return values are as with [Comparable.compareTo].
+  @visibleForTesting
+  static int compareRecentMessageIds(int? a, int? b) {
+    return switch ((a, b)) {
+      (int a, int b) => a.compareTo(b),
+      (int(),     _) => 1,
+      (_,     int()) => -1,
       _              => 0,
     };
   }

test/model/autocomplete_test.dart

@@ -359,13 +359,83 @@ void main() {
     Future<void> prepare({
       List<User> users = const [],
       List<RecentDmConversation> dmConversations = const [],
+      List<Message> messages = const [],
     }) async {
       store = eg.store(initialSnapshot: eg.initialSnapshot(
         recentPrivateConversations: dmConversations));
       await store.addUsers(users);
+      await store.addMessages(messages);
     }
 
-    group('MentionAutocompleteView.compareByDms', () {
+    group('compareRecentMessageIds', () {
+      test('both a and b are non-null', () async {
+        check(MentionAutocompleteView.compareRecentMessageIds(2, 5)).isLessThan(0);
+        check(MentionAutocompleteView.compareRecentMessageIds(5, 2)).isGreaterThan(0);
+        check(MentionAutocompleteView.compareRecentMessageIds(5, 5)).equals(0);
+      });
+
+      test('one of a and b is null', () async {
+        check(MentionAutocompleteView.compareRecentMessageIds(null, 5)).isLessThan(0);
+        check(MentionAutocompleteView.compareRecentMessageIds(5, null)).isGreaterThan(0);
+      });
+
+      test('both of a and b are null', () async {
+        check(MentionAutocompleteView.compareRecentMessageIds(null, null)).equals(0);
+      });
+    });
+
+    group('compareByRecency', () {
+      final userA = eg.otherUser;
+      final userB = eg.thirdUser;
+      final stream = eg.stream();
+      const topic1 = 'topic1';
+      const topic2 = 'topic2';
+
+      Message message(User sender, String topic) {
+        return eg.streamMessage(sender: sender, stream: stream, topic: topic);
+      }
+
+      int compareAB({required String? topic}) {
+        final resultAB = MentionAutocompleteView.compareByRecency(userA, userB,
+          streamId: stream.streamId, topic: topic, store: store);
+        final resultBA = MentionAutocompleteView.compareByRecency(userB, userA,
+          streamId: stream.streamId, topic: topic, store: store);
+        switch (resultAB) {
+          case <0: check(resultBA).isGreaterThan(0);
+          case >0: check(resultBA).isLessThan(0);
+          default: check(resultBA).equals(0);
+        }
+        return resultAB;
+      }
+
+      test('favor user most recent in topic', () async {
+        await prepare(messages: [message(userA, topic1), message(userB, topic1)]);
+        check(compareAB(topic: topic1)).isGreaterThan(0);
+      });
+
+      test('favor most recent in topic ahead of most recent in stream', () async {
+        await prepare(messages: [
+          message(userA, topic1), message(userB, topic1), message(userA, topic2)]);
+        check(compareAB(topic: topic1)).isGreaterThan(0);
+      });
+
+      test('no activity in topic -> favor user most recent in stream', () async {
+        await prepare(messages: [message(userA, topic1), message(userB, topic1)]);
+        check(compareAB(topic: topic2)).isGreaterThan(0);
+      });
+
+      test('no topic provided -> favor user most recent in stream', () async {
+        await prepare(messages: [message(userA, topic1), message(userB, topic2)]);
+        check(compareAB(topic: null)).isGreaterThan(0);
+      });
+
+      test('no activity in topic/stream -> favor none', () async {
+        await prepare(messages: []);
+        check(compareAB(topic: null)).equals(0);
+      });
+    });
+
+    group('compareByDms', () {
       const idA = 1;
       const idB = 2;
 
@@ -418,61 +488,137 @@ void main() {
       });
     });
 
-    group('autocomplete suggests relevant users in the intended order', () {
-      // The order should be:
-      // 1. Users most recent in the DM conversations
-
-      Future<void> checkResultsIn(Narrow narrow, {required List<int> expected}) async {
-        final users = [
-          eg.user(userId: 0),
-          eg.user(userId: 1),
-          eg.user(userId: 2),
-          eg.user(userId: 3),
-          eg.user(userId: 4),
-        ];
-
-        final dmConversations = [
-          RecentDmConversation(userIds: [3],    maxMessageId: 300),
-          RecentDmConversation(userIds: [0],    maxMessageId: 200),
-          RecentDmConversation(userIds: [0, 1], maxMessageId: 100),
-        ];
-
-        await prepare(users: users, dmConversations: dmConversations);
+    group('ranking across signals', () {
+      void checkPrecedes(Narrow narrow, User userA, Iterable<User> usersB) {
         final view = MentionAutocompleteView.init(store: store, narrow: narrow);
+        for (final userB in usersB) {
+          check(view.debugCompareUsers(userA, userB)).isLessThan(0);
+          check(view.debugCompareUsers(userB, userA)).isGreaterThan(0);
+        }
+      }
 
-        bool done = false;
-        view.addListener(() { done = true; });
-        view.query = MentionAutocompleteQuery('');
-        await Future(() {});
-        check(done).isTrue();
-        final results = view.results
-          .map((e) => (e as UserMentionAutocompleteResult).userId);
-        check(results).deepEquals(expected);
+      void checkRankEqual(Narrow narrow, List<User> users) {
+        final view = MentionAutocompleteView.init(store: store, narrow: narrow);
+        for (int i = 0; i < users.length; i++) {
+          for (int j = i + 1; j < users.length; j++) {
+            check(view.debugCompareUsers(users[i], users[j])).equals(0);
+            check(view.debugCompareUsers(users[j], users[i])).equals(0);
+          }
+        }
       }
 
-      test('StreamNarrow', () async {
-        await checkResultsIn(const StreamNarrow(1), expected: [3, 0, 1, 2, 4]);
+      test('TopicNarrow: topic recency > stream recency > DM recency', () async {
+        final users = List.generate(5, (i) => eg.user());
+        final stream = eg.stream();
+        final narrow = TopicNarrow(stream.streamId, 'this');
+        await prepare(users: users, messages: [
+          eg.streamMessage(sender: users[1], stream: stream, topic: 'this'),
+          eg.streamMessage(sender: users[0], stream: stream, topic: 'this'),
+          eg.streamMessage(sender: users[2], stream: stream, topic: 'other'),
+          eg.streamMessage(sender: users[1], stream: stream, topic: 'other'),
+          eg.dmMessage(from: users[3], to: [users[4], eg.selfUser]),
+          eg.dmMessage(from: users[2], to: [eg.selfUser]),
+        ]);
+        checkPrecedes(narrow, users[0], users.skip(1));
+        checkPrecedes(narrow, users[1], users.skip(2));
+        checkPrecedes(narrow, users[2], users.skip(3));
+        checkRankEqual(narrow, [users[3], users[4]]);
       });
 
-      test('TopicNarrow', () async {
-        await checkResultsIn(const TopicNarrow(1, 'topic'), expected: [3, 0, 1, 2, 4]);
+      test('StreamNarrow: stream recency > DM recency', () async {
+        final users = List.generate(4, (i) => eg.user());
+        final stream = eg.stream();
+        final narrow = StreamNarrow(stream.streamId);
+        await prepare(users: users, messages: [
+          eg.streamMessage(sender: users[1], stream: stream),
+          eg.streamMessage(sender: users[0], stream: stream),
+          eg.dmMessage(from: users[2], to: [users[3], eg.selfUser]),
+          eg.dmMessage(from: users[1], to: [eg.selfUser]),
+        ]);
+        checkPrecedes(narrow, users[0], users.skip(1));
+        checkPrecedes(narrow, users[1], users.skip(2));
+        checkRankEqual(narrow, [users[2], users[3]]);
       });
 
-      test('DmNarrow', () async {
-        await checkResultsIn(
-          DmNarrow.withUser(eg.selfUser.userId, selfUserId: eg.selfUser.userId),
-          expected: [3, 0, 1, 2, 4],
-        );
+      test('DmNarrow: DM recency > this-conversation recency or stream recency', () async {
+        final users = List.generate(4, (i) => eg.user());
+        await prepare(users: users, messages: [
+          eg.dmMessage(from: users[3], to: [eg.selfUser]),
+          eg.dmMessage(from: users[1], to: [users[2], eg.selfUser]),
+          eg.dmMessage(from: users[0], to: [eg.selfUser]),
+          eg.streamMessage(sender: users[1]),
+          eg.streamMessage(sender: users[2]),
+          eg.streamMessage(sender: users[3]),
+        ]);
+        for (final narrow in [
+          DmNarrow.withUser(users[3].userId, selfUserId: eg.selfUser.userId),
+          DmNarrow.withOtherUsers([users[1].userId, users[2].userId],
+            selfUserId: eg.selfUser.userId),
+          DmNarrow.withUser(users[1].userId, selfUserId: eg.selfUser.userId),
+        ]) {
+          checkPrecedes(narrow, users[0], users.skip(1));
+          checkRankEqual(narrow, [users[1], users[2]]);
+          checkPrecedes(narrow, users[1], users.skip(3));
+          checkPrecedes(narrow, users[2], users.skip(3));
+        }
       });
 
-      test('CombinedFeedNarrow', () async {
-        // As we do not expect a compose box in [CombinedFeedNarrow], it should
-        // not proceed to show any results.
-        await check(checkResultsIn(
-          const CombinedFeedNarrow(),
-          expected: [0, 1, 2, 3, 4])
-        ).throws();
+      test('CombinedFeedNarrow gives error', () async {
+        await prepare(users: [eg.user(), eg.user()], messages: []);
+        const narrow = CombinedFeedNarrow();
+        check(() => MentionAutocompleteView.init(store: store, narrow: narrow))
+          .throws<AssertionError>();
       });
     });
+
+    test('final results end-to-end', () async {
+      Future<Iterable<int>> getResults(
+          Narrow narrow, MentionAutocompleteQuery query) async {
+        bool done = false;
+        final view = MentionAutocompleteView.init(store: store, narrow: narrow);
+        view.addListener(() { done = true; });
+        view.query = query;
+        await Future(() {});
+        check(done).isTrue();
+        final results = view.results
+          .map((e) => (e as UserMentionAutocompleteResult).userId);
+        view.dispose();
+        return results;
+      }
+
+      final stream = eg.stream();
+      const topic = 'topic';
+      final topicNarrow = TopicNarrow(stream.streamId, topic);
+
+      final users = [
+        eg.user(userId: 1, fullName: 'User One'),
+        eg.user(userId: 2, fullName: 'User Two'),
+        eg.user(userId: 3, fullName: 'User Three'),
+        eg.user(userId: 4, fullName: 'User Four'),
+        eg.user(userId: 5, fullName: 'User Five'),
+      ];
+
+      await prepare(users: users, messages: [
+        eg.streamMessage(id: 50, sender: users[1-1], stream: stream, topic: topic),
+        eg.streamMessage(id: 60, sender: users[5-1], stream: stream, topic: 'other $topic'),
+      ], dmConversations: [
+        RecentDmConversation(userIds: [4],    maxMessageId: 300),
+        RecentDmConversation(userIds: [1],    maxMessageId: 200),
+        RecentDmConversation(userIds: [1, 2], maxMessageId: 100),
+      ]);
+
+      // Check the ranking of the full list of users.
+      // The order should be:
+      // 1. Users most recent in the current topic/stream.
+      // 2. Users most recent in the DM conversations.
+      check(await getResults(topicNarrow, MentionAutocompleteQuery('')))
+        .deepEquals([1, 5, 4, 2, 3]);
+
+      // Check the ranking applies also to results filtered by a query.
+      check(await getResults(topicNarrow, MentionAutocompleteQuery('t')))
+        .deepEquals([2, 3]);
+      check(await getResults(topicNarrow, MentionAutocompleteQuery('f')))
+        .deepEquals([5, 4]);
+    });
   });
 }