fix(ui): make the expander visible and the reply indent readable

Both were reported from the running application after the previous commit
claimed them working, and the tests that passed could not see either fault.

The expander took four attempts, each of which looked right in code:

  - QTreeView::drawBranches is the documented hook and does not work here. It
    runs BEFORE the row's cells, so with the expander on a content column the
    delegate's own background paints over it. A 60-pixel triangle survived as
    8, indistinguishable from the theme's near-invisible dot.
  - Sizing the glyph from the row rather than the branch rect put most of it
    outside that rect.
  - Moving it into SubjectDelegate but calling it from only the no-chip branch
    left every real row without one, since every real row has an account chip
    and takes the other branch.

It is now drawn by the delegate, which owns the cell and paints after the
background, from both branches, with setRootIsDecorated(false) so the style
does not draw its dot underneath.

The indent was 20px and invisible for a reason the geometry could not show: a
thread row draws an account chip before its subject and a reply row does not, so
a reply's text already starts about a chip's width LEFT of its thread's. The
indent has to beat that before any nesting reads at all, hence 72px.

The indent test asserted on visualRect, which was correctly indented the whole
time, and so passed against a build with no visible nesting. It now measures
where the TEXT lands, accounting for the chip, and fails at 20px. The new
expander test counts painted pixels of the glyph colour against a control row
with no replies, and fails when the call is dropped from either branch.

7 files changed, 231 insertions, 5 deletions

diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 44feb0b..d092f92 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -559,9 +559,24 @@ void MainWindow::buildUi()
     // paperclip out of a 28px column entirely.
     m_threadView->setTreePosition(ThreadListModel::SubjectColumn);
 
-    // The root thread rows are the top level, so no decoration for them beyond
-    // the expander a thread with replies gets on its own.
-    m_threadView->setRootIsDecorated(true);
+    // No style-drawn branch decoration. SubjectDelegate draws the expander
+    // itself, because drawBranches runs BEFORE the row's cells and the
+    // delegate's own background paints straight over anything put there: a
+    // 60-pixel triangle survived as 8 pixels, indistinguishable from the
+    // near-invisible dot this replaces. Leaving both enabled would draw the
+    // theme's dot underneath the delegate's glyph.
+    m_threadView->setRootIsDecorated(false);
+
+    // Wider than Qt's default 20px, and the reason is specific rather than
+    // aesthetic. A thread row carries an account chip in front of its subject
+    // and a reply row does not, so a reply's text already starts roughly a
+    // chip's width (~60px) to the LEFT of its thread's. At the default indent
+    // the 20px shift is swallowed by that difference and the replies read as
+    // flush with the thread, or even outdented. Verified against the running
+    // app, not assumed: visualRect reported a correct 20px indent while the
+    // rendered text showed none, because the geometry is indented and the
+    // delegate then lays the text out from its own left edge.
+    m_threadView->setIndentation(SubjectDelegate::kReplyIndent);
 
     // Two delegates, and the split is not cosmetic. RowStyleDelegate carries
     // only the selection fix every column needs: the read/unread dimming
diff --git a/src/tagchip.cpp b/src/tagchip.cpp
index 1f4ad79..b3e743e 100644
--- a/src/tagchip.cpp
+++ b/src/tagchip.cpp
@@ -156,6 +156,51 @@ void SubjectDelegate::paint(QPainter *painter, const QStyleOptionViewItem &optio
 
     const QString account =
         index.data(ThreadListModel::AccountLabelRole).toString();
+
+    // The expander is drawn HERE and not in QTreeView::drawBranches, which is
+    // the obvious place and does not work. drawBranches runs before the row's
+    // cells, so with the expander column set to the subject the delegate's own
+    // background fills straight over it: measured at 8 surviving pixels of a
+    // 60-pixel triangle, which is exactly the near-invisible dot that made this
+    // override necessary in the first place. The delegate owns this cell and
+    // paints after the background, so it is the only place the glyph survives.
+    const auto drawExpander = [&](const QRect &cell) {
+        if (!index.data(ThreadListModel::HasRepliesRole).toBool())
+            return;
+
+        const int size = qMax(7, qMin(cell.height() / 3, 10));
+        const QPoint centre(cell.left() + size,
+                            cell.top() + subjectBandHeight(option) / 2
+                                + kRowPadding);
+
+        QPolygon triangle;
+        if (option.state & QStyle::State_Open) {
+            triangle << QPoint(centre.x() - size / 2, centre.y() - size / 4)
+                     << QPoint(centre.x() + size / 2, centre.y() - size / 4)
+                     << QPoint(centre.x(), centre.y() + size / 2);
+        } else {
+            triangle << QPoint(centre.x() - size / 4, centre.y() - size / 2)
+                     << QPoint(centre.x() + size / 2, centre.y())
+                     << QPoint(centre.x() - size / 4, centre.y() + size / 2);
+        }
+
+        painter->save();
+        painter->setRenderHint(QPainter::Antialiasing, true);
+        painter->setPen(Qt::NoPen);
+        // From the palette, so it survives a theme change, and undimmed: this
+        // is the only cue that a thread can be opened at all.
+        painter->setBrush(option.palette.color(QPalette::Text));
+        painter->drawPolygon(triangle);
+        painter->restore();
+    };
+    // Room for the expander in front of whatever follows, on a thread row that
+    // has one. Reserved before either branch draws, so the chip and the bare
+    // subject are indented identically and a thread with replies does not sit
+    // a few pixels left of one without.
+    const bool hasReplies =
+        index.data(ThreadListModel::HasRepliesRole).toBool();
+    const int expanderWidth = hasReplies ? kExpanderWidth : 0;
+
     if (account.isEmpty()) {
         // No chip to draw, so the base class renders the text, confined to the
         // upper band: the lower one belongs to the row-wide pill strip that
@@ -163,8 +208,10 @@ void SubjectDelegate::paint(QPainter *painter, const QStyleOptionViewItem &optio
         QStyleOptionViewItem chrome = option;
         initStyleOption(&chrome, index);
         chrome.rect.setHeight(subjectBandHeight(option));
+        chrome.rect.setLeft(chrome.rect.left() + expanderWidth);
         QStyledItemDelegate::paint(painter, chrome, index);
 
+        drawExpander(option.rect);
         return;
     }
 
@@ -186,7 +233,7 @@ void SubjectDelegate::paint(QPainter *painter, const QStyleOptionViewItem &optio
     const int textBandHeight = subjectBandHeight(option);
     const int textTop = option.rect.top() + kRowPadding;
 
-    const QRect chipRect(option.rect.left() + TagChip::kSpacing,
+    const QRect chipRect(option.rect.left() + expanderWidth + TagChip::kSpacing,
                          textTop + (textBandHeight - chipSize.height()) / 2,
                          chipSize.width(), chipSize.height());
 
@@ -234,6 +281,13 @@ void SubjectDelegate::paint(QPainter *painter, const QStyleOptionViewItem &optio
                       rowMetrics.elidedText(index.data(Qt::DisplayRole).toString(),
                                             Qt::ElideRight, textRect.width()));
     painter->restore();
+
+    // Last, so the chrome fill above cannot cover it. BOTH branches of this
+    // function have to call it: a thread row with an account chip takes this
+    // one, and that is every row in the real application, so calling it only
+    // from the no-chip branch leaves the feature invisible in practice while
+    // still passing any test built on an untagged thread.
+    drawExpander(option.rect);
 }
 
 QSize SubjectDelegate::sizeHint(const QStyleOptionViewItem &option,
diff --git a/src/tagchip.h b/src/tagchip.h
index cc3b5de..3145358 100644
--- a/src/tagchip.h
+++ b/src/tagchip.h
@@ -92,6 +92,20 @@ public:
     /// Vertical breathing room above the subject and below the pill row.
     static constexpr int kRowPadding = 4;
 
+    /// How far a reply row is indented under its thread.
+    ///
+    /// Deliberately far wider than Qt's 20px default. A thread row carries an
+    /// account chip in front of its subject and a reply row does not, so a
+    /// reply's text starts about a chip's width to the LEFT of its thread's
+    /// before any indent is applied. 20px does not cover that, and the replies
+    /// come out looking flush or outdented; this has to beat a chip's width to
+    /// read as nesting at all.
+    static constexpr int kReplyIndent = 72;
+
+    /// Horizontal room reserved in front of a thread's subject for the
+    /// expander glyph the delegate draws.
+    static constexpr int kExpanderWidth = 18;
+
     /// The font the pill strip is drawn in: a size down from the row's own.
     ///
     /// At the same size the pills read as a second row of content competing
diff --git a/src/threadlistmodel.cpp b/src/threadlistmodel.cpp
index df083fc..4828229 100644
--- a/src/threadlistmodel.cpp
+++ b/src/threadlistmodel.cpp
@@ -212,6 +212,10 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const
             return node.messageId;
         case MessageDepthRole:
             return node.depth;
+        case HasRepliesRole:
+            // A reply never has its own expander: nesting past the first level
+            // is drawn from depth, not from further parent-child structure.
+            return false;
         case ThreadIdRole:
             // A message row still belongs to a thread, and a caller that only
             // needs the containing thread must not have to walk up itself.
@@ -274,6 +278,9 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const
     if (role == MessageDepthRole)
         return 0;
 
+    if (role == HasRepliesRole)
+        return hasChildren(index.siblingAtColumn(0));
+
     if (role == TagsRole)
         return thread.tags;
 
diff --git a/src/threadlistmodel.h b/src/threadlistmodel.h
index e39ade6..700b3a6 100644
--- a/src/threadlistmodel.h
+++ b/src/threadlistmodel.h
@@ -96,6 +96,13 @@ public:
         /// The message's reply depth, for the view's indentation. 1 for a
         /// direct reply, since depth 0 is the root row itself.
         MessageDepthRole,
+
+        /// True when the row is a thread that has replies to show.
+        ///
+        /// Read by SubjectDelegate, which draws the expander itself: the
+        /// delegate cannot call hasChildren without the model, and the same
+        /// answer has to reach the cell that reserves room for the glyph.
+        HasRepliesRole,
     };
 
     /// Row fill for a thread tagged `deleted`, and for one tagged `spam`.
diff --git a/src/threadlistview.h b/src/threadlistview.h
index 520d610..9e0da28 100644
--- a/src/threadlistview.h
+++ b/src/threadlistview.h
@@ -56,4 +56,5 @@ public:
 
 protected:
     void paintEvent(QPaintEvent *event) override;
+
 };
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index 58768b7..37bfaef 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -92,6 +92,7 @@ private slots:
     void aSelectedReadThreadIsNotDimmedIntoTheHighlight();
     void thePillRowSpansTheWholeWidthNotOneColumn();
     void childRowsAreIndentedUnderTheirThread();
+    void aThreadWithRepliesDrawsAVisibleExpander();
     void noTagStripIsPaintedUnderAMessageRow();
     void markAllReadIsDisabledUntilTheQueryFinishes();
     void markAllReadActsOnEveryRowAndUndoesInOneStep();
@@ -490,7 +491,12 @@ void TestMainWindow::childRowsAreIndentedUnderTheirThread()
     auto *view = window.findChild<QTreeView *>();
     QVERIFY2(view, "the thread list is not a QTreeView, so it cannot indent");
 
-    model->appendBatch({ makeThread(QStringLiteral("t1"), {}) });
+    // With an account tag, so the thread row draws the chip that a reply row
+    // does not. That asymmetry is the whole reason the indent has to be wide,
+    // and a test against an untagged thread never sees it.
+    model->appendBatch({ makeThread(
+        QStringLiteral("t1"),
+        QStringList{ TagColors::tagForAccountKey(QStringLiteral("work")) }) });
 
     MessageNode first;
     first.messageId = QStringLiteral("m0@example.org");
@@ -531,6 +537,128 @@ void TestMainWindow::childRowsAreIndentedUnderTheirThread()
 
     QVERIFY2(view->visualRect(child).left() > view->visualRect(rootCell).left(),
              "the reply is not indented relative to its thread");
+
+    // The geometry being indented is NOT the same as the reply LOOKING
+    // indented, and asserting only the former shipped a build with no visible
+    // nesting at all. A thread row draws an account chip before its subject and
+    // a reply row does not, so the reply's text starts about a chip's width to
+    // the left of the thread's; at Qt's default 20px indent that difference
+    // swallows the shift entirely.
+    //
+    // So the real property: where the TEXT lands. The reply's subject must
+    // begin to the right of the thread's, which is what the eye reads as
+    // nesting.
+    const int chipWidth =
+        TagChip::sizeFor(QFontMetrics(view->font()),
+                         model->data(rootCell, ThreadListModel::AccountLabelRole)
+                             .toString()).width();
+    QVERIFY2(chipWidth > 0,
+             "the thread row has no account chip, so this test cannot measure "
+             "the offset it is meant to compensate for");
+
+    const int threadTextLeft = view->visualRect(rootCell).left() + chipWidth;
+    QVERIFY2(view->visualRect(child).left() > threadTextLeft,
+             qPrintable(QStringLiteral("the reply's text starts at x=%1, not "
+                                       "right of the thread's text at x=%2: the "
+                                       "indent does not beat the account chip "
+                                       "and the nesting is invisible")
+                            .arg(view->visualRect(child).left())
+                            .arg(threadTextLeft)));
+}
+
+void TestMainWindow::aThreadWithRepliesDrawsAVisibleExpander()
+{
+    // The expander is the ONLY thing saying a thread can be opened, and it took
+    // four wrong attempts to get on screen, each of which looked correct in
+    // code:
+    //
+    //   - QTreeView::drawBranches, the documented hook, runs BEFORE the row's
+    //     cells, so with the expander on a content column the delegate's own
+    //     background paints over it. A 60-pixel triangle survived as 8.
+    //   - Sizing it from the row rather than the branch rect put most of it
+    //     outside that rect.
+    //   - Moving it into the delegate but calling it from only one of the two
+    //     branches left every real row without one, since every real row has an
+    //     account chip and takes the other branch.
+    //
+    // None of those is visible to a test that asserts on geometry or on model
+    // roles, so this one counts painted pixels of the palette colour the glyph
+    // is drawn in.
+    const Config config;
+    MainWindow window(config);
+
+    auto *model = window.findChild<ThreadListModel *>();
+    QVERIFY(model);
+    auto *view = window.findChild<QTreeView *>();
+    QVERIFY(view);
+
+    // Two threads: one with replies, one without. The second is the control,
+    // and without it a test that counts text pixels would pass on any row.
+    ThreadSummary withReplies = makeThread(
+        QStringLiteral("t1"),
+        QStringList{ TagColors::tagForAccountKey(QStringLiteral("work")) });
+    withReplies.totalCount = 3;
+    ThreadSummary lone = makeThread(
+        QStringLiteral("t2"),
+        QStringList{ TagColors::tagForAccountKey(QStringLiteral("work")) });
+    lone.totalCount = 1;
+    model->appendBatch({ withReplies, lone });
+
+    window.resize(1400, 300);
+    window.show();
+    QVERIFY(QTest::qWaitForWindowExposed(&window));
+    QApplication::processEvents();
+
+    const QModelIndex first =
+        model->index(0, ThreadListModel::SubjectColumn, QModelIndex());
+    const QModelIndex second =
+        model->index(1, ThreadListModel::SubjectColumn, QModelIndex());
+
+    // Guards: both rows on screen, and the model agreeing about which has
+    // replies. Without these a zero count could mean anything.
+    QVERIFY2(view->visualRect(first).height() > 0, "the first row is not drawn");
+    QVERIFY2(view->visualRect(second).height() > 0,
+             "the control row is not drawn");
+    QVERIFY(model->data(first, ThreadListModel::HasRepliesRole).toBool());
+    QVERIFY(!model->data(second, ThreadListModel::HasRepliesRole).toBool());
+
+    QImage shot(view->viewport()->size(), QImage::Format_ARGB32);
+    shot.fill(Qt::transparent);
+    view->viewport()->render(&shot);
+
+    // The exact colour the glyph is filled with, matched exactly rather than by
+    // a brightness threshold, which would count antialiased subject text.
+    const QRgb glyph = view->palette().color(QPalette::Text).rgb();
+
+    // Only the strip in front of the subject text, so the subject's own glyphs
+    // cannot be counted. kExpanderWidth is the room the delegate reserves.
+    const auto countGlyphPixels = [&](const QModelIndex &index) {
+        const QRect rect = view->visualRect(index);
+        int found = 0;
+        for (int y = rect.top(); y < qMin(rect.bottom(), shot.height()); ++y) {
+            for (int x = rect.left();
+                 x < qMin(rect.left() + SubjectDelegate::kExpanderWidth,
+                          shot.width());
+                 ++x) {
+                if ((shot.pixel(x, y) | 0xff000000) == (glyph | 0xff000000))
+                    ++found;
+            }
+        }
+        return found;
+    };
+
+    const int drawn = countGlyphPixels(first);
+    const int control = countGlyphPixels(second);
+
+    QVERIFY2(drawn > 12,
+             qPrintable(QStringLiteral("only %1 expander pixels: the glyph is "
+                                       "clipped or painted over, which is how "
+                                       "it shipped as an invisible dot")
+                            .arg(drawn)));
+
+    // The control must have none, or the count above is measuring something
+    // every row draws.
+    QCOMPARE(control, 0);
 }
 
 void TestMainWindow::noTagStripIsPaintedUnderAMessageRow()