summaryrefslogtreecommitdiffstats
path: root/docs/superpowers
diff options
context:
space:
mode:
Diffstat (limited to 'docs/superpowers')
-rw-r--r--docs/superpowers/plans/2026-08-09-card-list.md1958
1 files changed, 1958 insertions, 0 deletions
diff --git a/docs/superpowers/plans/2026-08-09-card-list.md b/docs/superpowers/plans/2026-08-09-card-list.md
new file mode 100644
index 0000000..582d8b9
--- /dev/null
+++ b/docs/superpowers/plans/2026-08-09-card-list.md
@@ -0,0 +1,1958 @@
+# Card List 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:** Replace the thread pane's five-column grid with a single-column list of three-line cards, so threads and replies read as a conversation rather than as a table of records.
+
+**Architecture:** The model collapses from five columns to one and keeps every existing role, gaining one pair for a reply's own tags. A new `CardDelegate` paints the entire card, replacing `SubjectDelegate` and taking over the tag chips that `ThreadListView::paintEvent` draws today; that `paintEvent` and its band arithmetic are deleted, leaving the view owning only the expander hit-test. Navigation moves off row-number arithmetic onto `indexBelow`/`indexAbove`.
+
+**Tech Stack:** C++17, Qt 6.11 (Widgets), CMake + Ninja, QtTest. No new dependencies.
+
+**Spec:** `docs/superpowers/specs/2026-08-09-card-list-design.md`
+
+---
+
+## Before you start
+
+**Read these first.** This plan assumes you have read the spec above and the
+"Architecture" and "Rendering probes lie" sections of `CLAUDE.md`. Several steps
+below will look like busywork if you have not.
+
+**Branch.** This work continues `item-20-message-rows`, which carries the tree
+model, the worker's reply walk, and `MessageNode`. All of that is kept. The
+branch is 15 commits behind `master`, so Task 0 rebases it first.
+
+**Build and test:**
+
+```bash
+cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Debug
+cmake --build build
+ctest --test-dir build --output-on-failure
+```
+
+Single test binary, for a tighter loop: `./build/tests/test_threadlistmodel`.
+
+**Two rules from `CLAUDE.md` that this plan leans on constantly:**
+
+1. **Nothing may be keyed on a row NUMBER.** A tree numbers rows per parent, so
+ `index.row()` is a position among siblings, not a position in the list.
+2. **Never count lit pixels to verify rendering.** Antialiasing makes bold and
+ regular light a similar number either way. Assert on computed geometry
+ instead, which is why Task 4 exists before any painting is written.
+
+**Commit style.** GPG-signed (`git commit -S`), on the branch, no PR flow.
+
+---
+
+## File Structure
+
+| File | Change | Responsibility after this plan |
+|---|---|---|
+| `src/types.h` | none | `ThreadSummary`, `MessageNode` unchanged. `MessageNode::tags` already exists and is what the set difference reads. |
+| `src/threadlistmodel.h` | modify | One column. Adds `MessageOwnTagsRole`, `MessageOwnColoursRole`. Drops the five `Column` enumerators. |
+| `src/threadlistmodel.cpp` | modify | `data()` answers one column; `headerData` gone. Computes the tag set difference. |
+| `src/cardlayout.h` | **create** | Pure geometry: given a row's properties and a width, returns every rect on the card. No painting, no Qt widgets. This is what the tests assert on. |
+| `src/cardlayout.cpp` | **create** | Implementation of the above. |
+| `src/carddelegate.h` | **create** | `CardDelegate`, painting a card from a `CardLayout`. Inherits `RowStyleDelegate`. |
+| `src/carddelegate.cpp` | **create** | Implementation. |
+| `src/tagchip.h` | modify | Keeps the `TagChip` namespace and `RowStyleDelegate`. `SubjectDelegate` deleted. |
+| `src/tagchip.cpp` | modify | Same. |
+| `src/threadlistview.h` | modify | Loses `paintEvent`. Keeps `mousePressEvent`. |
+| `src/threadlistview.cpp` | modify | Same. ~260 lines down to ~60. |
+| `src/mainwindow.cpp` | modify | View setup for one column; sort dropdown; navigation rewrite. |
+| `src/notmuchworker.h/.cpp` | modify | `runQuery` takes a sort order. |
+| `tests/test_cardlayout.cpp` | **create** | Geometry assertions. The bulk of the new tests. |
+
+**Why `CardLayout` is a separate file from `CardDelegate`.** A delegate needs a
+`QPainter` and a live view to do anything, which is exactly what makes delegate
+tests fragile: `CLAUDE.md` records that `viewport()->render()` returns a blank
+image in several ordinary situations and that a probe reporting "no ink" is more
+likely broken than the code. Splitting the arithmetic out gives every geometric
+claim a test that needs no painting at all.
+
+---
+
+## Task 0: Rebase the branch onto master
+
+**Files:** no source changes of your own; you are resolving other people's.
+
+- [ ] **Step 1: Confirm where you are**
+
+```bash
+git checkout item-20-message-rows
+git log --oneline -1
+```
+
+Expected: `029a50e docs: record message rows, and the user's verdict on them`
+
+- [ ] **Step 2: See what is coming**
+
+```bash
+git log --oneline item-20-message-rows..master
+```
+
+Expected: 15 commits, including `751ca62 fix(ui): stop a restored splitter
+position collapsing the message pane` and `99709c6 fix(config): report an
+out-of-range message_zoom`.
+
+- [ ] **Step 3: Rebase**
+
+```bash
+git rebase master
+```
+
+Eight files conflict. Resolve as follows:
+
+- `CHANGELOG.md`, `CLAUDE.md`, `docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md`:
+ **take master's version entirely** (`git checkout --theirs <file>` during the
+ rebase), then re-apply nothing. Master's copies already describe the branch,
+ because `69281b2` carried those entries across deliberately.
+- `docs/superpowers/plans/2026-08-08-item-20-message-rows.md`: keep the
+ branch's version.
+- `src/mainwindow.cpp`, `src/mainwindow.h`: master added `setMinimumWidth(300)`
+ and `setCollapsible(1, false)` on the splitter (item 55) and an
+ out-of-range zoom warning. **Both must survive.** The branch's changes here
+ are the view setup; they do not overlap the splitter lines.
+- `src/threadlistmodel.cpp`, `tests/test_mainwindow.cpp`: take both sides;
+ master's changes are the icon work and the splitter test.
+
+- [ ] **Step 4: Verify the rebase kept master's fixes**
+
+```bash
+grep -n "setMinimumWidth(300)\|setCollapsible(1, false)" src/mainwindow.cpp
+```
+
+Expected: both present. If either is missing you dropped an item-55 hunk;
+redo the conflict resolution rather than patching it back by hand.
+
+- [ ] **Step 5: Build and run the whole suite**
+
+```bash
+cmake --build build && ctest --test-dir build --output-on-failure
+```
+
+Expected: all tests pass. If `test_mainwindow` fails here, fix it before any
+new work: you are now debugging a merge, and every later task would be
+debugging a merge plus a feature.
+
+- [ ] **Step 6: Commit**
+
+The rebase commits itself. Verify signatures survived:
+
+```bash
+git log --format='%h %G? %s' -5
+```
+
+Expected: `G` in every second column. If any say `N`, re-sign:
+`git rebase --exec 'git commit --amend --no-edit -S' master`
+
+---
+
+## Task 1: The model's own tags for a reply
+
+The set difference from the spec: a reply card shows only the tags its message
+has and its thread does not. Model-side only, no worker change, because
+`MessageNode::tags` already carries the message's tags.
+
+**Files:**
+- Modify: `src/threadlistmodel.h` (the `Role` enum)
+- Modify: `src/threadlistmodel.cpp` (the `isMessageRow` branch of `data()`)
+- Test: `tests/test_threadlistmodel.cpp`
+
+- [ ] **Step 1: Write the failing test**
+
+Add to `tests/test_threadlistmodel.cpp`. Declare both in the `private slots:`
+block at the top of the class, then add the bodies:
+
+```cpp
+void TestThreadListModel::replyShowsOnlyItsOwnTags()
+{
+ ThreadListModel model;
+ ThreadSummary thread;
+ thread.threadId = QStringLiteral("T1");
+ thread.subject = QStringLiteral("Build fails");
+ thread.totalCount = 2;
+ thread.tags = { QStringLiteral("inbox"), QStringLiteral("work") };
+ model.appendBatch({ thread });
+
+ MessageNode reply;
+ reply.messageId = QStringLiteral("M2");
+ reply.threadId = QStringLiteral("T1");
+ reply.from = QStringLiteral("bob@example.org");
+ reply.depth = 1;
+ // Two the thread already has, one it does not.
+ reply.tags = { QStringLiteral("inbox"), QStringLiteral("work"),
+ QStringLiteral("todo") };
+ model.setReplies(QStringLiteral("T1"), { reply });
+
+ const QModelIndex threadIndex = model.index(0, 0);
+ QVERIFY(model.hasChildren(threadIndex));
+ const QModelIndex replyIndex = model.index(0, 0, threadIndex);
+ QVERIFY(replyIndex.isValid());
+
+ const QStringList own =
+ replyIndex.data(ThreadListModel::MessageOwnTagsRole).toStringList();
+ QCOMPARE(own, QStringList{ QStringLiteral("todo") });
+}
+
+void TestThreadListModel::replySharingEveryThreadTagShowsNone()
+{
+ ThreadListModel model;
+ ThreadSummary thread;
+ thread.threadId = QStringLiteral("T1");
+ thread.totalCount = 2;
+ thread.tags = { QStringLiteral("inbox"), QStringLiteral("work") };
+ model.appendBatch({ thread });
+
+ MessageNode reply;
+ reply.messageId = QStringLiteral("M2");
+ reply.threadId = QStringLiteral("T1");
+ reply.depth = 1;
+ reply.tags = { QStringLiteral("inbox"), QStringLiteral("work") };
+ model.setReplies(QStringLiteral("T1"), { reply });
+
+ const QModelIndex replyIndex = model.index(0, 0, model.index(0, 0));
+ QVERIFY(replyIndex.data(ThreadListModel::MessageOwnTagsRole)
+ .toStringList()
+ .isEmpty());
+}
+```
+
+**Check the method name first.** This plan calls the reply-filling method
+`setReplies`; the branch may name it differently. Run
+`grep -n "void set.*Replies\|void.*[Rr]eplies" src/threadlistmodel.h` and use
+whatever is there rather than renaming it.
+
+- [ ] **Step 2: Run it and watch it fail**
+
+```bash
+cmake --build build && ./build/tests/test_threadlistmodel
+```
+
+Expected: compile error, `MessageOwnTagsRole` is not a member. That is the
+correct first failure.
+
+- [ ] **Step 3: Add the roles**
+
+In `src/threadlistmodel.h`, at the END of the `Role` enum, after
+`HasRepliesRole`:
+
+```cpp
+ /// The tags this MESSAGE carries that its thread does not.
+ ///
+ /// A reply card shows these and nothing else. Showing a reply's full
+ /// tag set instead was measured against the user's own database and
+ /// rejected: of 48691 messages, 7 carry `unread` and 75 carry
+ /// `flagged`, and both are already drawn another way (the sender's
+ /// weight, and the mark on line 2). Every other tag is applied to a
+ /// whole thread and is identical on all its messages, so full sets
+ /// would repeat the thread's own chips down the entire expansion,
+ /// which is the striping the old row-wide strip existed to avoid.
+ ///
+ /// Empty on a thread row, which has no thread to differ from.
+ MessageOwnTagsRole,
+
+ /// The colours for MessageOwnTagsRole, in the same order. Supplied by
+ /// the model for the same reason as PillColoursRole: it owns the
+ /// TagColors instance, and a delegate reading config itself would be a
+ /// second source of truth.
+ MessageOwnColoursRole,
+```
+
+- [ ] **Step 4: Implement the difference**
+
+In `src/threadlistmodel.cpp`, in the message-row branch of `data()`, beside the
+existing `case PillTagsRole:`:
+
+```cpp
+ case MessageOwnTagsRole: {
+ // Set difference against the parent thread, not against a global
+ // list: "own" means "not already said by the card above this one".
+ const ThreadSummary *thread = threadFor(node.threadId);
+ const QStringList threadTags = thread ? thread->tags : QStringList();
+ QStringList own;
+ for (const QString &tag : node.tags) {
+ if (!threadTags.contains(tag))
+ own.append(tag);
+ }
+ // Sorted, so a reply does not reshuffle its own chips between
+ // repaints, matching what PillTagsRole already guarantees.
+ own.sort();
+ return own;
+ }
+ case MessageOwnColoursRole: {
+ const QStringList own =
+ data(index, MessageOwnTagsRole).toStringList();
+ QVariantList colours;
+ for (const QString &tag : own)
+ colours.append(pillColourFor(tag));
+ return colours;
+ }
+```
+
+`threadFor` and `pillColourFor` are helpers the branch may or may not have under
+those names. Check with `grep -n "threadFor\|pillColourFor\|ThreadSummary \*"
+src/threadlistmodel.h src/threadlistmodel.cpp`. If the colour lookup does not
+exist as a helper, copy whatever `case PillColoursRole:` does today.
+
+Also add, in the THREAD-row branch, so a thread card never draws these:
+
+```cpp
+ case MessageOwnTagsRole:
+ return QStringList();
+ case MessageOwnColoursRole:
+ return QVariantList();
+```
+
+- [ ] **Step 5: Run the tests**
+
+```bash
+cmake --build build && ./build/tests/test_threadlistmodel
+```
+
+Expected: PASS, both new tests.
+
+- [ ] **Step 6: Mutation-check it**
+
+Temporarily change `if (!threadTags.contains(tag))` to `if (true)`. Rerun.
+Expected: `replyShowsOnlyItsOwnTags` FAILS (it would report all three tags) and
+`replySharingEveryThreadTagShowsNone` FAILS. **Revert the mutation.** If either
+still passed, the test is not testing what it claims.
+
+- [ ] **Step 7: Commit**
+
+```bash
+git add src/threadlistmodel.h src/threadlistmodel.cpp tests/test_threadlistmodel.cpp
+git commit -S -m "feat(model): expose the tags a reply has and its thread does not
+
+A reply card shows only these. The alternative, a reply's full tag set, was
+rejected on measurement rather than taste: in the user's database 7 of 48691
+messages carry unread and 75 carry flagged, both already drawn another way, and
+every other tag is applied per thread and identical on all its messages. Full
+sets would repeat the thread's chips down the whole expansion, which is the
+striping the row-wide strip was built to avoid."
+```
+
+---
+
+## Task 2: Collapse the model to one column
+
+**Files:**
+- Modify: `src/threadlistmodel.h`, `src/threadlistmodel.cpp`
+- Test: `tests/test_threadlistmodel.cpp`
+
+This task deliberately breaks the build in several places. That is expected: the
+view and delegate still reference the old columns and are fixed in Tasks 5-6.
+Work through the compiler errors.
+
+- [ ] **Step 1: Write the failing test**
+
+```cpp
+void TestThreadListModel::modelHasOneColumn()
+{
+ ThreadListModel model;
+ ThreadSummary thread;
+ thread.threadId = QStringLiteral("T1");
+ thread.subject = QStringLiteral("Build fails");
+ thread.authors = QStringLiteral("alice@example.org");
+ thread.totalCount = 1;
+ model.appendBatch({ thread });
+
+ QCOMPARE(model.columnCount(), 1);
+
+ // Every field the five columns used to answer is still reachable, by role
+ // rather than by column, because the card draws them all.
+ const QModelIndex index = model.index(0, 0);
+ QCOMPARE(index.data(ThreadListModel::SubjectRole).toString(),
+ QStringLiteral("Build fails"));
+ QCOMPARE(index.data(ThreadListModel::SendersRole).toString(),
+ QStringLiteral("alice@example.org"));
+ QVERIFY(index.data(ThreadListModel::DateRole).isValid());
+}
+```
+
+- [ ] **Step 2: Run it and watch it fail**
+
+```bash
+cmake --build build && ./build/tests/test_threadlistmodel
+```
+
+Expected: compile error on `SubjectRole`.
+
+- [ ] **Step 3: Replace the Column enum with per-field roles**
+
+In `src/threadlistmodel.h`, DELETE the whole `Column` enum:
+
+```cpp
+ // DELETE THIS ENTIRE BLOCK
+ enum Column {
+ AttachmentColumn = 0,
+ FlagColumn,
+ DateColumn,
+ AuthorsColumn,
+ SubjectColumn,
+ ColumnCount,
+ };
+```
+
+Add to the END of the `Role` enum:
+
+```cpp
+ /// The card's own fields, by role rather than by column.
+ ///
+ /// Five columns used to answer these through Qt::DisplayRole. One
+ /// column cannot, and a card needs all five values at once, so each
+ /// gets a role and Qt::DisplayRole answers the subject alone (which is
+ /// what keyboard search and accessibility read).
+ SubjectRole,
+ SendersRole,
+ DateRole, ///< A QDateTime. The delegate formats it.
+ HasAttachmentRole, ///< bool
+ IsFlaggedRole, ///< bool
+ ReplyCountRole, ///< int; 0 when a thread has no replies.
+```
+
+- [ ] **Step 4: Make columnCount answer 1**
+
+In `src/threadlistmodel.cpp`:
+
+```cpp
+int ThreadListModel::columnCount(const QModelIndex &parent) const
+{
+ Q_UNUSED(parent);
+ // One column: the card is drawn whole by CardDelegate. The five-column
+ // grid is what item 53 removed.
+ return 1;
+}
+```
+
+- [ ] **Step 5: Rewrite data() to answer by role**
+
+In the THREAD-row branch of `data()`, replace the `case Qt::DisplayRole:` switch
+over columns with:
+
+```cpp
+ case Qt::DisplayRole:
+ case SubjectRole:
+ return summary.subject;
+ case SendersRole:
+ return summary.authors;
+ case DateRole:
+ return summary.date;
+ case HasAttachmentRole:
+ return summary.hasAttachment();
+ case IsFlaggedRole:
+ return summary.isFlagged();
+ case ReplyCountRole:
+ // totalCount includes the root message, which is the card itself.
+ return qMax(0, summary.totalCount - 1);
+```
+
+In the MESSAGE-row branch, the same shape:
+
+```cpp
+ case Qt::DisplayRole:
+ case SubjectRole:
+ return node.subject;
+ case SendersRole:
+ return node.from;
+ case DateRole:
+ return node.date;
+ case HasAttachmentRole:
+ return node.hasAttachment();
+ case IsFlaggedRole:
+ return node.isFlagged();
+ case ReplyCountRole:
+ // A reply never offers an expander: nesting past the first level is
+ // drawn from depth, not from further parent-child structure.
+ return 0;
+```
+
+Delete `headerData` entirely, and its declaration in the header.
+
+- [ ] **Step 6: Run the model tests**
+
+```bash
+cmake --build build 2>&1 | head -40
+```
+
+The library will not link yet: `mainwindow.cpp`, `threadlistview.cpp` and
+`tagchip.cpp` still name the deleted columns. **That is expected.** To run this
+task's test now, build only the model test target if your generator allows it;
+otherwise accept a red build and verify at the end of Task 6.
+
+- [ ] **Step 7: Commit (a knowingly broken build)**
+
+```bash
+git add src/threadlistmodel.h src/threadlistmodel.cpp tests/test_threadlistmodel.cpp
+git commit -S -m "refactor(model): collapse the thread list to a single column
+
+Five columns answered through Qt::DisplayRole; one column cannot, and a card
+needs every field at once, so each gets its own role. Qt::DisplayRole keeps
+answering the subject, which is what keyboard search and accessibility read.
+
+The build is red at this commit: the view and the delegates still name the
+deleted Column enumerators and are rewritten in the commits that follow."
+```
+
+---
+
+## Task 3: Sort order through the worker
+
+Independent of the painting work. Doing it here keeps it out of the way of the
+delegate tasks.
+
+**Files:**
+- Modify: `src/notmuchworker.h`, `src/notmuchworker.cpp:135`
+- Test: `tests/test_notmuchworker.cpp`
+
+- [ ] **Step 1: Write the failing test**
+
+```cpp
+void TestNotmuchWorker::oldestFirstReversesTheOrder()
+{
+ // The fixture database has at least two threads with different dates.
+ NotmuchWorker worker;
+ worker.openDatabase(m_configPath);
+
+ QVector<ThreadSummary> newest;
+ QVector<ThreadSummary> oldest;
+
+ connect(&worker, &NotmuchWorker::threadsReady, this,
+ [&](const QVector<ThreadSummary> &batch, quint64, bool) {
+ newest.append(batch);
+ });
+ worker.runQuery(QStringLiteral("*"), 1, NotmuchWorker::NewestFirst);
+ QTRY_VERIFY(newest.size() >= 2);
+
+ disconnect(&worker, nullptr, this, nullptr);
+ connect(&worker, &NotmuchWorker::threadsReady, this,
+ [&](const QVector<ThreadSummary> &batch, quint64, bool) {
+ oldest.append(batch);
+ });
+ worker.runQuery(QStringLiteral("*"), 2, NotmuchWorker::OldestFirst);
+ QTRY_VERIFY(oldest.size() >= 2);
+
+ QCOMPARE(oldest.size(), newest.size());
+ QCOMPARE(oldest.first().threadId, newest.last().threadId);
+ QCOMPARE(oldest.last().threadId, newest.first().threadId);
+}
+```
+
+**Match the existing signatures.** `runQuery` and `threadsReady` already exist
+with a particular shape on the branch. Run
+`grep -n "runQuery\|threadsReady" src/notmuchworker.h` and adapt the test to
+what is there, adding only the sort parameter.
+
+- [ ] **Step 2: Run it and watch it fail**
+
+```bash
+cmake --build build && ./build/tests/test_notmuchworker
+```
+
+Expected: compile error, `runQuery` takes no such argument.
+
+- [ ] **Step 3: Add the enum and the parameter**
+
+In `src/notmuchworker.h`, inside the class:
+
+```cpp
+ /// The sort orders offered to the user.
+ ///
+ /// Two, not four. notmuch also has NOTMUCH_SORT_MESSAGE_ID and
+ /// NOTMUCH_SORT_UNSORTED, and neither is an order a human wants. Sorting
+ /// by sender or subject is deliberately absent: notmuch cannot do it, so
+ /// the model would have to sort after results arrive, which fights the
+ /// batching that makes a 10k-thread query paint immediately.
+ enum SortOrder {
+ NewestFirst,
+ OldestFirst,
+ };
+ Q_ENUM(SortOrder)
+```
+
+Change `runQuery`'s declaration to take `SortOrder sort = NewestFirst`.
+
+- [ ] **Step 4: Use it**
+
+In `src/notmuchworker.cpp`, replace the hardcoded line 135:
+
+```cpp
+ notmuch_query_set_sort(nmQuery.get(),
+ sort == OldestFirst ? NOTMUCH_SORT_OLDEST_FIRST
+ : NOTMUCH_SORT_NEWEST_FIRST);
+```
+
+Leave `loadThread`'s `NOTMUCH_SORT_OLDEST_FIRST` (line ~216) alone: a thread's
+messages read in chronological order regardless of how the list is sorted.
+
+- [ ] **Step 5: Run the test**
+
+```bash
+cmake --build build && ./build/tests/test_notmuchworker
+```
+
+Expected: PASS.
+
+- [ ] **Step 6: Mutation-check**
+
+Change the ternary to always pass `NOTMUCH_SORT_NEWEST_FIRST`. Rerun. Expected:
+FAIL. **Revert.**
+
+- [ ] **Step 7: Commit**
+
+```bash
+git add src/notmuchworker.h src/notmuchworker.cpp tests/test_notmuchworker.cpp
+git commit -S -m "feat(worker): let a query choose newest or oldest first
+
+Sorting was hardcoded NEWEST_FIRST. Two orders only: notmuch's other two are
+MESSAGE_ID and UNSORTED, neither of which is an order a human wants, and sorting
+by sender or subject would have to happen in the model after results arrive,
+which fights the batching that makes a large query paint immediately.
+
+loadThread keeps OLDEST_FIRST unconditionally: a thread reads chronologically
+whichever way the list is sorted."
+```
+
+---
+
+## Task 4: `CardLayout`, the geometry with no painting
+
+**This is the most important task in the plan.** Everything the delegate draws
+is positioned by this file, and because it touches no `QPainter` and no widget,
+every claim about the card's shape gets a test that cannot be defeated by a
+blank render.
+
+**Files:**
+- Create: `src/cardlayout.h`, `src/cardlayout.cpp`
+- Create: `tests/test_cardlayout.cpp`
+- Modify: `src/CMakeLists.txt`, `tests/CMakeLists.txt`
+
+- [ ] **Step 1: Register the new files**
+
+In `src/CMakeLists.txt`, add `cardlayout.cpp` to the `qtmaildir_lib` source
+list (alphabetically, after `cidschemehandler.cpp`).
+
+In `tests/CMakeLists.txt`, add:
+
+```cmake
+add_qtmaildir_test(cardlayout)
+```
+
+- [ ] **Step 2: Write the failing test**
+
+Create `tests/test_cardlayout.cpp`:
+
+```cpp
+/*
+ * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs
+ * Copyright (C) 2026 Danilo M. <danix@danix.xyz>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include "cardlayout.h"
+
+#include <QFont>
+#include <QTest>
+
+class TestCardLayout : public QObject
+{
+ Q_OBJECT
+
+private slots:
+ void everyCardIsTheSameHeight();
+ void threeLinesStackWithoutOverlapping();
+ void replyIndentsByDepth();
+ void indentStopsAtTheCap();
+ void expanderSitsOnTheSecondLine();
+ void expanderIsEmptyWithoutReplies();
+ void dateIsFlushRight();
+};
+
+namespace {
+
+CardLayout::Input threadInput()
+{
+ CardLayout::Input in;
+ in.isMessage = false;
+ in.depth = 0;
+ in.replyCount = 3;
+ return in;
+}
+
+CardLayout::Input replyInput(int depth)
+{
+ CardLayout::Input in;
+ in.isMessage = true;
+ in.depth = depth;
+ in.replyCount = 0;
+ return in;
+}
+
+} // namespace
+
+void TestCardLayout::everyCardIsTheSameHeight()
+{
+ const QFont font;
+ const int thread = CardLayout::heightFor(font);
+
+ // The uniform height is the whole reason setUniformRowHeights(true)
+ // survives this design, so it is asserted directly rather than inferred
+ // from two cards happening to look alike.
+ //
+ // Note what is NOT varied here: the tag list. CardLayout reserves line 3
+ // unconditionally and never sees the tags, which is exactly the property
+ // being asserted. A version of this test that passed a tag list in would
+ // be testing a parameter that does not exist.
+ const CardLayout threadCard =
+ CardLayout::compute(threadInput(), QRect(0, 0, 400, thread), font);
+ const CardLayout deepReply =
+ CardLayout::compute(replyInput(3), QRect(0, 0, 400, thread), font);
+ CardLayout::Input noRepliesIn = threadInput();
+ noRepliesIn.replyCount = 0;
+ const CardLayout noReplies =
+ CardLayout::compute(noRepliesIn, QRect(0, 0, 400, thread), font);
+
+ QCOMPARE(threadCard.totalHeight, thread);
+ QCOMPARE(deepReply.totalHeight, thread);
+ QCOMPARE(noReplies.totalHeight, thread);
+
+ // The third line exists on every card, including one with nothing to put
+ // there. That blank band is the cost the uniform height was bought with.
+ QCOMPARE(noReplies.tagRect.height(), threadCard.tagRect.height());
+}
+
+void TestCardLayout::threeLinesStackWithoutOverlapping()
+{
+ const QFont font;
+ const int h = CardLayout::heightFor(font);
+ const CardLayout card =
+ CardLayout::compute(threadInput(), QRect(0, 0, 400, h), font);
+
+ QVERIFY(card.senderRect.height() > 0);
+ QVERIFY(card.subjectRect.height() > 0);
+ QVERIFY(card.tagRect.height() > 0);
+
+ // Guard: these must actually be three stacked bands. A layout that
+ // collapsed them all to the same rect would satisfy any assertion that
+ // only checked they exist.
+ QVERIFY(card.senderRect.bottom() <= card.subjectRect.top());
+ QVERIFY(card.subjectRect.bottom() <= card.tagRect.top());
+ QVERIFY(card.tagRect.bottom() <= h);
+}
+
+void TestCardLayout::replyIndentsByDepth()
+{
+ const QFont font;
+ const int h = CardLayout::heightFor(font);
+ const QRect rect(0, 0, 400, h);
+
+ const CardLayout root = CardLayout::compute(threadInput(), rect, font);
+ const CardLayout d1 = CardLayout::compute(replyInput(1), rect, font);
+ const CardLayout d2 = CardLayout::compute(replyInput(2), rect, font);
+
+ QVERIFY(d1.contentLeft > root.contentLeft);
+ QVERIFY(d2.contentLeft > d1.contentLeft);
+
+ // One spine per depth level, so the count is the depth itself.
+ QCOMPARE(root.spines.size(), 0);
+ QCOMPARE(d1.spines.size(), 1);
+ QCOMPARE(d2.spines.size(), 2);
+
+ // Each spine runs the full height of the card, which is what makes an
+ // expansion read as one continuous block rather than as dashes.
+ for (const QRect &spine : d2.spines) {
+ QCOMPARE(spine.top(), rect.top());
+ QCOMPARE(spine.bottom(), rect.bottom());
+ }
+}
+
+void TestCardLayout::indentStopsAtTheCap()
+{
+ const QFont font;
+ const int h = CardLayout::heightFor(font);
+ const QRect rect(0, 0, 400, h);
+
+ const CardLayout d4 = CardLayout::compute(replyInput(4), rect, font);
+ const CardLayout d5 = CardLayout::compute(replyInput(5), rect, font);
+ const CardLayout d9 = CardLayout::compute(replyInput(9), rect, font);
+
+ QCOMPARE(d5.contentLeft, d4.contentLeft);
+ QCOMPARE(d9.contentLeft, d4.contentLeft);
+ QCOMPARE(d5.spines.size(), d4.spines.size());
+ QCOMPARE(d9.spines.size(), d4.spines.size());
+
+ // Guard: the cap must not be so low that it has already bitten at depth 3,
+ // which would make the three assertions above true for the wrong reason.
+ const CardLayout d3 = CardLayout::compute(replyInput(3), rect, font);
+ QVERIFY(d3.contentLeft < d4.contentLeft);
+}
+
+void TestCardLayout::expanderSitsOnTheSecondLine()
+{
+ const QFont font;
+ const int h = CardLayout::heightFor(font);
+ const CardLayout card =
+ CardLayout::compute(threadInput(), QRect(0, 0, 400, h), font);
+
+ QVERIFY(!card.expanderRect.isEmpty());
+ // It is the reply count, so it belongs on the line the reply count is on.
+ QVERIFY(card.expanderRect.top() >= card.subjectRect.top());
+ QVERIFY(card.expanderRect.bottom() <= card.subjectRect.bottom());
+ // And it is on the right, where the count is drawn, not in a left gutter.
+ QVERIFY(card.expanderRect.left() > 400 / 2);
+}
+
+void TestCardLayout::expanderIsEmptyWithoutReplies()
+{
+ const QFont font;
+ const int h = CardLayout::heightFor(font);
+ CardLayout::Input in = threadInput();
+ in.replyCount = 0;
+
+ const CardLayout card = CardLayout::compute(in, QRect(0, 0, 400, h), font);
+ QVERIFY(card.expanderRect.isEmpty());
+}
+
+void TestCardLayout::dateIsFlushRight()
+{
+ const QFont font;
+ const int h = CardLayout::heightFor(font);
+ const QRect rect(0, 0, 400, h);
+ const CardLayout card = CardLayout::compute(threadInput(), rect, font);
+
+ QCOMPARE(card.dateRect.right(), rect.right() - CardLayout::kPaddingX);
+ // The sender must stop before the date starts, or a long sender overwrites
+ // it. This is the assertion that fails if the two are laid out
+ // independently.
+ QVERIFY(card.senderRect.right() <= card.dateRect.left());
+}
+
+QTEST_MAIN(TestCardLayout)
+#include "test_cardlayout.moc"
+```
+
+- [ ] **Step 3: Run it and watch it fail**
+
+```bash
+cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Debug
+cmake --build build 2>&1 | head -20
+```
+
+Expected: `cardlayout.h: No such file or directory`.
+
+- [ ] **Step 4: Write the header**
+
+Create `src/cardlayout.h` (GPL header as in every other file, then):
+
+```cpp
+#pragma once
+
+#include <QFont>
+#include <QRect>
+#include <QVector>
+
+/// Where everything on a card goes, with no painting and no widget.
+///
+/// Split out from CardDelegate on purpose. A delegate needs a live QPainter and
+/// an exposed view before it draws anything, which is what makes delegate tests
+/// fragile: CLAUDE.md records that viewport()->render() returns a blank image in
+/// several ordinary situations, and that a probe reporting "no ink anywhere" is
+/// far more likely broken than the code it is testing. Every geometric claim
+/// about a card is therefore made here, where a test is a function call.
+///
+/// The card is three lines, always:
+///
+/// sender ................................ date <- senderRect/dateRect
+/// * subject @ v 3 replies <- subjectRect/expanderRect
+/// [tag] [tag] <- tagRect
+struct CardLayout
+{
+ /// What the model says about the row. Deliberately plain data: the layout
+ /// must be computable in a test without a model or a view.
+ struct Input
+ {
+ bool isMessage = false;
+ int depth = 0; ///< 0 for a thread root, 1 for a direct reply.
+ int replyCount = 0; ///< 0 means no expander.
+ };
+
+ /// Horizontal breathing room at the card's edges.
+ static constexpr int kPaddingX = 8;
+
+ /// Vertical breathing room above the first line and below the last.
+ static constexpr int kPaddingY = 4;
+
+ /// How far one level of reply nesting indents.
+ static constexpr int kIndentStep = 18;
+
+ /// The depth past which nothing indents further.
+ ///
+ /// A mailing-list chain can nest a dozen deep, and without a cap the
+ /// sender is eventually pushed off the right edge. Item 20 accepted that
+ /// deep chains must be capped in the VIEW rather than flattened in the
+ /// model, and this is that cap. Rows past it draw at this depth's indent
+ /// with no marker saying so.
+ static constexpr int kMaxDepth = 4;
+
+ QRect senderRect;
+ QRect dateRect;
+ QRect subjectRect;
+ QRect tagRect;
+
+ /// The reply count's rect, and the click target that toggles the thread.
+ /// Empty when the row has no replies.
+ QRect expanderRect;
+
+ /// One full-height vertical line per depth level, outermost first.
+ QVector<QRect> spines;
+
+ /// Where the card's text starts, after any indent.
+ int contentLeft = 0;
+
+ int totalHeight = 0;
+
+ /// The height EVERY row gets, thread and reply alike.
+ ///
+ /// Uniform by design: it keeps setUniformRowHeights(true), which is the
+ /// single cheapest property of this layout, since no scrolling or
+ /// hit-testing arithmetic has to account for rows of differing size. The
+ /// cost is a blank third line on a card with no tags, which was accepted
+ /// explicitly.
+ static int heightFor(const QFont &font);
+
+ /// The font the tag chips and the reply count are drawn in: a size down
+ /// from the card's own, so they read as annotation rather than as a third
+ /// column of content.
+ static QFont smallFont(const QFont &cardFont);
+
+ static CardLayout compute(const Input &input, const QRect &rect,
+ const QFont &font);
+};
+```
+
+- [ ] **Step 5: Write the implementation**
+
+Create `src/cardlayout.cpp` (GPL header, then):
+
+```cpp
+#include "cardlayout.h"
+
+#include <QFontMetrics>
+
+QFont CardLayout::smallFont(const QFont &cardFont)
+{
+ QFont small = cardFont;
+ // Derived from the card's font rather than fixed, so it follows the
+ // desktop's font size instead of shrinking to nothing on a large one.
+ small.setPointSizeF(qMax(6.0, cardFont.pointSizeF() - 1.0));
+ return small;
+}
+
+int CardLayout::heightFor(const QFont &font)
+{
+ const QFontMetrics metrics(font);
+ const QFontMetrics smallMetrics(smallFont(font));
+ // Two lines at the card's font, one at the small one, plus the padding
+ // above the first and below the last.
+ return kPaddingY * 2 + metrics.height() * 2 + smallMetrics.height();
+}
+
+CardLayout CardLayout::compute(const Input &input, const QRect &rect,
+ const QFont &font)
+{
+ CardLayout out;
+ const QFontMetrics metrics(font);
+ const QFontMetrics smallMetrics(smallFont(font));
+
+ out.totalHeight = rect.height();
+
+ // Indent, capped. qMin rather than a branch so depth 5 and depth 50 land
+ // in exactly the same place.
+ const int depth = qMin(input.depth, kMaxDepth);
+ const int indent = depth * kIndentStep;
+ out.contentLeft = rect.left() + kPaddingX + indent;
+
+ // One spine per level actually indented, each running the card's full
+ // height so an expansion reads as one continuous block.
+ for (int level = 0; level < depth; ++level) {
+ const int x = rect.left() + kPaddingX + level * kIndentStep
+ + kIndentStep / 2;
+ out.spines.append(QRect(x, rect.top(), 2, rect.height()));
+ }
+
+ const int right = rect.right() - kPaddingX;
+ const int lineOneTop = rect.top() + kPaddingY;
+ const int lineTwoTop = lineOneTop + metrics.height();
+ const int lineThreeTop = lineTwoTop + metrics.height();
+
+ // The date is measured first and the sender gets what is left, so a long
+ // sender is elided rather than painting over the date.
+ const int dateWidth = metrics.horizontalAdvance(
+ QStringLiteral("8888-88-88 88:88"));
+ out.dateRect = QRect(right - dateWidth, lineOneTop, dateWidth,
+ metrics.height());
+ out.senderRect = QRect(out.contentLeft, lineOneTop,
+ qMax(0, out.dateRect.left() - out.contentLeft
+ - kPaddingX),
+ metrics.height());
+
+ // The expander is the reply count, on line two and on the right.
+ if (input.replyCount > 0) {
+ const int countWidth = smallMetrics.horizontalAdvance(
+ QStringLiteral("▾ 8888 replies"));
+ out.expanderRect = QRect(right - countWidth, lineTwoTop, countWidth,
+ metrics.height());
+ }
+
+ const int subjectRight = out.expanderRect.isEmpty()
+ ? right
+ : out.expanderRect.left() - kPaddingX;
+ out.subjectRect = QRect(out.contentLeft, lineTwoTop,
+ qMax(0, subjectRight - out.contentLeft),
+ metrics.height());
+
+ out.tagRect = QRect(out.contentLeft, lineThreeTop,
+ qMax(0, right - out.contentLeft),
+ smallMetrics.height());
+
+ return out;
+}
+```
+
+- [ ] **Step 6: Run the tests**
+
+```bash
+cmake --build build && ./build/tests/test_cardlayout
+```
+
+Expected: all 7 PASS.
+
+- [ ] **Step 7: Mutation-check three of them**
+
+Each of these must make its own test fail. Revert after each.
+
+1. Change `qMin(input.depth, kMaxDepth)` to `input.depth`.
+ Expected: `indentStopsAtTheCap` FAILS.
+2. Change `heightFor` to `metrics.height() * 2` (dropping the third line).
+ Expected: `threeLinesStackWithoutOverlapping` FAILS.
+3. Change `out.senderRect`'s width to `right - out.contentLeft`.
+ Expected: `dateIsFlushRight` FAILS.
+
+If any mutation leaves every test green, that test is decorative. Fix it before
+moving on.
+
+- [ ] **Step 8: Commit**
+
+```bash
+git add src/cardlayout.h src/cardlayout.cpp tests/test_cardlayout.cpp src/CMakeLists.txt tests/CMakeLists.txt
+git commit -S -m "feat(view): compute a card's geometry with no painting
+
+Split from the delegate deliberately. A delegate needs a live painter and an
+exposed view, which is what makes delegate tests fragile: viewport()->render()
+returns a blank image in several ordinary situations, and a probe reporting no
+ink is likelier broken than the code it tests. Every geometric claim about a
+card is made here, where a test is a function call.
+
+Three lines at a uniform height, so setUniformRowHeights(true) survives. Indent
+caps at depth 4 with qMin rather than a branch, so depth 5 and depth 50 land in
+the same place. The date is measured before the sender, so a long sender elides
+instead of painting over it."
+```
+
+---
+
+## Task 5: `CardDelegate`
+
+**Files:**
+- Create: `src/carddelegate.h`, `src/carddelegate.cpp`
+- Modify: `src/CMakeLists.txt`
+- Modify: `src/tagchip.h`, `src/tagchip.cpp` (delete `SubjectDelegate`)
+
+- [ ] **Step 1: Register the file**
+
+Add `carddelegate.cpp` to `qtmaildir_lib` in `src/CMakeLists.txt`.
+
+- [ ] **Step 2: Write the header**
+
+Create `src/carddelegate.h` (GPL header, then):
+
+```cpp
+#pragma once
+
+#include "tagchip.h"
+
+class CardLayout;
+
+/// Paints a whole card: three lines, all of it, including the tag chips.
+///
+/// It replaces both SubjectDelegate and ThreadListView::paintEvent. The view
+/// used to paint the tag strip because a delegate cannot paint outside its
+/// column and the strip spanned all five; with one column there is nothing to
+/// span, so the strip comes home to the delegate and the view stops painting
+/// entirely. That removes the two failure modes CLAUDE.md records for the
+/// strip, a deleted row cut in half and every other row showing a bare stripe,
+/// both of which existed because the view had to re-honour alternating
+/// colours, the selection and BackgroundRole across cells it did not own.
+///
+/// Inherits RowStyleDelegate for its one job, which still matters: Qt resolves
+/// Qt::ForegroundRole into the palette's Text roles and then prefers those over
+/// HighlightedText, so the read/unread dimming would otherwise win on a
+/// selected row and land as grey on the highlight colour.
+class CardDelegate : public RowStyleDelegate
+{
+ Q_OBJECT
+public:
+ using RowStyleDelegate::RowStyleDelegate;
+
+ void paint(QPainter *painter, const QStyleOptionViewItem &option,
+ const QModelIndex &index) const override;
+ QSize sizeHint(const QStyleOptionViewItem &option,
+ const QModelIndex &index) const override;
+
+ /// The expander's rect for a row, so the VIEW can hit-test a click without
+ /// duplicating the layout. The delegate draws it and the view owns the
+ /// click, because a delegate gets no click of its own without an editor.
+ static QRect expanderRectFor(const QStyleOptionViewItem &option,
+ const QModelIndex &index);
+};
+```
+
+- [ ] **Step 3: Write the implementation**
+
+Create `src/carddelegate.cpp` (GPL header, then):
+
+```cpp
+#include "carddelegate.h"
+
+#include "cardlayout.h"
+#include "threadlistmodel.h"
+
+#include <QApplication>
+#include <QDateTime>
+#include <QPainter>
+#include <QStyle>
+
+namespace {
+
+CardLayout::Input inputFor(const QModelIndex &index)
+{
+ CardLayout::Input in;
+ in.isMessage = index.data(ThreadListModel::IsMessageRole).toBool();
+ in.depth = index.data(ThreadListModel::MessageDepthRole).toInt();
+ in.replyCount = index.data(ThreadListModel::ReplyCountRole).toInt();
+ return in;
+}
+
+} // namespace
+
+QRect CardDelegate::expanderRectFor(const QStyleOptionViewItem &option,
+ const QModelIndex &index)
+{
+ return CardLayout::compute(inputFor(index), option.rect, option.font)
+ .expanderRect;
+}
+
+QSize CardDelegate::sizeHint(const QStyleOptionViewItem &option,
+ const QModelIndex &index) const
+{
+ Q_UNUSED(index);
+ // One height for every row, thread and reply alike. Asserted directly in
+ // test_cardlayout rather than left to two cards happening to agree.
+ return QSize(option.rect.width(), CardLayout::heightFor(option.font));
+}
+
+void CardDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option,
+ const QModelIndex &index) const
+{
+ // Background, selection and any model fill first, through the style, so a
+ // selected or doomed card looks right before anything is drawn on top.
+ QStyleOptionViewItem chrome = option;
+ initStyleOption(&chrome, index);
+ chrome.text.clear();
+ const QWidget *widget = option.widget;
+ QStyle *style = widget ? widget->style() : QApplication::style();
+ style->drawControl(QStyle::CE_ItemViewItem, &chrome, painter, widget);
+
+ const CardLayout card =
+ CardLayout::compute(inputFor(index), option.rect, option.font);
+
+ painter->save();
+
+ // Spines, under everything else.
+ for (const QRect &spine : card.spines)
+ painter->fillRect(spine, ThreadListModel::threadLineColour());
+
+ // Selection outranks the model's foreground, and the order matters: a read
+ // card carries a dimmed colour blended against the UNSELECTED background,
+ // so over the highlight it lands grey-on-highlight and close to unreadable.
+ const QVariant foreground = index.data(Qt::ForegroundRole);
+ if (option.state & QStyle::State_Selected)
+ painter->setPen(option.palette.highlightedText().color());
+ else if (foreground.isValid())
+ painter->setPen(foreground.value<QBrush>().color());
+ else
+ painter->setPen(option.palette.text().color());
+
+ // The model's font carries bold for unread and strike-out for deleted;
+ // initStyleOption resolved it into chrome.font.
+ painter->setFont(chrome.font);
+ const QFontMetrics metrics(chrome.font);
+
+ // Line 1: sender, then the date flush right.
+ painter->drawText(card.senderRect, Qt::AlignVCenter | Qt::AlignLeft,
+ metrics.elidedText(
+ index.data(ThreadListModel::SendersRole).toString(),
+ Qt::ElideRight, card.senderRect.width()));
+ const QDateTime date =
+ index.data(ThreadListModel::DateRole).toDateTime();
+ painter->drawText(card.dateRect, Qt::AlignVCenter | Qt::AlignRight,
+ date.toString(QStringLiteral("yyyy-MM-dd hh:mm")));
+
+ // Line 2: the flag mark, the subject, the attachment mark.
+ QString subject = index.data(ThreadListModel::SubjectRole).toString();
+ if (index.data(ThreadListModel::IsMessageRole).toBool()) {
+ // Every reply repeating "Re: <the thread's subject>" is the visual
+ // signature of a table of records, which is what item 53 is about.
+ static const QRegularExpression re(
+ QStringLiteral("^\\s*(?:[Rr][Ee]\\s*:\\s*)+"));
+ subject.remove(re);
+ }
+ QString line2;
+ if (index.data(ThreadListModel::IsFlaggedRole).toBool())
+ line2 += ThreadListModel::flagGlyph() + QLatin1Char(' ');
+ line2 += subject;
+ if (index.data(ThreadListModel::HasAttachmentRole).toBool())
+ line2 += QLatin1Char(' ') + ThreadListModel::attachmentGlyph();
+ painter->drawText(card.subjectRect, Qt::AlignVCenter | Qt::AlignLeft,
+ metrics.elidedText(line2, Qt::ElideRight,
+ card.subjectRect.width()));
+
+ // The reply count, which is also the expander.
+ if (!card.expanderRect.isEmpty()) {
+ painter->setFont(CardLayout::smallFont(chrome.font));
+ const int count = index.data(ThreadListModel::ReplyCountRole).toInt();
+ const QString glyph = (option.state & QStyle::State_Open)
+ ? QStringLiteral("▾")
+ : QStringLiteral("▸");
+ painter->drawText(card.expanderRect, Qt::AlignVCenter | Qt::AlignRight,
+ QStringLiteral("%1 %2").arg(glyph).arg(count));
+ painter->setFont(chrome.font);
+ }
+
+ painter->restore();
+
+ // Line 3: the chips. A thread card draws its own tags; a reply draws only
+ // the tags its thread does not already carry, so the thread's chips are
+ // not repeated down the whole expansion.
+ const bool isMessage =
+ index.data(ThreadListModel::IsMessageRole).toBool();
+ const QStringList tags =
+ index.data(isMessage ? ThreadListModel::MessageOwnTagsRole
+ : ThreadListModel::PillTagsRole)
+ .toStringList();
+ const QVariantList colours =
+ index.data(isMessage ? ThreadListModel::MessageOwnColoursRole
+ : ThreadListModel::PillColoursRole)
+ .toList();
+
+ const QFont chipFont = CardLayout::smallFont(chrome.font);
+ const QFontMetrics chipMetrics(chipFont);
+ painter->save();
+ painter->setFont(chipFont);
+ int x = card.tagRect.left();
+ for (int i = 0; i < tags.size(); ++i) {
+ const QSize size = TagChip::sizeFor(chipMetrics, tags.at(i));
+ if (x + size.width() > card.tagRect.right())
+ break; // Out of room; a clipped chip reads as a rendering fault.
+ const QColor colour = i < colours.size()
+ ? colours.at(i).value<QColor>()
+ : QColor(0x55, 0x55, 0x5f);
+ TagChip::paint(painter, QRect(QPoint(x, card.tagRect.top()), size),
+ tags.at(i), colour);
+ x += size.width() + TagChip::kSpacing;
+ }
+ painter->restore();
+}
+```
+
+Add `#include <QRegularExpression>` to the includes.
+
+- [ ] **Step 4: Delete `SubjectDelegate`**
+
+In `src/tagchip.h`, delete the entire `class SubjectDelegate` block (the one
+starting `/// Item delegate for the subject column:`). **Keep the `TagChip`
+namespace and `RowStyleDelegate`**, both of which `CardDelegate` uses.
+
+In `src/tagchip.cpp`, delete every `SubjectDelegate::` member definition.
+
+- [ ] **Step 5: Build**
+
+```bash
+cmake --build build 2>&1 | head -30
+```
+
+Expect errors in `threadlistview.cpp` and `mainwindow.cpp`, which still name
+`SubjectDelegate` and the columns. Task 6 fixes them. Do not patch them here.
+
+- [ ] **Step 6: Commit**
+
+```bash
+git add src/carddelegate.h src/carddelegate.cpp src/tagchip.h src/tagchip.cpp src/CMakeLists.txt
+git commit -S -m "feat(view): paint the whole card in one delegate
+
+Replaces SubjectDelegate. The tag chips come home from the view: the strip was
+painted there only because a delegate cannot paint outside its column and the
+strip spanned all five, and with one column there is nothing to span.
+
+RowStyleDelegate is inherited rather than dropped. Its job survives the
+redesign: Qt resolves ForegroundRole into the palette's Text roles and prefers
+those over HighlightedText, so the read/unread dimming would win on a selected
+row and land as grey on the highlight.
+
+A reply's Re: prefix is stripped here. Every reply repeating the thread's
+subject is the visual signature of a table of records, which is the thing item
+53 is about.
+
+The build is red at this commit; the view and window still name the old
+delegate."
+```
+
+---
+
+## Task 6: Strip the view, wire up the window
+
+**Files:**
+- Modify: `src/threadlistview.h`, `src/threadlistview.cpp`
+- Modify: `src/mainwindow.cpp`
+
+- [ ] **Step 1: Delete the view's painting**
+
+In `src/threadlistview.h`, delete the `paintEvent` declaration and replace the
+class comment (which is now describing something that no longer happens):
+
+```cpp
+/// The thread list.
+///
+/// It exists for ONE reason now: the expander is drawn by CardDelegate, and a
+/// delegate gets no click of its own without an editor, so the view has to own
+/// the hit-test. Everything else it used to do is gone.
+///
+/// Until item 53 this class also painted a row-wide strip of tag chips after
+/// the cells, because a delegate cannot paint outside its column and the strip
+/// spanned all five. With one column and one delegate painting the whole card,
+/// that reason is gone and so is the paintEvent, along with the two faults it
+/// kept producing: a deleted row cut in half, and every other row showing a
+/// bare stripe, both from the view having to re-honour alternating colours,
+/// the selection and BackgroundRole across cells it did not own.
+class ThreadListView : public QTreeView
+{
+ Q_OBJECT
+public:
+ using QTreeView::QTreeView;
+
+protected:
+ void mousePressEvent(QMouseEvent *event) override;
+};
+```
+
+In `src/threadlistview.cpp`, delete `ThreadListView::paintEvent` entirely and
+every include it alone needed (`QPainter`, `QScrollBar`, `tagchip.h` if unused).
+
+- [ ] **Step 2: Rewrite the hit-test**
+
+Replace `ThreadListView::mousePressEvent` with:
+
+```cpp
+void ThreadListView::mousePressEvent(QMouseEvent *event)
+{
+ const QModelIndex index = indexAt(event->pos());
+
+ // The reply count IS the expander. Anything outside its rect selects the
+ // card and opens it, which is what the rest of the card is for.
+ if (event->button() == Qt::LeftButton && index.isValid()
+ && index.data(ThreadListModel::ReplyCountRole).toInt() > 0) {
+
+ QStyleOptionViewItem option;
+ initViewItemOption(&option);
+ option.rect = visualRect(index);
+ // State_Open decides which way the glyph points, and the rect is the
+ // same either way, but pass it so the layout sees the true state.
+ if (isExpanded(index))
+ option.state |= QStyle::State_Open;
+
+ if (CardDelegate::expanderRectFor(option, index)
+ .contains(event->pos())) {
+ setExpanded(index, !isExpanded(index));
+ // Swallowed, so expanding does not also load the thread into the
+ // message pane: it is a request to see the thread's shape, not to
+ // read it.
+ event->accept();
+ return;
+ }
+ }
+
+ QTreeView::mousePressEvent(event);
+}
+```
+
+The old `siblingAtColumn(0)` dance is gone: with one column, `index` already
+is column 0.
+
+- [ ] **Step 3: Update the window's view setup**
+
+In `src/mainwindow.cpp`, find the thread-view setup (around line 548-580 on the
+branch). Replace the per-column delegate installation and header configuration
+with:
+
+```cpp
+ m_threadView->setModel(m_model);
+ m_threadView->setItemDelegate(new CardDelegate(m_threadView));
+ m_threadView->setHeaderHidden(true);
+ m_threadView->setUniformRowHeights(true);
+ m_threadView->setRootIsDecorated(false);
+ m_threadView->setIndentation(0); // CardLayout draws the indent itself.
+ m_threadView->setSelectionBehavior(QAbstractItemView::SelectRows);
+ m_threadView->setSelectionMode(QAbstractItemView::ExtendedSelection);
+ m_threadView->verticalScrollBar()->setSingleStep(
+ CardLayout::heightFor(m_threadView->font()));
+```
+
+Delete every `setColumnWidth`, `header()->` call, and `setHorizontalScrollMode`
+line for this view. Delete `setHorizontalScrollBarPolicy` too: with one column
+there is nothing to scroll to, which is what closes item 51.
+
+Set the row height on the vertical header as the branch already does, using
+`CardLayout::heightFor` in place of `SubjectDelegate::rowHeightFor`.
+
+- [ ] **Step 4: Build clean**
+
+```bash
+cmake --build build 2>&1 | head -30
+```
+
+Expected: no errors. Chase any remaining reference to a deleted column or
+delegate.
+
+- [ ] **Step 5: Run the whole suite**
+
+```bash
+ctest --test-dir build --output-on-failure
+```
+
+`test_mainwindow` will have failures referencing columns. Fix them by reading
+what each asserts: a test naming `SubjectColumn` either moves to a role or is
+deleted if the column was its whole subject.
+
+- [ ] **Step 6: Add the item 51 regression test**
+
+In `tests/test_mainwindow.cpp`:
+
+```cpp
+void TestMainWindow::cardsNeverScrollSideways()
+{
+ MainWindow window;
+ window.show();
+ QVERIFY(QTest::qWaitForWindowExposed(&window));
+
+ auto *view = window.findChild<ThreadListView *>();
+ QVERIFY(view);
+
+ // Item 51: clicking a row used to scroll the list sideways, because the
+ // subject column was wider than the viewport and auto-scroll brought the
+ // clicked index fully into view. A card is exactly viewport width, so
+ // there is nowhere to scroll to.
+ QCOMPARE(view->horizontalScrollBar()->minimum(),
+ view->horizontalScrollBar()->maximum());
+}
+```
+
+- [ ] **Step 7: Commit**
+
+```bash
+git add src/threadlistview.h src/threadlistview.cpp src/mainwindow.cpp tests/test_mainwindow.cpp
+git commit -S -m "refactor(view): stop the view painting, and hit-test the reply count
+
+ThreadListView::paintEvent and its band arithmetic are deleted. The view existed
+to paint a strip across five columns; with one column and one delegate painting
+the whole card there is nothing to span, and the two faults that arithmetic kept
+producing go with it: a deleted row cut in half, and every other row showing a
+bare stripe.
+
+What survives is the expander hit-test, because a delegate gets no click of its
+own without an editor. It now asks CardDelegate for the rect rather than
+recomputing it, so the drawn target and the clickable one cannot drift. The
+siblingAtColumn(0) dance is gone: with one column, the index already is column 0.
+
+Item 51 closes here rather than separately. A card is exactly viewport width, so
+the view has no horizontal scroll range for a click to scroll into, and the test
+asserts that directly."
+```
+
+---
+
+## Task 7: Navigation (item 60)
+
+**Files:**
+- Modify: `src/mainwindow.cpp:644-655` (both actions), and `addAction`
+- Modify: `src/keymap.cpp` (defaults)
+- Test: `tests/test_mainwindow.cpp`
+
+- [ ] **Step 1: Write the failing test**
+
+```cpp
+void TestMainWindow::nextThreadLeavesTheLastReply()
+{
+ MainWindow window;
+ window.show();
+ QVERIFY(QTest::qWaitForWindowExposed(&window));
+
+ auto *view = window.findChild<ThreadListView *>();
+ auto *model = qobject_cast<ThreadListModel *>(view->model());
+ QVERIFY(model);
+
+ ThreadSummary first;
+ first.threadId = QStringLiteral("T1");
+ first.totalCount = 2;
+ ThreadSummary second;
+ second.threadId = QStringLiteral("T2");
+ second.totalCount = 1;
+ model->appendBatch({ first, second });
+
+ MessageNode reply;
+ reply.messageId = QStringLiteral("M2");
+ reply.threadId = QStringLiteral("T1");
+ reply.depth = 1;
+ model->setReplies(QStringLiteral("T1"), { reply });
+
+ const QModelIndex root = model->index(0, 0);
+ view->expand(root);
+ const QModelIndex lastReply = model->index(0, 0, root);
+ QVERIFY(lastReply.isValid());
+ view->setCurrentIndex(lastReply);
+
+ // The defect: selectRow(current.row() + 1) asks for row 1 UNDER T1, which
+ // does not exist, so the action did nothing. It must land on T2.
+ window.findChild<QAction *>(QStringLiteral("next_thread"))->trigger();
+
+ QCOMPARE(view->currentIndex().data(ThreadListModel::ThreadIdRole).toString(),
+ QStringLiteral("T2"));
+}
+```
+
+**This test must start from the last reply of an EXPANDED thread.** A test that
+arrows down a collapsed list passes against the bug, because with nothing
+expanded every row is top-level and the arithmetic is accidentally correct.
+
+The action needs a findable name; if `addAction` does not call
+`setObjectName(name)`, add it there.
+
+- [ ] **Step 2: Run it and watch it fail**
+
+```bash
+cmake --build build && ./build/tests/test_mainwindow -functions | grep nextThread
+./build/tests/test_mainwindow nextThreadLeavesTheLastReply
+```
+
+Expected: FAIL, current index still on the reply.
+
+- [ ] **Step 3: Rewrite both actions**
+
+Replace the bodies at `src/mainwindow.cpp:644-655`:
+
+```cpp
+ addAction(QStringLiteral("next_thread"), tr("&Next thread"),
+ tr("Select the next thread"), [this]() {
+ // Walk by INDEX, never by row number. A tree numbers rows per parent,
+ // so current.row() + 1 named a sibling: from the last reply of an
+ // expanded thread it asked for a row that does not exist and the
+ // action silently did nothing (item 60).
+ QModelIndex index = m_threadView->indexBelow(
+ m_threadView->currentIndex());
+ while (index.isValid()
+ && index.data(ThreadListModel::IsMessageRole).toBool()) {
+ index = m_threadView->indexBelow(index);
+ }
+ if (index.isValid())
+ m_threadView->setCurrentIndex(index);
+ });
+ addAction(QStringLiteral("prev_thread"), tr("&Previous thread"),
+ tr("Select the previous thread"), [this]() {
+ QModelIndex index = m_threadView->indexAbove(
+ m_threadView->currentIndex());
+ while (index.isValid()
+ && index.data(ThreadListModel::IsMessageRole).toBool()) {
+ index = m_threadView->indexAbove(index);
+ }
+ if (index.isValid())
+ m_threadView->setCurrentIndex(index);
+ });
+```
+
+- [ ] **Step 4: Allow two shortcuts per action**
+
+In `src/mainwindow.cpp`, `addAction` currently calls `setShortcut` (singular) at
+about line 622. Change it to collect every sequence bound to the action:
+
+```cpp
+ const QList<QKeySequence> sequences = m_keymap.sequencesFor(name);
+ if (!sequences.isEmpty())
+ action->setShortcuts(sequences);
+```
+
+`zoom_reset` already uses `setShortcuts` at about line 773; follow that shape.
+If `KeyMap` has no `sequencesFor`, add it: the bindings map is
+sequence-to-action, so it is a loop collecting every key whose value matches.
+
+- [ ] **Step 5: Add the Alt defaults**
+
+In `src/keymap.cpp`, beside the existing Ctrl+J and Ctrl+K entries:
+
+```cpp
+ { QStringLiteral("Ctrl+J"), QStringLiteral("next_thread") },
+ { QStringLiteral("Ctrl+K"), QStringLiteral("prev_thread") },
+ // Alt, because Shift+Up/Down is QTreeView's built-in extend-selection,
+ // which multi-row tagging depends on, and plain Up/Down is the view's
+ // own navigation, which already steps INTO an expanded thread's
+ // replies and is what gives message-to-message movement for free.
+ //
+ // These must stay chords. Every action is a QAction with
+ // WindowShortcut, dispatched before the focused widget sees the key,
+ // and Qt withholds only plain LETTERS from editable widgets: a bare
+ // Up bound here would break the arrow keys in the query bar, the tag
+ // dialog and the web view at once, exactly as Return did.
+ { QStringLiteral("Alt+Down"), QStringLiteral("next_thread") },
+ { QStringLiteral("Alt+Up"), QStringLiteral("prev_thread") },
+```
+
+- [ ] **Step 6: Run the tests**
+
+```bash
+cmake --build build && ./build/tests/test_mainwindow
+```
+
+Expected: PASS, including `everyActionHasAShortcut`, which must survive the
+move to `setShortcuts`.
+
+- [ ] **Step 7: Add the skip test and mutation-check**
+
+```cpp
+void TestMainWindow::altDownSkipsReplies()
+{
+ // Same fixture as nextThreadLeavesTheLastReply, but starting from the
+ // thread ROOT with its replies showing: one step must land on the next
+ // thread, not on the first reply.
+ // ... build model, expand T1, setCurrentIndex(root) ...
+ window.findChild<QAction *>(QStringLiteral("next_thread"))->trigger();
+ QCOMPARE(view->currentIndex().data(ThreadListModel::ThreadIdRole).toString(),
+ QStringLiteral("T2"));
+ QVERIFY(!view->currentIndex().data(ThreadListModel::IsMessageRole).toBool());
+}
+```
+
+Mutation: delete the `while` loop that skips message rows. Expected:
+`altDownSkipsReplies` FAILS (it lands on the reply). **Revert.**
+
+- [ ] **Step 8: Commit**
+
+```bash
+git add src/mainwindow.cpp src/keymap.cpp tests/test_mainwindow.cpp
+git commit -S -m "fix(ui): step by index, not by row number (item 60)
+
+next_thread was selectRow(current.row() + 1). A QTableView numbers rows once for
+the whole view, so that was correct before message rows; a tree numbers them per
+parent, so from the last reply of an expanded thread it asked for a sibling that
+does not exist and did nothing at all. prev_thread failed the mirror case.
+rowCount() with no argument compounded it by counting top-level threads.
+
+Both now walk with indexBelow/indexAbove, skipping message rows, so they keep
+meaning thread-to-thread. Stepping message-to-message needs no code: QTreeView's
+own Up/Down walk VISIBLE rows and already enter an expanded thread, and being
+the view's key handling rather than a shortcut they stay inert when the message
+pane, a menu or an entry bar has focus.
+
+Alt+Up/Down added alongside Ctrl+J/K, which required addAction to move from
+setShortcut to setShortcuts. Alt because Shift+arrows is the built-in
+extend-selection that multi-row tagging depends on, and because a bare arrow
+cannot be a window shortcut without breaking every text field in the window, as
+Return already demonstrated.
+
+The test starts from the last reply of an expanded thread deliberately: one that
+arrows down a collapsed list passes against the bug."
+```
+
+---
+
+## Task 8: The sort dropdown
+
+**Files:**
+- Modify: `src/mainwindow.cpp` (query row, `runCurrentQuery`, UI state)
+- Test: `tests/test_mainwindow.cpp`
+
+- [ ] **Step 1: Write the failing test**
+
+```cpp
+void TestMainWindow::sortChoiceSurvivesRestart()
+{
+ {
+ MainWindow window;
+ window.show();
+ QVERIFY(QTest::qWaitForWindowExposed(&window));
+ auto *sort = window.findChild<QComboBox *>(QStringLiteral("sortOrder"));
+ QVERIFY(sort);
+ QCOMPARE(sort->currentIndex(), 0); // Newest first by default.
+ sort->setCurrentIndex(1);
+ window.close();
+ }
+
+ MainWindow second;
+ second.show();
+ QVERIFY(QTest::qWaitForWindowExposed(&second));
+ auto *sort = second.findChild<QComboBox *>(QStringLiteral("sortOrder"));
+ QCOMPARE(sort->currentIndex(), 1);
+}
+```
+
+**This writes to the real UI state file.** Follow whatever
+`test_mainwindow` already does to redirect `QStandardPaths` (it must, since it
+tests window geometry); if it uses `QStandardPaths::setTestModeEnabled(true)`,
+this test needs it too.
+
+- [ ] **Step 2: Run it and watch it fail**
+
+Expected: FAIL, no such child.
+
+- [ ] **Step 3: Add the combo box**
+
+In the query-row setup:
+
+```cpp
+ m_sortOrder = new QComboBox(this);
+ m_sortOrder->setObjectName(QStringLiteral("sortOrder"));
+ // Order matters: the index is what uistate.conf stores.
+ m_sortOrder->addItem(tr("Newest first"));
+ m_sortOrder->addItem(tr("Oldest first"));
+ m_sortOrder->setToolTip(tr("The order threads are listed in"));
+ connect(m_sortOrder, &QComboBox::currentIndexChanged,
+ this, &MainWindow::runCurrentQuery);
+```
+
+Add `QComboBox *m_sortOrder = nullptr;` to `mainwindow.h`.
+
+- [ ] **Step 4: Pass it to the worker**
+
+In `runCurrentQuery`, where the query is handed across:
+
+```cpp
+ const auto sort = m_sortOrder->currentIndex() == 1
+ ? NotmuchWorker::OldestFirst
+ : NotmuchWorker::NewestFirst;
+ emit queryRequested(query, ++m_generation, sort);
+```
+
+Update the signal's declaration and the queued connection to carry it. Since
+this crosses threads, `SortOrder` needs `qRegisterMetaType` unless `Q_ENUM` on a
+`QObject` subclass already covers it; if the connection warns at runtime about
+an unregistered type, add the registration in `main.cpp`.
+
+- [ ] **Step 5: Persist it**
+
+In `saveUiState()`:
+
+```cpp
+ settings.setValue(QStringLiteral("sortOrder"), m_sortOrder->currentIndex());
+```
+
+In the restore, guarding the range because a hand-edited or stale file can hold
+anything:
+
+```cpp
+ const int sort = settings.value(QStringLiteral("sortOrder"), 0).toInt();
+ m_sortOrder->setCurrentIndex(sort == 1 ? 1 : 0);
+```
+
+- [ ] **Step 6: Run the test**
+
+```bash
+cmake --build build && ./build/tests/test_mainwindow
+```
+
+Expected: PASS.
+
+- [ ] **Step 7: Commit**
+
+```bash
+git add src/mainwindow.cpp src/mainwindow.h tests/test_mainwindow.cpp
+git commit -S -m "feat(ui): let the user choose newest or oldest first
+
+Two entries, straight to notmuch. This adds a feature rather than replacing one:
+the column header was decorative and nothing implemented click-to-sort, so
+removing the header with the grid lost nothing.
+
+Stored in uistate.conf, never in the hand-edited config, and range-guarded on
+read: a stale file can hold anything, which is the lesson item 58 recorded."
+```
+
+---
+
+## Task 9: Documentation and the changelog
+
+**Files:**
+- Modify: `CLAUDE.md`, `CHANGELOG.md`
+- Modify: `docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md`
+
+- [ ] **Step 1: Correct the architecture diagram**
+
+`CLAUDE.md`'s diagram and its `ThreadListView` paragraph both describe the tag
+strip as the reason that class exists. That stops being true here. Rewrite the
+paragraph beginning **"`ThreadListView` exists because a delegate cannot paint
+outside its column"** to say what is true now: one column, one delegate that
+paints the whole card, and the view surviving only for the expander hit-test.
+
+Keep the historical lesson in one sentence, because it is why the file looks the
+way it does, but do not leave it stated as current behaviour.
+
+- [ ] **Step 2: Add the new traps**
+
+Append to `CLAUDE.md`, near the other Qt traps:
+
+```markdown
+**A card layout must be testable without a painter.** `CardLayout` computes
+every rect on a card and touches no `QPainter` and no widget, so the geometry
+has tests that a blank render cannot defeat. When changing what a card shows,
+change `CardLayout` and assert there; a test that renders the delegate and
+counts pixels proves nothing, for the reasons under "Rendering probes lie".
+
+**`QTreeView`'s Up/Down already walk into an expanded thread's replies**, and
+that is where message-to-message navigation comes from. Do not bind arrow keys
+as `QAction` shortcuts to get it: a shortcut is dispatched before the focused
+widget sees the key and Qt withholds only plain LETTERS from editable widgets,
+so a bare `Up` would break the query bar, the tag dialog and the web view at
+once. `Alt+Up`/`Alt+Down` are chords and therefore safe; `Shift+Up`/`Down` is
+the built-in extend-selection and must be left alone.
+```
+
+- [ ] **Step 3: Changelog**
+
+Under `## [Unreleased]`, in `### Changed`:
+
+```markdown
+- The thread list is now a list of cards rather than a table of columns. Each
+ thread shows its sender and date, its subject with the flag, attachment and
+ reply-count marks, and its tags, on three lines at one uniform height.
+ Expanding a thread shows its replies indented under a continuous spine,
+ carrying only the tags the thread itself does not have.
+- Threads can be listed newest or oldest first, from a new control beside the
+ query bar. The choice is remembered.
+```
+
+In `### Fixed`:
+
+```markdown
+- Next and previous thread no longer do nothing when the last reply of an
+ expanded thread is selected.
+- Clicking a thread no longer scrolls the list sideways.
+```
+
+Add an `### Upgrading` note: the column widths in a user's `uistate.conf` are
+now ignored, which is silent and harmless but worth saying.
+
+- [ ] **Step 4: Close the backlog items**
+
+Mark 51, 53 and 60 `done` in the status table, and item 20 `done` (the
+presentation it specified now exists in its revised form). Do not renumber.
+
+- [ ] **Step 5: Commit**
+
+```bash
+git add CLAUDE.md CHANGELOG.md docs/
+git commit -S -m "docs: record the card list, and correct what CLAUDE.md claims
+
+The architecture section described ThreadListView as existing to paint a strip
+across columns. That was true until this change and is now the opposite of
+true: it survives only for the expander hit-test. Kept as one sentence of
+history, since it explains the file's shape, but no longer stated as current
+behaviour.
+
+Items 20, 51, 53 and 60 close together."
+```
+
+---
+
+## Task 10: Hand verification
+
+Automated tests cannot cover the parts of this that matter most. `CLAUDE.md`
+records that a whole session was lost to rendering probes that lied, and that
+the original bug was the desktop's own font being configured Bold in qt6ct.
+
+- [ ] **Step 1: Run it against the real database**
+
+```bash
+./build/src/qtmaildir
+```
+
+- [ ] **Step 2: Check each of these by eye**
+
+- Cards are all the same height, tagged and untagged alike.
+- An expanded thread's replies indent, with a continuous line down each level.
+- A reply shows no `Re:` prefix.
+- A reply with a tag its thread lacks shows that tag; ordinary replies show
+ none.
+- Clicking the reply count expands; clicking anywhere else on the card opens
+ the message and does not expand.
+- **Up/Down step into an expanded thread's replies.** This is the one claim in
+ the whole design that rests on built-in Qt behaviour rather than on code
+ written here, and `CLAUDE.md` warns that `QTest::keyClick` is weak evidence
+ about key reachability. Verify it on a real keyboard.
+- Alt+Up/Alt+Down skip replies entirely.
+- Arrow keys still move the cursor in the query bar and scroll the message
+ pane, rather than moving the list.
+- The sort dropdown reverses the list, and the choice survives a restart.
+- The list does not scroll sideways on any click, at any window width.
+
+- [ ] **Step 3: Check the theme you do not use**
+
+Switch the desktop between light and dark and confirm the spine, the chips and
+the dimmed reply text are all still legible. Every colour here comes from the
+palette for this reason, but the only proof is looking.
+
+- [ ] **Step 4: Report back**
+
+Per the user's standing preference, hand the result over rather than declaring
+it done. Say what was checked and what was not.
+
+---
+
+## Self-Review Notes
+
+**Spec coverage.** Every section of the spec maps to a task: the card (4, 5),
+reply line 3 (1), the spine and indent cap (4), the expander (5, 6), sorting
+(3, 8), what is deleted (5, 6), what is kept (verified by the suite staying
+green), the new roles (1, 2), testing (each task), keyboard navigation (7), and
+returning to a whole thread (nothing to build, it is the root card).
+
+**Known gap, deliberately left.** The spec says `CardDelegate` should draw the
+account chip, and no task does. It is an addition to Task 5's `paint`, on line
+1 in front of the sender, using `AccountLabelRole` and `AccountColourRole`
+exactly as `SubjectDelegate` did. It is left out because the layout for it was
+not specified and inventing one here would be a guess; add it as Task 5b after
+seeing the cards on screen, or leave the chip out if the account is already
+clear from the sender.
+
+**Ordering risk.** Tasks 2, 5 and 6 leave the build red between commits. That
+is deliberate, since splitting a column removal across three files cannot be
+atomic without one enormous commit, but it means **the branch must not be
+merged mid-plan**. Finish through Task 6 before pushing anywhere the user might
+build from.