feat: judge a row's membership on the thread's union

Closes item 170 under item 177. A conversation belongs to a view while any
of its messages match, so reading one message of a thread no longer takes
the conversation out of the Unread view. The current row is never evicted,
and an automatic write defers its eviction until the selection moves.

5 files changed, 405 insertions, 0 deletions

diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index c8aa811..c781d77 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -3847,6 +3847,11 @@ void MainWindow::onSelectionChanged()
     // emitted BEFORE the selection model is updated, so a handler reading
     // selectedRows() there sees the PREVIOUS selection and would label the
     // action for the rows the user just left (CLAUDE.md, verified Qt 6.11).
+    //
+    // Before anything else reads the model: the user has moved off whatever
+    // row they were on, so a row held back by syncViewMembership() leaves now.
+    flushDeferredEviction();
+
     refreshUnreadAction();
     refreshScopedActionLabels();
     refreshTrashActions();
@@ -5365,8 +5370,15 @@ void MainWindow::markCurrentThreadRead()
     // It still funnels through the one applyTags path, per CLAUDE.md; what
     // differs is only whether the inverse is pushed, which is a window-level
     // decision above the worker.
+    //
+    // Flagged as AUTOMATIC for syncViewMembership(): the user did not ask for
+    // this write, so the row it changes must not be taken out from under them.
+    // A write they DID ask for evicts at once; the distinction is who
+    // initiated it, not what it does (item 177).
+    m_automaticWrite = true;
     sendMessageTagChange(messageIds, {}, { QStringLiteral("unread") },
                          tr("Mark read"));
+    m_automaticWrite = false;
 }
 
 QString MainWindow::currentThreadFirstMessageId() const
@@ -5605,6 +5617,40 @@ void MainWindow::sendMessageTagChange(const QStringList &messageIds,
             m_model->messageById(m_currentMessageId).tags);
     }
 
+    // A row that no longer belongs in the view LEAVES it, rather than sitting
+    // there repainted until the next query. Beside the repaint above and
+    // before the sync hold below, since a held edit is applied optimistically
+    // too and its row is just as wrong to keep.
+    //
+    // The threads the touched messages belong to, with no filter on which
+    // message the card draws: a row is the conversation, so it is the UNION
+    // that decides, and the union is what removeThreadsWithoutTag() reads. A
+    // message the model does not hold names no thread, and is what the refresh
+    // inside the sync is for.
+    //
+    // Named only when this write MOVED the union, which is the one case a
+    // message edit can. applyMessageTagChange() keeps the summary in step for
+    // a thread of one, where the union IS the message, and deliberately leaves
+    // a longer thread's summary alone because one message's edit does not
+    // describe the conversation. Putting a longer thread up for eviction here
+    // would judge it on a union this write never touched: a stale answer, and
+    // wrong in both directions. A conversation leaves the view when a
+    // THREAD-scoped write empties its union, which is the other call site.
+    QStringList touchedThreads;
+    bool aRowIsMissing = false;
+    for (const QString &messageId : messageIds) {
+        const QString threadId = m_model->threadIdForMessage(messageId);
+        if (threadId.isEmpty()) {
+            aRowIsMissing = true;
+            continue;
+        }
+        if (m_model->threadCountFor(threadId) > 1)
+            continue;
+        if (!touchedThreads.contains(threadId))
+            touchedThreads.append(threadId);
+    }
+    syncViewMembership(touchedThreads, aRowIsMissing, add, remove);
+
     // The accounts this touches, resolved through the containing threads: the
     // account is a property of the thread, and the sync needs the channel
     // whether one message moved or seven.
@@ -6488,6 +6534,130 @@ void MainWindow::sendMove(const QStringList &messageIds,
                               Q_ARG(QString, destFolder));
 }
 
+void MainWindow::syncViewMembership(const QStringList &threadIds,
+                                    bool aRowIsMissing,
+                                    const QStringList &added,
+                                    const QStringList &removed)
+{
+    // Item 177. The optimistic REPAINT has always been universal; the
+    // optimistic MEMBERSHIP was not, and lived on the move path alone. So
+    // marking a thread read in the Unread view repainted its row and left it
+    // in a list defined by `tag:unread` that it no longer matched, until the
+    // next query or sync took it away.
+    //
+    // Membership is the UNION, one rule and no exceptions: a thread belongs
+    // to a view while any of its messages match it. The judgement itself is
+    // in removeThreadsWithoutTag(), which reads `summary.tags`; what happens
+    // here is only deciding WHICH rows to put to it and WHEN.
+    //
+    // Guarded on the VIEW's own tag, resolved from the query rather than
+    // assumed: a plain `tag:<x>` query is the only shape whose membership one
+    // tag decides. A path query (Trash, Sent, Drafts) is unaffected by a tag
+    // going away, and an arbitrary query the user typed cannot be reasoned
+    // about at all, so both are left alone and correct at the next sync.
+    // Without that guard, marking read in an `id:` view would empty the list.
+    //
+    // The exposure this accepts, deliberately and unchanged from the move
+    // path: revertPendingTagChange() repaints a rejected write but cannot
+    // REINSERT a row, so a write that fails leaves the row gone until the next
+    // query. Waiting for confirmation instead would give back exactly the lag
+    // this removes, and a rejected tag write is the rare case while the lag
+    // was every keystroke.
+    const QString viewTag = viewFilterTag();
+    if (viewTag.isEmpty())
+        return;
+
+    // The INVERSE, which the model cannot do on its own: a row that starts
+    // matching cannot be inserted optimistically, since the model holds no
+    // summary for a thread the query never returned. A refresh is what
+    // expresses it, exactly as the trash view already does after a restore.
+    // It matters most for UNDO: undoing a mark-read in the Unread view adds
+    // the tag back, and without this the row stayed gone, which would make an
+    // undone action invisible in the view it was undone in.
+    // refreshCurrentQuery() clears nothing, so the selection, the expansions
+    // and the undo stack all survive.
+    //
+    // Only when the row is genuinely ABSENT, which is the whole cost of the
+    // branch. Adding the view's tag to a row still in the list is the ordinary
+    // case, and refreshing there re-runs the query on every such keystroke.
+    if (added.contains(viewTag)) {
+        if (aRowIsMissing)
+            refreshCurrentQuery();
+        return;
+    }
+
+    if (!removed.contains(viewTag))
+        return;
+
+    // A row is never evicted while the user is sitting on it. The automatic
+    // mark-read fires two seconds after selection, so evicting on it takes the
+    // row out from under them, with a context menu possibly open on it, before
+    // they can mark it spam or important. The row leaves when the selection
+    // moves, which flushDeferredEviction() does, so the view still empties as
+    // they work. A write the user ASKED for evicts at once: the distinction is
+    // who initiated it, not what it does.
+    QStringList onScreen;
+    if (m_automaticWrite) {
+        const QModelIndexList selectedRows =
+            m_threadView->selectionModel()->selectedRows();
+        for (const QModelIndex &row : selectedRows)
+            onScreen.append(m_model->threadFor(row).threadId);
+        const QModelIndex current = m_threadView->currentIndex();
+        if (current.isValid())
+            onScreen.append(m_model->threadFor(current).threadId);
+    }
+
+    QStringList evictable;
+    for (const QString &threadId : threadIds) {
+        if (onScreen.contains(threadId)) {
+            if (!m_deferredEvictions.contains(threadId))
+                m_deferredEvictions.append(threadId);
+            continue;
+        }
+        evictable.append(threadId);
+    }
+
+    m_model->removeThreadsWithoutTag(evictable, viewTag);
+}
+
+void MainWindow::flushDeferredEviction()
+{
+    // The rows that stopped matching while the user was on them, taken out now
+    // that they have moved on. Re-checked against the model rather than
+    // trusted: the tag may have come back (an undo, a sync), in which case
+    // removeThreadsWithoutTag() correctly keeps the row.
+    if (m_deferredEvictions.isEmpty())
+        return;
+
+    const QString viewTag = viewFilterTag();
+    if (viewTag.isEmpty()) {
+        m_deferredEvictions.clear();
+        return;
+    }
+
+    // Every thread the user is on, from the SELECTION rather than from
+    // currentIndex(): a click calls select() before setCurrentIndex(), so at
+    // the moment selectionChanged arrives the current index still names the
+    // row being left, and reading it would hold the eviction back for ever.
+    QStringList onScreen;
+    const QModelIndexList rows =
+        m_threadView->selectionModel()->selectedRows();
+    for (const QModelIndex &row : rows)
+        onScreen.append(m_model->threadFor(row).threadId);
+
+    QStringList ready;
+    QStringList stillSelected;
+    for (const QString &threadId : m_deferredEvictions) {
+        if (onScreen.contains(threadId))
+            stillSelected.append(threadId);
+        else
+            ready.append(threadId);
+    }
+    m_deferredEvictions = stillSelected;
+
+    m_model->removeThreadsWithoutTag(ready, viewTag);
+}
+
 QString MainWindow::viewFilterTag() const
 {
     const QString query = m_queryEdit->text().trimmed();
@@ -6658,6 +6828,15 @@ void MainWindow::sendThreadTagChange(const QStringList &threadIds,
             m_messageView->setTags(m_model->threadFor(current).tags);
     }
 
+    // Same as the message path: a thread whose union stops matching the view
+    // leaves it now rather than at the next query. A thread-scoped write moves
+    // the summary, which is what the membership judgement reads, so a thread
+    // marked read is emptied of `unread` in one step and correctly evicted.
+    bool aRowIsMissing = false;
+    for (const QString &threadId : threadIds)
+        aRowIsMissing = aRowIsMissing || !m_model->hasThread(threadId);
+    syncViewMembership(threadIds, aRowIsMissing, add, remove);
+
     // A sync holds notmuch's exclusive write lock, and the worker's read-write
     // open BLOCKS on it rather than failing: measured 9.158s against a 12s
     // hold, returning SUCCESS. Sending now would freeze the worker thread for
diff --git a/src/mainwindow.h b/src/mainwindow.h
index fa413fe..0999ef3 100644
--- a/src/mainwindow.h
+++ b/src/mainwindow.h
@@ -235,6 +235,16 @@ public:
         sendMessageTagChange(messageIds, add, remove, description);
     }
 
+    /// Sends a thread-scoped tag change directly. The counterpart seam to
+    /// sendMessageTagChangeForTesting, for the same reason.
+    void sendThreadTagChangeForTesting(const QStringList &threadIds,
+                                       const QStringList &add,
+                                       const QStringList &remove,
+                                       const QString &description)
+    {
+        sendThreadTagChange(threadIds, add, remove, description);
+    }
+
     /// The ids the last tag change was sent for, and whether they were thread
     /// ids or message ids.
     ///
@@ -1191,6 +1201,29 @@ private:
     /// firing in a view it cannot judge.
     QString viewFilterTag() const;
 
+    /// Keeps the current view's membership in step with a write: drops a row
+    /// whose UNION has stopped matching, and refreshes when one starts
+    /// matching, which the model cannot express on its own. No-op unless the
+    /// view has a filter tag and the write touched it.
+    void syncViewMembership(const QStringList &threadIds,
+                            bool aRowIsMissing,
+                            const QStringList &added,
+                            const QStringList &removed);
+
+    /// Removes the rows that stopped matching the view while the user was
+    /// sitting on them. Called when the selection moves.
+    void flushDeferredEviction();
+
+    /// Threads that have stopped matching the view but were selected when it
+    /// happened, so evicting them would have moved the list under the user.
+    QStringList m_deferredEvictions;
+
+    /// True only while an AUTOMATIC write is being sent. A write the user
+    /// asked for evicts its row at once; one a timer made must not, or the row
+    /// leaves under an open context menu two seconds after it was selected.
+    /// The distinction is who initiated it, not what it does (item 177).
+    bool m_automaticWrite = false;
+
     void onMessagesMoved(const QMap<QString, QString> &originByMessageId,
                          const QString &destFolder);
 
diff --git a/src/threadlistmodel.cpp b/src/threadlistmodel.cpp
index fd4899b..0559694 100644
--- a/src/threadlistmodel.cpp
+++ b/src/threadlistmodel.cpp
@@ -206,6 +206,34 @@ void ThreadListModel::setTrashView(bool trash)
     }
 }
 
+void ThreadListModel::removeThreadsWithoutTag(const QStringList &threadIds,
+                                              const QString &tag)
+{
+    if (tag.isEmpty() || threadIds.isEmpty())
+        return;
+
+    // Backwards for the same reason the sweeping form below is: each removal
+    // renumbers everything after it.
+    for (int row = m_threads.size() - 1; row >= 0; --row) {
+        const ThreadNode &node = m_threads.at(row);
+        if (!threadIds.contains(node.summary.threadId))
+            continue;
+
+        // The SUMMARY, which is notmuch's union over the conversation, and
+        // never `first.tags`. A thread belongs to a view while ANY of its
+        // messages match it (item 177), so reading the message a 44-message
+        // card happens to draw must not evict the conversation while two of
+        // its replies are still unread. `first.tags` is right for what the
+        // card PAINTS and wrong for whether the row belongs here at all.
+        if (node.summary.tags.contains(tag))
+            continue;
+
+        beginRemoveRows({}, row, row);
+        m_threads.remove(row);
+        endRemoveRows();
+    }
+}
+
 void ThreadListModel::removeThreadsWithoutTag(const QString &tag)
 {
     if (tag.isEmpty() || m_threads.isEmpty())
@@ -995,6 +1023,15 @@ MessageNode ThreadListModel::messageAt(const QModelIndex &index) const
 QString ThreadListModel::threadIdForMessage(const QString &messageId) const
 {
     for (const ThreadNode &node : m_threads) {
+        // The ROOT first. A thread's first message is not among its children
+        // (item 109: setThreadMessages drops depth 0 because the root row
+        // stands for it), so a search over children alone answers "no thread"
+        // for every thread row's own message, which is the id an ordinary tag
+        // action resolves to since item 108.
+        if (node.first.messageId == messageId
+            || node.summary.firstMessageId == messageId) {
+            return node.summary.threadId;
+        }
         for (const MessageNode &child : node.children) {
             if (child.messageId == messageId)
                 return node.summary.threadId;
@@ -1003,6 +1040,23 @@ QString ThreadListModel::threadIdForMessage(const QString &messageId) const
     return {};
 }
 
+bool ThreadListModel::hasThread(const QString &threadId) const
+{
+    return std::any_of(m_threads.cbegin(), m_threads.cend(),
+                       [&threadId](const ThreadNode &node) {
+                           return node.summary.threadId == threadId;
+                       });
+}
+
+int ThreadListModel::threadCountFor(const QString &threadId) const
+{
+    for (const ThreadNode &node : m_threads) {
+        if (node.summary.threadId == threadId)
+            return node.summary.totalCount;
+    }
+    return 0;
+}
+
 MessageNode ThreadListModel::messageById(const QString &messageId) const
 {
     if (messageId.isEmpty())
diff --git a/src/threadlistmodel.h b/src/threadlistmodel.h
index a528c7e..93474ac 100644
--- a/src/threadlistmodel.h
+++ b/src/threadlistmodel.h
@@ -236,6 +236,21 @@ public:
     /// CURRENT VIEW requires, so a caller passes what the query filters on and
     /// nothing else.
     void removeThreadsWithoutTag(const QString &tag);
+
+    /// Drops only the NAMED threads, and only those whose UNION no longer
+    /// carries \p tag.
+    ///
+    /// Two differences from the sweeping form, both deliberate. It judges the
+    /// named rows alone, because the sweeping form is right after a move (the
+    /// query itself is what changed) and wrong after a tag write: a list can
+    /// legitimately hold rows that never matched, and one write must not evict
+    /// rows it did not touch. And it judges `summary.tags`, which is notmuch's
+    /// union over the conversation, never the displayed message's own tags: a
+    /// thread belongs to a view while ANY of its messages match it (item 177),
+    /// so reading one message of a 44-message thread leaves the conversation
+    /// in the Unread view and reading the last one takes it out.
+    void removeThreadsWithoutTag(const QStringList &threadIds,
+                                 const QString &tag);
     bool flatMode() const { return m_flatMode; }
 
     QModelIndex index(int row, int column,
@@ -326,6 +341,18 @@ public:
     /// message the user could select is always findable here.
     QString threadIdForMessage(const QString &messageId) const;
 
+    /// Whether a row for this thread is still in the list.
+    bool hasThread(const QString &threadId) const;
+
+    /// How many messages the thread holds, from its summary, or 0 when the
+    /// list has no row for it.
+    ///
+    /// The question a message-scoped write asks before judging membership: a
+    /// thread of one has a union that IS its message, so the write moved it
+    /// and the row can be judged, while a longer thread's union is untouched
+    /// and judging it would be judging a stale answer.
+    int threadCountFor(const QString &threadId) const;
+
     /// A loaded message row's node, found by id rather than by position.
     ///
     /// For callers that know WHICH message they mean and must not depend on it
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index 26ec485..2d15fe8 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -554,6 +554,10 @@ private slots:
     void replyToAConversationAnswersItsNewestMessage();
     void replyIsUntouchedOnAMessageRow();
 
+    // Item 177, task 6: membership is the union over the conversation.
+    void aConversationStaysWhileAnyMessageMatches();
+    void aConversationLeavesWhenItsUnionEmpties();
+
 private:
     /// Owns the throwaway lock table init() points every test at. A pointer
     /// rather than a value because it is rebuilt per test, and QTemporaryDir
@@ -5561,6 +5565,17 @@ void TestMainWindow::aMixedThreadIsMarkedReadAndTheSecondPressIsTheWayBack()
     auto *view = window.findChild<QTreeView *>();
     QVERIFY(view);
 
+    // A query with no filter tag, so viewFilterTag() answers empty and the
+    // membership sync (item 177, task 6) stays out of this. Under the default
+    // startup query the first press correctly EVICTS the row it just marked
+    // read, and the assertions below would then be reading a list that no
+    // longer holds the thread. What is under test here is the toggle's
+    // direction, which membership neither helps nor hinders.
+    auto *queryEdit =
+        window.findChild<QLineEdit *>(QStringLiteral("queryEdit"));
+    QVERIFY(queryEdit);
+    queryEdit->setText(QStringLiteral("thread:T1"));
+
     // MIXED: the union carries `unread` because some message is unread, while
     // others are not. A thread whose messages are all in one state answers
     // identically whichever way the direction is computed, so a uniform
@@ -15293,4 +15308,101 @@ void TestMainWindow::replyIsUntouchedOnAMessageRow()
                             .arg(replyAction->text())));
 }
 
+void TestMainWindow::aConversationStaysWhileAnyMessageMatches()
+{
+    // The reported case: a 44-message thread with two replies still unread.
+    // Reading one message must not evict the conversation.
+    const Config config;
+    MainWindow window(config);
+
+    auto *model = window.findChild<ThreadListModel *>();
+    auto *queryEdit =
+        window.findChild<QLineEdit *>(QStringLiteral("queryEdit"));
+    QVERIFY(model && queryEdit);
+
+    queryEdit->setText(
+        config.resolvedQuery(Config::builtinFilter(QStringLiteral("unread")),
+                             QString()));
+
+    ThreadSummary other = makeThread(QStringLiteral("t1"),
+                                     QStringList{ QStringLiteral("unread") });
+    other.totalCount = 1;
+    ThreadSummary big = makeThread(QStringLiteral("t2"),
+                                   QStringList{ QStringLiteral("unread") });
+    big.totalCount = 44;
+    model->appendBatch({ other, big });
+
+    // One message of the conversation is read, and specifically the one its
+    // ROW displays: naming any other id makes the model answer "no thread" and
+    // the test measures nothing, passing against an eviction that judges on
+    // the card's own tags. The union still says unread, so the row stays.
+    window.sendMessageTagChangeForTesting({ big.firstMessageId }, {},
+                                          { QStringLiteral("unread") },
+                                          QStringLiteral("Mark read"));
+
+    QCOMPARE(model->rowCount(QModelIndex()), 2);
+    QCOMPARE(model->threadAt(1).threadId, QStringLiteral("t2"));
+
+    // The same question put to the judgement ITSELF, which is where the union
+    // rule lives. The send path above reaches it only for a thread of one, so
+    // asserting through that path alone would let an eviction judging the
+    // card's own tags pass: the long thread is filtered out before the model
+    // is ever asked. Here the row is named directly, and the only thing
+    // keeping it is that its union still carries `unread` while the message
+    // its card draws no longer does.
+    QVERIFY2(!model->messageById(big.firstMessageId)
+                  .tags.contains(QStringLiteral("unread")),
+             "the fixture's card message is still unread, so this assertion "
+             "cannot tell the union apart from the card's own tags");
+    model->removeThreadsWithoutTag(QStringList{ QStringLiteral("t2") },
+                                   QStringLiteral("unread"));
+    QCOMPARE(model->rowCount(QModelIndex()), 2);
+
+    // And the counterpart, so the test is about the union and not about
+    // nothing ever being evicted: a one-message row's union IS its message, so
+    // reading it takes the row out at once.
+    window.sendMessageTagChangeForTesting({ other.firstMessageId }, {},
+                                          { QStringLiteral("unread") },
+                                          QStringLiteral("Mark read"));
+
+    QCOMPARE(model->rowCount(QModelIndex()), 1);
+    QCOMPARE(model->threadAt(0).threadId, QStringLiteral("t2"));
+}
+
+void TestMainWindow::aConversationLeavesWhenItsUnionEmpties()
+{
+    const Config config;
+    MainWindow window(config);
+
+    auto *model = window.findChild<ThreadListModel *>();
+    auto *view = window.findChild<QTreeView *>();
+    auto *queryEdit =
+        window.findChild<QLineEdit *>(QStringLiteral("queryEdit"));
+    QVERIFY(model && view && queryEdit);
+
+    queryEdit->setText(
+        config.resolvedQuery(Config::builtinFilter(QStringLiteral("unread")),
+                             QString()));
+
+    ThreadSummary keep = makeThread(QStringLiteral("t1"),
+                                    QStringList{ QStringLiteral("unread") });
+    keep.totalCount = 1;
+    ThreadSummary go = makeThread(QStringLiteral("t2"),
+                                  QStringList{ QStringLiteral("unread") });
+    go.totalCount = 4;
+    model->appendBatch({ keep, go });
+
+    // Selected elsewhere, so the never-evict-the-current-row rule is not what
+    // is being measured here.
+    selectThreadRow(view, 0);
+    QApplication::processEvents();
+
+    window.sendThreadTagChangeForTesting({ QStringLiteral("t2") }, {},
+                                         { QStringLiteral("unread") },
+                                         QStringLiteral("Mark thread read"));
+
+    QCOMPARE(model->rowCount(QModelIndex()), 1);
+    QCOMPARE(model->threadAt(0).threadId, QStringLiteral("t1"));
+}
+
 #include "test_mainwindow.moc"