# 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 and then branches `card-list` off the result. **Nothing in this plan touches `master`.** That is a requirement, not a convention: the card list is an experiment on a design the user has already rejected once, and abandoning it must cost nothing. Every commit here lands on `card-list`; `master` keeps working and shippable throughout, and walking away is `git checkout master` with nothing to undo. Do not merge, do not fast-forward master, and do not push `card-list` anywhere the user builds from until they have seen it running (Task 10). **The escape hatch, should it be needed:** ```bash git checkout master # working application, unchanged git branch -D card-list # only if the experiment is being discarded ``` `item-20-message-rows` is left in place either way, as the reference for what the rejected presentation looked like. **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, then branch `card-list` off it **Files:** no source changes of your own; you are resolving other people's. **Do this on a copy, not on the original.** `item-20-message-rows` is the record of what the rejected presentation looked like, and rebasing it in place destroys that. Branch first, rebase the branch. - [ ] **Step 0: Verify master is clean and note where it is** ```bash git status --short git rev-parse --short master ``` Expected: no output from the first (a dirty tree here means someone left work behind, and it must be dealt with before a rebase). Write down the second: it is what `master` must still point at when this plan finishes. - [ ] **Step 1: Create the working branch** ```bash git checkout -b card-list 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 card-list..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 ` 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` - [ ] **Step 7: Confirm nothing happened to master or to the original branch** ```bash git rev-parse --short master # unchanged from Step 0 git rev-parse --short item-20-message-rows # still 029a50e git branch --show-current # card-list ``` If `master` moved, something rebased onto the wrong branch. Stop and fix it before writing any code: every later task assumes master is untouched and is the thing to fall back to. --- ## 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 newest; QVector oldest; connect(&worker, &NotmuchWorker::threadsReady, this, [&](const QVector &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 &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. * * 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 #include class TestCardLayout : public QObject { Q_OBJECT private slots: void everyCardIsTheSameHeight(); void threeLinesStackWithoutOverlapping(); void replyIndentsByDepth(); void indentStopsAtTheCap(); void expanderSitsOnTheSecondLine(); void expanderIsEmptyWithoutReplies(); void dateIsFlushRight(); void threadCardCarriesAnAccentBar(); void replyCardCarriesNoAccentBar(); }; 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()); } void TestCardLayout::threadCardCarriesAnAccentBar() { 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.accentRect.left(), rect.left()); QCOMPARE(card.accentRect.width(), CardLayout::kAccentWidth); // Full height, so a run of cards from one account reads as a continuous // edge rather than as dashes. QCOMPARE(card.accentRect.top(), rect.top()); QCOMPARE(card.accentRect.bottom(), rect.bottom()); // Nothing may be drawn on top of the colour. QVERIFY(card.contentLeft >= card.accentRect.right()); } void TestCardLayout::replyCardCarriesNoAccentBar() { const QFont font; const int h = CardLayout::heightFor(font); const CardLayout reply = CardLayout::compute(replyInput(1), QRect(0, 0, 400, h), font); // A reply's account is its thread's, stated once at the head. The spine // carries the accent instead, so the gutter never holds two lines. QVERIFY(reply.accentRect.isEmpty()); QCOMPARE(reply.spines.size(), 1); } 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 #include #include /// 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. }; /// Width of the account accent bar down a thread card's left edge. /// /// A starting value, not a settled one. Five accounts is enough that two /// colours distinct as chips can read alike as thin stripes, and that can /// only be judged against real cards on the user's own screen and theme /// (Task 10). Widen it there if the accounts are not tellable apart. static constexpr int kAccentWidth = 3; /// Horizontal breathing room at the card's edges, measured from the accent /// bar rather than from the card, so text does not sit on the colour. 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; /// The account accent bar down the card's left edge. /// /// Thread cards only. A reply's account is its thread's, stated once at the /// head of the conversation, and a second vertical line in a reply's gutter /// would sit a few pixels from the spine and compete with it. The spine /// carries the accent instead, so an expansion is bounded by one colour /// without ever drawing two lines. Empty on a reply. QRect accentRect; /// One full-height vertical line per depth level, outermost first. QVector 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 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(); // The accent bar sits flush against the card's left edge, on thread cards // only, and everything else starts after it so no text sits on the colour. if (!input.isMessage) { out.accentRect = QRect(rect.left(), rect.top(), kAccentWidth, rect.height()); } const int textLeft = rect.left() + kAccentWidth; // 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 = textLeft + 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 = textLeft + 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); /// An account's colour as a thin LINE rather than as a chip's fill. /// /// Never use the raw account colour for the accent bar or the spine. That /// colour is chosen to be a background with legible text drawn on top /// (TagColors::textColourOn picks black or white against it). The same /// colour as a few pixels of line on the pane's own background is a /// different problem: it has to be followable down a long expansion /// WITHOUT competing with the senders beside it, which is the constraint /// threadLineColour() states and meets by blending 0.35 toward the /// palette's text. This blends the account colour toward the palette's /// Base by the same weight, keeping the hue that identifies the account /// and dropping the saturation that would shout. static QColor accentLineColour(const QColor &accountColour); }; ``` - [ ] **Step 3: Write the implementation** Create `src/carddelegate.cpp` (GPL header, then): ```cpp #include "carddelegate.h" #include "cardlayout.h" #include "threadlistmodel.h" #include #include #include #include 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; } QColor CardDelegate::accentLineColour(const QColor &accountColour) { if (!accountColour.isValid()) return ThreadListModel::threadLineColour(); // The same 0.35 weight threadLineColour() uses, toward Base rather than // toward Text, so the two kinds of line sit at the same visual strength. const QColor base = QGuiApplication::palette().color(QPalette::Base); constexpr qreal kWeight = 0.35; const qreal inverse = 1.0 - kWeight; return QColor::fromRgbF( accountColour.redF() * kWeight + base.redF() * inverse, accountColour.greenF() * kWeight + base.greenF() * inverse, accountColour.blueF() * kWeight + base.blueF() * inverse); } 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(); // The account's colour, for both the accent bar and the spines. // // A reply must resolve its THREAD's colour, not its own: AccountColourRole // is empty on a message row, and a spine that fell back to the neutral // line under an accented root would break the one continuous edge this // design is built on. index.parent() is the thread for a depth-1 reply and // the containing subtree for a deeper one, so walk to the root. QModelIndex root = index; while (root.parent().isValid()) root = root.parent(); const QColor accountColour = root.data(ThreadListModel::AccountColourRole).value(); const QColor lineColour = accentLineColour(accountColour); // The accent bar, thread cards only. Drawn after the chrome so the // selection highlight cannot cover it: which account a card belongs to // must stay readable on the row the user is looking at. if (!card.accentRect.isEmpty()) painter->fillRect(card.accentRect, lineColour); // Spines, under everything else, in the same accent so an expanded thread // is bounded by one colour from its root to its last reply. for (const QRect &spine : card.spines) painter->fillRect(spine, lineColour); // 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().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: " 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(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 ` and `#include ` 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 account chip becomes a bar down the card's left edge, and the reply spines inherit its colour, so an expanded thread is bounded by one accent from its root to its last reply without a second line in the gutter. Neither uses the raw account colour: that colour is chosen to be a chip's fill with legible text on top, and the same value as a thin line has to be followable down an expansion without competing with the senders, so it is blended toward the palette's Base by the weight threadLineColour() already uses. A reply resolves its THREAD's colour by walking to the root, since AccountColourRole is empty on a message row and a neutral spine under an accented root would break the continuous edge. 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(); 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(); auto *model = qobject_cast(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(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 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(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(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(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: Colour the account dropdown's entries** The accent bar on a card says nothing until something maps a colour to an account name, and the dropdown is where the user already goes to think about accounts. In `src/mainwindow.cpp:399`, the loop that fills `m_accountBox`: ```cpp m_accountBox->addItem(tr("All accounts"), QString()); for (const Account &account : m_config.accounts()) { m_accountBox->addItem(account.key, account.key); // The RAW account colour here, not the blended line colour: a swatch // is a filled patch like a chip, not a thin line, so it wants the // colour the account was actually given. Qt renders a DecorationRole // colour as a swatch itself, with no delegate. m_accountBox->setItemData( m_accountBox->count() - 1, m_tagColors.colourFor(TagColors::tagForAccountKey(account.key)), Qt::DecorationRole); } ``` `colourFor` never fails: an account with no `color=` key gets a stable colour derived from its tag name, which is deliberate. Adding an account and forgetting to colour it degrades to something usable rather than to nothing. Check the member's name first: `grep -n "TagColors m_\|m_tagColors" src/mainwindow.h`. - [ ] **Step 8: Test it** ```cpp void TestMainWindow::accountEntriesCarryTheirColour() { MainWindow window; auto *box = window.findChild(QStringLiteral("accountBox")); QVERIFY(box); // "All accounts" is not an account and carries no swatch. QVERIFY(!box->itemData(0, Qt::DecorationRole).isValid()); // Every real account does. Skipped rather than failed when the test // environment has no accounts configured, since this reads real config. if (box->count() < 2) QSKIP("no accounts configured in this environment"); for (int i = 1; i < box->count(); ++i) { const QVariant swatch = box->itemData(i, Qt::DecorationRole); QVERIFY(swatch.isValid()); QVERIFY(swatch.value().isValid()); } } ``` `m_accountBox` needs `setObjectName(QStringLiteral("accountBox"))` if it does not have one. - [ ] **Step 9: 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. The account dropdown's entries now carry their account's colour as a swatch, which is what makes the accent bar on a card mean anything: a colour down a card's edge says nothing until something maps it to a name. Raw colour here rather than the blended line colour, since a swatch is a filled patch like a chip rather than a thin line." ``` --- ## 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. - An account's colour now runs down the left edge of its threads, and down the spine of their replies, replacing the account chip that used to sit in front of every subject. The account dropdown shows the same colours, so which colour means which account is readable in one place. ``` 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 2b: Settle the accent width, which cannot be settled from a mockup** `kAccentWidth` ships at 3px as a starting value. On a query spanning all five accounts, check: - **Are the five tellable apart?** Two colours distinct as chips can read alike as thin stripes. If not, widen `kAccentWidth` before reaching for different colours: the config's colours are the user's own choice and changing them is their call, not the delegate's. - **Does the spine still read as structure rather than as decoration?** It is now accent-coloured, and the blend was chosen to match `threadLineColour()`'s strength. If a bright account makes its expansion shout, raise the blend weight rather than special-casing that colour. - **Does the accent survive selection?** It is painted after the chrome deliberately, so it should stay visible on the highlighted row. Confirm. - **Check both themes.** The blend goes toward `QPalette::Base`, so it inverts with the theme; a value that looks right on dark can vanish on light. - [ ] **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, and stop** Per the user's standing preference, hand the result over rather than declaring it done. Say what was checked and what was not. **Do not merge to master, and do not offer to.** This design was reached by rejecting a previous one that was also finished, tested and green, so a passing suite is not the thing that decides it. The user looks at the cards and says whether they are right. Only then is there a merge to discuss. If the answer is no, `git checkout master` is the whole of the undo, and `card-list` stays on disk for whatever the next attempt reuses. --- ## 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). **The account chip gap is closed.** An earlier draft of this plan left the account chip unhandled, since its placement on a card was never specified. The user's answer was better than a chip: a coloured bar down the card's left edge, with the reply spines inheriting the same colour, and matching swatches in the account dropdown. That is in Task 4 (geometry), Task 5 (painting and the blend) and Task 8 (the dropdown). The chip itself is gone rather than relocated, which is a net simplification: it used to eat a third of line 2 to repeat a name the user already knows. **One judgement is deliberately deferred to Task 10.** `kAccentWidth` ships at 3px, and whether five accounts are tellable apart at that width on the user's own screen and theme cannot be decided from a mockup or a test. Step 2b of Task 10 is where it gets settled. **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.