summaryrefslogtreecommitdiffstats
path: root/docs
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-29 09:49:29 +0200
committerDanilo M. <danix@danix.xyz>2026-08-29 09:49:29 +0200
commitf051dd657757bca3ac7e36454d7f2fc21f322f73 (patch)
tree5804923ed3eb747ee2cd84284e1b36fe0817ab8e /docs
parentd128919ad80ecc37b1f58d2ba83a7487362c126f (diff)
downloadqtmaildir-f051dd657757bca3ac7e36454d7f2fc21f322f73.tar.gz
qtmaildir-f051dd657757bca3ac7e36454d7f2fc21f322f73.zip
fix: judge a conversation's trash state on all of its messages
Item 178. everySelectedRowIsInATrashFolder() read ThreadSummary::firstMessagePath for any row that was not a message row. That was correct while a thread row MEANT that message (item 108) and stopped being correct when item 177 made it mean the conversation. A conversation is in the trash only when ALL of its messages are, so a partly trashed thread answered on whichever message the query returned first: Delete could be hidden on a conversation that still had mail outside the trash, and Restore offered on one that mostly did not. Not data-affecting. Both actions are no-ops in the wrong direction: Delete on already-trashed mail takes moveMessages()' already-there branch, and Restore on mail that was never trashed finds nothing to move. qtmaildir cannot produce such a thread itself, since Delete is absent on a reply row and Restore is thread-scoped. Two things outside it can: another client trashing a single message, and a reply arriving after the conversation was trashed. ThreadDigest already walks every message of the selected conversation for its sender counts, and a filename is served from the index like everything else in it, so the paths ride along on a request the selection already makes rather than costing a walk on every query. ThreadDigest::messagePaths is relative to the mail root, for the reason firstMessagePath records: an absolute path matches no account and silently resolves every row to none. MainWindow keeps them beside the dashboard's thread id and clears them when the dashboard is left, so a late digest cannot answer about another row. One limit, stated in the code rather than hidden. The digest is requested only for a single selected conversation row, so that is the only case with a real answer; any other selection falls back to the summary's one path. That fallback IS the pre-177 answer and is wrong in exactly the same partial case, which is the point: a multi-row selection is left no worse than it was, rather than given a second, differently wrong rule of its own. Making it exhaustive costs a per-query walk over every message, which is what this avoids. Two tests, both mutation-checked. The worker test puts its two messages in different folders, since two in one folder answer identically whichever way the code resolves them. The window test asserts both directions, so a fix that simply hid Delete everywhere would fail it, and sets totalCount explicitly: a summary left at the default is a message row, and the test would otherwise exercise the other branch and pass for the wrong reason. Suite: 42 of 43, with undoMovesTheMessageBack failing as it does on master (item 136).
Diffstat (limited to 'docs')
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md66
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md2
2 files changed, 67 insertions, 1 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 f062106..55c5ee7 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
@@ -9249,3 +9249,69 @@ mail was repaired by hand the same day. The regression test builds a thread
whose messages DISAGREE about the tag, since two messages in the same state
answer identically whichever way the code resolves them, which is the trap item
87 already records.
+
+## 178. Delete and Restore judge a conversation on one message
+
+**Done 2026-08-29**, unreleased, on `thread-row-identity`. Split out of item
+168 by item 177 and flagged in that spec's "Open, deliberately".
+
+**Observed.** `MainWindow::everySelectedRowIsInATrashFolder()` read
+`ThreadSummary::firstMessagePath` for any row that was not a message row. That
+was correct while a thread row MEANT that message (item 108) and stopped being
+correct when item 177 made it mean the conversation. A conversation is in the
+trash when ALL of its messages are, so a partly trashed thread answered on
+whichever message the query returned first: Delete could be hidden on a
+conversation that still had mail outside the trash, and Restore offered on one
+that mostly did not.
+
+**Not data-affecting.** Both actions are no-ops in the wrong direction: Delete
+on already-trashed mail takes `moveMessages()`' already-there branch, and
+Restore on mail that was never trashed finds nothing to move.
+
+**qtmaildir cannot produce such a thread.** Delete and Archive are ABSENT on a
+reply row (item 177, at the user's own decision) and Restore is thread-scoped,
+so every path through this application is all-or-nothing. Two things outside it
+produce one: another client trashing a single message, which is live rather
+than hypothetical since the user runs Thunderbird (item 104), and a reply
+arriving after the conversation was trashed, which needs no other client at
+all.
+
+**Cause and fix.** The summary carries one path because the query walk stops at
+one message, and it stops there deliberately: the Sent branch's own measurement
+records that walk as free only because it breaks at the first match. Collecting
+every path on every query would make 36,000 rows pay for a question about the
+one the user clicked.
+
+`ThreadDigest` already walks every message of the selected conversation, for
+the sender counts, and a filename is served from the INDEX like everything else
+in it. So the paths ride along on a request the selection already makes:
+`ThreadDigest::messagePaths`, relative to the mail root for the reason
+`firstMessagePath` records, filled in `loadThreadDigest()` and consumed by
+`onThreadDigestLoaded()`. The predicate tests every path for a conversation row
+and one path for a message row.
+
+**One limit, stated rather than hidden.** The digest is requested only for a
+SINGLE selected conversation row, so that is the only case with a real answer.
+Any other selection falls back to the summary's one path. That fallback IS the
+pre-177 answer and is wrong in exactly the same partial case, which is the
+point: a multi-row selection is left no worse than it was, rather than being
+given a second, differently wrong rule of its own. Making it exhaustive costs a
+per-query walk over every message, which is what this fix exists to avoid.
+
+**Tests.** Two, both mutation-checked.
+
+- `aDigestCarriesEveryMessagePath` puts the two messages in DIFFERENT folders,
+ since two in one folder answer identically whichever way the code resolves
+ them (item 87's rule). It also asserts the paths are relative, because an
+ absolute one matches no account and silently resolves every row to none.
+- `aPartlyTrashedConversationIsNotJudgedOnOneMessage` asserts both directions:
+ Delete survives and Restore hides on a partly trashed conversation, and the
+ wholly trashed case still answers as it always did, which is what says the
+ fix narrowed nothing. Reverting the predicate fails it with the reported
+ symptom.
+
+The second test sets `totalCount` explicitly and asserts `isConversationRow()`
+before proceeding. A summary left at the default is a MESSAGE row, so a test
+meaning to exercise a conversation would quietly exercise the other branch and
+pass for the wrong reason, which is the fixture trap the top-level document
+records.
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 bf347c8..1f5a45d 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
@@ -249,7 +249,7 @@ taking that too literally.
| 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 | **done 2026-08-28**, unreleased, on `thread-row-identity`. `NotmuchWorker::applyTags()` reads each message's tags before writing and reports only the ids whose tags actually MOVED; a `TagCommand` base carries that effective set for both `ThreadTagCommand` and `MessageTagCommand`, which had the same defect on a multi-row selection. `tagsApplied` does NOT fire on an empty effective list, since an empty change would push an undo entry whose inverse adds a tag no message ever carried, the same bug one step later. `sendThreadTagChange` gained `onlyMessageIds` so it keeps its thread-scoped REPAINT while restricting the WRITE: the card that changed on screen and the messages that changed on disk are different sets on purpose. **The spec's own plan said item 177 would make a thread undo honest and shrink this to the multi-row case; that was wrong and is corrected in the spec**, an undo inverts an EFFECT, not a scope |
| 177 | A thread row means both a message and a conversation, and neither consistently | design | L | **done 2026-08-28**, unreleased, on `thread-row-identity`, eleven commits. Spec: `specs/2026-08-28-thread-row-identity-design.md`. `ThreadListModel::isConversationRow()` is the single predicate and `scopeForSelection()` the single resolver, replacing the `scopeFor()`/`messageScopeFor()` pair that made the CALLER choose. A summary with `totalCount == 1` is unchanged. **Reverses items 108, 110 and 111**, and the user confirmed they are happy to lose the two-tier chips; the `*_thread` submenu and its five action names are deleted with an `### Upgrading` note. Item 112's hiding rule is reversed too: with the absolute entries gone, hiding the toggle on a mixed selection leaves no way to act, so it is a catch-all and the write direction moves with the label. Membership is the union, with two user decisions kept (never evict the current row; an asked-for write evicts at once, an automatic one defers) and one documented lag (a long thread's summary is not updated by a message write, so reading its last unread message waits for the next query). Dashboard from a `ThreadDigest` read by its own worker walk. Two traps found while building: a `QStackedWidget` takes the LARGEST minimum width of its pages and the hidden dashboard was raising the pane's minimum to 395px over MainWindow's 300px floor, caught by an existing resize test; and the pane now holds two `TagStrip`s, so both are named |
-| 178 | Delete and Restore judge a conversation on one message | defect | XS | open, 2026-08-28, split out of item 168 by item 177 and flagged in that spec's "Open, deliberately". `MainWindow::everySelectedRowIsInATrashFolder()` reads `ThreadSummary::firstMessagePath` for any row that is not a message row, which was correct while a thread row MEANT that message and is not correct now that it means the conversation. A conversation is in the trash when ALL of its messages are, so a partly-trashed thread currently answers on whichever message the query returned first: Delete can be hidden on a conversation that still has mail outside the trash, and Restore offered on one that mostly does not. Not data-affecting, both actions are no-ops in the wrong direction, but it is an inconsistency the row-kind rule was supposed to remove. Needs the summary to carry the answer, or the paths of every message, which the digest walk already reads |
+| 178 | Delete and Restore judge a conversation on one message | defect | XS | **done 2026-08-29**, unreleased, on `thread-row-identity`. `ThreadDigest` carries every message's path, collected by the walk it already makes, so the predicate tests the whole conversation. Known for the SINGLE selected conversation row the digest was requested for; any other selection falls back to the summary's one path, which is the pre-177 answer, deliberately left no worse rather than given a second differently-wrong rule. Section in the closed file |
| 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 |
| 179 | Undo is one level deep in practice, and there is no Redo | workflow | ? | open, 2026-08-29, from the notes. The `QUndoStack` is real and multi-level; what is missing is a `redo` action (absent from `knownActions()`, never called) and an answer to the stack being CLEARED on every new query (`mainwindow.cpp:3458`), which is what makes a deep stack behave like a shallow one. The clear has a correct reason and cannot simply be removed. Redo re-applies a write to real mail, so item 176's rule binds it too |