diff --git a/src/ui/ConversationWidget.cpp b/src/ui/ConversationWidget.cpp index afb353a..e8ac8ee 100644 --- a/src/ui/ConversationWidget.cpp +++ b/src/ui/ConversationWidget.cpp @@ -3092,6 +3092,8 @@ void ConversationWidget::render(const sdk::State& state, renderedSegmentItemIds.clear(); renderedSegmentKeys.clear(); renderedSegmentWidgets.clear(); + renderedActivityRows.clear(); + renderedActivityRowSegments.clear(); clearLayout(timeline); }; @@ -3261,14 +3263,37 @@ void ConversationWidget::render(const sdk::State& state, for (const VisibleTimelineTurn& visibleTurn : visibleTurns) visibleTurnIds.append(fromUtf8(visibleTurn.turn->id.value)); - const auto removeRenderedTurn = [this, &timelineShrank, &timelineGeometryChanged](const QString& turnId) + const auto forgetRenderedActivityRows = [this](const QString& storage) + { + for (auto row = renderedActivityRowSegments.begin(); + row != renderedActivityRowSegments.end();) + { + if (row.value() != storage) + { + ++row; + continue; + } + renderedActivityRows.remove(row.key()); + row = renderedActivityRowSegments.erase(row); + } + }; + const auto forgetRenderedSegment = + [this, &forgetRenderedActivityRows](const QString& storage) + { + forgetRenderedActivityRows(storage); + renderedSegmentItemIds.remove(storage); + renderedSegmentKeys.remove(storage); + renderedSegmentWidgets.remove(storage); + }; + const auto removeRenderedTurn = [this, + &forgetRenderedSegment, + &timelineShrank, + &timelineGeometryChanged](const QString& turnId) { for (const QString& segmentId : renderedSegmentIds.take(turnId)) { const QString storage = segmentStorageKey(turnId, segmentId); - renderedSegmentItemIds.remove(storage); - renderedSegmentKeys.remove(storage); - renderedSegmentWidgets.remove(storage); + forgetRenderedSegment(storage); } renderedTurnLabels.remove(turnId); renderedTurnStatusLabels.remove(turnId); @@ -3384,90 +3409,34 @@ void ConversationWidget::render(const sdk::State& state, segmentIds.append(segment->id); const QStringList oldSegmentIds = renderedSegmentIds.value(turnId); - bool compatibleSegments = true; - qsizetype removedSegmentPrefix = 0; - if (!oldSegmentIds.isEmpty()) + const QSet nextSegmentIds( + segmentIds.cbegin(), segmentIds.cend()); + for (const QString& oldId : oldSegmentIds) { - if (segmentIds.isEmpty()) - { - compatibleSegments = false; - } - else - { - removedSegmentPrefix = oldSegmentIds.indexOf(segmentIds.front()); - compatibleSegments = removedSegmentPrefix >= 0 - && oldSegmentIds.size() - removedSegmentPrefix <= segmentIds.size(); - for (qsizetype segmentIndex = 0; - compatibleSegments && segmentIndex < oldSegmentIds.size() - removedSegmentPrefix; - ++segmentIndex) - { - const QString oldId = oldSegmentIds.at(removedSegmentPrefix + segmentIndex); - const QString storage = segmentStorageKey(turnId, oldId); - compatibleSegments = oldId == segmentIds.at(segmentIndex) - && renderedSegmentWidgets.contains(storage); - } - } - } - if (!compatibleSegments) - { - if (QWidget* turnWidget = renderedTurnWidgets.value(turnId); - pendingViewportAnchor && turnWidget - && (pendingViewportAnchor == turnWidget - || turnWidget->isAncestorOf(pendingViewportAnchor))) - pendingViewportAnchor.clear(); - clearLayout(itemLayout); - timelineShrank = timelineShrank || !oldSegmentIds.isEmpty(); - for (const QString& oldId : oldSegmentIds) - { - const QString storage = segmentStorageKey(turnId, oldId); - renderedSegmentItemIds.remove(storage); - renderedSegmentKeys.remove(storage); - renderedSegmentWidgets.remove(storage); - } - } - else - { - if (removedSegmentPrefix > 0 && pendingViewportAnchor) - { - bool anchorRemoved = false; - for (qsizetype segmentIndex = 0; segmentIndex < removedSegmentPrefix; ++segmentIndex) - { - QWidget* removed = renderedSegmentWidgets.value( - segmentStorageKey(turnId, oldSegmentIds.at(segmentIndex))); - anchorRemoved = anchorRemoved || pendingViewportAnchor == removed - || (removed && removed->isAncestorOf(pendingViewportAnchor)); - } - if (anchorRemoved && removedSegmentPrefix < oldSegmentIds.size()) - { - QWidget* survivor = renderedSegmentWidgets.value( - segmentStorageKey(turnId, oldSegmentIds.at(removedSegmentPrefix))); - pendingViewportAnchor = survivor; - if (survivor) - { - pendingViewportAnchorY = scrollArea->viewport() - ->mapFromGlobal(survivor->mapToGlobal(QPoint{})) - .y(); - } - } - } - for (qsizetype segmentIndex = 0; segmentIndex < removedSegmentPrefix; ++segmentIndex) + if (nextSegmentIds.contains(oldId)) + continue; + const QString storage = segmentStorageKey(turnId, oldId); + if (QWidget* widget = renderedSegmentWidgets.value(storage)) { - const QString storage = segmentStorageKey(turnId, oldSegmentIds.at(segmentIndex)); - if (QWidget* widget = renderedSegmentWidgets.take(storage)) - { - itemLayout->removeWidget(widget); - widget->hide(); - widget->deleteLater(); - timelineShrank = true; - timelineGeometryChanged = true; - } - renderedSegmentKeys.remove(storage); - renderedSegmentItemIds.remove(storage); + if (pendingViewportAnchor == widget + || (pendingViewportAnchor + && widget->isAncestorOf(pendingViewportAnchor))) + pendingViewportAnchor.clear(); + itemLayout->removeWidget(widget); + widget->hide(); + widget->deleteLater(); + timelineShrank = true; + timelineGeometryChanged = true; } + forgetRenderedSegment(storage); } - for (const TimelineSegment* segment : visibleTurn.segments) + for (qsizetype segmentIndex = 0; + segmentIndex < static_cast(visibleTurn.segments.size()); + ++segmentIndex) { + const TimelineSegment* segment = visibleTurn.segments.at( + static_cast(segmentIndex)); const QString storage = segmentStorageKey(turnId, segment->id); QStringList itemIds; itemIds.reserve(static_cast(segment->items.size())); @@ -3478,6 +3447,16 @@ void ConversationWidget::render(const sdk::State& state, } renderedSegmentItemIds.insert(storage, std::move(itemIds)); QWidget* oldWidget = renderedSegmentWidgets.value(storage); + if (oldWidget) + oldWidget->setProperty( + "timelineItemCount", timelineItemCount(*segment)); + if (oldWidget && itemLayout->indexOf(oldWidget) != segmentIndex) + { + itemLayout->removeWidget(oldWidget); + itemLayout->insertWidget( + segmentIndex, oldWidget, 0, Qt::AlignTop); + timelineGeometryChanged = true; + } const ConversationContentUpdates* segmentContentChanges = nullptr; ConversationContentUpdates segmentContentStorage; bool explicitlyAffected = false; @@ -3564,19 +3543,21 @@ void ConversationWidget::render(const sdk::State& state, const bool replacesAnchor = pendingViewportAnchor == oldWidget || (pendingViewportAnchor && oldWidget->isAncestorOf(pendingViewportAnchor)); - const int position = itemLayout->indexOf(oldWidget); itemLayout->removeWidget(oldWidget); oldWidget->hide(); oldWidget->deleteLater(); - itemLayout->insertWidget(position, newWidget, 0, Qt::AlignTop); + forgetRenderedActivityRows(storage); + itemLayout->insertWidget( + segmentIndex, newWidget, 0, Qt::AlignTop); if (replacesAnchor) - pendingViewportAnchor = newWidget; + pendingViewportAnchor.clear(); timelineShrank = true; timelineGeometryChanged = true; } else { - itemLayout->addWidget(newWidget, 0, Qt::AlignTop); + itemLayout->insertWidget( + segmentIndex, newWidget, 0, Qt::AlignTop); timelineGeometryChanged = true; } renderedSegmentWidgets.insert(storage, newWidget); diff --git a/src/ui/ConversationWidget.h b/src/ui/ConversationWidget.h index 9cb50bd..1f0c335 100644 --- a/src/ui/ConversationWidget.h +++ b/src/ui/ConversationWidget.h @@ -39,6 +39,7 @@ namespace codexui { class AnchoredTurnSurface; class UpcomingTurnDock; struct UpcomingTurnDraft; +struct ConversationWidgetTestAccess; struct ConversationContentAppend { @@ -104,6 +105,8 @@ class ConversationWidget : public QWidget void resizeEvent(QResizeEvent* event) override; private: + friend struct ConversationWidgetTestAccess; + void scheduleTimelineLayout(int previousScroll, bool followLatest, bool threadChanged, diff --git a/tests/ConversationLayoutTest.cpp b/tests/ConversationLayoutTest.cpp index b35923f..b6feb4f 100644 --- a/tests/ConversationLayoutTest.cpp +++ b/tests/ConversationLayoutTest.cpp @@ -32,6 +32,57 @@ #include #include +namespace codexui { + +struct ConversationWidgetTestAccess +{ + static bool primeViewportAnchor( + ConversationWidget& conversation, QWidget* anchor, int anchorY) + { + auto* scroll = conversation.scrollArea->verticalScrollBar(); + scroll->setValue(scroll->minimum()); + conversation.followingLatest = false; + conversation.pinLatestDuringLayout = true; + conversation.layoutSettleTimer->start(60'000); + conversation.pendingViewportAnchor = anchor; + conversation.pendingViewportAnchorY = anchorY; + return scroll->maximum() - scroll->value() > 72; + } + + static QWidget* viewportAnchor(const ConversationWidget& conversation) + { + return conversation.pendingViewportAnchor.data(); + } + + static int viewportAnchorY(const ConversationWidget& conversation) + { + return conversation.pendingViewportAnchorY; + } + + static bool tracksSegment( + const ConversationWidget& conversation, + const QString& turnId, + const QString& segmentId) + { + return conversation.renderedSegmentWidgets.contains( + turnId + QChar(0x1f) + segmentId); + } + + static void finishAnchoredReconciliation(ConversationWidget& conversation) + { + conversation.layoutSettleTimer->stop(); + conversation.layoutSettleTimer->setInterval(16); + conversation.pinLatestDuringLayout = false; + conversation.pendingViewportAnchor.clear(); + conversation.pendingFollowLatest = false; + conversation.pendingThreadChanged = false; + conversation.pendingTimelineShrink = false; + conversation.scrollArea->viewport()->setUpdatesEnabled(true); + } +}; + +} // namespace codexui + namespace { namespace frontend = ai::openai::codex::frontend; @@ -1057,6 +1108,281 @@ bool testPointerPreservingAppend() return passed; } +bool testKeyedSegmentInsertion() +{ + ThreadFixture activityFixture{ + "keyed-activity-growth", + {{"turn-keyed-activity-growth", + {{"activity-keyed-0", + frontend::ThreadItemKind::Reasoning, + "first activity"}, + {"activity-keyed-1", + frontend::ThreadItemKind::Reasoning, + "second activity"}, + {"message-keyed-tail", + frontend::ThreadItemKind::UserMessage, + "stable tail"}}}}}; + activityFixture.turns.front().status = "inProgress"; + activityFixture.turns.front().active = true; + activityFixture.turns.front().terminal = false; + for (int index = 0; index < 18; ++index) + { + activityFixture.turns.front().messages.push_back( + {"message-keyed-padding-" + std::to_string(index), + frontend::ThreadItemKind::UserMessage, + "padding row " + std::to_string(index)}); + } + codexui::ConversationWidget activityConversation; + activityConversation.resize(900, 420); + activityConversation.show(); + activityConversation.render( + makeState({activityFixture}), + QStringLiteral("keyed-activity-growth")); + settleTimeline(); + + QPointer activity = segment( + activityConversation, + QStringLiteral("activities:activity-keyed-0")); + QPointer activityTail = segment( + activityConversation, + QStringLiteral("message:message-keyed-tail")); + QWidget* const activityAddress = activity.data(); + QWidget* const activityTailAddress = activityTail.data(); + + const auto activityTailFixturePosition = std::find_if( + activityFixture.turns.front().messages.begin(), + activityFixture.turns.front().messages.end(), + [](const MessageFixture& message) + { + return message.id == "message-keyed-tail"; + }); + activityFixture.turns.front().messages.insert( + activityTailFixturePosition, + {"activity-keyed-2", + frontend::ThreadItemKind::Reasoning, + "third activity"}); + activityConversation.render( + makeState({activityFixture}), + QStringLiteral("keyed-activity-growth")); + settleTimeline(); + + bool passed = expect( + activity && activity.data() == activityAddress + && activity.data() + == segment( + activityConversation, + QStringLiteral("activities:activity-keyed-0")) + && activity->findChildren( + QStringLiteral("conversationActivityRow")) + .size() + == 3 + && activityTail && activityTail.data() == activityTailAddress + && activityTail.data() + == segment( + activityConversation, + QStringLiteral("message:message-keyed-tail")), + "an activity bucket that gains a row must retain its segment and every unchanged trailing widget"); + + QHash> unchangedActivitySegments; + unchangedActivitySegments.insert( + QStringLiteral("activities:activity-keyed-0"), activity); + unchangedActivitySegments.insert( + QStringLiteral("message:message-keyed-tail"), activityTail); + for (int index = 0; index < 18; ++index) + { + const QString segmentId = QStringLiteral("message:message-keyed-padding-%1") + .arg(index); + unchangedActivitySegments.insert( + segmentId, segment(activityConversation, segmentId)); + } + + auto& activityMessages = activityFixture.turns.front().messages; + std::rotate( + activityMessages.begin(), + activityMessages.begin() + 3, + activityMessages.begin() + 4); + activityConversation.render( + makeState({activityFixture}), + QStringLiteral("keyed-activity-growth")); + settleTimeline(); + + QLayout* activityLayout = activityTail && activityTail->parentWidget() + ? activityTail->parentWidget()->layout() + : nullptr; + bool allReorderedSegmentsRetained = true; + for (auto retained = unchangedActivitySegments.cbegin(); + retained != unchangedActivitySegments.cend(); + ++retained) + { + allReorderedSegmentsRetained = + allReorderedSegmentsRetained && retained.value() + && retained.value().data() + == segment(activityConversation, retained.key()); + } + passed &= expect( + allReorderedSegmentsRetained + && activity && activity.data() == activityAddress + && activity.data() + == segment( + activityConversation, + QStringLiteral("activities:activity-keyed-0")) + && activityTail && activityTail.data() == activityTailAddress + && activityTail.data() + == segment( + activityConversation, + QStringLiteral("message:message-keyed-tail")) + && activityLayout && activityLayout->indexOf(activityTail.data()) == 0 + && activityLayout->indexOf(activity.data()) == 1, + "reordering unchanged segment keys must move the original QWidgets into the new order"); + + constexpr int anchoredY = 37; + const bool anchorPathAvailable = + codexui::ConversationWidgetTestAccess::primeViewportAnchor( + activityConversation, activity.data(), anchoredY); + std::rotate( + activityMessages.begin(), + activityMessages.begin() + 1, + activityMessages.begin() + 4); + activityConversation.render( + makeState({activityFixture}), + QStringLiteral("keyed-activity-growth")); + passed &= expect( + anchorPathAvailable + && codexui::ConversationWidgetTestAccess::viewportAnchor( + activityConversation) + == activity.data() + && codexui::ConversationWidgetTestAccess::viewportAnchorY( + activityConversation) + == anchoredY, + "reordering a surviving segment must preserve its pending viewport anchor and offset"); + + codexui::ConversationWidgetTestAccess::primeViewportAnchor( + activityConversation, activity.data(), anchoredY); + const auto tailPosition = std::find_if( + activityMessages.begin(), + activityMessages.end(), + [](const MessageFixture& message) + { + return message.id == "message-keyed-tail"; + }); + activityMessages.erase(tailPosition); + activityConversation.render( + makeState({activityFixture}), + QStringLiteral("keyed-activity-growth")); + const bool removedTailUntracked = + !codexui::ConversationWidgetTestAccess::tracksSegment( + activityConversation, + QStringLiteral("turn-keyed-activity-growth"), + QStringLiteral("message:message-keyed-tail")); + QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete); + QCoreApplication::processEvents(); + passed &= expect( + removedTailUntracked && !activityTail + && activity && activity.data() == activityAddress + && codexui::ConversationWidgetTestAccess::viewportAnchor( + activityConversation) + == activity.data() + && codexui::ConversationWidgetTestAccess::viewportAnchorY( + activityConversation) + == anchoredY, + "removing a different segment must destroy only that widget and retain the surviving anchor"); + + codexui::ConversationWidgetTestAccess::primeViewportAnchor( + activityConversation, activity.data(), anchoredY); + activityMessages.erase( + std::remove_if( + activityMessages.begin(), + activityMessages.end(), + [](const MessageFixture& message) + { + return message.id.starts_with("activity-keyed-"); + }), + activityMessages.end()); + activityConversation.render( + makeState({activityFixture}), + QStringLiteral("keyed-activity-growth")); + const bool removedAnchorCleared = + !codexui::ConversationWidgetTestAccess::viewportAnchor( + activityConversation); + const bool removedAnchorUntracked = + !codexui::ConversationWidgetTestAccess::tracksSegment( + activityConversation, + QStringLiteral("turn-keyed-activity-growth"), + QStringLiteral("activities:activity-keyed-0")); + QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete); + QCoreApplication::processEvents(); + passed &= expect( + removedAnchorCleared && removedAnchorUntracked && !activity, + "destroying the anchor segment must clear its viewport anchor and delete the widget"); + codexui::ConversationWidgetTestAccess::finishAnchoredReconciliation( + activityConversation); + + ThreadFixture insertionFixture{ + "keyed-middle-insertion", + {{"turn-keyed-middle-insertion", + {{"message-keyed-left", + frontend::ThreadItemKind::UserMessage, + "left"}, + {"message-keyed-right", + frontend::ThreadItemKind::UserMessage, + "right"}}}}}; + insertionFixture.turns.front().status = "inProgress"; + insertionFixture.turns.front().active = true; + insertionFixture.turns.front().terminal = false; + codexui::ConversationWidget insertionConversation; + insertionConversation.resize(900, 700); + insertionConversation.show(); + insertionConversation.render( + makeState({insertionFixture}), + QStringLiteral("keyed-middle-insertion")); + settleTimeline(); + + QPointer left = segment( + insertionConversation, + QStringLiteral("message:message-keyed-left")); + QPointer right = segment( + insertionConversation, + QStringLiteral("message:message-keyed-right")); + QWidget* const leftAddress = left.data(); + QWidget* const rightAddress = right.data(); + + insertionFixture.turns.front().messages.insert( + insertionFixture.turns.front().messages.begin() + 1, + {"message-keyed-middle", + frontend::ThreadItemKind::UserMessage, + "middle"}); + insertionConversation.render( + makeState({insertionFixture}), + QStringLiteral("keyed-middle-insertion")); + settleTimeline(); + + QWidget* const middle = segment( + insertionConversation, + QStringLiteral("message:message-keyed-middle")); + QLayout* const orderedLayout = middle && middle->parentWidget() + ? middle->parentWidget()->layout() + : nullptr; + passed &= expect( + left && left.data() == leftAddress + && left.data() + == segment( + insertionConversation, + QStringLiteral("message:message-keyed-left")) + && right && right.data() == rightAddress + && right.data() + == segment( + insertionConversation, + QStringLiteral("message:message-keyed-right")), + "inserting a segment in the middle must preserve every unchanged keyed QWidget"); + passed &= expect( + orderedLayout && middle + && orderedLayout->indexOf(left.data()) == 0 + && orderedLayout->indexOf(middle) == 1 + && orderedLayout->indexOf(right.data()) == 2, + "a middle insertion must place the new segment between its surviving neighbors"); + return passed; +} + bool testInPlaceMessageReplacement() { ThreadFixture agentFixture{"in-place-agent", @@ -2587,6 +2913,7 @@ int main(int argc, char** argv) passed &= testHotTurnWindow(); passed &= testActivityDisclosureAndFullOutput(); passed &= testPointerPreservingAppend(); + passed &= testKeyedSegmentInsertion(); passed &= testInPlaceMessageReplacement(); passed &= testIncompleteThreadPresentation(); passed &= testIncompleteReplacementPreservesRenderedTimeline();