From be66069ab6dcd3b5676e2b00c3701ec2e184108f Mon Sep 17 00:00:00 2001 From: Volker Christian Date: Sat, 22 Aug 2026 16:32:25 +0200 Subject: [PATCH] Reconcile item upserts without full conversation refresh Introduce a structural thread scope for only resolved item upserts. Preserve exact append metadata through mailbox and frame coalescing, let deletion-capable scopes dominate, and continue from exact content application into keyed topology reconciliation without rebuilding unrelated presentation state. Call-site audit: stateUpdateScope has one production caller and 13 pre-existing direct test calls; only its two resolved ItemUpsert branches change. mergeScope has one production call. refreshState has 19 callers; only the 16 ms drain passes structural mode and the other 18 retain defaults. ConversationWidget::render has 80 current call sites (one production, 79 tests); the production caller passes the mode, the 76 pre-existing tests retain the default, and only the structural call among three new proof sites passes true. Deletion census: 152 lines removed under src/. ConversationWidget removes 78 lines by guarding/refactoring the existing title, completeness-proof, current-turn, summary, and render-signature paths into structural/full modes. Workbench removes 67 lines by extracting its inline frame accumulator and consolidating four pending-state fields. FrontendSessionWorker removes six old full-scope ItemUpsert lines in favor of structural mapping; FrontendSession removes one superseded merge line. No semantic block was discarded. Runtime proof covers both item-parent resolutions, legacy TurnUpsert full authority, bounded mailbox/full dominance, both exact/structural arrival orders through the production accumulator and UI, 50 new widgets with preserved identities, one incremental append with no rematerialization/replacement, and deletion destruction. No golden hash or protocol fingerprint changed. --- CMakeLists.txt | 2 + src/app/FrontendSession.cpp | 22 +- src/app/FrontendSession.h | 3 + src/app/FrontendSessionWorker.cpp | 35 ++- src/ui/ConversationWidget.cpp | 238 +++++++++++------ src/ui/ConversationWidget.h | 3 +- src/ui/PresentationRefreshAccumulator.cpp | 74 ++++++ src/ui/PresentationRefreshAccumulator.h | 16 ++ src/ui/WorkbenchWidget.cpp | 94 +++---- src/ui/WorkbenchWidget.h | 9 +- tests/ConversationLayoutTest.cpp | 176 +++++++++++++ tests/FrontendSessionTest.cpp | 259 ++++++++++++++++++- tests/PresentationRefreshAccumulatorTest.cpp | 104 ++++++++ 13 files changed, 877 insertions(+), 158 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ee69ff5..655970e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -145,6 +145,8 @@ if(BUILD_TESTING) src/ui/ExpandingPromptEditor.h src/ui/InspectorWidget.cpp src/ui/InspectorWidget.h + src/ui/PresentationRefreshAccumulator.cpp + src/ui/PresentationRefreshAccumulator.h src/ui/UpcomingTurnDock.cpp src/ui/UpcomingTurnDock.h ) diff --git a/src/app/FrontendSession.cpp b/src/app/FrontendSession.cpp index fa9e928..743ed56 100644 --- a/src/app/FrontendSession.cpp +++ b/src/app/FrontendSession.cpp @@ -61,8 +61,25 @@ void mergeScope(detail::StateUpdateScope& destination, if (!appendUniqueBounded(destination.affectedThreadIds, source.affectedThreadIds) || !appendUniqueBounded(destination.fullyAffectedThreadIds, - source.fullyAffectedThreadIds)) + source.fullyAffectedThreadIds)) { destination.allThreadsAffected = true; + } else { + for (const QString& threadId : destination.fullyAffectedThreadIds) + destination.structurallyAffectedThreadIds.removeAll(threadId); + for (const QString& threadId : + source.structurallyAffectedThreadIds) { + if (destination.fullyAffectedThreadIds.contains(threadId) + || destination.structurallyAffectedThreadIds.contains( + threadId)) + continue; + if (destination.structurallyAffectedThreadIds.size() + >= detail::maximumCoalescedPresentationIdentities) { + destination.allThreadsAffected = true; + break; + } + destination.structurallyAffectedThreadIds.push_back(threadId); + } + } } if (!destination.allInspectorsAffected && !appendUniqueBounded(destination.affectedInspectorThreadIds, @@ -165,6 +182,8 @@ void mergeScope(detail::StateUpdateScope& destination, > detail::maximumCoalescedPresentationIdentities || destination.fullyAffectedThreadIds.size() > detail::maximumCoalescedPresentationIdentities + || destination.structurallyAffectedThreadIds.size() + > detail::maximumCoalescedPresentationIdentities || destination.removedThreadIds.size() > detail::maximumCoalescedPresentationIdentities || static_cast(destination.affectedItemContents.size()) @@ -182,6 +201,7 @@ void mergeScope(detail::StateUpdateScope& destination, if (destination.allThreadsAffected) { destination.affectedThreadIds.clear(); destination.fullyAffectedThreadIds.clear(); + destination.structurallyAffectedThreadIds.clear(); // Keep exact removals even when the rest of the presentation scope // degrades to an all-thread refresh. Global omission provenance makes // a missing selected ID ambiguous without this bounded evidence. diff --git a/src/app/FrontendSession.h b/src/app/FrontendSession.h index 71d76ad..53e7fab 100644 --- a/src/app/FrontendSession.h +++ b/src/app/FrontendSession.h @@ -49,6 +49,9 @@ struct StateUpdateScope { QStringList affectedThreadIds; QStringList fullyAffectedThreadIds; + // Pure descendant additions: always a subset of affectedThreadIds. A + // full/deletion-capable scope for the same thread always dominates. + QStringList structurallyAffectedThreadIds; // Exact authoritative removals must survive mailbox coalescing. An // omitted thread is otherwise indistinguishable from one deleted by an // authoritative thread/read while the global snapshot remains bounded. diff --git a/src/app/FrontendSessionWorker.cpp b/src/app/FrontendSessionWorker.cpp index 00f53a1..02672d5 100644 --- a/src/app/FrontendSessionWorker.cpp +++ b/src/app/FrontendSessionWorker.cpp @@ -69,9 +69,26 @@ StateUpdateScope stateUpdateScope(const sdk::StateUpdate& update) }; const auto addFullyAffectedThread = [&scope, &addThread, &addUnique](std::string_view id) { addThread(id); - if (!scope.allThreadsAffected - && !addUnique(scope.fullyAffectedThreadIds, id)) + if (scope.allThreadsAffected) + return; + if (!addUnique(scope.fullyAffectedThreadIds, id)) { scope.allThreadsAffected = true; + return; + } + scope.structurallyAffectedThreadIds.removeAll( + QString::fromUtf8(id.data(), static_cast(id.size()))); + }; + const auto addStructurallyAffectedThread = + [&scope, &addThread, &addUnique](std::string_view id) { + addThread(id); + if (scope.allThreadsAffected) + return; + const QString threadId = QString::fromUtf8( + id.data(), static_cast(id.size())); + if (scope.fullyAffectedThreadIds.contains(threadId)) + return; + if (!addUnique(scope.structurallyAffectedThreadIds, id)) + scope.allThreadsAffected = true; }; const auto addInspectorThread = [&scope, &addUnique](std::string_view id) { if (!scope.allInspectorsAffected @@ -213,11 +230,16 @@ StateUpdateScope stateUpdateScope(const sdk::StateUpdate& update) } else if constexpr (std::is_same_v) { - if (value.threadId) - markThreadAndInspector(value.threadId->value); + if (value.threadId) { + addStructurallyAffectedThread(value.threadId->value); + addInspectorThread(value.threadId->value); + } else if (value.turnId) { - if (const auto* turn = update.state.turn(*value.turnId)) - markThreadAndInspector(turn->threadId.value); + if (const auto* turn = update.state.turn(*value.turnId)) { + addStructurallyAffectedThread( + turn->threadId.value); + addInspectorThread(turn->threadId.value); + } else { scope.allThreadsAffected = true; scope.allInspectorsAffected = true; @@ -308,6 +330,7 @@ StateUpdateScope stateUpdateScope(const sdk::StateUpdate& update) if (scope.allThreadsAffected) { scope.affectedThreadIds.clear(); scope.fullyAffectedThreadIds.clear(); + scope.structurallyAffectedThreadIds.clear(); scope.affectedItemContents.clear(); scope.coalescedContentDeltaBytes = 0; } diff --git a/src/ui/ConversationWidget.cpp b/src/ui/ConversationWidget.cpp index e8ac8ee..64955c6 100644 --- a/src/ui/ConversationWidget.cpp +++ b/src/ui/ConversationWidget.cpp @@ -2961,7 +2961,8 @@ void ConversationWidget::requestDeferredPresentationAtTail() void ConversationWidget::render(const sdk::State& state, const QString& threadId, bool newThreadDraft, - const ConversationContentUpdates* exactContentChanges) + const ConversationContentUpdates* exactContentChanges, + bool structurallyAffected) { auto* scrollBar = scrollArea->verticalScrollBar(); const int previousScroll = scrollBar->value(); @@ -2991,11 +2992,18 @@ void ConversationWidget::render(const sdk::State& state, && (!renderedThreadFullyLoaded || *renderedThreadFullyLoaded != thread->fullyLoaded); - upcomingTurnDock->setCanonicalConfiguration( - thread ? thread->executionConfiguration - : std::optional{}, - thread ? threadId : QString{}, - newThreadDraft); + const bool structuralReconciliation = structurallyAffected + && !threadChanged + && !threadCompletenessChanged + && thread && !newThreadDraft; + if (!structuralReconciliation) + { + upcomingTurnDock->setCanonicalConfiguration( + thread ? thread->executionConfiguration + : std::optional{}, + thread ? threadId : QString{}, + newThreadDraft); + } if (!threadChanged && shouldFreezePresentation(threadId, newThreadDraft)) { markPresentationDeferred(); @@ -3003,45 +3011,55 @@ void ConversationWidget::render(const sdk::State& state, } // Exact content appends cannot remove turns or items. Apply them before // the bounded incomplete-history proof so streaming on a partial thread - // remains proportional to the changed bytes. - if (exactContentChanges && !threadChanged && !threadCompletenessChanged - && thread && !newThreadDraft - && updateExactMessageContent(state, threadId, *exactContentChanges)) + // remains proportional to the changed bytes. A structural publication + // still has to derive the new segment topology, but must not apply these + // mutations a second time during that reconciliation. + const bool exactContentApplied = exactContentChanges + && !threadChanged + && !threadCompletenessChanged + && thread && !newThreadDraft + && updateExactMessageContent( + state, threadId, *exactContentChanges); + if (exactContentApplied && !structuralReconciliation) return; // A bounded replacement is not deletion authority. Keep the same-thread // widgets until an incomplete publication can account for every rendered // descendant; requester-local Merge will make that true, while Replace is // necessarily fullyLoaded and exact Absent changes the selection. - const bool renderedTimelineRetained = !renderedTurnIds.isEmpty(); - qsizetype recoveryInspectedItems = 0; - const bool incompleteTimelineRetained = - !thread || thread->fullyLoaded - || incompleteStateContainsRenderedTimeline( - state, - *thread, - renderedTurnIds, - renderedTurnItemRanges, - renderedSegmentIds, - renderedSegmentItemIds, - recoveryInspectedItems); - timelineHost->setProperty( - "recoveryInspectedTimelineItems", recoveryInspectedItems); - const bool incompleteTimelineRegressed = - !threadChanged && renderedTimelineRetained - && (selectedThreadUnresolved - || !incompleteTimelineRetained); - if (incompleteTimelineRegressed) - { - const QString recovery = QStringLiteral("History recovery pending"); - if (!threadDetail->text().contains(recovery)) + if (!structuralReconciliation) + { + const bool renderedTimelineRetained = !renderedTurnIds.isEmpty(); + qsizetype recoveryInspectedItems = 0; + const bool incompleteTimelineRetained = + !thread || thread->fullyLoaded + || incompleteStateContainsRenderedTimeline( + state, + *thread, + renderedTurnIds, + renderedTurnItemRanges, + renderedSegmentIds, + renderedSegmentItemIds, + recoveryInspectedItems); + timelineHost->setProperty( + "recoveryInspectedTimelineItems", recoveryInspectedItems); + const bool incompleteTimelineRegressed = + !threadChanged && renderedTimelineRetained + && (selectedThreadUnresolved + || !incompleteTimelineRetained); + if (incompleteTimelineRegressed) { - const QString detail = threadDetail->text(); - threadDetail->setText(detail.isEmpty() - ? recovery - : detail + QStringLiteral(" · ") + recovery); - threadDetail->setToolTip(threadDetail->text()); + const QString recovery = QStringLiteral("History recovery pending"); + if (!threadDetail->text().contains(recovery)) + { + const QString detail = threadDetail->text(); + threadDetail->setText( + detail.isEmpty() + ? recovery + : detail + QStringLiteral(" · ") + recovery); + threadDetail->setToolTip(threadDetail->text()); + } + return; } - return; } if (!thread && !threadChanged && !missingThreadPresentationChanged) return; @@ -3051,7 +3069,8 @@ void ConversationWidget::render(const sdk::State& state, deferredPresentationRequestScheduled = false; } const bool followLatest = threadChanged || wasNearBottom || followingLatest; - const bool exactContentOnly = exactContentChanges && !threadChanged + const bool exactContentOnly = !structuralReconciliation + && exactContentChanges && !threadChanged && !threadCompletenessChanged && thread && !newThreadDraft && !renderedSummaryKey.isEmpty(); const std::uint64_t generation = ++renderGeneration; @@ -3149,51 +3168,75 @@ void ConversationWidget::render(const sdk::State& state, } else { - const QString id = fromUtf8(thread->id.value); - const QString title = thread->title && !thread->title->empty() ? fromUtf8(*thread->title) : id; - threadTitle->setText(title); - threadTitle->setToolTip(title); - QStringList metadata; - if (thread->archived.value_or(false)) - metadata.append(QStringLiteral("Archived")); - else if (thread->status && !thread->status->empty()) - metadata.append(humanize(fromUtf8(*thread->status))); - if (thread->ephemeral.value_or(false)) - metadata.append(QStringLiteral("Temporary")); - metadata.append(QStringLiteral("%1 turn%2") - .arg(thread->orderedTurns.size()) - .arg(thread->orderedTurns.size() == 1 ? QString{} : QStringLiteral("s"))); - if (!thread->fullyLoaded) - metadata.append(QStringLiteral("History incomplete")); - threadDetail->setText(metadata.join(QStringLiteral(" · "))); - threadDetail->setToolTip(threadDetail->text()); + if (!structuralReconciliation) + { + const QString id = fromUtf8(thread->id.value); + const QString title = thread->title && !thread->title->empty() + ? fromUtf8(*thread->title) + : id; + threadTitle->setText(title); + threadTitle->setToolTip(title); + QStringList metadata; + if (thread->archived.value_or(false)) + metadata.append(QStringLiteral("Archived")); + else if (thread->status && !thread->status->empty()) + metadata.append(humanize(fromUtf8(*thread->status))); + if (thread->ephemeral.value_or(false)) + metadata.append(QStringLiteral("Temporary")); + metadata.append(QStringLiteral("%1 turn%2") + .arg(thread->orderedTurns.size()) + .arg(thread->orderedTurns.size() == 1 + ? QString{} + : QStringLiteral("s"))); + if (!thread->fullyLoaded) + metadata.append(QStringLiteral("History incomplete")); + threadDetail->setText(metadata.join(QStringLiteral(" · "))); + threadDetail->setToolTip(threadDetail->text()); + } const sdk::TurnState* currentTurn = nullptr; qsizetype currentIndex = -1; - for (qsizetype index = 0; index < static_cast(thread->orderedTurns.size()); ++index) + std::optional structuralWindow; + if (structuralReconciliation) + { + structuralWindow.emplace(latestTimelineWindow(state, *thread)); + if (!structuralWindow->turns.empty()) + currentTurn = structuralWindow->turns.back().turn; + } + else { - if (const auto* turn = state.turn(thread->id, thread->orderedTurns.at(index))) + for (qsizetype index = 0; + index < static_cast(thread->orderedTurns.size()); + ++index) { - currentTurn = turn; - currentIndex = index; + if (const auto* turn = state.turn( + thread->id, thread->orderedTurns.at(index))) + { + currentTurn = turn; + currentIndex = index; + } } } - const QByteArray summaryKey = exactContentOnly - ? renderedSummaryKey - : turnSummaryPresentationKey(currentTurn, currentIndex); - if (!exactContentOnly && summaryKey != renderedSummaryKey) + if (!structuralReconciliation) { - renderedSummaryKey = summaryKey; - turnFailure->hide(); - if (currentTurn) + const QByteArray summaryKey = exactContentOnly + ? renderedSummaryKey + : turnSummaryPresentationKey( + currentTurn, currentIndex); + if (!exactContentOnly && summaryKey != renderedSummaryKey) { - const QString failure = failureText(*currentTurn); - if (!failure.isEmpty()) + renderedSummaryKey = summaryKey; + turnFailure->hide(); + if (currentTurn) { - turnFailure->setText(failure); - turnFailure->setToolTip(failure); - turnFailure->show(); + const QString failure = failureText(*currentTurn); + if (!failure.isEmpty()) + { + turnFailure->setText(failure); + turnFailure->setToolTip(failure); + turnFailure->show(); + } } } } @@ -3220,7 +3263,9 @@ void ConversationWidget::render(const sdk::State& state, } else { - const TimelineWindow window = latestTimelineWindow(state, *thread); + const TimelineWindow window = structuralWindow + ? std::move(*structuralWindow) + : latestTimelineWindow(state, *thread); std::vector entries; entries.reserve(static_cast(window.renderedItems)); for (const TimelineTurnSlice& slice : window.turns) @@ -3460,7 +3505,7 @@ void ConversationWidget::render(const sdk::State& state, const ConversationContentUpdates* segmentContentChanges = nullptr; ConversationContentUpdates segmentContentStorage; bool explicitlyAffected = false; - if (oldWidget && exactContentOnly) + if (oldWidget && (exactContentOnly || exactContentApplied)) { for (const ConversationContentUpdate& update : *exactContentChanges) { @@ -3477,9 +3522,10 @@ void ConversationWidget::render(const sdk::State& state, segmentContentStorage.push_back(update); } explicitlyAffected = !segmentContentStorage.empty(); - if (!explicitlyAffected) + if (exactContentOnly && !explicitlyAffected) continue; - segmentContentChanges = &segmentContentStorage; + if (exactContentOnly && explicitlyAffected) + segmentContentChanges = &segmentContentStorage; } const bool typedPlanAvailable = turn->plan.has_value(); const bool turnStreaming = turnStreamsMessages(*turn); @@ -3493,6 +3539,44 @@ void ConversationWidget::render(const sdk::State& state, && renderedSegmentKeys.value(storage) == segmentKey) continue; + const bool exactMessageAlreadyApplied = + exactContentApplied && explicitlyAffected + && segment->items.size() == 1 + && segment->items.front() + && (segment->items.front()->kind.is( + frontend::ThreadItemKind::UserMessage) + || segment->items.front()->kind.is( + frontend::ThreadItemKind::AgentMessage)); + if (oldWidget && exactMessageAlreadyApplied) + { + const sdk::ItemState* item = segment->items.front(); + auto* status = oldWidget->findChild( + QStringLiteral("conversationMessageStatus")); + auto* content = oldWidget->findChild( + QStringLiteral("conversationMessageContent")); + auto* truncation = oldWidget->findChild( + QStringLiteral("conversationMessageTruncation")); + if (status && content && truncation) + { + const bool user = item->kind.is( + frontend::ThreadItemKind::UserMessage); + const bool metadataGeometryChanged = + applyMessageMetadata( + status, + content, + truncation, + messagePresentationMetadata( + *item, user, turnStreaming)); + renderedSegmentKeys.insert(storage, segmentKey); + timelineShrank = timelineShrank + || metadataGeometryChanged; + timelineGeometryChanged = + timelineGeometryChanged + || metadataGeometryChanged; + continue; + } + } + bool messageMayShrink = false; bool messageGeometryChanged = false; if (oldWidget diff --git a/src/ui/ConversationWidget.h b/src/ui/ConversationWidget.h index 1f0c335..e4b6a13 100644 --- a/src/ui/ConversationWidget.h +++ b/src/ui/ConversationWidget.h @@ -69,7 +69,8 @@ class ConversationWidget : public QWidget void render(const ai::openai::codex::frontend::client::State& state, const QString& threadId, bool newThreadDraft = false, - const ConversationContentUpdates* exactContentChanges = nullptr); + const ConversationContentUpdates* exactContentChanges = nullptr, + bool structurallyAffected = false); void setModelCatalog(const std::vector& catalog); [[nodiscard]] bool updateExactMessageContent( const ai::openai::codex::frontend::client::State& state, diff --git a/src/ui/PresentationRefreshAccumulator.cpp b/src/ui/PresentationRefreshAccumulator.cpp index f8f5112..eb6a9f4 100644 --- a/src/ui/PresentationRefreshAccumulator.cpp +++ b/src/ui/PresentationRefreshAccumulator.cpp @@ -17,6 +17,80 @@ namespace { } // namespace +void SelectedPresentationRefreshAccumulator::clear() noexcept +{ + refreshPending = false; + fullRefreshPending = false; + structuralReconciliationPending = false; + contentChanges.clear(); + retainedContentUtf8Bytes = 0; +} + +void mergeSelectedPresentationRefresh( + SelectedPresentationRefreshAccumulator& accumulator, + const StateUpdateScope& scope, + const QString& selectedThreadId, + bool awaitedSelectionAffected) +{ + accumulator.refreshPending = true; + const bool requiresFullRefresh = scope.allThreadsAffected + || awaitedSelectionAffected + || scope.fullyAffectedThreadIds.contains( + selectedThreadId); + if (requiresFullRefresh) + { + accumulator.fullRefreshPending = true; + accumulator.structuralReconciliationPending = false; + accumulator.contentChanges.clear(); + accumulator.retainedContentUtf8Bytes = 0; + return; + } + if (accumulator.fullRefreshPending) + return; + + const bool requiresStructuralReconciliation = + scope.structurallyAffectedThreadIds.contains(selectedThreadId); + accumulator.structuralReconciliationPending = + accumulator.structuralReconciliationPending + || requiresStructuralReconciliation; + + // A worker mailbox publication is individually bounded, but more than + // one publication can reach the GUI during this 16 ms frame window. + // Bound the aggregate again and fall back to the newest authoritative + // State instead of growing presentation metadata. + bool foundExactContent = false; + for (const auto& identity : scope.affectedItemContents) + { + if (identity.threadId != selectedThreadId) + continue; + foundExactContent = true; + if (mergeConversationContentUpdate( + accumulator.contentChanges, + accumulator.retainedContentUtf8Bytes, + identity) + == BoundedMergeResult::CapacityExceeded) + { + accumulator.fullRefreshPending = true; + accumulator.structuralReconciliationPending = false; + accumulator.contentChanges.clear(); + accumulator.retainedContentUtf8Bytes = 0; + return; + } + } + + // A structural addition deliberately carries no exact item identity: its + // segment list is reconciled from State while any accumulated exact text + // changes remain available. Other conversation-affecting updates without + // an identity still require the existing authoritative refresh. + if (!foundExactContent && !requiresStructuralReconciliation) + { + accumulator.fullRefreshPending = true; + accumulator.structuralReconciliationPending = false; + accumulator.contentChanges.clear(); + accumulator.retainedContentUtf8Bytes = 0; + } +} + BoundedMergeResult mergeConversationContentUpdate( ConversationContentUpdates& updates, std::uint64_t& retainedUtf8Bytes, diff --git a/src/ui/PresentationRefreshAccumulator.h b/src/ui/PresentationRefreshAccumulator.h index 626bd9d..ca4fa02 100644 --- a/src/ui/PresentationRefreshAccumulator.h +++ b/src/ui/PresentationRefreshAccumulator.h @@ -16,6 +16,22 @@ namespace codexui::detail { enum class BoundedMergeResult { Retained, CapacityExceeded }; +struct SelectedPresentationRefreshAccumulator { + bool refreshPending = false; + bool fullRefreshPending = false; + bool structuralReconciliationPending = false; + ConversationContentUpdates contentChanges; + std::uint64_t retainedContentUtf8Bytes = 0; + + void clear() noexcept; +}; + +void mergeSelectedPresentationRefresh( + SelectedPresentationRefreshAccumulator& accumulator, + const StateUpdateScope& scope, + const QString& selectedThreadId, + bool awaitedSelectionAffected); + [[nodiscard]] BoundedMergeResult mergeConversationContentUpdate( ConversationContentUpdates& updates, std::uint64_t& retainedUtf8Bytes, diff --git a/src/ui/WorkbenchWidget.cpp b/src/ui/WorkbenchWidget.cpp index c8525a1..604d97f 100644 --- a/src/ui/WorkbenchWidget.cpp +++ b/src/ui/WorkbenchWidget.cpp @@ -352,50 +352,11 @@ void WorkbenchWidget::scheduleStateRefresh(const detail::StateUpdateScope& scope && !submittedTurnAffected && !automaticResumeAffected) return; if (selectedAffected) - { - selectedPresentationRefreshPending = true; - const bool requiresFullRefresh = scope.allThreadsAffected || awaitedSelectionAffected - || scope.fullyAffectedThreadIds.contains(selectedThreadId); - if (requiresFullRefresh) - { - selectedPresentationFullRefreshPending = true; - selectedContentRefreshPending.clear(); - selectedContentRefreshPendingBytes = 0; - } - else if (!selectedPresentationFullRefreshPending) - { - // A worker mailbox publication is individually bounded, but more - // than one publication can reach the GUI during this 16 ms frame - // window. Bound the aggregate again and fall back to the newest - // authoritative State instead of growing presentation metadata. - bool foundExactContent = false; - for (const auto& identity : scope.affectedItemContents) - { - if (identity.threadId != selectedThreadId) - continue; - foundExactContent = true; - if (detail::mergeConversationContentUpdate( - selectedContentRefreshPending, - selectedContentRefreshPendingBytes, - identity) - == detail::BoundedMergeResult::CapacityExceeded) - { - selectedPresentationFullRefreshPending = true; - selectedContentRefreshPending.clear(); - selectedContentRefreshPendingBytes = 0; - break; - } - } - // A conversation-affecting update without an exact item identity - // must retain the existing bounded full reconciliation. - if (!foundExactContent) - { - selectedPresentationFullRefreshPending = true; - selectedContentRefreshPending.clear(); - selectedContentRefreshPendingBytes = 0; - } - } - } + detail::mergeSelectedPresentationRefresh( + selectedPresentationRefresh, + scope, + selectedThreadId, + awaitedSelectionAffected); inspectorRefreshPending = inspectorRefreshPending || selectedInspectorAffected; sidebarRefreshPending = sidebarRefreshPending || scope.sidebarAffected; if (scope.sidebarAffected) { @@ -432,19 +393,24 @@ void WorkbenchWidget::scheduleStateRefresh(const detail::StateUpdateScope& scope if (!stateRefreshPending) return; stateRefreshPending = false; - const bool refreshSelectedPresentation = selectedPresentationRefreshPending; + const bool refreshSelectedPresentation = + selectedPresentationRefresh.refreshPending; const bool refreshInspector = inspectorRefreshPending; const bool refreshSidebar = sidebarRefreshPending; const bool refreshFullSidebar = sidebarFullRefreshPending; QStringList sidebarThreadChanges = std::move(sidebarThreadRefreshPending); - const bool exactContentOnly = refreshSelectedPresentation - && !selectedPresentationFullRefreshPending - && !selectedContentRefreshPending.empty(); - ConversationContentUpdates exactContentChanges = std::move(selectedContentRefreshPending); - selectedPresentationRefreshPending = false; - selectedPresentationFullRefreshPending = false; - selectedContentRefreshPending.clear(); - selectedContentRefreshPendingBytes = 0; + const bool exactContentAvailable = refreshSelectedPresentation + && !selectedPresentationRefresh.fullRefreshPending + && !selectedPresentationRefresh.contentChanges.empty(); + const bool structuralReconciliation = refreshSelectedPresentation + && !selectedPresentationRefresh.fullRefreshPending + && selectedPresentationRefresh + .structuralReconciliationPending; + const bool exactContentOnly = exactContentAvailable + && !structuralReconciliation; + ConversationContentUpdates exactContentChanges = + std::move(selectedPresentationRefresh.contentChanges); + selectedPresentationRefresh.clear(); inspectorRefreshPending = false; sidebarRefreshPending = false; sidebarFullRefreshPending = false; @@ -458,10 +424,11 @@ void WorkbenchWidget::scheduleStateRefresh(const detail::StateUpdateScope& scope refreshState(refreshSelectedPresentation, refreshInspector, refreshSidebar, - exactContentOnly ? &exactContentChanges : nullptr, + exactContentAvailable ? &exactContentChanges : nullptr, refreshSidebar && !refreshFullSidebar && !sidebarThreadChanges.isEmpty() ? &sidebarThreadChanges - : nullptr); + : nullptr, + structuralReconciliation); }); } @@ -539,13 +506,11 @@ void WorkbenchWidget::refreshState(bool refreshSelectedPresentation, bool refreshInspector, bool refreshSidebar, const ConversationContentUpdates* exactContentChanges, - const QStringList* sidebarThreadChanges) + const QStringList* sidebarThreadChanges, + bool requiresStructuralReconciliation) { stateRefreshPending = false; - selectedPresentationRefreshPending = false; - selectedPresentationFullRefreshPending = false; - selectedContentRefreshPending.clear(); - selectedContentRefreshPendingBytes = 0; + selectedPresentationRefresh.clear(); inspectorRefreshPending = false; sidebarRefreshPending = false; sidebarFullRefreshPending = false; @@ -595,6 +560,8 @@ void WorkbenchWidget::refreshState(bool refreshSelectedPresentation, refreshSelectedPresentation = refreshSelectedPresentation || selectionChanged; refreshInspector = refreshInspector || selectionChanged; refreshSidebar = refreshSidebar || selectionChanged; + requiresStructuralReconciliation = requiresStructuralReconciliation + && !selectionChanged; if (refreshSidebar) { if (!selectionChanged && sidebarThreadChanges && !sidebarThreadChanges->isEmpty()) @@ -610,7 +577,8 @@ void WorkbenchWidget::refreshState(bool refreshSelectedPresentation, conversation->render(state, selectedThreadId, false, - selectionChanged ? nullptr : exactContentChanges); + selectionChanged ? nullptr : exactContentChanges, + requiresStructuralReconciliation); } reconcileAttachmentStaging(); reconcileSubmittedTurnSettings(); @@ -626,7 +594,7 @@ void WorkbenchWidget::refreshState(bool refreshSelectedPresentation, && !selectedThreadId.isEmpty() && !awaitingSelectedThread && omittedThreads > 0; - if (refreshSelectedPresentation) { + if (refreshSelectedPresentation && !requiresStructuralReconciliation) { const QString context = ready && selected && selected->cwd ? QString::fromStdString(selected->cwd->value) : QStringLiteral("No thread context"); @@ -641,7 +609,9 @@ void WorkbenchWidget::refreshState(bool refreshSelectedPresentation, ? context.left(20) + QChar(0x2026) + context.right(20) : context); threadContextStatus->setToolTip(ready && selected && selected->cwd ? plainTooltip(context) : QString{}); + } + if (refreshSelectedPresentation) { std::size_t agentActivities = 0; if (const auto* turn = ready ? latestTurn(state, selected) : nullptr) { for (const auto& itemId : turn->orderedItems) { diff --git a/src/ui/WorkbenchWidget.h b/src/ui/WorkbenchWidget.h index 68d7f64..8154298 100644 --- a/src/ui/WorkbenchWidget.h +++ b/src/ui/WorkbenchWidget.h @@ -5,6 +5,7 @@ #include "ui/InteractiveRequestDialog.h" #include "ui/ConversationWidget.h" +#include "ui/PresentationRefreshAccumulator.h" #include "ui/ThreadSetupDialog.h" #include "ui/UpcomingTurnDock.h" @@ -124,7 +125,8 @@ class WorkbenchWidget : public QWidget bool refreshInspector = true, bool refreshSidebar = true, const ConversationContentUpdates* exactContentChanges = nullptr, - const QStringList* sidebarThreadChanges = nullptr); + const QStringList* sidebarThreadChanges = nullptr, + bool requiresStructuralReconciliation = false); void refreshControls(); void refreshControllerStatus(); [[nodiscard]] bool writeOperationBusy() const noexcept; @@ -250,10 +252,7 @@ class WorkbenchWidget : public QWidget bool requestControllerAcquireInFlight = false; bool requestResponseInFlight = false; bool stateRefreshPending = false; - bool selectedPresentationRefreshPending = false; - bool selectedPresentationFullRefreshPending = false; - ConversationContentUpdates selectedContentRefreshPending; - std::uint64_t selectedContentRefreshPendingBytes = 0; + detail::SelectedPresentationRefreshAccumulator selectedPresentationRefresh; bool inspectorRefreshPending = false; bool sidebarRefreshPending = false; bool sidebarFullRefreshPending = false; diff --git a/tests/ConversationLayoutTest.cpp b/tests/ConversationLayoutTest.cpp index b6feb4f..fedbe25 100644 --- a/tests/ConversationLayoutTest.cpp +++ b/tests/ConversationLayoutTest.cpp @@ -2,6 +2,7 @@ #include "ui/ConversationWidget.h" #include "ui/InspectorWidget.h" +#include "ui/PresentationRefreshAccumulator.h" #include #include @@ -1383,6 +1384,180 @@ bool testKeyedSegmentInsertion() return passed; } +bool runStructuralItemUpsertReconciliation( + bool exactFirst, bool verifyDeletion) +{ + ThreadFixture fixture{ + "structural-item-upserts", + {{"turn-structural-item-upserts", + {{"structural-stream", + frontend::ThreadItemKind::AgentMessage, + "streaming prefix", + "started"}, + {"structural-stable", + frontend::ThreadItemKind::UserMessage, + "stable prompt"}}}}}; + fixture.turns.front().status = "inProgress"; + fixture.turns.front().active = true; + fixture.turns.front().terminal = false; + + codexui::ConversationWidget conversation; + conversation.resize(900, 700); + conversation.show(); + conversation.render( + makeState({fixture}), QStringLiteral("structural-item-upserts")); + settleTimeline(); + + QPointer stream = segment( + conversation, QStringLiteral("message:structural-stream")); + QPointer stable = segment( + conversation, QStringLiteral("message:structural-stable")); + QPointer streamContent = messageContent(stream); + QPointer streamStatus = messageLabel( + stream, QStringLiteral("conversationMessageStatus")); + QWidget* const streamAddress = stream.data(); + QWidget* const stableAddress = stable.data(); + QWidget* const streamContentAddress = streamContent.data(); + const qulonglong appendCountBefore = streamContent + ? streamContent + ->property("streamAppendCount") + .toULongLong() + : 0; + const qulonglong materializationsBefore = + streamContent + ? streamContent->property("sourceMaterializationCount").toULongLong() + : 0; + const qulonglong replacementsBefore = + streamContent + ? streamContent->property("fullReplacementCount").toULongLong() + : 0; + + const QString delta = QStringLiteral(" plus exact delta"); + fixture.turns.front().messages.front().text += delta.toStdString(); + fixture.turns.front().messages.front().status = "completed"; + for (int index = 0; index < 50; ++index) + { + fixture.turns.front().messages.push_back( + {"structural-added-" + std::to_string(index), + frontend::ThreadItemKind::UserMessage, + "new prompt " + std::to_string(index)}); + } + codexui::detail::StateUpdateScope structuralScope; + structuralScope.affectedThreadIds.push_back( + QStringLiteral("structural-item-upserts")); + structuralScope.structurallyAffectedThreadIds.push_back( + QStringLiteral("structural-item-upserts")); + codexui::detail::StateUpdateScope exactScope; + exactScope.affectedThreadIds.push_back( + QStringLiteral("structural-item-upserts")); + exactScope.affectedItemContents.push_back({ + QStringLiteral("structural-item-upserts"), + QStringLiteral("turn-structural-item-upserts"), + QStringLiteral("structural-stream"), + client::ItemContentChannel::AgentText, + codexui::detail::StateUpdateScope::ItemContentAppend{ + std::string_view("streaming prefix").size(), + 0, + delta.toUtf8(), + }, + }); + exactScope.coalescedContentDeltaBytes = + static_cast(delta.toUtf8().size()); + codexui::detail::SelectedPresentationRefreshAccumulator accumulator; + const auto accumulate = [&accumulator]( + const codexui::detail::StateUpdateScope& scope) + { + codexui::detail::mergeSelectedPresentationRefresh( + accumulator, + scope, + QStringLiteral("structural-item-upserts"), + false); + }; + if (exactFirst) + { + accumulate(exactScope); + accumulate(structuralScope); + } + else + { + accumulate(structuralScope); + accumulate(exactScope); + } + conversation.render( + makeState({fixture}), + QStringLiteral("structural-item-upserts"), + false, + &accumulator.contentChanges, + accumulator.structuralReconciliationPending); + settleTimeline(); + + bool everyAdditionRendered = true; + for (int index = 0; index < 50; ++index) + { + everyAdditionRendered = everyAdditionRendered + && segment( + conversation, + QStringLiteral("message:structural-added-%1") + .arg(index)); + } + bool passed = expect( + accumulator.refreshPending && !accumulator.fullRefreshPending + && accumulator.structuralReconciliationPending + && accumulator.contentChanges.size() == 1 + && everyAdditionRendered && stream && stream.data() == streamAddress + && stable && stable.data() == stableAddress + && streamContent && streamContent.data() == streamContentAddress, + "a structural batch of 50 item upserts must materialize every new segment while preserving every unchanged QWidget"); + passed &= expect( + streamContent + && messageSourceText(streamContent) + == QStringLiteral("streaming prefix plus exact delta") + && streamContent->property("streamAppendCount").toULongLong() + == appendCountBefore + 1 + && streamContent->property("sourceMaterializationCount").toULongLong() + == materializationsBefore + && streamContent->property("fullReplacementCount").toULongLong() + == replacementsBefore + && streamStatus + && streamStatus->text() == QStringLiteral("Completed"), + "a coalesced exact append and structural batch must apply the delta once, refresh metadata, and never replace or rematerialize canonical content"); + + if (!verifyDeletion) + return passed; + + const auto removedPosition = std::find_if( + fixture.turns.front().messages.begin(), + fixture.turns.front().messages.end(), + [](const MessageFixture& message) + { + return message.id == "structural-added-24"; + }); + QPointer removed = segment( + conversation, QStringLiteral("message:structural-added-24")); + fixture.turns.front().messages.erase(removedPosition); + conversation.render( + makeState({fixture}), QStringLiteral("structural-item-upserts")); + const bool removedUntracked = + !codexui::ConversationWidgetTestAccess::tracksSegment( + conversation, + QStringLiteral("turn-structural-item-upserts"), + QStringLiteral("message:structural-added-24")); + settleTimeline(); + passed &= expect( + removedUntracked + && !segment(conversation, QStringLiteral("message:structural-added-24")) + && !removed && stream && stream.data() == streamAddress, + "a deletion-capable reconciliation must untrack and destroy the removed segment without replacing survivors"); + return passed; +} + +bool testStructuralItemUpsertReconciliation() +{ + bool passed = runStructuralItemUpsertReconciliation(true, true); + passed &= runStructuralItemUpsertReconciliation(false, false); + return passed; +} + bool testInPlaceMessageReplacement() { ThreadFixture agentFixture{"in-place-agent", @@ -2914,6 +3089,7 @@ int main(int argc, char** argv) passed &= testActivityDisclosureAndFullOutput(); passed &= testPointerPreservingAppend(); passed &= testKeyedSegmentInsertion(); + passed &= testStructuralItemUpsertReconciliation(); passed &= testInPlaceMessageReplacement(); passed &= testIncompleteThreadPresentation(); passed &= testIncompleteReplacementPreservesRenderedTimeline(); diff --git a/tests/FrontendSessionTest.cpp b/tests/FrontendSessionTest.cpp index 843e431..c885141 100644 --- a/tests/FrontendSessionTest.cpp +++ b/tests/FrontendSessionTest.cpp @@ -567,6 +567,46 @@ bool testScopedItemPresentationChanges() ai::openai::codex::typed::TurnId{"target-turn"}}); const auto scoped = codexui::detail::stateUpdateScope(scopedUpdate); + codexui::FrontendSessionWorker resolvedParentSession; + std::vector resolvedParentOutbound; + frontend::Json resolvedParentThreads = frontend::Json::array({ + frontend::Json{ + {"id", "resolved-parent-thread"}, + {"fullyLoaded", true}, + {"turns", + frontend::Json::array({frontend::Json{ + {"id", "resolved-parent-turn"}, + {"threadId", "resolved-parent-thread"}, + {"status", "completed"}, + {"active", false}, + {"terminal", true}, + {"items", frontend::Json::array()}, + {"extensions", frontend::Json::object()}, + }})}, + {"extensions", frontend::Json::object()}, + }, + }); + const bool resolvedParentReady = + codexui::FrontendSessionWorkerTestAccess:: + synchronizeWithCapturedTransport( + resolvedParentSession, + resolvedParentOutbound, + std::move(resolvedParentThreads)); + sdk::StateUpdate resolvedParentItemUpdate; + resolvedParentItemUpdate.state = resolvedParentSession.state(); + resolvedParentItemUpdate.changes.push_back(sdk::ItemUpsertedChange{ + ai::openai::codex::typed::ItemId{"resolved-parent-item"}, + std::nullopt, + ai::openai::codex::typed::TurnId{"resolved-parent-turn"}}); + const auto resolvedParentItem = + codexui::detail::stateUpdateScope(resolvedParentItemUpdate); + sdk::StateUpdate resolvedTurnUpdate; + resolvedTurnUpdate.state = resolvedParentSession.state(); + resolvedTurnUpdate.changes.push_back(sdk::TurnUpsertedChange{ + ai::openai::codex::typed::TurnId{"resolved-parent-turn"}}); + const auto resolvedTurn = + codexui::detail::stateUpdateScope(resolvedTurnUpdate); + sdk::StateUpdate streamedUpdate; streamedUpdate.changes.push_back( sdk::ItemContentReplacedChange{ai::openai::codex::typed::ItemId{"streamed-item"}, @@ -644,6 +684,21 @@ bool testScopedItemPresentationChanges() sdk::ThreadUpsertedChange{ai::openai::codex::typed::ThreadId{"target-thread"}}); const auto threadScoped = codexui::detail::stateUpdateScope(threadUpdate); + sdk::StateUpdate structuralThenFullUpdate = scopedUpdate; + structuralThenFullUpdate.changes.push_back( + sdk::ThreadUpsertedChange{ + ai::openai::codex::typed::ThreadId{"target-thread"}}); + const auto structuralThenFull = + codexui::detail::stateUpdateScope(structuralThenFullUpdate); + + sdk::StateUpdate fullThenStructuralUpdate = threadUpdate; + fullThenStructuralUpdate.changes.push_back(sdk::ItemUpsertedChange{ + ai::openai::codex::typed::ItemId{"duplicate-item"}, + ai::openai::codex::typed::ThreadId{"target-thread"}, + ai::openai::codex::typed::TurnId{"target-turn"}}); + const auto fullThenStructural = + codexui::detail::stateUpdateScope(fullThenStructuralUpdate); + sdk::StateUpdate removedThreadUpdate; removedThreadUpdate.changes.push_back( sdk::ThreadRemovedChange{ai::openai::codex::typed::ThreadId{"removed-thread"}}); @@ -668,6 +723,7 @@ bool testScopedItemPresentationChanges() bool passed = expect(unresolvedTurn.affectedThreadIds.empty() && unresolvedTurn.fullyAffectedThreadIds.empty() + && unresolvedTurn.structurallyAffectedThreadIds.empty() && unresolvedTurn.affectedInspectorThreadIds.empty() && unresolvedTurn.allThreadsAffected && unresolvedTurn.allInspectorsAffected @@ -676,15 +732,40 @@ bool testScopedItemPresentationChanges() && unresolvedTurn.hasPresentationChange, "a turn upsert without a unique parent lookup must conservatively refresh all threads"); passed &= expect(scoped.affectedThreadIds == QStringList{QStringLiteral("target-thread")} - && scoped.fullyAffectedThreadIds + && scoped.fullyAffectedThreadIds.empty() + && scoped.structurallyAffectedThreadIds == QStringList{QStringLiteral("target-thread")} && scoped.affectedInspectorThreadIds == QStringList{QStringLiteral("target-thread")} && !scoped.allThreadsAffected && !scoped.allInspectorsAffected && !scoped.sidebarAffected && scoped.hasPresentationChange, - "a scoped item upsert must refresh its canonical conversation and Inspector"); + "a scoped item upsert must structurally reconcile its canonical conversation and refresh its Inspector"); + passed &= expect( + resolvedParentReady + && resolvedParentItem.affectedThreadIds + == QStringList{QStringLiteral("resolved-parent-thread")} + && resolvedParentItem.fullyAffectedThreadIds.empty() + && resolvedParentItem.structurallyAffectedThreadIds + == QStringList{QStringLiteral("resolved-parent-thread")} + && resolvedParentItem.affectedInspectorThreadIds + == QStringList{QStringLiteral("resolved-parent-thread")} + && !resolvedParentItem.allThreadsAffected + && !resolvedParentItem.allInspectorsAffected, + "an item upsert resolved through its retained turn must use the structural conversation scope"); + passed &= expect( + resolvedTurn.affectedThreadIds + == QStringList{QStringLiteral("resolved-parent-thread")} + && resolvedTurn.fullyAffectedThreadIds + == QStringList{QStringLiteral("resolved-parent-thread")} + && resolvedTurn.structurallyAffectedThreadIds.empty() + && resolvedTurn.affectedInspectorThreadIds + == QStringList{QStringLiteral("resolved-parent-thread")} + && resolvedTurn.affectedSidebarThreadIds + == QStringList{QStringLiteral("resolved-parent-thread")}, + "a resolved turn upsert must remain deletion-capable while legacy turn.updated can replace its items"); passed &= expect(streamed.affectedThreadIds == QStringList{QStringLiteral("target-thread")} && streamed.fullyAffectedThreadIds.empty() + && streamed.structurallyAffectedThreadIds.empty() && streamed.affectedInspectorThreadIds.empty() && streamed.affectedItemContents == std::vector{ @@ -698,6 +779,7 @@ bool testScopedItemPresentationChanges() == QStringList{QStringLiteral("target-thread")} && partiallyScoped.fullyAffectedThreadIds == QStringList{QStringLiteral("target-thread")} + && partiallyScoped.structurallyAffectedThreadIds.empty() && partiallyScoped.affectedItemContents.empty() && !partiallyScoped.allThreadsAffected, "partially scoped item content must require bounded full thread reconciliation"); @@ -722,18 +804,23 @@ bool testScopedItemPresentationChanges() && oversizedAppend.coalescedContentDeltaBytes == 0, "an oversized append hint must degrade to an authoritative replacement without entering the GUI mailbox"); passed &= expect(mixed.fullyAffectedThreadIds - == QStringList{QStringLiteral("target-thread")} + .empty() + && mixed.structurallyAffectedThreadIds + == QStringList{QStringLiteral("target-thread")} && mixed.affectedItemContents.size() == 1 && !mixed.allThreadsAffected, - "a structural change mixed with exact content must require full thread reconciliation"); + "a structural item upsert mixed with exact content must preserve both presentation hints"); passed &= expect(unscoped.affectedThreadIds.empty() && unscoped.allThreadsAffected && unscoped.fullyAffectedThreadIds.empty() + && unscoped.structurallyAffectedThreadIds.empty() && unscoped.affectedItemContents.empty() && unscoped.affectedInspectorThreadIds.empty() && unscoped.allInspectorsAffected && !unscoped.sidebarAffected && unscoped.hasPresentationChange, "an unscoped item change must conservatively refresh all thread-bound presentations"); - passed &= expect(replacement.allThreadsAffected && replacement.allInspectorsAffected + passed &= expect(replacement.allThreadsAffected + && replacement.structurallyAffectedThreadIds.empty() + && replacement.allInspectorsAffected && replacement.allSidebarThreadsAffected && replacement.sidebarAffected && replacement.hasPresentationChange, "a State replacement must conservatively refresh every presentation"); @@ -741,6 +828,7 @@ bool testScopedItemPresentationChanges() == QStringList{QStringLiteral("target-thread")} && threadScoped.fullyAffectedThreadIds == QStringList{QStringLiteral("target-thread")} + && threadScoped.structurallyAffectedThreadIds.empty() && threadScoped.affectedInspectorThreadIds == QStringList{QStringLiteral("target-thread")} && threadScoped.affectedSidebarThreadIds @@ -750,11 +838,20 @@ bool testScopedItemPresentationChanges() && !threadScoped.allSidebarThreadsAffected && threadScoped.sidebarAffected, "a thread upsert must target only its conversation, Inspector dependencies, and Sidebar row"); + passed &= expect( + structuralThenFull.fullyAffectedThreadIds + == QStringList{QStringLiteral("target-thread")} + && structuralThenFull.structurallyAffectedThreadIds.empty() + && fullThenStructural.fullyAffectedThreadIds + == QStringList{QStringLiteral("target-thread")} + && fullThenStructural.structurallyAffectedThreadIds.empty(), + "deletion-capable mapper scope must dominate structural scope in either change order"); passed &= expect( removedThreadScoped.affectedThreadIds == QStringList{QStringLiteral("removed-thread")} && removedThreadScoped.fullyAffectedThreadIds == QStringList{QStringLiteral("removed-thread")} + && removedThreadScoped.structurallyAffectedThreadIds.empty() && removedThreadScoped.removedThreadIds == QStringList{QStringLiteral("removed-thread")} && removedThreadScoped.affectedSidebarThreadIds @@ -771,6 +868,7 @@ bool testScopedItemPresentationChanges() && boundedIdentities.allSidebarThreadsAffected && boundedIdentities.affectedThreadIds.empty() && boundedIdentities.fullyAffectedThreadIds.empty() + && boundedIdentities.structurallyAffectedThreadIds.empty() && boundedIdentities.affectedInspectorThreadIds.empty() && boundedIdentities.affectedSidebarThreadIds.empty(), "an oversized identity batch must stop at the presentation bound and degrade to full refreshes"); @@ -2504,6 +2602,133 @@ bool testFacadeGenerationGating() return passed; } +bool testFacadeStructuralScopeMerge() +{ + codexui::FrontendSession session; + std::optional delivered; + QObject::connect( + &session, + &codexui::FrontendSession::stateChanged, + [&delivered](const auto& scope) { delivered = scope; }); + + const auto structuralScope = [](QString threadId) { + codexui::detail::StateUpdateScope scope; + scope.affectedThreadIds.push_back(threadId); + scope.structurallyAffectedThreadIds.push_back( + std::move(threadId)); + scope.hasPresentationChange = true; + return scope; + }; + const auto fullScope = [](QString threadId) { + codexui::detail::StateUpdateScope scope; + scope.affectedThreadIds.push_back(threadId); + scope.fullyAffectedThreadIds.push_back(std::move(threadId)); + scope.hasPresentationChange = true; + return scope; + }; + const auto exactScope = [](QString threadId, QString itemId) { + codexui::detail::StateUpdateScope scope; + scope.affectedThreadIds.push_back(threadId); + scope.affectedItemContents.push_back({ + threadId, + QStringLiteral("turn"), + std::move(itemId), + sdk::ItemContentChannel::AgentText, + codexui::detail::StateUpdateScope::ItemContentAppend{ + 4, + 0, + QByteArray(" delta"), + }, + }); + scope.coalescedContentDeltaBytes = 6; + scope.hasPresentationChange = true; + return scope; + }; + const auto preservedExact = [](const auto& scope, QStringView itemId) { + return scope.affectedItemContents.size() == 1 + && scope.affectedItemContents.front().itemId == itemId + && scope.affectedItemContents.front().append + && scope.affectedItemContents.front().append->deltaUtf8 + == QByteArray(" delta") + && scope.coalescedContentDeltaBytes == 6; + }; + + codexui::FrontendSessionFacadeTestAccess::enqueueState( + session, + 1, + exactScope(QStringLiteral("exact-first"), + QStringLiteral("exact-first-item"))); + codexui::FrontendSessionFacadeTestAccess::enqueueState( + session, 1, structuralScope(QStringLiteral("exact-first"))); + QCoreApplication::processEvents(); + bool passed = expect( + delivered + && delivered->structurallyAffectedThreadIds + == QStringList{QStringLiteral("exact-first")} + && delivered->fullyAffectedThreadIds.empty() + && preservedExact(*delivered, QStringView{u"exact-first-item"}), + "exact append metadata followed by structural scope must survive mailbox coalescing"); + + delivered.reset(); + codexui::FrontendSessionFacadeTestAccess::enqueueState( + session, 1, structuralScope(QStringLiteral("structural-first"))); + codexui::FrontendSessionFacadeTestAccess::enqueueState( + session, + 1, + exactScope(QStringLiteral("structural-first"), + QStringLiteral("structural-first-item"))); + QCoreApplication::processEvents(); + passed &= expect( + delivered + && delivered->structurallyAffectedThreadIds + == QStringList{QStringLiteral("structural-first")} + && delivered->fullyAffectedThreadIds.empty() + && preservedExact(*delivered, + QStringView{u"structural-first-item"}), + "structural scope followed by exact append metadata must survive mailbox coalescing"); + + delivered.reset(); + codexui::FrontendSessionFacadeTestAccess::enqueueState( + session, + 1, + exactScope(QStringLiteral("full-later"), + QStringLiteral("full-later-item"))); + codexui::FrontendSessionFacadeTestAccess::enqueueState( + session, 1, structuralScope(QStringLiteral("full-later"))); + codexui::FrontendSessionFacadeTestAccess::enqueueState( + session, 1, fullScope(QStringLiteral("full-later"))); + QCoreApplication::processEvents(); + passed &= expect( + delivered + && delivered->fullyAffectedThreadIds + == QStringList{QStringLiteral("full-later")} + && delivered->structurallyAffectedThreadIds.empty() + && preservedExact(*delivered, QStringView{u"full-later-item"}), + "a later deletion-capable scope must dominate structural scope without discarding exact metadata"); + + delivered.reset(); + codexui::FrontendSessionFacadeTestAccess::enqueueState( + session, 1, fullScope(QStringLiteral("full-first"))); + codexui::FrontendSessionFacadeTestAccess::enqueueState( + session, 1, structuralScope(QStringLiteral("full-first"))); + codexui::FrontendSessionFacadeTestAccess::enqueueState( + session, + 1, + exactScope(QStringLiteral("full-first"), + QStringLiteral("full-first-item"))); + QCoreApplication::processEvents(); + passed &= expect( + delivered + && delivered->fullyAffectedThreadIds + == QStringList{QStringLiteral("full-first")} + && delivered->structurallyAffectedThreadIds.empty() + && preservedExact(*delivered, QStringView{u"full-first-item"}), + "an existing deletion-capable scope must dominate a later structural scope without discarding exact metadata"); + + session.shutdown(); + return passed; +} + bool testFacadeScopeBound() { codexui::FrontendSession session; @@ -2540,11 +2765,31 @@ bool testFacadeScopeBound() bool passed = expect( delivered && delivered->allThreadsAffected && delivered->affectedThreadIds.empty() + && delivered->structurallyAffectedThreadIds.empty() && delivered->affectedItemContents.empty() && delivered->removedThreadIds == QStringList{QStringLiteral("removed-thread")}, "a blocked GUI must degrade an unbounded exact-scope burst to one bounded full refresh while retaining exact removals"); + delivered.reset(); + for (int index = 0; + index <= codexui::detail::maximumCoalescedPresentationIdentities; + ++index) { + codexui::detail::StateUpdateScope scope; + scope.affectedThreadIds.push_back(QStringLiteral("thread")); + scope.structurallyAffectedThreadIds.push_back( + QStringLiteral("structural-%1").arg(index)); + scope.hasPresentationChange = true; + codexui::FrontendSessionFacadeTestAccess::enqueueState( + session, 1, std::move(scope)); + } + QCoreApplication::processEvents(); + passed &= expect( + delivered && delivered->allThreadsAffected + && delivered->affectedThreadIds.empty() + && delivered->structurallyAffectedThreadIds.empty(), + "a blocked GUI must bound structural thread identities and let all-thread dominance clear them"); + delivered.reset(); { codexui::detail::StateUpdateScope removed; @@ -2629,7 +2874,9 @@ int main(int argc, char* argv[]) && testThreadedFacadeMailbox() && testFacadeReplaceableControlCoalescing() && testImmediateFacadeShutdown() - && testFacadeGenerationGating() && testFacadeScopeBound() + && testFacadeGenerationGating() + && testFacadeStructuralScopeMerge() + && testFacadeScopeBound() ? 0 : 1; } diff --git a/tests/PresentationRefreshAccumulatorTest.cpp b/tests/PresentationRefreshAccumulatorTest.cpp index ed6c703..7dd5530 100644 --- a/tests/PresentationRefreshAccumulatorTest.cpp +++ b/tests/PresentationRefreshAccumulatorTest.cpp @@ -135,6 +135,107 @@ bool testContiguousAppendMerge() return passed; } +detail::StateUpdateScope structuralScope() +{ + detail::StateUpdateScope scope; + scope.affectedThreadIds.push_back(QStringLiteral("thread")); + scope.structurallyAffectedThreadIds.push_back(QStringLiteral("thread")); + return scope; +} + +detail::StateUpdateScope exactScope() +{ + detail::StateUpdateScope scope; + scope.affectedThreadIds.push_back(QStringLiteral("thread")); + scope.affectedItemContents.push_back( + identity(0, QByteArray("append"), 7)); + return scope; +} + +bool retainedStructuralAppend( + const detail::SelectedPresentationRefreshAccumulator& accumulator) +{ + return accumulator.refreshPending + && !accumulator.fullRefreshPending + && accumulator.structuralReconciliationPending + && accumulator.contentChanges.size() == 1 + && accumulator.contentChanges.front().append + && accumulator.contentChanges.front().append->baseContentBytes == 7 + && accumulator.contentChanges.front().append->delta + == QStringLiteral("append") + && accumulator.retainedContentUtf8Bytes == 6; +} + +bool testStructuralAndExactMergeInBothOrders() +{ + const auto structural = structuralScope(); + const auto exact = exactScope(); + detail::SelectedPresentationRefreshAccumulator structuralThenExact; + detail::mergeSelectedPresentationRefresh( + structuralThenExact, structural, QStringLiteral("thread"), false); + detail::mergeSelectedPresentationRefresh( + structuralThenExact, exact, QStringLiteral("thread"), false); + + detail::SelectedPresentationRefreshAccumulator exactThenStructural; + detail::mergeSelectedPresentationRefresh( + exactThenStructural, exact, QStringLiteral("thread"), false); + detail::mergeSelectedPresentationRefresh( + exactThenStructural, structural, QStringLiteral("thread"), false); + + return expect( + retainedStructuralAppend(structuralThenExact) + && retainedStructuralAppend(exactThenStructural), + "structural reconciliation and exact append metadata must survive frame accumulation in both arrival orders"); +} + +bool testFullRefreshDominatesStructuralAndExact() +{ + auto structural = structuralScope(); + auto exact = exactScope(); + detail::StateUpdateScope full; + full.affectedThreadIds.push_back(QStringLiteral("thread")); + full.fullyAffectedThreadIds.push_back(QStringLiteral("thread")); + + detail::SelectedPresentationRefreshAccumulator accumulator; + detail::mergeSelectedPresentationRefresh( + accumulator, structural, QStringLiteral("thread"), false); + detail::mergeSelectedPresentationRefresh( + accumulator, exact, QStringLiteral("thread"), false); + detail::mergeSelectedPresentationRefresh( + accumulator, full, QStringLiteral("thread"), false); + bool passed = expect( + accumulator.refreshPending && accumulator.fullRefreshPending + && !accumulator.structuralReconciliationPending + && accumulator.contentChanges.empty() + && accumulator.retainedContentUtf8Bytes == 0, + "a later deletion-capable refresh must dominate structural and exact presentation metadata"); + + detail::mergeSelectedPresentationRefresh( + accumulator, structural, QStringLiteral("thread"), false); + detail::mergeSelectedPresentationRefresh( + accumulator, exact, QStringLiteral("thread"), false); + passed &= expect( + accumulator.fullRefreshPending + && !accumulator.structuralReconciliationPending + && accumulator.contentChanges.empty(), + "structural and exact publications must not weaken an accumulated full refresh"); + return passed; +} + +bool testUnscopedChangeFallsBackToFullRefresh() +{ + detail::StateUpdateScope unscoped; + unscoped.affectedThreadIds.push_back(QStringLiteral("thread")); + detail::SelectedPresentationRefreshAccumulator accumulator; + detail::mergeSelectedPresentationRefresh( + accumulator, unscoped, QStringLiteral("thread"), false); + return expect( + accumulator.refreshPending && accumulator.fullRefreshPending + && !accumulator.structuralReconciliationPending + && accumulator.contentChanges.empty(), + "a selected update without structural or exact metadata must retain the authoritative refresh fallback"); +} + bool testSidebarIdentityBound() { QStringList ordered; @@ -171,6 +272,9 @@ int main(int argc, char** argv) passed &= testAggregateByteBound(); passed &= testReplacementReleasesRetainedBytes(); passed &= testContiguousAppendMerge(); + passed &= testStructuralAndExactMergeInBothOrders(); + passed &= testFullRefreshDominatesStructuralAndExact(); + passed &= testUnscopedChangeFallsBackToFullRefresh(); passed &= testSidebarIdentityBound(); return passed ? 0 : 1; }