Skip to content

Commit f2ead44

Browse files
fix(acp): restore thinking UI height after session tab switch
Inactive session pages sit on a hidden QStackedWidget, so isVisible() is false while thoughts and tool cards still stream. Pin height from the logical collapse flag, force QTextDocument layout, and re-fit on showEvent so switching back does not clip the body to the header. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent fce02c6 commit f2ead44

6 files changed

Lines changed: 194 additions & 15 deletions

File tree

‎src/widgets/AcpMessageWidget.cpp‎

Lines changed: 82 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,11 @@
3131
#include <QMouseEvent>
3232
#include <QPainter>
3333
#include <QPixmap>
34+
#include <QPointer>
3435
#include <QRegularExpression>
3536
#include <QResizeEvent>
3637
#include <QScrollBar>
38+
#include <QShowEvent>
3739
#include <QStyle>
3840
#include <QTextBlock>
3941
#include <QTextBrowser>
@@ -156,6 +158,28 @@ QString ensureHardBreaks(const QString &md)
156158
return out;
157159
}
158160

161+
// QTextDocument lays out lazily: after setHtml()/setPlainText() the document
162+
// reports a stale (often single-line) size until something forces the layout
163+
// engine to run for the current text width. Measuring height before that pass
164+
// clips multi-line thought/assistant bodies to ~one line. Touching the layout's
165+
// documentSize() after pinning the text width forces the full pass, so the
166+
// subsequent doc->size() read is authoritative. Same helper as AcpToolCallCard.
167+
qreal layoutDocumentHeight(QTextDocument *doc, int textWidth)
168+
{
169+
if (!doc) return 0.0;
170+
doc->setTextWidth(textWidth);
171+
QAbstractTextDocumentLayout *layout = doc->documentLayout();
172+
if (!layout) return doc->size().height();
173+
qreal h = layout->documentSize().height();
174+
const QTextBlock last = doc->lastBlock();
175+
if (last.isValid()) {
176+
const QRectF r = layout->blockBoundingRect(last);
177+
if (r.isValid())
178+
h = qMax(h, r.bottom());
179+
}
180+
return qMax(h, doc->size().height());
181+
}
182+
159183
} // namespace
160184

161185
AcpMessageWidget::AcpMessageWidget(QString role, QWidget *parent)
@@ -203,8 +227,9 @@ AcpMessageWidget::AcpMessageWidget(QString role, QWidget *parent)
203227
m_layout->addWidget(m_browser);
204228

205229
connect(m_thoughtHeader, &QToolButton::toggled, this, [this](bool checked) {
206-
if (m_browser) m_browser->setVisible(checked);
207-
refitBrowserHeight();
230+
// Header checked = expanded. Keep m_collapsed in lockstep so height
231+
// fitting can gate on the logical flag, not QWidget::isVisible().
232+
applyCollapsed(!checked);
208233
});
209234
} else {
210235
// assistant + any other roles
@@ -449,6 +474,7 @@ void AcpMessageWidget::rerender()
449474
normalizeBlockMargins(m_browser->document());
450475
}
451476
refitBrowserHeight();
477+
scheduleRefit();
452478
}
453479

454480
void AcpMessageWidget::refitBrowserHeight()
@@ -461,14 +487,6 @@ void AcpMessageWidget::refitBrowserHeight()
461487
if (m_layout) {
462488
m_layout->getContentsMargins(&marginL, &marginT, &marginR, &marginB);
463489
}
464-
const int w = width() - marginL - marginR;
465-
if (w <= 0) {
466-
return;
467-
}
468-
QTextDocument *doc = m_browser->document();
469-
doc->setTextWidth(w);
470-
const int browserH = qMax(0, static_cast<int>(std::ceil(doc->size().height())));
471-
m_browser->setFixedHeight(browserH);
472490

473491
// Pin the bubble's own height too. setFixedHeight on the inner browser
474492
// only clamps the browser — QFrame's sizeHint cascades through QBoxLayout
@@ -480,15 +498,49 @@ void AcpMessageWidget::refitBrowserHeight()
480498
// adds style-derived button margins even with stylesheet padding:0,
481499
// which adds phantom vertical space inside the bubble.
482500
bubbleH += m_thoughtHeader->fontMetrics().height();
483-
if (m_browser->isVisible()) {
484-
bubbleH += m_layout->spacing() + browserH;
501+
// Gate on the LOGICAL expand state, not m_browser->isVisible(): a
502+
// thought on a hidden QStackedWidget page (inactive session tab) —
503+
// or a just-inserted widget not yet painted — reads isVisible()==false
504+
// even though the body WILL paint once the tab is shown. That under-
505+
// pins the frame to header-only height while the body keeps its
506+
// measured height, clipping the thinking text. m_collapsed is
507+
// independent of show timing.
508+
if (m_collapsed) {
509+
setFixedHeight(bubbleH);
510+
return;
485511
}
486-
} else {
487-
bubbleH += browserH;
512+
bubbleH += m_layout->spacing();
488513
}
514+
515+
const int w = width() - marginL - marginR;
516+
if (w <= 0) {
517+
// Width not settled (never-shown stack page). Keep whatever height we
518+
// already have unless we just collapsed to header-only above.
519+
return;
520+
}
521+
QTextDocument *doc = m_browser->document();
522+
// Force a full layout pass for the current width before measuring —
523+
// QTextDocument under-reports height for freshly-set multi-line text
524+
// until the layout engine has run, which clips an expanded thought down
525+
// to roughly its first line.
526+
const int browserH = qMax(0, static_cast<int>(std::ceil(layoutDocumentHeight(doc, w))));
527+
m_browser->setFixedHeight(browserH);
528+
bubbleH += browserH;
489529
setFixedHeight(bubbleH);
490530
}
491531

532+
void AcpMessageWidget::scheduleRefit()
533+
{
534+
if (m_refitScheduled) return;
535+
m_refitScheduled = true;
536+
QPointer<AcpMessageWidget> guard(this);
537+
QTimer::singleShot(0, this, [guard]() {
538+
if (!guard) return;
539+
guard->m_refitScheduled = false;
540+
guard->refitBrowserHeight();
541+
});
542+
}
543+
492544
void AcpMessageWidget::resizeEvent(QResizeEvent *event)
493545
{
494546
QFrame::resizeEvent(event);
@@ -501,11 +553,22 @@ void AcpMessageWidget::resizeEvent(QResizeEvent *event)
501553
}
502554
}
503555

556+
void AcpMessageWidget::showEvent(QShowEvent *event)
557+
{
558+
QFrame::showEvent(event);
559+
// Session-tab switch shows this page without a size change, so resizeEvent
560+
// may not run. Re-measure now that we are actually visible and the stacked
561+
// layout has given us a real width.
562+
refitBrowserHeight();
563+
scheduleRefit();
564+
}
565+
504566
void AcpMessageWidget::changeEvent(QEvent *event)
505567
{
506568
QFrame::changeEvent(event);
507569
if (event->type() == QEvent::FontChange) {
508570
refitBrowserHeight();
571+
scheduleRefit();
509572
} else if (event->type() == QEvent::PaletteChange
510573
|| event->type() == QEvent::ApplicationPaletteChange) {
511574
// Re-tint the copy glyph and re-skin code surfaces for the new theme.
@@ -867,6 +930,7 @@ void AcpMessageWidget::setChatFont(const QFont &font)
867930
rerender();
868931
} else {
869932
refitBrowserHeight();
933+
scheduleRefit();
870934
}
871935
}
872936

@@ -886,4 +950,8 @@ void AcpMessageWidget::applyCollapsed(bool collapsed)
886950
m_browser->setVisible(!collapsed);
887951
}
888952
refitBrowserHeight();
953+
// Body may have been laid out at a stale width while hidden; re-measure
954+
// once the expanded geometry settles so the thought doesn't clip on
955+
// first expand / tab show.
956+
if (!collapsed) scheduleRefit();
889957
}

‎src/widgets/AcpMessageWidget.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ class QTimer;
3232
class QToolButton;
3333
class QLabel;
3434
class QVBoxLayout;
35+
class QShowEvent;
3536

3637
// One transcript row representing either a user/assistant/thought/system
3738
// message. Assistant messages render markdown via QTextDocument::setMarkdown;
@@ -78,6 +79,11 @@ class AcpMessageWidget : public QFrame
7879

7980
protected:
8081
void resizeEvent(QResizeEvent *event) override;
82+
// QStackedWidget (session tabs) hides inactive pages without a size change,
83+
// so resizeEvent does not re-run on tab switch. Re-fit on show so a thought
84+
// that streamed in the background keeps its body height instead of staying
85+
// pinned to the header.
86+
void showEvent(QShowEvent *event) override;
8187
// Bubble height is derived from QTextDocument::size() under the current
8288
// font metrics; when the parent's font changes (Default Font preference)
8389
// we need to re-fit, because Qt does not auto-relayout content widgets on
@@ -90,6 +96,7 @@ class AcpMessageWidget : public QFrame
9096
private:
9197
void rerender();
9298
void refitBrowserHeight();
99+
void scheduleRefit();
93100
void applyCollapsed(bool collapsed);
94101
void scheduleRerender();
95102
void flushRerender();
@@ -115,6 +122,7 @@ class AcpMessageWidget : public QFrame
115122
QString m_text;
116123
bool m_collapsed = false;
117124
bool m_fromGoalAgent = false;
125+
bool m_refitScheduled = false; // coalesced deferred height pass already queued
118126

119127
// Chat (Default Font) typeface, pushed in via setChatFont(). Held so that
120128
// content created/re-rendered after the initial setFont() (streamed chunks,

‎src/widgets/AcpToolCallCard.cpp‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include <QPointer>
3030
#include <QRegularExpression>
3131
#include <QResizeEvent>
32+
#include <QShowEvent>
3233
#include <QStringList>
3334
#include <QTextBlock>
3435
#include <QTextBrowser>
@@ -1072,7 +1073,9 @@ void AcpToolCallCard::refitBodyHeight()
10721073

10731074
const int w = width() - marginL - marginR;
10741075
if (w <= 0) {
1075-
setFixedHeight(cardH);
1076+
// Width not settled (never-shown stack page). Keep whatever height we
1077+
// already have — pinning to header-only here clips an expanded body
1078+
// that streamed on a hidden tab.
10761079
return;
10771080
}
10781081
QTextDocument *doc = m_body->document();
@@ -1128,6 +1131,21 @@ void AcpToolCallCard::resizeEvent(QResizeEvent *event)
11281131
}
11291132
}
11301133

1134+
void AcpToolCallCard::showEvent(QShowEvent *event)
1135+
{
1136+
QFrame::showEvent(event);
1137+
// Session-tab switch shows this page without a size change, so resizeEvent
1138+
// may not run. Re-measure now that we are actually visible and the stacked
1139+
// layout has given us a real width.
1140+
refreshHeader();
1141+
if (!m_collapsed && m_bodyDirty) {
1142+
flushBodyRender();
1143+
} else {
1144+
refitBodyHeight();
1145+
}
1146+
scheduleRefit();
1147+
}
1148+
11311149
void AcpToolCallCard::setCollapsed(bool collapsed)
11321150
{
11331151
m_collapsed = collapsed;

‎src/widgets/AcpToolCallCard.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ class QTextBrowser;
3131
class QTimer;
3232
class QToolButton;
3333
class QVBoxLayout;
34+
class QShowEvent;
3435

3536
class AcpToolCallCard : public QFrame
3637
{
@@ -60,6 +61,11 @@ class AcpToolCallCard : public QFrame
6061

6162
protected:
6263
void resizeEvent(QResizeEvent *event) override;
64+
// QStackedWidget (session tabs) hides inactive pages without a size change,
65+
// so resizeEvent does not re-run on tab switch. Re-fit on show so an
66+
// expanded diff/output body that streamed in the background keeps its
67+
// height instead of staying pinned to the header.
68+
void showEvent(QShowEvent *event) override;
6369

6470
private:
6571
void refreshHeader();

‎tests/test_acp_message_widget_streaming.cpp‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#include <QtTest>
99
#include <QTextBrowser>
10+
#include <QToolButton>
1011

1112
#include "AcpMessageWidget.h"
1213

@@ -18,6 +19,8 @@ private slots:
1819
void assistant_streaming_buffers_chunks();
1920
void thought_collapse_after_streaming_done();
2021
void thought_renders_markdown_as_raw_text();
22+
void thought_keeps_body_height_while_hidden();
23+
void thought_reexpand_while_hidden_restores_body_height();
2124
};
2225

2326
void TestAcpMessageWidgetStreaming::assistant_streaming_buffers_chunks()
@@ -49,5 +52,51 @@ void TestAcpMessageWidgetStreaming::thought_renders_markdown_as_raw_text()
4952
QCOMPARE(browser->toPlainText().trimmed(), raw);
5053
}
5154

55+
void TestAcpMessageWidgetStreaming::thought_keeps_body_height_while_hidden()
56+
{
57+
// Inactive session tabs live on a hidden QStackedWidget page, so
58+
// QWidget::isVisible() is false while thought chunks still stream.
59+
// Height must follow the logical collapse flag, not isVisible() —
60+
// otherwise the bubble pins to header height and clips the body.
61+
AcpMessageWidget w(QStringLiteral("thought"));
62+
w.resize(420, 40);
63+
QVERIFY(!w.isVisible());
64+
65+
w.setText(QStringLiteral(
66+
"line one of the thought\n"
67+
"line two of the thought\n"
68+
"line three of the thought\n"
69+
"line four of the thought"));
70+
const int expandedH = w.height();
71+
72+
w.markStreamingDone();
73+
QVERIFY(w.isCollapsed());
74+
const int collapsedH = w.height();
75+
76+
QVERIFY2(expandedH > collapsedH,
77+
qPrintable(QStringLiteral("expanded=%1 collapsed=%2")
78+
.arg(expandedH)
79+
.arg(collapsedH)));
80+
}
81+
82+
void TestAcpMessageWidgetStreaming::thought_reexpand_while_hidden_restores_body_height()
83+
{
84+
AcpMessageWidget w(QStringLiteral("thought"));
85+
w.resize(420, 40);
86+
w.setText(QStringLiteral("a\nb\nc\nd"));
87+
w.markStreamingDone();
88+
QVERIFY(w.isCollapsed());
89+
const int collapsedH = w.height();
90+
91+
auto *header = w.findChild<QToolButton *>();
92+
QVERIFY(header);
93+
header->setChecked(true);
94+
QVERIFY(!w.isCollapsed());
95+
QVERIFY2(w.height() > collapsedH,
96+
qPrintable(QStringLiteral("expanded=%1 collapsed=%2")
97+
.arg(w.height())
98+
.arg(collapsedH)));
99+
}
100+
52101
QTEST_MAIN(TestAcpMessageWidgetStreaming)
53102
#include "test_acp_message_widget_streaming.moc"

‎tests/test_acp_tool_call_card.cpp‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ private slots:
2121
void diff_card_auto_expands_at_terminal_status();
2222
void running_diff_card_stays_collapsed();
2323
void collapsed_multiline_title_autosizes_to_two_lines();
24+
void expanded_diff_taller_than_collapsed_while_hidden();
2425
};
2526

2627
namespace {
@@ -198,5 +199,34 @@ void TestAcpToolCallCard::collapsed_multiline_title_autosizes_to_two_lines()
198199
QCOMPARE(two.height(), beforeToggle);
199200
}
200201

202+
void TestAcpToolCallCard::expanded_diff_taller_than_collapsed_while_hidden()
203+
{
204+
// Inactive session tabs live on a hidden QStackedWidget page. An
205+
// auto-expanded diff must still include the body in its pinned height
206+
// even though isVisible() is false — otherwise switching back clips
207+
// the diff to the header.
208+
AcpProtocol::AcpToolCall tc = baseCall();
209+
tc.status = QStringLiteral("completed");
210+
tc.content.append(diffBlock());
211+
212+
AcpToolCallCard expanded(tc);
213+
expanded.resize(480, 120);
214+
QVERIFY(!expanded.isVisible());
215+
QVERIFY(!expanded.isCollapsed());
216+
QTRY_VERIFY(bodyFor(expanded)->toPlainText().contains(QStringLiteral("a.cpp")));
217+
const int expandedH = expanded.height();
218+
219+
AcpProtocol::AcpToolCall running = baseCall();
220+
AcpToolCallCard collapsed(running);
221+
collapsed.resize(480, 120);
222+
QVERIFY(collapsed.isCollapsed());
223+
const int collapsedH = collapsed.height();
224+
225+
QVERIFY2(expandedH > collapsedH,
226+
qPrintable(QStringLiteral("expanded=%1 collapsed=%2")
227+
.arg(expandedH)
228+
.arg(collapsedH)));
229+
}
230+
201231
QTEST_MAIN(TestAcpToolCallCard)
202232
#include "test_acp_tool_call_card.moc"

0 commit comments

Comments
 (0)