From 44d62143a83af8acbd1c1d14653d39da37e5de4a Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Tue, 11 Aug 2026 12:41:14 +0200 Subject: feat(sent): add a Sent view, flat and by recipient Adds a `sent` key to [account.*] naming that account's sent folder, and a Sent button beside the saved queries that composes its query from every account carrying one. An account without the key is omitted silently, as a real account may keep no sent mail locally. With no account selected the button spans all of them; selecting one narrows it through the existing scope wrap rather than a second path. Composed at run time rather than shipped as a [queries] entry. A saved query is one fixed string: it cannot narrow to the selected account, and it goes stale the moment an account is added or a provider renames a folder. The design and the measurements behind it are in docs/superpowers/specs/2026-08-11-sent-mail-design.md. Three things there are worth repeating here. The composed path is QUOTED, and that is load-bearing. A real provider nests its sent folder under a bracketed parent, and "[" and "]" are Xapian syntax: unquoted, the query parses rather than matches and returns nothing while looking entirely plausible. Composition happens in one place so there is one chance to get it right, and a bracketed path is pinned in a test. Recipients are opt-in per query, which is a performance contract rather than a preference. notmuch_message_get_header(m, "To") is not served from the index, it reads the message file: folding every thread of a 4411-thread inbox took 38.2 seconds against 251 ms for the 601-thread sent view. The worker skips the walk entirely unless asked, and the refresh path carries the same flag so a background sync cannot blank the column mid-read. Always folding is mutation-tested: the data would be right and only the cost wrong, which nothing else here would notice. The messages reached through the thread are owned by it and freed with it, so recipientsOf() holds them raw and finishes while the thread is alive, exactly as walkReplies does. An NmMessage wrapper there is a double-free. Sent mail is presented flat, and the pane follows. A message you sent otherwise drags in the replies you received, so a view labelled Sent shows conversations rather than what you sent. ThreadListModel::setFlatMode() makes hasChildren() and ReplyCountRole answer differently and changes nothing else; runQuery() sets it on EVERY run, so any other query restores the tree on its way through and the flag cannot outlive the button that set it. The pane needed its own fix for the same reason: the single-message path depends on a field only filled when a thread is expanded, which never happens in a flat list, so loadThread() gained matchedOnly and drops the messages that did not match instead of rendering them as stubs. Recipients replace the sender through the existing SendersRole rather than a new one, so the delegate needs no branch and cannot disagree with the model about which name a row shows. It falls back to the sender when a To header is absent or unparseable, since a blank where a name belongs reads as a rendering fault. Address parsing uses GMime: a display name may contain a comma, so "Rossi, Mario" , info@example.net is two addresses and splitting reports three. internet_address_list_parse returns NULL for an empty string, which is a crash if unguarded. Backlog item 63. --- .../plans/2026-08-03-post-0.1.0-usability.md | 2 +- .../specs/2026-08-11-sent-mail-design.md | 62 +++++++++++++++++++++- 2 files changed, 62 insertions(+), 2 deletions(-) (limited to 'docs/superpowers') 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 534dc6e..a187874 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 @@ -119,7 +119,7 @@ taking that too literally. | 60 | Next thread dead-ends on the last reply of an expanded thread | defect | XS | **done**; already fixed by 5487d58, see below | | 61 | `test_mainwindow` fails intermittently, about 1 run in 20 | testing | S | open; predates the card list, reproduced on f72dba9 | | 62 | No config option for the date format on a card | presentation | XS | **done** 2026-08-11 | -| 63 | No way to see sent mail, and no filter for it | workflow | M | open; specified 2026-08-11 in `specs/2026-08-11-sent-mail-design.md` | +| 63 | No way to see sent mail, and no filter for it | workflow | M | **done** 2026-08-11; see `specs/2026-08-11-sent-mail-design.md` | | 64 | The Sync button carries a mailbox icon, not a refresh one | presentation | XS | **done** 2026-08-11 | | 65 | No full code review and optimization pass | correctness | ? | open, unspecified | diff --git a/docs/superpowers/specs/2026-08-11-sent-mail-design.md b/docs/superpowers/specs/2026-08-11-sent-mail-design.md index 53fbb6c..4934241 100644 --- a/docs/superpowers/specs/2026-08-11-sent-mail-design.md +++ b/docs/superpowers/specs/2026-08-11-sent-mail-design.md @@ -1,6 +1,7 @@ # Sent mail: a per-account folder, a composed button, and recipients on the card -**Status:** specified 2026-08-11, not implemented. +**Status:** specified and built 2026-08-11, hand-verified by the user. See +"Outcome" at the end for what the spec did not anticipate. **Resolves:** backlog item 63. **Size:** M, revised up from the backlog's S. The query half is the S that was scoped correctly; the recipients half is its own piece of work. @@ -114,6 +115,18 @@ Built beside the saved-query buttons, running the OR of every non-empty `ThreadSummary` gains a recipients summary, filled in the worker, shown by `CardDelegate` in the sender's place when the row belongs to a Sent view. +**The fold must be OPT-IN per query, and this is not a preference.** Measured +2026-08-11 against the real database: `notmuch_message_get_header(m, "To")` is +NOT served from the index, it reads the message file. Folding it for every +thread of a 4411-thread inbox took **38.2 seconds**, 8.7 ms per thread. The +same fold over the 601-thread Sent view took **663 ms**, 1.1 ms per thread, +which the existing 200-thread batching hides. + +So the worker takes a flag on the query, set only when the query is a Sent one, +and skips the walk entirely otherwise. A version that always folds turns an +instant inbox into a 38-second one, and it would look correct in every test: +the data is right, only the cost is wrong. + ## Constraints **Do not invent a tag qtmaildir applies itself.** v1 is read-and-organize; @@ -178,3 +191,50 @@ named `signals`, which Qt defines as a macro. The first and last are the ones that fail loudest if the quoting is wrong, and they are the reason to write them before the UI work rather than after. + +## Outcome (done 2026-08-11) + +Built in the three pieces above and hand-verified by the user. Four things the +spec did not anticipate, each found by using it rather than by reading code. + +**A flat list, which the spec never mentioned.** The user's first report was +that the Sent view showed the replies they had RECEIVED. That is correct +behaviour, since the query matches messages and the list groups them into +threads, but it is not what a Sent view is for: their stated mental model is +that sent mail "lives on its own". `ThreadListModel::setFlatMode()` makes +`hasChildren()` and `ReplyCountRole` answer differently and nothing else +changes. It is one flag on the existing model rather than a second model or a +filtered query, which was the user's condition for building it at all. + +**Flat mode cannot leak, and that is structural rather than careful.** +`runQuery()` sets the mode on EVERY run, so any query that is not the Sent +button restores the tree on its way through. The window test mutates this +directly: making the flag one-way leaves it passing every model test and +flattens the whole application from the first Sent click. + +**The pane needed its own fix, and the flat list is why.** With the list flat +and correct, selecting a Sent row still opened the whole conversation. The +single-message path needs `ThreadNode::first`, which is only filled when a +thread is EXPANDED, so in a flat list it is always empty and every selection +falls through to `loadThread`. That now takes `matchedOnly`, dropping the +messages that did not match rather than rendering them as stubs. The per-message +`matched` flag it needs was already computed. + +**The recipient fold is cheaper than the spec's measurement.** 251 ms for the +601-thread Sent view against the 663 ms measured while specifying, because the +worker stops at the first usable `To` per thread rather than reading every +message. The inbox, with the flag off, is unchanged at 148 ms for 4411 threads. +The opt-in is mutation-tested: always folding fails with "the To header was read +for a query that never asked for it". + +**One mutation survived, and the comment was corrected rather than the code.** +Removing the `haveMatchSet` guard beside `matchedOnly` changes nothing, because +`ref.matched` is already true for every message when no query was given. The +guard is redundant today and kept as a stated invariant at the point that +depends on it; the test that appeared to cover it now says plainly that it does +not. + +**Known limit, accepted.** A flat row is still one row per THREAD, not per sent +message: 795 sent messages live in 601 threads here, so a thread written to +twice appears once, dated by its newest match. The model is thread-keyed +throughout, so per-message rows would be a different piece of work. -- cgit v1.2.3