diff options
Diffstat (limited to 'docs/superpowers')
4 files changed, 2534 insertions, 67 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 1208c42..39026cb 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 @@ -8491,3 +8491,119 @@ 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. 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 437ceda..5e95804 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` | @@ -242,6 +242,8 @@ taking that too literally. | 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. @@ -365,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 @@ -1347,3 +1314,96 @@ id and a draft of a reply carries both. - 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. + +**Constraints.** + +- 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.** + +- 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. |