aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-28 11:00:49 +0200
committerDanilo M. <danix@danix.xyz>2026-08-28 11:00:49 +0200
commit177d37eeb3126ec34f45bc21b9ef7d6a3e48b051 (patch)
tree412bb85b5aa8c93d8f13f1c01ba10211a86aee71
parent6cf4973b761e4be0291d51c8fde8970edae145c0 (diff)
downloadqtmaildir-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.md107
-rw-r--r--docs/superpowers/specs/2026-08-28-thread-row-identity-design.md268
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.