diff options
Diffstat (limited to 'docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 694 |
1 files changed, 694 insertions, 0 deletions
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md index 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.** |