Say where a conversation belongs when it is added, changed or removed

include/session/client.hpp

@@ -1204,7 +1204,11 @@ class Client {
 
     void _emit_conversation_added(const ConversationId& id);
     void _emit_conversation_removed(const ConversationId& id);
-    void _emit_lists_replaced();
+    // Reports both lists as wholly changed -- a row added, removed, or moved to a new position --
+    // through whichever handlers the subscriber registered.  A replacement carries the order, so
+    // this cannot send one without also considering the order event, or a subscriber holding only
+    // that handler hears nothing.
+    void _report_lists_replaced(bool convos, bool requests);
     void _emit_history_replaced(const ConversationId& id);
     // Reports a message, and then reports every message that replies to it.
     //
@@ -1231,6 +1235,26 @@ class Client {
     bool _flush_scheduled = false;
     void _touch(const ConversationId& id);
     void _flush_pending();
+    // Reports one list, if a row in it changed and the subscriber asked for it.  The rows are the
+    // expensive part, so nothing is read for a handler that is not there.
+    // Which list each conversation was last *reported* in.  The subscriber's belief rather than
+    // the database's state, which is the point: it is what the subscriber has to take the row out
+    // of before putting it where it now belongs.
+    //
+    // Keyed by ConversationId and not by row id, because a removal is reported *after* the row is
+    // deleted -- `_delete_contact` commits the DELETE first -- so there is nothing left to look a
+    // row id up from, and an entry keyed that way could never be erased.
+    std::unordered_map<ConversationId, ConversationList> _placed;
+    // Where a conversation was and where it belongs now, and records the latter.
+    ListPlacement _place(const AnyConversation& convo);
+    void _report_list(
+            bool changed,
+            ConversationList list_kind,
+            std::vector<AnyConversation> (Client::*rows)(),
+            std::function<void(std::vector<AnyConversation>&&)> callbacks::* replaced);
+    // Reports both lists, given for each whether a row in it changed.  Called by `_flush_pending`,
+    // once per batch.
+    void _report_lists(bool convos_changed, bool requests_changed);
 
   public:
     /// The account state this Client is built on: keys, device group, configs, polling.  A

include/session/client/callbacks.hpp

@@ -11,6 +11,41 @@
 #include <vector>
 
 namespace session::client {
+/// Which of the two lists a conversation sits in.
+///
+/// `none` is a real answer rather than a missing one: a hidden conversation is in neither list, and
+/// so is one the subscriber has not been shown.
+enum class ConversationList {
+    none,
+    conversations,
+    requests,
+};
+
+/// Where a conversation was, and where it belongs now.
+///
+/// Enough to apply on its own, and it reads as the two steps it is:
+///
+///     if (p.from != ConversationList::none) remove(p.from, convo.id());
+///     if (p.to   != ConversationList::none) insert(p.to, std::move(convo), p.after);
+///
+/// `from` is the list the subscriber was last *told* this row was in, which is what it is holding
+/// rather than what the database now says. It saves searching the list the row did not come from;
+/// it does not save finding the row, which is a lookup by id either way.
+///
+/// The two lists are ordered differently -- conversations by `priority DESC, last_activity DESC,
+/// id` and requests by `last_activity DESC, id`, with no priority term -- so which list a position
+/// is in is part of the position rather than a detail.
+struct ListPlacement {
+    /// Where the subscriber is holding this row, so it knows which list to take it out of.
+    /// `none` when it is holding it nowhere: a row it has not been shown, or one that was hidden.
+    ConversationList from = ConversationList::none;
+    /// Where it belongs now.  `none` means neither list, which is what hiding does -- then it is
+    /// only removed.
+    ConversationList to = ConversationList::none;
+    /// The row it now follows in `to`; unset means first in that list.  Meaningless, and always
+    /// unset, when `to` is `none`.
+    std::optional<ConversationId> after;
+};
 
 /// Notifications of everything the conversation layer changes, so that an application never has to
 /// ask.  A caller sets the handlers it cares about and leaves the rest empty; an unset handler is
@@ -39,21 +74,37 @@ namespace session::client {
 /// is not a template on its argument — it is a promise by the caller that the object is spent
 /// afterwards.
 ///
-/// The conversation list an application maintains from these is expected to be *complete*: ordering
-/// is a comparison against every other conversation, so a partial list cannot be sorted.  Showing
-/// only part of it is fine, holding only part of it is not.
+/// The conversation list an application maintains from these is expected to be *complete*: the
+/// order is given as a whole list, so a partial one cannot be placed in it.  Showing only part of
+/// it is fine, holding only part of it is not.
+///
+/// The order itself is **ours, not the application's**.  Every handler that carries a list carries
+/// it already ordered, so an application never has to sort, and should not: the two lists are not
+/// sorted the same way and a comparator copied from one gets the other wrong.
 struct callbacks {
     /// A conversation now exists that did not before.
-    std::function<void(AnyConversation&&)> conversation_added;
+    ///
+    /// Carries where it belongs, on the same terms as `conversation_updated`: `from` is normally
+    /// `none`, since a row that did not exist was not being held anywhere.
+    std::function<void(AnyConversation&&, ListPlacement&&)> conversation_added;
 
     /// A conversation's contents changed: a new or edited message, a name, an unread count, its
     /// last activity.  Fired once with the conversation's settled state rather than once per
     /// underlying change, so a poll that delivers fifty messages to one conversation fires this
     /// once.
-    std::function<void(AnyConversation&&)> conversation_updated;
+    ///
+    /// The second argument says where the row was and where it belongs now, which is enough to
+    /// apply without consulting anything: remove it from `from`, insert it into `to`.
+    ///
+    /// **Applying these in order is what keeps a list correct.**  Each one places a row relative to
+    /// another, so one applied out of order, or skipped, leaves the list wrong with nothing to
+    /// detect it.  A subscriber that has not read the list once with `conversations()` has nothing
+    /// to place rows into.
+    std::function<void(AnyConversation&&, ListPlacement&&)> conversation_updated;
 
-    /// A conversation is gone and should be dropped from the list.
-    std::function<void(ConversationId&&)> conversation_removed;
+    /// A conversation is gone and should be dropped from the list it is in, which is the second
+    /// argument -- `none` if it was never shown in one.  Saves searching both.
+    std::function<void(ConversationId&&, ConversationList)> conversation_removed;
 
     /// Priorities changed — a pin, unpin, hide or unhide — carrying the whole list in its new
     /// order.  A replacement rather than a description of what moved, because one config update
@@ -71,6 +122,7 @@ struct callbacks {
     /// belongs to — and `Conversation::request` is what says which one a given handler is about.
     std::function<void(std::vector<AnyConversation>&&)> request_list_replaced;
 
+
     /// A message was added, whether received or sent from here.
     std::function<void(ConversationId&&, Message&&)> message_added;