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 | 303 |
1 files changed, 303 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 945aacd..84685c6 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 @@ -10375,3 +10375,306 @@ repository.** different thing in a different place. + + +## 187. There is no Spam view beside Trash + +**Observed.** The user asks for a Spam view next to Trash. Mail can be marked +spam today and there is no filter that lists it. + +**Cause.** `kQueryGenerators` (`config.cpp:62`) is a closed set of six: +`unread`, `inbox`, `flagged`, `sent`, `drafts`, `trash`. There is no `spam`. +The `spam` action has existed since the first toolbar and writes the tag +(`mainwindow.cpp:1770`, adds `spam`, removes `inbox`), so the write half is +built and the read half is missing. + +**Two wrong premises were corrected before any design, and both are worth +keeping.** This entry first said no account names a spam folder, so a tag +generator was the only option. Wrong: the accounts synced with `Patterns *` +had a spam folder all along. It then said the accounts with an explicit +`Patterns` list could never have one. Also wrong, and the cause was local +rather than remote: the provider exposes the folder over IMAP and mbsync was +simply never asked for it. Adding it to those three channels on 2026-08-29 +took one line each, verified against `mbsync --list` rather than guessed, +which matters because `Create Both` turns a wrong folder name into a folder +created on the server (item 103). + +**So every account can now reach a spam folder, and the design is Trash's.** +The user settled three things on 2026-08-29: + +- **Path-based, exactly like Trash.** Not a tag generator. A tag query finds + only what this application marked and misses everything the server filed, + which is most of what those folders hold. +- **Mark spam MOVES the file**, as Delete does. This is a change to an + existing action, not only a new view, and it is the part that makes the + path-based view honest. +- **`Junk` is out of scope.** One account has a `Junk` folder beside its + `Spam`; it is not used and the key names one folder. + +**Approach.** Follow item 103's implementation rather than inventing one. + +1. A mandatory per-account `spam` key beside `trash`, an `Account::spamQuery()` + beside `trashQuery()`, and `Config::allSpamQuery()` beside + `allTrashQuery()`. +2. `spam` added to `kQueryGenerators` and to `builtinFilter()`, threaded like + Trash rather than flat, composing with the account selector through the same + path in `resolvedQuery()`. +3. The `spam` action moves the file instead of only writing tags, through + `moveMessages()`, with an origin tag so it can come back. Restore already + reads `deleted-from:`; this needs the same for spam, or one shared origin + scheme. +4. A cleanup pass for mail tagged `spam` that never moved, which is every + message the action has ever touched. + +**The cleanup pass has a precedent and should copy it.** +`showStrandedDeletedMail()` (item 103) is the same problem one version earlier: +mail tagged `deleted` whose file never left its folder. It builds +`tag:deleted and not (<all trash folders>)`, puts it in the query bar, and +REPORTS, moving nothing, leaving the user to select and act. Do the same with +`tag:spam and not (<all spam folders>)`. Two details of it are load-bearing: +an empty folder list must never be written as `not ()`, which notmuch parses +happily and matches nothing, reporting a clean database; and it runs +`AlreadyScoped` so the account dropdown does not narrow it and hide other +accounts' stranded mail. + +**Constraints.** + +- **A mandatory key breaks every existing config on upgrade**, exactly as + `trash` did under item 103. That needs an `### Upgrading` note in the + changelog, and the same treatment `trash` got: name the missing key rather + than failing silently. +- **Naming a folder that does not exist reaches the server.** Item 103's + lesson, and the reason the three Gmail patterns were verified against + `mbsync --list` before being written. A default value is not safe here; the + key is named by the user or the account has no spam view. +- **`Config::matchNothingQuery()` for an account with no spam folder**, never + an empty string: notmuch reads an empty query as "match everything", so the + Spam button would show the whole Maildir. +- **The trash view's own predicate must not be confused by this.** + `everySelectedRowIsInATrashFolder()` decides which actions the message bar + and menus offer (items 185, 186). A spam folder is not a trash folder and + must not satisfy it, or Restore and the purges appear on spam. +- **Mark spam removing `inbox` stays.** The tag half is still what makes the + message leave the Inbox view; the move is in addition to it, not instead. +- **The label is translated, the generator is not.** `spam` is stored in + `queries.json` and matched against a closed set, so it is wire format; see + the `flagged`/"Important" note in `builtinFilter()`. +- **Adding a generator changes queries.json's readable set**, so an older build + reading a file that names `spam` reports an unknown generator and KEEPS the + row. Existing behaviour, no version bump. +## 190. Mark spam is not on the message bar, and its icon was never chosen for one + +**Observed (user, from the notes):** "add \"mark as spam\" to the message pane +toolbar. Use a bug as the icon (or a skull, or something that signifies +bad/evil)." + +**Cause, verified in the code.** Two independent halves, and neither is a +regression. + +The action exists and has since the first toolbar: `addAction("spam", tr("Mark +&spam"), ...)` at `mainwindow.cpp:1768` writes `spam` and removes `inbox` +through `tagSelected()`. It is reachable from the Message menu +(`mainwindow.cpp:2062`) and the thread context menu (`:2223`), and it carries a +shortcut, `Ctrl+Shift+S` (`keymap.cpp:151`). What it has never been on is the +message pane's own bar: `refreshBarActions()` fills the ordinary branch with +exactly `reply`, `forward`, `flag`, `archive`, `delete` (`mainwindow.cpp:2387`), +and item 189 added Star and Archive there without raising spam. + +It meets the bar's rule as it stands. The bar carries selection-scoped actions +with an undo behind them, which is why `mark_all_read` was kept off it under +item 189 and why Star and Archive were let on. `spam` is a `tagSelected()` call +like those two, so it qualifies on both counts today. + +The icon is the second half and is the same latent wrong choice item 189 found +in `flag`. `{ "spam", "mail-mark-junk" }` (`mainwindow.cpp:2144`) was chosen for +a MENU, where the label carries the meaning and the icon only decorates it. On +an icon-only bar the icon IS the control, which is what made Breeze's +exclamation-mark rendering of `mail-mark-important` a defect rather than a +preference. Whether `mail-mark-junk` reads as "bad/evil" on the user's theme is +a question only the user can answer by looking, and the note suggests it does +not. + +**Approach.** Add `spam` to the ordinary branch of `refreshBarActions()`. Order +is a decision, not a detail: the bar reads answer, then file, then destroy, and +spam is a filing act whose destination is hostile, so it belongs with Archive +rather than beside Delete or before Star. For the icon, offer the user the +theme names that exist rather than picking one unseen; a shipped SVG under +`assets/icons/marks/` is the fallback if no theme name reads right, but that is +the panes' convention and the bar is chrome (item 70), so it is a last resort +rather than a first move. + +**Constraints.** + +- **The trash branch must not gain it.** `everySelectedRowIsInATrashFolder()` + swaps the bar to Restore, Delete permanently and Empty trash (item 185); + marking already-trashed mail as spam is not an act the user asked for, and the + same question item 187 flags applies here from the other side. +- **The icon table forbids duplicates** for any action that can reach the + toolbar, by the test item 140 established. `mail-mark-junk` is unique today + and any replacement must stay so. +- **Item 187 changes what this action does**, from a tag write to a file move + with an origin tag. Doing 190 first puts a button on the bar whose behaviour + then changes underneath it; doing 187 first means the button arrives already + correct. Neither ordering is wrong and the user chooses, but they should not + be built in ignorance of each other. + +**Verification.** The bar's contents are a list in one function, so an assertion +on it is measurable and belongs in the same test item 189 corrected. The icon is +a visual judgement and belongs to the user, per the rule in `CLAUDE.md`: hand it +over and let them look. +## 195. Mark spam leaves the message unread + +**Observed (user, from the notes):** "marking a message as spam without reading +it doesn't remove the unread tag." + +**Cause.** Verified, not assumed. The `spam` action at `mainwindow.cpp:1786` +calls `tagSelected({ "spam" }, { "inbox" }, ...)`: it names exactly two tags, +so `unread` is untouched by construction. The message leaves the inbox and +keeps counting toward every unread view. + +**Approach.** Add `unread` to the removal list of that one call. It is a +two-word change and the surrounding machinery already covers it: the write goes +through `applyTags`, which reports only the ids whose tags actually moved (item +176), so a spam mark on an already-read message pushes no bogus undo, and +`syncViewMembership()` evicts it from Unread on the same funnel as any other +read. + +**Constraints.** Item 187 rewrites this action into a file move, so the cheapest +path is to fold this in there rather than shipping a separate commit that 187 +then rewrites. Doing it alone is still fine and costs nothing. + +**One question for the user.** Whether marking spam should mark READ, or whether +the right answer is that a spam message stops matching the unread views at all +once 187 makes those views path-based. The first is what the note literally +asks for; the second falls out of 187 for free and means an unread spam message +is still honestly unread if it is ever restored. They are not the same and the +choice is theirs. +## 197. No way to say a message is not spam + +**Observed.** Split out of the item 187 design on 2026-09-10, at the user's +decision, rather than built into it: "maybe we could already provision for a +future 'unmark spam' action so that we can revert a filter decision". + +**What already covers half of it.** Restore handles every message this +application moved. Mark spam writes `moved-from:<folder>` and Restore reads it +back, so unmarking is the existing gesture under a different name. + +**The real gap is the provider's filter, not ours.** Mail the provider caught +was never in an inbox, arrived directly in the spam folder, and carries no +origin tag. Restore falls back to the account's inbox for exactly this case, +which is a documented guess rather than a recorded destination. + +**Two questions decide the shape, and neither is answerable from the code.** + +1. Where does a message with no origin go? The account's inbox is the obvious + answer and is still a guess; a user who wants it filed somewhere else has no + way to say so. +2. Should anything tell the PROVIDER its filter was wrong, so it learns? That + is outbound network work, which this application does not do by design. It + would belong in a sidecar, like item 194's. + +**No seam is needed in the meantime.** `sendMove()` already takes any +destination and any tag lists, so a Not-spam action is a caller rather than a +capability. Provisioning for it now would be a hook with one hypothetical +caller, which is what YAGNI names. + +**Corrected 2026-09-14:** the interface half is item 201. Restore's visibility +is coupled to `everySelectedRowIsInATrashFolder()`, which by design never +answers for spam, so "Restore already covers what qtmaildir moved" was a +capability claim that no surface offered. Read item 201 for the narrower, +decision-free half. +## 201. A message in the Spam view cannot be un-spammed, even one qtmaildir put there + +**Observed (user, 2026-09-14, testing the `spam-view` branch).** "If a message +is in spam, how do I unmark it spam?" The only built-in answer found was Ctrl+Z +immediately after the move, before any other action; an edit of the `spam` tag +alone leaves the file in the spam folder. The user asked for this to be built on +`spam-view` before that branch merges. + +**Cause.** Verified in `src/`. Three facts together: + +1. `spam` is one-way, not a toggle. `MainWindow::spamSelected()` always resolves + the selection and calls `spamMessages()`/`spamThreads()`; unlike `delete` + (`mainwindow.cpp:1745`) and `flag` (`mainwindow.cpp:1832`) it never asks + `everySelectedRowHasTag("spam")`, so pressing Mark spam again moves the file + toward the folder it is already in rather than reverting it. +2. Restore is hidden outside the trash. `refreshTrashActions()` + (`mainwindow.cpp:3936`) sets `restore`'s visibility to + `(!haveSelection || inTrash) && !m_replySelectionHidesDelete`, where `inTrash` + is `everySelectedRowIsInATrashFolder()`, which compares `account.trash` only + BY DESIGN so the predicate never answers for spam (the spec requires that, so + Delete is not hidden and Purge not offered there). Restore is coupled to the + same predicate, which is what hid it in the Spam view. +3. The write side already works. `restoreSelectedFromTrash()` + (`mainwindow.cpp:6907`) resolves the origin from the DATABASE and calls + `sendMove()`, which takes any destination and any tag lists. Only the entry + point and the gating are missing. + +So item 197's "Restore already covers what qtmaildir moved" is true of the +function and false of the interface. Its stated gap was the PROVIDER-caught +message with no origin; this item is the narrower half, mail qtmaildir itself +moved and can put back, and it needs no new decision. + +**Approach.** One of two, and the difference is what the user should decide: + +- **Widen Restore into the Spam view**, decoupling Restore's visibility from + `everySelectedRowIsInATrashFolder()` so a message carrying a `moved-from:` + origin can be restored from spam too. Smallest change; makes one action serve + both folders. +- **Add a distinct `not_spam` action** shown on a spam-folder selection, doing + the existing restore and also stripping `spam`. Clearer on the bar and in the + menu, and costs the five registration places the rule names + (`KeyMap::knownActions()`, `defaultBindings()` optional, the icon table, the + action, a menu). + +Either reuses `sendMove()` and resolves the origin from the database, never the +model (the message-scoped `restoreSelected()` reads the model, the path item 176 +and the spam-view final review warn against). + +**Constraints.** A move's origin is resolved by the worker, never the model +(`CLAUDE.md`). An undo covers what the write CHANGED, not what it asked for +(item 176). No confirmation: this is a move and it is undoable. Item 197's open +questions, where PROVIDER-caught mail with no origin goes (the inbox guess) and +whether to tell the provider its filter was wrong, stay out of scope here. + +**Verification.** A `WorkerBackedWindow` test using `QTRY_VERIFY_WITH_TIMEOUT`: +mark a message spam, then un-spam it through the new path, asserting the file +returns to the folder it came from, `spam` and `moved-from:` are gone, and the +row leaves the Spam view. Because it is a move, assert the undo as well. +## 202. Mail in a spam folder keeps `inbox`, so it appears in the Inbox view + +**Observed (user, 2026-09-14, testing `spam-view`).** "Not spam is available in +the inbox view; it should appear only when viewing the spam view." The action is +not at fault: its predicate is folder-based (`everySelectedRowIsInAFolder()`, +the same rule Restore uses), so it correctly appears on mail whose FILE is in a +spam folder. Those rows should not have been in the Inbox view at all. + +**Cause.** Verified, and not in the application. notmuch's `new.tags` is +`new;unread;inbox`, so every newly indexed FILE gets `inbox` regardless of the +folder it sits in. The Inbox built-in filter is `tag:inbox` +(`Config::generatorTag("inbox")`), not path-scoped, so any file carrying `inbox` +appears there. The `post-new` hook already corrects this for mail that did not +ARRIVE: `NOT_ARRIVALS` in `assets/hooks/qtmaildirconf.py:84` is +`("sent", "drafts")`, and `strip_inbox_from_non_arrivals()` removes `inbox` from +a message whose files are ALL inside one of those folders. `spam` is missing +because the hook predates the per-account `spam` key this branch adds. Measured +live: 49 files tagged `inbox` sitting in a `[Gmail]/Spam` folder; 0 in trash, +drafts or sent. + +**Approach.** Add `spam` to `NOT_ARRIVALS` so the existing carve-out covers it +too, correct the function/doc prose (its names say "sent" while the list means +"not an arrival"), and extend the hook's tests with the spam cases. Then a +ONE-TIME cleanup of the existing messages, using the same all-files rule. + +**Constraints.** The all-files rule is load-bearing (`not_arrival_only()`): notmuch +deduplicates by Message-ID, so a message with one file in spam and one in an +inbox DID genuinely arrive and must keep `inbox`. Only `inbox` is removed; +`unread` is untouched because `maildir.synchronize_flags` is true. The cleanup +is a write to the live index and is confirmed with the user before running (it +was, 2026-09-14). Trash is deliberately out of scope: measured 0 such files, and +qtmaildir's own Delete strips `inbox` (item 168). + +**Verification.** `assets/hooks/test_post_new.py` (and the other hook suites) +green with a spam-folder case and a two-file (spam + inbox) case; after the +cleanup, `notmuch search --output=files 'tag:inbox' | grep -i '/spam/'` is +empty. |