From 4a4f82f7709ab6ee5a84ac0f3b191470b6424c36 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Fri, 14 Aug 2026 19:16:16 +0200 Subject: feat(pane): always render one message, never the conversation Selecting a thread root used to render the whole conversation, stubs plus the last messages expanded, but only until the thread had been expanded once. After that the identical click rendered a single message. The user reported the inconsistency and asked for the single-message behaviour throughout, and for the conversation view to go. The cause was a timing one, not a race. The root card stands for the thread's first message and onThreadSelected already preferred to load just that, but the model learned the id only when the replies arrived, so a fresh row fell through to a whole-thread render. ThreadSummary now carries firstMessageId from the query itself, so the id is known before any expansion and the fallback is unreachable. It is free: notmuch_thread_get_toplevel_messages reads the index, not the message files, and a walk with it is indistinguishable from one without over a 36,615-thread database. Contrast recipients, which reads every file and stays Sent-only. The Sent view keeps showing what the user sent rather than the thread's opening message, which is often someone else's. There is no matched-messages iterator in libnotmuch, only a count, so that branch walks oldest-first to the first NOTMUCH_MESSAGE_FLAG_MATCH and stops: 0.146s against a 0.143s baseline over 4,515 threads. onThreadLoaded merges into renderMessages, since onMessageLoaded was already delegating to it for the actual painting. It still takes a list because MessageView renders a list; collapsing that is a separate change to a class with its own tests. NotmuchWorker::loadThread is kept and documented as having no UI caller. It is a tested way to read a thread's messages with the match set resolved, used as a helper by the worker's own tests. Co-Authored-By: Claude Opus 5 --- tests/test_notmuchworker.cpp | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) (limited to 'tests/test_notmuchworker.cpp') diff --git a/tests/test_notmuchworker.cpp b/tests/test_notmuchworker.cpp index 9068ca3..f8dfe91 100644 --- a/tests/test_notmuchworker.cpp +++ b/tests/test_notmuchworker.cpp @@ -62,6 +62,7 @@ private slots: void loadMessageReturnsOnlyThatMessage(); void loadMessageOnAnUnknownIdReturnsNothing(); + void aQueryCarriesEachThreadsFirstMessageId(); void loadThreadTreeReportsReplyDepth(); void loadThreadTreeCarriesTheFactsARowNeeds(); @@ -241,6 +242,37 @@ void TestNotmuchWorker::loadMessageOnAnUnknownIdReturnsNothing() QCOMPARE(errors.count(), 0); } +void TestNotmuchWorker::aQueryCarriesEachThreadsFirstMessageId() +{ + // The root card IS the thread's first message, so selecting it must be + // able to load that message. Before this the id was known only after the + // thread had been EXPANDED, so a first click on an unexpanded root fell + // back to rendering the whole conversation, and the same click behaved + // differently once the thread had been opened. That inconsistency is what + // the user reported as item 66. + // + // Free to collect: measured against a real 36,615-thread database, a walk + // with this and a walk without are indistinguishable, because + // notmuch_thread_get_toplevel_messages reads the index rather than the + // message files. Contrast ThreadSummary::recipients, which reads every + // file and is Sent-only for that reason. + const QVector threads = runQuery(QStringLiteral("*")); + QVERIFY(!threads.isEmpty()); + + bool sawTheThread = false; + for (const ThreadSummary &t : threads) { + QVERIFY2(!t.firstMessageId.isEmpty(), + qPrintable(QStringLiteral("thread %1 carries no first message") + .arg(t.subject))); + if (t.subject == QStringLiteral("Release notes")) { + // a1 is the root, a2 its reply. The FIRST message, not the newest. + QCOMPARE(t.firstMessageId, QStringLiteral("a1@example.org")); + sawTheThread = true; + } + } + QVERIFY2(sawTheThread, "the two-message thread was not in the results"); +} + void TestNotmuchWorker::loadThreadTreeReportsReplyDepth() { // Thread A is a root plus one reply carrying In-Reply-To, which is what -- cgit v1.2.3