diff options
| author | Danilo M. <danix@danix.xyz> | 2026-09-06 15:07:35 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-09-06 15:07:35 +0200 |
| commit | 7074a4c57343777fe08f5cdd3174563a46007484 (patch) | |
| tree | e70fef4825cb43042c6f855c9465a4adbde771dc /docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | |
| parent | 5843f11f6809762aab52d37fbe54dd9b1792d00c (diff) | |
| download | qtmaildir-7074a4c57343777fe08f5cdd3174563a46007484.tar.gz qtmaildir-7074a4c57343777fe08f5cdd3174563a46007484.zip | |
fix: give the Sent and Drafts views one row per message, in date order
Both views are lists of the user's own messages and are flat, but
walkThreads() emitted one ThreadSummary per THREAD and then picked a
single matched message to stand for it, breaking at the first one the
oldest-first walk reached. A conversation replied to twice therefore
produced one row: dated by the thread, opening the OLDER of the two
messages, with the newer one reachable nowhere in the view. Reported
against real mail, where a message sent at 12:42 was missing while the
row above it, dated 12:42, opened a message from three weeks earlier.
The same wrongly chosen message supplied firstMessagePath, so Delete or
Archive on such a row would have moved a file the user was not looking
at, silently, and mbsync would have carried it to the server. That half
was never visible.
The Sent branch now emits one summary per matched message, each carrying
its own id, tags, sender, path, date and subject. withRecipients still
selects the branch, so Sent and Drafts both get this and no second flag
can disagree with the flat-mode flag.
Ordering was a second defect under the same item, found by hand once the
rows appeared: notmuch_query_set_sort is a THREAD sort, so every row of a
thread inherits that thread's single position and an older reply drew
above a newer one. Sorting each thread's rows in place is not enough
either, since a message from another thread dated between them still
cannot land between them. Flat rows are collected and sorted as one list
before emitting.
ThreadListModel::rowKeyFor() is the second consequence and would have
broken quietly: two rows now share a threadId, and reconcile() keyed its
QHash on exactly that, so a sync would have dropped one of them by a
different route. It answers what makes a row unique, the message id in
flat mode and the thread id otherwise.
Tests cover both halves over new fixture threads F and G. The
cross-thread ordering assertion passed for the wrong reason at first,
because the existing fixture threads happen not to interleave; thread G
exists to break that and failed the moment it was added.
oldestFirstReversesTheOrder is corrected rather than satisfied:
OLDEST_FIRST orders threads by their oldest message while NEWEST_FIRST
orders by their newest, so the two lists mirror each other only while no
thread's date span contains another's, which this fixture is the first to
violate.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jq9gXquUo9W4KXDagJXMmn
Diffstat (limited to 'docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 134 |
1 files changed, 134 insertions, 0 deletions
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 26f90fa..af1de0c 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 @@ -9807,3 +9807,137 @@ where it is unless the user overrules the reasoning above. have their own. **Closed 2026-08-29**, unreleased. Star and Archive added to the ordinary branch; Archive left the main toolbar; `mark_all_read` stays there by the user's decision, and the assertion that it does is now in the suite. + +--- + +## 191. The Sent view collapses two messages you sent in one conversation into one row + +**Observed (user, 2026-09-06, by hand against real mail).** The Sent view for +one account showed a row dated 12:42 today whose subject was right, but whose +message pane rendered a message from three weeks earlier. The message actually +sent at 12:42 appeared nowhere in the view. Thunderbird, pointed at the same +IMAP folder, showed both. The user's own statement of what the view owes them +settled the fix: "the sent view is for my sent messages, if I sent 2 messages in +a thread, I want to see both, not threaded (as the view is not threaded), +correctly tagged and dated." + +**Two wrong diagnoses came first and are worth recording**, because both were +plausible and both were contradicted by measurement. The first read the `draft` +chip on the row and blamed item 165; the chip is real and unrelated. The second +read the row's date, found the message in `Sent/`, and concluded the row was +correct and the user was comparing a thread count against a message count; the +counts do differ for that reason, but that was not what they were reporting. The +user's correction, that the pane opened a different message from the one the row +claimed, is what located the defect. **A row's date and the message its pane +opens are two separate reads, and a defect can sit precisely in the gap.** + +**Cause, verified in the code and against the real index.** +`NotmuchWorker::walkThreads()` (`notmuchworker.cpp`) is built on +`notmuch_query_search_threads`, so it emitted exactly one `ThreadSummary` per +thread. The Sent branch then chose ONE message to stand for that thread: + +``` +if (matched) { summary.firstMessageId = ...; break; } +``` + +`notmuch_thread_get_messages` walks oldest-first, so a thread the user had +replied to twice took their OLDEST reply and stopped. The row's `date` and +`subject` came from the thread (`notmuch_thread_get_newest_date`, +`notmuch_thread_get_subject`) while `firstMessageId` named that older message, +which is why the two disagreed on screen. + +Measured on the developer's database: the conversation reported +`[2/9]` — two matched messages of nine — and `notmuch search --output=messages +--sort=oldest-first` over the sent folder returned the 17 Aug message before the +6 Sep one. The view is already flat (`setFlatMode(m_sentView)`, +`mainwindow.cpp`, and `Config::generatorIsFlat` marks `sent` and `drafts`), so +the model was right about being unthreaded and only the worker was wrong. + +**This was also a data-safety defect, which the visible symptom hid.** +`firstMessagePath`, `firstMessageTags` and `firstMessageSender` were all read +from the same wrongly chosen message. `moveMessages` composes its destination +from that path, so Delete or Archive on such a row would have moved the OLDER +message's file, silently, and mbsync would have carried it to the server. The +row the user was looking at named a file they were not looking at. + +**Fix.** The Sent branch emits one `ThreadSummary` per matched message rather +than one per thread: it no longer breaks, and for each matched message it copies +the thread-wide summary and overrides the row's identity — +`firstMessageId`, `firstMessageTags`, `firstMessageSender`, `firstMessagePath`, +and now `date` and `subject`, read from the message with +`notmuch_message_get_date` and `notmuch_message_get_header`. Batching and the +`total` counter move inside that loop, and the thread iteration `continue`s +past the single-summary append below. + +`withRecipients` continues to select the branch, so Sent and Drafts both get +this and no new flag can disagree with the flat-mode flag: the two questions are +answered by one value, which is why the original branch keyed on it. + +**One consequence needed a second change, and it is the part that would have +broken quietly.** Two rows now share a `threadId`, and +`ThreadListModel::reconcile()` keyed its `QHash<QString,int> present` on exactly +that. Two rows mapping to one key means the second looks like a thread that has +vanished, so a sync would have dropped one of the user's sent messages from the +view again, by a different route. `ThreadListModel::rowKeyFor()` is the single +answer to "what makes a row unique": the message id in flat mode, the thread id +otherwise, falling back to the thread id when a flat row carries no message id. +All four keyed sites in `reconcile()` use it. + +**Verification.** Two tests in `test_notmuchworker.cpp`, over a new fixture +thread F (`f1` sent, `f2` received, `f3` sent) which is the shape the defect +needs and which no existing fixture had: + +- `aSentQueryEmitsOneRowPerMatchedMessage` asserts both ids are present, that + the two rows share a thread id (the property that made the reconcile change + necessary), and that the SAME messages under a non-Sent query still fold into + one row, so this is the flat branch's contract and not a change of meaning + for threaded views. +- `aSentRowCarriesItsOwnMessagesDateAndSubject` asserts each row's date and + subject are its own, which is the half the user saw first. + +`aSentQueryCarriesTheMatchedMessageNotTheThreadsFirst` was RETARGETED rather +than retired: its assertion still holds, but it filtered rows by the thread's +subject, and a flat row is now titled by its own message, so it matched nothing. +It filters on the message id instead and asserts the subject. + +Eight other failures were the new fixture's arithmetic, not the change: three +messages and one thread added, so hardcoded totals moved 5→6 threads and 6→9 +messages. + +**The ordering was a second defect under the same item, found by hand after +the first fix shipped to the user's screen.** With both rows present, the 17 +August one drew ABOVE the 6 September one. The cause is that +`notmuch_query_set_sort` is a THREAD sort: it orders the threads the walk +visits and says nothing about the messages inside one, so every row of a thread +inherits that thread's single position. Sorting each thread's own rows in place +is NOT enough either, and the fixture proved it: a message from another thread +dated between two of a thread's replies still cannot land between them. A flat +view is a list of messages, so `walkThreads()` collects flat rows in `flatRows` +and sorts the whole result once, by each row's own date, before emitting. +`std::stable_sort`, so rows sharing a timestamp keep the walk's order rather +than swapping between identical queries. + +`sentRowsAreOrderedByTheirOwnDate` covers it, in both sort directions and +across threads. **The cross-thread half of it passed for the wrong reason at +first**: fixture threads D, E and F happen not to interleave, so per-thread +sorting satisfied a whole-view order check. Fixture thread G exists purely to +break that, dated between thread F's two messages, and the assertion failed +the moment it was added. A mutation check confirms the within-thread half fails +without the sort. + +**One existing test had to be corrected rather than satisfied, and the +correction is a fact about notmuch worth keeping.** +`oldestFirstReversesTheOrder` asserted the two sort directions produce exactly +reversed lists. They do not: `NOTMUCH_SORT_OLDEST_FIRST` orders threads by +their OLDEST message while `NEWEST_FIRST` orders them by their NEWEST, so the +lists mirror each other only while no thread's date span contains another's. +Thread F starts before thread G and ends after it, which is the first fixture +data to violate that. The test keeps the assertion that does hold +(`oldest.first()` is `newest.last()`) and replaces the mirror with a +monotonicity check. It passed on master and fails here for a reason that is not +a regression: the fixture finally contains the shape that distinguishes the two +sorts. + +**Still open and separate:** the four autosave revisions of that reply sitting +in `Drafts/` with distinct Message-IDs, which is item 165 and is what puts a +`draft` chip on the conversation. |
