From 4621e954cc9e2dd19f966cea66073a50d59de04b Mon Sep 17 00:00:00 2001 From: Min Liu Date: Thu, 10 Sep 2026 22:37:43 -0700 Subject: [PATCH 1/2] Fix rendering flicker when switching to running sessions --- docs/desktop-design-system.md | 25 ++++ src/ava/app/desktop/qml/Main.qml | 44 +++--- src/ava/app/desktop/qml/MarkdownText.qml | 4 +- tests/test_desktop.py | 182 ++++++++++++++++++++++- 4 files changed, 235 insertions(+), 20 deletions(-) diff --git a/docs/desktop-design-system.md b/docs/desktop-design-system.md index 555d511..0a7c46c 100644 --- a/docs/desktop-design-system.md +++ b/docs/desktop-design-system.md @@ -55,6 +55,31 @@ Keep feature state in its existing pane. Avoid building a generic page framework List rows stay flat; menus, dialogs, and the composer carry elevation. Status labels use the existing Label rather than a new badge wrapper unless a badge is needed. +## Streaming transcripts + +Keep Qt's native Markdown renderer, but apply paragraph styling synchronously when +text changes, before the next ListView layout/paint. Qt's +[`setMarkdown()`](https://doc.qt.io/qt-6/qtextdocument.html#setMarkdown) replaces the +whole document; deferring our styling with +[`Qt.callLater()`](https://doc.qt.io/qt-6/qml-qtqml-qt.html#callLater-method) can expose +unstyled paragraph heights for a frame on every SSE update. Keep accumulating the +Markdown source: [`TextEdit.append()`](https://doc.qt.io/qt-6/qml-qtquick-textedit.html#append-method) +adds a paragraph, and parsing each arbitrary SSE fragment separately breaks syntax +split across chunks (for example, bold delimiters or code fences). + +Qt documents that [variable-height ListView delegates](https://doc.qt.io/qt-6/qml-qtquick-listview.html#variable-delegate-size-and-section-labels) +make its content-size estimate unstable. Follow the measured last row plus the +running-status height, not the estimated footer position. Geometry callbacks must +not re-enter layout with `forceLayout()`. Transcript delegates remain virtualized, +but are not pooled: unloading their lazy content while pooled exposes zero/stale +heights when reused. Scrolling up opts out of following; Jump to latest resumes it. + +The `switch_to_running_session` acceptance scenario replays durable mixed-height +history through the real backend, switches via the sidebar, holds the provider open, +then streams at 5/25 ms intervals. It checks per-frame tail/paragraph stability +within one logical pixel, delegate identity, Markdown formatting, and reading +position. Run it on both the native and offscreen renderers described below. + ## Evaluation Use the current implementation as the baseline, with the deterministic acceptance diff --git a/src/ava/app/desktop/qml/Main.qml b/src/ava/app/desktop/qml/Main.qml index d5ea985..6476824 100644 --- a/src/ava/app/desktop/qml/Main.qml +++ b/src/ava/app/desktop/qml/Main.qml @@ -519,9 +519,10 @@ ApplicationWindow { anchors.margins: 24 spacing: 22 clip: true - reuseItems: true - // Keep the tail alive while following: its measured bottom is stable - // even when pooled messages change the estimated content height. + // Pooling unloads the variable-height Loaders, feeding zero/stale + // heights back into ListView. Destroy offscreen delegates instead. + reuseItems: false + // Follow the measured last row, not the estimated footer position. currentIndex: follow ? count - 1 : -1 model: window.backend.transcript footer: Item { @@ -644,30 +645,39 @@ ApplicationWindow { } } property bool follow: true + function alignTail() { + if (!follow || !currentItem) + return; + contentY = Math.max(originY, currentItem.y + currentItem.height + (footerItem ? footerItem.height : 0) - height); + } function followLatest() { if (!follow) return; - const tail = footerItem && footerItem.visible ? footerItem : currentItem; - if (tail) { - forceLayout(); - contentY = Math.max(originY, tail.y + tail.height - height); - } + forceLayout(); + alignTail(); + } + // Geometry changes during polish must align in the same frame, + // without forceLayout() re-entering the layout that emitted them. + Connections { + target: conversation.currentItem + function onHeightChanged() { conversation.alignTail(); } + function onYChanged() { conversation.alignTail(); } + } + Connections { + target: conversation.footerItem + function onHeightChanged() { conversation.alignTail(); } } onMovementStarted: follow = false onMovementEnded: follow = atYEnd onHeightChanged: Qt.callLater(followLatest) onWidthChanged: Qt.callLater(followLatest) onCurrentItemChanged: Qt.callLater(followLatest) - onContentHeightChanged: Qt.callLater(followLatest) + onContentHeightChanged: alignTail() onCountChanged: { if (count === 0) follow = true; Qt.callLater(followLatest); } - Connections { - target: window.backend.transcript - function onDataChanged() { Qt.callLater(conversation.followLatest); } - } ScrollBar.vertical: ScrollBar { onPressedChanged: conversation.follow = !pressed && conversation.atYEnd } @@ -684,7 +694,6 @@ ApplicationWindow { required property int groupRunning required property int groupFailed required property bool outputExpanded - property bool pooled: false width: conversation.width height: messageColumn.implicitHeight Column { @@ -738,7 +747,7 @@ ApplicationWindow { Loader { id: messageLoader width: parent.width - active: !transcriptRow.pooled && (transcriptRow.groupCount <= 1 || transcriptRow.groupExpanded) + active: transcriptRow.groupCount <= 1 || transcriptRow.groupExpanded visible: active sourceComponent: TranscriptMessage { kind: transcriptRow.kind @@ -757,8 +766,6 @@ ApplicationWindow { } } } - ListView.onPooled: pooled = true - ListView.onReused: pooled = false } } NativeButton { @@ -772,7 +779,8 @@ ApplicationWindow { tip: "Jump to latest message" onClicked: { conversation.follow = true; - conversation.followLatest(); + conversation.positionViewAtEnd(); + Qt.callLater(conversation.followLatest); } } ColumnLayout { diff --git a/src/ava/app/desktop/qml/MarkdownText.qml b/src/ava/app/desktop/qml/MarkdownText.qml index e404188..6661662 100644 --- a/src/ava/app/desktop/qml/MarkdownText.qml +++ b/src/ava/app/desktop/qml/MarkdownText.qml @@ -28,7 +28,9 @@ TextArea { decorations = backend.formatMarkdown(textDocument, linkColor, codeBackground, codeFont); formatting = false; } - onTextChanged: scheduleFormat() + // Style before ListView measures this update, not a frame later: otherwise + // every streamed chunk briefly restores Qt's unstyled paragraph heights. + onTextChanged: formatDocument() onTextFormatChanged: { decorations = []; scheduleFormat(); diff --git a/tests/test_desktop.py b/tests/test_desktop.py index 09561d8..a85f3ba 100644 --- a/tests/test_desktop.py +++ b/tests/test_desktop.py @@ -6,6 +6,7 @@ import json import os import plistlib +import queue import re import shlex import sqlite3 @@ -109,6 +110,7 @@ def qt_app(): class Exchange: request: dict release: threading.Event = field(default_factory=threading.Event) + chunks: queue.Queue[str | None] = field(default_factory=queue.Queue) @pytest.fixture @@ -235,6 +237,13 @@ def chunk(delta, reason=None): self.wfile.flush() try: + if request.get("model") == "fixture-stream": + while (delta := exchange.chunks.get()) is not None: + chunk({"content": delta}) + chunk({}, "stop") + self.wfile.write(b"data: [DONE]\n\n") + self.wfile.flush() + return if request.get("model") == "fixture-browser-guards": results = [message for message in request["messages"] if message["role"] == "tool"] number = len(results) @@ -372,6 +381,7 @@ def chunk(delta, reason=None): shutting_down.set() for exchange in exchanges: exchange.release.set() + exchange.chunks.put(None) server.shutdown() server.server_close() thread.join(2) @@ -2034,13 +2044,14 @@ def save_screenshot(window, suffix): if QGuiApplication.focusWindow() != window: window.raise_() window.requestActivate() + assert QTest.qWaitForWindowExposed(window, 2000), "screenshot window was not exposed" # Deliver pending native resize events before waiting for the next rendered # frame. A frame already queued at the old size is not a layout checkpoint. QCoreApplication.processEvents() window.contentItem().ensurePolished() presented = QSignalSpy(window.frameSwapped) window.update() - assert presented.wait(2000), ( + assert presented.count() or presented.wait(2000), ( f"screenshot frame was not presented: exposed={window.isExposed()}, " f"active={window.isActive()}, app={QGuiApplication.applicationState()}" ) @@ -3769,6 +3780,175 @@ def test_desktop_activity_is_lazy_and_preserves_reading_position(desktop, model_ save_screenshot(window, "activity") +@pytest.mark.parametrize("interval", [5, 25]) +def test_desktop_switch_to_running_session_keeps_rendered_frames_stable( + desktop, model_server, home, project, interval +): + from ava.llm.types import Item, Role, make_reasoning_block, make_text_block + from ava.session import ( + AssistantMessage, + Log, + StepEnd, + StepEndReason, + StepStart, + TurnEnd, + TurnEndReason, + TurnStart, + UserMessage, + ) + + # Durable mixed-height history must be replayed through the real backend/SSE, + # not injected into the view model after switching sessions. + settings_path = home / "settings.json" + settings = json.loads(settings_path.read_text()) + settings["model"] = "fixture-stream" + settings["providers"]["desktop-test"]["models"]["fixture-stream"] = {"context_window": 200000} + settings_path.write_text(json.dumps(settings)) + log = Log.create_default(project, "desktop-test", "fixture-stream") + try: + for index in range(12): + body = "## Completed work\n\n" + "Completed 中文段落。\n\n" * (70 if index in (0, 11) else index % 3 + 1) + log.append_batch([ + TurnStart(index + 1), StepStart(index + 1, 1), + UserMessage(Item(role=Role.user, blocks=[make_text_block(f"Check step {index}")])), + AssistantMessage(str(index), Item(role=Role.assistant, blocks=[ + make_text_block(body), + *[make_reasoning_block("{}", "Checked the configuration") for _ in range(3)], + ])), + StepEnd(index + 1, 1, StepEndReason.completed), + TurnEnd(index + 1, TurnEndReason.completed), + ]) + finally: + log.close() + controller, window = desktop + controller.start() + until(lambda: controller.connected and bool(controller.transcript.rows), controller.changed) + identity = controller.chatId + type_message(window, "Check the existing MCP configuration") + click(window, "sendButton") + until(lambda: bool(model_server), controller.changed) + exchange = model_server[0] + prefix = ( + "## 实时 Markdown\n\n**粗体**与 `inline_code`。\n\n" + "```python\nprint('你好 👋')\n```\n\n" + "> 引用内容。\n\n- 第一项\n- 第二项\n\n" + "| 名称 | 内容 |\n| --- | --- |\n| 状态 | **已完成** |\n\n" + + "已经完成的段落，不应在后续输出时重新改变行高。\n\n" * 20 + + "Frozen anchor\n\n" + ) + model = controller.transcript + + # The reported gesture: leave a running session and click it in the sidebar. + click(window, "newChatButton") + until(lambda: controller.connected and controller.chatId != identity, controller.changed) + click(window, "session_" + identity) + until( + lambda: controller.connected and controller.chatId == identity + and bool(model.rows) and model.rows[-1]["body"] == "Check the existing MCP configuration", + controller.changed, + ) + assert controller.status == "running" + view = find_item(window, "transcriptView") + QTest.qWait(150) + frames = [] + + def record(): + tail = view.property("currentItem") + if tail is None: + frames.append((float("inf"), 0, 0)) + return + bottom = tail.mapToScene(QPointF(0, tail.height())).y() + viewport_bottom = view.mapToScene(QPointF(0, view.height())).y() + frames.append(( + bottom + find_item(window, "runStatus").height() - viewport_bottom, + bottom - tail.height(), getCppPointer(tail)[0], + )) + + window.frameSwapped.connect(record) + try: + # Hold the provider open without tokens: layout must converge, not oscillate. + QTest.qWait(800) + finally: + window.frameSwapped.disconnect(record) + assert len(frames) >= 10 + assert max(abs(frame[0]) for frame in frames) <= 1, frames + assert max(frame[1] for frame in frames) - min(frame[1] for frame in frames) <= 1, frames + assert len({frame[2] for frame in frames}) == 1 + + exchange.chunks.put(prefix) + until(lambda: model.rows[-1]["body"] == prefix, controller.changed) + QTest.qWait(150) + output = find_item(window, "assistantMarkdown") + quick_document = output.property("textDocument") + document = quick_document.textDocument() + assert document.begin().blockFormat().headingLevel() == 2 + assert document.find("粗体").charFormat().fontWeight() >= 600 + assert any(isinstance(frame, QTextTable) for frame in document.rootFrame().childFrames()) + anchor_y = document.documentLayout().blockBoundingRect(document.find("Frozen anchor").block()).y() + anchors = [] + resets = QSignalSpy(model.modelReset) + insertions = QSignalSpy(model.rowsInserted) + tail_id = getCppPointer(view.property("currentItem"))[0] + frames.clear() + + def record_stream(): + record() + anchors.append(document.documentLayout().blockBoundingRect(document.find("Frozen anchor").block()).y()) + + suffix = "".join(f"新内容 {index}：持续输出时保持视口稳定。\n\n" for index in range(45)) + chunks = iter(suffix[index:index + 18] for index in range(0, len(suffix), 18)) + timer = QTimer() + timer.setInterval(interval) + + def emit_chunk(): + chunk = next(chunks, None) + if chunk is None: + timer.stop() + else: + exchange.chunks.put(chunk) + + timer.timeout.connect(emit_chunk) + window.frameSwapped.connect(record_stream) + timer.start() + try: + until(lambda: model.rows[-1]["body"] == prefix + suffix and not timer.isActive(), window.frameSwapped) + QTest.qWait(80) + finally: + timer.stop() + window.frameSwapped.disconnect(record_stream) + assert len(frames) >= 10 + assert max(abs(frame[0]) for frame in frames) <= 1, frames + assert max(abs(anchor - anchor_y) for anchor in anchors) <= 1, anchors + assert {frame[2] for frame in frames} == {tail_id}, "Streaming must not rebuild the active delegate" + assert all(b[1] <= a[1] + 1 for a, b in zip(frames, frames[1:], strict=False)), frames + assert not resets.count() and not insertions.count() + + # Reading older output must still opt out of following; jumping resumes it. + position = view.mapToScene(QPointF(view.width() / 2, view.height() / 2)) + QTest.wheelEvent(window, position, QPoint(0, 1200)) + until(lambda: not view.property("moving"), view.movingChanged) + assert not view.property("follow") + reading_y = view.property("contentY") + extra = "后续输出，不打断阅读。\n\n" * 15 + "**Split bold**\n\n```python\nprint('SSE')\n```\n" + # Transport fragments can split Markdown delimiters; they are not paragraphs. + for start in range(0, len(extra), 3): + exchange.chunks.put(extra[start:start + 3]) + exchange.chunks.put(None) + until(lambda: controller.status == "idle", controller.changed) + QTest.qWait(80) + assert abs(view.property("contentY") - reading_y) <= 1 + assert model.rows[-1]["body"] == prefix + suffix + extra + click(window, "jumpToLatest") + until(lambda: view.property("atYEnd"), window.frameSwapped) + assert view.property("follow") + output = find_item(window, "assistantMarkdown") + quick_document = output.property("textDocument") + document = quick_document.textDocument() + assert document.find("Split bold").charFormat().fontWeight() >= 600 + assert "```" not in document.toPlainText() + save_screenshot(window, f"stable-session-switch-{interval}ms") + + def test_desktop_variable_message_heights_settle_at_latest(desktop, model_server): controller, window = desktop controller.start() From c3b1677def6ac6a1350b2592b67b1a5ec9cc8feb Mon Sep 17 00:00:00 2001 From: Min Liu Date: Thu, 10 Sep 2026 22:52:40 -0700 Subject: [PATCH 2/2] Verify rendered content after jumping to the latest message --- docs/desktop-design-system.md | 6 ++- tests/test_desktop.py | 84 +++++++++++++++++++++++++++++++++-- 2 files changed, 85 insertions(+), 5 deletions(-) diff --git a/docs/desktop-design-system.md b/docs/desktop-design-system.md index 0a7c46c..dc49031 100644 --- a/docs/desktop-design-system.md +++ b/docs/desktop-design-system.md @@ -78,7 +78,11 @@ The `switch_to_running_session` acceptance scenario replays durable mixed-height history through the real backend, switches via the sidebar, holds the provider open, then streams at 5/25 ms intervals. It checks per-frame tail/paragraph stability within one logical pixel, delegate identity, Markdown formatting, and reading -position. Run it on both the native and offscreen renderers described below. +position. The `jump_to_latest_renders_message_pixels` scenario drags back to the +beginning, receives more output while the latest message is offscreen, then clicks +the down arrow. It checks actual glyph pixels at the destination: `atYEnd` alone +can briefly report success even when the message is outside the viewport. Run both +scenarios on the native and offscreen renderers described below. ## Evaluation diff --git a/tests/test_desktop.py b/tests/test_desktop.py index a85f3ba..d101902 100644 --- a/tests/test_desktop.py +++ b/tests/test_desktop.py @@ -3780,10 +3780,9 @@ def test_desktop_activity_is_lazy_and_preserves_reading_position(desktop, model_ save_screenshot(window, "activity") -@pytest.mark.parametrize("interval", [5, 25]) -def test_desktop_switch_to_running_session_keeps_rendered_frames_stable( - desktop, model_server, home, project, interval -): +@pytest.fixture +def streaming_history(home, project, model_server): + """Replay realistic mixed-height history before driving the local SSE provider.""" from ava.llm.types import Item, Role, make_reasoning_block, make_text_block from ava.session import ( AssistantMessage, @@ -3820,6 +3819,13 @@ def test_desktop_switch_to_running_session_keeps_rendered_frames_stable( ]) finally: log.close() + + +@pytest.mark.usefixtures("streaming_history") +@pytest.mark.parametrize("interval", [5, 25]) +def test_desktop_switch_to_running_session_keeps_rendered_frames_stable( + desktop, model_server, interval +): controller, window = desktop controller.start() until(lambda: controller.connected and bool(controller.transcript.rows), controller.changed) @@ -3949,6 +3955,76 @@ def emit_chunk(): save_screenshot(window, f"stable-session-switch-{interval}ms") +@pytest.mark.usefixtures("streaming_history") +@pytest.mark.parametrize("paragraphs", [60, 600]) +def test_desktop_jump_to_latest_renders_message_pixels(desktop, model_server, paragraphs): + controller, window = desktop + window.setProperty("dark", False) + controller.start() + until(lambda: controller.connected and bool(controller.transcript.rows), controller.changed) + type_message(window, "Show the latest message") + click(window, "sendButton") + until(lambda: bool(model_server), controller.changed) + body = "## A long response\n\n" + "Completed paragraph with readable content.\n\n" * paragraphs + "LATEST MESSAGE VISIBLE" + model_server[0].chunks.put(body) + until(lambda: bool(controller.transcript.rows) and controller.transcript.rows[-1]["body"] == body, controller.changed) + view = find_item(window, "transcriptView") + until(lambda: view.property("atYEnd"), window.frameSwapped) + QTest.qWait(150) + + def latest_pixels(): + output = find_item(window, "assistantMarkdown") + assert output is not None and output.isVisible() + quick_document = output.property("textDocument") + document = quick_document.textDocument() + marker = document.find("LATEST MESSAGE VISIBLE") + assert not marker.isNull() + rect = output.mapRectToScene(document.documentLayout().blockBoundingRect(document.lastBlock())) + assert visible_rect(window, view).contains(rect.center()), ( + rect, view.property("contentY"), view.property("originY"), + view.property("contentHeight"), view.property("atYEnd"), + ) + frame = window.grabWindow() + scale = frame.width() / window.width() + region = frame.copy(int(rect.x() * scale), int(rect.y() * scale), + int(min(rect.width(), 300) * scale), int(rect.height() * scale)) + ink = sum(region.pixelColor(x, y).lightnessF() < 0.45 + for y in range(region.height()) for x in range(region.width())) + assert ink > 50, f"Latest-message geometry is visible, but its glyphs are missing ({ink} ink pixels)" + return ink + + before = latest_pixels() + original_tail = view.property("currentItem") + # Drag to the beginning so the latest delegate leaves the viewport/cache, + # unlike a short wheel scroll within the same long Markdown message. + scrollbar = next(item for item in view.childItems() if item.inherits("QQuickScrollBar")) + thumb = scrollbar.property("contentItem") + start = thumb.mapToScene(QPointF(thumb.width() / 2, thumb.height() / 2)).toPoint() + end = scrollbar.mapToScene(QPointF(scrollbar.width() / 2, 2)).toPoint() + QTest.mousePress(window, Qt.MouseButton.LeftButton, pos=start) + QTest.mouseMove(window, end, 20) + QTest.mouseRelease(window, Qt.MouseButton.LeftButton, pos=end) + until(lambda: not view.property("moving"), view.movingChanged) + assert not view.property("follow") + assert find_item(window, "jumpToLatest").isVisible() + QTest.qWait(150) + assert view.property("atYBeginning") + assert not isValid(original_tail) or visible_rect(window, original_tail).isEmpty() + reading_y = view.property("contentY") + extra = "\n\n" + "New output while reading older messages.\n\n" * 200 + "LATEST MESSAGE VISIBLE" + model_server[0].chunks.put(extra) + until(lambda: controller.transcript.rows[-1]["body"] == body + extra, controller.changed) + QTest.qWait(150) + assert abs(view.property("contentY") - reading_y) <= 1 + click(window, "jumpToLatest") + until(lambda: view.property("atYEnd"), window.frameSwapped) + QTest.qWait(150) + assert view.property("follow") + after = latest_pixels() + assert abs(after - before) <= before * 0.1, (before, after) + save_screenshot(window, f"jump-latest-{paragraphs}") + + def test_desktop_variable_message_heights_settle_at_latest(desktop, model_server): controller, window = desktop controller.start()