diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-28 11:00:49 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-28 11:00:49 +0200 |
| commit | 177d37eeb3126ec34f45bc21b9ef7d6a3e48b051 (patch) | |
| tree | 412bb85b5aa8c93d8f13f1c01ba10211a86aee71 | |
| parent | 6cf4973b761e4be0291d51c8fde8970edae145c0 (diff) | |
| download | qtmaildir-177d37eeb3126ec34f45bc21b9ef7d6a3e48b051.tar.gz qtmaildir-177d37eeb3126ec34f45bc21b9ef7d6a3e48b051.zip | |
docs: specify what a thread row stands for
Four separate questions in one session turned out to be one question: a row
means a message for display and action, and a thread for existence and
membership. The spec settles it as the conversation, and records item 177
plus item 176, the thread-scoped undo defect found while hand-testing.
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 107 | ||||
| -rw-r--r-- | docs/superpowers/specs/2026-08-28-thread-row-identity-design.md | 268 |
2 files changed, 374 insertions, 1 deletions
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 94d36d9..7e7c040 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 @@ -243,10 +243,14 @@ taking that too literally. | 167 | No way to tell one build of an unreleased version from another | enhancement | XS | **done 2026-08-25**, unreleased. The user chose a counter over a git description: `QTMAILDIR_BUILD_NUMBER`, a cmake option ON by default, increments a counter in the BUILD directory on every build and writes `buildnumber.h`. `QTMAILDIR_VERSION_DISPLAY` carries it; `QTMAILDIR_VERSION` stays clean and is what the window title, `applicationVersion` and the release procedure use | | 168 | Delete is offered on mail already in the trash, and does nothing | defect | S | **done 2026-08-25**, unreleased. Delete is hidden when every selected row is already in its account's trash, Restore when none is, both keyed on the PATH rather than the `deleted` tag. Delete also drops `unread` now, in the same TagChange so one undo returns the folder and the tag together | | 169 | A card shows the account only as a bar, with no fade and no avatar | presentation | M | **done** 2026-08-26, unreleased, on `card-avatars`, merged fast-forward. Both halves: a `QLinearGradient` from the account colour to the pane's base across the card, and a squircle avatar with initials, given a rect in `CardLayout` so the geometry is asserted without a painter. **Hand-testing found four defects**, all fixed in 9ae43f9: `Avatar::initialsFor()` normalises the display name first (drops the angle-addr, takes the first comma-separated author, unwraps quotes, treats a bare address as no name, requires a word to carry a letter or digit); the two-tone gradient axis spans the DIAMETER rather than a radius, which was letting one hue fill the whole face; the account fade runs right to left, anchored opaque at the card's right edge; and a flat view hashes `ThreadSummary::firstMessageRecipient` rather than the user's own address. The vCard half stays blocked on item 72 | -| 170 | A row that stops matching the view only leaves it on the Delete path | defect | S | open, 2026-08-26, from the notes, **cause found the same day and the premise is NOT stale**. The optimistic REPAINT is universal; the optimistic MEMBERSHIP is not. `removeThreadsWithoutTag()` has exactly one caller, on the move path, so marking a message read in the Unread view repaints the row and leaves it in a list it no longer belongs to | +| 170 | A row that stops matching the view only leaves it on the Delete path | defect | S | **done 2026-08-28**, unreleased. `MainWindow::syncViewMembership()` is the guard, moved off the move path and called from all three funnels (message, thread, move). It does the INVERSE too: a write that adds the view's tag back refreshes, since the model cannot insert a row for a thread the query never returned, and without it an undone mark-read stayed invisible in the view it was undone in. Found a second defect while testing: `ThreadListModel::applyTagChange()` never updated the ROOT message's own tags, which a thread row's card draws in preference to the summary since item 110, so an archived thread both kept drawing `inbox` and was judged to still match | | 171 | A forwarded HTML message reaches the recipient as plain text | defect | M | **done** 2026-08-27, unreleased. Design in `specs/2026-08-27-forward-html-design.md`. The forward sends ONE part chosen by the Send-as-HTML toggle, with the original shown in a read-only pane beside the editor, and remote content stripped by default with a per-forward opt-out. Hand-tested 2026-08-28. Four parts: `HtmlSanitiser` (an ALLOW-LIST, unlike `namespaceCids()`, because a missed strip is a beacon where a missed rewrite is a broken image), a text fallback for the ~9% of mail with no plain part, the MIME nesting, and the composer control | | 172 | A draft this application writes is tagged `unread` | defect | XS | **done** 2026-08-27, unreleased. `DraftStore::write()` was called with `"D"`, and `maildir.synchronize_flags` makes notmuch tag anything without `S` as `unread`. Self-healing on the next sync of that folder, which is what made it look intermittent | | 173 | The composer is a plain-text editor, not WYSIWYG | v2 | L | open, 2026-08-27, **asked for by the user** while hand-testing 171. This is a GUI mail client and should edit rich text the way one does: the forwarded original, and the user's own formatting, visible and editable in place. Supersedes the preview 171 shipped as a middle ground, and **subsumes item 133** (markdown syntax highlighting), which is the same want answered cheaply. See the entry: the draft format and the markdown-as-source-of-truth model both change | +| 176 | Undoing a thread-scoped action applies its inverse to messages it never changed | defect | S | open, 2026-08-28, found by hand while testing 170, **data-affecting and live**. `ThreadTagCommand::undo()` inverts the tags and keeps the THREAD as scope, so undoing `Mark thread read` adds `unread` to every message in the conversation rather than to the ones the write removed it from. Measured on the user's own mail: a 44-message thread had 2 unread, was marked read, and the undo left 43 unread. `maildir.synchronize_flags` is on, so it rewrote the files and would have reached the server. Repaired by hand the same day. `MessageTagCommand` has the same shape on a multi-message scope | +| 177 | A thread row means both a message and a conversation, and neither consistently | design | L | open, 2026-08-28, **specified**: `specs/2026-08-28-thread-row-identity-design.md`, read that rather than this row. A thread row becomes the CONVERSATION: it carries no first message, shows a dashboard rather than a message, and every action on it is thread-scoped. A summary with `totalCount == 1` stays a message row and does not change. Reverses items 108, 110 and 111 deliberately, deletes the `*_thread` submenu and its five action names (an `### Upgrading` note), and makes membership the union with no exceptions. **Item 170 is blocked on this** and item 176 gets easier under it | +| 174 | An external `notmuch new` reaches the index without the pending count noticing | defect | S | open, 2026-08-28, from the notes. Item 54 cleared the count for a sync run by `mailsync.sh`, which is what `SyncMonitor` watches; a bare `notmuch new` (a hand run, or a cron entry that is not the script) takes notmuch's own write lock and touches `/tmp/mbsync.lock` not at all, so nothing observes it. The user's framing is the approach: we own `mailsync.sh` and the whole process | +| 175 | The send countdown says Undo, and cannot be skipped | presentation | XS | open, 2026-08-28, from the notes. Two changes in one control: the button reads Abort, and a second button sends immediately rather than waiting the countdown out | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -1262,3 +1266,104 @@ rich-text composer with the original inside it, editable. `QSyntaxHighlighter` over the plain editor, which is the cheap answer to the same want ("show me what I am writing"). If this is built, 133 is moot; if this is deferred, 133 is the thing to do instead. Do not build both. + +## 174. An external `notmuch new` reaches the index without the pending count noticing + +**Observed (user, from the notes):** "the statusbar still reads that an +external notmuch run can clear the pending edits without the bar noticing. +Since we own mailsync and the whole process, we should fix that." + +**Cause (verified in the code, 2026-08-28).** Item 54 built exactly one +external path and it is narrower than the note assumes. `SyncMonitor` watches +the inode of `/tmp/mbsync.lock` in `/proc/locks` +(`syncmonitor.cpp:64`), which is the file `assets/mailsync.sh` flocks, and +`onExternalSyncStateChanged()` clears `m_pendingTagEdits` only when +`MailSync::lastRunOutcome()` reads an OK from that script's log +(`mainwindow.cpp:4812-4840`). + +A `notmuch new` that is not that script takes notmuch's own write lock inside +the Xapian directory and never touches `/tmp/mbsync.lock`. Nothing observes +it: no state change, no log line to read an outcome from, so the count keeps +reporting work that has already shipped and the exit prompt offers to sync for +it. This is item 54's symptom surviving through the one route item 54 did not +cover. + +**Approach (not decided).** The note names the lever: the process is ours, so +the honest fix is to stop inferring a sync from a lock file and observe the +INDEX instead. `notmuch_database_get_revision()` gives a monotonic uuid plus +revision that moves whenever anything is committed, whoever committed it; the +worker already holds a handle and reopens it. Watching that would cover the +script, a hand run and a cron entry alike, and would make the outcome question +moot: a revision that moved is evidence the write landed, where a log line is +a report about it. + +**Constraints.** +- Clearing the count on an observed revision bump is NOT sound on its own. A + revision moves for mail arriving as well as for our edits landing, so the + bump has to be read as "the index changed, re-check" rather than "our edits + are in". What settles it is the per-message check the pending map can + already do: every entry names a message and a tag. +- `SyncMonitor` stays whatever this becomes. It answers a different question, + "is a sync running", which drives the spinner and the write hold, and a + revision counter cannot answer it. +- Item 125 is open on the same monitor and should be read alongside: a + `mailsync.sh` that exits 75 leaves the spinner running for ever. + +## 175. The send countdown says Undo, and cannot be skipped + +**Observed (user, from the notes):** "the countdown popup has a 'undo' button +that would read better as 'Abort'", and "we could add a 'Send' that skips the +countdown and sends right away." + +**Cause.** Not a defect, a wording and a missing control. `SendDialog` runs +the undo window from item 123's design; the button is labelled for the undo +stack's vocabulary rather than for what it does here, which is to stop +something that has not happened yet. + +**Approach.** Rename the button, add a second one that fires the send +immediately. Both live in `SendDialog`; the timer already ends in the same +call the button would make, so skipping is stopping the timer and calling it. + +**Constraints.** +- The countdown IS the undo for a send, per CLAUDE.md's no-confirmation rule. + A skip button must not become a default, or the protection is gone for + everyone who learns to press it. +- Which button is the default on Return matters here and is the user's call. + +## 176. Undoing a thread-scoped action applies its inverse to messages it never changed + +**Observed (user, 2026-08-28):** found while hand-testing item 170. A +thread-scoped `Mark thread read`, then Ctrl+Z, left almost the whole +conversation unread rather than restoring the two messages that had been. + +**Cause (measured, not read).** `ThreadTagCommand::undo()` +(`src/mainwindow.h`) sends `sendThreadTagChange(m_threadIds, m_remove, m_add, +...)`: the tags are inverted and the SCOPE is not. `applyTags()` is a blind +add/remove over whatever ids the thread resolves to, so the inverse of +"remove `unread` from 44 messages" is "add `unread` to 44 messages", +regardless of which of them carried it. + +Measured on the live index: thread of 44 messages, 2 unread. Mark thread read +resolved 44 and removed the tag; the undo resolved 44 and added it, leaving +43 unread. The 42 that were read before the user touched anything were +rewritten. `maildir.synchronize_flags` is on, so the Maildir filenames were +rewritten too and the next sync would have carried it to the server. + +**Approach (not decided).** The command has to record what the write actually +CHANGED, not what it asked for. `applyTags()` is the only place that knows: +it holds each message open and can report the ids whose tags actually moved. +That is a worker change (`tagsApplied` carrying the effective set) plus a +command that stores it. + +**Constraints.** +- The undo stack is this application's substitute for confirmation dialogs + (CLAUDE.md), so an undo that damages state is worse than the dialog it + replaces. +- `MessageTagCommand` has the same shape. It is harmless on a single message, + where asked and changed agree, and has the same defect on a multi-row + selection. +- The five `*_thread` actions are the ones that resolve to a large id set, so + they carry almost all of the exposure. +- A test needs a thread whose messages DISAGREE about the tag. Two messages in + the same state answer identically whichever way the code resolves them, + which is the trap CLAUDE.md already records for item 87. diff --git a/docs/superpowers/specs/2026-08-28-thread-row-identity-design.md b/docs/superpowers/specs/2026-08-28-thread-row-identity-design.md new file mode 100644 index 0000000..ba60e00 --- /dev/null +++ b/docs/superpowers/specs/2026-08-28-thread-row-identity-design.md @@ -0,0 +1,268 @@ +# A thread row is the conversation + +**Date:** 2026-08-28 +**Status:** designed, not built +**Size:** L + +## The problem + +A row in the thread list means two different things at once, and every +ambiguity the application has about threads comes from that. + +Item 66 removed the conversation view, so a thread row's card displays ONE +message. Item 108 then made actions on that row scope to that one message, on +the sound reasoning that what you see is what you act on. Both changes were +right on their own terms. But the row still exists because a THREAD exists: it +is produced by a thread query, it carries a reply count, it expands into +children, and its subject, authors and date describe the conversation rather +than the message. + +So a row is a **message** for display and action, and a **thread** for +existence and membership. The user stated the consequence directly: + +> as it is today, I don't know how to address a thread, because on one side it +> is a message which happens to have replies, on the other is a discussion, and +> should be handled as such, as a whole. + +Four separate questions in one working session turned out to be this one +question wearing different clothes: + +- Does reading the displayed message remove the conversation from the Unread + view? (Answering "yes" made a 44-message thread with two unread replies + vanish two seconds after it was selected.) +- Does deleting the displayed message remove the conversation from the Inbox + view? (A shipped test asserts yes; consistency with the above demands no.) +- Whose tags does the card draw, the message's or the thread's? (Items 110 and + 111 answered "the message's first, the thread's second, in two tiers".) +- What does undoing a thread-scoped action cover? (Item 176: it inverts over + the whole thread while the write touched one message.) + +Answered one at a time, they produced four answers that contradict each other. + +## The decision + +**A thread row stands for the conversation.** It is not a message and does not +carry one. + +This reverses item 108 deliberately. That item fixed "the card shows one +message but acts on all of them" by moving the ACTION to match the card; this +fixes the same defect by moving the CARD to match the action. Same fork, other +branch. Item 108's own analysis is still correct about the state it found, and +its work is not wasted: the thread-scoped action bodies it wrote become the +ordinary path. + +## Two row kinds + +Decided by `ThreadSummary::totalCount`, which the query already carries. + +**`totalCount == 1` — a message row.** No expander. One click shows the +message. Every action scopes to it. This is the majority of mail and its +behaviour does not change at all. + +The user was explicit that this case must not regress: + +> A single email is a thread by concept, yes, but if it has no replies (yet) it +> should be viewed with a single click. The logic has to be able to see the +> difference. + +**`totalCount > 1` — a thread row.** It stands for the conversation. It shows +the subject, the participants, the reply count and the THREAD's tags. One click +shows the dashboard (below), never a message. Every action on it is +thread-scoped. Its messages live underneath it and are reached by expanding. + +## What a thread row stops carrying + +`ThreadSummary::firstMessageId`, `firstMessageTags` and `firstMessagePath` stop +feeding a thread row's identity. Consequences, each of which is a deletion: + +- `nodeFor()` no longer seeds `ThreadNode::first` from the summary, and + `setRootMessageTags()` goes with it. +- The `first.tags` substitution in the thread branch of `data()` goes. A thread + row's tags are `summary.tags`, notmuch's union, which is simply correct once + the row is the conversation. +- **Items 110 and 111 are reversed.** The two-tier chip display (own tags first, + siblings' muted behind them) exists only to reconcile "this card shows one + message" with "this row is a thread". With the row being the thread, one tier + is right. `PillOwnCountRole`, `CardLayout::siblingFont()` and + `CardDelegate::mutedChipColour()` are deleted. + +A message ROW keeps everything: `MessageOwnTagsRole` already computes "own" +as a set difference against the parent thread's union, which is exactly the +model this design wants, and needs no change. + +The fields stay on `ThreadSummary`. The dashboard and the Sent view still want +to know which message a thread query matched, and the query walk that fills +them is free (measured over 36,615 threads under item 66). + +## Actions and scope + +**Scope comes from what is selected, never from which menu entry was chosen.** + +| Selected | Scope | Label | +|---|---|---| +| Thread row | the conversation | `Mark thread read`, `Archive thread`, `Delete thread` | +| Message row (no replies) | that message | `Mark read`, `Archive`, `Delete` | +| Reply row | that message | `Mark read` | + +Labels name their scope, per the user: "Mark Read is fine for a single message, +but right clicking on the thread main row must read Mark thread Read or +something similar". There is precedent: items 99 and 112 already made the +unread action's label depend on the selection, so this is an established +pattern rather than a new mechanism. + +**The `Whole thread` submenu is deleted**, and with it the five `*_thread` +action names. Nothing is left to disambiguate once the row's identity decides +the scope. + +**Delete and Archive are conversation-level only.** On a reply row they are +ABSENT, not disabled. The user's reasoning: + +> I don't think I'd want to be able to remove a single reply from a thread, I +> can mark it with tags, important, spam, etc, I can reply/forward a single +> answer, yes, download that single message, yes, but delete should be +> available only as a conversation. + +This is Gmail's model rather than Thunderbird's or mutt's, and it is coherent +with the rest: irreversible actions work on the unit the user can see and +reason about, while reversible per-message things (tags, flags, read state, +reply, forward, save) work wherever they are. It also dissolves the "delete one +reply of five" membership question, which can no longer arise. + +`markAllRead` is untouched. It ignores the selection by design and is +thread-scoped by nature (item 108 recorded this). + +## Membership + +**The union, one rule, no exceptions.** A thread belongs to a view while ANY of +its messages match it. Reading one message of five does not remove the +conversation from Unread; reading the last one does. + +Two properties decided during the same session survive, and both are user +decisions rather than implementation details: + +- **A row is never evicted while it is current.** The automatic mark-read fires + two seconds after selection, so evicting on it takes the row out from under + the user, with a context menu possibly open on it, before they can mark it + spam or important. The row leaves when the user moves off it. The user's + framing: "if a selected row still is accessible for me to act on it, the view + has its meaning and utility." +- **A write the user asked for evicts at once; an automatic one does not.** The + distinction is who initiated it, not what it does. + +A row that starts matching cannot be inserted optimistically, since the model +holds no summary for a thread the query never returned; that case refreshes. + +## The dashboard + +Shown when a thread row is selected. The user asked for "a recap of the thread, +a well designed dashboard", and rejected the old conversation view, which +listed messages as stubs that could not be expanded and rendered only the last +two. + +Blocks, top to bottom: + +1. **Header** — subject, then `N people · N days`, and the account. +2. **Tag chips** — the thread's tags. One tier. +3. **Counts** — messages and unread, with a read-progress bar beneath. +4. **Waiting for you** — the unread messages, each as sender, relative time and + subject. When nothing is unread this block is replaced by a single + **All caught up** line and the rest of the pane is unchanged. +5. **Activity** — a sparkline over the thread's lifetime, with + `first → last · busiest <day>` beneath it. +6. **Thread actions** — Mark all read, Archive, Delete. + +**Not a web view.** The message pane's `QWebEngineView` exists to render mail +from strangers under a locked-down profile; a dashboard is our own widgets over +our own data, so it is a plain widget and none of that security surface applies +to it. `MessageView` gains a second stacked page and switches between them. +A single-message thread shows the message, never the dashboard. + +**Sparkline buckets are always 7**, the span divided into seven, with the real +range printed beneath. A fixed bucket count is what keeps the widget's geometry +testable; a thread spanning five days and one spanning two years cannot share a +bucket size, and the label carries the truth. + +**An entry under "Waiting for you" is clickable** and selects that message's +row in the list, expanding the thread if needed. Without it the dashboard is a +dead end at the moment it has just told the user what they have not read. + +## Data + +One worker entry point, `loadThreadDigest(threadId, generation)`, returning a +plain value struct over queued signals like everything else that crosses the +thread boundary: + +``` +struct ThreadDigest { + QString threadId; + QList<QPair<QString, int>> senders; // display name, message count + QVector<MessageRef> unread; // sender, date, subject, id + QVector<int> buckets; // always 7 + qint64 firstTimestamp, lastTimestamp; + int busiestBucket; +}; +``` + +Everything comes from the index; no message files are opened, so it costs about +what a thread load costs today. It needs its own generation counter, separate +from the query's: bumping `m_generation` for a digest would discard any thread +load in flight, which is the trap item 119's rule-count work already recorded. + +## Testing + +- **The two row kinds** are a model-level question and test without a painter: + a summary with `totalCount == 1` produces a row that reports no children and + resolves actions to its message; one with `totalCount > 1` reports children + and resolves to its thread. +- **Membership** needs a thread whose messages DISAGREE about the tag. Two + messages in the same state answer identically whichever way the code + resolves them, which is the trap CLAUDE.md records for item 87. Put the + disagreement under the SECOND thread, so a wrong answer is visible rather + than accidentally right. +- **The deferred eviction** must be driven through the mark-read TIMER, not by + calling the send path directly: the deferral keys on the write being + automatic, so a direct call tests the opposite branch. A bare `MainWindow` in + a test also runs its startup query against the live index, so a test that + waits long enough for that to land will see the model repopulate; fire the + timer with a zero interval and one event-loop turn rather than a `QTRY`. +- **The dashboard** is a widget over a value struct, so its content is + assertable without rendering: build a `ThreadDigest` and assert what the pane + shows. Do not count pixels, for every reason under "Rendering probes lie". +- **`ctest -R translations`** must stay clean: the new labels are user-facing + strings, and a label in an array needs `QT_TRANSLATE_NOOP` with the context + named on the literal. + +## Order of work + +1. **Model.** Row kind from `totalCount`; thread rows drop the first-message + identity; membership by union. The largest single piece, and where the tag + tier tests change. +2. **Actions.** Scope from the selection, dynamic labels, submenu deleted, + Delete and Archive absent on reply rows. +3. **Worker.** `loadThreadDigest`, the value struct, its own generation. +4. **Pane.** The dashboard widget, stacked with the message view. +5. **Item 176.** The undo fix, which this makes easier: a thread action's undo + covering the whole thread is now honestly what the user asked for, so the + remaining exposure is only the multi-row message case. + +This is one coherent change rather than a decomposition candidate. Splitting it +across releases would ship a state where rows mean one thing and actions +another, which is the defect being fixed. + +## Upgrading + +The five `*_thread` action names are removed. A `[keys]` section naming any of +them refers to an action that no longer exists, and `KeyMap` will report it as +unknown. The replacement is that the ordinary action name now does whatever the +selected row means, so a binding on `toggle_unread` covers both cases and the +thread-specific binding should simply be deleted. + +## Open, deliberately + +- **Per-sender colours in the dashboard** reuse the account-avatar palette from + item 169. Whether a participant's colour should be stable across threads is + not decided; nothing depends on it. +- **Item 168** (Delete hidden when every selected row is already in the trash) + needs restating in thread terms: a conversation is in the trash when all of + its messages are. Not part of the core change, and it is a defect if left + unstated. |
