mu-query-threads: small refinements

Ensure we sort correctly (sort_container), since the last message in a
thread is not necessarily the last (when not sorted by thread-date).

own_date_key is redundant, so let's remove it.

lib/mu-query-threads.cc

@@ -62,7 +62,7 @@ struct Container {
 		return up && up == other->ur_parent();
 	}
 
-	template <typename Func> void for_each_child(Func&& func) {
+	template <typename Func> void for_each_child(const Func& func) {
 		auto it{children.rbegin()};
 		while (it != children.rend()) {
 			auto next = std::next(it);
@@ -80,13 +80,6 @@ struct Container {
 
 	std::string thread_date_key;
 
-	// The container's own date-key, set once from its query-match and never
-	// overwritten by the bubble-up above. Siblings must sort by this, not by
-	// thread_date_key -- otherwise a subthread gaining a new, later message
-	// (e.g. a reply to one message in a patch series) reorders it among its
-	// siblings, even though its own date didn't change.
-	std::string own_date_key;
-
 	Option<QueryMatch&> query_match;
 	bool                is_nuked{};
 	Container*          parent{};
@@ -195,12 +188,14 @@ determine_id_table(QueryResultsType& qres)
 		}();
 
 		// We sort by date (ascending), *except* for the root; we don't
-		// know what query_matchs will be at the root level yet, so remember
-		// both. Moreover, even when sorting the top-level in descending
+		// know what query_matchs will be at the root level yet, so
+		// remember both.
+		//
+		// Moreover, even when sorting the top-level in descending
 		// order, still sort the thread levels below that in ascending
 		// order, keeping sibling order stable.
-		container.own_date_key = container.thread_date_key =
-		    container.query_match->date_key = mi.date_str().value_or("");
+		container.thread_date_key = container.query_match->date_key =
+		    mi.date_str().value_or("");
 		// initial guess for the thread-date; might be updated
 		// later.
 
@@ -494,17 +489,24 @@ sort_container(Container& container)
 	for (auto& child : container.children)
 		sort_container(*child);
 
-	// now sort this level, by each child's own date, falling back to
-	// thread_date_key if the container has no message of its own; use a
-	// stable sort so messages with equal dates keep their original (mset)
-	// order.
-	std::ranges::stable_sort(container.children, {}, [](const Container* c) {
-		return c->own_date_key.empty() ? c->thread_date_key : c->own_date_key;
+	// now sort this level, by each child's _own_ date, not the (bubbled-up)
+	// thread-date -- otherwise a subthread gaining a new message could
+	// reorder wrongly.
+	//
+	// Fall back to the thread-date for containers without a message of
+	// their own; use a stable sort so messages with equal dates keep their
+	// original (mset) order.
+	std::ranges::stable_sort(container.children, {},
+				 [](const Container* c) -> const std::string& {
+		return c->query_match ? c->query_match->date_key : c->thread_date_key;
 	});
 
 	// and 'bubble up' the date of the *newest* message with a date. We
-	// reasonably assume that it's later than its parent.
-	const auto& newest_date = container.children.back()->thread_date_key;
+	// reasonably assume that it's later than its parent. Note that since
+	// children are not sorted by thread-date, the newest is not necessarily
+	// the last child.
+	const auto& newest_date = std::ranges::max(
+	    container.children, {}, &Container::thread_date_key)->thread_date_key;
 	if (!newest_date.empty())
 		container.thread_date_key = newest_date;
 }
@@ -735,6 +737,32 @@ test_sibling_order_stable()
 			    });
 }
 
+static void
+test_thread_date_bubble_up()
+{
+	// thread A's newest message (date 9) hangs under a middle sibling;
+	// thread B (date 5) is older than that, so ascending order must put A
+	// *after* B.
+	auto results = MockQueryResults{
+	    MockQueryResult{"a", "1", {}},
+	    MockQueryResult{"a1", "2", {"a"}},
+	    MockQueryResult{"a2", "3", {"a"}},
+	    MockQueryResult{"reply-a1", "9", {"a", "a1"}},
+	    MockQueryResult{"b", "5", {}},
+	};
+
+	calculate_threads(results, false);
+
+	assert_thread_paths(results,
+			    {
+				{"b", "0"},
+				{"a", "1"},
+				{"a1", "1:0"},
+				{"reply-a1", "1:0:0"},
+				{"a2", "1:1"},
+			    });
+}
+
 static void
 test_id_table_inconsistent()
 {
@@ -966,6 +994,7 @@ try {
 	g_test_add_func("/threader/sort/ascending", test_sort_ascending);
 	g_test_add_func("/threader/sort/decending", test_sort_descending);
 	g_test_add_func("/threader/sort/sibling-order-stable", test_sibling_order_stable);
+	g_test_add_func("/threader/sort/thread-date-bubble-up", test_thread_date_bubble_up);
 
 	g_test_add_func("/threader/id-table-inconsistent", test_id_table_inconsistent);
 	g_test_add_func("/threader/dups/dup-last", test_dups_dup_last);

lib/mu-query-threads.hh

@@ -1,5 +1,5 @@
 /*
-** Copyright (C) 2021 Dirk-Jan C. Binnema <djcb@djcbsoftware.nl>
+** Copyright (C) 2021-2026 Dirk-Jan C. Binnema <djcb@djcbsoftware.nl>
 **
 ** This program is free software; you can redistribute it and/or modify it
 ** under the terms of the GNU General Public License as published by the
@@ -28,6 +28,9 @@ namespace Mu {
  * thread-paths for each message, so we can let Xapian order them in the correct
  * order.
  *
+ * This is strongly inspired by the JWZ message threading algorithm
+ *    https://www.jwz.org/doc/threading.html
+ *
  * Note - threads are sorted chronologically, and the messages below the top
  * level are always sorted in ascending orde
  *