feat: treat a mixed conversation's unread state as unread

Item 112 hid the toggle whenever the selection disagreed, because a union
is not a state and no honest label existed for it. That was affordable
because the "Whole thread" submenu sat beside it carrying two absolute
entries, which worked whatever the mix.

Item 177 deletes that submenu: the row decides the scope, so a second set
of actions is a second answer to a settled question. Hiding the toggle
then leaves the commonest conversation in the mailbox with no key at all.

The rule is a catch-all instead. Any unread message, a mixed conversation
included, reads "Mark thread as read" and marks every message read; only a
fully read selection reads "Mark thread as unread". Two presses therefore
reach either state from anywhere, which is what makes one key enough.

The write direction moves with the label. Computing it from
everySelectedRowHasTag() while the label promised "read" would mark a
mixed conversation unread, which is the item 112 report happening again
from the other end; the mutation putting that back fails the new test.

The three-valued selectionTagPresence() is unchanged and still asked, since
Every and Mixed differ for other callers. Only this label collapses them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PEK9z5D3oa1nVmJ6xpQhBs

2 files changed, 95 insertions, 56 deletions

diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 7d9cfbc..c8aa811 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -1763,7 +1763,14 @@ void MainWindow::registerActions()
         // comment said the direction came from the current row while the
         // change applied to the whole selection, which is the same split that
         // makes a mixed selection land in two states.
-        const bool unread = everySelectedRowHasTag(QStringLiteral("unread"));
+        //
+        // ANY unread rather than EVERY, which is item 177's catch-all rule and
+        // must be the same question refreshUnreadAction() asks for the label:
+        // a label promising "Mark thread read" over a write computed from
+        // Every would mark a mixed conversation UNREAD, which is the item 112
+        // report happening again from the other end.
+        const bool unread = selectionTagPresence(QStringLiteral("unread"))
+                            != TagPresence::None;
 
         // An explicit toggle overrides the automatic one. Without this, marking
         // a thread unread by hand would be undone a moment later by a timer
@@ -3669,16 +3676,21 @@ void MainWindow::refreshTrashActions()
 
 void MainWindow::refreshUnreadAction()
 {
-    // The user's design (item 112 and its duplicates 99/147): the label says
-    // which way the action will go, and on a selection with no single state
-    // the entry is HIDDEN rather than labelled wrongly.
+    // Item 112 hid this entry whenever the selection disagreed, because a
+    // union is not a state and no honest label existed. The route out was the
+    // "Whole thread" submenu, whose two entries were absolute rather than a
+    // toggle. Item 177 deleted that submenu: the ROW decides the scope, so a
+    // second set of actions was a second answer to a settled question.
+    //
+    // With one key left, hiding on disagreement leaves the commonest
+    // conversation in the mailbox with no key at all, so the rule is a
+    // catch-all instead: ANY unread message reads "Mark thread read" and marks
+    // every message read; only a fully read selection reads "Mark thread
+    // unread". Mixed is not a special case, it is the ordinary one.
     //
-    // The "Whole thread" submenu used to be the route out of the hidden case,
-    // with two absolute entries that worked whatever the mix. It is gone
-    // (item 177), and nothing replaces it: on a conversation row the label
-    // now names the THREAD, and the thread's union is a single state for the
-    // same reason it was not a message's. A genuinely mixed MULTI-row
-    // selection still hides the entry, and Edit tags beside it is the route.
+    // That keeps one key sufficient, which is what the hidden case cost. Two
+    // presses reach either state from anywhere: mark read collapses the mix to
+    // a state, and the second press toggles out of it.
     auto *action = m_actions.value(QStringLiteral("toggle_unread"));
     if (!action)
         return;
@@ -3690,9 +3702,15 @@ void MainWindow::refreshUnreadAction()
     const bool namesTheThread = kind == SelectionKind::Conversations
                                 || kind == SelectionKind::Mixed;
 
-    switch (selectionTagPresence(QStringLiteral("unread"))) {
-    case TagPresence::Every:
-        action->setVisible(true);
+    // Mixed joins Every rather than hiding: both mean "something here is
+    // unread", which is the question the direction actually turns on. The
+    // three-valued answer is still what is asked, because Every and Mixed
+    // differ for other callers; only this label collapses them.
+    const bool anyUnread =
+        selectionTagPresence(QStringLiteral("unread")) != TagPresence::None;
+
+    action->setVisible(true);
+    if (anyUnread) {
         action->setText(namesTheThread ? tr("Mark thread as &read")
                                        : tr("Mark as &read"));
         action->setStatusTip(
@@ -3700,9 +3718,7 @@ void MainWindow::refreshUnreadAction()
                 ? tr("Remove the unread tag from every message of the "
                      "selected threads")
                 : tr("Remove the unread tag from the selection"));
-        break;
-    case TagPresence::None:
-        action->setVisible(true);
+    } else {
         action->setText(namesTheThread ? tr("Mark thread as &unread")
                                        : tr("Mark as &unread"));
         action->setStatusTip(
@@ -3710,12 +3726,6 @@ void MainWindow::refreshUnreadAction()
                 ? tr("Add the unread tag to every message of the selected "
                      "threads")
                 : tr("Add the unread tag to the selection"));
-        break;
-    case TagPresence::Mixed:
-        // No honest label exists, so there is no label to show. Hidden rather
-        // than disabled, at the user's choice.
-        action->setVisible(false);
-        break;
     }
 }
 
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index 8fe1f87..26ec485 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -396,8 +396,8 @@ private slots:
     void emptyTrashAsksBeforeDestroyingAnything();
     void theUnreadLabelSaysWhichDirectionItWillGo();
     void theUnreadLabelFollowsAWriteWithoutReselecting();
-    void theUnreadActionIsHiddenOnAMixedSelection();
-    void aMixedThreadIsMarkedReadAndEditTagsIsTheWayBack();
+    void aMixedSelectionIsMarkedReadAndTheActionStaysVisible();
+    void aMixedThreadIsMarkedReadAndTheSecondPressIsTheWayBack();
     void toggleUnreadOnAReplyReadsTheReplysOwnState();
     void toggleUnreadOnAReplyRepaintsItInBothDirections();
     void taggingTheOpenReplyUpdatesTheMessagePaneStrip();
@@ -5469,15 +5469,17 @@ void TestMainWindow::theUnreadLabelFollowsAWriteWithoutReselecting()
                                        "marked read").arg(action->text())));
 }
 
-void TestMainWindow::theUnreadActionIsHiddenOnAMixedSelection()
+void TestMainWindow::aMixedSelectionIsMarkedReadAndTheActionStaysVisible()
 {
-    // The other half of the same note: "on a thread with mixed states it
-    // should be hidden, we have a submenu for thread actions".
+    // This test asserted the opposite until item 177. The note behind it said
+    // "on a thread with mixed states it should be hidden, we have a submenu
+    // for thread actions", and the second clause was the load-bearing one: the
+    // submenu carried two absolute entries that worked whatever the mix, so
+    // hiding the toggle cost the user nothing.
     //
-    // A selection spanning an unread row and a read one has no single state,
-    // so no honest label exists for it. Hiding the entry sends the user to
-    // the thread submenu, whose entries are absolute and work regardless of
-    // the mix.
+    // Item 177 deleted that submenu, which took the route out with it. The
+    // rule is a catch-all now: any unread row reads "mark read" and the entry
+    // is never hidden, so one key still reaches both states in two presses.
     const Config config;
     MainWindow window(config);
 
@@ -5497,28 +5499,44 @@ void TestMainWindow::theUnreadActionIsHiddenOnAMixedSelection()
     // index invalid, so a test using it passes against a missing guard
     // (CLAUDE.md).
     view->setCurrentIndex(model->index(0, 0, {}));
-    QVERIFY2(action->isVisible(), "a single row already has no single state");
 
     view->selectionModel()->select(
         model->index(1, 0, {}),
         QItemSelectionModel::Select | QItemSelectionModel::Rows);
     QCOMPARE(view->selectionModel()->selectedRows().size(), 2);
 
-    QVERIFY2(!action->isVisible(),
-             qPrintable(QStringLiteral("a mixed selection still offers the "
-                                       "unread action, labelled: %1")
+    QVERIFY2(action->isVisible(),
+             "the mixed selection hid the unread action, which now leaves no "
+             "key at all: the submenu that used to be the way out is gone");
+    QVERIFY2(action->text().contains(QStringLiteral("read"), Qt::CaseInsensitive)
+                 && !action->text().contains(QStringLiteral("unread"),
+                                             Qt::CaseInsensitive),
+             qPrintable(QStringLiteral("a mixed selection must promise the "
+                                       "read direction, not: %1")
                             .arg(action->text())));
 
-    // ...and it comes back when the selection agrees again, or the entry
-    // would be gone for the rest of the session.
-    view->selectionModel()->select(
-        model->index(1, 0, {}),
-        QItemSelectionModel::Deselect | QItemSelectionModel::Rows);
-    QVERIFY2(action->isVisible(),
-             "the action did not return when the selection agreed again");
+    // And the write goes the way the label promised. A direction computed from
+    // "every row unread" would mark this selection UNREAD while the label said
+    // read, which is the item 112 report from the other end.
+    action->trigger();
+
+    QVERIFY2(window.undoTextForTesting().contains(QStringLiteral("Mark read")),
+             qPrintable(QStringLiteral("the write disagreed with the label on "
+                                       "a mixed selection: %1")
+                            .arg(window.undoTextForTesting())));
+    QVERIFY2(!model->threadAt(0).isUnread() && !model->threadAt(1).isUnread(),
+             "the mixed selection did not end up uniformly read, so a second "
+             "press has no single state to toggle out of");
+
+    // Which is what makes one key enough: the second press is the way back.
+    QVERIFY2(action->text().contains(QStringLiteral("unread"),
+                                     Qt::CaseInsensitive),
+             qPrintable(QStringLiteral("the label did not follow the write, so "
+                                       "the second press repeats the first: %1")
+                            .arg(action->text())));
 }
 
-void TestMainWindow::aMixedThreadIsMarkedReadAndEditTagsIsTheWayBack()
+void TestMainWindow::aMixedThreadIsMarkedReadAndTheSecondPressIsTheWayBack()
 {
     // What item 112 became under item 177. That item's report was real: on a
     // thread with two unread replies, asking to mark the whole thread unread
@@ -5528,15 +5546,13 @@ void TestMainWindow::aMixedThreadIsMarkedReadAndEditTagsIsTheWayBack()
     //
     // Its fix was two fixed-direction thread actions in a submenu. Item 177
     // deleted that submenu: the ROW decides the scope, so a second set of
-    // actions was a second answer to a settled question. The cost is recorded
-    // here rather than hidden. On a mixed conversation the toggle still goes
-    // ONE way, and that way is "mark read", which is the safe direction: it
-    // takes the thread to a state it can then be toggled out of, where the
-    // reverse would have left it mixed and the key still dead.
+    // actions was a second answer to a settled question.
     //
-    // The way back is Edit tags, which is absolute rather than a toggle and
-    // works whatever the mix. That is what the deleted submenu was really
-    // providing, and it did not need six actions to provide it.
+    // Nothing is lost with it, which is the point of this test. On a mixed
+    // conversation the toggle goes ONE way, and that way is "mark read",
+    // which is the direction that RESOLVES the mix: the thread lands in a
+    // single state, and the second press toggles out of it. The reverse would
+    // have left it mixed and the key still dead.
     const Config config;
     MainWindow window(config);
 
@@ -5601,11 +5617,24 @@ void TestMainWindow::aMixedThreadIsMarkedReadAndEditTagsIsTheWayBack()
              "the thread's own tags did not move, so the write was scoped to "
              "one message and the other two are still unread");
 
-    // The route back exists and is not the toggle.
-    auto *editTags = window.findChild<QAction *>(QStringLiteral("edit_tags"));
-    QVERIFY2(editTags && editTags->isEnabled(),
-             "Edit tags is the absolute route the deleted submenu used to "
-             "provide, and it is not available");
+    // The route back is the same key, which is what makes one key enough. The
+    // label has to follow the write for that to be true: a stale "Mark thread
+    // as read" would make the second press repeat the first, and the user
+    // would report the key as dead exactly as they did under item 112.
+    QVERIFY2(toggle->isVisible(),
+             "the toggle vanished after resolving the mix, so there is no way "
+             "back to unread");
+    QVERIFY2(toggle->text().contains(QStringLiteral("unread"),
+                                     Qt::CaseInsensitive),
+             qPrintable(QStringLiteral("the label did not follow the write: %1")
+                            .arg(toggle->text())));
+
+    toggle->trigger();
+
+    QVERIFY2(model->threadAt(0).isUnread(),
+             "the second press did not take the thread back to unread, so one "
+             "key does not reach both states and the deleted submenu really "
+             "was carrying something");
 }
 
 void TestMainWindow::toggleUnreadOnAReplyReadsTheReplysOwnState()