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>

1 files changed, 188 insertions, 7 deletions

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