aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-16 21:58:18 +0200
committerDanilo M. <danix@danix.xyz>2026-08-16 21:58:18 +0200
commit019117aa8e52ce39cab58f77b57a9a67f510696f (patch)
treed96c71e0e43a777dcdbce05cb7f0e58f135139b1
parentb405e3288bf625bd478204a065572e41a93fb4c3 (diff)
downloadqtmaildir-019117aa8e52ce39cab58f77b57a9a67f510696f.tar.gz
qtmaildir-019117aa8e52ce39cab58f77b57a9a67f510696f.zip
feat(ui): act on the message a row displays, not its whole thread
A thread's card has rendered one message since item 66, but every tag action still acted on the entire conversation. Delete, Archive, Important, Mark spam and Toggle unread now act on the message the card shows; the whole-thread versions move to a "Whole thread" submenu in the Message menu and the thread list's context menu, on Ctrl+Alt+<key>. Closes items 87, 88, 105, 106, 107, 108, 109, 110 and 111. The defects fixed along the way, several found by reading rather than by report: - threadAt(current.row()) answered about the wrong thread for a reply row, because a tree numbers rows per parent. The audit found four live sites, not the one reported: Delete and Toggle unread each chose their DIRECTION from an unrelated thread, and the tag dialog counted the wrong thread's tags. threadFor(index) replaces them. - A message-scoped write made no optimistic model update and no reply row carried a doomed cue, so acting on a reply moved the pending-edit count and changed nothing on screen. - Both toggles read the state of a reply's THREAD, which a message-scoped write never changes, so they were one-way: the second press re-sent a tag the message already had. - flushHeldEdits() re-sent only thread-scoped edits, so a tag change made on one message during a sync was applied to the row, counted as unsynced, and then dropped without ever being written. - applyTagChange() updated a thread's summary but not its loaded replies, leaving an expanded thread's rows describing a state the database no longer held. - A thread's first message is not among its children, so both message-scoped lookups missed it: acting on a root card repainted nothing and emptied the message pane's chip row. - ThreadSummary::tags is notmuch's union over the thread, so a card standing for one message drew tags belonging to its siblings. The worker now reads that message's own tags in the walk that already finds its id, so the split is known before a row is ever opened. The card shows both tiers: its own message's tags at full size, the rest of the conversation's smaller and muted, so nothing appears to vanish when a row is selected. Auto mark-read is message-scoped as a result, and now arms for a reply, which it never did. With maildir.synchronize_flags on, the old thread-wide write reached the server for mail that had never been displayed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
-rw-r--r--CHANGELOG.md101
-rw-r--r--CLAUDE.md195
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md694
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md417
-rw-r--r--src/carddelegate.cpp85
-rw-r--r--src/carddelegate.h18
-rw-r--r--src/cardlayout.cpp26
-rw-r--r--src/cardlayout.h13
-rw-r--r--src/keymap.cpp29
-rw-r--r--src/mainwindow.cpp460
-rw-r--r--src/mainwindow.h101
-rw-r--r--src/notmuchworker.cpp6
-rw-r--r--src/tagchip.cpp14
-rw-r--r--src/tagchip.h13
-rw-r--r--src/threadlistmodel.cpp383
-rw-r--r--src/threadlistmodel.h120
-rw-r--r--src/types.h23
-rw-r--r--tests/test_carddelegate.cpp128
-rw-r--r--tests/test_mainwindow.cpp1001
-rw-r--r--tests/test_notmuchworker.cpp50
-rw-r--r--tests/test_threadlistmodel.cpp613
21 files changed, 4255 insertions, 235 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7235340..9ca51f1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,107 @@ point at which they are stable.
## [Unreleased]
+### Upgrading
+
+**Acting on a thread row now acts on the message it displays, not on the whole
+conversation.** Selecting a thread's card has shown one message since 0.22.0,
+but Delete, Archive, Important, Mark spam and Toggle unread still acted on every
+message in the thread. They now act on the message you are looking at.
+
+The whole-thread versions are still there, under **Whole thread** in the Message
+menu and in the thread list's right-click menu, with new bindings a modifier out
+from their old ones:
+
+| Action | Message | Whole thread |
+|---|---|---|
+| Archive | `Ctrl+E` | `Ctrl+Alt+E` |
+| Delete | `Ctrl+D` | `Ctrl+Alt+D` |
+| Mark spam | `Ctrl+Shift+S` | `Ctrl+Alt+S` |
+| Toggle unread | `Ctrl+U` | `Ctrl+Alt+U` |
+| Important | `Ctrl+I` | `Ctrl+Alt+I` |
+
+Existing `[keys]` entries keep working and keep their meaning: `delete` is still
+`delete`, now scoped to one message. The thread actions are separate names
+(`delete_thread`, `archive_thread`, `spam_thread`, `toggle_unread_thread`,
+`flag_thread`) and can be rebound like any other. "Mark all read" is unchanged:
+it never used the selection.
+
+### Changed
+
+- **Reading a message no longer marks its replies read.** The two-second
+ automatic mark-read applied to the whole thread, so opening a conversation
+ marked messages read that had never been shown. Because
+ `maildir.synchronize_flags` is on, that reached the server and could not be
+ undone from here. It now marks only the message on display, and it applies to
+ a reply as well, which previously was never marked read at all.
+- An unread reply is bold as well as undimmed, which is the pair of cues an
+ unread thread has carried since 0.11.0. Replies were deliberately plain
+ before, on the grounds that the thread row above already flags the
+ conversation; once a thread is expanded, that row cannot say which of its
+ messages are unread, and dimming alone was too quiet to notice. Replies stay
+ a size smaller than their thread, so the two kinds of row still read apart.
+
+### Fixed
+
+- Acting on a reply inside an expanded thread could read a different thread's
+ state, because a reply's position is counted within its own thread and was
+ being used as a position in the whole list. The first reply of any thread
+ therefore answered as the first thread in the list. Three actions chose what
+ to do from that wrong answer: **Delete** could undelete a thread that was
+ never deleted, **Toggle unread** could go the wrong way, and **Edit tags**
+ offered to remove tags the selected message did not carry, while showing the
+ ones it did as unset. Every action that acts on a selection now resolves the
+ thread through the row itself.
+- Acting on a reply gave no visual feedback at all: Delete and Toggle unread
+ moved the unsynced-changes count and left the row looking exactly as it did
+ before. A change scoped to one message now repaints that message's own row.
+ A deleted or spam reply is filled and struck through, as a thread in the same
+ state already was. The thread's own card deliberately does not change, since
+ one deleted reply does not delete the conversation.
+- Marking an expanded thread read left its replies looking unread. The write
+ reached every message, but the list only updated the thread's own card, so
+ the replies stayed bold and undimmed until the next query corrected them.
+ They now follow a thread-wide change as the card does.
+- Acting on a thread's own card did not repaint it, and emptied the message
+ pane's chip row. A message-scoped write was only ever applied to a thread's
+ loaded replies, and a thread's first message is not one of them: the card
+ stands for it. So Delete or Toggle unread on a root card changed nothing on
+ screen, and the pane's chips were replaced with the empty tag list the lookup
+ returned. Both now find a root card's own message.
+- **A thread's card showed tags belonging to other messages in the thread**, and
+ so did the message pane. notmuch reports a thread's tags as the union over
+ its messages, so a four-message thread whose third message is signed read as
+ signed everywhere, including on a card that stands for the first message and
+ in a pane showing only that message. Opening a message now records what it
+ really carries, and the card and the pane both use it.
+
+ A card still shows the whole conversation's tags, since a card sits above a
+ thread: its own message's tags come first at full size, and the ones only its
+ siblings carry follow, smaller and muted. The split arrives with the query,
+ so a row reads correctly before it has ever been selected.
+
+ This is also what was stopping a card from repainting: with no per-message
+ tags for a thread's first message, marking it read or deleting it had nothing
+ to change, so the row stayed bold or unstruck while the write went through.
+- The message pane's tag chips did not follow an edit to the message on
+ display. Tagging a reply repainted its row in the list and left the pane
+ describing the message as it was, until you selected another message and came
+ back. The pane now follows a message-scoped edit, as it already did a
+ thread-scoped one.
+- **A tag change made on a single message during a sync was silently lost.**
+ Edits made while a sync holds notmuch's write lock are held and sent when it
+ finishes; that queue only ever re-sent whole-thread changes, so a
+ message-scoped one was applied to the row, counted in the unsynced-changes
+ indicator, and then dropped without ever being written. The change appeared
+ to have been made and reported itself as pending right up until it vanished.
+- Delete and Toggle unread chose their direction from a reply's THREAD rather
+ than from the reply itself, which made both one-way on a reply. A
+ message-scoped write never changes the thread's tags, so the answer never
+ moved: pressing Toggle unread on an unread reply re-added the tag it already
+ had, and pressing Delete twice on a reply deleted it twice instead of putting
+ it back. Re-applying a tag a message already carries changes nothing, which
+ is why the key looked dead. Each row is now asked about what it stands for.
+
## [0.24.0] - 2026-08-15
Double-clicking a row now opens its thread on its own, expanded, with the row
diff --git a/CLAUDE.md b/CLAUDE.md
index b1764e3..50faf4b 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -473,13 +473,181 @@ its siblings and `threadAt(current.row())` on the first reply of any thread
returns the FIRST THREAD IN THE LIST. This shipped in `markCurrentThreadRead`,
was mostly masked while the write it guarded was thread-wide, and became "a
random message was marked read" the moment a fix scoped that write to one
-message (items 87 and 88). Reach the thread through the INDEX, never through a
-row number, unless you have already established the index is a thread row.
-
-The general rule is in the item 20 note further down, "anything keyed on a row
-NUMBER did not survive the port to a tree". This is a surviving instance, found
-by corrupting real mail rather than by reading, so treat every remaining
-`.row()` in `mainwindow.cpp` as suspect until checked.
+message (items 87 and 88).
+
+**Use `threadFor(const QModelIndex &)`**, which resolves a message row through
+its parent and a thread row through itself. Item 88 added it on 2026-08-16 and
+converted every caller; `threadAt(int)` survives only for loops over
+`rowCount()`, which genuinely hold a top-level number. A new caller that has an
+index and reaches for `threadAt(index.row())` is reintroducing the bug.
+
+**The audit found four live sites, not the one that was reported**, which is the
+part worth remembering: `delete` and `toggle_unread` each chose their DIRECTION
+from the wrong thread, and the tag dialog counted the wrong thread's tags. All
+three were reachable by clicking a reply, none had a test, and the reported
+symptom named only `markCurrentThreadRead` (which was in fact protected by an
+unrelated guard and could not fire). One bad accessor produced four defects with
+one symptom between them.
+
+**A thread's first message is NOT among its children, and two lookups forgot
+it.** `setThreadMessages` drops depth 0 because the root row stands for that
+message, so `children` never holds it. `applyMessageTagChange` and
+`messageById` both search the root first now (item 109); before that, a
+message-scoped write to a root card repainted nothing, and the strip refresh set
+the pane's chips to the empty node the lookup returned, destroying a strip that
+had been correct. Item 108 made that the ordinary gesture rather than an edge
+case: the two changes were each correct and broken together.
+
+**`ThreadSummary::tags` is notmuch's UNION over the thread, and a card that
+stands for one message must not draw it.** A four-message thread whose third
+message is `signed` reads as signed, so the root card and the message pane both
+claimed a tag the displayed message did not have (item 110). `MessageRef`
+carries the message's own tags and arrives on every load;
+`ThreadListModel::setRootMessageTags()` records them on `ThreadNode::first`, and
+a thread row's `data()` substitutes `first.tags` for the summary's when that
+node exists. Only the TAGS are substituted: the subject, authors, date and reply
+count describe the thread and are correct. The summary itself is never
+rewritten, because the thread-scoped actions and the query read it.
+
+That was also why a root card could not repaint: with no per-message tags, a
+message-scoped write had nothing to change. `applyMessageTagChange` keeps the
+summary in step only when `totalCount <= 1`, where the union IS the message.
+
+**The card shows BOTH tiers, and that is item 111 rather than a leftover.**
+`PillTagsRole` returns the displayed message's tags first and the thread's other
+tags after; `PillOwnCountRole` is the boundary the delegate switches fonts at.
+The second tier is drawn in `CardLayout::siblingFont()` and
+`CardDelegate::mutedChipColour()`.
+
+**The split comes from the QUERY, not from the message load**, and that
+distinction was worth a whole round trip. `ThreadSummary::firstMessageTags` is
+read by the same worker walk that finds `firstMessageId`, so an UNOPENED row
+already knows which tags are its own. Deriving it from the load instead left
+every unopened row drawing one tier and correcting itself on selection, which is
+most of the list and is exactly the "chip changed when I clicked" the feature
+exists to remove. `nodeFor()` seeds the node on arrival; `reconcile()` must
+refresh it AND compare it, since a survivor keeps its node and a sync can move
+the root's tags while the thread's union stands still.
+
+**A size step must be a FRACTION, not a subtraction, and the padding has to
+follow it.** One point off a 14pt desktop font is a 7% step and reads as the
+same size; the user reported exactly that. `CardLayout::siblingFont()` is 0.70
+of the card font. `TagChip::kPaddingX` is a fixed 9px a side, so an unscaled
+sibling chip is 18px of padding around ~30px of text and stays wide while its
+letters shrink: `TagChip::sizeFor()` takes a scale, and
+`CardDelegate::chipSize()` is where the tier chooses it. Assert on ratios rather
+than sizes, so the test is about the distinction and not the constant.
+
+Muting is **saturation only**. Hue stays so the tag is recognisable; lightness
+stays so `TagColors::textColourOn()` keeps its choice and the chip cannot become
+unreadable. Do not blend toward the background here: `accentLineColour()`
+records what that costs on a dark theme, and a chip is worse because its fill
+carries text.
+
+**`TagStrip::visibleTags()` measures the LAYOUT, not the data.** It is one row
+that collapses the overflow into a trailing "+N" chip, and an unshown window
+under the offscreen platform has no width, so nearly everything lands in
+`hiddenTags()`. A test asserting on `visibleTags()` alone passes or fails on how
+many tags happened to fit; two shipped that way before it was noticed. Assert on
+`visibleTags() + hiddenTags()`.
+
+**A message-scoped write repaints the MESSAGE's row, never the thread's.**
+`ThreadListModel::applyMessageTagChange()` is the counterpart to
+`applyTagChange()` and exists because there was no optimistic update at all for
+a one-message edit: the correct observation that repainting a thread card for a
+one-message change is a lie was turned into the wrong conclusion that nothing
+should repaint, so Delete and Toggle unread on a reply moved the pending count
+and changed nothing on screen (item 105). The thread card deliberately stays
+put; one deleted reply does not doom the conversation.
+
+**A thread ROW means the one message its card displays, not the conversation.**
+Item 108, 2026-08-16. `ThreadListModel::messageScopeFor()` is what the ordinary
+tag actions resolve through; `scopeFor()` still returns whole threads and is
+what the five `*_thread` actions use. A thread row's message is
+`ThreadSummary::firstMessageId`, carried from the query, so no expansion is
+needed; in the Sent view that is the first MATCHED message, which is right for
+the same reason it is right on the card. A row with no id contributes NOTHING
+rather than falling back to its thread: that fallback is the silent escalation
+this removed.
+
+The automatic mark-read follows the same rule (item 87): `m_markReadMessageId`,
+armed for a reply as well as a root. One approximation is deliberate and
+documented at the call site: a thread row arms from `ThreadSummary::isUnread()`,
+a union over the conversation, so it can arm for a thread whose displayed
+message is already read. The write is still scoped to that message, so the cost
+is a no-op rather than a wrong write.
+
+**Adding an action is four places, and three of them are enforced by tests that
+fail in confusing ways.** `KeyMap::knownActions()` (a `Q_ASSERT` in the
+constructor fires otherwise, and it surfaces in whichever suite happens to build
+a `MainWindow` first — `test_tagrules` did), `defaultBindings()` (every action
+must be keyboard-reachable), and the icon table (every action must carry one).
+The no-duplicate-icons rule is narrowed to actions that can reach the toolbar,
+by a named exception list; the five thread actions share their twins' icons
+because a submenu entry always carries text, and the test asserts none of them
+is on the toolbar so the exemption cannot be abused.
+
+**A toggle must read the state of what the row STANDS FOR, not of its thread.**
+`MainWindow::everySelectedRowHasTag()` is the one question `delete` and
+`toggle_unread` both ask; a reply row answers from its message, a thread row
+from its thread. Reading the thread makes a toggle ONE-WAY on a reply, and the
+failure is silent in a specific way worth knowing: the write is message-scoped,
+so it never changes the thread's tags, so the answer never moves however many
+times the key is pressed. The second press re-sends a tag the message already
+has, which is a no-op, and a no-op repaints nothing. The user reports this as
+"the key does nothing", not as "the key did the wrong thing" (item 105).
+
+This is the SECOND fix to the same three lines. Item 88 corrected which thread
+they resolved; that was necessary and not sufficient, because a reply needs a
+message read rather than a better thread. "Resolved through the index" and
+"resolved to the right object" are separate properties, and a test for the first
+passes against the second being wrong.
+
+**Any state a thread row draws, a reply row has to draw too, and this was
+missed once already.** The message-row branch of `data()` is a separate switch
+from the thread branch, so a cue added to one is simply absent from the other
+with nothing to flag it. The doomed fill and the strike-out were thread-only
+from item 13 until item 105, which is why updating the node was not enough on
+its own to make Delete visible. When adding a visual state, check both branches.
+One asymmetry is deliberate and must survive: a reply carries no tag strip. A
+reply is now BOLD when unread, at the user's request on 2026-08-16, combining
+with the dimming for the same two-cue reason a thread row has both; the smaller
+reply font is what keeps it subordinate. A `deleted` reply deliberately shows
+the chip as well as the fill and strike-out, matching the thread row, confirmed
+with the user rather than treated as redundancy to remove.
+
+**And the reverse: a thread-scoped write must reach the thread's LOADED
+replies.** `applyTagChange` updated the summary only, so marking an expanded
+thread read left every reply bold and undimmed until the next query (item 107).
+The symptom reads as a missed repaint and is not: the rows were redrawn from
+data that had not changed. When a model update looks like it did not paint,
+check whether the data behind those rows actually moved.
+
+**Every path a thread-scoped write travels, a message-scoped one travels too,
+and each one was missed separately.** Three of them, found one hand-test round
+apart: the optimistic model update (item 105), the message pane's tag strip
+(also 105, keyed on `m_currentMessageId` and read by id through
+`ThreadListModel::messageById()`, never from `currentIndex()`), and
+`flushHeldEdits()`, which re-sent only thread edits and therefore DROPPED any
+message edit made during a sync after showing it and counting it as pending
+(item 106, data loss, never reported). When adding anything to
+`sendThreadTagChange`, check whether `sendMessageTagChange` needs it. Escalating
+a message edit to its thread is never the fix: it deletes a whole conversation
+when the user deleted one reply.
+
+**Anything applied optimistically must also be reverted.**
+`revertPendingTagChange()` keyed on `m_pendingThreadIds` alone, so adding the
+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`.
+
+**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
+answer identically whichever way the code resolves them. That second point is
+why the reverted item 87 fix was mutation-checked and green while corrupting
+real mail. For a toggle, assert on `undoText()`: both directions push one
+command over the same rows, so depth and ids cannot tell them apart.
**A test for a mutation on a data-writing path must exercise the REPLY case,
not only the root.** The reverted fix above was mutation-checked and green: it
@@ -503,6 +671,12 @@ measuring nothing:
children are populated by the expansion. `hasChildren()` is the pre-expansion
question and falls back to `summary.totalCount > 1`. An assertion on
`rowCount` fails against correct code.
+- **A `ThreadSummary` fixture needs `firstMessageId`.** Since item 108 an
+ ordinary tag action resolves a thread row to that id, so a summary without one
+ names no message and every action on it silently does nothing. Ten tests
+ failed this way at once, all reporting "the action did not happen", which
+ reads as a defect in the action rather than a gap in the fixture.
+ `makeThread()` sets it; a hand-built summary must too.
- **`currentThreadId()` reports INTENT, not content.** It is assigned
synchronously in the selection handler before any worker round-trip, so a test
asserting on it passes with `onThreadLoaded()` disabled entirely, measured.
@@ -543,6 +717,13 @@ a defect that did not exist because of these; each was believed until it was con
buttons to the query row: the thread list shrank, and a test sizing its window to 300px
started failing with a message naming a defect that did not exist. Assert the rect is
INSIDE the viewport, not merely non-empty.
+- **A probe can be correct and still measure nothing, by being pointed at the
+ wrong object.** A test for the sibling chip's padding called
+ `TagChip::sizeFor()` directly: that proves what the function does and nothing
+ about whether the delegate asks it for a scaled padding, so a mutation
+ dropping the scale at the call site stayed green. Assert through the function
+ the production path actually calls (`CardDelegate::chipSize()`), not through
+ the one it calls INTO.
- **A "saturated pixel" threshold catches antialiased edges of the selection highlight**,
hundreds of distinct near-background colours, and will pass whatever the code does. Match
the exact colours the model supplies instead. Two versions of one test passed under
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 30271d5..3620684 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
@@ -5624,3 +5624,697 @@ it falls out of the existing code rather than needing a guard.
**Nothing was built for getting back**, per the user: the filter buttons already
are that.
+
+## 88. `threadAt(current.row())` answers about the wrong thread for a reply row
+
+**Observed (user, 2026-08-14):** "I clicked on a message and another random
+message was marked read."
+
+**Cause (verified).** `markCurrentThreadRead` calls
+`m_model->threadAt(current.row())`. A `QTreeView` numbers rows PER PARENT, so a
+reply's `row()` indexes its siblings: `threadAt(0)` on the first reply of any
+thread returns the FIRST THREAD IN THE LIST. The guards then compare correct ids
+against the wrong thread and let a write through.
+
+**`CLAUDE.md` already records this trap**, from item 20: "what did NOT survive
+that port is anything keyed on a row NUMBER". This is a surviving instance, not
+a new discovery, which is the uncomfortable part.
+
+**Mostly masked before item 87's attempted fix.** With a thread-wide write the
+mismatch usually resolved to the same thread or to a no-op; scoping the write to
+one message made it visible immediately, as unrelated mail being marked read.
+
+**Approach.** `threadAt(int)` takes a row and cannot be safe here. Every caller
+that has a `QModelIndex` should reach the thread through the index: for a
+message row, `ThreadListModel::threadIdForMessage()` or the parent index; for a
+thread row, `index.row()` is correct. Audit every `threadAt(` and every
+`.row()` in `mainwindow.cpp` rather than fixing this one site, since the same
+shape almost certainly appears elsewhere.
+
+**Consider deleting the row-taking overload** once the callers are converted, so
+the trap cannot be re-entered. That is the change that makes this permanent
+rather than fixed once.
+
+**A second live site, found 2026-08-16** during the notes reconciliation, before
+any of the audit was done. The `delete` action's toggle
+(`src/mainwindow.cpp:825`) decides its direction by looping over the selected
+rows with `m_model->threadAt(index.row()).isDeleted()`. With a reply row in the
+selection that reads an unrelated thread's state, so Delete can choose the wrong
+direction: pressing it on a reply of an undeleted thread can undelete instead of
+delete, or the reverse. Less damaging than item 87's case, since the write itself
+goes through `scopeFor(rows)` and lands on the right threads, but the direction
+is chosen from the wrong data. The audit is therefore confirmed as necessary
+rather than precautionary, and this is at least the second site. Item 98 wants to
+add a THIRD in the same shape, by copying this loop for "Important"; fix this
+first or it propagates.
+
+**Size: S** for the one site, **M** if the audit finds several. The second site
+above pushes this toward M.
+
+**Outcome, 2026-08-16: M. The audit found FOUR live sites, not one**, which
+settles the question of whether auditing was worth it over fixing the reported
+call. Every one of them read a thread's STATE for a selected index, and every one
+of them was wrong the same way for a reply:
+
+| Site | What it decided | What went wrong for a reply |
+|---|---|---|
+| `delete` action | delete or undelete | direction read from the first thread in the list |
+| `toggle_unread` action | mark read or unread | same, so the key went the wrong way |
+| `selectionTagCounts` (was inline in `editTagsOnSelection`) | the tag dialog's tri-state boxes | offered to remove tags the selection did not carry |
+| `markCurrentThreadRead` | whether to write | unreachable today, see below |
+
+**The fix is one accessor, not four patches.**
+`ThreadListModel::threadFor(const QModelIndex &)` resolves a message row through
+its PARENT and a thread row through itself, so the conversion happens once
+instead of at every call site that has to remember which kind of row it is
+holding. The row-taking `threadAt(int)` survives for the three loops over
+`rowCount()` that legitimately have a top-level row number, and now carries a
+doc comment saying why it is wrong for anything else. No `.row()` on a selected
+index remains in `mainwindow.cpp`.
+
+**`markCurrentThreadRead` was already protected, by accident.**
+`onThreadSelected` clears `m_currentThreadId` for a message row, and the handler
+returns early when that does not match `m_markReadThreadId`, so a reply could not
+reach the bad read. That is incidental protection standing in front of a real
+defect, and item 87 does not remove it, so the test asserts on the RESOLUTION
+rather than on a write it cannot currently provoke. Recorded because "it cannot
+happen today" is exactly the reasoning that left this in place after item 20.
+
+**Two things the tests had to do that the reverted attempt did not.** The
+fixture puts the reply under the SECOND thread, so `threadAt(0)` returns a
+plausible wrong answer rather than the right one by luck; and the two threads are
+in OPPOSITE states, since two threads in the same state give the same answer
+either way. That second point is why the reverted fix was mutation-checked and
+still green. All four tests were confirmed failing against the old behaviour,
+each naming the wrong direction it chose, before the fix went back in.
+
+**The `undoText` seam is what made the toggles testable at all.** Delete and
+Undelete push one command each and touch the same rows, so depth and ids are
+identical between right and wrong; only the description differs.
+
+
+## 105. Acting on a reply changes the counter and nothing on screen
+
+**Observed (user, 2026-08-16),** hand-testing item 88's fix: "I'm hitting delete
+on a reply to a thread, I see the edits counter increasing but I have no
+feedback if that message is being deleted. Same issue when marking read/unread
+on a reply from a thread."
+
+**Two independent causes, and fixing either alone leaves the symptom.**
+
+*The write made no optimistic update.* `sendMessageTagChange` deliberately
+skipped one, on reasoning that was correct as far as it went: `applyTagChange`
+is keyed by THREAD, so calling it for a one-message edit would repaint the whole
+card as though every message in it had changed. The conclusion did not follow.
+The reply has its own row, and that is where the feedback belongs.
+`ThreadListModel::applyMessageTagChange()` updates the node and emits
+`dataChanged` for that row alone.
+
+*A reply row had no doomed cue at all.* Thread rows have been filled crimson and
+struck through since item 13; the message-row branch of `data()` returned
+`replyBackground()` unconditionally and never consulted the node's tags. So even
+with the node updated, a deleted reply painted identically to an undeleted one.
+Both the fill and the strike-out are now mirrored onto reply rows, and the
+foreground goes white over the fill, since `readColour()` is mixed toward the
+BACKGROUND and would otherwise compute a grey against the pane's base and paint
+it over crimson.
+
+**The read/unread half was NOT fixed by the above, and the first attempt
+stopped one layer short.** Hand-tested the same day: Delete now repainted and
+read/unread still did nothing. The model was right and the write was right; the
+DIRECTION was wrong. Both toggles read the state of the row's THREAD, and a
+message-scoped write never changes a thread's tags, so the answer never moved.
+On a read thread `toggle_unread` therefore evaluated to "add unread" on every
+press, re-adding a tag the reply already carried; re-applying a tag a message
+has is a no-op, and a no-op repaints nothing. The user found the repaint only by
+deleting and undoing, which is a DIFFERENT write forcing the row to redraw.
+
+`MainWindow::everySelectedRowHasTag()` is now the single question both toggles
+ask, and each row answers about what it STANDS FOR: a reply row about its
+message, a thread row about its thread. `delete` had the identical latent bug,
+found by reading rather than by report: pressing Delete twice on a reply deleted
+it twice instead of undeleting, since its thread never became deleted.
+
+**Item 88 fixed WHICH thread these read, and that was necessary but not
+sufficient.** A reply does not need a better thread, it needs a message. Two
+fixes in two sessions on the same three lines, and the second was only found by
+the user pressing the key: worth remembering that "resolved through the index"
+and "resolved to the right OBJECT" are separate properties.
+
+**Bold on replies, at the user's request.** Replies were deliberately never
+bold, on the reasoning that the thread row above already carries the unread cue
+for the conversation. True of the thread and useless for the reply: once a
+thread is expanded, the reply row is the only thing that can say which messages
+in it are unread, and dimming alone (white against `#a8a8a8`) was too quiet.
+Bold now combines with the dimming, which is the same two-cue pair a thread row
+has had since 0.11.0 and for the same recorded reason: a desktop configured with
+a Bold UI font makes `setBold()` a no-op, so a single cue has a single point of
+failure. The smaller reply font stays, since that is what separates a reply from
+a thread heading.
+
+`CardLayout` needed no change: it already measures the date rect BOLD whatever
+font it is handed, precisely so an unread card cannot clip its date. Checked
+rather than assumed, since a newly bold row is exactly what that guard exists
+for.
+
+**A third pass: the message pane's chips.** Hand-tested again after the toggle
+fix, and the LIST was correct while the pane was not: "for it to sync I have to
+change message and go back to the edited one". `sendThreadTagChange` had always
+refreshed the strip when the edited thread was the open one; `sendMessageTagChange`
+had no equivalent, so the chips kept describing the message as it was before the
+edit. Mirrored now, keyed on `m_currentMessageId`, and reading the tags by ID via
+the new `ThreadListModel::messageById()` rather than from `currentIndex()`: the
+two agree today, and a guard that depends on them agreeing puts the WRONG
+message's tags in the pane on the day they do not.
+
+Both halves are separately mutation-checked, because a refresh with no guard and
+a guard with no refresh each pass the other's test: removing the refresh
+reproduces the user's report, and dropping the guard makes an edit to a
+different reply repaint the open one with that reply's tags.
+
+**Three hand-test rounds for one defect, each finding a real and different
+cause.** Optimistic update, then direction, then the pane. Recorded as the
+argument for handing UI work back after every round rather than assuming the
+last fix finished it, which is the lesson the memory file already carries and
+this is the clearest instance of it.
+
+**Reading that flush turned up item 106**, a silent data loss on the same path,
+which had never been reported and would not have been.
+
+**The redundant `deleted` chip is deliberate and was left alone.** A deleted
+reply shows the fill, the strike-out AND a `deleted` chip. The user raised it;
+the thread row has always done the same, with the model recording that "a doomed
+thread is rare and worth naming", so replies match rather than diverge.
+Confirmed with the user 2026-08-16 rather than silently changed.
+
+**One consequence that had to be fixed with it.**
+`revertPendingTagChange()` checked only `m_pendingThreadIds`, so with an
+optimistic update now applied for message writes, a FAILED message write would
+have kept showing its optimistic state permanently, with nothing to correct it
+until the next query. It reverts both scopes now. Undo needed no change:
+`MessageTagCommand` routes back through `sendMessageTagChange`, so it repaints
+by the same path.
+
+**Testing note.** The model test proves `applyMessageTagChange` works; it does
+not prove the ACTION reaches it, which was the half actually missing. The
+window-level test triggers the real Delete action on a real reply row and
+asserts the row's background changed, and that the thread's did not. Both were
+mutation-checked: no-oping the new method fails them with the user's own
+symptom printed back.
+
+## 106. A tag change made on one message during a sync is silently lost
+
+**Observed: nowhere.** No user report. Found by reading `flushHeldEdits()` while
+fixing item 105's strip refresh, which is worth recording because the failure is
+invisible from the outside until the mail is already gone.
+
+**Cause (verified).** An edit made while notmuch's write lock is held is queued
+as a `HeldEdit` and re-sent when the sync ends. The flush looped over
+`edit.threadIds` and called `sendThreadTagChange()` and nothing else. A
+message-scoped edit carries NO thread ids: `sendMessageTagChange()` queues
+`HeldEdit{ {}, TagChange{ messageIds, ... } }`. So the loop ran zero times, the
+send returned immediately on an empty list, and the edit was dropped.
+
+**What the user would have seen, and why it is worse than a plain loss.** The
+optimistic update had already been applied, so the row showed the change. The
+pending-edit indicator counted it, so the window reported an unsynced change. On
+the next sync end the count cleared. Every signal the application gives said the
+edit was made and then safely written; none of it happened. Item 28 is the
+record of a wrong pending count being treated as serious on its own, and this is
+that failure pointed at real mail.
+
+**Reachability.** Present since message-scoped writes shipped, and rare in
+practice: it needs an edit on a REPLY (or any message row) made in the window
+where a sync holds the lock, which is around 35 seconds per run. Item 105 made
+message-scoped edits considerably easier to make deliberately, so the odds were
+about to go up.
+
+**Fix.** The flush dispatches by scope: thread ids through
+`sendThreadTagChange`, `change.messageIds` through `sendMessageTagChange`, each
+guarded on being non-empty. The optimistic update is taken back for both kinds
+before re-sending, since both send paths re-apply it.
+
+**The tempting wrong fix is escalation.** Sending a held message edit as a
+thread edit would make the queue "work" and would delete every message in a
+conversation when the user deleted one reply. The test asserts the scope
+survives the hold, not merely that something was sent.
+
+**Size: XS**, once seen. The whole cost was in noticing.
+
+## 107. A thread-scoped write leaves the loaded replies showing their old tags
+
+**Observed (user, 2026-08-16):** "if I hit read/unread on the main thread
+message, the status bar announces 'mark as read: N messages (whole thread)'.
+When this happens only the main message is repainted, together with its chip in
+the right pane. The replies don't get repainted."
+
+**Cause (verified).** `ThreadListModel::applyTagChange()` updated
+`m_threads[row].summary.tags` and emitted `dataChanged` for that one row. It
+never touched `m_threads[row].children`, so an expanded thread's reply rows kept
+the tags they were loaded with.
+
+**Not a repaint bug, which is what the symptom looks like.** The rows were
+repainted; they were repainted from data that had not changed. A thread-scoped
+write reaches every message in the thread in the DATABASE, so the replies were
+genuinely read while the model still described them as unread: bold, undimmed,
+and correct-looking. They fixed themselves on the next query, which is exactly
+the signature of a stale model rather than a missed signal.
+
+**Fix.** The same tag arithmetic is applied to every loaded child, followed by
+one `dataChanged` spanning the whole expansion rather than a signal per reply.
+Only LOADED replies are updated: an unexpanded thread has no child rows, and
+messages the model does not hold are the database's business.
+
+**The test asserts both halves**, because they fail independently: that the
+child nodes carry the change, and that a `dataChanged` actually covered the
+reply rows. Mutating the node without telling the view leaves the old pixels on
+screen; signalling without mutating repaints the same wrong data.
+
+**Size: XS.** Found while the user was hand-testing item 105.
+
+## 108. Acting on a thread root means the whole thread, though it displays one message
+
+**Observed (user, 2026-08-16),** after item 107:
+
+> we should split marking one message read/unread from marking the whole thread
+> read/unread, as they are two different actions
+
+and, asked what the root card should mean:
+
+> since clicking on a thread main message displays the main message only, acting
+> on it should affect that message only. A separate menu for thread actions
+> would be ideal, like a submenu when right clicking and the same submenu under
+> "Message" in the top menu.
+
+**Cause: a deliberate design that item 66 invalidated and nobody revisited.**
+`ThreadListModel::scopeFor()` maps a thread row to `threadIds` and a message row
+to `messageIds`. That was coherent when a root card RENDERED the whole
+conversation: what you saw was what you acted on. Item 66 made the root render
+one message and left the scope alone, so the card now shows one message and acts
+on all of them. The status bar's "(whole thread)" suffix is the only thing that
+says so, and it says it after the fact.
+
+**The end state the user described.** A thread row means the message it
+displays, exactly as a reply row already does. Whole-thread actions still exist,
+in their own submenu, reachable from the thread list's context menu and from the
+`Message` menu, which today are two views of one set of `QAction` objects.
+
+**Approach.**
+
+- `scopeFor()` resolves a thread row to its first message id rather than its
+ thread id. `ThreadSummary::firstMessageId` is already carried from the query
+ for exactly this kind of question (item 66), so no expansion is needed.
+- A second set of actions, thread-scoped, resolving the selection's threads and
+ going through `sendThreadTagChange`. These are the current actions' bodies,
+ unchanged.
+- One submenu built once and added to both menus, per the user's description.
+- The toolbar keeps the message-scoped actions. It is the beginner surface and
+ the frequent case; a submenu on a toolbar button is a worse affordance than
+ the menu entry it duplicates.
+
+**Constraints, and these are what make it M rather than S.**
+
+- **Every keybinding keeps its meaning name.** `Ctrl+U` stays `toggle_unread`
+ and now means one message; the thread action needs its OWN action name for
+ `[keys]`, since a user's config names actions. Do not reuse a name with new
+ semantics, and do not rename an existing one, which silently breaks a config
+ that mentions it.
+- **`markAllRead` is not part of this.** It deliberately ignores the selection
+ and acts on every row in the view, and it is thread-scoped by nature. Leave
+ it alone; it is the one action where "whole thread" is not a scope question.
+- **The undo stack must record the scope it actually used.** `MessageTagCommand`
+ and `ThreadTagCommand` already differ; a thread action that pushed the message
+ command would undo a fraction of what it did.
+- **Auto mark-read is item 87 and gets simpler, not harder.** That item exists
+ because the timer marks a whole thread read while showing one message. Under
+ this change the natural scope for it is the message, which is what 87 asks
+ for. Do 108 first and 87 becomes small; doing them in the other order means
+ writing the same decision twice.
+- **The status bar's "(whole thread)" suffix stays**, and becomes rarer and more
+ meaningful: it will mean the user chose the thread action rather than that the
+ application widened the scope for them.
+- A multi-row selection mixing thread rows and reply rows stays honest, as
+ `ActionScope` already is: nothing is escalated or narrowed silently.
+
+**This is a user-visible behaviour change to every tag action.** Anyone with the
+current habits will find `Ctrl+D` on a thread root deleting one message where it
+used to delete the conversation. Semver on the user-visible surface makes it a
+minor bump with an `### Upgrading` section, and the changelog has to say plainly
+that the old behaviour moved rather than disappeared.
+
+**Verification.** The scope decision is testable without a painter, which is
+where the assertions belong: `scopeFor()` on a thread row returns a message id,
+the thread action returns a thread id, and each pushes the matching undo
+command. A rendering probe adds nothing here.
+
+**Size: M.** The code is small and touches every action; the review cost is in
+the naming and the config compatibility.
+
+**Outcome, 2026-08-16. The estimate held: the code was small and the review cost
+was where it was predicted to be.**
+
+`ThreadListModel::messageScopeFor()` sits beside `scopeFor()` rather than
+replacing it, so both meanings stay expressible and the thread actions reuse the
+already-tested path. `MainWindow::TagScope` defaults to `Message`, so every
+existing caller kept its line. Five `*_thread` actions, each its message-scoped
+twin with `TagScope::Thread`, in a "Whole thread" submenu added to both the
+`Message` menu and the thread list's context menu.
+
+**Three things the plan did not anticipate, all caught by existing tests rather
+than by reading.**
+
+- **`Q_ASSERT(m_actions.size() == KeyMap::knownActions().size())` fired**, from
+ `test_tagrules` of all places, because that test builds a `MainWindow`. Five
+ new actions and no entries in `knownActions()`. The assertion is eight months
+ old and did exactly its job; worth noting that the failure surfaced in a suite
+ with nothing to do with actions.
+- **"Unbound by default" was wrong and the suite said so.**
+ `everyActionHasAShortcut` requires every action to be keyboard-reachable. The
+ bindings are `Ctrl+Alt+<same key>`, one modifier out from each twin.
+ `Ctrl+Shift` was the obvious pairing and is taken twice over, by `spam` and
+ `mark_all_read`, both shipped.
+- **The no-duplicate-icons rule had to be narrowed, with the user's agreement.**
+ These five deliberately share their twin's icon. That rule exists because the
+ toolbar can be icon-only, where the icon is the whole control; these live only
+ in a submenu, whose entries always carry text. The exception is a NAMED list,
+ and the test now also asserts none of them is on the toolbar, so putting one
+ there fails rather than silently passing.
+
+**The fixture change worth remembering.** `makeThread()` in `test_mainwindow`
+set no `firstMessageId`, so after this change every thread row resolved to no
+message and ten tests failed with "the action did not happen" — which reads as a
+defect in the action rather than a gap in the fixture. The real worker fills
+that field from the query; the fixture now does too.
+
+**Tests that encoded the OLD meaning were repointed, not deleted.** Three of
+them (the delete toggle, the mixed selection, the "(whole thread)" suffix) are
+about thread semantics and now drive the thread actions; three held-edit tests
+assert on a thread row and drive `flag_thread` for the same reason. One, about
+the `flagged` tag NAME, was rewritten to assert on the change that was sent
+rather than on a model row, since the tag is the same under either scope.
+
+## 87. Auto mark-read marks a whole thread, including replies never displayed
+
+**Observed (user, 2026-08-14):** "with the first message in a thread selected
+(not expanded), the 2s delay that marks it read applies to the whole thread, so
+all answers are marked read as well."
+
+**Cause (verified in the code).** `markCurrentThreadRead`
+(`src/mainwindow.cpp`) sends `sendThreadTagChange` with the thread id, so
+`unread` is removed from every message in the thread.
+
+**This was coherent until item 66 and is not any more.** While a thread root
+rendered the whole conversation, everything marked read HAD been displayed.
+Item 66 made a root render one message; the thread-wide write stayed.
+
+**Not cosmetic.** `maildir.synchronize_flags` is on, so removing `unread`
+rewrites Maildir filenames and the next sync carries it to the server. Mail the
+user never opened stops being unread everywhere, and nothing here can put it
+back except reading the messages again by hand.
+
+**Approach: unblocked 2026-08-16, and the ordering was the lesson.** The obvious
+fix is to send `sendMessageTagChange` for the message on screen. That was
+written, tested, mutation-checked, shipped and reverted the same evening,
+because it exposed item 88: the guards above the write read
+`threadAt(current.row())`, which answers about an unrelated thread when a reply
+row is current, and a fix that narrows the write turns that mismatch into "a
+random message is now read".
+
+Item 88 is now done. `markCurrentThreadRead` resolves through
+`ThreadListModel::threadFor(index)`, so the guards read the right thread for a
+reply. **Do not take that as the whole of it:** 88's fix deliberately left the
+`m_currentThreadId` guard in place, which is what keeps a reply from reaching
+the write at all today. Narrowing the write means deciding what that guard
+should be once a message row is a legitimate target, rather than assuming 88
+cleared the road.
+
+**Constraints for the second attempt.**
+
+- **Test selecting a REPLY, not only a root.** The reverted work asserted on a
+ root selection, which is the one case where `current.row()` is correct, so a
+ mutation-checked test passed against a bug that corrupted mail.
+- **Consider doing item 108 first.** It makes a thread row mean the message it
+ displays, which is the same decision this item has to make for the timer, in
+ the same place. Done in that order this becomes small; done the other way the
+ decision is written twice and has to agree with itself.
+- **The repaint half is done**, as item 105 on 2026-08-16.
+ `ThreadListModel::applyMessageTagChange()` exists, `sendMessageTagChange`
+ calls it, and a reply row now carries the doomed fill and strike-out it had
+ never had. What remains for this item is the THREAD card, below.
+- A thread reads as unread while ANY message does, so the card can only stop
+ looking unread when nothing else in it carries the tag. For an unexpanded
+ thread the per-message tags are not loaded, so the honest answer needs
+ per-message counts in `ThreadSummary` or nothing at all.
+
+**Size: S**, after 88.
+
+
+**Outcome, 2026-08-16, and doing 108 first was the whole saving.** The decision
+"what does a thread row stand for" belongs to 108; once it was made, this item
+was a rename and two call sites.
+
+`m_markReadThreadId` became `m_markReadMessageId`, `scheduleMarkRead` takes an
+id and an unread flag rather than a `ThreadSummary`, and the handler sends
+`sendMessageTagChange`. The guard now compares against what the PANE is showing
+rather than against the selection, which is the same thing for both kinds of row
+and is what "the message the user is reading" actually means.
+
+**A reply arms the timer now, which it never did.** The old code stopped it
+explicitly for a message row, with a comment saying a per-message write did not
+fit a thread-keyed pending map. That objection died with item 105's
+message-scoped path, and leaving it would have made the message the user is
+actually reading the one kind that is never marked read.
+
+**The approximation that remains, and it is deliberate.** A thread row arms from
+`ThreadSummary::isUnread()`, which is a union over the conversation, so a thread
+whose first message is already read but whose reply is not will arm and then
+write `unread` off a message that does not carry it. That is a no-op, not a
+wrong write: the id is the displayed message's either way. Removing it needs
+per-message state in `ThreadSummary`, which is the same thing the third
+constraint below asks for and which nothing carries yet.
+
+**The third constraint looked unmet and was closed two items later.** A thread
+reads as unread while ANY message does, so an unexpanded card could not honestly
+stop looking unread when one message in it was marked read, and the automatic
+mark-read therefore had no visible effect on an unexpanded row. Item 110 settled
+it from the other direction: the card stops drawing the thread's union at all
+and draws the tags of the message it displays, which arrive with the message
+load. No per-message counts in `ThreadSummary` were needed. What remains is
+narrower: a row that has never been opened has no per-message answer and still
+shows the union.
+
+**Mutation-checked** by putting the thread-wide write back: both tests fail, one
+naming the six undisplayed messages.
+
+## 109. A root card's own message is invisible to a message-scoped write
+
+**Observed (user, 2026-08-16),** hand-testing item 108: "delete single message
+on the root message of a thread doesn't trigger the repaint, delete whole thread
+does (correctly repaints all messages) - right pane loses the chip row when
+repainting, it simply disappears", and the same for read/unread.
+
+**Cause: one assumption, two symptoms.** `setThreadMessages` drops the depth-0
+message because the root ROW stands for it, so a thread's first message is never
+in `children`. Both message-scoped lookups searched `children` and nothing else:
+
+- `applyMessageTagChange` found no node, updated nothing, emitted no
+ `dataChanged`. The card did not repaint.
+- `messageById` returned a DEFAULT-CONSTRUCTED node, and the strip refresh
+ (item 105) set the pane's chips to its empty tag list. That is worse than not
+ refreshing: it destroyed a strip that had been correct.
+
+**Item 108 turned this from an edge case into the ordinary gesture.** Before it,
+a thread row meant the whole thread and took the thread-scoped path, which works;
+the message-scoped path was only ever reached from a reply row, where the node
+does exist. Making a root card act on its own message walked straight into the
+one case neither lookup handled. The two changes were correct separately and
+broken together, which is the argument for hand-testing after each one rather
+than at the end.
+
+**Fix.** Both look at the root first, matching on `ThreadNode::first` when it is
+loaded and on `ThreadSummary::firstMessageId` when it is not, since the user acts
+on unexpanded threads constantly. `messageById` synthesises a `MessageNode` from
+the summary in the unexpanded case.
+
+**The summary is updated only for a single-message thread**, and that
+distinction is load-bearing rather than cautious. `notmuch_thread_get_tags` is a
+UNION over the thread's messages: for a thread of one it IS that message's tags,
+so writing to it is exact; for a thread of seven, deleting one message does not
+delete the conversation, and repainting the card crimson would claim it did. The
+test asserts both halves, because a fix that updated the summary unconditionally
+passes the first and is a lie in the second.
+
+**What that leaves, stated rather than hidden.** On a multi-message thread the
+root card still shows no change for a one-message edit, because the card draws
+the thread's state and the thread's state has not moved. The message pane's
+chips follow, and the reply rows are individually correct once expanded. Making
+the card honest needs per-message counts in `ThreadSummary`, which is the same
+thing item 87's third constraint asks for. Recorded in the changelog as a
+deliberate limit.
+
+**A test-only trap found on the way**, and it had already been shipped twice in
+item 105's tests without being noticed. `TagStrip` is a single row that collapses
+whatever does not fit into a trailing "+N" chip, so under the offscreen platform
+with an unshown window `visibleTags()` returns almost nothing and `hiddenTags()`
+holds the rest. Asserting on `visibleTags()` measures the LAYOUT, not the data.
+The two earlier tests passed only because their tag counts happened to fit. All
+three now assert on `visibleTags() + hiddenTags()`.
+
+**Size: S.**
+
+## 110. A card and the message pane show tags belonging to a message's siblings
+
+**Observed (user, 2026-08-16),** hand-testing item 109 against a real
+four-message thread whose root carried `unread` and whose THIRD message carried
+`signed`: "If I select the root message and wait 2s, the right pane chips update
+and both signed and unread disappear. Changing message and going back to the
+root message makes them reappear."
+
+Plus two repaint reports in the same message: marking the root message read left
+the row bold, and deleting it left the row unpainted, while the whole-thread
+versions of both worked.
+
+**One cause under all three, and the first report is the one that revealed it.**
+`ThreadSummary::tags` comes from `notmuch_thread_get_tags`, which is the UNION
+over the thread's messages. Measured against the user's thread:
+
+| | tags |
+|---|---|
+| thread union | `inbox mailing-list/SBo signed unread` |
+| root message | `inbox mailing-list/SBo unread` |
+
+So `signed` belonged to a sibling and the pane was claiming it for the root
+message. The 2s mark-read write replaced the strip with the root's REAL tags and
+correctly dropped both `signed` (never its own) and `unread` (just removed).
+**The pane was lying before the write, not after it**, and reselecting restored
+the lie because selection sets the strip from the summary.
+
+**The same gap explains the missing repaints.** The model held no per-message
+tags for a thread's first message at all, so a message-scoped write had nothing
+to update and the card kept drawing the summary. Item 109 had made the write
+find the root, but the card still read the union, which had not changed.
+
+**Fix: the load is the authority.** `MessageRef` already carries the message's
+own tags and arrives on every open. `ThreadListModel::setRootMessageTags()`
+records them on `ThreadNode::first`, and a thread row's `data()` substitutes
+`first.tags` for the summary's when that node exists. `MainWindow::onMessageLoaded`
+calls it and re-sets the pane's strip from the same source.
+
+**Only the TAGS are substituted.** The subject, the authors, the date and the
+reply count on a card describe the THREAD and are correct as they are; only the
+tags were ever the union that lied. And the summary itself is not rewritten: it
+is what the thread-scoped actions and the query read, and three unread siblings
+do not stop being unread because this message was read.
+
+**What is still approximate, and it is now much narrower.** Before a row has
+ever been opened there is no per-message answer, so an unopened card shows the
+union and can display a sibling's mark. Narrowing that further means asking the
+query for per-message tags, which is a worker change rather than a UI one.
+
+**This closes item 87's third constraint by accident**, which had said an
+unexpanded card could not honestly stop looking unread. It can now, for the
+message it displays, because it draws that message's tags rather than the
+conversation's.
+
+**Both halves mutation-checked**: making the card read the summary again fails
+the repaint assertion, and removing the recording from the load fails the
+sibling-tag assertion.
+
+**Size: S.**
+
+## 111. A card should show its siblings' tags smaller, not drop them
+
+**Observed (user, 2026-08-16),** with a screenshot of the unselected card from
+item 110's thread, still showing `mailing-list/SBo` and `signed`:
+
+> I don't mind it actually, but "signed" disappears as soon as I select the root
+> thread, so it still looks like a bug even though it is not. The root thread
+> should rightfully display its own info only, but maybe we could leverage this
+> "bug" in another way, by showing the unioned tags smaller (and only on the
+> left pane) instead of removing them completely.
+
+**The design is the user's and it is better than either extreme.** Item 110 was
+correct that a card must not CLAIM a sibling's tag as the displayed message's,
+and the honest fix made a chip vanish on selection, which reads as a fault
+whatever the reasoning behind it. Showing both tiers keeps the card's real job:
+it stands for one message but sits above a conversation, and what the rest of
+that conversation carries is worth seeing at a lower weight.
+
+**No new data was needed.** The model already holds both: `ThreadNode::first`
+(the displayed message, from the message load) and `ThreadSummary::tags` (the
+union, from the query). `PillTagsRole` now returns own-then-sibling and
+`PillOwnCountRole` says where the boundary is.
+
+**The first attempt derived the split from the message LOAD, and the user
+rejected it immediately:** "not selecting the thread shows the chips at 'main'
+size, not smaller, not dimmed. After selecting the thread the unioned chips
+repaint to the correct size/color."
+
+That was honest and useless. A list is mostly UNOPENED rows, so the feature was
+invisible exactly where it was meant to be read, and selecting a row still
+changed the card, which is the thing this item exists to stop.
+
+**The query knows, and it was already looking.** The worker walks to the card's
+message to read its id; `ThreadSummary::firstMessageTags` reads that same
+message's tags in the same pass, from the index, so the split arrives with the
+row. `ThreadListModel::nodeFor()` seeds `ThreadNode::first` from it, and
+`reconcile()` refreshes both the field and the node for a survivor.
+
+**The reconcile half was a real gap found by reading**, not by the report: a
+surviving row deliberately keeps its NODE, because that is the expansion state
+reconcile exists to preserve, so the per-message tags had to be refreshed
+explicitly AND added to the change detector. A sync where only the root message
+changed leaves the thread's union identical, so nothing else would have noticed.
+
+**`first` is left empty when the query carried no per-message tags.** An empty
+list would be indistinguishable from "this message carries nothing", which would
+put every chip in the sibling tier and mute the whole card.
+
+**Two visual decisions, both settled with the user and both tested rather than
+eyeballed.**
+
+*Muting is saturation only.* Not a blend toward the background, which is the
+mistake `accentLineColour()` already records: on a dark theme that lands on the
+background and the chip disappears. Worse here, because a chip's fill has to
+carry legible text. Hue is untouched so a muted `signed` is recognisably the
+same colour as a full-size one, and LIGHTNESS is untouched so
+`TagColors::textColourOn()` keeps choosing the same text colour. The test
+asserts that last property directly, since contrast is exactly what a look at
+the screen will not reliably catch.
+
+*The smaller font is a FRACTION of the card's, not a fixed subtraction.* The
+first version took one point off `smallFont()` and the user reported the tiers
+as indistinguishable. The arithmetic is why: their desktop is 14pt, so the two
+tiers were 13 and 12, a 7% step. Subtraction gives a step whose size depends on
+the desktop font, which is backwards — it is largest exactly where the text is
+already small enough to be fragile. `CardLayout::siblingFont()` is 0.70 of the
+card font, giving 13 and 10 there, and the test asserts a RATIO rather than a
+size so it stays about the distinction rather than about the constant.
+
+*The padding has to scale with it, and this was half the problem.*
+`TagChip::kPaddingX` is 9 a side, so a sibling chip was 18px of padding around
+roughly 30px of text: the chip stayed wide while its letters shrank, which reads
+as "same chip, smaller text". `TagChip::sizeFor()` takes a scale, floored at 2px
+a side because the corner radius is half the height and text flush against it
+sits on the curve. Measured on the user's own font, `signed` goes from 71px to
+52px and the height from 25 to 20.
+
+The pixel branch is covered specifically: `pointSizeF()` is -1 for a font set in
+pixels, which qt6ct does, so a point-only implementation returns the original
+size and both tiers render identically. That trap is in `CLAUDE.md` and would
+have shipped silently on the user's own desktop.
+
+**A test that measured nothing, caught by mutation.** The padding test first
+called `TagChip::sizeFor()` directly, which proves what that function does and
+nothing about whether the delegate ASKS for a scaled padding. Dropping the scale
+at the call site left it green. `CardDelegate::chipSize()` now holds that
+decision and the test goes through it. This is the "rendering probes lie" rule
+in a new shape: the probe was fine, it was pointed at the wrong object.
+
+Chips are bottom-aligned so a smaller one shares a baseline with its neighbours
+rather than stepping down mid-row.
+
+**Message rows are unaffected**: a reply already draws only the tags its thread
+does not carry, and has no second tier to show.
+
+**Size: S.**
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 7b44e7e..944dfee 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
@@ -131,8 +131,8 @@ taking that too literally.
| 64 | The Sync button carries a mailbox icon, not a refresh one | presentation | XS | **done** 2026-08-11 |
| 65 | No full code review and optimization pass | correctness | ? | open, unspecified |
| 66 | Selecting a thread root leaves the message pane blank until a reply has been selected | defect | S | **done** 2026-08-14, unreleased. Not the blank pane it was filed as: the root rendered the CONVERSATION until the thread had been expanded once, then one message. Now always one message, and the conversation view is removed at the user's request. **One case unverified by hand:** the notes also report a single-message `id:` query whose card would not open, which is the same empty-`MessageIdRole` failure and should be gone; confirmed 2026-08-15 as a SEPARATE defect with a different cause, see item 96 |
-| 87 | Auto mark-read marks a whole thread, including replies never displayed | defect | S | open; measured 2026-08-14. Reachable only after 66 removed the conversation view. Blocked on 88 |
-| 88 | `threadAt(current.row())` answers about the wrong thread for a reply row | defect | S | open; found 2026-08-14 by shipping a fix that marked an unrelated message read. Row numbers are per parent in a tree |
+| 87 | Auto mark-read marks a whole thread, including replies never displayed | defect | S | **done** 2026-08-16, unreleased. Built on 108, which is why it stayed small: the timer tracks a MESSAGE id now, and arms for a reply too, which it never did before |
+| 88 | `threadAt(current.row())` answers about the wrong thread for a reply row | defect | M | **done** 2026-08-16, unreleased. The audit found FOUR live sites, not one. `ThreadListModel::threadFor(index)` resolves a reply through its parent; every caller holding a selected index converted, and no `.row()` on a selected index remains in `mainwindow.cpp`. Unblocks 87 |
| 67 | The placeholder pane counts unread, flagged and inbox, but not sent or drafts | information | XS | **done** 2026-08-11, shipped in 0.15.0 |
| 68 | A forwarded subject gets no `passed` tag | workflow | S | open; no subject rule exists, measured 2026-08-11. Decision needed: display mark (XS) or write the flag (S, syncs out) |
| 69 | `passed` and `replied` read as words where every other state is a glyph | presentation | S | **done** 2026-08-11, inside item 70 |
@@ -162,6 +162,20 @@ taking that too literally.
| 94 | `pinned` has nothing left to decide once the buttons are built-in | maintenance | S | open; **blocked on 93**, and deliberately not part of it. A user-visible removal: the row becomes built-ins only and every saved query lives in the menu |
| 96 | A query returning the thread already on display opens onto the placeholder | defect | S | **done** 2026-08-15, unreleased. Split from 66's unverified half, which had a different cause. Reproduced from two screenshots after four measured eliminations |
| 97 | An edit made during a sync is reverted in the list when the sync ends | defect | S | **done** 2026-08-15, unreleased. Found by hand-testing item 89's fix. The sync-end refresh ran BEFORE the held-edit flush, so it read a database that still carried the old tag |
+| 98 | "Important" adds the tag but cannot remove it, unlike every other toggle | defect | XS | open, found 2026-08-16 in the notes reconciliation |
+| 99 | The unread action is labelled "Toggle unread" whichever way it will go | presentation | S | open; depends on 98's toggle shape, and the label is harder than it looks |
+| 100 | The message pane offers Back, Forward, Reload and Save page, none of which mean anything | defect | XS | open, found 2026-08-16. Chromium's standard menu is added wholesale |
+| 101 | Sync is account-aware for edits but not for the account the user is looking at | workflow | S | open; item 49 built the edit half deliberately. Needs a decision, see the entry |
+| 102 | The rules table shows no note, so the field explaining a rule is invisible until it is opened | workflow | XS | open, found 2026-08-16 |
+| 103 | What Delete does to mail on the server is undocumented and unverified | clarification | S | open; a question first, possibly no code at all |
+| 104 | Mail visible in Thunderbird never reaches qtmaildir | defect | ? | open, reported 2026-08-16, cause NOT established. Most likely outside this repo; see the entry before writing code |
+| 109 | A root card's own message is invisible to a message-scoped write | defect | S | **done** 2026-08-16, unreleased. Found by hand-testing 108. `applyMessageTagChange` and `messageById` searched only the loaded replies, and a root's message is never among them, so the ORDINARY gesture repainted nothing and wiped the pane's chip row |
+| 110 | A card and the message pane show tags belonging to a message's siblings | defect | S | **done** 2026-08-16, unreleased. Found by hand-testing 109 against a real 4-message thread. `ThreadSummary::tags` is notmuch's UNION; a card standing for one message drew it. Also the reason a root card could not repaint at all |
+| 111 | A card should show its siblings' tags smaller, not drop them | presentation | S | **done** 2026-08-16, unreleased. The user's own design, from looking at 110's result: own tags full size, the thread's others smaller and muted, so nothing appears to vanish on selection |
+| 105 | Acting on a reply changes the counter and nothing on screen | defect | M | **done** 2026-08-16, unreleased. Found by hand-testing 88, and took three passes. FOUR causes: no optimistic update for a message-scoped write, no doomed cue on a reply row, both toggles reading the reply's THREAD state so they were one-way, and the message pane's strip not following a message edit. Also bolds an unread reply, at the user's request |
+| 106 | A tag change made on one message during a sync is silently lost | defect | XS | **done** 2026-08-16, unreleased. Found by READING while fixing 105, never reported. `flushHeldEdits` re-sent only thread-scoped edits, so a message-scoped one was shown, counted as pending, and never written |
+| 107 | A thread-scoped write leaves the loaded replies showing their old tags | defect | XS | **done** 2026-08-16, unreleased. `applyTagChange` updated the summary only, so marking a thread read left its expanded replies bold |
+| 108 | Acting on a thread root means the whole thread, though it displays one message | workflow | M | **done** 2026-08-16, unreleased. `messageScopeFor()` beside `scopeFor()`; five `*_thread` actions in a "Whole thread" submenu on `Ctrl+Alt+<key>`. User-visible: minor bump, `### Upgrading` written |
Sizes are rough: XS under an hour, S a sitting, M a session.
@@ -255,81 +269,6 @@ recorded.
**Size: `?`, unspecified.** Do not propose a design for this; ask.
-## 87. Auto mark-read marks a whole thread, including replies never displayed
-
-**Observed (user, 2026-08-14):** "with the first message in a thread selected
-(not expanded), the 2s delay that marks it read applies to the whole thread, so
-all answers are marked read as well."
-
-**Cause (verified in the code).** `markCurrentThreadRead`
-(`src/mainwindow.cpp`) sends `sendThreadTagChange` with the thread id, so
-`unread` is removed from every message in the thread.
-
-**This was coherent until item 66 and is not any more.** While a thread root
-rendered the whole conversation, everything marked read HAD been displayed.
-Item 66 made a root render one message; the thread-wide write stayed.
-
-**Not cosmetic.** `maildir.synchronize_flags` is on, so removing `unread`
-rewrites Maildir filenames and the next sync carries it to the server. Mail the
-user never opened stops being unread everywhere, and nothing here can put it
-back except reading the messages again by hand.
-
-**Approach: blocked on item 88, and that ordering is the lesson.** The obvious
-fix is to send `sendMessageTagChange` for the message on screen. That was
-written, tested, mutation-checked, shipped and reverted the same evening,
-because it exposed item 88: the guards above the write read
-`threadAt(current.row())`, which answers about an unrelated thread when a reply
-row is current, and a fix that narrows the write turns that mismatch into "a
-random message is now read". Fix 88 first.
-
-**Constraints for the second attempt.**
-
-- **Test selecting a REPLY, not only a root.** The reverted work asserted on a
- root selection, which is the one case where `current.row()` is correct, so a
- mutation-checked test passed against a bug that corrupted mail.
-- The card must repaint. `sendMessageTagChange` deliberately makes no
- optimistic model update, since `applyTagChange` is thread-keyed; without a
- message-scoped equivalent the write lands, the unsynced count rises, and the
- card stays bold, which the user reported as a separate fault.
-- A thread reads as unread while ANY message does, so the card can only stop
- looking unread when nothing else in it carries the tag. For an unexpanded
- thread the per-message tags are not loaded, so the honest answer needs
- per-message counts in `ThreadSummary` or nothing at all.
-
-**Size: S**, after 88.
-
-## 88. `threadAt(current.row())` answers about the wrong thread for a reply row
-
-**Observed (user, 2026-08-14):** "I clicked on a message and another random
-message was marked read."
-
-**Cause (verified).** `markCurrentThreadRead` calls
-`m_model->threadAt(current.row())`. A `QTreeView` numbers rows PER PARENT, so a
-reply's `row()` indexes its siblings: `threadAt(0)` on the first reply of any
-thread returns the FIRST THREAD IN THE LIST. The guards then compare correct ids
-against the wrong thread and let a write through.
-
-**`CLAUDE.md` already records this trap**, from item 20: "what did NOT survive
-that port is anything keyed on a row NUMBER". This is a surviving instance, not
-a new discovery, which is the uncomfortable part.
-
-**Mostly masked before item 87's attempted fix.** With a thread-wide write the
-mismatch usually resolved to the same thread or to a no-op; scoping the write to
-one message made it visible immediately, as unrelated mail being marked read.
-
-**Approach.** `threadAt(int)` takes a row and cannot be safe here. Every caller
-that has a `QModelIndex` should reach the thread through the index: for a
-message row, `ThreadListModel::threadIdForMessage()` or the parent index; for a
-thread row, `index.row()` is correct. Audit every `threadAt(` and every
-`.row()` in `mainwindow.cpp` rather than fixing this one site, since the same
-shape almost certainly appears elsewhere.
-
-**Consider deleting the row-taking overload** once the callers are converted, so
-the trap cannot be re-entered. That is the change that makes this permanent
-rather than fixed once.
-
-**Size: S** for the one site, **M** if the audit finds several.
-
## 68. A forwarded subject gets no `passed` tag
**Observed (user, from the notes):** "passed tag should appear when subject is
@@ -494,6 +433,330 @@ bump either way: an ignored optional field is not a breaking change.
**Size: S.** Removing a field, two UI affordances and their tests.
+## 98. "Important" adds the tag but cannot remove it, unlike every other toggle
+
+**Observed (user, from the notes):** "the add 'Important' action should be a
+toggle (like unread)."
+
+**Cause (verified in the code).** `src/mainwindow.cpp:863` registers `flag` as a
+one-way add:
+
+```cpp
+addAction(QStringLiteral("flag"), tr("&Important"),
+ tr("Mark the selected threads as important"), [this]() {
+ tagSelected({ QStringLiteral("flagged") }, {}, tr("Mark important"));
+});
+```
+
+Adding a tag that is already there is a no-op the user cannot see, so pressing
+the key or the button on an already-important thread appears to do nothing at
+all. Nothing in the UI removes `flagged` except the general tag dialog.
+
+**The two neighbouring actions are already toggles**, so this is an
+inconsistency rather than a missing feature. `delete`
+(`src/mainwindow.cpp:825`) and `toggle_unread` (`:867`) both read the current
+state and choose a direction, and `delete`'s comment states the rule this should
+follow: one direction for the WHOLE selection, flipping only when every selected
+thread is already in the target state, because a single keystroke that leaves a
+selection in two states is worse than either outcome.
+
+**Everything needed is already loaded.** `ThreadSummary::isFlagged()`
+(`src/types.h:64`) reads the tag off the summary, so the direction can be
+decided without a worker round trip, exactly as `isDeleted()` is.
+
+**Approach, now a two-line change.** Item 105 extracted
+`MainWindow::everySelectedRowHasTag()`, which is the whole of the direction
+logic:
+`everySelectedRowHasTag("flagged") ? tagSelected({}, {"flagged"}, tr("Unmark important")) : the current add`.
+The undo stack needs nothing new, since `TagChange::inverted()` already covers
+both directions.
+
+**Constraints.**
+
+- Call `everySelectedRowHasTag()`, never a hand-rolled loop. Two separate fixes
+ went into that logic on 2026-08-16 (items 88 and 105) and both were bugs a
+ copy of the then-current `delete` loop would have inherited: resolving a
+ reply's row number to the wrong thread, and asking a reply's thread instead
+ of the reply.
+- The action's tooltip says "Mark the selected threads as important" and would
+ become wrong. Item 99 is the same problem for `toggle_unread` and the two
+ should be decided together.
+- The label question belongs to item 99, not here. This item is the behaviour
+ only: the key stops being a no-op.
+
+**Size: XS.**
+
+## 99. The unread action is labelled "Toggle unread" whichever way it will go
+
+**Observed (user, from the notes):** "the label for 'toggle unread' should be
+dynamic: on an 'unread' message it should be 'Mark as read', on a 'read'
+message it should be 'Mark as unread'."
+
+**Cause (verified in the code).** `src/mainwindow.cpp:867` registers one static
+label, `tr("Toggle &unread")`, and the lambda decides the direction at
+invocation time from the current row. The action carries that text in three
+places at once: the Message menu (`:1060`), the thread context menu (`:1167`)
+and the toolbar (`:1122`, with the `mail-mark-unread` icon). Nothing updates it
+when the selection changes.
+
+**Not as simple as reading the current row**, which is why this is S and not XS.
+
+- The action applies to the WHOLE selection and picks one direction from the
+ current row, so with a mixed selection any label naming a single outcome is
+ either wrong for some rows or has to describe the rule ("Mark all as read").
+- A menu action's text is read when the menu opens, but a TOOLBAR button's text
+ is on screen continuously, so it has to track `selectionChanged` rather than
+ being computed at popup time. `currentRowChanged` is the wrong signal for
+ anything selection-shaped, per `CLAUDE.md`.
+- The accelerator is inside the word (`Toggle &unread`). Two different labels
+ need two accelerators chosen so neither collides in the Message menu, which
+ already holds "Mark &spam" and "&Important".
+- The shortcut list (Help > Keyboard shortcuts) and the config's `[keys]`
+ section both name the action `toggle_unread`. The action NAME must not change
+ with the label, or every user's config breaks. Same rule as item 57, which
+ changed "Flag" to "Important" on screen and left the action and tag alone.
+
+**Approach.** Compute the label from the same state the lambda already uses,
+which since item 105 is `MainWindow::everySelectedRowHasTag("unread")`, update
+it on `selectionChanged`, and keep a neutral fallback for an empty or mixed
+selection. Decide with item 98, which raises the identical question for
+"Important".
+
+**Use that helper rather than re-deriving the state**, or the label and the
+action can disagree. It already encodes the two things this gets wrong on its
+own: a reply answers about its MESSAGE, not its thread, and the answer is over
+the whole selection rather than the current row.
+
+**Constraints.** Every label is user-facing and needs `tr()`. Since the strings
+are chosen at runtime rather than written once, all of them must exist as
+literals `lupdate` can see; a string built by concatenation is not translatable.
+`ctest -R translations` is the check.
+
+**Size: S.** Mostly the mixed-selection and toolbar decisions, not the code.
+
+## 100. The message pane offers Back, Forward, Reload and Save page, none of which mean anything
+
+**Observed (user, from the notes):** "back/forward/save page in the right pane
+don't make sense, shouldn't be visible."
+
+**Cause (verified in the code).** `MessageView::showBodyContextMenu`
+(`src/messageview.cpp:619`) starts from Chromium's own menu:
+
+```cpp
+QMenu *menu = m_view->createStandardContextMenu();
+```
+
+That menu is built for a browser and carries the navigation and page actions
+whole. The pane is not a browser: every document arrives through `setHtml()`
+with a fixed base URL, so there is no history to go back to, nothing to reload
+from, and the request interceptor blocks everything by default anyway. The
+entries are inert as well as meaningless.
+
+**Deliberate as far as it goes.** The comment above the call says the page's own
+menu comes first so "copy, select all and the rest stay exactly as they were",
+which is right for the editing actions and wrong for the navigation ones. The
+item is that the filter was never applied, not that the base menu was a mistake.
+
+**Approach.** Keep the menu, drop the actions that cannot apply. Qt names them
+as `QWebEnginePage::WebAction` values (`Back`, `Forward`, `Reload`,
+`SavePage`, and `ViewSource` is worth the same look), and each has a
+`pageAction()` whose pointer can be matched against the standard menu's entries
+and removed. Removing by matching the action pointer is safer than matching by
+text, which is translated.
+
+**Constraints.**
+
+- Do not rebuild the menu from scratch. Copy, Copy link address and Select all
+ are the reason the standard menu is used, and item 85's search entries are
+ appended to it.
+- `Save page` is not the attachment save. Attachments have their own bar and
+ their own path-traversal checks (see the web view security notes in
+ `CLAUDE.md`); nothing here should grow a second way to write a file.
+- Verify against a real right-click on a real message. The offscreen platform
+ builds the menu but a screenshot of it proves nothing, and the entry list
+ depends on what the page reports as available at that moment.
+
+**Size: XS.**
+
+## 101. Sync is account-aware for edits but not for the account the user is looking at
+
+**Observed (user, from the notes):** "sync button should be account-aware."
+
+**Cause (verified in the code).** `MainWindow::pendingSyncChannels()`
+(`src/mainwindow.cpp:3550`) resolves channels from `m_editedAccounts`, the set of
+accounts the user has made EDITS in, and from nothing else. The account dropdown
+is not consulted. With nothing pending it returns empty on purpose, and
+`mailsync.sh` turns that into `mbsync -a`, every channel.
+
+**Item 49 built exactly this and the reasoning still holds.** The comment states
+it: with nothing pending the run is a FETCH, and narrowing a fetch to wherever
+the last edit happened would "quietly stop collecting mail everywhere else".
+Fetching is global by nature; carrying edits is not.
+
+**So this needs a decision, not a fix.** The note does not say which of two
+things the user means, and they are different features:
+
+*Sync only the selected account, on demand.* A deliberate "sync this account"
+that ignores the pending set, presumably beside the existing Sync rather than
+replacing it. Useful when one account is slow and the user wants their mail from
+another one now. The risk is the one item 49 named: a button that looks like
+Sync and quietly does not collect the rest of the mail.
+
+*Show which accounts a sync will cover.* No behaviour change at all, just making
+the existing account-awareness visible, since today the user cannot tell whether
+a run is narrowed or full. The status bar already names each channel as mbsync
+reaches it (item 42), so most of this exists.
+
+**Constraints.**
+
+- The account dropdown is a VIEW filter. Making it also steer sync couples two
+ things the user may reasonably want apart: looking at one account while
+ fetching all of them is the normal case, not an edge case.
+- Whatever narrows a run must still carry every pending edit, or an edit is
+ stranded with nothing on screen to say so. `pendingSyncChannels()` already
+ falls back to a full sync when it cannot resolve a channel for an edited
+ account, and that safety must survive.
+- An account with no `[account.<key>]` section, or one whose section names no
+ channel, has no channel to sync. The fallback covers it today.
+
+**Size: S** for the on-demand button, XS for the visibility half. Ask which.
+
+## 102. The rules table shows no note, so the field explaining a rule is invisible until it is opened
+
+**Observed (user, from the notes):** "add 'notes' column to the filters table."
+
+**Cause (verified in the code).** `TagRule` carries a `note` field
+(`src/tagrules.h:37`, "Why the rule is shaped this way. Shown in the dialog"),
+and the editor below the table edits it, but the table itself lists five columns
+and none of them is the note (`src/tagrulesdialog.cpp:116`):
+
+```cpp
+m_list->setHeaderLabels({ tr("On"), tr("Stage"), tr("Rule"), tr("Tags"),
+ tr("Matches") });
+```
+
+So the one field written specifically to explain a rule can only be read one rule
+at a time, by selecting it. With several rules the note is exactly the thing that
+would let the user pick the right one without opening each.
+
+**Approach.** A sixth column. The column widths already persist (item 75), so a
+new column needs a sensible default width and nothing else in the way of state.
+
+**Constraints.**
+
+- The note is free text of any length and would stretch the column. Elide it and
+ put the full text in the tooltip; the `Rule` column already faces the same
+ problem with a long query and is the pattern to match.
+- `ColumnCount + 1` in `setColumnCount` is load-bearing: the enum drives the
+ column indices and there is a spare. Add the enum value rather than hardcoding
+ 5, and check every place that indexes a column by number.
+- Notes are the user's own words and can be empty. An empty cell is correct
+ here; do not substitute a placeholder.
+
+**Size: XS.**
+
+## 103. What Delete does to mail on the server is undocumented and unverified
+
+**Observed (user, from the notes):** "verify how 'delete' works", with two
+sub-questions of their own: "trash bin (?)" and "delete from server (?)".
+
+**This is a question first.** The user is not reporting a defect; they are saying
+they do not know what the button does to their mail, which for a destructive
+action is its own problem regardless of the answer.
+
+**What the code does (verified).** `src/mainwindow.cpp:825` adds and removes the
+`deleted` tag, and nothing else. It is a toggle, it goes through the undo stack,
+and it writes a notmuch tag.
+
+**What that means downstream is what needs verifying, and it is NOT in this
+repo.** `maildir.synchronize_flags` is true, so notmuch maps certain tags to
+Maildir filename flags, and mbsync carries filename flags to the server. Whether
+`deleted` is one of those, whether the user's `~/.mbsyncrc` has `Expunge Both`
+(it does, on every channel), and what each provider does with a message flagged
+deleted, together decide whether this button is reversible. The undo stack makes
+the TAG reversible; it says nothing about what a sync did with it in between.
+
+**Approach.** Measure before designing anything: the notmuch config's
+`maildir.synchronize_flags` and its tag-to-flag mapping, one real message tagged
+and synced in a test account, and what the server shows afterwards. Then decide
+whether the UI needs to say what it does, whether "Delete" is even the right
+word for it, and whether a trash view is wanted.
+
+**Constraints.**
+
+- **This is the one place the no-confirmation rule should be re-examined rather
+ than assumed.** `CLAUDE.md` records that a human at a GUI gets undo instead of
+ confirmation dialogs, and that is right for tags. If the measurement shows the
+ next sync expunges mail from the server, then undo does not in fact cover this
+ action, and the premise the rule rests on does not hold for it.
+- Do not test this against the user's real accounts. A message that is expunged
+ to prove that it is expunged is still gone.
+- Any answer that involves a trash view is a much larger item and should be
+ split out rather than folded in here.
+
+**Size: S** for the investigation and whatever the UI needs to say. Unknown
+beyond that, and deliberately not sized further until the measurement exists.
+
+## 104. Mail visible in Thunderbird never reaches qtmaildir
+
+**Observed (user, from the notes):** "sync doesn't work compared to thunderbird.
+New mail received on thunderbird did not appear in qtmaildir. Need to investigate
+further."
+
+**Cause: NOT established.** Recorded because it is a defect report about mail
+going missing, which is the most serious kind this backlog carries, and it has
+been sitting in the notes unrecorded. What follows is one measured mechanism that
+would produce exactly this symptom, not a diagnosis.
+
+**qtmaildir cannot show what mbsync did not fetch, and mbsync fetches folders by
+pattern.** Three of the five channels in the user's `~/.mbsyncrc` name their
+folders explicitly:
+
+```
+Patterns "INBOX" "[Gmail]/Posta inviata" "[Gmail]/Bozze" "[Gmail]/Speciali"
+```
+
+and one names only `"INBOX"`. The two non-Gmail channels use `Patterns *`.
+Gmail applies labels, and a message whose label is not one of those four is in a
+folder mbsync never asks for. Thunderbird speaks IMAP directly and sees every
+folder, so the same message is visible there and absent locally. This is a
+configuration property of the user's mbsyncrc, outside this repository entirely.
+
+**One inconsistency worth reporting regardless**, found while checking the
+above: one of the Gmail accounts is configured in `qtmaildir.conf` with
+`sent = [Gmail]/Posta inviata` and `drafts = [Gmail]/Bozze`, while its mbsync
+channel has `Patterns "INBOX"` and fetches neither. The Sent and Drafts filters
+for that account can therefore only ever be empty. That is real, and it is
+independent of whatever this item turns out to be.
+
+**Approach.** Reproduce before anything else, and the reproduction has to
+distinguish three layers, because the fix lives in a different place for each:
+
+1. Is the message on disk? `find` in the Maildir, or `notmuch count` on a term
+ from it. If not, this is mbsync or `.mbsyncrc`, and there is nothing to
+ change here.
+2. If it is on disk, is it indexed? `notmuch new` and count again. If not, this
+ is notmuch config, `new.ignore` or the hook.
+3. Only if it is indexed and still not shown is this qtmaildir's defect, and
+ then the question is which query hid it: the account scope, the built-in
+ filter, or a rule that tagged it out of the inbox.
+
+**Constraints.**
+
+- Ask the user for one concrete example before investigating: which account,
+ roughly when, and what Thunderbird shows for it. A general "sync doesn't work"
+ cannot be reproduced, and the last four defects in this backlog were all found
+ from a specific message.
+- The `post-new` hook from mailctl tags mail unattended. A rule that removes
+ `inbox` would make a correctly fetched, correctly indexed message vanish from
+ the default view, which looks identical to a sync failure from the outside.
+ `notmuch search` without a filter is what tells them apart.
+- Do not change `.mbsyncrc` as part of this. It is the user's, it is outside the
+ repo, and a Patterns change refetches folders.
+
+**Size: `?`** until reproduced. Most likely not a code change here at all.
+
+
## Deferred, unsized, or split out
Items noted while triaging but not part of the original list. Same numbering
diff --git a/src/carddelegate.cpp b/src/carddelegate.cpp
index d7d06aa..4e27e6e 100644
--- a/src/carddelegate.cpp
+++ b/src/carddelegate.cpp
@@ -20,6 +20,7 @@
#include "cardlayout.h"
#include "marks.h"
+#include "tagchip.h"
#include "threadlistmodel.h"
#include <QApplication>
@@ -31,6 +32,12 @@
namespace {
+/// How much of a full-size chip's padding a SIBLING chip keeps.
+///
+/// Matched to CardLayout::siblingFont()'s own scale, so the chip shrinks as a
+/// whole rather than keeping full-size margins around smaller letters.
+constexpr qreal kSiblingPaddingScale = 0.70;
+
CardLayout::Input inputFor(const QModelIndex &index)
{
CardLayout::Input in;
@@ -58,6 +65,39 @@ QRect CardDelegate::expanderRectFor(const QStyleOptionViewItem &option,
.expanderRect;
}
+QSize CardDelegate::chipSize(const QFontMetrics &metrics, const QString &text,
+ bool own)
+{
+ // The padding shrinks with the font for a sibling chip. Left fixed it is
+ // 18px around roughly 30px of text, so the chip stays wide while its
+ // letters shrink and the tier reads as "same chip, smaller text".
+ return own ? TagChip::sizeFor(metrics, text)
+ : TagChip::sizeFor(metrics, text, kSiblingPaddingScale);
+}
+
+QColor CardDelegate::mutedChipColour(const QColor &chipColour)
+{
+ if (!chipColour.isValid())
+ return chipColour;
+
+ // Saturation only, and NOT a blend toward the background. The accent bar
+ // above records what blending toward Base costs: on a dark theme it lands
+ // on the background and the thing disappears. A chip is worse, because its
+ // fill also has to carry legible text on top of it.
+ //
+ // Hue is untouched, so a muted `signed` is still recognisably the same
+ // colour as a full-size `signed` elsewhere in the list. Lightness is
+ // untouched too, which is what keeps TagColors::textColourOn() picking the
+ // same text colour: draining saturation alone moves the fill toward grey
+ // without moving it toward either black or white, so contrast is preserved
+ // by construction rather than by hoping.
+ constexpr float kSaturationScale = 0.45f;
+
+ float h = 0, s = 0, l = 0, a = 0;
+ chipColour.getHslF(&h, &s, &l, &a);
+ return QColor::fromHslF(h, s * kSaturationScale, l, a);
+}
+
QColor CardDelegate::accentLineColour(const QColor &accountColour)
{
if (!accountColour.isValid())
@@ -304,20 +344,47 @@ void CardDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option,
: ThreadListModel::PillColoursRole)
.toList();
- const QFont chipFont = CardLayout::smallFont(chrome.font);
- const QFontMetrics chipMetrics(chipFont);
+ // A thread card draws its own tags at full size and the rest of the
+ // conversation's smaller and muted (item 111). The count is where the two
+ // tiers meet; a message row has no such split and reports its whole list.
+ //
+ // Shown rather than dropped, at the user's request: a card sits above a
+ // conversation, so what its siblings carry is worth seeing, just not at
+ // the same weight. Before the row has been opened everything is in the own
+ // tier, so a chip SHRINKS when the split becomes known and none vanishes.
+ const int ownCount =
+ isMessage ? tags.size()
+ : index.data(ThreadListModel::PillOwnCountRole).toInt();
+
+ const QFont ownFont = CardLayout::smallFont(chrome.font);
+ const QFont siblingFont = CardLayout::siblingFont(chrome.font);
+ const QFontMetrics ownMetrics(ownFont);
+ const QFontMetrics siblingMetrics(siblingFont);
+
painter->save();
- painter->setFont(chipFont);
int x = card.tagRect.left();
for (int i = 0; i < tags.size(); ++i) {
- const QSize size = TagChip::sizeFor(chipMetrics, tags.at(i));
+ const bool own = i < ownCount;
+ const QFontMetrics &metrics = own ? ownMetrics : siblingMetrics;
+
+ const QSize size = chipSize(metrics, tags.at(i), own);
if (x + size.width() > card.tagRect.right())
break; // Out of room; a clipped chip reads as a rendering fault.
- const QColor colour = i < colours.size()
- ? colours.at(i).value<QColor>()
- : QColor(0x55, 0x55, 0x5f);
- TagChip::paint(painter, QRect(QPoint(x, card.tagRect.top()), size),
- tags.at(i), colour);
+
+ QColor colour = i < colours.size() ? colours.at(i).value<QColor>()
+ : QColor(0x55, 0x55, 0x5f);
+ if (!own)
+ colour = mutedChipColour(colour);
+
+ // Bottom-aligned, so a smaller chip sits on the same baseline as its
+ // neighbours rather than floating in the middle of the row. Top
+ // alignment would step the tier down and read as a layout fault.
+ const int top = card.tagRect.top()
+ + (ownMetrics.height() - metrics.height());
+
+ painter->setFont(own ? ownFont : siblingFont);
+ TagChip::paint(painter, QRect(QPoint(x, top), size), tags.at(i),
+ colour);
x += size.width() + TagChip::kSpacing;
}
painter->restore();
diff --git a/src/carddelegate.h b/src/carddelegate.h
index 74dee8e..1846359 100644
--- a/src/carddelegate.h
+++ b/src/carddelegate.h
@@ -76,4 +76,22 @@ public:
///
/// Falls back to threadLineColour() for a thread with no account tag.
static QColor accentLineColour(const QColor &accountColour);
+
+ /// A tag chip's colour, drained for the SIBLING tier (item 111).
+ ///
+ /// Saturation only: hue stays so the tag is still recognisable, and
+ /// lightness stays so `TagColors::textColourOn()` keeps choosing the same
+ /// text colour and the chip cannot become unreadable. Exposed for a test,
+ /// since "muted" has to be asserted on rather than eyeballed.
+ static QColor mutedChipColour(const QColor &chipColour);
+
+ /// The size of one tag chip on a card, for either tier.
+ ///
+ /// The delegate's own arithmetic rather than a duplicate of it: a test
+ /// calling `TagChip::sizeFor` directly proves what that function does and
+ /// nothing about what the delegate ASKS for, which is where the padding
+ /// scale is chosen. A mutation dropping the scale at the call site
+ /// survived exactly that kind of test.
+ static QSize chipSize(const QFontMetrics &metrics, const QString &text,
+ bool own);
};
diff --git a/src/cardlayout.cpp b/src/cardlayout.cpp
index 0febb4b..3719591 100644
--- a/src/cardlayout.cpp
+++ b/src/cardlayout.cpp
@@ -107,6 +107,32 @@ QFont CardLayout::smallFont(const QFont &cardFont)
return small;
}
+QFont CardLayout::siblingFont(const QFont &cardFont)
+{
+ // A FRACTION of the card's font, not a fixed number of points off it.
+ //
+ // Subtracting one point was the first attempt and the user reported the
+ // tiers as indistinguishable. The reason is arithmetic: their desktop is
+ // 14pt, so the two chip tiers were 13 and 12, a 7% step. Subtraction gives
+ // a step whose size depends on the desktop font, which is exactly backwards
+ // — it is largest where the text is already small enough to be fragile.
+ //
+ // 0.70 of the card font, against smallFont()'s one point off, so on a 14pt
+ // desktop the tiers are 13 and 10. Chosen with the user against rendered
+ // sizes rather than picked.
+ constexpr qreal kSiblingScale = 0.70;
+
+ QFont small = cardFont;
+ // pointSizeF() returns -1 for a font set in PIXELS, which qt6ct does, and
+ // scaling -1 asks for an invalid size that Qt silently ignores, leaving
+ // both tiers identical. Same split as smallFont(), same reason.
+ if (small.pointSizeF() > 0.0)
+ small.setPointSizeF(qMax(6.0, cardFont.pointSizeF() * kSiblingScale));
+ else if (small.pixelSize() > 0)
+ small.setPixelSize(qMax(8, qRound(cardFont.pixelSize() * kSiblingScale)));
+ return small;
+}
+
int CardLayout::heightFor(const QFont &font)
{
const QFontMetrics metrics(font);
diff --git a/src/cardlayout.h b/src/cardlayout.h
index 3512242..de4c41f 100644
--- a/src/cardlayout.h
+++ b/src/cardlayout.h
@@ -160,6 +160,19 @@ struct CardLayout
/// column of content.
static QFont smallFont(const QFont &cardFont);
+ /// The font a SIBLING's tag chip is drawn in: a size down again from
+ /// smallFont().
+ ///
+ /// A card stands for one message but sits above a conversation, and shows
+ /// both tiers (item 111). Size is what says which is which, so the two
+ /// must be visibly different; taking one more step from the same base
+ /// keeps it following the desktop's font rather than being fixed.
+ ///
+ /// Floored like smallFont(), and the floor really is reachable: a desktop
+ /// at the minimum size gives both tiers the same size, which is a legible
+ /// degradation rather than an illegible chip.
+ static QFont siblingFont(const QFont &cardFont);
+
static CardLayout compute(const Input &input, const QRect &rect,
const QFont &font);
diff --git a/src/keymap.cpp b/src/keymap.cpp
index f6ef6a4..c731bbb 100644
--- a/src/keymap.cpp
+++ b/src/keymap.cpp
@@ -37,6 +37,21 @@ QStringList KeyMap::knownActions()
QStringLiteral("edit_tags"),
QStringLiteral("tag_rules"),
QStringLiteral("flag"),
+ // The whole-thread counterparts (item 108). The names above act on the
+ // message a row displays; these act on its entire thread. Separate
+ // names rather than a scope flag, because a name is what a user writes
+ // in [keys]: giving `delete` new semantics would silently change an
+ // existing config, and renaming it would break one that mentions it.
+ //
+ // Unbound by default. They are reached through the "Whole thread"
+ // submenu, and inventing five more default chords for actions most
+ // users will rarely want is worse than leaving them to bind what they
+ // use.
+ QStringLiteral("archive_thread"),
+ QStringLiteral("delete_thread"),
+ QStringLiteral("spam_thread"),
+ QStringLiteral("toggle_unread_thread"),
+ QStringLiteral("flag_thread"),
QStringLiteral("focus_query"),
QStringLiteral("complete_query"),
QStringLiteral("save_query"),
@@ -90,6 +105,20 @@ QList<QPair<QString, QString>> KeyMap::defaultBindings()
// action takes the harder chord rather than the easier one.
{ QStringLiteral("Ctrl+Shift+U"), QStringLiteral("mark_all_read") },
{ QStringLiteral("Ctrl+I"), QStringLiteral("flag") },
+ // The whole-thread tier (item 108), one modifier out from each
+ // message-scoped twin: Ctrl+D deletes the message a row displays,
+ // Ctrl+Alt+D deletes its conversation.
+ //
+ // Ctrl+ALT, not Ctrl+Shift. The obvious pairing is taken twice over:
+ // Ctrl+Shift+S is `spam` and Ctrl+Shift+U is `mark_all_read`, both
+ // shipped and both in users' fingers. Reusing either would silently
+ // change what an existing key does, which is the same objection that
+ // made these separate action names rather than a flag on the old ones.
+ { QStringLiteral("Ctrl+Alt+E"), QStringLiteral("archive_thread") },
+ { QStringLiteral("Ctrl+Alt+D"), QStringLiteral("delete_thread") },
+ { QStringLiteral("Ctrl+Alt+S"), QStringLiteral("spam_thread") },
+ { QStringLiteral("Ctrl+Alt+U"), QStringLiteral("toggle_unread_thread") },
+ { QStringLiteral("Ctrl+Alt+I"), QStringLiteral("flag_thread") },
{ QStringLiteral("Ctrl+T"), QStringLiteral("edit_tags") },
// Shifted against Ctrl+T for the same reason Ctrl+Shift+U is shifted
// against Ctrl+U: this is the standing version of tagging, applied to
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index fcf97df..ee883b0 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -832,15 +832,14 @@ void MainWindow::registerActions()
// independently would leave one keystroke with the selection in two
// states, which is worse than either outcome, so undelete only when
// every selected thread is already deleted.
- const QModelIndexList rows =
- m_threadView->selectionModel()->selectedRows();
- bool allDeleted = !rows.isEmpty();
- for (const QModelIndex &index : rows) {
- if (!m_model->threadAt(index.row()).isDeleted()) {
- allDeleted = false;
- break;
- }
- }
+ //
+ // Each row's own state, message or thread: a reply row is asked about
+ // the MESSAGE it stands for. Asking its thread made Delete one-way on
+ // a reply, since a message-scoped write never changes the thread's
+ // tags and the answer therefore stayed "not deleted" however many
+ // times it was pressed. Item 88 fixed which thread was read here; this
+ // is about reading a message at all.
+ const bool allDeleted = everySelectedRowHasTag(QStringLiteral("deleted"));
if (allDeleted)
tagSelected({}, { QStringLiteral("deleted") }, tr("Undelete"));
@@ -866,21 +865,26 @@ void MainWindow::registerActions()
});
addAction(QStringLiteral("toggle_unread"), tr("Toggle &unread"),
tr("Toggle the unread tag"), [this]() {
- // The direction comes from the current row, but the change applies to
- // the whole selection, so a mixed selection lands in one consistent
- // state rather than each row flipping its own way.
- const QModelIndex current = m_threadView->currentIndex();
- if (!current.isValid())
- return;
- const ThreadSummary thread = m_model->threadAt(current.row());
+ // The state of whatever the rows STAND FOR, which for a reply is the
+ // message and not its thread. See everySelectedRowHasTag(): reading
+ // the thread here made the key dead on a reply.
+ //
+ // Item 88 fixed WHICH thread this read. That was necessary and not
+ // sufficient: a reply needs a message read, not a better thread.
+ //
+ // Per selection rather than per current row, matching Delete. The old
+ // 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"));
// An explicit toggle overrides the automatic one. Without this, marking
// a thread unread by hand would be undone a moment later by a timer
// armed when it was opened, and the key would look broken.
m_markReadTimer->stop();
- m_markReadThreadId.clear();
+ m_markReadMessageId.clear();
- if (thread.isUnread())
+ if (unread)
tagSelected({}, { QStringLiteral("unread") }, tr("Mark read"));
else
tagSelected({ QStringLiteral("unread") }, {}, tr("Mark unread"));
@@ -894,6 +898,58 @@ void MainWindow::registerActions()
tr("Add or remove any tag on the selected threads"), [this]() {
editTagsOnSelection();
});
+
+ // The whole-thread counterparts (item 108). Separate action NAMES, because
+ // a name is what a user writes in [keys]: reusing `delete` with new
+ // semantics would silently change what an existing config does, and
+ // renaming it would break one that mentions it. These are unbound by
+ // default; the submenu is how they are reached.
+ //
+ // Each one is its message-scoped twin with TagScope::Thread, so the two
+ // cannot drift in what they write, only in what they write it to.
+ addAction(QStringLiteral("archive_thread"), tr("&Archive thread"),
+ tr("Remove inbox from every message of the selected threads"),
+ [this]() {
+ tagSelected({}, { QStringLiteral("inbox") }, tr("Archive thread"),
+ TagScope::Thread);
+ });
+ addAction(QStringLiteral("delete_thread"), tr("&Delete thread"),
+ tr("Add or remove the deleted tag on whole threads"), [this]() {
+ if (everySelectedRowHasTag(QStringLiteral("deleted"), TagScope::Thread)) {
+ tagSelected({}, { QStringLiteral("deleted") },
+ tr("Undelete thread"), TagScope::Thread);
+ } else {
+ tagSelected({ QStringLiteral("deleted") }, {}, tr("Delete thread"),
+ TagScope::Thread);
+ }
+ });
+ addAction(QStringLiteral("spam_thread"), tr("Mark thread as &spam"),
+ tr("Add spam and remove inbox on whole threads"), [this]() {
+ tagSelected({ QStringLiteral("spam") }, { QStringLiteral("inbox") },
+ tr("Mark thread spam"), TagScope::Thread);
+ });
+ addAction(QStringLiteral("toggle_unread_thread"), tr("Toggle &unread"),
+ tr("Toggle the unread tag on whole threads"), [this]() {
+ // Cancels the automatic mark-read for the same reason its
+ // message-scoped twin does: a thread marked unread by hand must not be
+ // undone a moment later by a timer armed when it was opened.
+ m_markReadTimer->stop();
+ m_markReadMessageId.clear();
+
+ if (everySelectedRowHasTag(QStringLiteral("unread"), TagScope::Thread)) {
+ tagSelected({}, { QStringLiteral("unread") },
+ tr("Mark thread read"), TagScope::Thread);
+ } else {
+ tagSelected({ QStringLiteral("unread") }, {},
+ tr("Mark thread unread"), TagScope::Thread);
+ }
+ });
+ addAction(QStringLiteral("flag_thread"), tr("&Important"),
+ tr("Mark every message of the selected threads as important"),
+ [this]() {
+ tagSelected({ QStringLiteral("flagged") }, {},
+ tr("Mark thread important"), TagScope::Thread);
+ });
addAction(QStringLiteral("tag_rules"), tr("Tagging &rules..."),
tr("Edit the rules that tag mail as it arrives"), [this]() {
showTagRulesDialog();
@@ -975,7 +1031,7 @@ void MainWindow::registerActions()
m_messageView->clear();
showPlaceholderPane();
m_markReadTimer->stop();
- m_markReadThreadId.clear();
+ m_markReadMessageId.clear();
});
addAction(QStringLiteral("clear_selection"), tr("Clear &selection"),
tr("Blank the message pane and deselect every thread"),
@@ -1010,7 +1066,7 @@ void MainWindow::registerActions()
m_messageView->clear();
showPlaceholderPane();
m_markReadTimer->stop();
- m_markReadThreadId.clear();
+ m_markReadMessageId.clear();
});
addAction(QStringLiteral("select_all"), tr("Select &all threads"),
tr("Select every thread in the current result list"), [this]() {
@@ -1061,6 +1117,8 @@ void MainWindow::buildMenus()
messageMenu->addAction(m_actions.value(QStringLiteral("mark_all_read")));
messageMenu->addAction(m_actions.value(QStringLiteral("edit_tags")));
messageMenu->addAction(m_actions.value(QStringLiteral("flag")));
+ messageMenu->addSeparator();
+ messageMenu->addMenu(buildThreadActionsMenu(messageMenu));
// Separated from the entries above: those act on the selection, this edits
// a rule store shared with mailctl and changes nothing that is on screen.
messageMenu->addSeparator();
@@ -1141,6 +1199,20 @@ void MainWindow::buildMenus()
{ QStringLiteral("zoom_in"), QStringLiteral("zoom-in") },
{ QStringLiteral("zoom_out"), QStringLiteral("zoom-out") },
{ QStringLiteral("zoom_reset"), QStringLiteral("zoom-original") },
+
+ // The whole-thread tier (item 108) deliberately SHARES each icon with
+ // its message-scoped twin. The no-duplicates rule exists because the
+ // toolbar can be icon-only, where the icon is the entire control;
+ // these five never reach the toolbar. They live in a submenu whose
+ // entries always carry text, and "Delete thread" beside the delete
+ // icon is the honest pairing: the same operation, a wider scope, with
+ // the words saying which. Inventing five different shapes for the same
+ // five operations would be less clear, not more.
+ { QStringLiteral("archive_thread"), QStringLiteral("mail-archive") },
+ { QStringLiteral("delete_thread"), QStringLiteral("edit-delete") },
+ { QStringLiteral("spam_thread"), QStringLiteral("mail-mark-junk") },
+ { QStringLiteral("toggle_unread_thread"), QStringLiteral("mail-mark-unread") },
+ { QStringLiteral("flag_thread"), QStringLiteral("mail-mark-important") },
};
for (auto it = themeIcons.cbegin(); it != themeIcons.cend(); ++it) {
QAction *action = m_actions.value(it.key());
@@ -1168,6 +1240,8 @@ void MainWindow::buildMenus()
m_threadContextMenu->addAction(m_actions.value(QStringLiteral("flag")));
m_threadContextMenu->addAction(m_actions.value(QStringLiteral("edit_tags")));
m_threadContextMenu->addSeparator();
+ m_threadContextMenu->addMenu(buildThreadActionsMenu(m_threadContextMenu));
+ m_threadContextMenu->addSeparator();
m_threadContextMenu->addAction(m_actions.value(QStringLiteral("select_all")));
m_threadView->setContextMenuPolicy(Qt::CustomContextMenu);
@@ -2384,7 +2458,7 @@ void MainWindow::markAllRead()
// An automatic mark-read armed for the open thread would fire after this
// and push a second, redundant command onto the stack.
m_markReadTimer->stop();
- m_markReadThreadId.clear();
+ m_markReadMessageId.clear();
const QString description = tr("Mark all read");
sendThreadTagChange(threadIds, {}, { QStringLiteral("unread") },
@@ -2448,16 +2522,16 @@ void MainWindow::onSelectionChanged()
// Qt 6.11), so that handler still sees the old count and returns
// without loading anything.
//
- // Compared per row kind. A message row's row number indexes its
- // siblings, so threadAt() on one answers about an unrelated thread and
- // the comparison below would be against the wrong id.
+ // Compared per row kind: a message row is identified by its message id
+ // and a thread row by its thread id, which are different questions.
+ // threadFor() resolves the thread either way, so the row-number trap
+ // (item 88) cannot be re-entered here even if this branch changes.
const QModelIndex current = m_threadView->currentIndex();
if (current.isValid()) {
const bool changed =
m_model->isMessageRow(current)
? m_model->messageAt(current).messageId != m_currentMessageId
- : m_model->threadAt(current.row()).threadId
- != m_currentThreadId;
+ : m_model->threadFor(current).threadId != m_currentThreadId;
if (changed)
onThreadSelected(current, QModelIndex());
}
@@ -2507,7 +2581,7 @@ void MainWindow::onSelectionChanged()
// current, so onThreadSelected never runs and its guard never fires. The
// pane and the pending timer have to be dealt with here as well.
m_markReadTimer->stop();
- m_markReadThreadId.clear();
+ m_markReadMessageId.clear();
m_currentThreadId.clear();
m_currentMessageId.clear();
m_currentMessageThreadId.clear();
@@ -2563,7 +2637,7 @@ void MainWindow::onThreadSelected(const QModelIndex &current,
// pane that no longer shows the thread.
if (m_threadView->selectionModel()->selectedRows().size() > 1) {
m_markReadTimer->stop();
- m_markReadThreadId.clear();
+ m_markReadMessageId.clear();
m_currentThreadId.clear();
m_currentMessageId.clear();
m_currentMessageThreadId.clear();
@@ -2572,21 +2646,22 @@ void MainWindow::onThreadSelected(const QModelIndex &current,
return;
}
- // A message row renders that message ALONE. Checked before threadAt(),
- // which takes a top-level row number: a child's row number indexes its
- // siblings, so passing it here would silently load whichever thread happens
- // to sit at that position in the list.
+ // A message row renders that message ALONE, so the kind of row still has
+ // to be checked here: this is a different render path, not a different way
+ // of naming the same thread.
if (m_model->isMessageRow(current)) {
const MessageNode node = m_model->messageAt(current);
if (node.messageId.isEmpty())
return;
- // No mark-read timer for a message row in this pass. Marking one
- // message of a thread read is a per-message tag write, and the
- // pending-edit map is keyed by thread; item 28 is the record of what
- // happens when that count goes wrong.
+ // Armed for a reply too, since item 87. It deliberately was not
+ // before, because the write was thread-scoped and reading one reply
+ // would have marked the whole conversation read. With the write scoped
+ // to one message that objection is gone, and leaving it unarmed would
+ // make the message the user is actually reading the one kind that
+ // never gets marked read.
m_markReadTimer->stop();
- m_markReadThreadId.clear();
+ m_markReadMessageId.clear();
m_currentThreadId.clear();
m_currentMessageId = node.messageId;
@@ -2594,16 +2669,27 @@ void MainWindow::onThreadSelected(const QModelIndex &current,
// thread it came from is what the refreshed list is checked against.
m_currentMessageThreadId = node.threadId;
m_messageView->setTags(node.tags);
+
+ // After m_currentMessageId is set: the handler compares against it to
+ // tell "still showing this" from "the selection moved on".
+ scheduleMarkRead(node.messageId, node.isUnread());
+
QMetaObject::invokeMethod(m_worker, "loadMessage", Qt::QueuedConnection,
Q_ARG(QString, node.messageId),
Q_ARG(quint64, m_generation));
return;
}
- const ThreadSummary thread = m_model->threadAt(current.row());
+ const ThreadSummary thread = m_model->threadFor(current);
m_currentThreadId = thread.threadId;
m_messageView->setTags(thread.tags);
- scheduleMarkRead(thread);
+
+ // The message the card displays, not the thread. The summary's `unread` is
+ // a union over the conversation, so this can arm for a thread whose first
+ // message is already read; the write is scoped to that message either way,
+ // so the cost is a no-op rather than a wrong write. Narrowing it properly
+ // needs per-message state in ThreadSummary, which nothing carries yet.
+ scheduleMarkRead(thread.firstMessageId, thread.isUnread());
// The root card IS the thread's first message, so selecting it renders
// that message. Never the whole conversation: that path is gone (item 66).
@@ -2654,6 +2740,25 @@ void MainWindow::onMessageLoaded(const QVector<MessageRef> &messages,
if (m_currentMessageId.isEmpty())
return;
+ // The worker's answer is the authority on what THIS message carries, and
+ // it is the only place that truth arrives. Until it does, a thread row can
+ // only offer ThreadSummary::tags, which is notmuch's union over the
+ // conversation: a four-message thread whose third message is signed makes
+ // the root card and the pane both claim `signed` for a message that is not
+ // (item 110). Recording it here corrects the card and gives a
+ // message-scoped write something to update, which is why marking a root
+ // message read left the row bold before.
+ //
+ // A reply already has its own node from the thread tree, and
+ // setRootMessageTags ignores anything that is not a root.
+ for (const MessageRef &ref : messages)
+ m_model->setRootMessageTags(ref.messageId, ref.tags);
+
+ // The pane follows the same correction. setTags() at selection time can
+ // only have used the union.
+ if (messages.size() == 1)
+ m_messageView->setTags(messages.first().tags);
+
renderMessages(messages);
}
@@ -2741,7 +2846,12 @@ void MainWindow::renderMessages(const QVector<MessageRef> &messages)
void MainWindow::revertPendingTagChange()
{
- if (m_pendingThreadIds.isEmpty())
+ // Either scope can be in flight: a thread-scoped write names threads, a
+ // message-scoped one names messages, and both are now applied
+ // optimistically. Checking only the thread ids left a failed message write
+ // showing its optimistic state for good, with nothing to correct it until
+ // the next query.
+ if (m_pendingThreadIds.isEmpty() && m_pendingChange.messageIds.isEmpty())
return;
// Put the rows back the way they were. Only the model is touched: the
@@ -2750,6 +2860,10 @@ void MainWindow::revertPendingTagChange()
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.
@@ -2805,18 +2919,37 @@ void MainWindow::flushHeldEdits()
m_flushGeneration = m_generation;
for (const HeldEdit &edit : edits) {
- // Take the optimistic update back before sending, because
- // sendThreadTagChange() applies it again. applyTagChange() is
- // idempotent per tag so the 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.
+ // Take the optimistic update back before sending, because the send
+ // applies it again. Both apply functions are idempotent per tag so the
+ // 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);
+ }
- sendThreadTagChange(edit.threadIds, edit.change.added,
- edit.change.removed, edit.change.description);
+ // By SCOPE. A held edit is one or the other, never both: a
+ // message-scoped edit carries no thread ids, so sending it through
+ // sendThreadTagChange() sent an empty list, which returns immediately.
+ // The edit was applied to the row, counted as unsynced and then
+ // dropped without ever being written, which is data loss with a
+ // pending count claiming the opposite.
+ //
+ // Escalating it to its thread instead would be worse: Delete on one
+ // reply would delete every message in the conversation.
+ if (!edit.threadIds.isEmpty()) {
+ sendThreadTagChange(edit.threadIds, edit.change.added,
+ edit.change.removed, edit.change.description);
+ }
+ if (!edit.change.messageIds.isEmpty()) {
+ sendMessageTagChange(edit.change.messageIds, edit.change.added,
+ edit.change.removed, edit.change.description);
+ }
}
// Held edits stop counting as held; what counts now is whatever
@@ -3149,7 +3282,7 @@ void MainWindow::onRowDoubleClicked(const QModelIndex &index)
// the recovery selects. What is cancelled is the arming for a row the user
// is leaving.
m_markReadTimer->stop();
- m_markReadThreadId.clear();
+ m_markReadMessageId.clear();
// Reuses the stale-thread recovery outright, which already runs thread:<id>,
// expands the thread when the row arrives, selects the target message once
@@ -3604,27 +3737,32 @@ void MainWindow::updatePendingIndicator()
m_pendingLabel->show();
}
-void MainWindow::scheduleMarkRead(const ThreadSummary &thread)
+void MainWindow::scheduleMarkRead(const QString &messageId, bool unread)
{
- // Any pending timer belongs to a thread that is no longer on screen.
+ // Any pending timer belongs to a message that is no longer on screen.
// Stopping unconditionally is what makes this a restart rather than a
- // stack: arrowing down ten threads must mark only the one still selected
+ // stack: arrowing down ten rows must mark only the one still selected
// when the timer finally fires.
m_markReadTimer->stop();
- m_markReadThreadId.clear();
+ m_markReadMessageId.clear();
// Negative disables the behaviour entirely, per the config key.
const int delay = m_config.markReadDelayMs();
if (delay < 0)
return;
- // Nothing to do for a thread that is already read. Checked here rather
- // than in the handler so no timer is even armed, which keeps a read thread
- // from arming one that would fire into a no-op write.
- if (!thread.tags.contains(QStringLiteral("unread")))
+ // A row the model cannot name a message for. Marking its thread instead
+ // would be the escalation item 108 removed.
+ if (messageId.isEmpty())
+ return;
+
+ // Nothing to do for a message that is already read. Checked here rather
+ // than in the handler so no timer is even armed, which keeps a read
+ // message from arming one that would fire into a no-op write.
+ if (!unread)
return;
- m_markReadThreadId = thread.threadId;
+ m_markReadMessageId = messageId;
// Zero means immediately, and a zero-interval timer still fires through
// the event loop rather than reentering the selection handler.
@@ -3633,62 +3771,167 @@ void MainWindow::scheduleMarkRead(const ThreadSummary &thread)
void MainWindow::markCurrentThreadRead()
{
- if (m_markReadThreadId.isEmpty())
+ if (m_markReadMessageId.isEmpty())
return;
// The selection can have moved on between the timer being armed and it
- // firing, and the thread can have been marked read by hand in that window.
- // Both mean this timer has nothing left to do.
- if (m_markReadThreadId != m_currentThreadId) {
- m_markReadThreadId.clear();
- return;
- }
-
- const QModelIndex current = m_threadView->currentIndex();
- if (!current.isValid()) {
- m_markReadThreadId.clear();
- return;
- }
-
- const ThreadSummary thread = m_model->threadAt(current.row());
- if (thread.threadId != m_markReadThreadId
- || !thread.tags.contains(QStringLiteral("unread"))) {
- m_markReadThreadId.clear();
+ // firing, and the message can have been marked read by hand in that
+ // window. Both mean this timer has nothing left to do.
+ //
+ // Compared against what the PANE is showing rather than against the
+ // selection: those are the same thing for both kinds of row, and the pane
+ // is what "the message the user is reading" means.
+ const QString showing = m_currentMessageId.isEmpty()
+ ? currentThreadFirstMessageId()
+ : m_currentMessageId;
+ if (m_markReadMessageId != showing) {
+ m_markReadMessageId.clear();
return;
}
- const QStringList threadIds = { m_markReadThreadId };
- m_markReadThreadId.clear();
+ const QStringList messageIds = { m_markReadMessageId };
+ m_markReadMessageId.clear();
- // sendThreadTagChange, NOT tagSelected: this deliberately does not go on
+ // sendMessageTagChange, NOT tagSelected: this deliberately does not go on
// the undo stack. The user never took this action, so hijacking Ctrl+Z to
// reverse it would undo something they did not do, and toggle_unread
// already gives them a direct way to put it back. Decided 2026-08-03.
//
+ // MESSAGE-scoped since item 87. The thread-wide write was coherent while a
+ // root card rendered the whole conversation; item 66 made it render one
+ // message and left the write alone, so reading one message marked replies
+ // read that had never been displayed. maildir.synchronize_flags is on, so
+ // that reached the server and nothing here could put it back.
+ //
// 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.
- sendThreadTagChange(threadIds, {}, { QStringLiteral("unread") },
- tr("Mark read"));
+ sendMessageTagChange(messageIds, {}, { QStringLiteral("unread") },
+ tr("Mark read"));
}
-void MainWindow::editTagsOnSelection()
+QString MainWindow::currentThreadFirstMessageId() const
+{
+ // The message a selected THREAD row displays. m_currentThreadId is what
+ // the pane was opened from, so this resolves through the model rather than
+ // through the selection, which can have moved.
+ if (m_currentThreadId.isEmpty())
+ return {};
+
+ for (int row = 0; row < m_model->rowCount(QModelIndex()); ++row) {
+ const ThreadSummary thread = m_model->threadAt(row);
+ if (thread.threadId == m_currentThreadId)
+ return thread.firstMessageId;
+ }
+ return {};
+}
+
+bool MainWindow::everySelectedRowHasTag(const QString &tag,
+ TagScope scope) const
{
+ // What a toggle asks before choosing its direction, for both Delete and
+ // Toggle unread.
+ //
+ // Per ROW, and each row is asked about what it stands for: a reply row
+ // reports the message's tags, a thread row the thread's. Asking a reply's
+ // THREAD is the trap both toggles fell into. The write is message-scoped,
+ // so it never changes the thread's tags; the thread's answer therefore
+ // never moves however many times the key is pressed, and the toggle
+ // becomes one-way. On the second press it re-sends a tag the message
+ // already has, which is a no-op, and a no-op repaints nothing.
+ //
+ // One direction for the WHOLE selection, which is the rule Delete
+ // established: toggling each row independently would leave one keystroke
+ // with the selection in two states, which is worse than either outcome.
const QModelIndexList rows =
m_threadView->selectionModel()->selectedRows();
- if (rows.isEmpty()) {
- showTransientStatus(tr("Select a thread first"));
- return;
+ if (rows.isEmpty())
+ return false;
+
+ for (const QModelIndex &index : rows) {
+ QStringList tags;
+ if (scope == TagScope::Thread) {
+ tags = m_model->threadFor(index).tags;
+ } else if (m_model->isMessageRow(index)) {
+ tags = m_model->messageAt(index).tags;
+ } else {
+ // The thread's summary, and this is a KNOWN approximation rather
+ // than an oversight. A thread row acts on the message its card
+ // displays, but that message's own tags are never in the model:
+ // setThreadMessages drops depth 0 because the root row stands for
+ // it, so there is no node to read and messageById() cannot find
+ // one. The summary is a union over the thread, so it answers
+ // "unread" while ANY message is.
+ //
+ // The consequence is bounded and only affects the DIRECTION a
+ // toggle picks, never what it writes: on a thread whose first
+ // message is read while a later one is not, Toggle unread reads
+ // the thread as unread and marks the first message read again, a
+ // no-op. Fixing it properly needs per-message state in
+ // ThreadSummary, which is the same thing item 87 needs; leave it
+ // for that item rather than guessing here.
+ tags = m_model->threadFor(index).tags;
+ }
+ if (!tags.contains(tag))
+ return false;
}
+ return true;
+}
+
+ThreadSummary MainWindow::threadForCurrentRowForTesting() const
+{
+ return m_model->threadFor(m_threadView->currentIndex());
+}
- // How many of the selected threads carry each tag, which is what tells a
- // tag that is on all of them from one that is on some.
+QMenu *MainWindow::buildThreadActionsMenu(QWidget *parent)
+{
+ // Built per call rather than shared. A QMenu belongs to one place in one
+ // menu tree, and adding the same instance to both the menu bar and the
+ // context menu gives whichever added it last the object. The ACTIONS are
+ // shared, which is what has to stay consistent; the menu holding them is
+ // just a container.
+ auto *menu = new QMenu(tr("&Whole thread"), parent);
+ menu->setObjectName(QStringLiteral("threadActionsMenu"));
+ menu->addAction(m_actions.value(QStringLiteral("archive_thread")));
+ menu->addAction(m_actions.value(QStringLiteral("delete_thread")));
+ menu->addAction(m_actions.value(QStringLiteral("spam_thread")));
+ menu->addSeparator();
+ menu->addAction(m_actions.value(QStringLiteral("toggle_unread_thread")));
+ menu->addAction(m_actions.value(QStringLiteral("flag_thread")));
+ return menu;
+}
+
+QHash<QString, int> MainWindow::selectionTagCounts() const
+{
+ // How many of the selected rows carry each tag, which is what tells a tag
+ // that is on all of them from one that is on some. The dialog's tri-state
+ // checkboxes are built from this, so a wrong count offers to remove a tag
+ // the selection does not have.
+ //
+ // threadFor(index), NOT threadAt(index.row()): a reply's row number named
+ // an unrelated thread, so selecting one counted the tags of whichever
+ // thread sat at that position in the list (item 88).
QHash<QString, int> counts;
+ const QModelIndexList rows =
+ m_threadView->selectionModel()->selectedRows();
for (const QModelIndex &index : rows) {
- const ThreadSummary thread = m_model->threadAt(index.row());
+ const ThreadSummary thread = m_model->threadFor(index);
for (const QString &tag : thread.tags)
counts[tag] += 1;
}
+ return counts;
+}
+
+void MainWindow::editTagsOnSelection()
+{
+ const QModelIndexList rows =
+ m_threadView->selectionModel()->selectedRows();
+ if (rows.isEmpty()) {
+ showTransientStatus(tr("Select a thread first"));
+ return;
+ }
+
+ const QHash<QString, int> counts = selectionTagCounts();
// m_knownTags is the same list the query completer uses, so the dialog
// offers every tag in the database without a round trip.
@@ -3708,7 +3951,7 @@ void MainWindow::editTagsOnSelection()
}
void MainWindow::tagSelected(const QStringList &add, const QStringList &remove,
- const QString &description)
+ const QString &description, TagScope tagScope)
{
const QModelIndexList rows =
m_threadView->selectionModel()->selectedRows();
@@ -3719,7 +3962,13 @@ void MainWindow::tagSelected(const QStringList &add, const QStringList &remove,
// A message row's row number indexes its siblings, so the old
// threadAt(index.row()) mapping silently acted on whichever thread sat at
// that position in the list.
- const ActionScope scope = m_model->scopeFor(rows);
+ //
+ // Message scope by default since item 108: a thread row displays one
+ // message, so acting on it acts on that message. Thread scope is what the
+ // "Whole thread" actions ask for explicitly.
+ const ActionScope scope = tagScope == TagScope::Thread
+ ? m_model->scopeFor(rows)
+ : m_model->messageScopeFor(rows);
if (scope.isEmpty())
return;
@@ -3758,10 +4007,33 @@ void MainWindow::sendMessageTagChange(const QStringList &messageIds,
if (messageIds.isEmpty())
return;
- // No optimistic model update. applyTagChange is keyed by THREAD and would
- // repaint the whole row as though every message in it had changed, which
- // for a one-message edit is a lie the user would see and then watch
- // silently correct itself on the next query.
+ // Optimistically applied to each MESSAGE's own row. applyTagChange is
+ // keyed by thread and would repaint the whole card as though every message
+ // in it had changed, which for a one-message edit is a lie; that is why
+ // 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);
+
+ // 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
+ // thread path has carried this since the strip existed; without it here, a
+ // message-scoped edit repainted the list row and left the pane's chips
+ // describing the message as it was, until the user selected away and back.
+ //
+ // Keyed on m_currentMessageId, which is set only for a message row, so a
+ // write to some other reply cannot repaint the open one with its tags.
+ //
+ // Read by ID, not from currentIndex(): the two agree today, and a guard
+ // that depends on them agreeing would put the WRONG message's tags in the
+ // pane on the day they do not. The id is what the pane is actually
+ // showing.
+ if (!m_currentMessageId.isEmpty()
+ && messageIds.contains(m_currentMessageId)) {
+ m_messageView->setTags(
+ m_model->messageById(m_currentMessageId).tags);
+ }
// The accounts this touches, resolved through the containing threads: the
// account is a property of the thread, and the sync needs the channel
@@ -3821,7 +4093,7 @@ void MainWindow::sendThreadTagChange(const QStringList &threadIds,
if (threadIds.contains(m_currentThreadId)) {
const QModelIndex current = m_threadView->currentIndex();
if (current.isValid())
- m_messageView->setTags(m_model->threadAt(current.row()).tags);
+ m_messageView->setTags(m_model->threadFor(current).tags);
}
// A sync holds notmuch's exclusive write lock, and the worker's read-write
diff --git a/src/mainwindow.h b/src/mainwindow.h
index 614430f..e741416 100644
--- a/src/mainwindow.h
+++ b/src/mainwindow.h
@@ -167,6 +167,37 @@ public:
/// command was pushed, which is what "this did nothing" has to assert.
int undoDepthForTesting() const { return m_undoStack.count(); }
+ /// The text of the command on top of the undo stack.
+ ///
+ /// A test seam for the DIRECTION a toggle chose. Delete and Undelete both
+ /// push one command and touch the same rows, so a depth or an id says
+ /// nothing about which way the toggle went, which is exactly what item 88
+ /// got wrong.
+ QString undoTextForTesting() const { return m_undoStack.undoText(); }
+
+ /// The tag counts the tag dialog would be built from, for the current
+ /// selection. A test seam: the dialog is modal, so the counts cannot be
+ /// observed through it.
+ QHash<QString, int> selectionTagCountsForTesting() const
+ {
+ return selectionTagCounts();
+ }
+
+ /// The thread the current row belongs to. A test seam for item 88's
+ /// resolution itself, reachable when the write it guards is not.
+ ThreadSummary threadForCurrentRowForTesting() const;
+
+ /// Sends a message-scoped tag change directly. A test seam for the cases
+ /// where driving the action would move the selection, which is sometimes
+ /// the very thing under test.
+ void sendMessageTagChangeForTesting(const QStringList &messageIds,
+ const QStringList &add,
+ const QStringList &remove,
+ const QString &description)
+ {
+ sendMessageTagChange(messageIds, add, remove, description);
+ }
+
/// The ids the last tag change was sent for, and whether they were thread
/// ids or message ids.
///
@@ -180,6 +211,11 @@ public:
return m_pendingChange.messageIds;
}
+ /// The whole change last sent, for tests about WHAT was written rather
+ /// than what it was written to. The tags are the same under either scope,
+ /// so a test about a tag name should read this instead of a model row.
+ TagChange pendingChangeForTesting() const { return m_pendingChange; }
+
/// The generation a worker reply must carry to be accepted.
///
/// A test seam: onQueryFinished() discards a reply whose generation is
@@ -563,13 +599,32 @@ private:
const QString &description,
const std::function<void()> &handler);
+ /// What a tag action acts on.
+ ///
+ /// Since item 108 a thread ROW means the one message its card displays, so
+ /// Message is the default and Thread is the explicit choice the user makes
+ /// through the "Whole thread" submenu. Before that there was no choice:
+ /// a thread row always meant the conversation.
+ enum class TagScope {
+ Message, ///< The message each selected row displays.
+ Thread, ///< Every message of each selected row's thread.
+ };
+
void tagSelected(const QStringList &add, const QStringList &remove,
- const QString &description);
+ const QString &description,
+ TagScope scope = TagScope::Message);
/// Starts, restarts or cancels the mark-read timer for a newly opened
- /// thread. Cancels outright for a thread that is not unread, so an already
- /// read thread never schedules a write that would change nothing.
- void scheduleMarkRead(const ThreadSummary &thread);
+ /// MESSAGE. Cancels outright for one that is not unread, so an already read
+ /// message never schedules a write that would change nothing.
+ ///
+ /// Takes the id and the state separately because the two come from
+ /// different places: a reply row has a MessageNode, and a thread row has
+ /// only its summary, whose `unread` is a union over the conversation.
+ void scheduleMarkRead(const QString &messageId, bool unread);
+
+ /// The message id of the thread the pane was opened from, or empty.
+ QString currentThreadFirstMessageId() const;
/// Removes `unread` from the thread the timer was armed for, if it is still
/// the one on screen.
@@ -631,6 +686,29 @@ private:
///
/// The only route to an arbitrary tag: every other tag action writes a
/// hardcoded name.
+ /// The "Whole thread" submenu, built fresh for each parent that needs one.
+ ///
+ /// A QMenu lives in one menu tree, so the menu bar and the context menu get
+ /// their own instance. The actions inside are shared, which is what has to
+ /// stay consistent between them.
+ QMenu *buildThreadActionsMenu(QWidget *parent);
+
+ /// Per-tag counts across the selected rows, for the tag dialog.
+ QHash<QString, int> selectionTagCounts() const;
+
+ /// True when every selected row already carries \p tag, which is what a
+ /// toggle asks before choosing its direction.
+ ///
+ /// Under Message scope each row answers about what it STANDS FOR: a reply
+ /// row about its message, a thread row about the message its card
+ /// displays. Asking a reply's thread makes a toggle one-way, since the
+ /// message-scoped write never changes the thread's tags.
+ ///
+ /// Under Thread scope a row answers about its whole thread, so the
+ /// question matches the write the thread actions are about to make.
+ bool everySelectedRowHasTag(const QString &tag,
+ TagScope scope = TagScope::Message) const;
+
void editTagsOnSelection();
/// Set once the user has answered the exit prompt, or once a sync started
@@ -981,10 +1059,17 @@ private:
/// fires, not each one passed through.
QTimer *m_markReadTimer = nullptr;
- /// The thread m_markReadTimer will mark read. Compared against the current
- /// selection when it fires, so a timer that outlives its thread does
- /// nothing rather than marking the wrong one.
- QString m_markReadThreadId;
+ /// The MESSAGE m_markReadTimer will mark read. Compared against what the
+ /// pane is showing when it fires, so a timer that outlives its message
+ /// does nothing rather than marking the wrong one.
+ ///
+ /// A message id, not a thread id, since item 87. The timer used to mark
+ /// the whole thread, which was coherent while a root card rendered the
+ /// whole conversation and stopped being so when item 66 made it render
+ /// one message: reading one message marked replies read that had never
+ /// been displayed, and with maildir.synchronize_flags on that reaches the
+ /// server.
+ QString m_markReadMessageId;
/// Debounces the automatic sync that follows a tag edit (item 71).
///
diff --git a/src/notmuchworker.cpp b/src/notmuchworker.cpp
index a3a2fd5..b152830 100644
--- a/src/notmuchworker.cpp
+++ b/src/notmuchworker.cpp
@@ -314,6 +314,9 @@ void NotmuchWorker::runQuery(const QString &query, quint64 generation,
if (matched) {
summary.firstMessageId = QString::fromUtf8(
notmuch_message_get_message_id(message));
+ // The card's own tags, beside the thread's union above.
+ // Same walk, same index read, no extra query.
+ summary.firstMessageTags = tagsOf(message);
break;
}
}
@@ -323,6 +326,9 @@ void NotmuchWorker::runQuery(const QString &query, quint64 generation,
if (notmuch_message_t *first = notmuch_messages_get(top)) {
summary.firstMessageId = QString::fromUtf8(
notmuch_message_get_message_id(first));
+ // The card's own tags, beside the thread's union above.
+ // Same walk, same index read, no extra query.
+ summary.firstMessageTags = tagsOf(first);
}
}
}
diff --git a/src/tagchip.cpp b/src/tagchip.cpp
index d770a56..ec83641 100644
--- a/src/tagchip.cpp
+++ b/src/tagchip.cpp
@@ -29,8 +29,18 @@ namespace TagChip {
QSize sizeFor(const QFontMetrics &metrics, const QString &text)
{
- return QSize(metrics.horizontalAdvance(text) + kPaddingX * 2,
- metrics.height() + kPaddingY * 2);
+ return sizeFor(metrics, text, 1.0);
+}
+
+QSize sizeFor(const QFontMetrics &metrics, const QString &text, qreal scale)
+{
+ // Floored at 2 a side: the corner radius is half the chip's height, so the
+ // leftmost and rightmost pixels of the fill are curve rather than usable
+ // width, and text set flush against it touches the round end.
+ const int padX = qMax(2, qRound(kPaddingX * scale));
+ const int padY = qMax(0, qRound(kPaddingY * scale));
+ return QSize(metrics.horizontalAdvance(text) + padX * 2,
+ metrics.height() + padY * 2);
}
void paint(QPainter *painter, const QRect &rect, const QString &text,
diff --git a/src/tagchip.h b/src/tagchip.h
index 5514cae..68e1e55 100644
--- a/src/tagchip.h
+++ b/src/tagchip.h
@@ -43,6 +43,19 @@ constexpr int kSpacing = 4;
QSize sizeFor(const QFontMetrics &metrics, const QString &text);
+/// The same, with the padding scaled by \p scale.
+///
+/// The padding is a fixed pixel count, which is right for one chip size and
+/// wrong the moment there are two: at 1.0 it is 18px around roughly 30px of
+/// text on a sibling chip, so the chip stays wide while its text shrinks and
+/// the tier reads as "same size, smaller letters" rather than as a smaller
+/// chip. Scaling it with the font is what makes the second tier actually look
+/// smaller (item 111).
+///
+/// Floored at 2px a side, because the corner radius is half the height and a
+/// chip with no horizontal padding has its text touching the curve.
+QSize sizeFor(const QFontMetrics &metrics, const QString &text, qreal scale);
+
/// Paints the chip into `rect`, using `text` and `background`. The text colour
/// is derived from the fill so it stays legible.
void paint(QPainter *painter, const QRect &rect, const QString &text,
diff --git a/src/threadlistmodel.cpp b/src/threadlistmodel.cpp
index 3e079ed..6ddd85c 100644
--- a/src/threadlistmodel.cpp
+++ b/src/threadlistmodel.cpp
@@ -347,6 +347,15 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const
case DateFormatRole:
return m_dateFormat;
case Qt::BackgroundRole:
+ // Doomed first: a reply tagged deleted or spam is on its way out
+ // and the user has to see that the moment they act, exactly as a
+ // thread row does. Without this branch a message-scoped Delete
+ // repainted a reply identically to an undeleted one, so the
+ // pending count moved and nothing on screen did.
+ if (node.isDoomed())
+ return QBrush(node.isDeleted() ? deletedColour()
+ : spamColour());
+
// Tinted, so an expanded thread reads as one block rather than as
// more table rows. Applied per cell here; ThreadListView fills the
// same colour across the strip's band so the row does not end up
@@ -354,18 +363,47 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const
return replyBackground();
case Qt::FontRole: {
// A size down from the thread rows, so a thread reads as the
- // heading and its replies as the contents. Never bold: an unread
- // reply is still subordinate to the thread it belongs to, and the
- // thread row above already carries the unread cue for the whole
- // conversation.
+ // heading and its replies as the contents. The size is what keeps
+ // a reply subordinate; bold on top of it is the unread cue, at the
+ // user's request on 2026-08-16.
+ //
+ // Replies were unbolded deliberately at first, on the reasoning
+ // that the thread row above already says the conversation has
+ // unread mail. That is true of the THREAD and useless for the
+ // reply: once a thread is expanded, the row telling the user which
+ // messages in it are unread is the only one that can, and dimming
+ // alone left the user unable to see a read/unread change at all.
QFont font = QGuiApplication::font();
if (font.pointSize() > 0)
font.setPointSize(qMax(6, font.pointSize() - 1));
else if (font.pixelSize() > 0)
font.setPixelSize(qMax(8, font.pixelSize() - 2));
+
+ // Bold combines with the dimming rather than replacing it: two
+ // cues for one state, which is what the thread row has had since
+ // 2026-08-07 and for the same reason. If the desktop's own font is
+ // configured Bold, setBold() changes nothing and the dimming is
+ // the whole cue, which CLAUDE.md records as a real configuration
+ // on this user's machine.
+ if (node.isUnread())
+ font.setBold(true);
+
+ // Struck through when doomed, for the same reason the thread row
+ // is: the state then survives a screenshot, a colourblind reader,
+ // and a theme that overrides the background. A reply had neither
+ // this nor the fill, so a message-scoped Delete was invisible.
+ if (node.isDoomed())
+ font.setStrikeOut(true);
return font;
}
case Qt::ForegroundRole:
+ // White over the doomed fill, matching the thread row. The dimmed
+ // read colour is mixed toward the BACKGROUND, so leaving it here
+ // would compute a grey against the pane's base and then paint it
+ // over red.
+ if (node.isDoomed())
+ return QBrush(QColor(Qt::white));
+
// Dimmed whether read or not, for the same reason as the font: a
// reply is subordinate content. An unread one is left undimmed so
// it can still be found.
@@ -378,7 +416,26 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const
if (index.row() >= m_threads.size())
return {};
- const ThreadSummary &thread = m_threads.at(index.row()).summary;
+ const ThreadNode &rowNode = m_threads.at(index.row());
+
+ // A card stands for ONE message since item 108, so it must draw that
+ // message's tags and not the thread's. `ThreadSummary::tags` is notmuch's
+ // UNION over the conversation: a four-message thread whose third message
+ // is signed reads as signed, and the card said so about a message that was
+ // not (item 110).
+ //
+ // Only the tags are substituted. Everything else on the card, the subject,
+ // the authors, the date and the reply count, describes the THREAD and is
+ // correct as it stands; only the tags were ever the union that lied.
+ //
+ // `first.tags` is populated when the message is loaded, which is when the
+ // user selects the row. Before that the union is the only answer available
+ // and is what the card shows, which is why an unopened row can still
+ // display a sibling's mark. Narrowing that further needs per-message state
+ // in the query itself.
+ ThreadSummary thread = rowNode.summary;
+ if (!rowNode.first.messageId.isEmpty())
+ thread.tags = rowNode.first.tags;
if (role == ThreadIdRole)
return thread.threadId;
@@ -424,7 +481,8 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const
if (role == MessageOwnColoursRole)
return QVariantList();
- if (role == PillTagsRole || role == PillColoursRole) {
+ if (role == PillTagsRole || role == PillColoursRole
+ || role == PillOwnCountRole) {
// Everything the row already says another way is dropped: the account
// is the chip in the subject cell, flagged is the star column,
// attachment is the paperclip, unread is the row not being dimmed, and
@@ -443,16 +501,43 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const
QStringLiteral("unread"),
};
- QStringList pills;
- for (const QString &tag : thread.tags) {
- if (hidden.contains(tag) || isDrawnAsAMark(tag)
- || TagColors::isAccountTag(tag))
- continue;
- pills.append(tag);
+ const auto pillsFrom = [&](const QStringList &tags) {
+ QStringList pills;
+ for (const QString &tag : tags) {
+ if (hidden.contains(tag) || isDrawnAsAMark(tag)
+ || TagColors::isAccountTag(tag))
+ continue;
+ pills.append(tag);
+ }
+ // Sorted rather than in notmuch's order, which is not guaranteed
+ // stable: a row whose pills reordered between repaints would
+ // flicker.
+ pills.sort();
+ return pills;
+ };
+
+ // `thread.tags` is the displayed message's own tags once the row has
+ // been opened, and the thread's union before that (see the
+ // substitution above). The union is always the full set, so the
+ // difference is what belongs only to siblings.
+ QStringList pills = pillsFrom(thread.tags);
+ const int ownCount = pills.size();
+
+ // The sibling tier, appended after the message's own. Shown rather
+ // than dropped at the user's request: a card sits above a
+ // conversation, so what the rest of it carries is worth seeing, just
+ // not at the same weight. The delegate draws these smaller and muted.
+ //
+ // Empty until the row has been opened, because before that
+ // `thread.tags` IS the union and the difference is nothing. That is
+ // what makes a chip shrink rather than appear.
+ for (const QString &tag : pillsFrom(rowNode.summary.tags)) {
+ if (!pills.contains(tag))
+ pills.append(tag);
}
- // Sorted rather than in notmuch's order, which is not guaranteed
- // stable: a row whose pills reordered between repaints would flicker.
- pills.sort();
+
+ if (role == PillOwnCountRole)
+ return ownCount;
if (role == PillTagsRole)
return pills;
@@ -598,6 +683,23 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const
return {};
}
+ThreadListModel::ThreadNode
+ThreadListModel::nodeFor(const ThreadSummary &summary)
+{
+ ThreadNode node{ summary, {}, {}, false };
+
+ // Only when the query actually supplied them. An empty list here would be
+ // indistinguishable from "this message carries nothing", which would put
+ // every chip in the sibling tier and mute the whole card.
+ if (!summary.firstMessageId.isEmpty()
+ && !summary.firstMessageTags.isEmpty()) {
+ node.first.messageId = summary.firstMessageId;
+ node.first.threadId = summary.threadId;
+ node.first.tags = summary.firstMessageTags;
+ }
+ return node;
+}
+
void ThreadListModel::appendBatch(const QVector<ThreadSummary> &batch)
{
// beginInsertRows with an empty range violates Qt's contract, so the guard
@@ -608,7 +710,7 @@ void ThreadListModel::appendBatch(const QVector<ThreadSummary> &batch)
const int first = m_threads.size();
beginInsertRows({}, first, first + batch.size() - 1);
for (const ThreadSummary &summary : batch)
- m_threads.append(ThreadNode{ summary, {}, {}, false });
+ m_threads.append(nodeFor(summary));
endInsertRows();
}
@@ -657,7 +759,7 @@ void ThreadListModel::reconcile(const QVector<ThreadSummary> &threads)
if (it == present.constEnd()) {
const int at = qMin(target, m_threads.size());
beginInsertRows({}, at, at);
- m_threads.insert(at, ThreadNode{ summary, {}, {}, false });
+ m_threads.insert(at, nodeFor(summary));
endInsertRows();
// Every later row shifted by one, and the map is read again on the
@@ -707,8 +809,26 @@ void ThreadListModel::reconcile(const QVector<ThreadSummary> &threads)
|| m_threads.at(row).summary.authors != summary.authors
|| m_threads.at(row).summary.date != summary.date
|| m_threads.at(row).summary.totalCount != summary.totalCount
- || m_threads.at(row).summary.matchedCount != summary.matchedCount) {
+ || m_threads.at(row).summary.matchedCount != summary.matchedCount
+ // The card's OWN message, which can move while the thread's union
+ // does not: a root read elsewhere leaves the thread unread as long
+ // as any reply is. Without this the card kept the tags it was
+ // first given, and the sibling tier with them.
+ || m_threads.at(row).summary.firstMessageTags
+ != summary.firstMessageTags) {
m_threads[row].summary = summary;
+
+ // The node too, since the card draws its tags from there. Only the
+ // tags: the node's children and loaded flag are the expansion
+ // state this whole method exists to preserve, and `first` carries
+ // no children.
+ if (!summary.firstMessageId.isEmpty()
+ && !summary.firstMessageTags.isEmpty()) {
+ m_threads[row].first.messageId = summary.firstMessageId;
+ m_threads[row].first.threadId = summary.threadId;
+ m_threads[row].first.tags = summary.firstMessageTags;
+ }
+
emit dataChanged(index(row, 0), index(row, 0));
}
}
@@ -804,6 +924,108 @@ QString ThreadListModel::threadIdForMessage(const QString &messageId) const
return {};
}
+void ThreadListModel::setRootMessageTags(const QString &messageId,
+ const QStringList &tags)
+{
+ if (messageId.isEmpty())
+ return;
+
+ for (int row = 0; row < m_threads.size(); ++row) {
+ ThreadNode &node = m_threads[row];
+ if (node.summary.firstMessageId != messageId
+ && node.first.messageId != messageId) {
+ continue;
+ }
+
+ if (node.first.tags == tags && !node.first.messageId.isEmpty())
+ return; // Nothing changed; do not churn the view.
+
+ // Enough of a node for the card to draw from. The rest of the display
+ // still comes from the summary, which is correct for it: the subject,
+ // the authors and the date describe the thread, and only the TAGS were
+ // ever the union that lied about this message.
+ node.first.messageId = messageId;
+ node.first.threadId = node.summary.threadId;
+ node.first.tags = tags;
+
+ const QModelIndex threadIndex = index(row, 0, QModelIndex());
+ emit dataChanged(threadIndex, threadIndex);
+ return;
+ }
+}
+
+MessageNode ThreadListModel::messageById(const QString &messageId) const
+{
+ if (messageId.isEmpty())
+ return {};
+
+ for (const ThreadNode &node : m_threads) {
+ // 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
+ // every root message, and a caller that trusted it set the message
+ // pane's tag strip to that empty tag list, wiping a strip that had
+ // been correct.
+ if (!node.first.messageId.isEmpty()
+ && node.first.messageId == messageId) {
+ return node.first;
+ }
+
+ // Before expansion there is no node, so the answer is assembled from
+ // the summary: for a thread of one, its tags ARE this message's, since
+ // a thread's tags are a union over its messages. For a longer thread
+ // they are a union over messages this one is only part of, which is
+ // wider than the truth but is also exactly what the card shows, so a
+ // caller repainting from it stays consistent with the row beside it.
+ if (node.summary.firstMessageId == messageId) {
+ MessageNode root;
+ root.messageId = node.summary.firstMessageId;
+ root.threadId = node.summary.threadId;
+ root.subject = node.summary.subject;
+ root.date = node.summary.date;
+ root.tags = node.summary.tags;
+ return root;
+ }
+
+ for (const MessageNode &child : node.children) {
+ if (child.messageId == messageId)
+ return child;
+ }
+ }
+ return {};
+}
+
+ActionScope ThreadListModel::messageScopeFor(
+ const QModelIndexList &selection) const
+{
+ ActionScope scope;
+
+ for (const QModelIndex &index : selection) {
+ QString messageId;
+ if (isMessageRow(index)) {
+ messageId = messageAt(index).messageId;
+ } else {
+ if (index.row() < 0 || index.row() >= m_threads.size())
+ continue;
+ // The message the CARD displays, which the query already named.
+ // Not the loaded children: a thread the user never expanded still
+ // shows its first message, and this must work without one.
+ messageId = m_threads.at(index.row()).summary.firstMessageId;
+ }
+
+ // Skipped rather than widened. Falling back to the thread here would
+ // silently act on messages the row does not display, which is the
+ // behaviour item 108 removed.
+ if (messageId.isEmpty() || scope.messageIds.contains(messageId))
+ continue;
+
+ scope.messageIds.append(messageId);
+ scope.messageCount += 1;
+ }
+
+ return scope;
+}
+
ActionScope ThreadListModel::scopeFor(const QModelIndexList &selection) const
{
ActionScope scope;
@@ -847,6 +1069,19 @@ ThreadSummary ThreadListModel::threadAt(int row) const
return m_threads.at(row).summary;
}
+ThreadSummary ThreadListModel::threadFor(const QModelIndex &index) const
+{
+ if (!index.isValid())
+ return {};
+
+ // The parent's row for a message, its own for a thread. Both are top-level
+ // numbers by the time threadAt() sees them, which is the whole point: the
+ // conversion happens once, here, instead of at every call site that has to
+ // remember which kind of row it is holding.
+ const QModelIndex threadIndex = isMessageRow(index) ? index.parent() : index;
+ return threadAt(threadIndex.row());
+}
+
QStringList ThreadListModel::accountKeysForThread(const QString &threadId) const
{
QStringList keys;
@@ -884,7 +1119,117 @@ void ThreadListModel::applyTagChange(const QString &threadId,
// The whole card repaints: unread state drives its font, and the tags
// it draws on line 3 have just changed.
- emit dataChanged(index(row, 0), index(row, 0));
+ const QModelIndex threadIndex = index(row, 0);
+ emit dataChanged(threadIndex, threadIndex);
+
+ // And every LOADED reply, because a thread-scoped write reaches every
+ // message in the thread. Updating only the summary left an expanded
+ // thread showing replies that still carried the old tags: marking a
+ // thread read repainted the card and left its replies bold and
+ // undimmed, describing a state the database no longer held. They
+ // corrected themselves on the next query, which is what made it look
+ // like a repaint bug rather than a stale model.
+ //
+ // Only the loaded ones exist to update. An unexpanded thread has no
+ // child rows, and the replies it does not hold are the database's
+ // business, not this model's.
+ QVector<MessageNode> &children = m_threads[row].children;
+ if (children.isEmpty())
+ return;
+
+ for (MessageNode &child : children) {
+ for (const QString &tag : removed)
+ child.tags.removeAll(tag);
+ for (const QString &tag : added) {
+ if (!child.tags.contains(tag))
+ child.tags.append(tag);
+ }
+ }
+
+ // One span for the whole expansion rather than a signal per reply: the
+ // rows are contiguous under this parent and a view coalesces them
+ // anyway.
+ emit dataChanged(index(0, 0, threadIndex),
+ index(children.size() - 1, 0, threadIndex));
+ return;
+ }
+}
+
+void ThreadListModel::applyMessageTagChange(const QString &messageId,
+ const QStringList &added,
+ const QStringList &removed)
+{
+ if (messageId.isEmpty())
return;
+
+ const auto retag = [&](QStringList &tags) {
+ for (const QString &tag : removed)
+ tags.removeAll(tag);
+ for (const QString &tag : added) {
+ if (!tags.contains(tag))
+ tags.append(tag);
+ }
+ };
+
+ for (int row = 0; row < m_threads.size(); ++row) {
+ ThreadNode &node = m_threads[row];
+ const QModelIndex threadIndex = index(row, 0, QModelIndex());
+
+ // The ROOT card's own message, which is not among the children:
+ // setThreadMessages drops depth 0 because the root row stands for it.
+ // Searching only the children meant a write to the message a root card
+ // displays found nothing and repainted nothing, and item 108 made that
+ // the ordinary gesture rather than an edge case.
+ //
+ // Matched on the summary's id as well as the loaded node's, because the
+ // node is empty until the thread has been expanded and the user acts on
+ // unexpanded threads constantly.
+ const bool isRoot =
+ node.summary.firstMessageId == messageId
+ || (!node.first.messageId.isEmpty()
+ && node.first.messageId == messageId);
+ if (isRoot) {
+ // The root's own node, which is what the card draws its tags from
+ // once the message has been loaded. Seeded from the summary when
+ // the message has never been loaded, so an edit made before the
+ // row was ever opened still has somewhere to land; the summary is
+ // the union, which is the widest honest starting point.
+ if (node.first.messageId.isEmpty()) {
+ node.first.messageId = node.summary.firstMessageId;
+ node.first.threadId = node.summary.threadId;
+ node.first.tags = node.summary.tags;
+ }
+ retag(node.first.tags);
+
+ // The SUMMARY only for a single-message thread. A thread's tags are
+ // a UNION over its messages: for a thread of one that union IS this
+ // message, so keeping the two in step is exact; for a longer
+ // thread, deleting one message does not delete the conversation,
+ // and the summary must keep describing the conversation because
+ // that is what the thread-scoped actions and the query read.
+ //
+ // The CARD does not depend on this either way: since item 110 it
+ // draws its tags from first.tags, which was just updated. This
+ // keeps the summary honest for everything else that reads it.
+ if (node.summary.totalCount <= 1)
+ retag(node.summary.tags);
+
+ emit dataChanged(threadIndex, threadIndex);
+ return;
+ }
+
+ QVector<MessageNode> &children = node.children;
+ for (int child = 0; child < children.size(); ++child) {
+ if (children.at(child).messageId != messageId)
+ continue;
+
+ retag(children[child].tags);
+
+ // The reply's own row, and only that row. Its chips, its marks,
+ // its dimming and its doomed fill all read the node's tags.
+ const QModelIndex replyIndex = index(child, 0, threadIndex);
+ emit dataChanged(replyIndex, replyIndex);
+ return;
+ }
}
}
diff --git a/src/threadlistmodel.h b/src/threadlistmodel.h
index 2b8d2b0..717537c 100644
--- a/src/threadlistmodel.h
+++ b/src/threadlistmodel.h
@@ -65,6 +65,21 @@ public:
/// config itself would be a second source of truth.
PillColoursRole,
+ /// How many of PillTagsRole's entries belong to the message the card
+ /// DISPLAYS, the rest belonging only to its siblings.
+ ///
+ /// The card stands for one message but sits above a conversation, so
+ /// it shows both: the message's own tags first at full size, then the
+ /// thread's other tags smaller and muted. Without the split a card
+ /// either claimed a sibling's tag as its own (item 110) or dropped it
+ /// and looked like it had lost information.
+ ///
+ /// Equals the whole list until the row has been opened, since the
+ /// per-message tags arrive with the message load and before that the
+ /// union is the only answer there is. Chips therefore SHRINK when the
+ /// split becomes known; none ever disappears.
+ PillOwnCountRole,
+
/// True when the row is a MESSAGE row rather than a thread root.
/// Drives both the action scope and whether the view paints a tag
/// strip under the row.
@@ -225,8 +240,29 @@ public:
/// contradict the sort the user selected.
void reconcile(const QVector<ThreadSummary> &threads);
+ /// The thread at a TOP-LEVEL row.
+ ///
+ /// **Wrong for any index that might be a reply**, and that is item 88. A
+ /// tree numbers rows per parent, so a reply's row() indexes its siblings:
+ /// threadAt(0) on the first reply of any thread returns the FIRST THREAD IN
+ /// THE LIST, and the caller acts on unrelated mail while every id it
+ /// compares looks right. Safe only for a row number that came from a loop
+ /// over rowCount(), never from an index the user selected.
+ ///
+ /// Prefer threadFor(index), which cannot be handed the wrong number.
ThreadSummary threadAt(int row) const;
+ /// The thread an index belongs to, whichever kind of row it is.
+ ///
+ /// A thread row resolves to itself; a message row resolves through its
+ /// PARENT rather than through its own row number. This is the accessor
+ /// every caller holding a QModelIndex wants, and it exists because the
+ /// row-taking one above silently answers about unrelated mail for a reply.
+ ///
+ /// An invalid or unknown index gives a default-constructed summary, whose
+ /// empty threadId every caller here already treats as "nothing to do".
+ ThreadSummary threadFor(const QModelIndex &index) const;
+
/// Fills in a thread's message rows once the worker has walked its tree.
///
/// The depth-0 message is dropped: it is the thread's first message and the
@@ -249,14 +285,58 @@ public:
/// message the user could select is always findable here.
QString threadIdForMessage(const QString &messageId) const;
- /// Resolves a selection into what an action should touch.
+ /// Records the tags a MESSAGE really carries, as the worker reported them.
+ ///
+ /// Exists because `ThreadSummary::tags` is notmuch's UNION over the
+ /// thread, which is right for a card standing for a conversation and wrong
+ /// for one standing for a message. A four-message thread whose third
+ /// message is signed makes the whole thread read as signed, so the root
+ /// card and the message pane both claimed a tag the displayed message did
+ /// not have.
+ ///
+ /// Only the ROOT needs this: reply rows already carry their own nodes from
+ /// setThreadMessages. Calling it for anything else is a no-op.
+ ///
+ /// The thread's summary is deliberately NOT rewritten. It describes the
+ /// conversation, and three unread siblings do not stop being unread
+ /// because this message was read.
+ void setRootMessageTags(const QString &messageId, const QStringList &tags);
+
+ /// A loaded message row's node, found by id rather than by position.
///
- /// Mixed selections are honoured as given: a thread root and an unrelated
- /// reply act on that whole thread and that one message. Nothing is
- /// escalated or narrowed silently, which is the point of the scope being
- /// visible in the first place.
+ /// For callers that know WHICH message they mean and must not depend on it
+ /// being the row the user has selected. Default-constructed when no
+ /// expanded thread holds it.
+ MessageNode messageById(const QString &messageId) const;
+
+ /// Resolves a selection into whole THREADS, for the thread-scoped actions.
+ ///
+ /// A thread row contributes its thread; a message row still contributes
+ /// only itself, since a reply's own row cannot be widened into its
+ /// conversation without escalating silently. Mixed selections are honoured
+ /// as given: a thread root and an unrelated reply act on that whole thread
+ /// and that one message.
+ ///
+ /// **Not the default any more.** Since item 108 the ordinary actions use
+ /// messageScopeFor(); this is what the explicit "whole thread" submenu
+ /// resolves through.
ActionScope scopeFor(const QModelIndexList &selection) const;
+ /// Resolves a selection into individual MESSAGES, which is what the
+ /// ordinary tag actions act on since item 108.
+ ///
+ /// A thread row contributes the ONE message its card displays, not its
+ /// whole conversation. That is `ThreadSummary::firstMessageId`, carried
+ /// from the query, so this needs no expansion and no worker round trip.
+ /// In the Sent view that field is the first MATCHED message rather than
+ /// the thread's opening one, which is right here for the same reason it is
+ /// right on the card: both answer "the message this row shows".
+ ///
+ /// A thread row whose `firstMessageId` is empty contributes nothing. That
+ /// is a row the model cannot name a message for, and acting on the whole
+ /// thread instead would be the silent escalation this exists to remove.
+ ActionScope messageScopeFor(const QModelIndexList &selection) const;
+
/// The account keys behind a thread's account tags, for item 49's
/// per-account sync.
///
@@ -273,6 +353,21 @@ public:
void applyTagChange(const QString &threadId, const QStringList &added,
const QStringList &removed);
+ /// The same, for a change scoped to ONE message.
+ ///
+ /// Repaints that message's own row and leaves the thread alone. The thread
+ /// row deliberately does not follow: it stands for the whole conversation,
+ /// so redrawing it for a one-message edit would claim every message in it
+ /// had changed. That reasoning is why no optimistic update existed here at
+ /// all, which left Delete and Toggle unread on a reply moving the pending
+ /// count and changing nothing on screen.
+ ///
+ /// A message id that no expanded thread holds is a no-op: only expanded
+ /// threads have message rows, so there is nothing to repaint.
+ void applyMessageTagChange(const QString &messageId,
+ const QStringList &added,
+ const QStringList &removed);
+
private:
/// One thread root and the message rows expanded under it.
///
@@ -301,6 +396,21 @@ private:
bool loaded = false;
};
+ /// A newly arrived thread, with its card's own message seeded from the
+ /// query.
+ ///
+ /// `ThreadSummary::tags` is notmuch's union over the conversation, and the
+ /// card stands for ONE message. The worker reads that message's own tags
+ /// in the same walk that finds its id, so the two tiers are known from the
+ /// first paint (item 111). Deriving them from the message LOAD instead
+ /// left every unopened row drawing one tier and correcting itself when the
+ /// user selected it, which is most of the list.
+ ///
+ /// `first` is left empty when the query carried no per-message tags, so
+ /// anything that supplies only a summary keeps the old behaviour rather
+ /// than claiming the union as one message's.
+ static ThreadNode nodeFor(const ThreadSummary &summary);
+
QVector<ThreadNode> m_threads;
const TagColors *m_tagColors = nullptr;
QString m_dateFormat;
diff --git a/src/types.h b/src/types.h
index f4eaeba..409ce79 100644
--- a/src/types.h
+++ b/src/types.h
@@ -48,6 +48,20 @@ struct ThreadSummary
/// with recipients; the two have nothing in common but their position here.
QString firstMessageId;
+ /// The tags of that ONE message, as opposed to `tags` above, which is
+ /// notmuch's union over the whole thread.
+ ///
+ /// A card stands for one message but sits above a conversation, and shows
+ /// both: its own tags at full size, the thread's others smaller (item
+ /// 111). Without this the split is unknown until the row is opened and the
+ /// message loads, so every chip renders as the card's own and then shrinks
+ /// on selection, which is what the user reported.
+ ///
+ /// Free, for the same reason `firstMessageId` is: the walk that finds that
+ /// message is already happening and this reads the INDEX, not the message
+ /// file. Do not move it behind a flag by analogy with `recipients`.
+ QStringList firstMessageTags;
+
/// Who the thread's messages were sent TO, summarised for one line.
///
/// Empty unless the query asked for it, and that is a performance
@@ -140,6 +154,15 @@ struct MessageNode
{
return tags.contains(QStringLiteral("attachment"));
}
+
+ bool isDeleted() const { return tags.contains(QStringLiteral("deleted")); }
+ bool isSpam() const { return tags.contains(QStringLiteral("spam")); }
+
+ /// True while the message is tagged for removal, exactly as the thread
+ /// predicate of the same name. A reply carries its own fate: a
+ /// message-scoped Delete tags one message, and the reply's row is the only
+ /// place the user can see that happen.
+ bool isDoomed() const { return isDeleted() || isSpam(); }
};
/// What an action is about to touch, resolved from the selection.
diff --git a/tests/test_carddelegate.cpp b/tests/test_carddelegate.cpp
index d5e55b5..a14671d 100644
--- a/tests/test_carddelegate.cpp
+++ b/tests/test_carddelegate.cpp
@@ -18,6 +18,9 @@
#include "carddelegate.h"
+#include "cardlayout.h"
+#include "tagchip.h"
+#include "tagcolors.h"
#include "threadlistmodel.h"
#include <QTest>
@@ -30,6 +33,9 @@ private slots:
void theAccentLiftsAMutedAccountColour();
void theAccentKeepsEachAccountTellableApart();
void anAccountWithNoColourFallsBackToTheNeutralLine();
+ void aSiblingChipIsMutedButStaysLegibleAndRecognisable();
+ void aSiblingChipFontIsSmallerThanItsOwnTier();
+ void aSiblingChipsPaddingShrinksWithItsFont();
};
namespace {
@@ -131,5 +137,127 @@ void TestCardDelegate::anAccountWithNoColourFallsBackToTheNeutralLine()
ThreadListModel::threadLineColour());
}
+void TestCardDelegate::aSiblingChipIsMutedButStaysLegibleAndRecognisable()
+{
+ // Item 111: a card shows its own tags at full size and the rest of the
+ // conversation's smaller and muted. "Muted" has two hard requirements that
+ // a look at the screen will not catch, so they are asserted here.
+ for (const QColor &colour : sampleAccounts()) {
+ const QColor muted = CardDelegate::mutedChipColour(colour);
+
+ // Actually muted, or the tier is not distinguishable at all.
+ QVERIFY2(saturationOf(muted) < saturationOf(colour),
+ qPrintable(QStringLiteral("%1 was not drained at all")
+ .arg(colour.name())));
+
+ // Same HUE. A sibling's `signed` has to stay recognisably the same
+ // colour as a full-size `signed` elsewhere in the list, or the muting
+ // reads as a different tag rather than a quieter one.
+ float h1 = 0, h2 = 0, s = 0, l = 0, a = 0;
+ colour.getHslF(&h1, &s, &l, &a);
+ muted.getHslF(&h2, &s, &l, &a);
+ QVERIFY2(qAbs(h1 - h2) < 0.001f,
+ qPrintable(QStringLiteral("%1 changed hue when muted")
+ .arg(colour.name())));
+
+ // Same LIGHTNESS, which is what keeps the text legible: TagColors
+ // picks the text colour from the fill, and a fill that drifted toward
+ // black or white could flip that choice or land mid-grey where neither
+ // works. Blending toward the background would do exactly that, which
+ // is the mistake accentLineColour() records.
+ QCOMPARE(lightnessOf(muted), lightnessOf(colour));
+ QCOMPARE(TagColors::textColourOn(muted),
+ TagColors::textColourOn(colour));
+ }
+
+ // An invalid colour stays invalid rather than becoming a real one.
+ QVERIFY(!CardDelegate::mutedChipColour(QColor()).isValid());
+}
+
+void TestCardDelegate::aSiblingChipFontIsSmallerThanItsOwnTier()
+{
+ // Size is what says whose tag a chip is, so the two tiers must differ, and
+ // by enough to SEE. The first version subtracted a point from smallFont(),
+ // and the user reported the tiers as indistinguishable: on their 14pt
+ // desktop that gave 13 and 12, a 7% step.
+ //
+ // The step is now a fraction of the card font, so it does not shrink as
+ // the desktop's font grows. Asserted as a ratio rather than as a size, to
+ // keep this about the DISTINCTION rather than about the constant.
+ QFont card;
+ card.setPointSizeF(14.0); // The user's own desktop size.
+ const qreal own = CardLayout::smallFont(card).pointSizeF();
+ const qreal sibling = CardLayout::siblingFont(card).pointSizeF();
+
+ QVERIFY(sibling < own);
+ QVERIFY2(sibling < own * 0.85,
+ qPrintable(QStringLiteral("sibling %1pt against own %2pt is under "
+ "a 15%% step, which reads as the same "
+ "size")
+ .arg(sibling)
+ .arg(own)));
+
+ // Proportional, not a fixed subtraction: the step must survive a larger
+ // desktop font rather than becoming proportionally smaller.
+ QFont big;
+ big.setPointSizeF(28.0);
+ QVERIFY(CardLayout::siblingFont(big).pointSizeF()
+ < CardLayout::smallFont(big).pointSizeF() * 0.85);
+
+ // The pixel branch too: qt6ct sets fonts in PIXELS, and pointSizeF() is -1
+ // for those, so a point-only implementation silently returns the original
+ // size and both tiers render identically. CLAUDE.md records this trap.
+ QFont pixels;
+ pixels.setPixelSize(14);
+ QVERIFY(pixels.pointSizeF() < 0);
+ QVERIFY2(CardLayout::siblingFont(pixels).pixelSize()
+ < CardLayout::smallFont(pixels).pixelSize(),
+ "a pixel-sized desktop font gives both tiers the same size, so "
+ "the distinction disappears entirely");
+
+ // Floored rather than shrinking without limit.
+ QFont tiny;
+ tiny.setPointSizeF(6.0);
+ QVERIFY(CardLayout::siblingFont(tiny).pointSizeF() >= 6.0);
+}
+
+void TestCardDelegate::aSiblingChipsPaddingShrinksWithItsFont()
+{
+ // Half of "smaller" is the padding, and leaving it fixed is why the first
+ // version still looked the same size. kPaddingX is 9 a side: on a sibling
+ // chip that is 18px of padding around roughly 30px of text, so the chip
+ // stayed wide while its letters shrank, which reads as "same chip, smaller
+ // text" rather than as a smaller chip.
+ QFont card;
+ card.setPointSizeF(14.0);
+ const QFontMetrics ownMetrics(CardLayout::smallFont(card));
+ const QFontMetrics siblingMetrics(CardLayout::siblingFont(card));
+
+ const QString tag = QStringLiteral("signed");
+ // Through CardDelegate::chipSize(), which is what the paint loop calls.
+ // Calling TagChip::sizeFor() directly here proved what THAT function does
+ // and nothing about whether the delegate asks it for a scaled padding: a
+ // mutation dropping the scale at the call site survived that version of
+ // this test.
+ const QSize own = CardDelegate::chipSize(ownMetrics, tag, true);
+ const QSize scaled = CardDelegate::chipSize(siblingMetrics, tag, false);
+ const QSize unscaled = TagChip::sizeFor(siblingMetrics, tag);
+
+ // The font alone is not enough: scaling the padding as well takes off
+ // measurably more width.
+ QVERIFY2(scaled.width() < unscaled.width(),
+ "the padding did not scale, so the chip keeps full-size margins "
+ "around smaller letters");
+ QVERIFY(scaled.width() < own.width());
+ QVERIFY(scaled.height() < own.height());
+
+ // Floored rather than collapsing to nothing: the corner radius is half the
+ // height, so a chip with no horizontal padding has its text on the curve.
+ const QSize tiny = TagChip::sizeFor(siblingMetrics, tag, 0.0);
+ QVERIFY2(tiny.width() > siblingMetrics.horizontalAdvance(tag),
+ "a zero scale left no horizontal padding at all, so the text sits "
+ "on the chip's rounded end");
+}
+
QTEST_MAIN(TestCardDelegate)
#include "test_carddelegate.moc"
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index f4357c6..07cc56b 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -57,6 +57,7 @@
#include <QComboBox>
#include <QScrollBar>
#include "tagchip.h"
+#include "tagstrip.h"
#include "threadlistmodel.h"
#include "threadlistview.h"
#include "notmuchfixture.h"
@@ -273,6 +274,23 @@ private slots:
void escapeBlanksTheMessagePane();
void deleteTogglesOnAnAlreadyDeletedThread();
void deleteOnAMixedSelectionDeletesRatherThanSplittingIt();
+ void deleteOnAReplyReadsItsOwnThreadNotTheFirstInTheList();
+ void toggleUnreadOnAReplyReadsItsOwnThreadNotTheFirstInTheList();
+ void editTagsOnAReplyCountsItsOwnThreadNotTheFirstInTheList();
+ void markCurrentThreadReadResolvesTheThreadThroughTheIndex();
+ void deletingAReplyRepaintsThatReplyRow();
+ void toggleUnreadOnAReplyReadsTheReplysOwnState();
+ void toggleUnreadOnAReplyRepaintsItInBothDirections();
+ void taggingTheOpenReplyUpdatesTheMessagePaneStrip();
+ void taggingAnUnrelatedReplyLeavesTheStripAlone();
+ void aHeldMessageEditIsSentWhenTheSyncEnds();
+ void anActionOnAThreadRowActsOnTheMessageItDisplays();
+ void theThreadSubmenuIsReachableFromBothMenus();
+ void autoMarkReadTouchesOnlyTheMessageOnDisplay();
+ void autoMarkReadArmsForAReplyToo();
+ void taggingTheOpenRootMessageKeepsTheStripPopulated();
+ void aLoadedMessageCorrectsTheStripFromTheThreadsUnion();
+ void aLoadedRootMessageGivesTheCardItsOwnTags();
void aTransientStatusMessageExpires();
void theSelectionCountIsStateAndDoesNotExpire();
void anEditUndoneNettsBackToZero();
@@ -790,6 +808,13 @@ static ThreadSummary makeThread(const QString &id, const QStringList &tags)
thread.subject = QStringLiteral("Subject ") + id;
thread.authors = QStringLiteral("Someone <someone@example.org>");
thread.tags = tags;
+
+ // The message the row displays, which the real worker fills in from the
+ // query. Required since item 108: an ordinary tag action resolves a thread
+ // row to THIS id, so a summary without one names no message and every
+ // action on it does nothing. A fixture missing it fails with "the action
+ // did not happen", which reads as a defect in the action.
+ thread.firstMessageId = id + QStringLiteral("-first@example.org");
return thread;
}
@@ -1372,8 +1397,11 @@ void TestMainWindow::anActionOnAThreadRowSaysItHitTheWholeThread()
selectThreadRow(view, 0);
QApplication::processEvents();
- auto *archive = window.findChild<QAction *>(QStringLiteral("archive"));
- QVERIFY2(archive, "no archive action to trigger");
+ // The THREAD action since item 108. The plain `archive` now acts on the
+ // one message a card displays, and would rightly not claim to have taken
+ // the whole thread; this suffix belongs to the action that really does.
+ auto *archive = window.findChild<QAction *>(QStringLiteral("archive_thread"));
+ QVERIFY2(archive, "no archive_thread action to trigger");
archive->trigger();
// Read BEFORE processEvents, deliberately. This binary has no worker
@@ -4327,7 +4355,13 @@ void TestMainWindow::aHeldEditIsSentBeforeTheSyncEndRefreshReadsTheDatabase()
// The write went out. Nothing else in this handler sends one, so its
// presence is what proves the flush ran, and the refresh below is what it
// has to have run BEFORE.
- QVERIFY2(!window.pendingThreadIdsForTesting().isEmpty(),
+ //
+ // Either scope counts. The gesture is Toggle unread on a thread row, which
+ // is message-scoped since item 108, so the ids land in the message list;
+ // the ORDER this test exists for is the same either way, and pinning the
+ // scope here would make it fail for a reason it does not care about.
+ QVERIFY2(!window.pendingThreadIdsForTesting().isEmpty()
+ || !window.pendingMessageIdsForTesting().isEmpty(),
"the held edit was dropped rather than sent");
const quint64 after = window.currentGenerationForTesting();
@@ -4593,7 +4627,10 @@ void TestMainWindow::deleteTogglesOnAnAlreadyDeletedThread()
QVERIFY(model);
auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
- auto *action = window.findChild<QAction *>(QStringLiteral("delete"));
+ // The THREAD action, since this is about a THREAD's state. Item 108 made
+ // the plain `delete` act on the one message a card displays, and a thread
+ // summary carrying `deleted` says nothing about that message's own tags.
+ auto *action = window.findChild<QAction *>(QStringLiteral("delete_thread"));
QVERIFY(action);
model->appendBatch({ makeThread(QStringLiteral("t1"),
@@ -4621,7 +4658,7 @@ void TestMainWindow::deleteOnAMixedSelectionDeletesRatherThanSplittingIt()
QVERIFY(model);
auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
- auto *action = window.findChild<QAction *>(QStringLiteral("delete"));
+ auto *action = window.findChild<QAction *>(QStringLiteral("delete_thread"));
QVERIFY(action);
model->appendBatch({ makeThread(QStringLiteral("t1"),
@@ -4637,6 +4674,889 @@ void TestMainWindow::deleteOnAMixedSelectionDeletesRatherThanSplittingIt()
"a mixed selection split instead of deleting the whole selection");
}
+/// Builds a window whose SECOND thread is expanded and carries one reply, with
+/// the two threads deliberately in opposite states.
+///
+/// Item 88's shape in one place. A tree numbers rows per parent, so the first
+/// reply of any thread has row() == 0 and threadAt(0) answers about the FIRST
+/// THREAD IN THE LIST. Every test below selects that reply and asserts on
+/// behaviour that can only be right if the thread was resolved through the
+/// index: with the row number, each one reads t1's state while acting on t2.
+///
+/// The opposite states are what makes the tests able to fail. Two threads in
+/// the same state give the same answer either way, which is how the reverted
+/// item 87 fix passed while corrupting mail.
+///
+/// \p replyTags defaults to the thread's own tags, which is the usual case.
+/// Pass it explicitly to make a reply DISAGREE with its thread, which is what
+/// separates "reads the right thread" from "reads the right message": a reply
+/// can be unread inside a thread that is not, and vice versa.
+static QModelIndex expandSecondThreadAndSelectItsReply(
+ QTreeView *view, ThreadListModel *model, const QStringList &firstTags,
+ const QStringList &secondTags,
+ const std::optional<QStringList> &replyTags = std::nullopt)
+{
+ ThreadSummary first = makeThread(QStringLiteral("t1"), firstTags);
+ ThreadSummary second = makeThread(QStringLiteral("t2"), secondTags);
+ second.totalCount = 2;
+ model->appendBatch({ first, second });
+
+ MessageNode root;
+ root.messageId = QStringLiteral("m0@example.org");
+ root.threadId = QStringLiteral("t2");
+ root.tags = secondTags;
+ root.depth = 0;
+ MessageNode reply;
+ reply.messageId = QStringLiteral("m1@example.org");
+ reply.threadId = QStringLiteral("t2");
+ reply.tags = replyTags.value_or(secondTags);
+ reply.depth = 1;
+ model->setThreadMessages(QStringLiteral("t2"), { root, reply });
+
+ const QModelIndex threadRow = model->index(1, 0, QModelIndex());
+ view->expand(threadRow);
+
+ const QModelIndex replyRow = model->index(0, 0, threadRow);
+ if (!replyRow.isValid() || !model->isMessageRow(replyRow))
+ return {};
+
+ // Row 0 under its parent, which is the trap: the number is a plausible
+ // top-level row and threadAt() cannot tell the difference.
+ if (replyRow.row() != 0)
+ return {};
+
+ view->selectionModel()->select(
+ replyRow, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows);
+ view->setCurrentIndex(replyRow);
+ QApplication::processEvents();
+ return replyRow;
+}
+
+void TestMainWindow::deleteOnAReplyReadsItsOwnThreadNotTheFirstInTheList()
+{
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+ auto *action = window.findChild<QAction *>(QStringLiteral("delete"));
+ QVERIFY(action);
+
+ // t1 deleted, t2 not. Reading t1's state for a reply of t2 makes the
+ // toggle choose UNDELETE for a thread that was never deleted.
+ const QModelIndex reply = expandSecondThreadAndSelectItsReply(
+ view, model, { QStringLiteral("deleted") }, {});
+ QVERIFY2(reply.isValid(),
+ "the fixture did not produce a reply row at row 0, so this test "
+ "would assert nothing about item 88's trap");
+
+ action->trigger();
+
+ // Delete, because the message's own thread is not deleted. The write goes
+ // through scopeFor() and lands on the message either way; what is under
+ // test is the DIRECTION, which is chosen from the state that was read.
+ QVERIFY2(window.pendingMessageIdsForTesting().contains(
+ QStringLiteral("m1@example.org")),
+ "Delete on a reply did not act on that reply");
+ QCOMPARE(window.undoDepthForTesting(), 1);
+ QVERIFY2(window.undoTextForTesting().contains(QStringLiteral("Delete")),
+ qPrintable(QStringLiteral(
+ "Delete on a reply of an undeleted thread chose "
+ "the wrong direction: %1. It read the FIRST "
+ "thread's state, which is deleted.")
+ .arg(window.undoTextForTesting())));
+}
+
+void TestMainWindow::toggleUnreadOnAReplyReadsItsOwnThreadNotTheFirstInTheList()
+{
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+ auto *action = window.findChild<QAction *>(QStringLiteral("toggle_unread"));
+ QVERIFY(action);
+
+ // t1 unread, t2 read. Reading t1's state marks an already-read message
+ // read again, which is a no-op write the user sees as a dead key.
+ const QModelIndex reply = expandSecondThreadAndSelectItsReply(
+ view, model, { QStringLiteral("unread") }, {});
+ QVERIFY2(reply.isValid(), "the fixture did not produce a reply row at row 0");
+
+ action->trigger();
+
+ QCOMPARE(window.undoDepthForTesting(), 1);
+ QVERIFY2(window.undoTextForTesting().contains(QStringLiteral("Mark unread")),
+ qPrintable(QStringLiteral(
+ "Toggle unread on a reply of a READ thread chose "
+ "the wrong direction: %1. It read the FIRST "
+ "thread's state, which is unread.")
+ .arg(window.undoTextForTesting())));
+}
+
+void TestMainWindow::editTagsOnAReplyCountsItsOwnThreadNotTheFirstInTheList()
+{
+ // The tag dialog is modal, so what is tested is the count it is BUILT
+ // from. Those counts drive its tri-state checkboxes, so a wrong count
+ // offers to remove a tag the message does not carry and shows the ones it
+ // does as unset.
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+
+ const QModelIndex reply = expandSecondThreadAndSelectItsReply(
+ view, model, { QStringLiteral("t1only") }, { QStringLiteral("t2only") });
+ QVERIFY2(reply.isValid(), "the fixture did not produce a reply row at row 0");
+
+ const QHash<QString, int> counts = window.selectionTagCountsForTesting();
+
+ QVERIFY2(counts.contains(QStringLiteral("t2only")),
+ "the tag dialog would not offer the tag the selected reply "
+ "actually carries");
+ QVERIFY2(!counts.contains(QStringLiteral("t1only")),
+ "the tag dialog counted the FIRST thread's tags for a reply of "
+ "the second, so it would offer to remove a tag that is not there");
+}
+
+void TestMainWindow::markCurrentThreadReadResolvesTheThreadThroughTheIndex()
+{
+ // Item 87 is blocked on this and will scope the write to one message. Today
+ // an unrelated guard hides the defect: onThreadSelected clears
+ // m_currentThreadId for a message row, so markCurrentThreadRead returns
+ // before it can read the wrong thread. That guard is not the protection
+ // this needs, and item 87 does not remove it, so the assertion here is on
+ // the resolution itself rather than on a write that cannot currently
+ // happen.
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+
+ const QModelIndex reply = expandSecondThreadAndSelectItsReply(
+ view, model, { QStringLiteral("unread") }, { QStringLiteral("unread") });
+ QVERIFY2(reply.isValid(), "the fixture did not produce a reply row at row 0");
+
+ // The question the timer's handler asks, in isolation: which thread is the
+ // current row part of. With the row number this answers "t1" for a reply
+ // of t2.
+ QCOMPARE(window.threadForCurrentRowForTesting().threadId,
+ QStringLiteral("t2"));
+}
+
+void TestMainWindow::deletingAReplyRepaintsThatReplyRow()
+{
+ // The user's report, at the gesture level: "I'm hitting delete on a reply
+ // to a thread, I see the edits counter increasing but I have no feedback
+ // if that message is being deleted." The model-level test proves
+ // applyMessageTagChange works; this proves the action reaches it, which is
+ // the half that was actually missing.
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+ auto *action = window.findChild<QAction *>(QStringLiteral("delete"));
+ QVERIFY(action);
+
+ const QModelIndex reply =
+ expandSecondThreadAndSelectItsReply(view, model, {}, {});
+ QVERIFY2(reply.isValid(), "the fixture did not produce a reply row at row 0");
+
+ // Nothing to see before the gesture, so the assertion after it means
+ // something.
+ QVERIFY(!model->messageAt(reply).isDeleted());
+ const QVariant before = model->data(reply, Qt::BackgroundRole);
+
+ QSignalSpy spy(model, &QAbstractItemModel::dataChanged);
+ action->trigger();
+
+ QVERIFY2(model->messageAt(reply).isDeleted(),
+ "Delete on a reply left the reply's own row unchanged, so the "
+ "pending count moved and the user saw nothing");
+ QVERIFY2(spy.count() >= 1, "no repaint was requested for the reply's row");
+ QVERIFY2(model->data(reply, Qt::BackgroundRole) != before,
+ "the deleted reply paints exactly as it did before");
+
+ // The THREAD row must not follow: it stands for the whole conversation,
+ // and one deleted reply does not doom it.
+ const QModelIndex threadRow = reply.parent();
+ QVERIFY2(!model->threadFor(threadRow).isDeleted(),
+ "deleting one reply marked its whole thread deleted");
+}
+
+void TestMainWindow::toggleUnreadOnAReplyReadsTheReplysOwnState()
+{
+ // The user's report: "read/unread still doesn't trigger a repaint of the
+ // reply". The write was already message-scoped and the model already
+ // repaints a message row, so neither was the fault. The DIRECTION was:
+ // the action read threadFor(current).isUnread(), the THREAD's state, even
+ // when the selected row is a reply.
+ //
+ // The consequence is a dead key rather than a wrong write. On a read
+ // thread the answer is always "add unread", so pressing it on an
+ // already-unread reply re-adds a tag it has, which is a no-op the model
+ // correctly declines to repaint. Item 88 fixed WHICH thread this reads;
+ // this is about reading a message at all.
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+ auto *action = window.findChild<QAction *>(QStringLiteral("toggle_unread"));
+ QVERIFY(action);
+
+ // A thread that is READ carrying a reply that is UNREAD. That disagreement
+ // is the whole test: with the thread's state the answer is "mark unread",
+ // with the message's it is "mark read".
+ const QModelIndex reply = expandSecondThreadAndSelectItsReply(
+ view, model, {}, {}, QStringList{ QStringLiteral("unread") });
+ QVERIFY2(reply.isValid(), "the fixture did not produce a reply row at row 0");
+ QVERIFY(model->messageAt(reply).isUnread());
+ QVERIFY2(!model->threadFor(reply).isUnread(),
+ "the fixture's thread is unread too, so this test cannot tell the "
+ "two sources apart");
+
+ action->trigger();
+
+ QVERIFY2(!model->messageAt(reply).isUnread(),
+ "Toggle unread on an unread reply did not mark it read: the "
+ "direction came from the THREAD, which is already read, so it "
+ "re-added a tag the reply already had and nothing changed");
+ QVERIFY2(window.undoTextForTesting().contains(QStringLiteral("Mark read")),
+ qPrintable(QStringLiteral("wrong direction: %1")
+ .arg(window.undoTextForTesting())));
+}
+
+void TestMainWindow::toggleUnreadOnAReplyRepaintsItInBothDirections()
+{
+ // Visible BOTH ways. The user reached the repaint only by deleting and
+ // undoing, which is a different write forcing the row to redraw; the
+ // unread change itself has to do it on its own, in each direction.
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+ auto *action = window.findChild<QAction *>(QStringLiteral("toggle_unread"));
+ QVERIFY(action);
+
+ const QModelIndex reply = expandSecondThreadAndSelectItsReply(
+ view, model, {}, {}, QStringList{ QStringLiteral("unread") });
+ QVERIFY(reply.isValid());
+
+ const QVariant unreadForeground = model->data(reply, Qt::ForegroundRole);
+ const QVariant unreadFont = model->data(reply, Qt::FontRole);
+
+ QSignalSpy spy(model, &QAbstractItemModel::dataChanged);
+ action->trigger();
+
+ QVERIFY2(spy.count() >= 1, "marking a reply read requested no repaint");
+ QVERIFY2(model->data(reply, Qt::ForegroundRole) != unreadForeground,
+ "a reply marked read paints exactly as it did while unread");
+ QVERIFY2(model->data(reply, Qt::FontRole) != unreadFont,
+ "a reply marked read keeps the unread font");
+
+ // And back. A toggle that is only visible one way is half a toggle.
+ spy.clear();
+ action->trigger();
+ QVERIFY(model->messageAt(reply).isUnread());
+ QVERIFY2(spy.count() >= 1, "marking a reply unread again requested no repaint");
+ QCOMPARE(model->data(reply, Qt::ForegroundRole), unreadForeground);
+ QCOMPARE(model->data(reply, Qt::FontRole), unreadFont);
+}
+
+void TestMainWindow::taggingTheOpenReplyUpdatesTheMessagePaneStrip()
+{
+ // The user's report: "the right pane chips are not [repainted], for it to
+ // sync I have to change message and go back to the edited one".
+ //
+ // sendThreadTagChange refreshes the strip when the edited thread is the
+ // open one. sendMessageTagChange had no equivalent, so a message-scoped
+ // write updated the list row and left the pane's chips describing the
+ // message as it was before the edit.
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+ auto *strip = window.findChild<TagStrip *>();
+ QVERIFY2(strip, "no tag strip in the message pane");
+
+ // visible + hidden: TagStrip collapses what does not fit into a "+N" chip,
+ // and an unshown window has no width, so visibleTags() alone measures the
+ // layout rather than the data.
+ const auto stripTags = [strip]() {
+ return strip->visibleTags() + strip->hiddenTags();
+ };
+ auto *action = window.findChild<QAction *>(QStringLiteral("delete"));
+ QVERIFY(action);
+
+ // A tag the strip will actually draw. Account tags are filtered out by the
+ // strip, and `unread` and `inbox` are hidden on the card but not here, so
+ // the fixture uses a plain functional tag to keep the assertion honest.
+ const QModelIndex reply = expandSecondThreadAndSelectItsReply(
+ view, model, {}, {}, QStringList{ QStringLiteral("todo") });
+ QVERIFY2(reply.isValid(), "the fixture did not produce a reply row at row 0");
+
+ // Selecting the reply is what puts it in the pane, and the strip has to be
+ // showing the reply's own tags before the edit or this asserts nothing.
+ QVERIFY2(stripTags().contains(QStringLiteral("todo")),
+ "the strip does not show the selected reply's tags, so this test "
+ "cannot tell a missing refresh from a strip that never had them");
+ QVERIFY(!stripTags().contains(QStringLiteral("deleted")));
+
+ action->trigger();
+
+ QVERIFY2(stripTags().contains(QStringLiteral("deleted")),
+ "the message pane's chips still describe the reply as it was "
+ "before the edit; the user has to select away and back to see it");
+}
+
+void TestMainWindow::taggingAnUnrelatedReplyLeavesTheStripAlone()
+{
+ // The guard, not the refresh. The strip describes the message ON DISPLAY,
+ // so a write to a different message must not repaint it with that
+ // message's tags. The thread path has the same guard, keyed on
+ // m_currentThreadId.
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+ auto *strip = window.findChild<TagStrip *>();
+ QVERIFY(strip);
+
+ // visible + hidden: TagStrip collapses what does not fit into a "+N" chip,
+ // and an unshown window has no width, so visibleTags() alone measures the
+ // layout rather than the data.
+ const auto stripTags = [strip]() {
+ return strip->visibleTags() + strip->hiddenTags();
+ };
+
+ ThreadSummary thread = makeThread(QStringLiteral("t1"), {});
+ thread.totalCount = 3;
+ model->appendBatch({ thread });
+
+ MessageNode root;
+ root.messageId = QStringLiteral("m0@example.org");
+ root.threadId = QStringLiteral("t1");
+ root.depth = 0;
+ MessageNode first;
+ first.messageId = QStringLiteral("m1@example.org");
+ first.threadId = QStringLiteral("t1");
+ first.tags = QStringList{ QStringLiteral("todo") };
+ first.depth = 1;
+ MessageNode second;
+ second.messageId = QStringLiteral("m2@example.org");
+ second.threadId = QStringLiteral("t1");
+ second.tags = QStringList{ QStringLiteral("later") };
+ second.depth = 1;
+ model->setThreadMessages(QStringLiteral("t1"), { root, first, second });
+
+ const QModelIndex threadRow = model->index(0, 0, QModelIndex());
+ view->expand(threadRow);
+
+ // The FIRST reply is the one on display.
+ const QModelIndex displayed = model->index(0, 0, threadRow);
+ view->selectionModel()->select(
+ displayed,
+ QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows);
+ view->setCurrentIndex(displayed);
+ QApplication::processEvents();
+ QVERIFY(stripTags().contains(QStringLiteral("todo")));
+
+ // A write to the OTHER reply, reaching the send path directly: driving it
+ // through the action would move the selection and change what is on
+ // display, which is the thing being held still.
+ window.sendMessageTagChangeForTesting({ QStringLiteral("m2@example.org") },
+ { QStringLiteral("deleted") }, {},
+ QStringLiteral("Delete"));
+
+ QVERIFY2(!stripTags().contains(QStringLiteral("deleted")),
+ "the strip took on the tags of a message that is not the one in "
+ "the pane");
+ QVERIFY2(stripTags().contains(QStringLiteral("todo")),
+ "the strip stopped describing the message on display");
+}
+
+void TestMainWindow::aHeldMessageEditIsSentWhenTheSyncEnds()
+{
+ // Found by reading while fixing the strip refresh, not reported.
+ //
+ // flushHeldEdits() looped over edit.threadIds and called
+ // sendThreadTagChange() only. A message-scoped edit held during a sync
+ // carries no thread ids at all, so the loop did nothing, the send
+ // early-returned on an empty list, and the edit was DROPPED: applied
+ // optimistically to the row, counted as unsynced, and never written. The
+ // user would have seen the change, been told it was pending, and lost it.
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+ auto *action = window.findChild<QAction *>(QStringLiteral("delete"));
+ QVERIFY(action);
+
+ const QModelIndex reply =
+ expandSecondThreadAndSelectItsReply(view, model, {}, {});
+ QVERIFY(reply.isValid());
+
+ QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
+ Q_ARG(SyncMonitor::State,
+ SyncMonitor::State::Running));
+ action->trigger();
+ QVERIFY2(window.hasEditAwaitingSend(),
+ "a message edit made during a sync was not held");
+
+ QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
+ Q_ARG(SyncMonitor::State,
+ SyncMonitor::State::Idle));
+
+ QVERIFY2(!window.hasEditAwaitingSend(),
+ "the sync ending did not send the held message edit, so it was "
+ "silently dropped: shown on the row, counted as pending, never "
+ "written");
+
+ // Sent for the MESSAGE, not escalated to its thread. Losing the scope on
+ // the way out of the hold would delete every message in the thread.
+ QVERIFY2(window.pendingMessageIdsForTesting().contains(
+ QStringLiteral("m1@example.org")),
+ "the held edit was not sent with its message scope");
+ QVERIFY2(window.pendingThreadIdsForTesting().isEmpty(),
+ "a held MESSAGE edit was sent as a thread edit, which would tag "
+ "every message in the thread");
+
+ // And the row still shows it: the flush takes the optimistic update back
+ // before re-sending, so a bug there leaves the row wrong in the other
+ // direction.
+ QVERIFY2(model->messageAt(reply).isDeleted(),
+ "sending the held edit lost the tag from the reply's row");
+}
+
+void TestMainWindow::anActionOnAThreadRowActsOnTheMessageItDisplays()
+{
+ // Item 108, the whole point of it. A root card renders ONE message since
+ // item 66, so acting on it acts on that message; the conversation is
+ // reached through the explicit thread actions.
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+
+ ThreadSummary t = makeThread(QStringLiteral("t1"), {});
+ t.totalCount = 7;
+ model->appendBatch({ t });
+ selectThreadRow(view, 0);
+
+ auto *deleteAction = window.findChild<QAction *>(QStringLiteral("delete"));
+ QVERIFY(deleteAction);
+ deleteAction->trigger();
+
+ QCOMPARE(window.pendingMessageIdsForTesting(),
+ QStringList{ QStringLiteral("t1-first@example.org") });
+ QVERIFY2(window.pendingThreadIdsForTesting().isEmpty(),
+ "the ordinary Delete still acted on the whole thread, so it "
+ "touched six messages the card does not display");
+
+ // The thread action is how the conversation is reached, and it must still
+ // work from the same selection.
+ auto *deleteThread =
+ window.findChild<QAction *>(QStringLiteral("delete_thread"));
+ QVERIFY(deleteThread);
+ deleteThread->trigger();
+
+ QCOMPARE(window.pendingThreadIdsForTesting(),
+ QStringList{ QStringLiteral("t1") });
+
+ // Two commands, one per gesture, each recording the scope it used: a thread
+ // action that pushed the message command would undo a fraction of what it
+ // did.
+ QCOMPARE(window.undoDepthForTesting(), 2);
+}
+
+void TestMainWindow::theThreadSubmenuIsReachableFromBothMenus()
+{
+ // The user asked for "a submenu when right clicking and the same submenu
+ // under Message in the top menu". Both, not one: the context menu is where
+ // the gesture starts and the menu bar is where a shortcut is discovered.
+ //
+ // A QMenu belongs to ONE menu tree, so these are two instances holding the
+ // same actions. Adding a single instance to both silently gives it to
+ // whichever added it last, which is the failure this pins.
+ const Config config;
+ MainWindow window(config);
+
+ auto *context =
+ window.findChild<QMenu *>(QStringLiteral("threadContextMenu"));
+ QVERIFY(context);
+
+ const QStringList expected = {
+ QStringLiteral("archive_thread"),
+ QStringLiteral("delete_thread"),
+ QStringLiteral("spam_thread"),
+ QStringLiteral("toggle_unread_thread"),
+ QStringLiteral("flag_thread"),
+ };
+
+ // Every submenu instance in the window, wherever it was added.
+ const QList<QMenu *> submenus =
+ window.findChildren<QMenu *>(QStringLiteral("threadActionsMenu"));
+ QVERIFY2(submenus.size() >= 2,
+ qPrintable(QStringLiteral("expected the thread submenu in both "
+ "the context menu and the menu bar, "
+ "found %1 instance(s)")
+ .arg(submenus.size())));
+
+ for (QMenu *menu : submenus) {
+ QStringList names;
+ for (QAction *action : menu->actions()) {
+ if (!action->isSeparator())
+ names.append(action->objectName());
+ }
+ QCOMPARE(names, expected);
+ }
+
+ // One of them is the context menu's own, reached as a submenu rather than
+ // as a loose action.
+ bool inContextMenu = false;
+ for (QAction *action : context->actions()) {
+ if (action->menu()
+ && action->menu()->objectName()
+ == QStringLiteral("threadActionsMenu")) {
+ inContextMenu = true;
+ break;
+ }
+ }
+ QVERIFY2(inContextMenu,
+ "right-clicking a thread offers no whole-thread submenu");
+}
+
+void TestMainWindow::autoMarkReadTouchesOnlyTheMessageOnDisplay()
+{
+ // Item 87, reported 2026-08-14: "with the first message in a thread
+ // selected (not expanded), the 2s delay that marks it read applies to the
+ // whole thread, so all answers are marked read as well."
+ //
+ // Not cosmetic. maildir.synchronize_flags is on, so removing `unread`
+ // rewrites Maildir filenames and the next sync carries it to the server:
+ // mail the user never opened stops being unread everywhere, and nothing
+ // here can put it back except reading it again by hand.
+ QTemporaryDir dir;
+ QVERIFY(dir.isValid());
+ const QString path = dir.filePath(QStringLiteral("qtmaildir.conf"));
+ QFile file(path);
+ QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text));
+ file.write("[general]\nmark_read_delay_ms = 0\n");
+ file.close();
+
+ Config config;
+ config.load(path);
+ QCOMPARE(config.markReadDelayMs(), 0);
+
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+ auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer"));
+ QVERIFY(timer);
+
+ ThreadSummary t = makeThread(QStringLiteral("t1"),
+ { QStringLiteral("unread") });
+ t.totalCount = 7;
+ model->appendBatch({ t });
+
+ selectThreadRow(view, 0);
+ QApplication::processEvents();
+ QVERIFY2(timer->isActive() || !window.pendingMessageIdsForTesting().isEmpty(),
+ "selecting an unread thread armed no mark-read at all");
+
+ // Fire it. A zero-interval timer still goes through the event loop.
+ QTRY_VERIFY_WITH_TIMEOUT(!timer->isActive(), 2000);
+ QApplication::processEvents();
+
+ // ONE message, the one the card renders, and named rather than merely
+ // counted: a thread of seven whose first message is the target is exactly
+ // the case where a count of one could still be the wrong one.
+ QCOMPARE(window.pendingMessageIdsForTesting(),
+ QStringList{ QStringLiteral("t1-first@example.org") });
+ QVERIFY2(window.pendingThreadIdsForTesting().isEmpty(),
+ "the automatic mark-read still wrote to the whole thread, so six "
+ "messages the user never displayed were marked read and the next "
+ "sync carries that to the server");
+
+ // Still not on the undo stack. The user never took this action, so
+ // hijacking Ctrl+Z to reverse it would undo something they did not do.
+ QCOMPARE(window.undoDepthForTesting(), 0);
+}
+
+void TestMainWindow::autoMarkReadArmsForAReplyToo()
+{
+ // Selecting a reply displays that message, so the same rule applies to it.
+ // Before item 87 the timer was deliberately not armed for a message row,
+ // because the write it would have made was thread-scoped and would have
+ // marked the whole conversation read. With the write scoped to one message
+ // that objection is gone, and leaving it unarmed would mean the message
+ // the user is reading is the one kind that never gets marked read.
+ QTemporaryDir dir;
+ QVERIFY(dir.isValid());
+ const QString path = dir.filePath(QStringLiteral("qtmaildir.conf"));
+ QFile file(path);
+ QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text));
+ file.write("[general]\nmark_read_delay_ms = 0\n");
+ file.close();
+
+ Config config;
+ config.load(path);
+
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+ auto *timer = window.findChild<QTimer *>(QStringLiteral("markReadTimer"));
+ QVERIFY(timer);
+
+ // The reply is unread; its thread is not, so a thread-keyed timer would
+ // have declined to arm at all.
+ //
+ // No "is it still unread" guard before the wait: the delay is 0 and the
+ // helper pumps the event loop, so the write has already happened by the
+ // time selection returns. The assertions below are on the write itself,
+ // which is what this test is about, and the fixture above is what
+ // establishes the reply started unread.
+ const QModelIndex reply = expandSecondThreadAndSelectItsReply(
+ view, model, {}, {}, QStringList{ QStringLiteral("unread") });
+ QVERIFY(reply.isValid());
+
+ QTRY_VERIFY_WITH_TIMEOUT(!timer->isActive(), 2000);
+ QApplication::processEvents();
+
+ QCOMPARE(window.pendingMessageIdsForTesting(),
+ QStringList{ QStringLiteral("m1@example.org") });
+ QVERIFY2(window.pendingThreadIdsForTesting().isEmpty(),
+ "reading one reply marked its whole thread read");
+
+ // And the row shows it, which is the half item 105 built.
+ QVERIFY2(!model->messageAt(reply).isUnread(),
+ "the reply was marked read without its row following");
+}
+
+void TestMainWindow::taggingTheOpenRootMessageKeepsTheStripPopulated()
+{
+ // The user, 2026-08-16: "right pane loses the chip row when repainting, it
+ // simply disappears".
+ //
+ // The strip refresh added for item 105 reads the message's tags through
+ // messageById(), which searches only the loaded CHILDREN. A root card's
+ // message is never among them, so the lookup returned a default-constructed
+ // node and the refresh set the strip to that node's empty tag list, wiping
+ // a strip that had been correct a moment earlier. Worse than not
+ // refreshing: it actively destroyed what was there.
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+ auto *strip = window.findChild<TagStrip *>();
+ QVERIFY(strip);
+
+ ThreadSummary t = makeThread(QStringLiteral("t1"),
+ { QStringLiteral("todo") });
+ t.totalCount = 1;
+ model->appendBatch({ t });
+
+ selectThreadRow(view, 0);
+ QApplication::processEvents();
+
+ // visible + hidden, not visible alone. TagStrip is a single row that
+ // collapses whatever does not fit into a trailing "+N" chip, and this
+ // window is never shown, so it has no width to lay out with and puts
+ // almost everything in the hidden half. Asserting on visibleTags() alone
+ // measures the layout, not the data, and fails for a reason this test does
+ // not care about.
+ const auto stripTags = [strip]() {
+ return strip->visibleTags() + strip->hiddenTags();
+ };
+
+ // The guard: the strip has to be showing something before the edit, or
+ // this cannot tell "wiped" from "never populated".
+ QVERIFY2(stripTags().contains(QStringLiteral("todo")),
+ "the strip never showed the selected thread's tags");
+
+ auto *action = window.findChild<QAction *>(QStringLiteral("delete"));
+ QVERIFY(action);
+ action->trigger();
+
+ QVERIFY2(!stripTags().isEmpty(),
+ "the chip row was emptied: the refresh looked the message up "
+ "among the loaded replies, where a root card's message never is, "
+ "and set the strip to the resulting empty tag list");
+ QVERIFY2(stripTags().contains(QStringLiteral("todo")),
+ "the strip lost the tag the message still carries");
+ QVERIFY2(stripTags().contains(QStringLiteral("deleted")),
+ "the strip did not pick up the tag just written");
+}
+
+void TestMainWindow::aLoadedMessageCorrectsTheStripFromTheThreadsUnion()
+{
+ // Reported by hand, 2026-08-16, against a real four-message thread whose
+ // root carried `unread` and whose THIRD message carried `signed`:
+ // "the right pane chips update and both signed and unread disappear ...
+ // changing message and going back makes them reappear".
+ //
+ // Neither half was the write's doing. Selecting a thread row sets the strip
+ // from ThreadSummary::tags, which is notmuch's UNION over the thread, so
+ // the pane claimed the root message was `signed` when a sibling was. The
+ // mark-read write then replaced it with the root's real tags, correctly
+ // dropping both, and reselecting put the union back. The pane was lying
+ // BEFORE the write, not after it.
+ //
+ // The load is the authority: MessageRef carries the message's own tags.
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+ auto *strip = window.findChild<TagStrip *>();
+ QVERIFY(strip);
+
+ const auto stripTags = [strip]() {
+ return strip->visibleTags() + strip->hiddenTags();
+ };
+
+ // The union carries `signed`; the root message does not.
+ ThreadSummary t = makeThread(QStringLiteral("t1"),
+ { QStringLiteral("inbox"),
+ QStringLiteral("signed"),
+ QStringLiteral("unread") });
+ t.totalCount = 4;
+ model->appendBatch({ t });
+
+ selectThreadRow(view, 0);
+ QApplication::processEvents();
+
+ // Before the load the strip can only show the union, which is what the
+ // model holds. That is the state the user saw and reported.
+ QVERIFY(stripTags().contains(QStringLiteral("signed")));
+
+ // The worker answers with the message's OWN tags.
+ MessageRef ref;
+ ref.messageId = QStringLiteral("t1-first@example.org");
+ ref.tags = QStringList{ QStringLiteral("inbox"), QStringLiteral("unread") };
+ QMetaObject::invokeMethod(
+ &window, "onMessageLoaded", Qt::DirectConnection,
+ Q_ARG(QVector<MessageRef>, QVector<MessageRef>{ ref }),
+ Q_ARG(quint64, window.currentGenerationForTesting()));
+ QApplication::processEvents();
+
+ QVERIFY2(!stripTags().contains(QStringLiteral("signed")),
+ "the pane still claims the root message is signed, which is a "
+ "sibling's tag: it is showing the thread's union rather than the "
+ "message on display");
+ QVERIFY2(stripTags().contains(QStringLiteral("unread")),
+ "the pane lost a tag the message really carries");
+}
+
+void TestMainWindow::aLoadedRootMessageGivesTheCardItsOwnTags()
+{
+ // The same correction, reaching the MODEL, which is what fixes the two
+ // repaint reports: "if I mark the root message read the left pane entry
+ // doesn't repaint (stays bold)" and the same for delete.
+ //
+ // The card could not repaint because the model had no per-message tags for
+ // a root at all, so a message-scoped write updated the thread summary only
+ // when the thread was a single message. A load gives the root the same
+ // per-message node a reply has had all along, and from then on the card
+ // draws the message it displays.
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+
+ ThreadSummary t = makeThread(QStringLiteral("t1"),
+ { QStringLiteral("inbox"),
+ QStringLiteral("signed"),
+ QStringLiteral("unread") });
+ t.totalCount = 4;
+ model->appendBatch({ t });
+
+ selectThreadRow(view, 0);
+ QApplication::processEvents();
+
+ MessageRef ref;
+ ref.messageId = QStringLiteral("t1-first@example.org");
+ ref.tags = QStringList{ QStringLiteral("inbox"), QStringLiteral("unread") };
+ QMetaObject::invokeMethod(
+ &window, "onMessageLoaded", Qt::DirectConnection,
+ Q_ARG(QVector<MessageRef>, QVector<MessageRef>{ ref }),
+ Q_ARG(quint64, window.currentGenerationForTesting()));
+ QApplication::processEvents();
+
+ // The model now knows what the root message itself carries.
+ const MessageNode root =
+ model->messageById(QStringLiteral("t1-first@example.org"));
+ QCOMPARE(root.messageId, QStringLiteral("t1-first@example.org"));
+ QVERIFY2(!root.tags.contains(QStringLiteral("signed")),
+ "the root's node still carries a sibling's tag");
+
+ const QModelIndex threadIndex = model->index(0, 0, QModelIndex());
+ QSignalSpy spy(model, &QAbstractItemModel::dataChanged);
+
+ auto *toggle = window.findChild<QAction *>(QStringLiteral("toggle_unread"));
+ QVERIFY(toggle);
+ toggle->trigger();
+
+ QVERIFY2(spy.count() >= 1, "marking the root message read repainted nothing");
+ QVERIFY2(!model->messageById(QStringLiteral("t1-first@example.org"))
+ .isUnread(),
+ "the root message is still unread after being marked read");
+
+ // The card now draws that message, so it stops looking unread. Asserted on
+ // the FONT, which is what the user means by "stays bold".
+ QVERIFY2(!model->data(threadIndex, Qt::FontRole).value<QFont>().bold(),
+ "the card still reads as unread, so the row stays bold and the "
+ "user sees nothing");
+ QVERIFY2(model->threadAt(0).isUnread(),
+ "the thread summary was rewritten, claiming a four-message thread "
+ "is read when three of its messages still are not");
+}
+
void TestMainWindow::aTransientStatusMessageExpires()
{
// "Sync complete" describes an event, not a state, and reads as though it
@@ -4796,7 +5716,10 @@ void TestMainWindow::anEditDuringABackgroundSyncIsNotSentYet()
QVERIFY(model);
auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
- auto *action = window.findChild<QAction *>(QStringLiteral("flag"));
+ // The THREAD action: this test asserts on the thread ROW, which a
+ // message-scoped write deliberately leaves alone since item 108. What
+ // is under test is the HOLD, which is identical either way.
+ auto *action = window.findChild<QAction *>(QStringLiteral("flag_thread"));
QVERIFY2(action, "no flag action registered");
model->appendBatch({ makeThread(QStringLiteral("t1"), {}) });
@@ -4827,7 +5750,10 @@ void TestMainWindow::aHeldEditIsSentWhenTheBackgroundSyncEnds()
QVERIFY(model);
auto *view = window.findChild<QTreeView *>();
QVERIFY(view);
- auto *action = window.findChild<QAction *>(QStringLiteral("flag"));
+ // The THREAD action: this test asserts on the thread ROW, which a
+ // message-scoped write deliberately leaves alone since item 108. What
+ // is under test is the HOLD, which is identical either way.
+ auto *action = window.findChild<QAction *>(QStringLiteral("flag_thread"));
QVERIFY(action);
model->appendBatch({ makeThread(QStringLiteral("t1"), {}) });
@@ -5360,15 +6286,19 @@ void TestMainWindow::theImportantActionStillWritesTheFlaggedTag()
QVERIFY(action);
action->trigger();
- // sendThreadTagChange() applies the change to the model optimistically, so
- // the tag the action really wrote is observable here without a worker.
- QVERIFY2(model->threadAt(0).isFlagged(),
+ // Asserted on the CHANGE that was sent rather than on the thread's row.
+ // Since item 108 this action is message-scoped, so it writes to the
+ // message the card displays and the thread summary is deliberately left
+ // alone. The tag name is what this test is about, and the change carries
+ // it whichever scope the action uses.
+ const TagChange sent = window.pendingChangeForTesting();
+ QVERIFY2(sent.added.contains(QStringLiteral("flagged")),
"the renamed action no longer writes the `flagged` tag");
- QVERIFY2(model->threadAt(0).tags.contains(QStringLiteral("flagged")),
- "the tag written was not `flagged`");
- QVERIFY2(!model->threadAt(0).tags.contains(QStringLiteral("important")),
+ QVERIFY2(!sent.added.contains(QStringLiteral("important")),
"the rename reached the mail store: an `important` tag was "
"written, which no other tool reading this Maildir knows");
+ QVERIFY2(sent.removed.isEmpty(),
+ "marking important removed a tag, which it must not");
}
void TestMainWindow::theToolbarUsesTheConfiguredIconSize()
@@ -5756,20 +6686,59 @@ void TestMainWindow::noTwoActionsShareAnIcon()
// Compared by cacheKey() rather than by the theme NAME, which this window
// does not keep. Two distinct names that resolve to the same art on a given
// theme are just as ambiguous on screen, and that is what the user sees.
+ // Narrowed by item 108 to the actions that can reach the TOOLBAR, which is
+ // where the rule comes from: an icon-only toolbar makes the icon the whole
+ // control. The five whole-thread actions live only in the "Whole thread"
+ // submenu, whose entries always carry text, and each deliberately shares
+ // the icon of its message-scoped twin: same operation, wider scope, with
+ // the words saying which. Giving them five invented shapes would be less
+ // clear than the pairing.
+ //
+ // Named as an exception list rather than by asking the toolbar what it
+ // holds, so that PUTTING one of these on the toolbar fails this test
+ // rather than silently passing it.
+ static const QStringList menuOnlyThreadActions = {
+ QStringLiteral("archive_thread"),
+ QStringLiteral("delete_thread"),
+ QStringLiteral("spam_thread"),
+ QStringLiteral("toggle_unread_thread"),
+ QStringLiteral("flag_thread"),
+ };
+
const Config config;
MainWindow window(config);
+ // The exception must not become a hiding place: every one of them still
+ // has to carry an icon, which everyActionCarriesAnIcon asserts, and none
+ // may sit on the toolbar.
+ auto *toolBar = window.findChild<QToolBar *>();
+ QVERIFY(toolBar);
+ for (const QString &name : menuOnlyThreadActions) {
+ auto *action = window.findChild<QAction *>(name);
+ QVERIFY2(action, qPrintable(QStringLiteral("no action named %1").arg(name)));
+ QVERIFY2(!toolBar->actions().contains(action),
+ qPrintable(QStringLiteral("%1 is on the toolbar, where a "
+ "shared icon is ambiguous, so it "
+ "cannot be exempt from this rule")
+ .arg(name)));
+ }
+
QHash<qint64, QString> owners;
QStringList collisions;
int withIcons = 0;
+ int compared = 0;
for (const QString &name : KeyMap::knownActions()) {
auto *action = window.findChild<QAction *>(name);
QVERIFY2(action, qPrintable(QStringLiteral("no action named %1").arg(name)));
+ if (!action->icon().isNull())
+ ++withIcons;
+ if (menuOnlyThreadActions.contains(name))
+ continue;
if (action->icon().isNull())
continue;
+ ++compared;
- ++withIcons;
const qint64 key = action->icon().cacheKey();
const auto existing = owners.constFind(key);
if (existing != owners.constEnd()) {
@@ -5789,6 +6758,10 @@ void TestMainWindow::noTwoActionsShareAnIcon()
.arg(withIcons)
.arg(KeyMap::knownActions().size())));
+ // And the exception list did not swallow the comparison itself.
+ QCOMPARE(compared, KeyMap::knownActions().size()
+ - menuOnlyThreadActions.size());
+
QVERIFY2(collisions.isEmpty(),
qPrintable(QStringLiteral("actions sharing one icon: %1")
.arg(collisions.join(QStringLiteral("; ")))));
diff --git a/tests/test_notmuchworker.cpp b/tests/test_notmuchworker.cpp
index de96dbf..899fd11 100644
--- a/tests/test_notmuchworker.cpp
+++ b/tests/test_notmuchworker.cpp
@@ -52,6 +52,7 @@ private slots:
void applyTagsIgnoresUnknownMessageIds();
void applyTagsWithNoIdsDoesNothing();
void queryStillWorksAfterWrite();
+ void aThreadCarriesItsCardMessagesOwnTags();
void applyTagsToThreadsTagsEveryMessage();
void applyTagsToThreadsSpansMultipleThreads();
@@ -274,6 +275,55 @@ void TestNotmuchWorker::aQueryCarriesEachThreadsFirstMessageId()
QVERIFY2(sawTheThread, "the two-message thread was not in the results");
}
+void TestNotmuchWorker::aThreadCarriesItsCardMessagesOwnTags()
+{
+ // Item 111. A card draws its own message's tags at full size and the rest
+ // of the conversation's smaller, so it needs BOTH: `tags` is notmuch's
+ // union over the thread and `firstMessageTags` is the one message the card
+ // stands for.
+ //
+ // Derived from the message LOAD at first, which meant an unopened row had
+ // no split and drew everything as its own, correcting itself only when the
+ // user selected it. The user reported exactly that. The query knows, and
+ // the walk that finds firstMessageId is already holding the message, so
+ // this is the same index read rather than a second pass.
+
+ // A tag on the REPLY only, which is the case that separates the two: a1 is
+ // the card's message, a2 its reply.
+ NotmuchWorker writer(m_fixture.configPath());
+ writer.applyTags(TagChange{ { QStringLiteral("a2@example.org") },
+ { QStringLiteral("signed") },
+ {},
+ QStringLiteral("Sign the reply") });
+
+ const QVector<ThreadSummary> threads = runQuery(QStringLiteral("*"));
+ bool sawTheThread = false;
+ for (const ThreadSummary &t : threads) {
+ if (t.subject != QStringLiteral("Release notes"))
+ continue;
+ sawTheThread = true;
+
+ QCOMPARE(t.firstMessageId, QStringLiteral("a1@example.org"));
+
+ // The union carries the reply's tag, as notmuch reports it.
+ QVERIFY2(t.tags.contains(QStringLiteral("signed")),
+ "the thread's own tags stopped being the union, which the "
+ "sibling tier is derived from");
+
+ // The card's message does not, and this is the whole point: without it
+ // the card claims a tag belonging to a message it does not display.
+ QVERIFY2(!t.firstMessageTags.isEmpty(),
+ "the query carried no per-message tags, so an unopened row "
+ "has no split and draws every chip at full size");
+ QVERIFY2(!t.firstMessageTags.contains(QStringLiteral("signed")),
+ "the card's message was given its reply's tag");
+
+ // And it does carry its own.
+ QVERIFY(t.firstMessageTags.contains(QStringLiteral("inbox")));
+ }
+ QVERIFY2(sawTheThread, "the two-message thread was not in the results");
+}
+
void TestNotmuchWorker::aSentQueryCarriesTheMatchedMessageNotTheThreadsFirst()
{
// THE case the Sent branch exists for, and the one hardest to get right.
diff --git a/tests/test_threadlistmodel.cpp b/tests/test_threadlistmodel.cpp
index e338661..811b3e3 100644
--- a/tests/test_threadlistmodel.cpp
+++ b/tests/test_threadlistmodel.cpp
@@ -68,6 +68,20 @@ private slots:
void threadIdIsReachableFromAnIndex();
void invalidIndexesReturnNothing();
void threadAtOutOfRangeIsSafe();
+ void threadForResolvesAReplyThroughItsParent();
+ void applyMessageTagChangeRepaintsThatReplyAlone();
+ void aDeletedReplyIsPaintedAsDoomed();
+ void aDeletedReplyIsStruckThrough();
+ void markingAReplyReadChangesItsForeground();
+ void anUnreadReplyIsBoldAndStillSmallerThanItsThread();
+ void aThreadTagChangeReachesItsLoadedReplies();
+ void messageScopeResolvesAThreadRowToTheMessageItDisplays();
+ void messageScopeSkipsAThreadRowItCannotNameAMessageFor();
+ void aMessageTagChangeReachesTheRootCardsOwnMessage();
+ void aMessageTagChangeOnOneOfManyLeavesTheThreadSummaryAlone();
+ void aCardListsItsOwnTagsBeforeItsSiblings();
+ void theSplitIsKnownBeforeTheRowIsEverOpened();
+ void reconcileRefreshesASurvivorsOwnMessageTags();
void updatesTagsForMessage();
void tagChangeIsIdempotent();
void tagChangeSignalsExactlyTheChangedRow();
@@ -877,6 +891,605 @@ void TestThreadListModel::threadAtOutOfRangeIsSafe()
QCOMPARE(model.threadAt(0).threadId, QStringLiteral("t1"));
}
+void TestThreadListModel::threadForResolvesAReplyThroughItsParent()
+{
+ // Item 88. threadAt() takes a top-level row and a tree numbers rows per
+ // parent, so the first reply of ANY thread has row() == 0 and threadAt(0)
+ // answers "t1" for a reply of t2. threadFor() resolves through the parent
+ // instead, which is what every caller holding an index needs.
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("one")),
+ makeThread(QStringLiteral("t2"), QStringLiteral("two")) });
+ model.setThreadMessages(QStringLiteral("t2"),
+ { makeNode(QStringLiteral("m0@example.org"), 0),
+ makeNode(QStringLiteral("m1@example.org"), 1) });
+
+ const QModelIndex second = model.index(1, 0, QModelIndex());
+ QCOMPARE(model.threadFor(second).threadId, QStringLiteral("t2"));
+
+ const QModelIndex reply = model.index(0, 0, second);
+ QVERIFY(reply.isValid());
+ QVERIFY2(model.isMessageRow(reply),
+ "the fixture did not produce a message row");
+ QCOMPARE(reply.row(), 0); // The trap: a plausible top-level row number.
+
+ QCOMPARE(model.threadFor(reply).threadId, QStringLiteral("t2"));
+
+ // And the row-taking overload still does the wrong thing for that index,
+ // which is why it is documented as unsafe rather than merely deprecated.
+ QCOMPARE(model.threadAt(reply.row()).threadId, QStringLiteral("t1"));
+
+ // An invalid index gives an empty summary, which every caller treats as
+ // "nothing to do" rather than acting on row 0.
+ QVERIFY(model.threadFor(QModelIndex()).threadId.isEmpty());
+}
+
+void TestThreadListModel::applyMessageTagChangeRepaintsThatReplyAlone()
+{
+ // The user's report: hitting Delete or Ctrl+U on a reply moved the pending
+ // count and changed nothing on screen. sendMessageTagChange made no
+ // optimistic update at all, on the correct reasoning that repainting the
+ // THREAD row would claim every message in it had changed. The row that
+ // should have repainted is the reply's own.
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("one")) });
+
+ MessageNode root = makeNode(QStringLiteral("m0@example.org"), 0);
+ MessageNode reply = makeNode(QStringLiteral("m1@example.org"), 1);
+ reply.tags = QStringList{ QStringLiteral("unread") };
+ model.setThreadMessages(QStringLiteral("t1"), { root, reply });
+
+ const QModelIndex threadIndex = model.index(0, 0, QModelIndex());
+ const QModelIndex replyIndex = model.index(0, 0, threadIndex);
+ QVERIFY(model.isMessageRow(replyIndex));
+
+ const QStringList threadTagsBefore =
+ model.data(threadIndex, ThreadListModel::TagsRole).toStringList();
+
+ QSignalSpy spy(&model, &QAbstractItemModel::dataChanged);
+
+ model.applyMessageTagChange(QStringLiteral("m1@example.org"),
+ { QStringLiteral("deleted") },
+ { QStringLiteral("unread") });
+
+ // The node carries the change, which is what every reply-row role reads.
+ QCOMPARE(model.messageAt(replyIndex).isDeleted(), true);
+ QCOMPARE(model.messageAt(replyIndex).isUnread(), false);
+
+ // And the view was told, or the change is invisible until something else
+ // happens to repaint the row.
+ QCOMPARE(spy.count(), 1);
+ QCOMPARE(spy.at(0).at(0).toModelIndex(), replyIndex);
+
+ // The THREAD is untouched. Claiming the whole thread changed is the lie
+ // the missing update was avoiding, and it must stay avoided.
+ QCOMPARE(model.data(threadIndex, ThreadListModel::TagsRole).toStringList(),
+ threadTagsBefore);
+ QCOMPARE(model.messageAt(model.index(0, 0, threadIndex)).messageId,
+ QStringLiteral("m1@example.org"));
+
+ // An unknown message is a no-op rather than a wrong row repainted.
+ spy.clear();
+ model.applyMessageTagChange(QStringLiteral("nobody@example.org"),
+ { QStringLiteral("deleted") }, {});
+ QCOMPARE(spy.count(), 0);
+}
+
+void TestThreadListModel::aDeletedReplyIsPaintedAsDoomed()
+{
+ // Updating the node is not enough on its own: a reply row had no doomed
+ // branch at all, so a deleted reply repainted identically to an undeleted
+ // one and the user still saw nothing. The thread row has carried this cue
+ // since item 13.
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("one")) });
+
+ MessageNode root = makeNode(QStringLiteral("m0@example.org"), 0);
+ MessageNode reply = makeNode(QStringLiteral("m1@example.org"), 1);
+ model.setThreadMessages(QStringLiteral("t1"), { root, reply });
+
+ const QModelIndex threadIndex = model.index(0, 0, QModelIndex());
+ const QModelIndex replyIndex = model.index(0, 0, threadIndex);
+
+ const QVariant plainBackground =
+ model.data(replyIndex, Qt::BackgroundRole);
+
+ model.applyMessageTagChange(QStringLiteral("m1@example.org"),
+ { QStringLiteral("deleted") }, {});
+
+ const QVariant doomedBackground =
+ model.data(replyIndex, Qt::BackgroundRole);
+ QVERIFY2(doomedBackground != plainBackground,
+ "a deleted reply paints exactly like an undeleted one, so the "
+ "user has no way to see that Delete did anything");
+ QCOMPARE(doomedBackground.value<QBrush>().color(),
+ ThreadListModel::deletedColour());
+
+ // Spam is the other half of isDoomed() and gets its own colour, so the two
+ // are told apart by hue rather than by shade.
+ model.applyMessageTagChange(QStringLiteral("m1@example.org"),
+ { QStringLiteral("spam") },
+ { QStringLiteral("deleted") });
+ QCOMPARE(model.data(replyIndex, Qt::BackgroundRole).value<QBrush>().color(),
+ ThreadListModel::spamColour());
+}
+
+void TestThreadListModel::aDeletedReplyIsStruckThrough()
+{
+ // The fill is not the only cue, deliberately: a strike-out survives a
+ // screenshot, a colourblind reader and a theme that overrides the
+ // background. The thread row has carried both since item 13; a reply had
+ // neither.
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("one")) });
+ model.setThreadMessages(QStringLiteral("t1"),
+ { makeNode(QStringLiteral("m0@example.org"), 0),
+ makeNode(QStringLiteral("m1@example.org"), 1) });
+
+ const QModelIndex replyIndex =
+ model.index(0, 0, model.index(0, 0, QModelIndex()));
+
+ QVERIFY(!model.data(replyIndex, Qt::FontRole).value<QFont>().strikeOut());
+
+ model.applyMessageTagChange(QStringLiteral("m1@example.org"),
+ { QStringLiteral("deleted") }, {});
+
+ QVERIFY2(model.data(replyIndex, Qt::FontRole).value<QFont>().strikeOut(),
+ "a deleted reply is not struck through, so the only cue it has "
+ "is a background colour");
+
+ // The reply's smaller font is not lost to the strike-out branch: a reply
+ // reads as subordinate whatever its tags say.
+ const QFont threadFont =
+ model.data(model.index(0, 0, QModelIndex()), Qt::FontRole).value<QFont>();
+ const QFont replyFont =
+ model.data(replyIndex, Qt::FontRole).value<QFont>();
+ if (threadFont.pointSize() > 0 && replyFont.pointSize() > 0)
+ QVERIFY(replyFont.pointSize() < threadFont.pointSize());
+}
+
+void TestThreadListModel::markingAReplyReadChangesItsForeground()
+{
+ // The user's second report: marking a reply read or unread moved the
+ // counter with no visible change. A reply is deliberately NEVER bold, so
+ // unlike a thread row its only cue is the foreground dimming. That cue has
+ // to at least exist and change, which is what this pins.
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("one")) });
+
+ MessageNode root = makeNode(QStringLiteral("m0@example.org"), 0);
+ MessageNode reply = makeNode(QStringLiteral("m1@example.org"), 1);
+ reply.tags = QStringList{ QStringLiteral("unread") };
+ model.setThreadMessages(QStringLiteral("t1"), { root, reply });
+
+ const QModelIndex replyIndex =
+ model.index(0, 0, model.index(0, 0, QModelIndex()));
+
+ const QVariant unreadForeground =
+ model.data(replyIndex, Qt::ForegroundRole);
+
+ model.applyMessageTagChange(QStringLiteral("m1@example.org"), {},
+ { QStringLiteral("unread") });
+
+ const QVariant readForeground = model.data(replyIndex, Qt::ForegroundRole);
+ QVERIFY2(readForeground != unreadForeground,
+ "marking a reply read changed nothing about how its row paints");
+ QCOMPARE(readForeground.value<QBrush>().color(),
+ ThreadListModel::readColour());
+
+ // And back, so the toggle is visible in both directions rather than only
+ // on the way to read.
+ model.applyMessageTagChange(QStringLiteral("m1@example.org"),
+ { QStringLiteral("unread") }, {});
+ QCOMPARE(model.data(replyIndex, Qt::ForegroundRole), unreadForeground);
+}
+
+void TestThreadListModel::anUnreadReplyIsBoldAndStillSmallerThanItsThread()
+{
+ // Requested by the user on 2026-08-16: "I prefer the bold on replies
+ // combined with the dimming." Replies were deliberately never bold before
+ // that, so this pins the decision rather than describing the code.
+ //
+ // Both halves matter. Bold is the second cue, next to the dimming; the
+ // smaller size is what still separates a reply from the thread heading
+ // above it, and dropping it would make an unread reply indistinguishable
+ // from a thread row.
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("one")) });
+
+ MessageNode root = makeNode(QStringLiteral("m0@example.org"), 0);
+ MessageNode reply = makeNode(QStringLiteral("m1@example.org"), 1);
+ reply.tags = QStringList{ QStringLiteral("unread") };
+ model.setThreadMessages(QStringLiteral("t1"), { root, reply });
+
+ const QModelIndex threadIndex = model.index(0, 0, QModelIndex());
+ const QModelIndex replyIndex = model.index(0, 0, threadIndex);
+
+ const QFont unreadFont =
+ model.data(replyIndex, Qt::FontRole).value<QFont>();
+ QVERIFY2(unreadFont.bold(), "an unread reply is not bold");
+
+ const QFont threadFont =
+ model.data(threadIndex, Qt::FontRole).value<QFont>();
+ if (threadFont.pointSize() > 0 && unreadFont.pointSize() > 0) {
+ QVERIFY2(unreadFont.pointSize() < threadFont.pointSize(),
+ "a bold reply is the same size as its thread row, so the two "
+ "kinds of row no longer read apart");
+ }
+
+ // Bold is the unread cue specifically, not decoration on every reply.
+ model.applyMessageTagChange(QStringLiteral("m1@example.org"), {},
+ { QStringLiteral("unread") });
+ QVERIFY2(!model.data(replyIndex, Qt::FontRole).value<QFont>().bold(),
+ "a read reply is still bold, so bold says nothing");
+}
+
+void TestThreadListModel::aThreadTagChangeReachesItsLoadedReplies()
+{
+ // The user's report: "if I hit read/unread on the main thread message [...]
+ // only the main message is repainted [...] the replies don't get
+ // repainted."
+ //
+ // A thread-scoped write reaches every message in the thread IN THE
+ // DATABASE. applyTagChange only ever updated the thread's summary, so an
+ // expanded thread kept showing replies with their old tags: bold, undimmed
+ // and unstruck, describing a state the database no longer held. The rows
+ // corrected themselves on the next query, which is what made this look
+ // like a repaint problem rather than a stale-model one.
+ ThreadListModel model;
+ model.appendBatch({ makeThread(QStringLiteral("t1"), QStringLiteral("one")) });
+
+ MessageNode root = makeNode(QStringLiteral("m0@example.org"), 0);
+ root.tags = QStringList{ QStringLiteral("unread") };
+ MessageNode reply = makeNode(QStringLiteral("m1@example.org"), 1);
+ reply.tags = QStringList{ QStringLiteral("unread") };
+ model.setThreadMessages(QStringLiteral("t1"), { root, reply });
+
+ const QModelIndex threadIndex = model.index(0, 0, QModelIndex());
+ const QModelIndex replyIndex = model.index(0, 0, threadIndex);
+ QVERIFY(model.messageAt(replyIndex).isUnread());
+
+ QSignalSpy spy(&model, &QAbstractItemModel::dataChanged);
+
+ model.applyTagChange(QStringLiteral("t1"), {},
+ { QStringLiteral("unread") });
+
+ QVERIFY2(!model.messageAt(replyIndex).isUnread(),
+ "a thread marked read left its loaded replies carrying unread, so "
+ "the rows describe a state the database does not hold");
+
+ // The reply's row was told to repaint, not merely mutated behind the view.
+ bool replyRepainted = false;
+ for (const QList<QVariant> &call : spy) {
+ const QModelIndex from = call.at(0).toModelIndex();
+ const QModelIndex to = call.at(1).toModelIndex();
+ if (from.parent() == threadIndex && replyIndex.row() >= from.row()
+ && replyIndex.row() <= to.row()) {
+ replyRepainted = true;
+ break;
+ }
+ }
+ QVERIFY2(replyRepainted,
+ "no dataChanged covered the reply rows, so the view has no reason "
+ "to redraw them");
+
+ // Both directions, since a toggle is only fixed if it is visible each way.
+ model.applyTagChange(QStringLiteral("t1"), { QStringLiteral("unread") }, {});
+ QVERIFY(model.messageAt(replyIndex).isUnread());
+}
+
+void TestThreadListModel::messageScopeResolvesAThreadRowToTheMessageItDisplays()
+{
+ // Item 108. A thread root RENDERS one message since item 66, so acting on
+ // it acts on that message. The thread's other messages are reached through
+ // the explicit thread actions, which still resolve through scopeFor().
+ ThreadListModel model;
+ ThreadSummary t = makeThread(QStringLiteral("t1"),
+ QStringLiteral("A subject"));
+ t.totalCount = 7;
+ t.firstMessageId = QStringLiteral("m0@example.org");
+ model.appendBatch({ t });
+
+ const QModelIndex root = model.index(0, 0, QModelIndex());
+
+ // Unexpanded, which is the case that matters: the id comes from the query,
+ // so this needs no children loaded.
+ QCOMPARE(model.rowCount(root), 0);
+
+ const ActionScope scope = model.messageScopeFor({ root });
+ QCOMPARE(scope.messageIds, QStringList{ QStringLiteral("m0@example.org") });
+ QVERIFY2(scope.threadIds.isEmpty(),
+ "a thread row still resolved to its whole thread, so every action "
+ "on a root card would touch messages it does not display");
+ QCOMPARE(scope.messageCount, 1);
+ QVERIFY2(!scope.wholeThread,
+ "the status bar would claim '(whole thread)' for a one-message "
+ "action");
+
+ // The old resolver is unchanged and is what the thread actions use.
+ const ActionScope threadScope = model.scopeFor({ root });
+ QCOMPARE(threadScope.threadIds, QStringList{ QStringLiteral("t1") });
+ QCOMPARE(threadScope.messageCount, 7);
+ QVERIFY(threadScope.wholeThread);
+
+ // A reply row is unchanged in both: it always stood for one message.
+ model.setThreadMessages(QStringLiteral("t1"),
+ { makeNode(QStringLiteral("m0@example.org"), 0),
+ makeNode(QStringLiteral("m1@example.org"), 1) });
+ const QModelIndex reply = model.index(0, 0, root);
+ QCOMPARE(model.messageScopeFor({ reply }).messageIds,
+ QStringList{ QStringLiteral("m1@example.org") });
+
+ // A root and one of its own replies is two DISTINCT messages, not one
+ // deduplicated to the thread.
+ const ActionScope both = model.messageScopeFor({ root, reply });
+ QCOMPARE(both.messageIds,
+ (QStringList{ QStringLiteral("m0@example.org"),
+ QStringLiteral("m1@example.org") }));
+ QCOMPARE(both.messageCount, 2);
+}
+
+void TestThreadListModel::messageScopeSkipsAThreadRowItCannotNameAMessageFor()
+{
+ // firstMessageId is populated by the worker from the query. A summary that
+ // arrived without one names no message, and the tempting fallback is to
+ // act on the whole thread instead. That is exactly the silent escalation
+ // item 108 exists to remove: the user would ask to act on one message and
+ // hit the conversation.
+ ThreadListModel model;
+ ThreadSummary t = makeThread(QStringLiteral("t1"),
+ QStringLiteral("A subject"));
+ t.totalCount = 4;
+ t.firstMessageId.clear();
+ model.appendBatch({ t });
+
+ const QModelIndex root = model.index(0, 0, QModelIndex());
+ const ActionScope scope = model.messageScopeFor({ root });
+
+ QVERIFY2(scope.isEmpty(),
+ "a thread row with no message id was escalated to its whole "
+ "thread rather than skipped");
+ QCOMPARE(scope.messageCount, 0);
+}
+
+void TestThreadListModel::aMessageTagChangeReachesTheRootCardsOwnMessage()
+{
+ // The user, 2026-08-16: "delete single message on the root message of a
+ // thread doesn't trigger the repaint, delete whole thread does".
+ //
+ // applyMessageTagChange only searched `children`, and the root message is
+ // never there: setThreadMessages drops depth 0 because the root row stands
+ // for it. So a write to the message a root card displays found nothing,
+ // updated nothing and repainted nothing, while the same write on a reply
+ // worked. Item 108 made this the ORDINARY case, so the every-day gesture
+ // was the broken one.
+ ThreadListModel model;
+ ThreadSummary t = makeThread(QStringLiteral("t1"), QStringLiteral("one"));
+ t.totalCount = 1; // A single-message thread.
+ t.firstMessageId = QStringLiteral("m0@example.org");
+ t.tags = QStringList{ QStringLiteral("inbox") };
+ model.appendBatch({ t });
+
+ const QModelIndex threadIndex = model.index(0, 0, QModelIndex());
+ QSignalSpy spy(&model, &QAbstractItemModel::dataChanged);
+
+ model.applyMessageTagChange(QStringLiteral("m0@example.org"),
+ { QStringLiteral("deleted") }, {});
+
+ // The card has to repaint, which is the whole report.
+ QCOMPARE(spy.count(), 1);
+ QCOMPARE(spy.at(0).at(0).toModelIndex(), threadIndex);
+
+ // And it has to LOOK deleted. For a one-message thread the thread's tags
+ // ARE that message's tags: notmuch_thread_get_tags is a union over the
+ // thread, and a union over one message is that message.
+ QVERIFY2(model.threadAt(0).isDeleted(),
+ "the root card does not show the state of the message it "
+ "displays, so Delete on it looks like it did nothing");
+
+ // Works before the thread has ever been expanded, which is the case the
+ // user hits: nothing loads a root's node until then.
+ QCOMPARE(model.rowCount(threadIndex), 0);
+}
+
+void TestThreadListModel::aMessageTagChangeOnOneOfManyLeavesTheThreadSummaryAlone()
+{
+ // The other half, and the reason the fix is not "write it to the summary".
+ // A thread's tags are a UNION over its messages, so deleting one message of
+ // seven does not make the conversation deleted, and painting the card
+ // crimson would claim it did.
+ ThreadListModel model;
+ ThreadSummary t = makeThread(QStringLiteral("t1"), QStringLiteral("one"));
+ t.totalCount = 7;
+ t.firstMessageId = QStringLiteral("m0@example.org");
+ t.tags = QStringList{ QStringLiteral("inbox"), QStringLiteral("unread") };
+ model.appendBatch({ t });
+
+ const QModelIndex threadIndex = model.index(0, 0, QModelIndex());
+ QSignalSpy spy(&model, &QAbstractItemModel::dataChanged);
+
+ model.applyMessageTagChange(QStringLiteral("m0@example.org"),
+ { QStringLiteral("deleted") },
+ { QStringLiteral("unread") });
+
+ // Still repaints: MessageIdRole and anything else keyed on the root's own
+ // node has changed, and the row is what the user is looking at.
+ QCOMPARE(spy.count(), 1);
+
+ QVERIFY2(!model.threadAt(0).isDeleted(),
+ "deleting one message of a seven-message thread painted the whole "
+ "conversation as deleted");
+ QVERIFY2(model.threadAt(0).isUnread(),
+ "marking one message of a seven-message thread read claimed the "
+ "whole conversation was read, though six messages still are not");
+}
+
+void TestThreadListModel::aCardListsItsOwnTagsBeforeItsSiblings()
+{
+ // The user, 2026-08-16, looking at a real four-message thread: the card
+ // showed `mailing-list/SBo` and `signed`, and `signed` vanished the moment
+ // the row was selected, because it belongs to a SIBLING and item 110 made
+ // the card stop claiming it.
+ //
+ // Their answer, which is better than either extreme: show both, and let
+ // size say whose is whose. Own tags first at full size, the thread's other
+ // tags after, smaller and muted. Nothing disappears; a chip only shrinks
+ // once the split becomes known.
+ ThreadListModel model;
+ ThreadSummary t = makeThread(QStringLiteral("t1"), QStringLiteral("one"));
+ t.totalCount = 4;
+ t.firstMessageId = QStringLiteral("m0@example.org");
+ // The UNION, as notmuch reports it: `signed` is a sibling's.
+ t.tags = QStringList{ QStringLiteral("inbox"),
+ QStringLiteral("mailing-list/SBo"),
+ QStringLiteral("signed"),
+ QStringLiteral("unread") };
+ model.appendBatch({ t });
+
+ const QModelIndex threadIndex = model.index(0, 0, QModelIndex());
+
+ // Before the row is opened there is no per-message answer, so every chip
+ // is in the own tier. This is what stops anything from appearing to vanish
+ // later: the split narrows the tier, it does not remove a chip.
+ const QStringList before =
+ model.data(threadIndex, ThreadListModel::PillTagsRole).toStringList();
+ QVERIFY(before.contains(QStringLiteral("mailing-list/SBo")));
+ QVERIFY(before.contains(QStringLiteral("signed")));
+ QCOMPARE(model.data(threadIndex, ThreadListModel::PillOwnCountRole).toInt(),
+ before.size());
+
+ // The message loads, carrying what it really has.
+ model.setRootMessageTags(QStringLiteral("m0@example.org"),
+ { QStringLiteral("inbox"),
+ QStringLiteral("mailing-list/SBo"),
+ QStringLiteral("unread") });
+
+ const QStringList after =
+ model.data(threadIndex, ThreadListModel::PillTagsRole).toStringList();
+
+ // Same chips, still all present. The user explicitly did not want the
+ // sibling's tag dropped.
+ QVERIFY2(after.contains(QStringLiteral("signed")),
+ "the sibling's tag was dropped from the card rather than being "
+ "shown smaller, which is what looked like a bug");
+ QVERIFY2(after.contains(QStringLiteral("mailing-list/SBo")),
+ "the card lost a tag the message really carries");
+
+ // Own first, siblings after, and the count is where the delegate switches
+ // fonts.
+ const int own =
+ model.data(threadIndex, ThreadListModel::PillOwnCountRole).toInt();
+ QVERIFY2(own > 0 && own < after.size(),
+ "the split did not happen: every chip is in one tier");
+ QCOMPARE(after.mid(0, own),
+ QStringList{ QStringLiteral("mailing-list/SBo") });
+ QCOMPARE(after.mid(own), QStringList{ QStringLiteral("signed") });
+
+ // Colours stay aligned with the tags, since the delegate walks them in
+ // step and a shift would colour a chip with its neighbour's colour.
+ QCOMPARE(model.data(threadIndex, ThreadListModel::PillColoursRole)
+ .toList()
+ .size(),
+ after.size());
+}
+
+void TestThreadListModel::theSplitIsKnownBeforeTheRowIsEverOpened()
+{
+ // The user, 2026-08-16: "not selecting the thread shows the chips at 'main'
+ // size, not smaller, not dimmed. After selecting the thread the unioned
+ // chips repaint to the correct size/color."
+ //
+ // The first version derived the split from the message LOAD, so an unopened
+ // row had no per-message answer and put every chip in the own tier. That is
+ // honest and useless: the list is mostly unopened rows, so the feature was
+ // invisible exactly where it was meant to be read, and selecting a row
+ // still changed the card.
+ //
+ // The query knows. The worker already walks to the card's message to get
+ // its id, so it reads that message's tags in the same pass and the split
+ // arrives with the row.
+ ThreadListModel model;
+ ThreadSummary t = makeThread(QStringLiteral("t1"), QStringLiteral("one"));
+ t.totalCount = 4;
+ t.firstMessageId = QStringLiteral("m0@example.org");
+ t.tags = QStringList{ QStringLiteral("inbox"),
+ QStringLiteral("mailing-list/SBo"),
+ QStringLiteral("signed"),
+ QStringLiteral("unread") };
+ // What the worker now supplies: the CARD's message, not the thread.
+ t.firstMessageTags = QStringList{ QStringLiteral("inbox"),
+ QStringLiteral("mailing-list/SBo"),
+ QStringLiteral("unread") };
+ model.appendBatch({ t });
+
+ const QModelIndex threadIndex = model.index(0, 0, QModelIndex());
+
+ // Never opened, never expanded.
+ QCOMPARE(model.rowCount(threadIndex), 0);
+
+ const QStringList pills =
+ model.data(threadIndex, ThreadListModel::PillTagsRole).toStringList();
+ const int own =
+ model.data(threadIndex, ThreadListModel::PillOwnCountRole).toInt();
+
+ QVERIFY2(own < pills.size(),
+ "an unopened row still puts every chip in the own tier, so the "
+ "card renders them all at full size and only corrects itself "
+ "when the row is selected");
+ QCOMPARE(pills.mid(0, own), QStringList{ QStringLiteral("mailing-list/SBo") });
+ QCOMPARE(pills.mid(own), QStringList{ QStringLiteral("signed") });
+
+ // And a message-scoped write still lands, without a load having happened.
+ model.applyMessageTagChange(QStringLiteral("m0@example.org"),
+ { QStringLiteral("deleted") }, {});
+ QVERIFY(model.messageById(QStringLiteral("m0@example.org")).isDeleted());
+ QVERIFY2(!model.threadAt(0).isDeleted(),
+ "the thread summary was rewritten for a one-message edit on a "
+ "four-message thread");
+}
+
+void TestThreadListModel::reconcileRefreshesASurvivorsOwnMessageTags()
+{
+ // reconcile() keeps a surviving row's NODE, deliberately: its children and
+ // its loaded flag are the expansion state the method exists to preserve.
+ // That means the per-message tags have to be refreshed explicitly, and the
+ // change detector has to notice when only they moved.
+ //
+ // The case: a sync where the root message alone changed, which is exactly
+ // what an external `notmuch tag` or another client does. The thread's union
+ // can be identical while the card's own message is not.
+ ThreadListModel model;
+ ThreadSummary before = makeThread(QStringLiteral("t1"),
+ QStringLiteral("one"));
+ before.totalCount = 2;
+ before.firstMessageId = QStringLiteral("m0@example.org");
+ before.tags = QStringList{ QStringLiteral("inbox"),
+ QStringLiteral("unread") };
+ before.firstMessageTags = QStringList{ QStringLiteral("inbox"),
+ QStringLiteral("unread") };
+ model.appendBatch({ before });
+
+ QVERIFY(model.messageById(QStringLiteral("m0@example.org")).isUnread());
+
+ // The root was read elsewhere. The THREAD is still unread, because its
+ // reply is, so the union does not move at all.
+ ThreadSummary after = before;
+ after.firstMessageTags = QStringList{ QStringLiteral("inbox") };
+
+ QSignalSpy spy(&model, &QAbstractItemModel::dataChanged);
+ model.reconcile({ after });
+
+ QVERIFY2(!model.messageById(QStringLiteral("m0@example.org")).isUnread(),
+ "a sync that changed only the card's own message left the row "
+ "showing the old per-message tags");
+ QVERIFY2(spy.count() >= 1,
+ "the change was applied without telling the view, so the card "
+ "keeps its old pixels until something else repaints it");
+
+ // The expansion state is still what reconcile() exists to preserve.
+ QCOMPARE(model.threadAt(0).threadId, QStringLiteral("t1"));
+}
+
void TestThreadListModel::updatesTagsForMessage()
{
ThreadListModel model;