aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--AGENTS.md11
-rw-r--r--CHANGELOG.md5
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md42
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md1
-rw-r--r--src/mainwindow.cpp132
-rw-r--r--src/mainwindow.h6
-rw-r--r--src/threadlistmodel.cpp111
-rw-r--r--src/threadlistmodel.h20
-rw-r--r--tests/test_mainwindow.cpp37
-rw-r--r--tests/test_threadlistmodel.cpp49
-rw-r--r--translations/qtmaildir_it_IT.ts8
11 files changed, 357 insertions, 65 deletions
diff --git a/AGENTS.md b/AGENTS.md
index 6df8e18..ce0f380 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -872,6 +872,17 @@ message-scoped optimistic update would have left a FAILED message write showing
its optimistic state for good. Both scopes revert now. Undo needs nothing extra:
`MessageTagCommand` routes back through `sendMessageTagChange`.
+**A loop of optimistic writes goes through `batchModelWrites()`, and a
+model lookup by id is a hash, not a walk.** `MainWindow`'s `dataChanged`
+handler refreshes the action labels, and that refresh reads the whole
+selection. Run once per written row it made Select all + Archive on 10k rows
+cubic, about 10^12 string compares, and the window froze before the write ever
+reached the worker (item 211). `batchModelWrites()` holds the refresh and runs
+it once. `ThreadListModel::rowForThread()` / `rowForMessage()` answer from an
+index rebuilt lazily after any structural signal. A change to a node's ids
+that emits none (as `reconcile` and `setThreadMessages` do) must set
+`m_indexStale` by hand, which `idLookupsFollowEveryChangeToTheRows` guards.
+
**Testing this needs two things that are easy to miss.** Put the reply under the
SECOND thread, so the wrong answer is plausible rather than accidentally right,
and give the two threads OPPOSITE states, since two threads in the same state
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 21744de..736fca0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -39,6 +39,11 @@ point at which they are stable.
### Fixed
+- Acting on thousands of selected rows at once no longer freezes the window.
+ Archiving a 10k-row selection spent its time refreshing the menu labels once
+ per row, against the whole selection each time. It now does that once.
+ The status bar says an action is applying until the write has landed, then
+ that it is done, rather than letting the message fade on a timer.
- Searching on selected text containing a double quote no longer lets the rest
of the selection escape the query. Quotes are now doubled, which is notmuch's
own escape; the backslash form ended the quoted term early.
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md
index f5ae9f0..c83e41c 100644
--- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md
+++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md
@@ -10870,3 +10870,45 @@ not a backlog item picked up on a Tuesday.
to, since there are three and nothing in the files names a preference.
**Shares the write-during-sync question with 205.** Answer it once.
+
+## 211. Archiving thousands of selected rows freezes the window
+
+**Observed.** From the user's notes: a `tag:promo` search returned about 10k
+rows; Select all, then Archive, froze the window with one CPU pinned and no
+sign of progress.
+
+**Cause.** Measured on the live process, not read: three gdb samples of the
+UI thread, all identical, worker thread idle. Nothing had reached the worker,
+because the optimistic model update runs before the write is sent. The loop in
+`sendMessageTagChange` calls `applyMessageTagChange` once per message, which
+walked every thread to find it and emitted `dataChanged`. `MainWindow`'s
+`dataChanged` handler ran `refreshUnreadAction`, whose `selectionTagPresence`
+walked the whole selection calling `messageById`, itself another walk of
+every thread. N writes times N selected rows times N threads is about 10^12
+string compares at 10k. `QStringList::contains` in the membership code added
+smaller quadratic terms on top.
+
+**Approach.** `ThreadListModel` answers id lookups from a lazily rebuilt hash
+(`rowForThread`, `rowForMessage`). The model's own structural signals mark it
+stale, and so do the two in-place id changes (`reconcile` replacing a summary,
+`setThreadMessages` setting `first`). The first row wins, which matches the
+walks it replaces. `MainWindow::batchModelWrites()` holds the
+`dataChanged`-driven label refresh while a loop of optimistic writes runs and
+refreshes once afterwards. All nine loops go through it. Thread-id membership
+tests use a `QSet`. The whole path is now linear.
+
+**Constraints.** Correctness now depends on the index being invalidated.
+`idLookupsFollowEveryChangeToTheRows` asks a question whose answer moved after
+each kind of change, and it was mutation-checked against dropping the
+`reconcile` mark and the `rowsRemoved` hook. Both failed it.
+
+**Feedback.** First skipped as unneeded, then asked for after the hand test,
+where the archive worked (0 promo messages left in the inbox) and the user
+could not tell. The announcement used to vanish on a timer. It now reads
+"<action>, applying…" until the worker has run the write, then a transient
+"<action>, done". The trigger is a queued round trip through the worker's
+queue, not `tagsApplied()`, which stays silent for a write that moved nothing,
+while moves report through another signal. An error bumps the same ticket, so
+a late "done" cannot cover the error message. Tested by
+`aWriteSaysItIsApplyingUntilTheWorkerHasRunIt`, which fails when the reply is
+dropped.
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
index 48c31af..a3df5be 100644
--- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
+++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
@@ -148,6 +148,7 @@ taking that too literally.
| 208 | No way to add a new correspondent to the contacts | workflow | ? | open, 2026-09-18, **asked for by the user** while settling 204: an "add to contacts" button on mail from someone not in the store. Writes a vcard, so it lands on 205's writer rather than 204's reader. **Needs a brainstorm when picked up** |
| 209 | The calendar has no Week view | presentation | M | open, from 206's brainstorm. The occurrence list is the seam: a new painter over CalendarItem, no store change |
| 210 | A repeating event cannot be edited "this and following" | workflow | S-M | open, from 206's brainstorm. Splits a series into two files, divides EXDATEs and overrides, shifts moved RECURRENCE-IDs, converts COUNT; see the spec's Out of scope |
+| 211 | Archiving thousands of selected rows freezes the window | defect | S | **done** 2026-10-31, unreleased. Cubic optimistic-update path made linear: a lazily rebuilt id index in `ThreadListModel` and `MainWindow::batchModelWrites()` refreshing the action labels once per loop. A write now reads "applying…" until the worker has run it, then "done". Hand-tested on a 10158-message archive |
| 73 | This backlog is past four thousand lines | maintenance | S | **done** 2026-08-13; 5056 lines to 578, closed sections moved to `2026-08-03-post-0.1.0-usability-closed.md` |
| 74 | "Searching..." keeps claiming a query is running while rows are already arriving | feedback | XS | **done** 2026-08-15, unreleased. The status-bar half only: the bar now counts threads per batch. The cold-cache delay itself was measured in 2026-08-11 and is not fixable here |
| 75 | The tagging rules window forgets its size and its column widths | persistence | S | **done** 2026-08-13, shipped in 0.17.0. The window-kind question is left open, see the closed-items file |
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index e93898c..4377870 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -44,6 +44,7 @@
#include <QPlainTextEdit>
#include <QPushButton>
#include <QRegularExpression>
+#include <QScopedValueRollback>
#include <QSettings>
#include <QSplitter>
#include <QStandardPaths>
@@ -1011,6 +1012,11 @@ void MainWindow::buildUi()
// the model rather than on each of the six call sites that apply an
// optimistic update, so a new one cannot forget.
connect(m_model, &QAbstractItemModel::dataChanged, this, [this]() {
+ // Inside batchModelWrites() the refresh runs ONCE at the end. Per
+ // row it is O(selection) for every row written, which made a bulk
+ // archive cubic and froze the window on ten thousand rows (item 211).
+ if (m_batchingModelWrites)
+ return;
refreshUnreadAction();
refreshTrashActions();
});
@@ -4863,14 +4869,16 @@ void MainWindow::revertPendingTagChange()
// Put the rows back the way they were. Only the model is touched: the
// worker never applied the change, so there is nothing to undo there.
- for (const QString &threadId : m_pendingThreadIds) {
- m_model->applyTagChange(threadId, m_pendingChange.removed,
- m_pendingChange.added);
- }
- for (const QString &messageId : m_pendingChange.messageIds) {
- m_model->applyMessageTagChange(messageId, m_pendingChange.removed,
- m_pendingChange.added);
- }
+ batchModelWrites([this] {
+ for (const QString &threadId : m_pendingThreadIds) {
+ m_model->applyTagChange(threadId, m_pendingChange.removed,
+ m_pendingChange.added);
+ }
+ for (const QString &messageId : m_pendingChange.messageIds) {
+ m_model->applyMessageTagChange(messageId, m_pendingChange.removed,
+ m_pendingChange.added);
+ }
+ });
// The undo entry describes a change that never landed, so it would apply a
// spurious inverse if the user pressed undo.
@@ -4894,6 +4902,9 @@ void MainWindow::revertPendingTagChange()
void MainWindow::onWorkerError(const QString &message)
{
+ // The error is what the user needs to read, not a "done" still in flight.
+ ++m_announceTicket;
+
// Spec: the UI updates optimistically and reverts if the write fails.
// Without this the list would keep showing a tag the database never got.
//
@@ -4950,14 +4961,16 @@ void MainWindow::flushHeldEdits()
// rows do not visibly flicker; without this the change is applied
// twice and a later revert undoes only one of them, leaving a row
// showing a tag the database never got.
- for (const QString &threadId : edit.threadIds) {
- m_model->applyTagChange(threadId, edit.change.removed,
- edit.change.added);
- }
- for (const QString &messageId : edit.change.messageIds) {
- m_model->applyMessageTagChange(messageId, edit.change.removed,
- edit.change.added);
- }
+ batchModelWrites([&] {
+ for (const QString &threadId : edit.threadIds) {
+ m_model->applyTagChange(threadId, edit.change.removed,
+ edit.change.added);
+ }
+ for (const QString &messageId : edit.change.messageIds) {
+ m_model->applyMessageTagChange(messageId, edit.change.removed,
+ edit.change.added);
+ }
+ });
// By SCOPE. A held edit is one or the other, never both: a
// message-scoped edit carries no thread ids, so sending it through
@@ -5867,7 +5880,32 @@ void MainWindow::onExternalSyncStateChanged(SyncMonitor::State state)
void MainWindow::announceAction(const QString &text)
{
if (!aSyncHoldsTheWriteLock()) {
- showTransientStatus(text);
+ if (!m_worker) {
+ showTransientStatus(text);
+ return;
+ }
+
+ // Item 211. Held until the write has actually landed, then replaced
+ // by a transient "done": a bulk write can take a while, and a status
+ // that vanished on a timer left no way to tell whether it finished.
+ //
+ // A round trip through the worker's queue rather than a hook on
+ // tagsApplied(), which deliberately stays silent for a write that
+ // moved nothing, and a move reports through another signal anyway.
+ // The worker runs its queue in order, so this lambda runs after the
+ // write the caller just queued, and its reply is posted behind that
+ // write's confirmation. The destructor waits on the worker thread
+ // before `this` goes, so the reply cannot outlive the window.
+ m_statusTimer->stop();
+ m_statusLabel->setText(tr("%1, applying…").arg(text));
+ const quint64 ticket = ++m_announceTicket;
+ QMetaObject::invokeMethod(m_worker, [this, text, ticket]() {
+ QMetaObject::invokeMethod(this, [this, text, ticket]() {
+ // A newer announcement, or an error, owns the label now.
+ if (ticket == m_announceTicket)
+ showTransientStatus(tr("%1, done").arg(text));
+ }, Qt::QueuedConnection);
+ }, Qt::QueuedConnection);
return;
}
@@ -6511,6 +6549,16 @@ void MainWindow::editTagsOnSelection()
tagSelected(add, remove, tr("Edit tags"));
}
+void MainWindow::batchModelWrites(const std::function<void()> &writes)
+{
+ {
+ const QScopedValueRollback<bool> guard(m_batchingModelWrites, true);
+ writes();
+ }
+ refreshUnreadAction();
+ refreshTrashActions();
+}
+
void MainWindow::tagSelected(const QStringList &add, const QStringList &remove,
const QString &description)
{
@@ -6582,8 +6630,10 @@ void MainWindow::sendMessageTagChange(const QStringList &messageIds,
// this path had no optimistic update at all, and the cost was that Delete
// and Toggle unread on a reply moved the pending count and changed nothing
// the user could see. The reply's own row is where the feedback belongs.
- for (const QString &messageId : messageIds)
- m_model->applyMessageTagChange(messageId, add, remove);
+ batchModelWrites([&] {
+ for (const QString &messageId : messageIds)
+ m_model->applyMessageTagChange(messageId, add, remove);
+ });
// The strip shows the tags of the message ON DISPLAY, so it has to follow
// an edit to that message rather than waiting for the next selection. The
@@ -6624,6 +6674,7 @@ void MainWindow::sendMessageTagChange(const QStringList &messageIds,
// 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;
+ QSet<QString> seen; // a list's contains() made this quadratic (item 211)
bool aRowIsMissing = false;
for (const QString &messageId : messageIds) {
const QString threadId = m_model->threadIdForMessage(messageId);
@@ -6633,8 +6684,10 @@ void MainWindow::sendMessageTagChange(const QStringList &messageIds,
}
if (m_model->threadCountFor(threadId) > 1)
continue;
- if (!touchedThreads.contains(threadId))
+ if (!seen.contains(threadId)) {
+ seen.insert(threadId);
touchedThreads.append(threadId);
+ }
}
syncViewMembership(touchedThreads, aRowIsMissing, add, remove);
@@ -6889,8 +6942,10 @@ void MainWindow::trashThreads(const QStringList &threadIds)
// holding it back is what made a deleted thread sit unchanged until it was
// clicked. It also keeps the toggle's direction readable immediately, so a
// second press restores rather than deleting again.
- for (const QString &threadId : threadIds)
- m_model->applyTagChange(threadId, { QStringLiteral("deleted") }, {});
+ batchModelWrites([&] {
+ for (const QString &threadId : threadIds)
+ m_model->applyTagChange(threadId, { QStringLiteral("deleted") }, {});
+ });
m_pendingThreadScope = threadIds;
QMetaObject::invokeMethod(m_worker, "resolveThreadMessages",
@@ -6987,9 +7042,11 @@ void MainWindow::spamThreads(const QStringList &threadIds)
// synchronously before it: the same shape as trashThreads(), for the same
// two reasons. An unexpanded thread's reply ids and paths exist only in
// the database; and the card must not wait for that round trip.
- for (const QString &threadId : threadIds)
- m_model->applyTagChange(threadId, { QStringLiteral("spam") },
- { QStringLiteral("inbox") });
+ batchModelWrites([&] {
+ for (const QString &threadId : threadIds)
+ m_model->applyTagChange(threadId, { QStringLiteral("spam") },
+ { QStringLiteral("inbox") });
+ });
m_pendingThreadScope = threadIds;
QMetaObject::invokeMethod(m_worker, "resolveThreadMessages",
@@ -7121,8 +7178,10 @@ void MainWindow::untrashThreads(const QStringList &threadIds)
return;
// Repainted synchronously, as the delete direction is.
- for (const QString &threadId : threadIds)
- m_model->applyTagChange(threadId, {}, { QStringLiteral("deleted") });
+ batchModelWrites([&] {
+ for (const QString &threadId : threadIds)
+ m_model->applyTagChange(threadId, {}, { QStringLiteral("deleted") });
+ });
m_pendingThreadScope = threadIds;
QMetaObject::invokeMethod(
@@ -7177,8 +7236,10 @@ void MainWindow::notSpamThreads(const QStringList &threadIds)
// Repainted synchronously, as the mark-spam direction is: the card is what
// the user watches, and it must not wait for the worker round trip.
- for (const QString &threadId : threadIds)
- m_model->applyTagChange(threadId, {}, { QStringLiteral("spam") });
+ batchModelWrites([&] {
+ for (const QString &threadId : threadIds)
+ m_model->applyTagChange(threadId, {}, { QStringLiteral("spam") });
+ });
m_pendingThreadScope = threadIds;
QMetaObject::invokeMethod(
@@ -7818,8 +7879,11 @@ void MainWindow::sendMove(const QStringList &messageIds,
// while the latter deliberately leaves a multi-message thread's summary
// alone because one message's edit does not describe the conversation.
if (wholeThreadIds.isEmpty()) {
- for (const QString &messageId : messageIds)
- m_model->applyMessageTagChange(messageId, displayAdd, displayRemove);
+ batchModelWrites([&] {
+ for (const QString &messageId : messageIds)
+ m_model->applyMessageTagChange(messageId, displayAdd,
+ displayRemove);
+ });
}
// A row that no longer belongs in the view LEAVES it, rather than sitting
@@ -8136,8 +8200,10 @@ void MainWindow::sendThreadTagChange(const QStringList &threadIds,
{
// Optimistic: the rows change now, so a bulk archive of hundreds of threads
// feels instant. Recorded so onWorkerError() can put them back.
- for (const QString &threadId : threadIds)
- m_model->applyTagChange(threadId, add, remove);
+ batchModelWrites([&] {
+ for (const QString &threadId : threadIds)
+ m_model->applyTagChange(threadId, add, remove);
+ });
// Which accounts this touches, recorded HERE and not in onTagsApplied():
// TagChange carries message ids, while the account is a property of the
diff --git a/src/mainwindow.h b/src/mainwindow.h
index 87ed171..868964a 100644
--- a/src/mainwindow.h
+++ b/src/mainwindow.h
@@ -1179,6 +1179,10 @@ private:
/// something, the same rule refreshUnreadAction() applies to the label.
void refreshTrashActions();
+ /// Runs a loop of optimistic model writes with the dataChanged-driven
+ /// action refresh held, then refreshes once (item 211).
+ void batchModelWrites(const std::function<void()> &writes);
+
/// Whether every selected row's file already sits in its account's trash
/// folder. Empty selection answers false.
///
@@ -1209,6 +1213,8 @@ private:
/// for exit has finished. Stops closeEvent asking a second time, and is
/// what lets the deferred close through.
bool m_closeApproved = false;
+ bool m_batchingModelWrites = false; ///< see batchModelWrites()
+ quint64 m_announceTicket = 0; ///< see announceAction()
/// True while a sync started by the exit prompt is running. The window
/// stays open until it finishes: killing the process mid-sync is exactly
diff --git a/src/threadlistmodel.cpp b/src/threadlistmodel.cpp
index ad1359b..139d592 100644
--- a/src/threadlistmodel.cpp
+++ b/src/threadlistmodel.cpp
@@ -20,12 +20,11 @@
#include "composecontext.h"
-#include <QSet>
-
#include <QBrush>
#include <QFont>
#include <QGuiApplication>
#include <QPalette>
+#include <QSet>
#include <QFontDatabase>
#include <QFontMetrics>
@@ -131,6 +130,49 @@ QColor ThreadListModel::readColour()
ThreadListModel::ThreadListModel(QObject *parent)
: QAbstractItemModel(parent)
{
+ // Keyed on the model's own structural signals rather than on each call
+ // site that inserts or removes, so a new one cannot forget.
+ const auto stale = [this]() { m_indexStale = true; };
+ connect(this, &QAbstractItemModel::rowsInserted, this, stale);
+ connect(this, &QAbstractItemModel::rowsRemoved, this, stale);
+ connect(this, &QAbstractItemModel::rowsMoved, this, stale);
+ connect(this, &QAbstractItemModel::modelReset, this, stale);
+ connect(this, &QAbstractItemModel::layoutChanged, this, stale);
+}
+
+void ThreadListModel::rebuildIndex() const
+{
+ m_rowByThread.clear();
+ m_rowByMessage.clear();
+ // tryEmplace keeps the FIRST row for an id, matching the walks this
+ // replaces, which returned at the first hit.
+ for (int row = 0; row < m_threads.size(); ++row) {
+ const ThreadNode &node = m_threads.at(row);
+ m_rowByThread.tryEmplace(node.summary.threadId, row);
+ if (!node.summary.firstMessageId.isEmpty())
+ m_rowByMessage.tryEmplace(node.summary.firstMessageId, row);
+ if (!node.first.messageId.isEmpty())
+ m_rowByMessage.tryEmplace(node.first.messageId, row);
+ for (const MessageNode &child : node.children)
+ m_rowByMessage.tryEmplace(child.messageId, row);
+ }
+ m_indexStale = false;
+}
+
+int ThreadListModel::rowForThread(const QString &threadId) const
+{
+ if (m_indexStale)
+ rebuildIndex();
+ return m_rowByThread.value(threadId, -1);
+}
+
+int ThreadListModel::rowForMessage(const QString &messageId) const
+{
+ if (messageId.isEmpty())
+ return -1;
+ if (m_indexStale)
+ rebuildIndex();
+ return m_rowByMessage.value(messageId, -1);
}
QModelIndex ThreadListModel::index(int row, int column,
@@ -212,11 +254,15 @@ void ThreadListModel::removeThreadsWithoutTag(const QStringList &threadIds,
if (tag.isEmpty() || threadIds.isEmpty())
return;
+ // A set, because this runs over every row with every touched thread:
+ // a list made a bulk archive quadratic here too (item 211).
+ const QSet<QString> wanted(threadIds.cbegin(), threadIds.cend());
+
// 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))
+ if (!wanted.contains(node.summary.threadId))
continue;
// The SUMMARY, which is notmuch's union over the conversation, and
@@ -933,6 +979,7 @@ void ThreadListModel::reconcile(const QVector<ThreadSummary> &threads)
|| m_threads.at(row).summary.totalCount != summary.totalCount
|| m_threads.at(row).summary.matchedCount != summary.matchedCount) {
m_threads[row].summary = summary;
+ m_indexStale = true; // firstMessageId may have moved
emit dataChanged(index(row, 0), index(row, 0));
}
}
@@ -997,6 +1044,7 @@ void ThreadListModel::setThreadMessages(const QString &threadId,
// Kept so the root card can render its own message. It is the card the
// user clicks to read the thread's opening message.
m_threads[row].first = nodes.isEmpty() ? MessageNode() : nodes.first();
+ m_indexStale = true; // a thread of one inserts no rows to say so
if (!children.isEmpty()) {
beginInsertRows(parent, 0, children.size() - 1);
@@ -1057,7 +1105,11 @@ MessageNode ThreadListModel::messageAt(const QModelIndex &index) const
QString ThreadListModel::threadIdForMessage(const QString &messageId) const
{
- for (const ThreadNode &node : m_threads) {
+ const int row = rowForMessage(messageId);
+ if (row < 0)
+ return {};
+ {
+ const ThreadNode &node = m_threads.at(row);
// 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"
@@ -1077,27 +1129,22 @@ QString ThreadListModel::threadIdForMessage(const QString &messageId) const
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;
- });
+ return rowForThread(threadId) >= 0;
}
int ThreadListModel::threadCountFor(const QString &threadId) const
{
- for (const ThreadNode &node : m_threads) {
- if (node.summary.threadId == threadId)
- return node.summary.totalCount;
- }
- return 0;
+ const int row = rowForThread(threadId);
+ return row < 0 ? 0 : m_threads.at(row).summary.totalCount;
}
MessageNode ThreadListModel::messageById(const QString &messageId) const
{
- if (messageId.isEmpty())
+ const int row = rowForMessage(messageId);
+ if (row < 0)
return {};
-
- for (const ThreadNode &node : m_threads) {
+ {
+ const ThreadNode &node = m_threads.at(row);
// The root's own message first, and it is not among the children:
// setThreadMessages drops depth 0 because the root row stands for it.
// Searching only the children returned a default-constructed node for
@@ -1199,18 +1246,15 @@ ThreadSummary ThreadListModel::threadFor(const QModelIndex &index) const
QStringList ThreadListModel::accountKeysForThread(const QString &threadId) const
{
QStringList keys;
- for (const ThreadNode &node : m_threads) {
- const ThreadSummary &thread = node.summary;
- if (thread.threadId != threadId)
+ const int row = rowForThread(threadId);
+ if (row < 0)
+ return keys;
+ for (const QString &tag : m_threads.at(row).summary.tags) {
+ if (!TagColors::isAccountTag(tag))
continue;
- for (const QString &tag : thread.tags) {
- if (!TagColors::isAccountTag(tag))
- continue;
- const QString key = TagColors::accountKeyForTag(tag);
- if (!key.isEmpty() && !keys.contains(key))
- keys.append(key);
- }
- break;
+ const QString key = TagColors::accountKeyForTag(tag);
+ if (!key.isEmpty() && !keys.contains(key))
+ keys.append(key);
}
return keys;
}
@@ -1219,10 +1263,10 @@ void ThreadListModel::applyTagChange(const QString &threadId,
const QStringList &added,
const QStringList &removed)
{
- for (int row = 0; row < m_threads.size(); ++row) {
- if (m_threads.at(row).summary.threadId != threadId)
- continue;
-
+ const int row = rowForThread(threadId);
+ if (row < 0)
+ return;
+ {
QStringList &tags = m_threads[row].summary.tags;
for (const QString &tag : removed)
tags.removeAll(tag);
@@ -1285,7 +1329,10 @@ void ThreadListModel::applyMessageTagChange(const QString &messageId,
}
};
- for (int row = 0; row < m_threads.size(); ++row) {
+ const int row = rowForMessage(messageId);
+ if (row < 0)
+ return;
+ {
ThreadNode &node = m_threads[row];
const QModelIndex threadIndex = index(row, 0, QModelIndex());
diff --git a/src/threadlistmodel.h b/src/threadlistmodel.h
index bed099d..fc7f65a 100644
--- a/src/threadlistmodel.h
+++ b/src/threadlistmodel.h
@@ -20,6 +20,7 @@
#include <QAbstractItemModel>
#include <QColor>
+#include <QHash>
#include <QVector>
#include "tagcolors.h"
@@ -450,7 +451,26 @@ private:
/// than claiming the union as one message's.
static ThreadNode nodeFor(const ThreadSummary &summary);
+ /// The top-level row holding a thread or a message, or -1.
+ ///
+ /// Answered from a hash rather than a walk, because a bulk action asks
+ /// once per message and a walk per message made archiving ten thousand
+ /// rows quadratic in the lookups alone (item 211). The FIRST row in list
+ /// order wins, which is the row every linear walk here used to stop at,
+ /// so the two are interchangeable; a flat view can hold one thread on
+ /// several rows and still answers with the first.
+ int rowForThread(const QString &threadId) const;
+ int rowForMessage(const QString &messageId) const;
+ void rebuildIndex() const;
+
QVector<ThreadNode> m_threads;
+ // Rebuilt lazily on the next lookup after anything renumbers rows or
+ // changes which ids a node carries. Every structural change signals, and
+ // the two in-place id changes (reconcile, setThreadMessages) mark it by
+ // hand.
+ mutable QHash<QString, int> m_rowByThread;
+ mutable QHash<QString, int> m_rowByMessage;
+ mutable bool m_indexStale = true;
const TagColors *m_tagColors = nullptr;
QString m_dateFormat;
QStringList m_forwardPrefixes;
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index 2b7008d..71795fa 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -295,6 +295,7 @@ private slots:
void narrowingAnEmptyQueryBarIsAPlainSearch();
void aMalformedAccountIsReportedWithoutBlockingTheConstructor();
void aWorkerBackedWindowReturnsRealThreads();
+ void aWriteSaysItIsApplyingUntilTheWorkerHasRunIt();
void aPurgeTakesTheRowsOutOfTheViewWithoutARefresh();
void theDashboardFollowsAWriteToTheConversationItShows();
void anEditHeldByASyncSaysSoInsteadOfClaimingItLanded();
@@ -9892,6 +9893,42 @@ void TestMainWindow::aWorkerBackedWindowReturnsRealThreads()
QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000);
}
+void TestMainWindow::aWriteSaysItIsApplyingUntilTheWorkerHasRunIt()
+{
+ // Item 211, from the user: after archiving ten thousand rows, "how do I
+ // know if it worked? The UI doesn't give me a feedback". The announcement
+ // used to vanish on a timer whether or not the write had landed.
+ WorkerBackedWindow backed;
+ QVERIFY(backed.fixture().addMessage(
+ QStringLiteral("inbox"), QStringLiteral("root@example.org"),
+ QStringLiteral("A subject"), QStringLiteral("sender@example.org"),
+ // Friday, verified with `date -d 2026-08-14 +%A`.
+ QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"),
+ QStringLiteral("Body text.")));
+ QVERIFY2(backed.build(), qPrintable(backed.error()));
+
+ MainWindow window(backed.config());
+ auto *queryEdit = window.findChild<QLineEdit *>(QStringLiteral("queryEdit"));
+ auto *model = window.findChild<ThreadListModel *>();
+ auto *view = window.findChild<ThreadListView *>();
+ auto *status = window.findChild<QLabel *>(QStringLiteral("statusMessage"));
+ QVERIFY(queryEdit && model && view && status);
+
+ queryEdit->setText(QStringLiteral("tag:inbox"));
+ queryEdit->returnPressed();
+ QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000);
+ view->setCurrentIndex(model->index(0, 0, QModelIndex()));
+
+ window.findChild<QAction *>(QStringLiteral("archive"))->trigger();
+
+ // Synchronous: the worker's reply is posted to this thread and cannot be
+ // delivered before the event loop runs, so this cannot race.
+ QVERIFY2(status->text().contains(QStringLiteral("applying")),
+ qPrintable(status->text()));
+ QTRY_VERIFY2_WITH_TIMEOUT(status->text().contains(QStringLiteral("done")),
+ qPrintable(status->text()), 15000);
+}
+
/// Item 181, from the user: "the thread dashboard doesn't update live with the
/// modifications applied to the list pane. If I mark the thread as read, the
/// dash still reports N unread".
diff --git a/tests/test_threadlistmodel.cpp b/tests/test_threadlistmodel.cpp
index 28d3a8b..76ac0e8 100644
--- a/tests/test_threadlistmodel.cpp
+++ b/tests/test_threadlistmodel.cpp
@@ -109,6 +109,7 @@ private slots:
void aConversationListsItsFirstMessageAsAChild();
void aMessageRowDoesNotListItselfBeneathItself();
void aMixedSelectionCarriesBothScopes();
+ void idLookupsFollowEveryChangeToTheRows();
};
static ThreadSummary makeThread(const QString &id, const QString &subject)
@@ -2355,5 +2356,53 @@ void TestThreadListModel::aMixedSelectionCarriesBothScopes()
QCOMPARE(scope.threadIds, QStringList{ QStringLiteral("t2") });
}
+void TestThreadListModel::idLookupsFollowEveryChangeToTheRows()
+{
+ // Item 211. Lookups by id go through a lazily rebuilt hash rather than a
+ // walk, so every way the rows can change has to invalidate it. Each step
+ // asks a question whose answer moved, so a stale index answers wrongly.
+ const QString a = QStringLiteral("a@example.org");
+ const QString c = QStringLiteral("c@example.org");
+ const QString d = QStringLiteral("d@example.org");
+ const QString e = QStringLiteral("e@example.org");
+ const QString t1 = QStringLiteral("t1");
+ const QString t2 = QStringLiteral("t2");
+
+ ThreadListModel model;
+ ThreadSummary one = makeThread(t1, QStringLiteral("one"));
+ one.firstMessageId = a;
+ ThreadSummary two = makeThread(t2, QStringLiteral("two"));
+ two.firstMessageId = c;
+ model.appendBatch({ one, two });
+ QCOMPARE(model.threadIdForMessage(a), t1); // builds the index
+
+ // Expansion adds ids the index has never seen.
+ MessageNode first = makeNode(c, 0);
+ first.threadId = t2;
+ MessageNode reply = makeNode(d, 1);
+ reply.threadId = t2;
+ model.setThreadMessages(t2, { first, reply });
+ QCOMPARE(model.threadIdForMessage(d), t2);
+
+ // A removal renumbers: t2 moves from row 1 to row 0.
+ model.applyTagChange(t1, {}, { QStringLiteral("inbox") });
+ model.removeThreadsWithoutTag({ t1 }, QStringLiteral("inbox"));
+ QVERIFY(!model.hasThread(t1));
+ QVERIFY(model.threadIdForMessage(a).isEmpty());
+ QCOMPARE(model.threadCountFor(t2), 2);
+ model.applyMessageTagChange(d, { QStringLiteral("x") }, {});
+ QVERIFY(model.messageById(d).tags.contains(QStringLiteral("x")));
+
+ // reconcile() replaces a summary IN PLACE, with no structural signal.
+ two.firstMessageId = e;
+ two.tags = QStringList{ QStringLiteral("inbox") };
+ model.reconcile({ two });
+ QCOMPARE(model.threadIdForMessage(e), t2);
+
+ model.clear();
+ QVERIFY(!model.hasThread(t2));
+ QVERIFY(model.messageById(d).messageId.isEmpty());
+}
+
QTEST_MAIN(TestThreadListModel)
#include "test_threadlistmodel.moc"
diff --git a/translations/qtmaildir_it_IT.ts b/translations/qtmaildir_it_IT.ts
index 51bf793..7a57598 100644
--- a/translations/qtmaildir_it_IT.ts
+++ b/translations/qtmaildir_it_IT.ts
@@ -1981,6 +1981,14 @@ protocollo di rete: il recupero e l&apos;invio sono comandi esterni.
<translation>Sincronizzazione in corso in background...</translation>
</message>
<message>
+ <source>%1, applying…</source>
+ <translation>%1, in corso…</translation>
+ </message>
+ <message>
+ <source>%1, done</source>
+ <translation>%1, fatto</translation>
+ </message>
+ <message>
<source>%1, waiting for the running sync to finish</source>
<translation>%1, in attesa che termini la sincronizzazione in corso</translation>
</message>