diff options
Diffstat (limited to 'docs')
4 files changed, 3548 insertions, 423 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 2e941ec..7261bd9 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 @@ -7751,3 +7751,960 @@ new action and the existing ones gathered under it rather than duplicated. - **Item 160 unblocked this** on 2026-08-25: the status bar exists, so a manual save reports through `refreshDraftStatus()` like an autosave. Route `Ctrl+S` through `saveDraftNow()` and the reporting is already done. + +## 162. Delete fails while a sync is renaming the file underneath it + +**Observed (user, 2026-08-25):** deleting a draft reported `Cannot move +<file> to <account>/Trash`. + +**Cause (verified against the live Maildir, not read):** a stale path, and +neither Delete nor item 158 is at fault. + +1. The composer autosaves a draft as `<name>:2,D` and item 158 indexes it + under exactly that filename. +2. **mbsync uploads it and RENAMES it** to `<name>,U=<uid>:2,D`, recording the + server UID in the filename. +3. notmuch still holds the pre-`U=` name until that sync's `notmuch new` runs. +4. `moveMessages()` reads the filename from notmuch and calls + `QFile::rename()` on a path that no longer exists. It fails, the error is + emitted, and the message is skipped. + +Measured, in this order: `notmuch search --output=files` named a file that was +not on disk while `Background sync running...` was up, and the same query was +clean once the sync finished, with the file present under its new `,U=4` name. +That is why it reads as intermittent, and why it heals itself. + +**It is truthful and it loses nothing.** The move is skipped, no wrong folder +is created, no file is destroyed, and the next sync reconciles. The defect is +that the message blames a folder for a timing problem, and that the action +silently does nothing when the user asked for something. + +**This is `CLAUDE.md`'s `,U=` trap from the other side.** `MaildirName::fresh()` +exists because CARRYING that infix across a folder boundary produced +`Maildir error: duplicate UID` on real mail. Here mbsync is ADDING it and the +index lags; the same infix, the opposite direction. + +**Approach, and it needs a decision.** Two candidates: + +- **Refuse the move while a sync holds the lock.** `SyncMonitor` already + reports this, and the held-edit machinery from items 97 and 106 already + exists for exactly this shape: a tag edit made during a sync is held and + flushed when it ends. Delete would join it rather than inventing anything. + This is the likelier right answer, since it matches what every other + mutation already does. +- **Re-resolve the filename** from notmuch immediately before the rename and + re-query the message if the path is gone. Smaller, but it races the same + window it is trying to close, and a second lookup can be stale by the time + it is used. + +**Constraints.** + +- **Delete reaches the real mail server.** Read `CLAUDE.md`'s item 103 notes + before touching `moveMessages()`: a wrong folder name is created, adopted by + mbsync, and propagated to every other client. +- Whatever is built, **the message must say a sync is running**, not name a + folder. The current wording sent the user looking for a broken folder + configuration, which was correct and configured. +- A test cannot see this in the ordinary fixture layout, where nothing renames + a file underneath the index. Driving it means renaming the file between the + index write and the move, which is what the reproducer has to do. + +**Fixed 2026-08-25.** `moveMessages()` re-resolves by MESSAGE ID when the +recorded path is gone: one `reindexFolder()` of that directory, then the +filename from `notmuch_message_get_filenames()` that exists on disk. Bounded +to a single retry, so a genuinely missing file still reports rather than +becoming a silent no-op. + +**The hold candidate above was investigated and rejected**, and the reason +matters more than the fix. `sendMove()` ALREADY refuses while a sync holds the +lock and queues onto `m_heldMoves` (items 97 and 106 built it), yet the defect +still fired. `aSyncHoldsTheWriteLock()` tracks notmuch's write lock, while this +window sits between mbsync's RENAME and that sync's `notmuch new`; mbsync +renames throughout its run without touching that lock, so the damaging window +is open when there is nothing to observe. Refusing on the lock guards the wrong +resource. That refusal is correct for its own purpose and was left alone. + +Covered by `moveMessagesRecoversWhenASyncRenamedTheFile()` and +`moveMessagesStillReportsAMessageThatIsReallyGone()`, the second pinning the +bounded half. The test renames without reindexing, which is the window mbsync +opens, and asserts the database still names the old path so it cannot pass +against a fixture that quietly reindexed. Mutation-checked. + +## 163. The message pane shows a stale path, and the composer forks the draft + +**Observed (user, 2026-08-25):** selecting a draft filled the pane with +`(unreadable message)` and `This message could not be parsed.`, naming a file +under the account's drafts folder. + +**Cause (verified against the live Maildir):** the path in the pane ended +`.dnx:2,D`, and the only file on disk ended `.dnx,U=4:2,D`. Same mechanism as +item 162: mbsync renames an uploaded file to record its server UID, and the +name the application is holding stops existing. + +**The site is different, and so is the fix.** Item 162 is the WRITE path, +`moveMessages()` reading a filename from notmuch. This is the READ path, and +by the time it fires notmuch is already CORRECT: measured, the index named the +`,U=4` file while the pane still named the pre-`U=` one. The stale path is the +MODEL's, cached when the row was loaded, so refusing to act while a sync runs +(162's likely fix) would not help here at all. + +**The report is honest, which is why it is confusing.** `MimeParser` opened a +path that did not exist and said so. Nothing is lost and the next query +repairs it. + +**Approach.** The read path should RECOVER rather than refuse: on a failed +parse, re-resolve the message id through notmuch and retry once before +reporting. `recoverStaleThread()` already exists for the neighbouring problem +(item 91 reuses it) and is the shape to follow. + +**A SECOND site, found 2026-08-25 while hand-testing item 164, and its +consequence is worse than the pane's.** `MainWindow::openComposerFor()` passes +`ref.filePath` to `ComposeContextBuilder::forDraft()`, and `MessageRef::filePath` +is `notmuch_message_get_filename()` captured when the QUERY ran. After mbsync +renames an uploaded draft, a row loaded before that sync names a file that no +longer exists, `MimeParser::parse` fails, `forDraft` returns an empty context +and the composer reports "That draft could not be read". + +Measured, in order: draft written 10:42, sync at 10:50:43 logged "Detected 2 +file renames" and the file became `,U=5`, the user reopened it at 10:52 and +saw the error. + +**The pane's version is cosmetic and self-repairing. This one silently forks +the draft.** The refusal happens BEFORE `openComposer()`, so the user is left +with no composer for that draft. Composing again opens a FRESH one with +`m_draftPath` empty, so its first autosave has no previous path to unlink: +it writes a new file, `DraftStore` unlinks nothing, and the old revision +stays. Both are then uploaded and both reach the server. + +Two properties make this worse than it first reads. Each save mints a NEW +Message-ID (four saves in the hand test produced four ids), so the revisions +are distinct MESSAGES to notmuch rather than two files of one, and no dedup +anywhere will collapse them. And the unlink machinery is entirely correct +throughout, which is why the code reads fine: `forDraft` sets `draftPath`, +the composer seeds `m_draftPath`, the autosave passes it as `previousPath`, +and `DraftStore::write` unlinks it. The comment at `composecontext.cpp:501` +already names this exact failure ("one message becomes two"). None of it runs, +because the reopen was refused before any of it was reached. + +So the fix below covers this site too, and fixing only the pane would leave +the draft-forking half in place. + +**Constraints.** + +- **A retry must be bounded.** A message that genuinely cannot be parsed + (item 41's territory) must still report, or a real defect becomes an + infinite loop. +- Re-resolving by id is what makes this safe; re-scanning the folder is not, + since two files can carry the same id. +- The placeholder wording is correct and should stay for the genuine case. +- **The composer site must recover, not merely report better.** A clearer + error still leaves the user composing a second copy of their own draft. +- Whether a draft should keep a STABLE Message-ID across revisions is a + separate question this entry does not decide. It is what turns a stale path + into two server-side messages rather than one replaced file, and it wants + its own item; note that a draft's id is not yet the sent message's id, so + changing it is not obviously free. + +**Fixed 2026-08-25.** `MaildirName::resolveRenamed()` answers the filesystem +question: the path unchanged when it still exists, otherwise the file in that +same directory whose unique stem matches. mbsync preserves the stem +(`<stem>:2,D` becomes `<stem>,U=5:2,D`), which is what makes resolving by +filename safe here at all. It never recurses and never crosses a folder +boundary; a file that changed FOLDERS is a different question that only the +message id can answer, and `moveMessages()` re-resolves that way for item 162. + +Two refusals in it are deliberate and both have tests. An ambiguous match (two +files sharing a stem, which a correct Maildir cannot produce) yields nothing +rather than a guess, because opening or moving the wrong message is worse than +reporting none. And a genuinely missing file yields nothing too, so a real +deletion still reports instead of becoming a wrong answer. + +**Three call sites, not the one this was filed for.** The pane +(`renderMessages`), Reply and Forward (`openComposerFor`), and the draft reopen +(`forDraft`). The third is the one that cost data, and its shape is worth +keeping: the refusal happened BEFORE any composer existed, so the user composed +again into a fresh window whose autosave had no `previousPath` to unlink. The +unlink machinery was correct at every step and simply never ran. + +`forDraft()` seeds `draftPath` from the RESOLVED path. Seeding the caller's +would let the reopen succeed and the unlink still miss, which is the same fork +arriving one step later; the integration test asserts on `draftPath` for +exactly that reason. + +Covered by five unit tests on the resolver and by +`aDraftRenamedByASyncStillReopensAndReplacesItsFile()`, which renames the draft +the way mbsync does and asserts the file COUNT, the shape the fork actually +takes. Mutation-checked: removing the resolution fails it with the reported +symptom. + +**Left undecided, deliberately:** each save mints a NEW Message-ID, which is +what turns a stale path into two server-side MESSAGES rather than one replaced +file. That wants its own item. A draft's id is not yet the sent message's id, +so changing it is not obviously free. + + +## 104. Mail visible in Thunderbird never reaches qtmaildir + +**Observed (user, from the notes):** "sync doesn't work compared to thunderbird. +New mail received on thunderbird did not appear in qtmaildir. Need to investigate +further." + +**Cause: ESTABLISHED 2026-08-25, and it was this repository after all.** This +entry previously named mbsync's folder `Patterns` as the leading theory and +concluded "most likely not a code change here at all". That was wrong; the +superseded reasoning is kept at the bottom because the reproduction is what +overturned it. + +**`NotmuchWorker` opened one read-only notmuch handle and kept it for the +process lifetime.** A read-only handle is a Xapian SNAPSHOT taken when it is +opened; it never observes a write made by another process afterwards. The sync +script's `notmuch new` is exactly such a process, so every query the worker +answered after startup was served from the index as it stood when the +application launched. `openReadOnly()` returned early on `if (m_db) return +true;` and there was no `notmuch_database_reopen` anywhere in the tree. + +This accounts for every symptom, including the ones that defeated three earlier +theories during the diagnosis: + +- The post-sync refresh found nothing, so `refreshCurrentQuery()` and + `ThreadListModel::reconcile()` were each suspected in turn. Both are correct. +- A query the user typed BY HAND also found nothing. That is what rules out the + model, the generation counter and the account scope together: a fresh query + clears the model and re-runs from scratch, and it still hits the same stale + handle. +- A restart showed the mail instantly, with no sync in between. +- Tag WRITES were never affected, which is why the defect reads as "reading is + broken" rather than "notmuch is broken". `applyTags` opens its own read-write + handle per call, so it always sees current data. + +**Why it survived from 2026-08-16 to 2026-08-25.** The symptom needs mail to +arrive from outside the process while the window stays open, which is the +ordinary way this application is used and the one thing no test did: every +fixture opens a worker, queries it, and drops it. `TestNotmuchWorker::runQuery()` +builds a FRESH worker per call, so the suite was structurally incapable of +reproducing it, and a test written through that helper passes against the bug. + +**Fixed** in `NotmuchWorker::openReadOnly()`: when a handle already exists, +`notmuch_database_reopen(m_db, NOTMUCH_DATABASE_MODE_READ_ONLY)` before +returning it. Every read path begins by asking for the handle, so one call +covers all of them; putting it at the call sites instead would be one more +place to forget. A reopen failure is deliberately NOT fatal, since the existing +handle is still usable and answering from a slightly stale index beats refusing +to answer at all. + +Covered by `aQuerySeesMailIndexedAfterTheWorkerOpened`, which holds ONE worker +across two queries and runs `notmuch new` in a second process between them. +Mutation-checked: `after.size()` is 0 without the fix, 1 with it. The first +query asserts zero results before the message is written, so "found nothing" +cannot mean "the query was malformed". + +**The reproduction, kept because this entry's Approach section asked for exactly +this and it took three wrong turns to get there.** Four messages sent to one +account on 2026-08-25, viewed in that account's Inbox, synced with the app's own +Sync button. The three layers resolved as: on disk (yes), indexed (yes), shown +(no), which is layer 3 and therefore this repository. Two of the four matched +the running view's exact query (`path:"<account>/**" and (tag:inbox)`, 2 results +from the shell) and were absent from a window that had been open across the +sync. + +Two measurement errors made during that diagnosis, both worth repeating because +each produced a confident wrong answer: + +- `notmuch count 'inbox and path:...'` was used to check the view's contents. A + bare `inbox` is a FREE-TEXT term, not a tag term; the app generates + `tag:inbox`. The bare form returned 0 where the real query returns 2, which + briefly made the defect look like a tagging problem. +- The messages' tags were first read across every file matching the subject, + including the sender-side Sent copies in other accounts. That mixed three + accounts' messages into one answer. + +**Superseded theory, kept for the record.** mbsync fetches Gmail folders by +pattern and three of the five channels name their folders explicitly, so a +message labelled anything else is in a folder mbsync never asks for while +Thunderbird, speaking IMAP directly, sees it. That mechanism is real and would +produce a similar symptom, but it is not what was happening here: the mail was +on disk and indexed. It remains a plausible cause of any FUTURE report of this +shape, so check layer 1 before assuming this fix covers it. + +**One inconsistency worth reporting regardless**, found while checking the above +and still true: one of the Gmail accounts is configured in `qtmaildir.conf` with +a sent and a drafts folder, while its mbsync channel has `Patterns "INBOX"` and +fetches neither. The Sent and Drafts filters for that account can therefore only +ever be empty. That is real, independent of this item, and outside this +repository. + +**Size: XS.** Done. + +## 167. No way to tell one build of an unreleased version from another + +**Observed (user, from the notes):** "we should add a dev build number to be +pushed everytime we rebuild after a fix, so that I can verify if I'm in the +correct app version." The note has sat unrecorded through several sessions; +the 2026-08-25 reconciliation is the first to pick it up. + +**Cause (verified in the code, 2026-08-25.)** The version lives in exactly one +place, `project(qtmaildir VERSION ...)`, and `src/version.h.in` interpolates +`@PROJECT_VERSION@` and nothing else. That is correct for a release and says +nothing between two of them: the string moves only when the release procedure +bumps it, so every rebuild of `0.27.0` reports `0.27.0`. The status table above +shows why it bites in practice, since most closed items since 0.27.0 read +"unreleased" and the user hand-tests each one against a binary they rebuilt +themselves. + +Both surfaces that show the version take it from the same macro, so whatever is +added reaches them at once: the window title (`mainwindow.cpp:939`), the About +dialog (`mainwindow.cpp:2449`), the placeholder pane (`messageview.cpp:547`), +`--version` and `--help` (`main.cpp`). + +**Approach.** Needs a DECISION before any code, because the two candidates fail +in opposite directions. + +A git description (`git describe --always --dirty`, or the short hash) is +accurate and self-explaining: it names the commit the binary was built from, and +a reviewer can check out exactly that. Its cost is that CMake computes it at +CONFIGURE time, so a build after a new commit reports the previous hash unless +the configure step is made to re-run, which is a custom command with a dependency +on `.git/HEAD` and the packed refs, and is the part that usually ships subtly +wrong. + +A monotonic counter always moves and needs no git, but it means nothing on its +own: build 412 does not say which fix is in it, and it differs between the user's +machine and any other, so it cannot be quoted in a report. + +**Constraints.** A release build must keep printing a clean `X.Y.Z`, since the +SlackBuild in the `my-slackbuilds` repo builds from the release tarball where +there is no git checkout at all, and the release procedure checks +`./build/src/qtmaildir --version`. Whatever is added is therefore an addition to +the string in a dev build and absent in a release one, not a change to the +version itself. + +**Decision (user, 2026-08-25): the counter.** The git description was +offered as the recommendation and was not chosen; what the user wants is to +know a rebuild happened, not which commit it was. + +**Built 2026-08-25, unreleased.** `QTMAILDIR_BUILD_NUMBER`, a cmake option ON +by default, runs `cmake/BuildNumber.cmake` as a build step: it increments a +counter and writes `buildnumber.h`, which `version.h` includes. +`QTMAILDIR_VERSION_DISPLAY` is `X.Y.Z build N` when that macro is defined and +plain `X.Y.Z` when it is not. + +Two macros, not one, and the split is the load-bearing part. +`QTMAILDIR_VERSION` stays clean and keeps the window title, `applicationVersion` +and anything that might ever compare versions; `QTMAILDIR_VERSION_DISPLAY` goes +to the three surfaces the user picked: `--version`, `--help`, the About dialog +and the placeholder pane. The window title was offered and declined, since the +number would then sit in every screenshot. + +**The counter had to be a BUILD step, not `configure_file`.** That is the whole +reason this is not two lines: `configure_file` runs once per cmake run, so a +counter interpolated into `version.h.in` sits still across exactly the rebuilds +this item exists to distinguish. `version.h.in` therefore includes a second +generated header rather than carrying the number itself. + +The counter file lives in the build directory and is not tracked, so it cannot +conflict on a pull or dirty the tree; a fresh build directory restarts at 1, +which is honest, because it is a different build tree. A release build passes +`-DQTMAILDIR_BUILD_NUMBER=OFF` and the header is written empty. + +**Verified by running it**, since none of this is reachable from a C++ test: +three consecutive builds reported `build 2`, `build 3`, `build 4`, and a +separate Release configure with the option OFF reported a clean `0.27.0`. The +suite is 37 of 38, the one failure being item 136 on an unrelated path. + +**Size: XS**, as sized. + +## 166. Mail you send to your own other account loses `inbox` + +**Observed (agent, 2026-08-25, while setting up msmtp.)** Four test messages +were sent to one of the user's own accounts, one from each configured sending +account. All four were delivered and indexed. The two sent from accounts whose +Sent folder is fetched locally arrived in the recipient account's Inbox +**without the `inbox` tag**, so they were absent from that account's Inbox view. +The two sent from an account whose Sent folder is not fetched kept `inbox` +normally. + +**Cause: established, and it is the `post-new` hook, not this binary.** +`strip_inbox_from_sent()` in `assets/hooks/post-new` removes `inbox` from any +message matching a configured sent folder's PATH. Its docstring states the +assumption exactly: "the provenance is the file's own path: a message inside a +configured sent folder is one this system sent, and `inbox` was never true of +it." + +That holds for one file. It fails for one MESSAGE, because **notmuch +deduplicates by Message-ID and a message can have several files**. When the +sender and the recipient are both the user's own accounts, mbsync fetches two +copies: the sender's Sent copy and the recipient's Inbox copy. notmuch stores +them as ONE message with two filenames. The carve-out's query matches via the +Sent filename and strips `inbox` from the message object, which is the same +object the recipient's Inbox copy belongs to. + +Measured: one message, two paths, one in the sender account's sent folder and +one in the recipient account's `Inbox/cur`. + +The assumption is not merely incomplete, it is false in this case: the message +was genuinely sent AND genuinely received. There is no single right answer for +"was `inbox` ever true of this message", because it was true of one file and +false of another. + +**Approach.** Not settled, and the choice matters more than the code: + +1. **Strip only when EVERY file is in a sent folder.** Closest to the existing + intent, and it makes the predicate match the docstring's claim. A + self-addressed message keeps `inbox`, which is right: it did arrive. +2. **Strip only when the message has exactly one file.** Simpler to express, + but it silently stops protecting any sent message that happens to be + duplicated for an unrelated reason. +3. **Leave it.** Self-addressed mail is rare outside testing. The cost is that + it is invisible when it happens, and it looks exactly like the sync defect + item 104 turned out to be, which is how this was found. + +Option 1 is the one that makes the code true to what it already says it does. + +**Constraints.** + +- **The hook is this repo's**, `assets/hooks/post-new`, which the live + `database.hook_dir` symlinks to. It has its own suites beside it; run + `./test_post_new.py` and `./test_mailrules.py` from `assets/hooks/`. +- The hook **tags real mail unattended, every ten minutes, on the user's live + index.** A predicate that is wrong in the other direction would strip `inbox` + from arriving mail, which is the failure mode PROTECTED_REMOVALS exists to + prevent. Test against a throwaway database first. +- `notmuch tag` matching zero messages SUCCEEDS, so a log line saying the + carve-out ran is not evidence it matched anything. The count added for item + 164 is what distinguishes them; use it. +- Do not fix this by narrowing the query to exclude the recipient account. The + bug is in the per-file predicate, not in which folders are configured. + +**Fixed 2026-08-25, option 1**, the one the entry named: strip only when +every file is in a sent folder. + +`sent_only()` in `assets/hooks/post-new` filters the matches and the tag is +then applied per id. **It is a loop because no query can express it**, and both +plausible query forms were measured against a real two-file message before the +loop was written: `not path:"Inbox/**"` does NOT exclude the message, and +`notmuch count --output=files` on a path query reports every file of every +matching message rather than the files that matched. Both read as if they +worked and are wrong for one reason, that a notmuch term is a predicate over a +MESSAGE while the distinction here is between its FILES. + +The root comes from `database.mail_root`, not `database.path`, since this index +is split and no message file sits under the index directory. The mutation +putting `database.path` back passes every pre-existing test, because the +ordinary fixture keeps the index inside the mail root and both keys return the +same string; `setup_accounts(split_index=True)` is what catches it, and is the +Python counterpart to `NotmuchFixture::splitIndex()`. + +Two mutations fail: `all` to `any` loses `inbox` on the self-addressed message, +`mail_root` to `path` silently stops stripping anything. + +**Verified read-only against the live index**, tagging nothing: of 807 messages +matching a sent path, 780 are still stripped and 27 are spared, every one of +them two files with one in another account's Inbox. No arrival is affected. + +**Size: S.** Done. + +## 112. Toggle unread on a whole thread cannot reach "all unread" on a partly-read thread + +**Observed (user, 2026-08-17):** clicking a thread root and asking to mark the +whole thread unread does not do it. On a seven-message thread with two unread +replies, the result is that every message is toggled unread **except those +two**, which are left as they were. The user asks for an explicit "mark whole +thread read/unread" rather than a toggle. + +**Cause (verified in code):** the action exists, and its direction is the +defect. `toggle_unread_thread` (`src/mainwindow.cpp:931`, `Ctrl+Alt+U`) chooses +between adding and removing by asking +`everySelectedRowHasTag("unread", TagScope::Thread)`, which reads +`ThreadListModel::threadFor(index).tags`. That is notmuch's **union over the +thread** (`CLAUDE.md`, item 110), so a thread containing even one unread message +answers "unread" and the action picks *Mark thread read*. There is no input a +user can give that reaches *Mark thread unread* on a mixed thread: the only +threads that take that branch are the ones already entirely read, and the only +threads reporting "not unread" are the ones the user does not need the action +for. + +The write itself is absolute and correct. `tagSelected` with `TagScope::Thread` +adds or removes `unread` across every message, so the two unread replies in the +report are not skipped by the write. They are the reason the write ran in the +opposite direction from the one the user wanted. + +**A union is not a state, and a toggle needs a state.** This is the same class +as item 110 and the third time the union has produced a defect. Items 105 and 88 +fixed *which object* a toggle resolved; this one is about a thread having no +single answer to give. `everySelectedRowHasTag` is a two-valued predicate over a +three-valued reality: all read, all unread, or mixed. The mixed case is the one +that has no correct toggle direction, and picking either one silently is what +ships as "the action does the wrong thing". + +**Approach.** The user has already named it: stop toggling at thread scope. + +- Split `toggle_unread_thread` into two explicit actions, **Mark thread read** + and **Mark thread unread**, each with a fixed direction. Both appear in the + "Whole thread" submenu, where an entry always carries text, so a fixed label + is honest in a way a toggle's cannot be. +- The message-scoped `toggle_unread` stays a toggle. One message has a real + two-valued state, so the trap does not exist there. Do not "unify" the two: + the asymmetry is the point. + +**Constraints.** + +- **Adding an action is four places**, all enforced by tests that fail + confusingly: `KeyMap::knownActions()`, `defaultBindings()`, the icon table, + and the no-duplicate-icons exception list. See `CLAUDE.md`. Splitting one + action into two means one new entry in each, and the pair shares the twin's + icon under the existing named exemption for thread actions. +- **`Ctrl+Alt+U` is taken by the action being split**, and the whole-thread + bindings are already one modifier out from their twins because `Ctrl+Shift+U` + was claimed. Two directions need two sequences; if a second chord cannot be + found that is not worse than the menu, bind one and leave the other to the + submenu rather than inventing a three-modifier chord nobody will press. +- **This interacts with items 98 and 99**, which is the reason to decide all + three together. 99 asks for a dynamic label on the message-scoped toggle, + which is the opposite move: keep the toggle, make the label tell the truth. + A thread cannot do that, because on a mixed thread there is no true label to + show. Deciding 99 first will produce the wrong answer here by analogy. +- The undo entry must name the direction that ran (`Mark thread unread`), not + the action. `tagSelected` already takes the text, so this comes free from + splitting. +- **The test needs a MIXED thread**, which is the whole defect: a thread whose + messages are all in one state answers identically whichever way the direction + is computed, so a fixture built from a uniformly-unread thread passes against + the bug. Same trap as item 88's opposite-states requirement, recorded in + `CLAUDE.md`. + +**Built 2026-08-25 to the USER'S NOTE, not to the approach above**, which had +this half right and was shipped that way first. The approach proposed splitting +the thread toggle and explicitly said to leave the message-scoped one alone, +deciding 99 separately. The user's note is ONE design across both, and the +entry's own constraint said so ("this interacts with items 98 and 99, which is +the reason to decide all three together") without following it. The half-built +version was handed over, corrected by the user, and rebuilt. + +Four parts, all of them the note's: + +- The thread toggle splits into `mark_thread_read` and `mark_thread_unread`, + both absolute. **Neither carries a default chord**, at the user's choice: + since item 132 a shortcut is a chosen subset, and `Ctrl+Alt+U` meant + whichever direction the union happened to pick, which is what made it wrong. + It is now unbound. +- The message-scoped `toggle_unread` STAYS a toggle, because one message has a + real two-valued state, and gains a label naming the direction it will go. +- On a selection with no single state that entry is **hidden**, chosen over + disabled by the user. There is no honest label for a mixed selection, and + the thread submenu is the route the note points at. +- The label follows a WRITE as well as a selection change, keyed on the + model's `dataChanged` rather than on the six call sites that apply an + optimistic update, so a new one cannot forget. Without it, marking the + current row read left the entry offering to do it again. + +`selectionTagPresence()` is the three-valued predicate this needed; +`everySelectedRowHasTag()` now delegates to it and keeps its two-valued +answer, which is all a DIRECTION needs. A label needs the third value, and +asking a two-valued predicate a three-valued question is what this item was. + +**Three mutations fail:** restoring the union predicate reports "wrong +direction on a mixed thread: Mark thread read", which is the user's original +symptom; showing the action on a mixed selection; and dropping the +`dataChanged` refresh. The suite is 37 of 38, the one failure being item 136 on +an unrelated path, and the four new strings are translated with `lrelease` +reporting 0 unfinished. + +**Closes 99 and 147 with it**, which were the same note recorded twice. + +**Size: S.** Done, at roughly twice the entry's scope because the entry's scope +was wrong. + +## 118. No way to empty the trash from inside the app + +**Observed (user, 2026-08-17):** raised while reviewing item 103's spec, as +something that had been forgotten rather than newly noticed: "we could add +'Empty Trash' to the backlog as a future item. I forgot it existed, but I don't +want to squeeze it in this spec." + +**Blocked on 103**, which creates the trash folder this would empty. Until that +ships there is nothing to empty: Delete writes a tag and moves no file, so no +account has a populated trash folder except through another client. + +**Deliberately excluded from 103's spec**, at the user's request and recorded in +its "Out of scope" section. Worth keeping separate for a reason beyond scope +control: emptying the trash is the first action in this application that would +destroy mail with no undo. Every mutation so far is a tag or, after 103, a move, +and both are reversible. A purge is not. + +**Approach, unspecified.** The shape depends on decisions not yet made, and the +spec for 103 answers none of them: + +- **Local or remote.** Deleting the files locally and letting `Expunge Both` + carry it to the server is one thing; asking the provider to empty its own + trash is another, and mbsync offers no verb for the latter. The first is + probably what "Empty Trash" should mean here. +- **Whether the no-confirmation rule survives it.** It does not, on the face of + it. `CLAUDE.md` grants undo in place of confirmation dialogs, and this is the + action where undo cannot exist. That makes it the second item, after 103, that + re-examines the rule rather than assuming it, and unlike 103 it will probably + have to break it. +- **Per-account or all-accounts**, which should follow whatever the Trash filter + does once 103 ships rather than being decided independently. + +**Built 2026-08-25**, unblocked by 103. The three questions the entry left open +were put to the user and answered: + +- **Local, and let the sync carry it.** The files go, and a channel with + `Expunge Both` propagates that to the server. mbsync offers no verb for + asking a provider to empty its own trash, so the alternative was to delete + locally and not care, which brings the mail back on the next sync and reads + as the action having silently failed. +- **It confirms**, naming the count and the account, defaulting to Cancel, with + no default shortcut. CLAUDE.md now records this as the ONE exception to the + no-confirmation rule, in the same paragraph that states the rule, so the next + reader meets both together. +- **Scoped to the account selector**, like every other account-aware surface, + which is what the entry asked for. + +`NotmuchWorker::purgeMessages()` is a separate entry point from +`moveMessages()` rather than a flag on it, because the two look alike and only +one can be undone. It takes named ids only, never a folder sweep, so the blast +radius is what the dialog enumerated and the user confirmed. It deletes EVERY +file of a message: notmuch deduplicates by Message-ID, and leaving one behind +would leave the message alive in the folder the user emptied, which is the same +one-message-many-files property item 166 turned on. + +`resolveQueryMessages()` is a four-line wrapper over the existing private +`resolveQuery()`, so enumerating what is about to be destroyed needed no new +walk. The count in the dialog comes from the DATABASE rather than the model, +which holds whatever the current view is showing and is usually not the trash. + +**A defect surfaced while writing the tests**, and it is the one worth +remembering: the first version counted a message whose file was already gone as +destroyed, so the number reported for an irreversible action overstated it. An +absent file is correctly not an ERROR, since the index can name a path a sync +has removed; the mistake was treating "not an error" as "destroyed". The +mutation that restores it now fails. + +**A second defect was found by the user's own hand test**: the mail was +destroyed correctly and the LIST went on showing it until they re-ran the query +themselves. A purge is the one mutation with no optimistic update available, +because it removes rows rather than changing them, so `messagesPurged` re-runs +the current query. Nothing was connected to that signal at all, which is the +kind of gap a green suite is happy to keep. + +Verified against the live index after the user emptied one real account's +trash: zero files on disk, zero in the index. + +**Item 168 was filed from the same hand test**, on Delete being offered on mail +already in the trash. + +**Size: S.** Done. + +## 168. Delete is offered on mail already in the trash, and does nothing + +**Observed (user, 2026-08-25, while hand-testing item 118):** "I noticed I can +hit delete via context menu on a message already in the trash. Seems like a +bug, unless that action doesn't do for one message what Empty trash does for +the whole view." + +It does not, and the guess in the second half is worth recording as the reason +this matters: the user's mental model was that Delete on already-trashed mail +might PURGE it. It does not, and nothing about the menu says so. + +**Cause (verified in code, 2026-08-25.)** `moveMessages()` compares the file's +directory against the destination and takes an early-return branch when they +match (`notmuchworker.cpp`, the "already where it was asked to go" branch, +added when a fresh Maildir name made a path comparison useless). That branch +appends the id to `moved` and records an origin, so the message is reported as +having moved when nothing happened. The UI counts an unsynced change for it. + +Nothing is destroyed and nothing is corrupted; the cost is a menu entry that +lies about having done something, and a pending-changes count that overstates +what a sync has to carry. + +**The mirror of the same defect is already shipped beside it.** `restore` is +added unconditionally to both the Message menu (`mainwindow.cpp:1956`) and the +thread context menu (`mainwindow.cpp:2119`), so it is offered on mail that was +never deleted, where it has as little meaning as Delete has in the trash. + +**Approach.** The user chose to hide each action where it has no meaning, +which is the principle item 112 established for the unread entry: an action +with no honest meaning for the selection is absent rather than present and +inert. + +- Delete is hidden when every selected row is already in a trash folder. +- Restore is hidden when no selected row is. +- The test for both needs a MIXED selection as well as uniform ones, for the + reason item 112 records: a selection whose rows agree answers identically + whichever way the predicate is computed. + +**Constraints.** + +- **The question is about the PATH, not the tag.** A message trashed by + another client carries no `deleted` tag at all, which is why item 103 made + the trash view path-based. Asking `tags.contains("deleted")` here would + offer Delete on exactly the mail the user is most likely to be looking at + in a trash view. +- **`selectionTagPresence()` is the wrong instrument** for the same reason, + though it is the right shape. A path predicate needs the row's path, which + `MessageNode` carries. +- Deciding this does not require deciding item 118's relationship to it: a + purge stays an explicit whole-view action, and hiding Delete does not make + Delete a purge. + +**A second request, from the same tangent (user, 2026-08-25):** "messages moved +to the trash should be automatically marked `-unread`." Deleting is a decision +about the message, so leaving it bold and unread in the trash is noise; the +count of unread mail should not include what the user threw away. + +It is one line where Delete already composes its tag change, and it carries a +constraint worth stating rather than discovering. `maildir.synchronize_flags` +is true, so removing `unread` REWRITES the Maildir filename and reaches the +server on the next mbsync. That is acceptable here and is a deliberate +exception: it is the same mechanism the `post-new` hook refuses to touch on +arriving mail, for the good reason that the hook acts unattended on mail the +user has not seen. A Delete is an explicit gesture on a message in front of +them, which is the difference. + +Undo must put it back. `TagChange::inverted()` already does, provided the +removal travels as part of the SAME change rather than as a second write, so +one undo returns both the folder and the tag. + +**Built 2026-08-25**, both halves, to the user's own choice of "hide each +where it has no meaning". + +`everySelectedRowIsInATrashFolder()` asks each row about its own file, a reply +row's message and a thread row's displayed message, the same rule +`everySelectedRowHasTag()` follows. `refreshTrashActions()` runs beside +`refreshUnreadAction()` on both the selection change and the model's +`dataChanged`, so the entries follow a write as well as a selection. + +The `unread` removal travels inside the SAME `sendMove()` call rather than as a +second write, which is what makes one undo return the folder and the tag +together. + +**A mutation survived the first round and is worth recording**: comparing the +prefix WITHOUT its trailing separator passed every test, because no fixture had +a folder whose name starts with the trash folder's. `acct/trash-old` is a +different folder, and under that mutation Delete silently disappeared from mail +that had never been trashed, which is the quiet half of the same mistake. The +fixture carries that row now and the mutation fails. + +All three properties are mutation-checked: the separator, Restore's visibility, +and the `unread` removal. The suite is 37 of 38, the failure being item 136 on +an unrelated path, and no new user-facing strings were added. + +**Size: S** for the visibility half, XS for the `unread` half. Done. + + +## 68. A forwarded subject gets no `passed` tag + +**Observed (user, from the notes):** "passed tag should appear when subject is +`Fwd:` and `Fw:`." Refined in session on 2026-08-11: the user had noticed +`passed` appearing on messages whose subject carried `Fwd:` and not on `Fw:`, +and asked to expand the rule to both. + +**Cause:** there is no rule to expand. `passed` is the Maildir `P` flag in the +message filename, translated into a tag by notmuch because +`maildir.synchronize_flags=true`. The flag is written by whichever client +forwarded the message, or by the server over IMAP; nothing reads a subject line +anywhere in the chain. qtmaildir only ever colours the tag +(`src/tagcolors.cpp:36-37`) and the database's `post-new` hook does not mention +it either. + +**Measured against the real database (2026-08-11):** + +| Query | Count | +|---|---| +| `tag:passed` | 6 | +| `tag:passed and subject:"Fwd:"` | 1 | +| `tag:passed and subject:"Fw:"` | 0 | +| `subject:"Fwd:" and not tag:passed` | 194 | +| `subject:"Fw:" and not tag:passed` | 28 | + +Six tagged messages in the whole database, and every one of them carries `P` in +its filename flags. The single overlap with `Fwd:` is a message that was +forwarded and whose subject was already a forward, not evidence of a rule: 194 +`Fwd:` subjects carry no tag at all. The correlation the observation rests on +does not exist. + +**Approach and the decision it needs first.** Two different features, and the +measurements above decide how far apart they are. + +*Display only.* The card shows a forwarded mark when the subject matches. Touches +no mail, changes no flag, reversible by deleting the rule. XS. + +*Write the tag.* qtmaildir sets `P` from a subject heuristic. With +`maildir.synchronize_flags=true` that flag is a filename change that mbsync +carries out to the server, on 222 existing messages, on a guess about a string. +Not cleanly undoable, and it asserts a meaning for a flag this application did +not define. Recommended against; recorded so the choice is deliberate rather than +forgotten. + +**Constraints:** localised clients use their own prefixes, and `Fwd:` can appear +inside a subject rather than at its head, so whatever matches must be anchored. +If the tag is ever written, it must not be re-applied on every sync in a way that +produces pending edits the user never made, item 28 is the record of a count +going wrong. The display-only route avoids that entirely, since it derives the +mark at paint time and stores nothing. + +**Size: S** as written, XS if it is display only. Most of it is the decision, not +the code. + +**Status:** left open deliberately on 2026-08-11. The cause is settled and the +options are costed; the user has not chosen, and no code was written. + +**Built 2026-08-26, and the observation was wrong in a way worth recording.** +The note asked for one thing (expand a subject rule to `Fw:`) and the +measurement above had already shown there was no subject rule and no +correlation to expand. Taken literally the item was unbuildable; taken as what +the user actually wanted ("I want to know visually if someone has forwarded a +message to me") it split into three, and the user chose all three. + +**1. `replied` on a reply, `passed` on a forward.** The gap the item was +really sitting on, and it was never reported. Measured 2026-08-26 against the +developer's own index: 317 `replied` and 6 `passed`, spread over five +accounts, every one of them written by another client or the server. Nothing +in qtmaildir has ever written either flag. `ComposeWindow` emits +`sourceMessageAnswered` after a SUCCESSFUL send; `MainWindow` routes it +through `sendMessageTagChange`, message-scoped, off the undo stack for the +reason `markCurrentThreadRead` gives (the flag records that the mail went, and +the send cannot be undone, so an undo that retracted only the flag would leave +the two disagreeing). + +**Two traps here, one of which was caught only by reading.** `inReplyTo` is +deliberately EMPTY on a forward (carrying In-Reply-To would file the forward +under the thread it left, in the recipient's client), so keying the emit on it +made the `passed` half dead code that compiled and never fired. `ComposeContext` +carries `sourceMessageId` instead, set for all three kinds. And a resumed +`Kind::Draft` is excluded: its kind records how the FILE was opened, not what +the user is doing, so a draft that began as a reply cannot be told from one +that began as a new message. The cost is a missing flag on a reply finished in +two sittings, which is the safe direction, since `maildir.synchronize_flags` is +on and a wrong flag reaches the server. + +**2. A received-forward mark, display only.** `Marks::Mark::ReceivedForward`, +a seventh SVG, drawn from `ThreadListModel::IsReceivedForwardRole` in BOTH the +thread and the message branch per CLAUDE.md's rule. It is a different mark +from `passed` on purpose: `P` means "I forwarded this", which is a different +fact about a different person, and setting it from a subject guess would +assert something false on 222 existing messages and propagate it to the +server. Derived at paint time, stores nothing. + +**3. `[general] forward_prefixes`.** `subjectIsForwarded()` lives beside +`forwardSubject()` and shares its prefix table, so "do not double the prefix" +and "this is a forward" cannot drift apart. The config key EXTENDS that table +rather than replacing it, so adding a locale does not lose the measured +English/German/Iberian/French spellings. A `Re:` chain is stripped first +(bounded to 8, since the subject is input from a stranger and this runs per row +per repaint), so `Re: Fwd: x` is recognised. + +**A mutation survived the first round and corrected a claim in the code.** The +word-validation guard on a configured prefix was commented, and tested, as +protecting against an invalid pattern from an unescaped `(`. Measured with a +standalone probe: `QRegularExpression::escape` already makes punctuation inert +rather than invalid, so that test passed against the guard being removed. What +the guard actually buys is narrower and real: a configured `-` would match +`-: x` and a digit would match `2: x`. The comment and the test now assert +that instead. + +**Not built, and left as the item's own recommendation:** writing `P` from a +subject heuristic. Rejected on the same grounds the entry gave before the +work started. + + +## 119. The unsynced-changes count cannot be opened to see what it counts + +**Observed (user, from the notes):** "the bottom left statusbar message needs to +be clickable and show what 'N unsynced changes' are in a modal window". + +**Cause (verified in the code).** `m_pendingLabel` is a plain `QLabel` added to +the status bar with `addPermanentWidget` (`src/mainwindow.cpp:502-505`). A +`QLabel` has no clicked signal and none is installed, so there is nothing to +click and no route to a list. It carries a tooltip and nothing else. + +**The count is a SUM OVER FOUR SOURCES, and that is what makes this bigger than +it looks.** `pendingEditCount()` returns +`m_pendingTagEdits.size() + m_unnettablePendingEdits + held + heldMoves`. +Three of those can name what they hold: `m_pendingTagEdits` is a +`QHash<QString, bool>` keyed by message id, `m_heldEdits` and `m_heldMoves` are +queues of edits waiting for a sync to end. **`m_unnettablePendingEdits` is a +bare `int`** (`src/mainwindow.h:1248`), deliberately so: it counts confirmed +changes that carry no message ids and therefore cannot be netted against +anything. + +So a dialog built from what is currently kept would list three of the four +groups and then have to account for a remainder it cannot describe. Showing "and +3 more" is worse than the tooltip, because the user opened the window +specifically to find out what those were. + +**Approach.** Two halves, and the second is the real work. + +- The clickable half is small: a label that emits on click (an event filter, or + a flat `QToolButton` styled as a label), plus a dialog listing what the three + describable groups hold. The message pane already resolves an id to a subject. +- The complete half needs `m_unnettablePendingEdits` to become something that + can name its entries. Its comment says why it is an int: understating the + indicator is the direction that costs the user work, so it counts what it + cannot identify rather than dropping it. Making it describable means finding + out what those changes actually are and whether they can carry an id. + +**Constraints.** + +- **The count is deliberately conservative and must stay so.** Item 28 and item + 54 both landed on this indicator being wrong in the direction that made the + user think their work was safe. A dialog that lists fewer changes than the + count claims is the same failure in a new place: reconcile the two, or state + the remainder honestly rather than hiding it. +- **An external `notmuch` run can clear pending changes without this count + noticing**, which the tooltip already admits. A dialog makes that staleness + much more visible, since a listed change may no longer exist. Worth deciding + whether the dialog re-verifies against the database before showing. +- Read-only. This is an information window, not a place to retry or discard a + change; either would be a new mutation path with its own undo question. + +**Size: S** for the clickable half over the three describable groups. **Unknown** +for the fourth, and the item is not complete without it. + +**Closed 2026-08-26.** The blocker above was investigated first and did not +survive: `m_unnettablePendingEdits` counted confirmed changes carrying no +message ids, and `NotmuchWorker::applyTags()` (the only emitter of +`tagsApplied`) returns early on an empty id list, which is that exact +condition. `applyTagsToThreads()` resolves through a query and errors out on +an empty result, so it cannot hand `applyTags()` an empty list either. + +**Measured rather than read**, twice, because reading is what produced the +wrong answer the first time: a `qFatal` in the branch fired in 4 of 70 +`test_mainwindow` cases, all four building a `TagChange` by hand and invoking +the slot directly with no worker, and a `Q_ASSERT` before the worker's own +emit never fired across the whole suite. The counter was deleted and the +guard it shadowed is pinned where it lives, by +`applyTagsWithNoIdsDoesNothing()` in `test_notmuchworker`. + +Built in four commits: the snapshot, the subject resolve, the dialog and the +click, then a sizing fix after a hand test. + +Three decisions the user made, each of which shapes the code: + +- **Scope follows the ACTION, not the storage.** A thread action shows one + thread row with the count of messages it covered; a message action shows + its message. The three queues already encoded this, so nothing is expanded + and nothing is escalated: `HeldEdit` is thread-scoped because a `*_thread` + action made it, and everything else carries message ids. +- **A snapshot, frozen.** Taken at the click and never refreshed under the + user, who asked for exactly this: "if I keep the popup open for 20 minutes, + I don't want the popup to keep updating the info it's showing me." +- **Subjects resolved, stale rows kept.** An id the index no longer holds + still gets a row saying its subject is unknown, because the count the user + clicked has to equal the list they are shown. + +The re-verify question this item worried about mostly dissolved: item 54 +already clears the count when an external sync carries the edits, so a change +applied by cron does not survive to be clicked on. + +`resolvePendingSubjects()` answers POSITIONALLY, one subject per input row, +because one id can legitimately appear on several rows and a combined query +returns a set. `PendingChangeRow::startsMessage` is carried rather than +inferred from a non-empty subject, so an unresolved id still opens a run of +its own instead of folding its actions under the message above it. + +Read-only, per the constraint above. The dialog's height is sized to its +content; that is a hand test, since the offscreen platform returns an +identical frame either way. + 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 6041e28..69aac44 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 @@ -131,12 +131,12 @@ taking that too literally. | 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 | **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 | +| 65 | No full code review and optimization pass | correctness | ? | open, **narrowed 2026-08-26**: the notes now name it as a dead-code and duplication sweep, not a performance or security pass. Produces a LIST for the user to decide on, not a diff. See the entry | | 66 | Selecting a thread root leaves the message pane blank until a reply has been selected | defect | S | **done** 2026-08-14, unreleased. Not the blank pane it was filed as: the root rendered the CONVERSATION until the thread had been expanded once, then one message. Now always one message, and the conversation view is removed at the user's request. **One case unverified by hand:** the notes also report a single-message `id:` query whose card would not open, which is the same empty-`MessageIdRole` failure and should be gone; confirmed 2026-08-15 as a SEPARATE defect with a different cause, see item 96 | | 87 | Auto mark-read marks a whole thread, including replies never displayed | defect | S | **done** 2026-08-16, unreleased. Built on 108, which is why it stayed small: the timer tracks a MESSAGE id now, and arms for a reply too, which it never did before | | 88 | `threadAt(current.row())` answers about the wrong thread for a reply row | defect | M | **done** 2026-08-16, unreleased. The audit found FOUR live sites, not one. `ThreadListModel::threadFor(index)` resolves a reply through its parent; every caller holding a selected index converted, and no `.row()` on a selected index remains in `mainwindow.cpp`. Unblocks 87 | | 67 | The placeholder pane counts unread, flagged and inbox, but not sent or drafts | information | XS | **done** 2026-08-11, shipped in 0.15.0 | -| 68 | A forwarded subject gets no `passed` tag | workflow | S | open; no subject rule exists, measured 2026-08-11. Decision needed: display mark (XS) or write the flag (S, syncs out) | +| 68 | A forwarded subject gets no `passed` tag | workflow | S | **done 2026-08-26**, unreleased, as THREE things once the premise was measured away. The note asked to expand a subject rule to `Fw:`; there was no subject rule, and the correlation it rested on did not exist. What did exist was a gap nobody had reported: qtmaildir has never written `R` or `P`, so a reply and a forward now flag their source (off the undo stack, per the auto-mark-read precedent), and `subjectIsForwarded()` drives a SEPARATE received-forward mark, display only, extendable through `[general] forward_prefixes`. The user chose all three | | 69 | `passed` and `replied` read as words where every other state is a glyph | presentation | S | **done** 2026-08-11, inside item 70 | | 70 | Pane icons are a private set where the main window uses the system theme | presentation | M | **done** 2026-08-11; six shipped SVGs | | 71 | A toolbar action does not sync, so the edit sits until the next cron run | workflow | S | **done** 2026-08-11; 2s default, `auto_sync_delay_ms` | @@ -165,12 +165,12 @@ taking that too literally. | 96 | A query returning the thread already on display opens onto the placeholder | defect | S | **done** 2026-08-15, unreleased. Split from 66's unverified half, which had a different cause. Reproduced from two screenshots after four measured eliminations | | 97 | An edit made during a sync is reverted in the list when the sync ends | defect | S | **done** 2026-08-15, unreleased. Found by hand-testing item 89's fix. The sync-end refresh ran BEFORE the held-edit flush, so it read a database that still carried the old tag | | 98 | "Important" adds the tag but cannot remove it, unlike every other toggle | defect | XS | **done** 2026-08-17, unreleased. Calls `everySelectedRowHasTag()`, as the entry required. Its reply test needed THREE different states (list-first thread, the reply's own thread, the reply) before it could tell the two wrong answers apart; with the reply defaulted to its thread's state the item 105 mutation stayed green, measured | -| 99 | The unread action is labelled "Toggle unread" whichever way it will go | presentation | S | open; depends on 98's toggle shape, and the label is harder than it looks | +| 99 | The unread action is labelled "Toggle unread" whichever way it will go | presentation | S | **done 2026-08-25**, unreleased, with 112: the user's note is ONE design across both. The label names the direction it will go, and the entry is hidden on a selection with no single state. `refreshUnreadAction()` reads the new three-valued `selectionTagPresence()` | | 100 | The message pane offers Back, Forward, Reload and Save page, none of which mean anything | defect | XS | **done** 2026-08-17, unreleased. `MessageView::removeBrowserActions()` filters the standard menu by `pageAction()` POINTER, never by text; `ViewSource` went with them, and stranded separators are swept | | 101 | Sync is account-aware for edits but not for the account the user is looking at | workflow | S | open; item 49 built the edit half deliberately. Needs a decision, see the entry | | 102 | The rules table shows no note, so the field explaining a rule is invisible until it is opened | workflow | XS | **done** 2026-08-17, unreleased. A Note column before `ColumnCount`, so the appended Matches column stays last. Found a second defect on the way: `restoreState` REFUSES a header state with a different column count, and the sized flags were being set regardless | | 103 | What Delete does to mail on the server is undocumented and unverified | clarification | S+M | done; Delete moves to the account trash, with Restore and a stranded-mail cleanup. Section in the closed file | -| 104 | Mail visible in Thunderbird never reaches qtmaildir | defect | ? | open, reported 2026-08-16, cause NOT established. Most likely outside this repo; see the entry before writing code | +| 104 | Mail visible in Thunderbird never reaches qtmaildir | defect | XS | **done 2026-08-25**, hand-tested. The worker never reopened its read-only notmuch handle, so no query saw mail indexed after startup. Confirmed on a sync run from the application that added 20 messages: they appeared without a restart | | 109 | A root card's own message is invisible to a message-scoped write | defect | S | **done** 2026-08-16, unreleased. Found by hand-testing 108. `applyMessageTagChange` and `messageById` searched only the loaded replies, and a root's message is never among them, so the ORDINARY gesture repainted nothing and wiped the pane's chip row | | 110 | A card and the message pane show tags belonging to a message's siblings | defect | S | **done** 2026-08-16, unreleased. Found by hand-testing 109 against a real 4-message thread. `ThreadSummary::tags` is notmuch's UNION; a card standing for one message drew it. Also the reason a root card could not repaint at all | | 111 | A card should show its siblings' tags smaller, not drop them | presentation | S | **done** 2026-08-16, unreleased. The user's own design, from looking at 110's result: own tags full size, the thread's others smaller and muted, so nothing appears to vanish on selection | @@ -178,14 +178,14 @@ taking that too literally. | 106 | A tag change made on one message during a sync is silently lost | defect | XS | **done** 2026-08-16, unreleased. Found by READING while fixing 105, never reported. `flushHeldEdits` re-sent only thread-scoped edits, so a message-scoped one was shown, counted as pending, and never written | | 107 | A thread-scoped write leaves the loaded replies showing their old tags | defect | XS | **done** 2026-08-16, unreleased. `applyTagChange` updated the summary only, so marking a thread read left its expanded replies bold | | 108 | Acting on a thread root means the whole thread, though it displays one message | workflow | M | **done** 2026-08-16, unreleased. `messageScopeFor()` beside `scopeFor()`; five `*_thread` actions in a "Whole thread" submenu on `Ctrl+Alt+<key>`. User-visible: minor bump, `### Upgrading` written | -| 112 | Toggle unread on a whole thread cannot reach "all unread" on a partly-read thread | defect | S | open, found 2026-08-17. A toggle over a UNION has no direction on a mixed thread | +| 112 | Toggle unread on a whole thread cannot reach "all unread" on a partly-read thread | defect | S | **done 2026-08-25**, unreleased. Built to the user's own note rather than to this entry's approach, which had it only half right. The thread toggle splits into two absolute actions AND the message-scoped one keeps its toggle with a dynamic label, hidden when the selection disagrees. Closes 99 and 147 with it | | 113 | No way to see a message's HTML source | information | S | open, 2026-08-17. Chromium's own View source cannot work here; needs our own plain-text dialog. Item 100 removed the dead entry, which was an overreach: the user had not asked for it | | 114 | Save image is offered on every image and does nothing | defect | S | open, found 2026-08-17, re-confirmed by hand 2026-08-20. No `downloadRequested` handler exists, so the request is emitted and never answered. The handler is per-profile, so it must decide per request or it revives the Save link item 127 removed | | 115 | A copy from the message pane gives no confirmation | presentation | XS | **done** 2026-08-19, unreleased. Four entries report, each naming what it copied; connected to the page's own QActions, so the entry is covered wherever it is triggered from | | 116 | Copy image copies markup instead of the image | defect | XS | **dropped** 2026-08-17, same day. NOT A DEFECT: `wl-paste --list-types` run immediately after a copy reports `image/png`, `application/x-qt-image` and 30 more image flavours. The clipboard is correct and Chromium is behaving. The earlier "text only" reading was taken minutes late off a clipboard that had been overwritten, and a whole cause was theorised on it | | 117 | The message pane offers no Select all | workflow | XS | **done** 2026-08-19, unreleased. `addPaneActions()` supplies it. The call site is NOT covered by a test and cannot be: the production menu needs a real context-menu event. Stated in the test rather than faked | -| 118 | No way to empty the trash from inside the app | workflow | S | open, 2026-08-17. **Blocked on 103**, which creates the trash in the first place. Deliberately left out of 103's spec at the user's request rather than squeezed in | -| 119 | The unsynced-changes count cannot be opened to see what it counts | information | S | open, 2026-08-19, from the notes. One of the four things it sums carries no message ids at all, so a list cannot be complete without a change to how the count is kept | +| 118 | No way to empty the trash from inside the app | workflow | S | **done 2026-08-25**, unreleased. Unblocked by 103. `Message > Empty trash...`, scoped to the account selector, no shortcut. The one confirmation in this application, and CLAUDE.md now records it as the single exception rather than leaving it to be discovered. Found a defect while testing: the count claimed messages whose files were already gone | +| 119 | The unsynced-changes count cannot be opened to see what it counts | information | S | **done** 2026-08-26, unreleased. **The stated blocker was not real**: the fourth term counted confirmed changes with no message ids, and `applyTags()` returns early on exactly that condition, so it could never fire. Measured before removing it, not read. The label opens a read-only list, grouped as the user asked: subject once, actions beneath. Scope follows the ACTION, so a held thread edit stays one thread row and reports its message count. A snapshot, frozen once open | | 121 | The thread list shows nothing while a query is running | feedback | S | open, 2026-08-20, from the notes. Follows item 74, which fixed the status-bar half and left the list itself blank | | 122 | The README documents a version of the app that no longer exists | documentation | M | **done** 2026-08-23, unreleased, inside item 123 task 13. `trash`, `send_command` and the whole `[compose]` section were undocumented; a Composing section is added and "sending is not implemented" removed. Every default was read from `config.h` rather than from the prose, which caught `send_html` documented as false when it defaults to true | @@ -215,8 +215,8 @@ taking that too literally. | 143 | The formatting buttons are text, where every editor uses icons | presentation | XS | **done** 2026-08-24, unreleased, inside 142. `QIcon::fromTheme` per CLAUDE.md's chrome rule, the words kept as the tooltip, and an action whose theme lacks the name keeps its text rather than rendering an empty button | | 144 | "Also send a formatted copy" is prominent and does not say what it does | presentation | XS | **done** 2026-08-24, unreleased, inside 142. "Send as HTML", icon and text, alone at the right end of the editor bar where it reads as a control of the editor rather than as a formatting button. The Italian entry was refreshed with it, and `lrelease` reports 477 finished, 0 unfinished | | 145 | Cc and Bcc are permanent rows on every composer | presentation | S | **done** 2026-08-24, unreleased, inside 142. A `QToolButton` disclosure beside To:. `revealCcBccIfUsed()` is the load-bearing half the entry called for: it only ever SHOWS, never hides, so nothing but the user's own click can make a field holding an address invisible. `ComposeContext` carries no `bcc` at all, so the seeded-Bcc case can only arrive from a reopened draft, which is what its test drives. The LABEL is hidden with each field: a `QFormLayout` holds the two as separate items, so hiding the line edit alone strands a `Cc:` over empty space | -| 146 | The unsynced-changes count cannot be opened to see what it counts | information | S | **duplicate of 119**, recorded 2026-08-23 from the notes. Same request, and 119 already carries the blocker: one of the four things the count sums holds no message ids, so a list cannot be complete without changing how the count is kept | -| 147 | Toggle unread reads the same whichever way it will go | presentation | S | **duplicate of 99**, recorded 2026-08-23 from the notes. The notes ask for exactly what 99 describes: "Mark as read" on an unread message and the reverse. 99 already records that the label is harder than it looks, since a multi-row selection has no single direction | +| 146 | The unsynced-changes count cannot be opened to see what it counts | information | S | **done as 119** 2026-08-26. Duplicate, recorded 2026-08-23 from the notes; closed by the same work | +| 147 | Toggle unread reads the same whichever way it will go | presentation | S | **duplicate of 99**, recorded 2026-08-23 from the notes, and closed with it on 2026-08-25 | | 148 | Ctrl+W does not close the composer | discoverability | XS | **done** 2026-08-24, unreleased. A `QAction` parented to the composer, so it is a WindowShortcut dispatched to the active composer only and the main window's namespace is untouched, exactly like the formatting shortcuts. It calls `close()` rather than doing anything of its own: `closeEvent()` already decides whether the draft is saved, and a second route out that skipped it would lose the message. Not registered in `KeyMap`, so item 132's rules do not apply | | 149 | A reply's cursor lands on the attribution line, not on blank space | defect | XS | **done** 2026-08-24, unreleased, in TWO passes. The first fixed the cursor within each branch (`End` under Above, `Start` under Below) and the user still saw the old layout, because the branches were already right and the DEFAULT was wrong: `above` shipped, and the layout asked for is what `below` produces. Default flipped, and the composer now focuses the body whenever To: is already filled, which a Reply and a Forward always are. Both halves were invisible to the existing `theQuotePositionDecidesWhereTheQuoteLands`, which asserts the quote's position and never the cursor's | | 150 | The receive-only ribbon stays up after the message that raised it is gone | defect | S | **done** 2026-08-24, unreleased. One line in `MessageView::clear()`, beside the blocked-content bar, the stale notice and the attachment bar it already reset by hand. Only `setReceiveOnlyAccount()` hid the ribbon, which every SELECTION change reaches, so a row-to-row move was never the reproducer: it survived the FOUR routes that blank the pane without one (`clear_pane`, `clear_selection`, a new query, a multi-row selection). The first test written for it passed against the defect for exactly that reason | @@ -235,9 +235,15 @@ taking that too literally. | 160 | The composer never says a draft was autosaved | feedback | S | **done** 2026-08-25, unreleased. A status bar on the composer: the age line left, the `○ unsaved content` cue beside it. **The fix is a funnel, not a label.** `m_dirty` had SEVEN writers and four of them clear it, only two of which are a save, so a cue hung off the save path silently missed the constructor and the send; `setDirty()` is the one writer now and refreshes both cues plus `setWindowModified()`. Presentation was **reworked after the user looked at it**: it first reused item 151's yellow ribbon treatment, which reads as a misplaced widget on a bare status label, and the cue sat in the permanent (right-hand) tray. Two defects found by probing rather than by reading, see the section | | 161 | The composer has no menu bar | discoverability | S | **done** 2026-08-25, unreleased. File / Edit / Format, to the user's own chosen scope. **Save draft (`Ctrl+S`) is the only NEW action**; everything else is gathered, and the menus show the toolbar's own `QAction` objects rather than copies, per item 140's rule. Two needed hand-building: the HTML toggle is a `QToolButton` and cannot go in a menu, so a checkable twin mirrors it BOTH ways; and the signature entry takes the switch's own `QMenu` pointer, since that menu is rebuilt when the signatures change and copied entries would go stale. Edit's entries follow the editor's own `undoAvailable`/`copyAvailable`. `theMenuBarReachesEveryComposerAction()` is item 132's rule applied to the composer, walking the real menu bar and finding actions by `findChildren`, so a future action added to the toolbar and forgotten in the menus fails without touching the test | -| 162 | Delete fails while a sync is renaming the file underneath it | defect | S | open, 2026-08-25, found by hand. `Cannot move <file> to <folder>`. NOT a Delete defect and not item 158's: mbsync renames an uploaded file to add its `,U=<uid>` infix, and notmuch keeps the pre-`U=` name until that sync's `notmuch new` runs, so `moveMessages` renames a path that no longer exists. Truthful, harmless and SELF-HEALING, which is why it reads as intermittent: verified a ghost present mid-sync and gone after. The message names a folder as though the folder were the problem. Delete reaches the real mail server, so read `CLAUDE.md` before touching it | -| 163 | The message pane shows a stale path and reports the message unreadable | defect | S | open, 2026-08-25, found by hand. Same root as 162 and a DIFFERENT site: the model keeps the filename a row was loaded with, mbsync renames the file to add `,U=<uid>`, and `MimeParser` then opens a path that no longer exists and honestly reports "could not be parsed". notmuch is CORRECT by then; the UI is behind, so 162's fix (refuse the write while a sync runs) does not touch this. The read path needs to recover rather than refuse | -| 164 | Every newly synced draft carries `inbox` | defect | S | open, 2026-08-25, found by hand. Measured `draft inbox unread` on a draft this application wrote. `strip_inbox_from_sent()` in `assets/hooks/post-new` reads `qtmaildirconf.sent_folders()` only, and `qtmaildirconf.py` has no drafts equivalent, so the carve-out never covers a drafts folder. **Contradicts a shipped 0.27.0 changelog entry** claiming both are kept out of the inbox, so it is a documentation defect as well. Item 158's measurement was correct and did not cover this: `index_file` assigns no tags, but mbsync's upload and the next `notmuch new` re-tag the file | +| 162 | Delete fails while a sync is renaming the file underneath it | defect | S | **done, 2026-08-25.** mbsync renames an uploaded file to add its `,U=<uid>` infix and notmuch keeps the pre-`U=` name until that sync's `notmuch new` runs, so `moveMessages` renamed a path that no longer existed and Delete silently did nothing while blaming the destination folder. `moveMessages` now re-resolves by MESSAGE ID when the recorded path is gone: one reindex of that directory, then the filename that exists on disk. Bounded to one retry, so a file genuinely gone still reports. Holding the move during a sync was the other candidate and is NOT the fix: `sendMove` already refuses on notmuch's write lock, but this window sits between mbsync's rename and that sync's `notmuch new`, which touches no lock | +| 163 | The message pane shows a stale path, and the composer forks the draft | defect | S | **done, 2026-08-25.** mbsync renames an uploaded file to add its `,U=<uid>` infix while the model still holds the name the query returned. `MaildirName::resolveRenamed()` returns the path unchanged when it exists, else finds the file in that one directory whose unique stem matches; it refuses an ambiguous match and yields nothing for a genuinely missing file. Wired into all THREE read sites: the pane, Reply/Forward, and the draft reopen. The reopen was the one that cost data, forking a draft into two files with two Message-IDs, both reaching the server | +| 164 | A draft this application saved keeps `inbox` | defect | S | open, 2026-08-25, **cause corrected 2026-08-25**. The first diagnosis blamed a missing drafts helper and was WRONG: `NOT_ARRIVALS` in `qtmaildirconf.py` is `("sent", "drafts")`, the folder list includes every account's drafts folder, and `notmuch count` confirms the carve-out query MATCHES the affected draft. The carve-out is scoped to `tag:new`, and the draft carries `inbox` while `tag:new` is 0, so it was never in scope when the hook ran. Measured separately: an mbsync-style rename does NOT re-add `new.tags`, so the retag theory is out too. What remains unestablished is WHICH pass tagged it; establish that before writing code | +| 165 | A draft gets a new Message-ID on every autosave | enhancement | ? | open, 2026-08-25, found while hand-testing 163 and 164. `MessageBuilder::build()` generates an id unconditionally and every autosave calls it, so each revision is a distinct MESSAGE to notmuch and to the server rather than a new version of one. Invisible while the file is replaced correctly, which item 163's fix restores; it is what turned that fork into two messages rather than one duplicated file. Needs a DECISION on what a draft's identity is before any code: a stable id reused at send, a stable id discarded at send, or the status quo. Neither `ComposeContext` nor `OutgoingMessage` has a field to carry an id, so it is not a changed call site | +| 166 | Mail you send to your own other account loses `inbox` | defect | S | **done 2026-08-25**, unreleased. `sent_only()` keeps a message only when EVERY file is inside a sent folder, which is what the carve-out's docstring already claimed. No query can express it, measured; the root comes from `database.mail_root`, with a split-index fixture the ordinary layout cannot provide. Verified read-only against the live index: 780 of 807 still stripped, 27 spared, no arrival affected | +| 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 | open, 2026-08-26, from the notes. The accent bar exists (`CardLayout::accentRect`, `CardDelegate::accentLineColour()`); the gradient fade and the sender avatar do not. The avatar's initials source is decided, the vCard half is 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 | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -361,71 +367,36 @@ batches of 200 so a 10k-thread query paints immediately) already holds. An optimization pass with no measurement behind it is the kind of work that produces a large diff and no change a user can notice. -**What it needs before it can be sized.** The user saying which of these they -meant: a correctness/security review of a named area, a specific operation that -feels slow with the query that makes it slow, a dead-code and duplication sweep, -or the translatability audit that is already item 22. The first three are -different pieces of work with different sizes, and the fourth is already -recorded. - -**Size: `?`, unspecified.** Do not propose a design for this; ask. - -## 68. A forwarded subject gets no `passed` tag - -**Observed (user, from the notes):** "passed tag should appear when subject is -`Fwd:` and `Fw:`." Refined in session on 2026-08-11: the user had noticed -`passed` appearing on messages whose subject carried `Fwd:` and not on `Fw:`, -and asked to expand the rule to both. - -**Cause:** there is no rule to expand. `passed` is the Maildir `P` flag in the -message filename, translated into a tag by notmuch because -`maildir.synchronize_flags=true`. The flag is written by whichever client -forwarded the message, or by the server over IMAP; nothing reads a subject line -anywhere in the chain. qtmaildir only ever colours the tag -(`src/tagcolors.cpp:36-37`) and the database's `post-new` hook does not mention -it either. - -**Measured against the real database (2026-08-11):** - -| Query | Count | -|---|---| -| `tag:passed` | 6 | -| `tag:passed and subject:"Fwd:"` | 1 | -| `tag:passed and subject:"Fw:"` | 0 | -| `subject:"Fwd:" and not tag:passed` | 194 | -| `subject:"Fw:" and not tag:passed` | 28 | - -Six tagged messages in the whole database, and every one of them carries `P` in -its filename flags. The single overlap with `Fwd:` is a message that was -forwarded and whose subject was already a forward, not evidence of a rule: 194 -`Fwd:` subjects carry no tag at all. The correlation the observation rests on -does not exist. - -**Approach and the decision it needs first.** Two different features, and the -measurements above decide how far apart they are. - -*Display only.* The card shows a forwarded mark when the subject matches. Touches -no mail, changes no flag, reversible by deleting the rule. XS. - -*Write the tag.* qtmaildir sets `P` from a subject heuristic. With -`maildir.synchronize_flags=true` that flag is a filename change that mbsync -carries out to the server, on 222 existing messages, on a guess about a string. -Not cleanly undoable, and it asserts a meaning for a flag this application did -not define. Recommended against; recorded so the choice is deliberate rather than -forgotten. - -**Constraints:** localised clients use their own prefixes, and `Fwd:` can appear -inside a subject rather than at its head, so whatever matches must be anchored. -If the tag is ever written, it must not be re-applied on every sync in a way that -produces pending edits the user never made, item 28 is the record of a count -going wrong. The display-only route avoids that entirely, since it derives the -mark at paint time and stores nothing. - -**Size: S** as written, XS if it is display only. Most of it is the decision, not -the code. - -**Status:** left open deliberately on 2026-08-11. The cause is settled and the -options are costed; the user has not chosen, and no code was written. +**Narrowed by the user, 2026-08-26.** The notes now name two sub-bullets, and +they are the same piece of work rather than two: "deduplication of +functionalities" and "check for dead code (functionalities superseded by other +additions, rendering them useless now)". So this is a dead-code and duplication +sweep, NOT a performance pass and not a security review. Nothing slow has been +reported, and the translatability audit it might have meant is item 22, already +done. + +**What that makes it.** A read of the whole tree looking for a function with a +newer twin and for a path nothing reaches any more. The codebase has precedent +for both: `threadAt(int)` survives beside `threadFor(index)` for one legitimate +caller, `SubjectDelegate` was deleted outright at item 53, and item 132 deleted +a whole test rule that had stopped serving. The output is a LIST first, one +entry per candidate with the evidence that it is dead or duplicated, not a +diff; the user decides what goes. + +**Constraints.** + +- "Unreachable from the UI" is not the same as dead. Item 16's + double-press-to-undelete branch reads as dead and is not, because stranded + mail reaches it. Every candidate needs the reachability argument written out + before it is cut. +- A test is a caller. Deleting production code with only test callers is + usually right; deleting the test with it needs saying so explicitly. +- The sweep is worth nothing if it is not run against a green suite before and + after, since the whole value is that nothing observable changed. + +**Size: still `?` until the list exists.** The sweep that produces the list is +S to M; what it finds is the work. + ## 72. No khard/khal integration @@ -534,140 +505,6 @@ reaches it (item 42), so most of this exists. **Size: S** for the on-demand button, XS for the visibility half. Ask which. -## 104. Mail visible in Thunderbird never reaches qtmaildir - -**Observed (user, from the notes):** "sync doesn't work compared to thunderbird. -New mail received on thunderbird did not appear in qtmaildir. Need to investigate -further." - -**Cause: NOT established.** Recorded because it is a defect report about mail -going missing, which is the most serious kind this backlog carries, and it has -been sitting in the notes unrecorded. What follows is one measured mechanism that -would produce exactly this symptom, not a diagnosis. - -**qtmaildir cannot show what mbsync did not fetch, and mbsync fetches folders by -pattern.** Three of the five channels in the user's `~/.mbsyncrc` name their -folders explicitly: - -``` -Patterns "INBOX" "[Gmail]/Posta inviata" "[Gmail]/Bozze" "[Gmail]/Speciali" -``` - -and one names only `"INBOX"`. The two non-Gmail channels use `Patterns *`. -Gmail applies labels, and a message whose label is not one of those four is in a -folder mbsync never asks for. Thunderbird speaks IMAP directly and sees every -folder, so the same message is visible there and absent locally. This is a -configuration property of the user's mbsyncrc, outside this repository entirely. - -**One inconsistency worth reporting regardless**, found while checking the -above: one of the Gmail accounts is configured in `qtmaildir.conf` with -`sent = [Gmail]/Posta inviata` and `drafts = [Gmail]/Bozze`, while its mbsync -channel has `Patterns "INBOX"` and fetches neither. The Sent and Drafts filters -for that account can therefore only ever be empty. That is real, and it is -independent of whatever this item turns out to be. - -**Approach.** Reproduce before anything else, and the reproduction has to -distinguish three layers, because the fix lives in a different place for each: - -1. Is the message on disk? `find` in the Maildir, or `notmuch count` on a term - from it. If not, this is mbsync or `.mbsyncrc`, and there is nothing to - change here. -2. If it is on disk, is it indexed? `notmuch new` and count again. If not, this - is notmuch config, `new.ignore` or the hook. -3. Only if it is indexed and still not shown is this qtmaildir's defect, and - then the question is which query hid it: the account scope, the built-in - filter, or a rule that tagged it out of the inbox. - -**Constraints.** - -- Ask the user for one concrete example before investigating: which account, - roughly when, and what Thunderbird shows for it. A general "sync doesn't work" - cannot be reproduced, and the last four defects in this backlog were all found - from a specific message. -- The `post-new` hook from mailctl tags mail unattended. A rule that removes - `inbox` would make a correctly fetched, correctly indexed message vanish from - the default view, which looks identical to a sync failure from the outside. - `notmuch search` without a filter is what tells them apart. -- Do not change `.mbsyncrc` as part of this. It is the user's, it is outside the - repo, and a Patterns change refetches folders. - -**Size: `?`** until reproduced. Most likely not a code change here at all. - - -## 112. Toggle unread on a whole thread cannot reach "all unread" on a partly-read thread - -**Observed (user, 2026-08-17):** clicking a thread root and asking to mark the -whole thread unread does not do it. On a seven-message thread with two unread -replies, the result is that every message is toggled unread **except those -two**, which are left as they were. The user asks for an explicit "mark whole -thread read/unread" rather than a toggle. - -**Cause (verified in code):** the action exists, and its direction is the -defect. `toggle_unread_thread` (`src/mainwindow.cpp:931`, `Ctrl+Alt+U`) chooses -between adding and removing by asking -`everySelectedRowHasTag("unread", TagScope::Thread)`, which reads -`ThreadListModel::threadFor(index).tags`. That is notmuch's **union over the -thread** (`CLAUDE.md`, item 110), so a thread containing even one unread message -answers "unread" and the action picks *Mark thread read*. There is no input a -user can give that reaches *Mark thread unread* on a mixed thread: the only -threads that take that branch are the ones already entirely read, and the only -threads reporting "not unread" are the ones the user does not need the action -for. - -The write itself is absolute and correct. `tagSelected` with `TagScope::Thread` -adds or removes `unread` across every message, so the two unread replies in the -report are not skipped by the write. They are the reason the write ran in the -opposite direction from the one the user wanted. - -**A union is not a state, and a toggle needs a state.** This is the same class -as item 110 and the third time the union has produced a defect. Items 105 and 88 -fixed *which object* a toggle resolved; this one is about a thread having no -single answer to give. `everySelectedRowHasTag` is a two-valued predicate over a -three-valued reality: all read, all unread, or mixed. The mixed case is the one -that has no correct toggle direction, and picking either one silently is what -ships as "the action does the wrong thing". - -**Approach.** The user has already named it: stop toggling at thread scope. - -- Split `toggle_unread_thread` into two explicit actions, **Mark thread read** - and **Mark thread unread**, each with a fixed direction. Both appear in the - "Whole thread" submenu, where an entry always carries text, so a fixed label - is honest in a way a toggle's cannot be. -- The message-scoped `toggle_unread` stays a toggle. One message has a real - two-valued state, so the trap does not exist there. Do not "unify" the two: - the asymmetry is the point. - -**Constraints.** - -- **Adding an action is four places**, all enforced by tests that fail - confusingly: `KeyMap::knownActions()`, `defaultBindings()`, the icon table, - and the no-duplicate-icons exception list. See `CLAUDE.md`. Splitting one - action into two means one new entry in each, and the pair shares the twin's - icon under the existing named exemption for thread actions. -- **`Ctrl+Alt+U` is taken by the action being split**, and the whole-thread - bindings are already one modifier out from their twins because `Ctrl+Shift+U` - was claimed. Two directions need two sequences; if a second chord cannot be - found that is not worse than the menu, bind one and leave the other to the - submenu rather than inventing a three-modifier chord nobody will press. -- **This interacts with items 98 and 99**, which is the reason to decide all - three together. 99 asks for a dynamic label on the message-scoped toggle, - which is the opposite move: keep the toggle, make the label tell the truth. - A thread cannot do that, because on a mixed thread there is no true label to - show. Deciding 99 first will produce the wrong answer here by analogy. -- The undo entry must name the direction that ran (`Mark thread unread`), not - the action. `tagSelected` already takes the text, so this comes free from - splitting. -- **The test needs a MIXED thread**, which is the whole defect: a thread whose - messages are all in one state answers identically whichever way the direction - is computed, so a fixture built from a uniformly-unread thread passes against - the bug. Same trap as item 88's opposite-states requirement, recorded in - `CLAUDE.md`. - -**Size: S.** The write path is already correct and thread-scoped; the work is -the action split, the four registration sites, the binding decision, and a test -over a mixed thread. - - ## 113. No way to see a message's HTML source **Observed (user, 2026-08-17):** reviewing item 100's removals, "view source @@ -803,94 +640,6 @@ make Save image work must not make Save link reachable again. The test fails if it does, which is the point: the handler is per-profile, so the natural implementation would light up both entries at once. -## 118. No way to empty the trash from inside the app - -**Observed (user, 2026-08-17):** raised while reviewing item 103's spec, as -something that had been forgotten rather than newly noticed: "we could add -'Empty Trash' to the backlog as a future item. I forgot it existed, but I don't -want to squeeze it in this spec." - -**Blocked on 103**, which creates the trash folder this would empty. Until that -ships there is nothing to empty: Delete writes a tag and moves no file, so no -account has a populated trash folder except through another client. - -**Deliberately excluded from 103's spec**, at the user's request and recorded in -its "Out of scope" section. Worth keeping separate for a reason beyond scope -control: emptying the trash is the first action in this application that would -destroy mail with no undo. Every mutation so far is a tag or, after 103, a move, -and both are reversible. A purge is not. - -**Approach, unspecified.** The shape depends on decisions not yet made, and the -spec for 103 answers none of them: - -- **Local or remote.** Deleting the files locally and letting `Expunge Both` - carry it to the server is one thing; asking the provider to empty its own - trash is another, and mbsync offers no verb for the latter. The first is - probably what "Empty Trash" should mean here. -- **Whether the no-confirmation rule survives it.** It does not, on the face of - it. `CLAUDE.md` grants undo in place of confirmation dialogs, and this is the - action where undo cannot exist. That makes it the second item, after 103, that - re-examines the rule rather than assuming it, and unlike 103 it will probably - have to break it. -- **Per-account or all-accounts**, which should follow whatever the Trash filter - does once 103 ships rather than being decided independently. - -**Size: S**, provisionally, and not worth sizing properly until 103 exists. - -## 119. The unsynced-changes count cannot be opened to see what it counts - -**Observed (user, from the notes):** "the bottom left statusbar message needs to -be clickable and show what 'N unsynced changes' are in a modal window". - -**Cause (verified in the code).** `m_pendingLabel` is a plain `QLabel` added to -the status bar with `addPermanentWidget` (`src/mainwindow.cpp:502-505`). A -`QLabel` has no clicked signal and none is installed, so there is nothing to -click and no route to a list. It carries a tooltip and nothing else. - -**The count is a SUM OVER FOUR SOURCES, and that is what makes this bigger than -it looks.** `pendingEditCount()` returns -`m_pendingTagEdits.size() + m_unnettablePendingEdits + held + heldMoves`. -Three of those can name what they hold: `m_pendingTagEdits` is a -`QHash<QString, bool>` keyed by message id, `m_heldEdits` and `m_heldMoves` are -queues of edits waiting for a sync to end. **`m_unnettablePendingEdits` is a -bare `int`** (`src/mainwindow.h:1248`), deliberately so: it counts confirmed -changes that carry no message ids and therefore cannot be netted against -anything. - -So a dialog built from what is currently kept would list three of the four -groups and then have to account for a remainder it cannot describe. Showing "and -3 more" is worse than the tooltip, because the user opened the window -specifically to find out what those were. - -**Approach.** Two halves, and the second is the real work. - -- The clickable half is small: a label that emits on click (an event filter, or - a flat `QToolButton` styled as a label), plus a dialog listing what the three - describable groups hold. The message pane already resolves an id to a subject. -- The complete half needs `m_unnettablePendingEdits` to become something that - can name its entries. Its comment says why it is an int: understating the - indicator is the direction that costs the user work, so it counts what it - cannot identify rather than dropping it. Making it describable means finding - out what those changes actually are and whether they can carry an id. - -**Constraints.** - -- **The count is deliberately conservative and must stay so.** Item 28 and item - 54 both landed on this indicator being wrong in the direction that made the - user think their work was safe. A dialog that lists fewer changes than the - count claims is the same failure in a new place: reconcile the two, or state - the remainder honestly rather than hiding it. -- **An external `notmuch` run can clear pending changes without this count - noticing**, which the tooltip already admits. A dialog makes that staleness - much more visible, since a listed change may no longer exist. Worth deciding - whether the dialog re-verifies against the database before showing. -- Read-only. This is an information window, not a place to retry or discard a - change; either would be a new mutation path with its own undo question. - -**Size: S** for the clickable half over the three describable groups. **Unknown** -for the fourth, and the item is not complete without it. - - ## 121. The thread list shows nothing while a query is running **Observed (user, from the notes):** "can we show a spinner in the left panel @@ -1341,138 +1090,266 @@ The 70-second duration recorded above fits a `QTRY_*` waiting for a file that is never going to appear, which is consistent with a wrong destination rather than a slow one. -## 162. Delete fails while a sync is renaming the file underneath it - -**Observed (user, 2026-08-25):** deleting a draft reported `Cannot move -<file> to <account>/Trash`. - -**Cause (verified against the live Maildir, not read):** a stale path, and -neither Delete nor item 158 is at fault. - -1. The composer autosaves a draft as `<name>:2,D` and item 158 indexes it - under exactly that filename. -2. **mbsync uploads it and RENAMES it** to `<name>,U=<uid>:2,D`, recording the - server UID in the filename. -3. notmuch still holds the pre-`U=` name until that sync's `notmuch new` runs. -4. `moveMessages()` reads the filename from notmuch and calls - `QFile::rename()` on a path that no longer exists. It fails, the error is - emitted, and the message is skipped. - -Measured, in this order: `notmuch search --output=files` named a file that was -not on disk while `Background sync running...` was up, and the same query was -clean once the sync finished, with the file present under its new `,U=4` name. -That is why it reads as intermittent, and why it heals itself. - -**It is truthful and it loses nothing.** The move is skipped, no wrong folder -is created, no file is destroyed, and the next sync reconciles. The defect is -that the message blames a folder for a timing problem, and that the action -silently does nothing when the user asked for something. - -**This is `CLAUDE.md`'s `,U=` trap from the other side.** `MaildirName::fresh()` -exists because CARRYING that infix across a folder boundary produced -`Maildir error: duplicate UID` on real mail. Here mbsync is ADDING it and the -index lags; the same infix, the opposite direction. - -**Approach, and it needs a decision.** Two candidates: - -- **Refuse the move while a sync holds the lock.** `SyncMonitor` already - reports this, and the held-edit machinery from items 97 and 106 already - exists for exactly this shape: a tag edit made during a sync is held and - flushed when it ends. Delete would join it rather than inventing anything. - This is the likelier right answer, since it matches what every other - mutation already does. -- **Re-resolve the filename** from notmuch immediately before the rename and - re-query the message if the path is gone. Smaller, but it races the same - window it is trying to close, and a second lookup can be stale by the time - it is used. +## 164. A draft this application saved keeps `inbox` + +**Observed (developer, 2026-08-25):** `notmuch search --output=tags` on a +draft this application had just written reported `draft inbox unread`. + +**The first cause recorded here was WRONG, and the correction is the useful +part.** It said `strip_inbox_from_sent()` reads a sent-only folder list and +that `qtmaildirconf.py` has no drafts equivalent. Neither is true: + +- `NOT_ARRIVALS` is `("sent", "drafts")`, so `sent_folders()` already returns + both. The name says "sent" and the contents do not, which is what made the + wrong reading plausible. +- Run against the real config it returns every account's drafts folder. +- `notmuch count "(<carve-out query>) and id:<the draft>"` returns **1**. The + query the hook builds MATCHES the affected message. + +So the folder list and the query are correct, and the fix is not there. + +**What is actually established.** + +- The carve-out is scoped to `SCOPE = "tag:new"` (`post-new:106`). +- The affected draft carries `inbox`, and `notmuch count tag:new` is **0**. +- The installed hooks are SYMLINKS into this repository, so the code read is + the code that runs. Verified rather than assumed. +- An mbsync-style rename does **not** re-apply `new.tags`: measured in a + throwaway database, a file renamed to add `,U=4` and reindexed kept the tags + it had. The "the rename retags it" theory is therefore also out. + +**What is NOT established, and must be before any code is written:** which +pass put `inbox` on this file, and why it was not carrying `tag:new` when the +hook's carve-out ran. The likely shape is an ordering one, since item 158 +indexes a draft from the application itself, outside `notmuch new`, and a file +already known to the database is not a new file on the next pass. But that is +a hypothesis and the last two hypotheses here were both wrong. + +**The reproducer was built (2026-08-25) and it settles the mechanism.** Seven +variants were driven in throwaway databases, modelling `indexDraftFile()` with +a real `notmuch_database_index_file` call rather than the CLI, because no CLI +command indexes an untracked path without applying `new.tags`. + +What the sweep established, each measured rather than reasoned: + +- `index_file` applies **no tags at all**. A draft the application indexes is + therefore never in `tag:new` scope, and the hook has nothing to carve out. +- Whenever the file IS in `tag:new` scope, the carve-out strips `inbox` + correctly, in every filename shape tried: `:2,DS`, `:2,D`, no info suffix, + in `cur/` and in `new/`, with and without the `,U=4` infix. The real file's + shape (`,U=4:2,D`) is among them. +- It survives the orderings too: `notmuch new` first then the app's index, + the app's index first then the rename, an autosave landing between + `notmuch new` and the hook, and the stale-path `remove_message` that makes + the renamed file arrive as new mail. All six left the draft clean. +- The `D` flag is what puts `draft` on the message (`synchronize_flags`), and + the `S` flag is what removes `unread`. The affected file is `:2,D`, which is + why it carries `unread`, and that matches the reported tag set exactly. + +**The one variant that reproduces it** is the general shape rather than a +filename detail: a pass where `inbox` is applied while `tag:new` has ALREADY +been consumed. Modelled as a file indexed at a path the carve-out does not +cover and moved into the drafts folder afterwards, it ends in precisely the +live end state, `draft inbox unread` in Drafts with `,U=4` and `tag:new` at 0. +Nothing revisits a message once the marker is gone, so the tag is permanent. + +**What is still NOT established, and the next step.** The affected account +writes drafts straight to `<account>/Drafts`, which the carve-out +covers (verified against the live config and the live query, which matches the +message by id today), so the reproducing variant's premise does not hold for +it as written. The live log for the pass that added it reads + + 10:10:52 Added 1 new message to the database. Detected 9 file renames. + 10:10:52 post-new: sent-folder carve-out applied over 9 folder(s) + +so the hook DID run on that pass, over a path the query covers, and logged +success. The remaining candidates are all about what the path or the marker +looked like at that instant, not about the query text: the carve-out logs +"applied" on a `notmuch tag` that matched zero messages, so a successful log +line is not evidence the message was in scope. Instrumenting the hook to log +the carve-out's MATCH COUNT, and leaving it to run until the next draft, is +the cheapest way to close it, and is a log-only change to code that tags real +mail unattended. + +The filename also rules one thing in: `1787645266.M802P16149Q3.<host>` is +exactly `MaildirName::fresh()` output, so the application wrote this file. It +is not a draft another client left behind. + +The reproducer scripts are throwaway and were not kept; `indexfile.c` is +fifteen lines around one `notmuch_database_index_file` call and is trivial to +rebuild from this entry if the instrumentation points back at the hook. **Constraints.** -- **Delete reaches the real mail server.** Read `CLAUDE.md`'s item 103 notes - before touching `moveMessages()`: a wrong folder name is created, adopted by - mbsync, and propagated to every other client. -- Whatever is built, **the message must say a sync is running**, not name a - folder. The current wording sent the user looking for a broken folder - configuration, which was correct and configured. -- A test cannot see this in the ordinary fixture layout, where nothing renames - a file underneath the index. Driving it means renaming the file between the - index write and the move, which is what the reproducer has to do. - -## 163. The message pane shows a stale path and reports the message unreadable - -**Observed (user, 2026-08-25):** selecting a draft filled the pane with -`(unreadable message)` and `This message could not be parsed.`, naming a file -under the account's drafts folder. - -**Cause (verified against the live Maildir):** the path in the pane ended -`.dnx:2,D`, and the only file on disk ended `.dnx,U=4:2,D`. Same mechanism as -item 162: mbsync renames an uploaded file to record its server UID, and the -name the application is holding stops existing. - -**The site is different, and so is the fix.** Item 162 is the WRITE path, -`moveMessages()` reading a filename from notmuch. This is the READ path, and -by the time it fires notmuch is already CORRECT: measured, the index named the -`,U=4` file while the pane still named the pre-`U=` one. The stale path is the -MODEL's, cached when the row was loaded, so refusing to act while a sync runs -(162's likely fix) would not help here at all. - -**The report is honest, which is why it is confusing.** `MimeParser` opened a -path that did not exist and said so. Nothing is lost and the next query -repairs it. - -**Approach.** The read path should RECOVER rather than refuse: on a failed -parse, re-resolve the message id through notmuch and retry once before -reporting. `recoverStaleThread()` already exists for the neighbouring problem -(item 91 reuses it) and is the shape to follow. +- **The hook tags real mail unattended every ten minutes.** Nothing here is + worth a speculative change. +- The 0.27.0 changelog claims sent mail and drafts both stay out of the inbox. + Whatever the cause, that claim is currently false for drafts and the entry + needs correcting with the fix. +- Only `inbox` may be touched. A draft legitimately carries `draft` and + `unread`, and `maildir.synchronize_flags` means removing `unread` rewrites + the filename and reaches the server. +- The hook must keep refusing to consume `tag:new` when a carve-out fails. +- `test_post_new.py` and `test_qtmaildirconf.py` both live beside the hook and + have sent-carve-out tests to copy. + +## 165. A draft gets a new Message-ID on every autosave + +**Observed (developer, 2026-08-25), while hand-testing items 163 and 164.** +Four saved drafts produced four distinct Message-IDs, and one reopen-and-edit +turned one id into another. A draft therefore has no stable identity across +its own revisions. + +**Cause (verified in the code, not inferred).** `MessageBuilder::build()` +calls `g_mime_utils_generate_message_id()` unconditionally on every call +(`messagebuilder.cpp:311`), and every autosave calls `build()`. +`OutgoingMessage` has no field to carry an existing id in, and +`ComposeContext` has no field for the draft's OWN id either: it carries +`inReplyTo` and `references`, which are the ORIGINAL's id when replying, and +`ComposeContextBuilder::forDraft()` never reads the draft's Message-ID back +out of the file it parses. So this is not a changed call site; it needs a +field that does not exist yet, threaded from `forDraft()` through +`ComposeContext` and `OutgoingMessage` into `build()`. + +**Why it matters, and why it is NOT urgent.** To notmuch and to the server, +each revision is a different MESSAGE, not a new version of one. While the +file is replaced correctly this is invisible: one file in, one file out. It +becomes visible whenever a revision is NOT replaced, and item 163 is the +proof, where a stale path forked a draft into two files that were also two +messages and that nothing will ever collapse. Item 163's fix removes the +known way to reach that state; this entry is about the property that turned a +one-file mistake into a two-message one. + +An interrupted save is the remaining route: `DraftStore::write()` unlinks the +previous revision only AFTER the new file is safely in place (deliberately, +so a failed write cannot lose the draft), so a crash between the two leaves +two files, and with two ids they are two drafts rather than one duplicated. + +**Approach, and it needs a decision rather than an implementation.** The +question is what a draft's identity IS, and it is not obviously "the id it +will be sent under": + +- A stable id reused at send time makes the draft and the sent message one + message, which is what a user means by "my draft became this email". It + also means the id was in a file mbsync uploaded to the drafts folder before + the message was ever sent, and the server has seen it. +- A stable id DISCARDED at send time keeps revisions collapsed while drafting + and mints a fresh id for the sent copy. Two identities, and the sent one is + the one that threads. +- The current behaviour is a third position, and its only virtue is that no + id is ever reused for two different things. + +Whichever is chosen must be checked against `In-Reply-To`/`References` on the +eventual send, since `referencesForReply()` builds those from the ORIGINAL's +id and a draft of a reply carries both. **Constraints.** -- **A retry must be bounded.** A message that genuinely cannot be parsed - (item 41's territory) must still report, or a real defect becomes an - infinite loop. -- Re-resolving by id is what makes this safe; re-scanning the folder is not, - since two files can carry the same id. -- The placeholder wording is correct and should stay for the genuine case. - -## 164. Every newly synced draft carries `inbox` - -**Observed (developer, 2026-08-25):** `notmuch search --output=tags` on a draft -this application had just written reported `draft inbox unread`. - -**Cause (verified in the hook):** `strip_inbox_from_sent()` in -`assets/hooks/post-new` builds its query from -`qtmaildirconf.sent_folders()`, and `assets/hooks/qtmaildirconf.py` exposes -`sent_folders()` and `sent_query()` and **no drafts equivalent**. The -carve-out therefore never covers a drafts folder, and every draft that -completes a sync round trip is tagged `inbox` by `notmuch new` like any other -newly indexed file. - -**Item 158's measurement was right and did not cover this.** That item -measured `notmuch_database_index_file` assigning NO tags, which is true and is -why a freshly autosaved draft is clean. The tagging happens later: mbsync -uploads the file, renames it, and the next `notmuch new` indexes it as new -mail. +- **A Message-ID reaches the server and every recipient**, so a reused id is + not a local matter. Two different messages sharing an id is worse than two + ids for one draft, which is what makes the current behaviour defensible as + a default rather than simply wrong. +- `MessageBuilder::build()` is on the SEND path as well as the autosave path. + A change that makes ids stable must not make two different sent messages + share one. +- The comment at `messagebuilder.cpp:297` records that GMime generates + neither Date nor Message-ID unless asked, and that a message without one + cannot be threaded by anything receiving it, this application's own index + of the sent copy included. Any "just omit it while drafting" variant has to + answer that. +- Item 163's fix stands on its own and this does not block it: the file is + replaced correctly now, so the fork this would have mitigated no longer + happens by that route. + +## 169. A card shows the account only as a bar, with no fade and no avatar + +**Observed (user, from the notes):** "the left border of a card expresses the +account the mail belongs to. the background color of the card should fade left +to right from the account color to the current background color we are using (or +to transparent to work both in light and dark themes). On the left we should +leave room for an account avatar (a squircle), for now it could be extracted +from the sender name "From: john doe" becomes "JD" in the avatar. As soon as we +include khard (or some other vcard provider/manager) we will switch to images if +the corresponding vCard has one." + +**Cause (verified in the code):** not a defect. Half of it shipped. The account +colour is drawn as a solid bar down the left edge, `CardLayout::accentRect` +placed by `CardLayout`, filled by `CardDelegate::paint()` with +`CardDelegate::accentLineColour()`. There is no gradient anywhere on a card, and +nothing draws an avatar: `CardLayout` reserves no rect for one, so the geometry +would have to grow before the painting could. + +**Approach.** Two separable pieces, and the avatar is the one that changes the +layout. + +- The fade is a `QLinearGradient` fill over the card rect, from the accent + colour to the pane's background. `accentLineColour()` already records why + blending toward the background is wrong for a CHIP; a card's background is + exactly where such a blend belongs, so the constraint does not carry over. + Both themes come free if the far stop is the palette's own base rather than + a literal. +- The avatar needs a rect in `CardLayout`, which is where it becomes testable + without a painter, and it shifts `contentLeft` for every card. The initials + come from the display name already carried on the summary; a sender with no + display name (an address only) needs an answer before this is built. -**This contradicts shipped documentation.** The 0.27.0 changelog says "Sent -mail and drafts no longer appear in the inbox", and the drafts half has never -been true. Fixing the hook and correcting the entry are one item. +**Constraints.** -**Approach.** A `drafts_folders()` / `drafts_query()` pair beside the sent -ones, and one more carve-out call, or a single function taking the folder kind -so the two cannot drift. +- The vCard half is blocked on item 72, which is itself unspecified. Build the + initials only; do not design the image path in advance. +- A gradient behind the text has to keep the text readable at the left edge in + both themes, which is the same failure mode `accentLineColour()` guards + against on a dark palette. +- This is a looks question, so it is settled by the user looking at it rather + than by a test: assert the geometry in `CardLayout`, and hand the appearance + over per `tests-only-for-measurable-things`. + +## 170. A row that stops matching the view only leaves it on the Delete path + +**Observed (user, from the notes):** "should we refactor the list UI to be +responsive so changes are applied immediately instead of waiting for a view +change to repaint?" + +**Cause (verified in the code, 2026-08-26).** Two different properties were +being called "responsive", and only one of them was built. + +The optimistic **repaint** is universal. `ThreadListModel::applyTagChange()` +covers a thread-scoped write, `applyMessageTagChange()` a message-scoped one +(items 105 to 111), and `revertPendingTagChange()` undoes either if the write +is rejected. A chip, a bold row and a dimmed row all move the moment the user +acts. + +The optimistic **membership** is not. `ThreadListModel::removeThreadsWithoutTag()` +has exactly ONE caller, in `trashMessages()`, added last session because Delete +strips `inbox` and a deleted message sat in the Inbox view across restarts. The +ordinary tag path never calls it: neither `sendMessageTagChange()` nor +`sendThreadTagChange()` asks whether the row still belongs in the view. + +So in the Unread view, marking a message read repaints the row and leaves it in +a list defined by `tag:unread`, which it no longer matches. Un-flagging in the +Flagged view is the same, and so is removing `inbox` by hand from the Inbox +view. It corrects itself at the next query or sync, which is exactly the "waits +for a view change" the note describes. + +**Approach.** Not a refactor. `viewFilterTag()` already resolves the view's own +tag from the query, and `removeThreadsWithoutTag()` already does the removal. +The gap is that the guard sits in `trashMessages()` rather than at the funnel +every tag write passes. Move it, or call it from both send paths. **Constraints.** -- **This is a two-repo change in spirit but not in fact.** The hooks moved - into this repository in 0.27.0, so `mailrules.py` is not involved and the - shared-format procedure does not apply. Check that before assuming - otherwise: `CLAUDE.md` still describes the hook as shipping from `mailctl` - in places. -- The hook must keep refusing to consume `tag:new` when a carve-out fails. - Clearing the marker while the rules did not run orphans that mail - permanently, which the sent half already gets right. -- Only `inbox` may be touched. A draft legitimately carries `draft` and - `unread`, and `maildir.synchronize_flags` means removing `unread` rewrites - the filename and reaches the server. -- `test_post_new.py` and `test_qtmaildirconf.py` are in the same directory and - must both be extended; the sent carve-out has tests to copy. +- The guard's existing reasoning is what makes this safe and must be kept: only + a plain `tag:<x>` view has a membership one tag decides. A path query (Trash, + Sent, Drafts) is unaffected by a tag going away, and a hand-typed query cannot + be reasoned about. Both are left alone. Without that, marking read in an `id:` + view would empty the list. +- A row leaving is not revertible by `revertPendingTagChange()`, which repaints + rather than reinserts. A REJECTED write would leave the row gone until the + next query. The move path already carries that exposure; check whether it is + acceptable at the tag path's much higher frequency, or make the removal wait + for confirmation there. +- The inverse case is deliberately out of scope: a row that starts matching + cannot be inserted optimistically, since the model has no summary for a + thread the query never returned. +- Undo goes back through the same funnel, so a removal must not make an undone + mark-read invisible in the view it was undone in. diff --git a/docs/superpowers/plans/2026-08-26-card-avatars.md b/docs/superpowers/plans/2026-08-26-card-avatars.md new file mode 100644 index 0000000..03ea78a --- /dev/null +++ b/docs/superpowers/plans/2026-08-26-card-avatars.md @@ -0,0 +1,1981 @@ +# Card Avatars and the Account Fade Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Give every card a sender avatar in its own gutter and fade the account colour across the card's left 60%, backlog item 169. + +**Architecture:** Three new namespaces of free functions over values (`Avatar` for initials and fill choice, `BusinessSenders` for the sender list), so all of it is testable without a painter or a widget, exactly as `CardLayout`, `SearchTerm` and `MarkdownFormat` already are. `CardLayout` gains one rect and shifts `contentLeft`; `CardDelegate` paints the fade and the squircle. `ThreadSummary` gains `firstMessageSender`, filled by the worker walk that already fills `firstMessageId`. + +**Tech Stack:** Qt 6.11 (`QCryptographicHash` from Qt Core, `QLinearGradient`/`QPainterPath` from Qt Gui), libnotmuch, C++17. Build with CMake + Ninja. + +**Spec:** `docs/superpowers/specs/2026-08-26-card-avatars-design.md`. Read it before starting; this plan implements it and does not restate its reasoning. + +--- + +## Before you start + +**Never run a test binary without `QT_QPA_PLATFORM=offscreen`, and never launch `./build/src/qtmaildir`.** Running the application is the user's hand test. See `CLAUDE.md`; a direct run of `test_mainwindow` throws over a hundred windows onto the user's screen and they have asked for it to stop. + +Build and test commands used throughout: + +```bash +cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Debug +cmake --build build +ctest --test-dir build -R <name> --output-on-failure +``` + +Commits are GPG-signed (`git commit -S`). Work directly on `master`. + +## File Structure + +**Created:** + +- `src/avatar.h` / `src/avatar.cpp` — namespace `Avatar`. Initials, fill choice, and the two generated fills. No painting of its own beyond returning a `QPixmap`; no widget, no model. +- `src/businesssenders.h` / `src/businesssenders.cpp` — namespace `BusinessSenders`. Parsing, matching and appending candidates for `~/.config/qtmaildir/business-senders`. +- `tests/test_avatar.cpp` +- `tests/test_businesssenders.cpp` + +**Modified:** + +- `src/types.h` — one field on `ThreadSummary`. +- `src/notmuchworker.cpp` — fill that field in the existing walk; collect senders after a sync. +- `src/cardlayout.h` / `src/cardlayout.cpp` — `avatarRect`, gutter constants, `contentLeft`. +- `src/threadlistmodel.h` / `src/threadlistmodel.cpp` — two roles carrying the sender and the account address. +- `src/carddelegate.h` / `src/carddelegate.cpp` — the fade and the squircle. +- `src/mainwindow.cpp` — load the list at startup, append candidates at sync end. +- `src/CMakeLists.txt`, `tests/CMakeLists.txt` — register the new files. +- `CHANGELOG.md` — final task. + +**Why two namespaces and not one:** `Avatar` is pure presentation of a value, `BusinessSenders` is file I/O. They change for different reasons and only one of them touches the disk. + +--- + +### Task 1: `ThreadSummary::firstMessageSender` + +The card has no address to hash today. `authors` is notmuch's summarised string and carries display names only, measured on the real index as `'Ryanair'`, `'The Hacker News tramite LinkedIn'`, with no `@` anywhere. + +**Files:** +- Modify: `src/types.h` +- Modify: `src/notmuchworker.cpp` +- Test: `tests/test_notmuchworker.cpp` + +- [ ] **Step 1: Write the failing test** + +Add to `tests/test_notmuchworker.cpp`, and declare it in the class's `private slots:` block: + +```cpp +void TestNotmuchWorker::queryCarriesTheFirstMessageSender() +{ + NotmuchFixture fixture; + fixture.addMessage("sender-probe@example.org", QStringLiteral("Probe subject")); + fixture.index(); + + NotmuchWorker worker(fixture.configPath()); + QVERIFY(worker.open()); + + QSignalSpy spy(&worker, &NotmuchWorker::threadsReady); + worker.runQuery(QStringLiteral("subject:\"Probe subject\""), 1, + NotmuchWorker::NewestFirst, false); + QVERIFY(spy.count() > 0); + + const auto threads = spy.first().at(0).value<QVector<ThreadSummary>>(); + QCOMPARE(threads.size(), 1); + // The bare address, not the display name and not notmuch's authors string. + QCOMPARE(threads.first().firstMessageSender, + QStringLiteral("sender-probe@example.org")); +} +``` + +Check the fixture's actual helper names first with `grep -n 'void addMessage\|QString configPath\|void index' tests/test_notmuchworker.cpp` and adapt the three calls above to match; the assertion is the part that matters. + +- [ ] **Step 2: Run test to verify it fails** + +```bash +cmake --build build && ctest --test-dir build -R notmuchworker --output-on-failure +``` + +Expected: FAIL, `firstMessageSender` is not a member of `ThreadSummary` (compile error). + +- [ ] **Step 3: Add the field** + +In `src/types.h`, immediately after `firstMessageTags`: + +```cpp + /// That message's sender, as a BARE ADDRESS with no display name. + /// + /// `authors` above is notmuch's own summarised string and carries display + /// names ONLY: measured against the real index, 'Ryanair' and 'The Hacker + /// News tramite LinkedIn', with no `@` anywhere. A card therefore has no + /// address to hash for its avatar and nothing for the business-sender list + /// to match, which is why this exists (item 169). + /// + /// Hashing the display name instead was rejected: notmuch BUILDS those + /// strings, so one sender's identity varies as the string does. + /// + /// Free, for the same reason `firstMessageId` and `firstMessageTags` are: + /// the walk that finds that message is already happening and From is + /// served from the INDEX, not the message file. Measured 2026-08-26 on the + /// developer's database: 1322 distinct senders in 12 ms, 5105 messages + /// enumerated in 76 ms. Do not move it behind a flag by analogy with + /// `recipients`. + QString firstMessageSender; +``` + +- [ ] **Step 4: Fill it in the worker walk** + +In `src/notmuchworker.cpp`, find where `summary.firstMessageId` and `summary.firstMessageTags` are assigned from the resolved message (search for `firstMessageTags =`). Both branches, the `withRecipients` one and the ordinary one, resolve a message; assign beside them in each: + +```cpp + summary.firstMessageSender = senderAddressOf(message); +``` + +Add this helper in the anonymous namespace near `recipientsOf`: + +```cpp +/// The bare address of a message's From, with any display name discarded. +/// +/// Index-served, unlike recipientsOf() above, which is why this is not behind +/// the withRecipients flag: `From` is in notmuch's index and `To` is not. +/// +/// The header is untrusted, so it is parsed rather than split: a display name +/// may legally contain an `@`, and "Ian <a@b>" split on `@` yields nonsense. +QString senderAddressOf(notmuch_message_t *message) +{ + const char *from = notmuch_message_get_header(message, "From"); + if (!from || !*from) + return QString(); + + InternetAddressList *list = internet_address_list_parse(nullptr, from); + if (!list) + return QString(); + + QString address; + const int count = internet_address_list_length(list); + for (int i = 0; i < count; ++i) { + InternetAddress *entry = internet_address_list_get_address(list, i); + if (!entry || !INTERNET_ADDRESS_IS_MAILBOX(entry)) + continue; + const char *addr = + internet_address_mailbox_get_addr(INTERNET_ADDRESS_MAILBOX(entry)); + if (addr && *addr) { + address = QString::fromUtf8(addr); + break; + } + } + g_object_unref(list); + return address; +} +``` + +`notmuchworker.cpp` already includes gmime for `recipientSummary`'s neighbours; if it does not, add `#include <gmime/gmime.h>` **before every Qt header** in that file. glib declares a field named `signals`, which Qt defines as a macro, so the order is not stylistic. + +- [ ] **Step 5: Run test to verify it passes** + +```bash +cmake --build build && ctest --test-dir build -R notmuchworker --output-on-failure +``` + +Expected: PASS. + +- [ ] **Step 6: Commit** + +```bash +git add src/types.h src/notmuchworker.cpp tests/test_notmuchworker.cpp +git commit -S -m "feat: carry the first message's sender address on a thread summary" +``` + +--- + +### Task 2: `Avatar::initialsFor()` + +**Files:** +- Create: `src/avatar.h`, `src/avatar.cpp` +- Create: `tests/test_avatar.cpp` +- Modify: `src/CMakeLists.txt`, `tests/CMakeLists.txt` + +- [ ] **Step 1: Write the failing test** + +Create `tests/test_avatar.cpp`. Use the GPLv2 header from any existing test file verbatim, then: + +```cpp +#include <QTest> + +#include "avatar.h" + +class TestAvatar : public QObject +{ + Q_OBJECT + +private slots: + void twoWordNameTakesOneLetterFromEach(); + void oneWordNameTakesItsFirstTwoLetters(); + void bareAddressTakesLocalAndDomain(); + void nothingUsableFallsBackToTheAccountLabel(); + void initialsAreAlwaysTwoLetters(); +}; + +void TestAvatar::twoWordNameTakesOneLetterFromEach() +{ + QCOMPARE(Avatar::initialsFor(QStringLiteral("John Doe"), + QStringLiteral("john@example.org"), + QStringLiteral("Work")), + QStringLiteral("JD")); + // Three words still take the FIRST two, not the first and last. + QCOMPARE(Avatar::initialsFor(QStringLiteral("Maria Grazia Rossi"), + QStringLiteral("maria@example.org"), + QStringLiteral("Work")), + QStringLiteral("MG")); +} + +void TestAvatar::oneWordNameTakesItsFirstTwoLetters() +{ + QCOMPARE(Avatar::initialsFor(QStringLiteral("Cofidis"), + QStringLiteral("noreply@cofidis.it"), + QStringLiteral("Work")), + QStringLiteral("CO")); +} + +void TestAvatar::bareAddressTakesLocalAndDomain() +{ + QCOMPARE(Avatar::initialsFor(QString(), + QStringLiteral("noreply@cofidis.it"), + QStringLiteral("Work")), + QStringLiteral("NC")); +} + +void TestAvatar::nothingUsableFallsBackToTheAccountLabel() +{ + // No name and no address at all: the account's label is the last resort, + // so a card always carries a squircle rather than a hole. + QCOMPARE(Avatar::initialsFor(QString(), QString(), + QStringLiteral("Work")), + QStringLiteral("WO")); + // And with nothing whatsoever, still two characters rather than empty. + QCOMPARE(Avatar::initialsFor(QString(), QString(), QString()).size(), 2); +} + +void TestAvatar::initialsAreAlwaysTwoLetters() +{ + // The shape is the point: every squircle reads the same. An address with + // no domain, a one-letter local part and a name of one letter all still + // produce two characters. + const QStringList names { QString(), QStringLiteral("X"), + QStringLiteral("A B") }; + const QStringList addresses { QStringLiteral("a@b.org"), + QStringLiteral("malformed"), + QString() }; + for (const QString &name : names) { + for (const QString &address : addresses) { + const QString initials = + Avatar::initialsFor(name, address, QStringLiteral("Acct")); + QCOMPARE(initials.size(), 2); + } + } +} + +QTEST_MAIN(TestAvatar) +#include "test_avatar.moc" +``` + +- [ ] **Step 2: Register the files and run the test to verify it fails** + +Add `avatar.cpp` to the `qtmaildir_lib` list in `src/CMakeLists.txt` (alphabetically, before `busyindicator.cpp`), and `add_qtmaildir_test(avatar)` to `tests/CMakeLists.txt` beside the others. + +```bash +cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Debug && cmake --build build +``` + +Expected: FAIL, `avatar.h` not found. + +- [ ] **Step 3: Write the header** + +Create `src/avatar.h` with the GPLv2 header, then: + +```cpp +#pragma once + +#include <QColor> +#include <QPixmap> +#include <QString> + +/// A card's sender avatar: which letters it carries and what fills it. +/// +/// A NAMESPACE of free functions over values, deliberately, for the reason +/// CardLayout is a struct with no painter: the letters and the fill choice are +/// decisions with right answers, and they must be assertable without a widget, +/// a model or an exposed view. Only pixmapFor() touches a QPainter, and it +/// paints into an image it owns rather than onto a widget. +namespace Avatar +{ + +/// Which of the two generated fills a sender gets. +enum class Fill +{ + /// A 5x5 symmetric grid from the hash bits, under a darkening veil. + Identicon, + /// Two related hues from the hash, split at an angle, initials on a large + /// flat field. + TwoTone, +}; + +/// Always exactly two characters, upper-cased. +/// +/// In order: a display name of two or more words gives one letter from each of +/// the first two; a one-word name gives its own first two; a bare address +/// gives the first of the local part and the first of the domain; and with +/// nothing usable, the account's label. The uniform length is the point, so +/// every squircle reads as the same shape. +QString initialsFor(const QString &displayName, const QString &address, + const QString &accountLabel); + +/// Which fill, given whether the list claims this address as a business one. +/// +/// The list wins first, then the presence of a display name. That order is +/// what lets `Ian Farrell <notifications@github.com>` read as a person while +/// a listed address stays a business whatever name it presents. +Fill fillFor(const QString &displayName, bool isBusinessSender); + +/// A stable colour for an address. Same input, same colour, always. +/// +/// Generated at a FIXED saturation and lightness so the initials keep their +/// contrast in both themes, exactly as TagColors::colourFor() does for a tag +/// with nothing configured. +QColor colourFor(const QString &address); + +/// The finished squircle, `side` pixels a side, ready to draw. +/// +/// `seed` is what the fill is generated from, normally the sender's address +/// and the account's own address when there is no sender. +QPixmap pixmapFor(const QString &seed, const QString &initials, Fill fill, + int side, const QFont &font); + +} // namespace Avatar +``` + +- [ ] **Step 4: Implement `initialsFor` only** + +Create `src/avatar.cpp` with the GPLv2 header, then: + +```cpp +#include "avatar.h" + +#include <QCryptographicHash> +#include <QPainter> +#include <QPainterPath> + +namespace { + +QString twoFrom(const QString &text) +{ + const QString trimmed = text.trimmed(); + if (trimmed.size() >= 2) + return trimmed.left(2).toUpper(); + if (trimmed.size() == 1) + return (trimmed + trimmed).toUpper(); + return QString(); +} + +} // namespace + +namespace Avatar { + +QString initialsFor(const QString &displayName, const QString &address, + const QString &accountLabel) +{ + const QStringList words = displayName.split(QLatin1Char(' '), + Qt::SkipEmptyParts); + if (words.size() >= 2) { + return (words.at(0).left(1) + words.at(1).left(1)).toUpper(); + } + if (words.size() == 1) { + const QString one = twoFrom(words.at(0)); + if (!one.isEmpty()) + return one; + } + + // No usable name. The local part and the domain each give one letter, + // which never degrades to a single letter the way the local part alone + // would, and never reads as a truncated word. + const int at = address.indexOf(QLatin1Char('@')); + if (at > 0) { + const QString local = address.left(at).trimmed(); + const QString domain = address.mid(at + 1).trimmed(); + if (!local.isEmpty() && !domain.isEmpty()) + return (local.left(1) + domain.left(1)).toUpper(); + } + // An address with no `@` is still something to show. + const QString bare = twoFrom(address); + if (!bare.isEmpty()) + return bare; + + const QString account = twoFrom(accountLabel); + if (!account.isEmpty()) + return account; + + // Nothing at all. Two characters regardless, so the shape never breaks. + return QStringLiteral("??"); +} + +} // namespace Avatar +``` + +- [ ] **Step 5: Run test to verify it passes** + +```bash +cmake --build build && QT_QPA_PLATFORM=offscreen ctest --test-dir build -R avatar --output-on-failure +``` + +Expected: PASS, 5 tests. + +- [ ] **Step 6: Commit** + +```bash +git add src/avatar.h src/avatar.cpp src/CMakeLists.txt tests/test_avatar.cpp tests/CMakeLists.txt +git commit -S -m "feat: derive a sender's avatar initials" +``` + +--- + +### Task 3: `Avatar::fillFor()` and `Avatar::colourFor()` + +**Files:** +- Modify: `src/avatar.cpp` +- Test: `tests/test_avatar.cpp` + +- [ ] **Step 1: Write the failing test** + +Add to `tests/test_avatar.cpp`, declaring each in `private slots:`: + +```cpp +void TestAvatar::aDisplayNameMeansAPerson() +{ + // The case the user asked for by name: a corporate address that presents + // itself as a person reads as a person. + QCOMPARE(Avatar::fillFor(QStringLiteral("Ian Farrell"), false), + Avatar::Fill::Identicon); + QCOMPARE(Avatar::fillFor(QString(), false), Avatar::Fill::TwoTone); +} + +void TestAvatar::theListOverridesADisplayName() +{ + // A listed address stays a business even when it sets a friendly name. + QCOMPARE(Avatar::fillFor(QStringLiteral("Cofidis"), true), + Avatar::Fill::TwoTone); +} + +void TestAvatar::aColourIsStablePerAddress() +{ + const QColor first = Avatar::colourFor(QStringLiteral("a@example.org")); + const QColor again = Avatar::colourFor(QStringLiteral("a@example.org")); + QCOMPARE(first, again); + QVERIFY(first.isValid()); + QVERIFY(Avatar::colourFor(QStringLiteral("b@example.org")) != first); +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +```bash +cmake --build build 2>&1 | tail -5 +``` + +Expected: FAIL, `fillFor` and `colourFor` undefined (link error). + +- [ ] **Step 3: Implement both** + +Append to the `Avatar` namespace in `src/avatar.cpp`: + +```cpp +Fill fillFor(const QString &displayName, bool isBusinessSender) +{ + // The list first: it is the user's explicit override and must beat the + // heuristic, or a listed sender could never be pinned. + if (isBusinessSender) + return Fill::TwoTone; + return displayName.trimmed().isEmpty() ? Fill::TwoTone : Fill::Identicon; +} + +QColor colourFor(const QString &address) +{ + // The same construction TagColors::colourFor() uses for a tag with nothing + // configured: hashed so it is stable, at a fixed saturation and lightness + // so it cannot come out neon and cannot lose its contrast with the + // initials. The lightness differs from that function's deliberately: a + // chip carries dark text, a squircle carries white. + const QByteArray digest = + QCryptographicHash::hash(address.toUtf8(), QCryptographicHash::Md5); + const int hue = static_cast<quint8>(digest.at(0)) * 360 / 256; + return QColor::fromHsl(hue, 110, 95); +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +```bash +cmake --build build && QT_QPA_PLATFORM=offscreen ctest --test-dir build -R avatar --output-on-failure +``` + +Expected: PASS, 8 tests. + +- [ ] **Step 5: Commit** + +```bash +git add src/avatar.cpp tests/test_avatar.cpp +git commit -S -m "feat: choose an avatar fill and derive its colour" +``` + +--- + +### Task 4: `Avatar::pixmapFor()` + +**Files:** +- Modify: `src/avatar.cpp` +- Test: `tests/test_avatar.cpp` + +Note on what is asserted here. Per `CLAUDE.md`, counting lit pixels proves almost nothing and a rendering probe that reports "no ink" is more likely broken than the code. So this asserts **determinism and difference**, which a pixel comparison genuinely can establish, and leaves the appearance to the user's eye. + +- [ ] **Step 1: Write the failing test** + +```cpp +void TestAvatar::aPixmapIsStableAndDiffersPerSeed() +{ + const QFont font; + const QPixmap first = Avatar::pixmapFor(QStringLiteral("a@example.org"), + QStringLiteral("AE"), + Avatar::Fill::Identicon, 44, font); + QCOMPARE(first.size(), QSize(44, 44)); + QVERIFY(!first.isNull()); + + const QPixmap again = Avatar::pixmapFor(QStringLiteral("a@example.org"), + QStringLiteral("AE"), + Avatar::Fill::Identicon, 44, font); + // Same seed, same image, byte for byte: the identity must not drift + // between repaints. + QCOMPARE(first.toImage(), again.toImage()); + + const QPixmap other = Avatar::pixmapFor(QStringLiteral("b@example.org"), + QStringLiteral("AE"), + Avatar::Fill::Identicon, 44, font); + // Different sender, different image, even with identical initials. + QVERIFY(first.toImage() != other.toImage()); + + const QPixmap twoTone = Avatar::pixmapFor(QStringLiteral("a@example.org"), + QStringLiteral("AE"), + Avatar::Fill::TwoTone, 44, font); + // The two fills are actually different renderings, not one with a flag. + QVERIFY(first.toImage() != twoTone.toImage()); +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +```bash +cmake --build build 2>&1 | tail -5 +``` + +Expected: FAIL, `pixmapFor` undefined. + +- [ ] **Step 3: Implement it** + +Append to the `Avatar` namespace in `src/avatar.cpp`: + +```cpp +QPixmap pixmapFor(const QString &seed, const QString &initials, Fill fill, + int side, const QFont &font) +{ + QPixmap pixmap(side, side); + pixmap.fill(Qt::transparent); + + const QByteArray digest = + QCryptographicHash::hash(seed.toUtf8(), QCryptographicHash::Md5); + const QColor base = colourFor(seed); + + QPainter painter(&pixmap); + painter.setRenderHint(QPainter::Antialiasing, true); + + // The squircle. A rounded rect at ~30% of the side reads as one without + // needing a superellipse, and clipping to it means neither fill has to + // know the shape. + QPainterPath squircle; + squircle.addRoundedRect(QRectF(0, 0, side, side), side * 0.3, side * 0.3); + painter.setClipPath(squircle); + + if (fill == Fill::Identicon) { + // A 5x5 grid, mirrored about the vertical axis, so only the left + // three columns come from the hash: 15 cells, one bit each, which is + // two bytes of the digest. Symmetry is what makes the shape read as a + // deliberate mark rather than as noise. + painter.fillRect(QRect(0, 0, side, side), base.darker(220)); + const qreal cell = qreal(side) / 5.0; + for (int col = 0; col < 3; ++col) { + for (int row = 0; row < 5; ++row) { + const int bit = col * 5 + row; + const bool on = + (static_cast<quint8>(digest.at(bit / 8)) >> (bit % 8)) & 1; + if (!on) + continue; + painter.fillRect(QRectF(col * cell, row * cell, cell, cell), + base); + const int mirrored = 4 - col; + painter.fillRect( + QRectF(mirrored * cell, row * cell, cell, cell), base); + } + } + // The veil. Without it the initials sit on whatever the pattern + // happens to do behind them, which is the classic legibility failure + // this fill invites. Tune the opacity against the real font before + // calling it done. + painter.fillRect(QRect(0, 0, side, side), QColor(0, 0, 0, 77)); + } else { + // Two related hues split at an angle, both from the hash. The field + // behind the letters stays large and flat, which is the whole reason + // this fill exists beside the identicon. + const int angle = static_cast<quint8>(digest.at(1)) * 360 / 256; + QLineF axis = QLineF::fromPolar(side, angle); + axis.translate(side / 2.0, side / 2.0); + QLinearGradient gradient(axis.p2(), axis.p1()); + gradient.setColorAt(0.0, base); + gradient.setColorAt(0.499, base); + gradient.setColorAt(0.5, base.darker(135)); + gradient.setColorAt(1.0, base.darker(135)); + painter.fillRect(QRect(0, 0, side, side), gradient); + } + + // The letters. White with a soft shadow rather than a computed contrast + // colour: the fills are generated at a fixed lightness precisely so one + // choice works for all of them. + QFont letters = font; + letters.setBold(true); + letters.setPixelSize(qMax(8, int(side * 0.36))); + painter.setFont(letters); + painter.setPen(QColor(0, 0, 0, 120)); + painter.drawText(QRect(1, 1, side, side), Qt::AlignCenter, initials); + painter.setPen(Qt::white); + painter.drawText(QRect(0, 0, side, side), Qt::AlignCenter, initials); + + return pixmap; +} +``` + +Add `#include <QLinearGradient>` and `#include <QLineF>` to the top of `src/avatar.cpp`. + +- [ ] **Step 4: Run test to verify it passes** + +```bash +cmake --build build && QT_QPA_PLATFORM=offscreen ctest --test-dir build -R avatar --output-on-failure +``` + +Expected: PASS, 9 tests. + +- [ ] **Step 5: Commit** + +```bash +git add src/avatar.cpp tests/test_avatar.cpp +git commit -S -m "feat: paint the avatar squircle from a hashed seed" +``` + +--- + +### Task 5: `BusinessSenders` parsing and matching + +**Files:** +- Create: `src/businesssenders.h`, `src/businesssenders.cpp` +- Create: `tests/test_businesssenders.cpp` +- Modify: `src/CMakeLists.txt`, `tests/CMakeLists.txt` + +- [ ] **Step 1: Write the failing test** + +Create `tests/test_businesssenders.cpp` with the GPLv2 header, then: + +```cpp +#include <QTemporaryDir> +#include <QTest> + +#include "businesssenders.h" + +class TestBusinessSenders : public QObject +{ + Q_OBJECT + +private slots: + void anExactAddressMatches(); + void aDomainEntryMatchesEveryAddressUnderIt(); + void commentsAndBlankLinesAreIgnored(); + void whitespaceAroundAnEntryIsIgnored(); + void matchingIsCaseInsensitive(); + void anAbsentFileMatchesNothing(); +}; + +void TestBusinessSenders::anExactAddressMatches() +{ + const BusinessSenders::List list = BusinessSenders::parse( + QStringLiteral("noreply@cofidis.it\n")); + QVERIFY(BusinessSenders::contains(list, + QStringLiteral("noreply@cofidis.it"))); + QVERIFY(!BusinessSenders::contains(list, + QStringLiteral("someone@cofidis.it"))); +} + +void TestBusinessSenders::aDomainEntryMatchesEveryAddressUnderIt() +{ + const BusinessSenders::List list = + BusinessSenders::parse(QStringLiteral("@cofidis.it\n")); + QVERIFY(BusinessSenders::contains(list, + QStringLiteral("noreply@cofidis.it"))); + QVERIFY(BusinessSenders::contains(list, + QStringLiteral("billing@cofidis.it"))); + QVERIFY(!BusinessSenders::contains(list, + QStringLiteral("a@example.org"))); +} + +void TestBusinessSenders::commentsAndBlankLinesAreIgnored() +{ + // A commented entry is the REJECT gesture: present in the file, not + // applied. This is the property the whole file format rests on. + const BusinessSenders::List list = BusinessSenders::parse( + QStringLiteral("# noreply@cofidis.it (47 messages)\n" + "\n" + " \n" + "billing@example.org\n")); + QVERIFY(!BusinessSenders::contains(list, + QStringLiteral("noreply@cofidis.it"))); + QVERIFY(BusinessSenders::contains(list, + QStringLiteral("billing@example.org"))); +} + +void TestBusinessSenders::whitespaceAroundAnEntryIsIgnored() +{ + const BusinessSenders::List list = + BusinessSenders::parse(QStringLiteral(" billing@example.org \n")); + QVERIFY(BusinessSenders::contains(list, + QStringLiteral("billing@example.org"))); +} + +void TestBusinessSenders::matchingIsCaseInsensitive() +{ + // Addresses arrive from headers in whatever case the sender used, so a + // list entry that matched only one casing would look broken at random. + const BusinessSenders::List list = + BusinessSenders::parse(QStringLiteral("NoReply@Cofidis.IT\n")); + QVERIFY(BusinessSenders::contains(list, + QStringLiteral("noreply@cofidis.it"))); +} + +void TestBusinessSenders::anAbsentFileMatchesNothing() +{ + QTemporaryDir dir; + const BusinessSenders::List list = + BusinessSenders::load(dir.filePath(QStringLiteral("does-not-exist"))); + QVERIFY(!BusinessSenders::contains(list, QStringLiteral("a@example.org"))); +} + +QTEST_MAIN(TestBusinessSenders) +#include "test_businesssenders.moc" +``` + +- [ ] **Step 2: Register and run the test to verify it fails** + +Add `businesssenders.cpp` to `src/CMakeLists.txt` and `add_qtmaildir_test(businesssenders)` to `tests/CMakeLists.txt`. + +```bash +cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Debug && cmake --build build +``` + +Expected: FAIL, `businesssenders.h` not found. + +- [ ] **Step 3: Write the header** + +Create `src/businesssenders.h` with the GPLv2 header, then: + +```cpp +#pragma once + +#include <QSet> +#include <QString> +#include <QStringList> + +/// The list of senders that read as businesses rather than people. +/// +/// `~/.config/qtmaildir/business-senders`, plain text, one entry per line, +/// `#` comments, blank lines ignored. Deliberately NOT in qtmaildir.conf and +/// deliberately not INI: the user's stated workflow is grep-and-edit, QSettings +/// would fight a bare list, and the main config is already large. +/// +/// An entry is an exact address (`noreply@cofidis.it`) or a whole domain +/// (`@cofidis.it`). No globs: a pattern language is a rule the user cannot grep +/// for literally, which defeats the file's purpose. +namespace BusinessSenders +{ + +/// Parsed entries, lower-cased. Two sets rather than one list so a lookup is a +/// hash probe per repaint rather than a walk. +struct List +{ + QSet<QString> addresses; + QSet<QString> domains; ///< Stored WITHOUT the leading '@'. +}; + +List parse(const QString &contents); + +/// Reads `path`. A missing or unreadable file yields an empty list rather than +/// an error: the feature is cosmetic and must never block startup. +List load(const QString &path); + +bool contains(const List &list, const QString &address); + +/// `~/.config/qtmaildir/business-senders`, built from +/// QStandardPaths::GenericConfigLocation. +QString defaultPath(); + +} // namespace BusinessSenders +``` + +- [ ] **Step 4: Implement it** + +Create `src/businesssenders.cpp` with the GPLv2 header, then: + +```cpp +#include "businesssenders.h" + +#include <QDir> +#include <QFile> +#include <QStandardPaths> +#include <QTextStream> + +namespace BusinessSenders { + +List parse(const QString &contents) +{ + List list; + const QStringList lines = contents.split(QLatin1Char('\n')); + for (const QString &raw : lines) { + const QString line = raw.trimmed(); + // A commented entry is the reject gesture: it stays in the file so it + // is never proposed again, and it is not applied. + if (line.isEmpty() || line.startsWith(QLatin1Char('#'))) + continue; + + const QString entry = line.toLower(); + if (entry.startsWith(QLatin1Char('@'))) + list.domains.insert(entry.mid(1)); + else + list.addresses.insert(entry); + } + return list; +} + +List load(const QString &path) +{ + QFile file(path); + if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) + return List(); + return parse(QString::fromUtf8(file.readAll())); +} + +bool contains(const List &list, const QString &address) +{ + const QString lowered = address.trimmed().toLower(); + if (lowered.isEmpty()) + return false; + if (list.addresses.contains(lowered)) + return true; + + const int at = lowered.indexOf(QLatin1Char('@')); + if (at < 0) + return false; + return list.domains.contains(lowered.mid(at + 1)); +} + +QString defaultPath() +{ + const QString base = QStandardPaths::writableLocation( + QStandardPaths::GenericConfigLocation); + return QDir(base).filePath( + QStringLiteral("qtmaildir/business-senders")); +} + +} // namespace BusinessSenders +``` + +- [ ] **Step 5: Run test to verify it passes** + +```bash +cmake --build build && QT_QPA_PLATFORM=offscreen ctest --test-dir build -R businesssenders --output-on-failure +``` + +Expected: PASS, 6 tests. + +- [ ] **Step 6: Commit** + +```bash +git add src/businesssenders.h src/businesssenders.cpp src/CMakeLists.txt tests/test_businesssenders.cpp tests/CMakeLists.txt +git commit -S -m "feat: read the business-senders list" +``` + +--- + +### Task 6: Appending candidates + +This is the data-adjacent half and deserves the most care. Two rules: never write an uncommented entry, and never re-propose an address already present in any form. + +**Files:** +- Modify: `src/businesssenders.h`, `src/businesssenders.cpp` +- Test: `tests/test_businesssenders.cpp` + +- [ ] **Step 1: Write the failing test** + +```cpp +void TestBusinessSenders::candidatesAreAppendedCommentedOut() +{ + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("business-senders")); + + QHash<QString, int> counts; + counts.insert(QStringLiteral("noreply@cofidis.it"), 47); + BusinessSenders::appendCandidates(path, counts); + + QFile file(path); + QVERIFY(file.open(QIODevice::ReadOnly | QIODevice::Text)); + const QString written = QString::fromUtf8(file.readAll()); + + // Commented, and carrying the count so the user can judge it. + QVERIFY(written.contains(QStringLiteral("# noreply@cofidis.it"))); + QVERIFY(written.contains(QStringLiteral("47"))); + + // Nothing it wrote may take effect on its own. + const BusinessSenders::List list = BusinessSenders::load(path); + QVERIFY(!BusinessSenders::contains(list, + QStringLiteral("noreply@cofidis.it"))); +} + +void TestBusinessSenders::anAddressAlreadyPresentIsNeverReproposed() +{ + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("business-senders")); + + // Both forms count as present: an active entry and a rejected one. The + // rejected case is the one that matters, since re-proposing it would undo + // the user's decision every ten minutes with no explanation. + QFile seed(path); + QVERIFY(seed.open(QIODevice::WriteOnly | QIODevice::Text)); + seed.write("billing@example.org\n# noreply@cofidis.it (47 messages)\n"); + seed.close(); + const qint64 sizeBefore = QFileInfo(path).size(); + + QHash<QString, int> counts; + counts.insert(QStringLiteral("noreply@cofidis.it"), 51); + counts.insert(QStringLiteral("billing@example.org"), 12); + BusinessSenders::appendCandidates(path, counts); + + QCOMPARE(QFileInfo(path).size(), sizeBefore); +} + +void TestBusinessSenders::onlyBulkLookingLocalPartsAreProposed() +{ + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("business-senders")); + + QHash<QString, int> counts; + counts.insert(QStringLiteral("noreply@a.org"), 3); + counts.insert(QStringLiteral("john.doe@b.org"), 3); + BusinessSenders::appendCandidates(path, counts); + + QFile file(path); + QVERIFY(file.open(QIODevice::ReadOnly | QIODevice::Text)); + const QString written = QString::fromUtf8(file.readAll()); + QVERIFY(written.contains(QStringLiteral("noreply@a.org"))); + QVERIFY(!written.contains(QStringLiteral("john.doe@b.org"))); +} +``` + +```cpp +void TestBusinessSenders::theFirstRunScansEverything() +{ + QTemporaryDir dir; + const QString missing = dir.filePath(QStringLiteral("business-senders")); + + // No file at all: a week of mail would propose almost nothing and the + // list would take months to become useful, so the first run pays for a + // full scan once. + QCOMPARE(BusinessSenders::scanQuery(missing), QStringLiteral("*")); + + // A file holding ONLY rejected candidates is still a first run: nothing + // has been accepted yet. Rescanning re-proposes none of them, since + // appendCandidates skips anything already mentioned. + QFile rejected(missing); + QVERIFY(rejected.open(QIODevice::WriteOnly | QIODevice::Text)); + rejected.write("# noreply@cofidis.it (47 messages)\n"); + rejected.close(); + QCOMPARE(BusinessSenders::scanQuery(missing), QStringLiteral("*")); +} + +void TestBusinessSenders::alaterRunScansOnlyRecentMail() +{ + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("business-senders")); + QFile file(path); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + file.write("billing@example.org\n"); + file.close(); + + QCOMPARE(BusinessSenders::scanQuery(path), QStringLiteral("date:1week..")); +} +``` + +Declare all five in `private slots:` and add `#include <QFileInfo>` to the test's includes. + +- [ ] **Step 2: Run test to verify it fails** + +```bash +cmake --build build 2>&1 | tail -5 +``` + +Expected: FAIL, `appendCandidates` undefined. + +- [ ] **Step 3: Declare it** + +In `src/businesssenders.h`, inside the namespace: + +```cpp +/// True when a local part looks like bulk mail rather than a person. +/// +/// A GUESS, and openly one. It misses senders and proposes wrong ones, which +/// is exactly why nothing it produces takes effect until the user uncomments +/// it. +bool looksLikeBulk(const QString &address); + +/// Appends anything in `counts` that looks like bulk and is not already in the +/// file, COMMENTED OUT, with its message count. +/// +/// Two rules, both load-bearing. It never writes an uncommented entry, so +/// nothing on screen changes until the user acts. And it skips an address +/// already present in ANY form, commented or not, so an entry the user +/// rejected is never re-proposed, and one they deleted only returns if that +/// sender writes again. +void appendCandidates(const QString &path, const QHash<QString, int> &counts); + +/// The query the candidate scan should run. +/// +/// A week of mail once the file exists, so the step stays incremental and +/// cheap. EVERYTHING when the file is missing or holds no entries, because +/// that is the first run: a week's mail proposes almost nothing, and the file +/// would then take months to become useful. The whole-database scan is +/// affordable precisely because it happens once, measured at 76 ms over 5105 +/// messages. +/// +/// Returns notmuch query syntax, which is wire format and is never translated. +QString scanQuery(const QString &path); +``` + +Add `#include <QHash>` to the header. + +- [ ] **Step 4: Implement it** + +In `src/businesssenders.cpp`: + +```cpp +bool looksLikeBulk(const QString &address) +{ + static const QStringList kBulkLocalParts { + QStringLiteral("noreply"), QStringLiteral("no-reply"), + QStringLiteral("donotreply"), QStringLiteral("do-not-reply"), + QStringLiteral("info"), QStringLiteral("support"), + QStringLiteral("billing"), QStringLiteral("newsletter"), + QStringLiteral("notifications"), QStringLiteral("mailer-daemon"), + }; + const int at = address.indexOf(QLatin1Char('@')); + if (at <= 0) + return false; + const QString local = address.left(at).toLower(); + for (const QString &candidate : kBulkLocalParts) { + if (local == candidate || local.startsWith(candidate)) + return true; + } + return false; +} + +void appendCandidates(const QString &path, const QHash<QString, int> &counts) +{ + // Every address the file MENTIONS, active or rejected. Parsed separately + // from parse() above, which deliberately drops comments: here a comment is + // exactly what must be remembered. + QSet<QString> mentioned; + QFile existing(path); + if (existing.open(QIODevice::ReadOnly | QIODevice::Text)) { + const QStringList lines = + QString::fromUtf8(existing.readAll()).split(QLatin1Char('\n')); + for (const QString &raw : lines) { + QString line = raw.trimmed(); + if (line.startsWith(QLatin1Char('#'))) + line = line.mid(1).trimmed(); + if (line.isEmpty()) + continue; + // "noreply@cofidis.it (47 messages)" mentions the address before + // its count. + mentioned.insert(line.section(QLatin1Char(' '), 0, 0).toLower()); + } + existing.close(); + } + + QStringList additions; + for (auto it = counts.constBegin(); it != counts.constEnd(); ++it) { + const QString address = it.key().trimmed().toLower(); + if (address.isEmpty() || mentioned.contains(address)) + continue; + if (!looksLikeBulk(address)) + continue; + additions.append(QStringLiteral("# %1 (%2 messages)") + .arg(address) + .arg(it.value())); + } + if (additions.isEmpty()) + return; + + additions.sort(); + + QDir().mkpath(QFileInfo(path).absolutePath()); + QFile file(path); + if (!file.open(QIODevice::Append | QIODevice::Text)) + return; + QTextStream out(&file); + for (const QString &line : additions) + out << line << '\n'; +} +``` + +```cpp +QString scanQuery(const QString &path) +{ + // "*" is notmuch's match-everything. An EMPTY string would also match + // everything, which is why Config::matchNothingQuery() exists elsewhere in + // this codebase; being explicit here means a reader never has to wonder + // which of the two an empty return meant. + const List existing = load(path); + if (existing.addresses.isEmpty() && existing.domains.isEmpty()) + return QStringLiteral("*"); + return QStringLiteral("date:1week.."); +} +``` + +Add `#include <QFileInfo>` to `src/businesssenders.cpp`. + +Note what the emptiness test is deliberately NOT: it asks whether the file holds +any usable ENTRY, not whether the file exists or has bytes. A file holding only +rejected candidates, every line commented out, is still a first run as far as +this is concerned, and rescanning it costs 76 ms and re-proposes nothing, since +`appendCandidates` skips everything already mentioned. + +- [ ] **Step 5: Run test to verify it passes** + +```bash +cmake --build build && QT_QPA_PLATFORM=offscreen ctest --test-dir build -R businesssenders --output-on-failure +``` + +Expected: PASS, 11 tests. + +- [ ] **Step 6: Commit** + +```bash +git add src/businesssenders.h src/businesssenders.cpp tests/test_businesssenders.cpp +git commit -S -m "feat: propose business-sender candidates, always commented out" +``` + +--- + +### Task 7: `CardLayout::avatarRect` + +**Files:** +- Modify: `src/cardlayout.h`, `src/cardlayout.cpp` +- Test: `tests/test_cardlayout.cpp` + +- [ ] **Step 1: Write the failing test** + +Add to `tests/test_cardlayout.cpp`, declaring each in `private slots:`: + +```cpp +void TestCardLayout::everyRowCarriesAnAvatar() +{ + const QFont font; + const QRect rect(0, 0, 600, CardLayout::heightFor(font)); + + CardLayout::Input thread; + const CardLayout rootCard = CardLayout::compute(thread, rect, font); + QVERIFY(!rootCard.avatarRect.isEmpty()); + + // A reply gets one too: it is the row where the sender actually changes. + CardLayout::Input reply; + reply.isMessage = true; + reply.depth = 1; + const CardLayout replyCard = CardLayout::compute(reply, rect, font); + QVERIFY(!replyCard.avatarRect.isEmpty()); +} + +void TestCardLayout::theAvatarPushesTheContentRight() +{ + const QFont font; + const QRect rect(0, 0, 600, CardLayout::heightFor(font)); + const CardLayout card = CardLayout::compute(CardLayout::Input(), rect, font); + + // The text starts after the squircle, never on it. + QVERIFY(card.contentLeft >= card.avatarRect.right() + 1); +} + +void TestCardLayout::theAvatarFollowsTheIndent() +{ + const QFont font; + const QRect rect(0, 0, 600, CardLayout::heightFor(font)); + + CardLayout::Input shallow; + shallow.isMessage = true; + shallow.depth = 1; + CardLayout::Input deep; + deep.isMessage = true; + deep.depth = 3; + + const CardLayout shallowCard = CardLayout::compute(shallow, rect, font); + const CardLayout deepCard = CardLayout::compute(deep, rect, font); + + // The squircle sits inside the card's own rect and moves with the nesting, + // which is the same reason contentLeft does. Asserting on the RECT here is + // safe precisely because it is CardLayout's own output, not a visualRect. + QVERIFY(deepCard.avatarRect.left() > shallowCard.avatarRect.left()); +} + +void TestCardLayout::theAvatarIsSquareAndFitsTheCard() +{ + const QFont font; + const QRect rect(0, 0, 600, CardLayout::heightFor(font)); + const CardLayout card = CardLayout::compute(CardLayout::Input(), rect, font); + + QCOMPARE(card.avatarRect.width(), card.avatarRect.height()); + QVERIFY(card.avatarRect.top() >= rect.top()); + QVERIFY(card.avatarRect.bottom() <= rect.bottom()); +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +```bash +cmake --build build 2>&1 | tail -5 +``` + +Expected: FAIL, `avatarRect` is not a member of `CardLayout`. + +- [ ] **Step 3: Add the rect and the constant** + +In `src/cardlayout.h`, beside `accentRect`: + +```cpp + /// The sender's avatar squircle, in its own gutter before the text. + /// + /// On EVERY row, thread and reply alike: a reply is where the sender + /// actually changes, so it is the row whose author is most worth seeing. + /// Square, and inset vertically so it does not touch the card's edges. + QRect avatarRect; + + /// Space between the avatar and the text that follows it. + static constexpr int kAvatarGap = 8; +``` + +- [ ] **Step 4: Compute it** + +In `src/cardlayout.cpp`, inside `compute()`, immediately after `out.contentLeft` is first assigned and **before** `right`, `lineOneTop` and the rects that use `contentLeft` are computed: + +```cpp + // The avatar, square, in the gutter between the indent and the text. + // Sized from the card's HEIGHT rather than from a pixel constant, so it + // follows the desktop's font exactly as markSide() does. + const int avatarSide = qMax(0, rect.height() - kPaddingY * 2); + out.avatarRect = QRect(out.contentLeft, rect.top() + kPaddingY, + avatarSide, avatarSide); + // Everything after it starts past the squircle. This is what the item's + // cost is: a deep reply loses the gutter on top of its indent. + out.contentLeft = out.avatarRect.right() + 1 + kAvatarGap; +``` + +- [ ] **Step 5: Run test to verify it passes** + +```bash +cmake --build build && QT_QPA_PLATFORM=offscreen ctest --test-dir build -R cardlayout --output-on-failure +``` + +Expected: PASS. If existing tests in this file assert absolute positions of `senderRect` or `subjectRect`, they will now fail correctly, since the content genuinely moved. Update those expectations to be relative to `contentLeft` rather than to fixed numbers, and note in the commit that they were adjusted. + +- [ ] **Step 6: Commit** + +```bash +git add src/cardlayout.h src/cardlayout.cpp tests/test_cardlayout.cpp +git commit -S -m "feat: reserve a card's avatar gutter" +``` + +--- + +### Task 8: Model roles for the sender and the account address + +**Files:** +- Modify: `src/threadlistmodel.h`, `src/threadlistmodel.cpp` +- Test: `tests/test_threadlistmodel.cpp` + +- [ ] **Step 1: Write the failing test** + +```cpp +void TestThreadListModel::aRowCarriesItsSenderAndAccountAddress() +{ + ThreadListModel model; + ThreadSummary summary; + summary.threadId = QStringLiteral("t1"); + summary.subject = QStringLiteral("Subject"); + summary.authors = QStringLiteral("John Doe"); + summary.firstMessageId = QStringLiteral("m1"); + summary.firstMessageSender = QStringLiteral("john@example.org"); + model.setThreads({ summary }); + + const QModelIndex index = model.index(0, 0); + QCOMPARE(index.data(ThreadListModel::SenderAddressRole).toString(), + QStringLiteral("john@example.org")); + // The display name comes from `authors`, which is all notmuch gives. + QCOMPARE(index.data(ThreadListModel::SenderNameRole).toString(), + QStringLiteral("John Doe")); +} +``` + +Check the model's actual seeding helper (`setThreads` or equivalent) with `grep -n 'void setThreads\|void addThreads' src/threadlistmodel.h` and adapt. + +- [ ] **Step 2: Run test to verify it fails** + +```bash +cmake --build build 2>&1 | tail -5 +``` + +Expected: FAIL, `SenderAddressRole` is not a member. + +- [ ] **Step 3: Add the roles** + +In `src/threadlistmodel.h`, in the role enum beside `AccountColourRole`: + +```cpp + /// The bare address of the message this row stands for, for the + /// avatar's hash and for the business-senders lookup. Empty when the + /// query did not resolve one, which the delegate handles by falling + /// back to the account. + SenderAddressRole, + /// The display name to take initials from. `authors` for a thread row, + /// `recipients` in a flat view, matching what the card already shows. + SenderNameRole, +``` + +- [ ] **Step 4: Serve them** + +In `src/threadlistmodel.cpp`, in the thread-row branch of `data()`, beside the existing `AccountColourRole` case: + +```cpp + case SenderAddressRole: + return thread.firstMessageSender; + case SenderNameRole: + // The same string the card's first line shows: recipients in a flat + // view, where `authors` is the user on every row and says nothing. + return !thread.recipients.isEmpty() ? thread.recipients + : thread.authors; +``` + +**Add the same two cases to the MESSAGE-row branch**, which is a separate switch. `CLAUDE.md` records that a cue added to one branch and not the other is simply absent with nothing to flag it, and that this has already been missed once. For a message row, serve the node's own sender and name. + +- [ ] **Step 5: Run test to verify it passes** + +```bash +cmake --build build && QT_QPA_PLATFORM=offscreen ctest --test-dir build -R threadlistmodel --output-on-failure +``` + +Expected: PASS. + +- [ ] **Step 6: Commit** + +```bash +git add src/threadlistmodel.h src/threadlistmodel.cpp tests/test_threadlistmodel.cpp +git commit -S -m "feat: expose a row's sender to the delegate" +``` + +--- + +### Task 9: Painting the fade + +**Files:** +- Modify: `src/carddelegate.h`, `src/carddelegate.cpp` +- Test: `tests/test_carddelegate.cpp` + +The fade's geometry is asserted through a static helper rather than by counting pixels, per `CLAUDE.md`: a probe pointed at the function the production path calls into, rather than at the painter, is the one that a mutation cannot survive. + +- [ ] **Step 1: Write the failing test** + +```cpp +void TestCardDelegate::theFadeEndsAtSixtyPercentOfTheCard() +{ + const QRect card(0, 0, 500, 60); + const QRect root = CardDelegate::fadeRectFor(card, QRect()); + QCOMPARE(root.left(), card.left()); + QCOMPARE(root.width(), 300); +} + +void TestCardDelegate::aReplyFadeStartsAtItsOwnSpine() +{ + const QRect card(0, 0, 500, 60); + // The innermost spine of a nested reply, which is its own coloured border. + const QRect spine(80, 0, 2, 60); + const QRect reply = CardDelegate::fadeRectFor(card, spine); + + // It hangs off the spine, not off the card's edge. + QCOMPARE(reply.left(), spine.left()); + // And still ends at 60% of the CARD, so a deeper reply's wash is shorter + // as well as further right. + QCOMPARE(reply.right(), CardDelegate::fadeRectFor(card, QRect()).right()); + QVERIFY(reply.width() < CardDelegate::fadeRectFor(card, QRect()).width()); +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +```bash +cmake --build build 2>&1 | tail -5 +``` + +Expected: FAIL, `fadeRectFor` is not a member of `CardDelegate`. + +- [ ] **Step 3: Declare the helper** + +In `src/carddelegate.h`, in the public static section beside `accentLineColour`: + +```cpp + /// Where the account's fade runs, given the card and the row's innermost + /// spine (an empty rect for a thread root, which has none). + /// + /// A root's fade starts at the card's left edge; a reply's starts at its + /// own spine, which IS its coloured left border, so the wash steps right + /// with the nesting. Both end at 60% of the card's width, so a deeper + /// reply's wash is shorter as well as further right. + /// + /// Static and rect-in, rect-out so the geometry is assertable without a + /// painter, for the same reason CardLayout is. + static QRect fadeRectFor(const QRect &card, const QRect &innermostSpine); + + /// How far across the card the account's colour reaches. + static constexpr qreal kFadeFraction = 0.60; +``` + +- [ ] **Step 4: Implement it and paint** + +In `src/carddelegate.cpp`: + +```cpp +QRect CardDelegate::fadeRectFor(const QRect &card, const QRect &innermostSpine) +{ + // The EXCLUSIVE right edge, then a rect built from it: QRect::right() is + // inclusive, which is the trap CardLayout already documents. + const int end = card.left() + int(card.width() * kFadeFraction); + const int start = innermostSpine.isEmpty() ? card.left() + : innermostSpine.left(); + if (end <= start) + return QRect(); + return QRect(start, card.top(), end - start, card.height()); +} +``` + +In `paint()`, immediately **after** the chrome is drawn and **before** the accent bar (so the bar sits on top of its own fade): + +```cpp + // The account's fade. Under everything but the chrome, so the selection + // highlight and the doomed-row tint still cover it: a selected row reading + // mostly as selection is expected, not a fault. + const QRect fade = + fadeRectFor(option.rect, + card.spines.isEmpty() ? QRect() : card.spines.last()); + if (!fade.isEmpty() && accountColour.isValid()) { + QColor from = lineColour; + // A reply's wash is weaker than its root's, so an expanded thread + // reads as one block with the root leading it. + from.setAlphaF(card.accentRect.isEmpty() ? 0.14 : 0.30); + QLinearGradient gradient(fade.topLeft(), fade.topRight()); + gradient.setColorAt(0.0, from); + from.setAlphaF(0.0); + gradient.setColorAt(1.0, from); + painter->fillRect(fade, gradient); + } +``` + +`card.spines.last()` is the innermost level, since `compute()` appends outermost first. Add `#include <QLinearGradient>` to `src/carddelegate.cpp`. + +- [ ] **Step 5: Run test to verify it passes** + +```bash +cmake --build build && QT_QPA_PLATFORM=offscreen ctest --test-dir build -R carddelegate --output-on-failure +``` + +Expected: PASS. + +- [ ] **Step 6: Commit** + +```bash +git add src/carddelegate.h src/carddelegate.cpp tests/test_carddelegate.cpp +git commit -S -m "feat: fade the account colour across a card" +``` + +--- + +### Task 10: Painting the avatar + +**Files:** +- Modify: `src/carddelegate.h`, `src/carddelegate.cpp` +- Test: `tests/test_carddelegate.cpp` + +- [ ] **Step 1: Write the failing test** + +```cpp +void TestCardDelegate::theDelegateAsksForAScaledSquircle() +{ + // Asserted through the function the PRODUCTION path calls, not through + // Avatar::pixmapFor() directly: a test pointed at the function being + // called into proves what that function does and nothing about whether the + // delegate asks it for the right thing. CLAUDE.md records a mutation that + // survived exactly that mistake. + const QRect card(0, 0, 500, 60); + const QFont font; + const CardLayout layout = + CardLayout::compute(CardLayout::Input(), card, font); + + const QPixmap pixmap = CardDelegate::avatarFor( + QStringLiteral("john@example.org"), QStringLiteral("John Doe"), + QStringLiteral("me@example.org"), QStringLiteral("Work"), false, + layout.avatarRect.width(), font); + + QCOMPARE(pixmap.size(), + QSize(layout.avatarRect.width(), layout.avatarRect.width())); +} + +void TestCardDelegate::aRowWithNoSenderFallsBackToTheAccount() +{ + const QFont font; + // No sender address at all: the squircle is still drawn, seeded from the + // account, so a card never shows a hole. + const QPixmap fallback = CardDelegate::avatarFor( + QString(), QString(), QStringLiteral("me@example.org"), + QStringLiteral("Work"), false, 44, font); + QVERIFY(!fallback.isNull()); + + // And it is the ACCOUNT's identity, not an arbitrary one: seeding from the + // same account twice agrees. + const QPixmap again = CardDelegate::avatarFor( + QString(), QString(), QStringLiteral("me@example.org"), + QStringLiteral("Work"), false, 44, font); + QCOMPARE(fallback.toImage(), again.toImage()); +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +```bash +cmake --build build 2>&1 | tail -5 +``` + +Expected: FAIL, `avatarFor` is not a member. + +- [ ] **Step 3: Declare it** + +In `src/carddelegate.h`: + +```cpp + /// The squircle for one row, resolved from what the model supplies. + /// + /// Falls back to the ACCOUNT when the row has no sender address, so every + /// card carries an avatar rather than a hole: the seed becomes the + /// account's own address and the letters come from its label. + static QPixmap avatarFor(const QString &senderAddress, + const QString &senderName, + const QString &accountAddress, + const QString &accountLabel, + bool isBusinessSender, int side, + const QFont &font); +``` + +- [ ] **Step 4: Implement it and paint** + +In `src/carddelegate.cpp`: + +```cpp +QPixmap CardDelegate::avatarFor(const QString &senderAddress, + const QString &senderName, + const QString &accountAddress, + const QString &accountLabel, + bool isBusinessSender, int side, + const QFont &font) +{ + const bool haveSender = !senderAddress.trimmed().isEmpty(); + const QString seed = haveSender ? senderAddress : accountAddress; + const QString initials = + Avatar::initialsFor(senderName, senderAddress, accountLabel); + const Avatar::Fill fill = Avatar::fillFor(senderName, isBusinessSender); + return Avatar::pixmapFor(seed, initials, fill, side, font); +} +``` + +In `paint()`, after the fade and the accent bar: + +```cpp + if (!card.avatarRect.isEmpty()) { + const QString senderAddress = + index.data(ThreadListModel::SenderAddressRole).toString(); + const QString senderName = + index.data(ThreadListModel::SenderNameRole).toString(); + painter->drawPixmap( + card.avatarRect, + avatarFor(senderAddress, senderName, m_accountAddress, + m_accountLabel, + BusinessSenders::contains(m_businessSenders, + senderAddress), + card.avatarRect.width(), option.font)); + } +``` + +Add three members to `CardDelegate`, with a setter for each, defaulting empty: `m_accountAddress`, `m_accountLabel`, `m_businessSenders` (a `BusinessSenders::List`). `MainWindow` fills them in Task 11. Include `avatar.h` and `businesssenders.h` in `src/carddelegate.cpp`. + +- [ ] **Step 5: Run test to verify it passes** + +```bash +cmake --build build && QT_QPA_PLATFORM=offscreen ctest --test-dir build -R carddelegate --output-on-failure +``` + +Expected: PASS. + +- [ ] **Step 6: Commit** + +```bash +git add src/carddelegate.h src/carddelegate.cpp tests/test_carddelegate.cpp +git commit -S -m "feat: draw a sender's avatar on every card" +``` + +--- + +### Task 11: Wiring the list into the window + +**Files:** +- Modify: `src/mainwindow.cpp` +- Test: `tests/test_mainwindow.cpp` + +- [ ] **Step 1: Write the failing test** + +```cpp +void TestMainWindow::theBusinessSenderListIsLoadedAtStartup() +{ + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("business-senders")); + QFile file(path); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + file.write("@cofidis.it\n"); + file.close(); + + MainWindow window; + window.loadBusinessSenders(path); + + QVERIFY(window.businessSendersForTest().domains.contains( + QStringLiteral("cofidis.it"))); +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +```bash +cmake --build build 2>&1 | tail -5 +``` + +Expected: FAIL, `loadBusinessSenders` is not a member. + +- [ ] **Step 3: Implement** + +In `MainWindow`, add: + +```cpp + /// Reads the business-senders list and hands it to the delegate. + /// + /// Once at startup and on an explicit reload, never per repaint and never + /// stat-per-row: the file is small and the painting path runs on every + /// row of every scroll. + void loadBusinessSenders(const QString &path = QString()); + + /// Test accessor, so the load can be asserted without reaching into the + /// delegate. + const BusinessSenders::List &businessSendersForTest() const + { + return m_businessSenders; + } +``` + +```cpp +void MainWindow::loadBusinessSenders(const QString &path) +{ + m_businessSenders = BusinessSenders::load( + path.isEmpty() ? BusinessSenders::defaultPath() : path); + m_cardDelegate->setBusinessSenders(m_businessSenders); +} +``` + +Call it from the constructor, after the delegate is created. Also set the delegate's account address and label wherever the account selection is applied, so the fallback avatar has something to seed from; search for `AccountColourRole` in `mainwindow.cpp` for where account data already reaches the view. + +- [ ] **Step 4: Run test to verify it passes** + +```bash +cmake --build build && QT_QPA_PLATFORM=offscreen ctest --test-dir build -R mainwindow --output-on-failure +``` + +Expected: PASS. Note the suite takes about 25 seconds. + +- [ ] **Step 5: Commit** + +```bash +git add src/mainwindow.cpp src/mainwindow.h tests/test_mainwindow.cpp +git commit -S -m "feat: load the business-senders list at startup" +``` + +--- + +### Task 12: Proposing candidates after a sync + +**Files:** +- Modify: `src/notmuchworker.h`, `src/notmuchworker.cpp`, `src/mainwindow.cpp` +- Test: `tests/test_notmuchworker.cpp` + +- [ ] **Step 1: Write the failing test** + +```cpp +void TestNotmuchWorker::sendersAreCountedForTheCandidateList() +{ + NotmuchFixture fixture; + fixture.addMessage("noreply@shop.example", QStringLiteral("Receipt one")); + fixture.addMessage("noreply@shop.example", QStringLiteral("Receipt two")); + fixture.addMessage("john@example.org", QStringLiteral("Hello")); + fixture.index(); + + NotmuchWorker worker(fixture.configPath()); + QVERIFY(worker.open()); + + QSignalSpy spy(&worker, &NotmuchWorker::senderCountsReady); + worker.countSenders(QStringLiteral("*")); + QVERIFY(spy.count() > 0); + + const auto counts = spy.first().at(0).value<QHash<QString, int>>(); + QCOMPARE(counts.value(QStringLiteral("noreply@shop.example")), 2); + QCOMPARE(counts.value(QStringLiteral("john@example.org")), 1); +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +```bash +cmake --build build 2>&1 | tail -5 +``` + +Expected: FAIL, `countSenders` is not a member. + +- [ ] **Step 3: Implement the worker side** + +In `src/notmuchworker.h`: + +```cpp +public slots: + /// Counts messages per sender address over `query`. + /// + /// Index-served, so it is cheap: measured 2026-08-26 on the developer's + /// database, 1322 distinct senders in 12 ms over 5105 messages. It does + /// NOT touch m_generation, which is the QUERY generation: bumping it would + /// discard a thread load in flight and blank the message pane because the + /// user synced. Item 169, following the same rule requestMessageCounts + /// already follows. + void countSenders(const QString &query); + +signals: + void senderCountsReady(const QHash<QString, int> &counts); +``` + +In `src/notmuchworker.cpp`: + +```cpp +void NotmuchWorker::countSenders(const QString &query) +{ + QHash<QString, int> counts; + if (!m_database) { + emit senderCountsReady(counts); + return; + } + + NmQuery nmQuery(notmuch_query_create(m_database.get(), + query.toUtf8().constData())); + if (!nmQuery) { + emit senderCountsReady(counts); + return; + } + + notmuch_messages_t *messages = nullptr; + if (notmuch_query_search_messages(nmQuery.get(), &messages) + != NOTMUCH_STATUS_SUCCESS) { + emit senderCountsReady(counts); + return; + } + + for (; messages && notmuch_messages_valid(messages); + notmuch_messages_move_to_next(messages)) { + notmuch_message_t *message = notmuch_messages_get(messages); + if (!message) + continue; + const QString sender = senderAddressOf(message); + if (!sender.isEmpty()) + counts[sender.toLower()] += 1; + } + + emit senderCountsReady(counts); +} +``` + +Register the metatype beside the others so a queued `QHash<QString, int>` is not dropped, exactly as `SortOrder` is: `qRegisterMetaType<QHash<QString, int>>("QHash<QString,int>");` in the same place. `Q_ENUM`-style registration is not enough for a queued argument, which `CLAUDE.md` records. + +- [ ] **Step 4: Wire it to the sync** + +In `MainWindow`, where a sync completes (search for where the unsynced count is cleared), request the counts, and on `senderCountsReady`: + +```cpp + connect(m_worker, &NotmuchWorker::senderCountsReady, this, + [this](const QHash<QString, int> &counts) { + // Never applies anything: appendCandidates writes commented + // lines only, so nothing on screen changes until the user + // uncomments one. The list is then reloaded so an entry they + // uncommented by hand takes effect without a restart. + BusinessSenders::appendCandidates( + BusinessSenders::defaultPath(), counts); + loadBusinessSenders(); + }); +``` + +Scope the request with `BusinessSenders::scanQuery()`, added below: a week of mail once the file exists, and everything on the first run. + +```cpp + countSenders(BusinessSenders::scanQuery(BusinessSenders::defaultPath())); +``` + +- [ ] **Step 5: Run test to verify it passes** + +```bash +cmake --build build && ctest --test-dir build -R notmuchworker --output-on-failure +``` + +Expected: PASS. + +- [ ] **Step 6: Commit** + +```bash +git add src/notmuchworker.h src/notmuchworker.cpp src/mainwindow.cpp tests/test_notmuchworker.cpp +git commit -S -m "feat: propose business senders from newly synced mail" +``` + +--- + +### Task 13: Full suite, translations and documentation + +**Files:** +- Modify: `translations/qtmaildir_it_IT.ts` +- Modify: `README.md`, `CHANGELOG.md` + +- [ ] **Step 1: Run the whole suite** + +```bash +ctest --test-dir build --output-on-failure +``` + +Expected: everything passes except `test_mainwindow undoMovesTheMessageBack`, which is backlog item 136 and pre-existing. Confirm that is the only failure; if anything else fails, it belongs to this work. + +- [ ] **Step 2: Refresh the translation** + +This feature adds no user-facing string if nothing was wrapped in `tr()`. Run it regardless, since the check is cheap and a missed string is now a regression: + +```bash +lupdate-qt6 src/ -ts translations/qtmaildir_it_IT.ts -no-obsolete -locations none +ctest --test-dir build -R translations --output-on-failure +``` + +Expected: zero context warnings, and the translations test passes. If a new string appeared, translate it in the `.ts` file; `lrelease` silently DROPS an unfinished string and ships it as English inside an otherwise Italian UI. + +- [ ] **Step 3: Document the list** + +Add a section to `README.md` beside the existing configuration documentation: + +```markdown +### `~/.config/qtmaildir/business-senders` + +Addresses that should read as businesses rather than people, one per line. +A card's avatar takes its pattern from this: a listed address gets the +two-tone fill, anything presenting a display name gets the identicon. + + # a comment, and the form the application itself writes + # noreply@cofidis.it (47 messages) + billing@example.org + @newsletter.example.com + +An entry is either an exact address or a whole domain written `@example.com`. +Comments and blank lines are ignored. + +After each sync the application appends addresses that look like bulk mail, +**always commented out**, so nothing changes appearance until you uncomment +it. Anything already in the file, commented or not, is never proposed again: +commenting a line out is therefore the permanent way to reject it, while +deleting it lets that sender be proposed again if they write to you. + +The first scan, when the file does not exist or holds no active entry, covers +the whole database so the list is useful straight away. Afterwards it covers +the last week's mail. +``` + +- [ ] **Step 4: Update the changelog** + +Under `## [Unreleased]`, in `### Added`: + +```markdown +- Cards carry the sender's avatar: a squircle with their initials, filled with + a pattern generated from their address so the same sender always looks the + same. Senders that present a display name get an identicon, bulk senders a + two-tone fill, and `~/.config/qtmaildir/business-senders` decides the + borderline cases. Nothing is fetched from the network. +- The account's colour now fades across the left of a card instead of only + marking its edge, and a reply's fade starts at its own indent. +``` + +- [ ] **Step 5: Commit** + +```bash +git add README.md CHANGELOG.md translations/qtmaildir_it_IT.ts +git commit -S -m "docs: document card avatars and the business-senders list" +``` + +--- + +### Task 14: Hand off for the look + +**This item is judged by looking, not by the suite.** Per the standing rule, tests cover what has a right answer; the appearance is the user's call. + +- [ ] **Step 1: Tell the user what to look at** + +Do not launch the application. Report that the work is ready and name what to check: + +- Whether the avatar gutter costs too much subject on a deeply nested reply. +- Whether the initials stay legible over an identicon at the desktop's own font size, which is the veil's opacity (`QColor(0, 0, 0, 77)` in `avatar.cpp`). +- Whether the fade at 60% reads right on a maximised window as well as a narrow one. +- Whether a reply's weaker fade reads as belonging to its root. +- Whether the two fills are distinguishable enough to be worth having as two. + +- [ ] **Step 2: Wait for the verdict before any tuning commit** + +The constants most likely to move are the veil's alpha, `kFadeFraction`, the two fade alphas in `carddelegate.cpp`, and `Avatar::colourFor`'s saturation and lightness. + +--- + +## Notes for whoever executes this + +- **`ThreadSummary` fixtures need `firstMessageSender` now.** A hand-built summary without one produces a fallback avatar rather than a sender's, which is correct behaviour and a confusing test failure. `makeThread()` in `test_mainwindow.cpp` should set it. +- **Two branches in `data()`.** The thread-row and message-row switches are separate; a role added to one is silently absent from the other. +- **Do not assert an indent with `visualRect`.** `setIndentation(0)` means the view reports the same left edge for a thread and its reply. Assert on `CardLayout`'s own output. +- **`QRect::right()` is inclusive.** Both `CardLayout` and `fadeRectFor` carry exclusive right edges for this reason. +- **Never run a test binary without `QT_QPA_PLATFORM=offscreen`.** diff --git a/docs/superpowers/specs/2026-08-26-card-avatars-design.md b/docs/superpowers/specs/2026-08-26-card-avatars-design.md new file mode 100644 index 0000000..e396a34 --- /dev/null +++ b/docs/superpowers/specs/2026-08-26-card-avatars-design.md @@ -0,0 +1,310 @@ +# Card avatars and the account fade + +Design for backlog item 169. Brainstormed with the user on 2026-08-26. + +**Status:** specified, unbuilt. + +## The problem + +The user's note asks for two things about a card: + +> the left border of a card expresses the account the mail belongs to. the +> background color of the card should fade left to right from the account color +> to the current background color we are using (or to transparent to work both +> in light and dark themes). On the left we should leave room for an account +> avatar (a squircle), for now it could be extracted from the sender name "From: +> john doe" becomes "JD" in the avatar. As soon as we include khard (or some +> other vcard provider/manager) we will switch to images if the corresponding +> vCard has one. + +Half of it already shipped. The account colour is drawn as a solid bar down the +left edge (`CardLayout::accentRect`, filled in `CardDelegate::paint()` through +`CardDelegate::accentLineColour()`), and a reply's spines carry the same colour +muted against the pane's base. What does not exist is any gradient, and any +avatar: `CardLayout` reserves no rect for one, so the geometry has to grow +before the painting can. + +The note calls it an "account avatar", but the brainstorm settled that it is +the SENDER's, not the account's. The account is already expressed twice, by the +accent bar and by the fade; an avatar in the account's colour would conflate +"which mailbox received this" with "who wrote it", which are different facts. + +## What the card looks like + +A card stays exactly three lines and every row keeps the same height, so +`setUniformRowHeights(true)` survives untouched. + +``` ++--+-------+------------------------------------------------+ +| | | sender 26/08 09:14 | +|##| [AV] | * subject @ v 3 replies | +| | | [tag] [tag] | ++--+-------+------------------------------------------------+ + ^ ^ + | +-- avatarRect, full height, its own gutter + +-------- accentRect, unchanged +``` + +The avatar is a full-height squircle in its own gutter, on roots and replies +alike. `contentLeft` shifts right by the gutter, on top of the existing indent. + +Three alternatives were shown to the user and rejected: a two-line squircle +with the tag strip running full width beneath it, and a small squircle inline +on the sender line costing no width. The full-height form won on presence, with +the cost accepted explicitly: at `kMaxDepth` the subject loses the gutter's +width on top of the indent it already loses. + +## The fade + +A horizontal gradient of the account colour, drawn after the chrome and before +the text, so the selection highlight and the deleted-row tint still cover it. +That a selected row reads mostly as selection is expected, not a fault. + +- **On a thread root** it starts at the card's left edge. +- **On a reply** it starts at that reply's INNERMOST spine, which is its own + coloured left border. The wash therefore steps right with the nesting, and a + deeper reply's outer spines stand in plain background. +- **It ends at 60% of the card's width**, in both cases. The end is + proportional, so a reply's wash is shorter as well as further right. +- **A reply's gradient is weaker than a root's**, so an expanded thread reads as + one coloured block with the root leading it. + +The user chose the reply origin against the alternative of starting every fade +at the card's left edge. The spine reading is the correct one because the +spines are ALREADY the account colour: `carddelegate.cpp` muted them at 0.55 +against `QPalette::Base` and resolves a reply's colour by walking to its root, +so the coloured border the fade hangs off is a thing that exists rather than +one this item introduces. + +60% was chosen by the user over 50%. It is a percentage of card WIDTH, which +means the wash grows with the window; a fixed pixel distance and an +anchor-to-the-layout variant were both offered and declined. + +## Whose face + +The **sender's**, except in the flat views (Sent and Drafts) where it is the +**recipient's**, since those views already show recipients in the sender's slot +and `authors` is the user on every row there. + +This costs no extra query. `MainWindow::m_sentView` is really "flat view": it is +assigned from `FlatResult`, and `generatorIsFlat()` in `config.cpp` is the closed +set `{sent, drafts}`, so both already request the recipients fold. The name is +misleading and deserves a comment, but not a rename inside this item. + +For a thread row the sender is the one belonging to `firstMessageId`, the +message the card already stands for. Nothing new is resolved. + +## The initials + +Two letters, always, so every squircle reads the same shape: + +| Input | Rule | Example | +|---|---|---| +| Display name, two or more words | first letter of the first two words | `John Doe` -> `JD` | +| Display name, one word | first two letters of that word | `Cofidis` -> `CO` | +| No display name | first of the local part, first of the domain | `noreply@cofidis.it` -> `NC` | +| Nothing usable at all | first two of the account's label | see the fallback below | + +The user chose the local-plus-domain form over first-two-of-local (`NO`) and +over a single letter (`N`), because it never degrades to one letter and never +reads as a truncated word. + +## The fill + +Two fills, both generated locally from a hash of the sender's address, both +stable per sender. + +- **Identicon** - a 5x5 symmetric grid from the hash bits, with a translucent + dark veil between the pattern and the letters so the initials stay readable + whatever the pattern does. +- **Two-tone** - two related hues and a split angle from the hash, initials on a + large flat field. + +Which one, in order: + +1. The address is listed in `business-senders` -> **two-tone**, whatever display + name it presents. +2. Otherwise a display name is present -> **identicon**. +3. Otherwise -> **two-tone**. + +Rule 2 is the one the user asked for by name: `Ian Farrell +<notifications@github.com>` gets `IF` and the identicon, because it presented +itself as a person, even though the address is corporate. Rule 1 is the override +that lets a listed sender be forced back to two-tone. + +Colours are generated at a FIXED lightness, so `TagColors::textColourOn()`-style +contrast reasoning holds and the initials stay legible in both themes. + +**None of this is Gravatar in the sense of contacting Gravatar.** A real lookup +would send a hash of every correspondent's address to a third party on each +repaint, which is out on the project's privacy stance and on the no-network +rule. Only the generated half of the idea is taken, and it works entirely +offline with `QCryptographicHash`, which Qt Core already provides. + +Item 72's vCard photo would later replace the fill without touching the rect. + +## The fallback + +There is always an avatar. When no sender address is available at all, the +squircle is hashed from the **account's own address**, which the card always +knows through its account, and the initials are the first two letters of the +account's label. This keeps a stable, themed squircle rather than a hole, and +it cannot be confused with a real sender because the two-letter source is the +account. + +## `ThreadSummary::firstMessageSender` + +**This is the one structural change, and it exists because the card currently +has no address to hash.** + +`ThreadSummary::authors` is notmuch's own summarised string and carries display +names ONLY. Measured against the real index on 2026-08-26: `'Standreas'`, +`'Randstad Italia'`, `'Ryanair'`, `'The Hacker News tramite LinkedIn'`. No `@` +anywhere. The initials rule survives that, but two things do not: + +- the identicon has nothing stable to hash, and +- the `business-senders` list has nothing to match. + +Hashing the display name instead was considered and rejected: notmuch builds +those strings, so one sender varies its identity as the string varies +(`The Hacker News tramite LinkedIn` is a constructed label, not a header value). + +So `ThreadSummary` gains `firstMessageSender`, the bare address of the message +the card stands for. It is filled by the SAME worker walk that already fills +`firstMessageId` and `firstMessageTags`, from the same message, and `From` is +served from notmuch's index rather than from the file. This is exactly the +pattern item 111 used for `firstMessageTags` and it carries the same "free, for +the same reason" note. + +The Sent/normal split is already resolved in that walk and must be respected: +`withRecipients` selects the first MATCHED message for a flat view and the +thread's opening message otherwise, so the address follows whichever message +the card is standing for. + +**Measured cost of reading senders from the index**, on the developer's own +database: 1322 distinct senders deduplicated in 12 ms, and 5105 messages +enumerated in 76 ms. There is no cost problem here, which is what makes the +whole feature and its list practical. + +## The `business-senders` list + +`~/.config/qtmaildir/business-senders`. Plain text, one entry per line, `#` +comments, blank lines ignored. + +Deliberately NOT in `qtmaildir.conf` and deliberately not INI. The user's stated +workflow is grep-and-edit ("if a personal email ends in the list by mistake I +can grep it out and remove it"), QSettings would fight a bare list, and the main +config is already large. + +An entry is either: + +- an exact address, `noreply@cofidis.it`, or +- a whole domain, `@cofidis.it`, which is what a company sending from six + addresses actually needs. + +**No globs.** A pattern language nobody asked for is a rule the user cannot grep +for literally, which defeats the file's whole purpose. + +Read once at startup and on an explicit reload. Never per repaint, and never +stat-per-row. + +## How the list fills itself + +At the end of every sync the application runs, the worker collects the senders +of the newly arrived mail and appends CANDIDATES, commented out: + +``` +# noreply@cofidis.it (47 messages) +``` + +**How much mail the scan covers** depends on whether the list has ever been +used. With no file, or a file holding no active entry, it scans the WHOLE +database; afterwards it scans the last week. The first run is exactly when a +full scan earns its cost: a week of mail proposes almost nothing, so a +week-only rule would leave the list taking months to become useful. It is +affordable because it happens once, measured at 76 ms over 5105 messages. + +A file holding only rejected candidates still counts as unused. Rescanning it +re-proposes none of them, since anything already mentioned is skipped. + +A candidate is an address whose local part is in a small built-in word list +(`noreply`, `no-reply`, `donotreply`, `info`, `support`, `billing`, +`newsletter`, `notifications`, `mailer-daemon`), or one that recurs with no +display name. + +Two rules make this safe, and both are borrowed from `mailrules.py`'s existing +discipline: + +- **Never writes an uncommented entry.** Nothing on screen changes until the + user uncomments a line. A step that silently reclassified forty senders would + have to be audited line by line anyway. +- **Never removes, and never re-adds.** An address already present in the file + in ANY form, commented or not, is skipped. An entry the user grepped out + therefore stays out, instead of reappearing within ten minutes with no + explanation. + +### Why the application writes it, and not the `post-new` hook + +Putting the scan in `assets/hooks/` was considered at length and rejected. It +is the better placement on paper: `post-new` runs after EVERY `notmuch new` +whatever started it, including the user's cron, whereas `mailsync.sh` is only +the route qtmaildir drives, and the hook is already scoped to `tag:new` so it +would see only new mail. + +It was rejected on weight. The hook is Python and the reader is C++, so it would +recreate the two-implementations-of-one-format situation that `CLAUDE.md` +documents for `rules.json`, with the same requirement that both sides change +together and the same by-test-only agreement. That is a heavy contract for a +cosmetic feature. + +The application writing it has one implementation, already knows when a sync +finished, and the scan is free. The only case lost is mail indexed by cron while +qtmaildir is not running, and those senders are picked up at the next run. + +## What is testable, and what is not + +Per the standing rule (`tests-only-for-measurable-things`), this feature is +mostly judged by looking. Split accordingly: + +**Assert in tests:** + +- `CardLayout` places `avatarRect` and moves `contentLeft`, at several depths + and for a root against a reply. Geometry only, no painter, per the existing + rule that a card layout must be testable without one. +- The initials function, over all four rules in the table above, including the + one-word and no-display-name cases and an address with no `@` at all. +- The fill CHOICE, over the three ordered rules, including a listed address + that presents a display name. +- Hash stability: the same address yields the same fill twice, and two + different addresses differ. +- `business-senders` parsing: exact entries, `@domain` entries, comments, blank + lines, and an entry with leading or trailing space. +- The append step: never writes an uncommented line, and skips an address + already present commented out. This is the data-loss-adjacent property, so it + is the one worth the most care. +- `ThreadSummary::firstMessageSender` is filled by the query walk, for a normal + view and for a flat one, where the two must resolve DIFFERENT messages. + +**Hand to the user to look at:** the fade's weight and its 60% end, the two +fills against real mail, whether the initials are legible over an identicon at +the desktop's own font size, and whether the gutter costs too much subject at +depth. + +A rendering probe must not be used to judge any of that, for the reasons +`CLAUDE.md` records under "Rendering probes lie". + +## Risks + +- **The gutter compounds with the indent.** A deep reply already loses + `kMaxDepth * kIndentStep`; it now loses the avatar gutter as well. If it reads + badly, the cap is the knob to turn, not the avatar. +- **An identicon behind two letters is the classic legibility failure.** The + veil exists for exactly that and its opacity is a value to tune against the + real font, not to fix on the first guess. +- **`firstMessageSender` is a new field on a struct crossing the thread + boundary.** It is a plain `QString` and crosses like the rest, so there is no + ownership question, but the walk that fills it must finish while the + `NmThread` is alive, exactly as the surrounding code already documents. +- **The candidate word list is a guess.** It will miss senders and propose + wrong ones, which is precisely why nothing it writes takes effect until the + user uncomments it. |