From 9dd6f0b60083dc84679fa3ffaa706d5bb5b1229c Mon Sep 17 00:00:00 2001 From: Volker Christian Date: Fri, 21 Aug 2026 19:08:49 +0200 Subject: [PATCH 1/2] Reduce presentation support work on state updates --- CMakeLists.txt | 3 +- src/app/AttachmentManager.cpp | 48 +++- src/app/AttachmentManager.h | 12 +- src/app/FrontendSession.cpp | 53 ++-- src/app/FrontendSession.h | 3 + src/app/FrontendSessionWorker.cpp | 64 ++++- src/ui/InspectorWidget.cpp | 13 + src/ui/InspectorWidget.h | 3 + src/ui/SidebarWidget.cpp | 200 +++++++++++----- src/ui/SidebarWidget.h | 13 + src/ui/WorkbenchWidget.cpp | 386 +++++++++++++++++++++++++----- src/ui/WorkbenchWidget.h | 46 +++- tests/AttachmentManagerTest.cpp | 17 ++ tests/ConversationLayoutTest.cpp | 45 ++++ tests/FrontendSessionTest.cpp | 89 ++++++- tests/Phase1ThreadTurnUxTest.cpp | 75 ++++++ 16 files changed, 901 insertions(+), 169 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6648f8c..8899659 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -10,7 +10,7 @@ include(GNUInstallDirs) set(CMAKE_AUTOMOC ON) find_package(AISuite 0.5.0 CONFIG REQUIRED) -find_package(Qt6 REQUIRED COMPONENTS Network Widgets) +find_package(Qt6 REQUIRED COMPONENTS Concurrent Network Widgets) qt_add_executable( codex-ui @@ -52,6 +52,7 @@ target_link_libraries( codex-ui PRIVATE AISuite::OpenAICodexFrontendClient + Qt6::Concurrent Qt6::Network Qt6::Widgets ) diff --git a/src/app/AttachmentManager.cpp b/src/app/AttachmentManager.cpp index f4fdf6e..f0b679e 100644 --- a/src/app/AttachmentManager.cpp +++ b/src/app/AttachmentManager.cpp @@ -63,8 +63,18 @@ QString uniqueDestinationName(const QString& requested, QSet& occupiedN bool copyFileAtomically(const QString& sourcePath, const QString& destinationPath, - QString* errorMessage) + QString* errorMessage, + const AttachmentManager::CancellationCheck& cancelled) { + const auto reportCancellation = [errorMessage]() { + if (errorMessage) + *errorMessage = QStringLiteral("Attachment preparation was cancelled."); + }; + if (cancelled && cancelled()) { + reportCancellation(); + return false; + } + QFile source(sourcePath); if (!source.open(QIODevice::ReadOnly)) { if (errorMessage) @@ -85,9 +95,20 @@ bool copyFileAtomically(const QString& sourcePath, return false; } + const auto cancelDestination = [&]() { + destination.cancelWriting(); + // QSaveFile's direct-write fallback cannot roll back by itself. This + // path is always a fresh file inside a fresh staging directory. + (void)QFile::remove(destinationPath); + reportCancellation(); + return false; + }; + constexpr qint64 chunkSize = 1024 * 1024; QByteArray buffer(static_cast(chunkSize), Qt::Uninitialized); while (!source.atEnd()) { + if (cancelled && cancelled()) + return cancelDestination(); const qint64 count = source.read(buffer.data(), chunkSize); if (count < 0 || (count > 0 && destination.write(buffer.constData(), count) != count)) { destination.cancelWriting(); @@ -102,7 +123,11 @@ bool copyFileAtomically(const QString& sourcePath, } if (count == 0) break; + if (cancelled && cancelled()) + return cancelDestination(); } + if (cancelled && cancelled()) + return cancelDestination(); if (!destination.commit()) { if (errorMessage) *errorMessage = errorWithPath( @@ -493,7 +518,8 @@ bool AttachmentManager::prepare(const QList& attachments, const QString& workspace, const QString& threadId, AttachmentPreparation* result, - QString* errorMessage) + QString* errorMessage, + CancellationCheck cancelled) { if (!result) { if (errorMessage) @@ -501,6 +527,11 @@ bool AttachmentManager::prepare(const QList& attachments, return false; } *result = {}; + if (cancelled && cancelled()) { + if (errorMessage) + *errorMessage = QStringLiteral("Attachment preparation was cancelled."); + return false; + } if (!validateForWorkspace(attachments, workspace, errorMessage)) return false; @@ -533,6 +564,14 @@ bool AttachmentManager::prepare(const QList& attachments, QSet occupiedNames; QStringList promptLines; for (const AttachmentInfo& attachment : attachments) { + if (cancelled && cancelled()) { + if (errorMessage) + *errorMessage = QStringLiteral("Attachment preparation was cancelled."); + if (result->stagingLease) + (void)result->stagingLease->cleanup(); + *result = {}; + return false; + } PreparedAttachment prepared; prepared.source = attachment; if (attachment.kind == AttachmentInfo::Kind::Image) { @@ -543,7 +582,10 @@ bool AttachmentManager::prepare(const QList& attachments, safeFileName(attachment.displayName), occupiedNames); prepared.effectivePath = QDir(stagingDirectory).filePath(destinationName); prepared.staged = true; - if (!copyFileAtomically(attachment.sourcePath, prepared.effectivePath, errorMessage)) { + if (!copyFileAtomically(attachment.sourcePath, + prepared.effectivePath, + errorMessage, + cancelled)) { (void)result->stagingLease->cleanup(); *result = {}; return false; diff --git a/src/app/AttachmentManager.h b/src/app/AttachmentManager.h index 76195df..023fdf6 100644 --- a/src/app/AttachmentManager.h +++ b/src/app/AttachmentManager.h @@ -7,6 +7,7 @@ #include #include +#include #include class QSettings; @@ -82,12 +83,14 @@ struct PersistedAttachmentStaging class AttachmentManager final { public: - // Staging is deliberately synchronous and therefore bounded well below - // filesystem limits. Image contents travel by local path, not on the - // frontend protocol wire. + // Generic-file staging is bounded and may run off the GUI thread. Callers + // can cooperatively cancel it between fixed-size copy chunks. Image + // contents travel by local path, not on the frontend protocol wire. static constexpr qint64 MaximumSingleFileBytes = 64LL * 1024LL * 1024LL; static constexpr qint64 MaximumTotalBytes = 256LL * 1024LL * 1024LL; + using CancellationCheck = std::function; + [[nodiscard]] static bool inspectFile(const QString& path, AttachmentInfo* result, QString* errorMessage = nullptr); @@ -98,7 +101,8 @@ class AttachmentManager final const QString& workspace, const QString& threadId, AttachmentPreparation* result, - QString* errorMessage = nullptr); + QString* errorMessage = nullptr, + CancellationCheck cancelled = {}); [[nodiscard]] static QString composePrompt(const QString& userPrompt, const AttachmentPreparation& preparation); [[nodiscard]] static QString formatSize(qint64 sizeBytes); diff --git a/src/app/FrontendSession.cpp b/src/app/FrontendSession.cpp index 81c02d8..e4c0123 100644 --- a/src/app/FrontendSession.cpp +++ b/src/app/FrontendSession.cpp @@ -22,14 +22,17 @@ namespace sdk = ai::openai::codex::frontend::client; namespace { -constexpr qsizetype maximumCoalescedPresentationIdentities = 1'024; - -void appendUnique(QStringList& destination, const QStringList& source) +bool appendUniqueBounded(QStringList& destination, const QStringList& source) { for (const QString& value : source) { - if (!destination.contains(value)) - destination.push_back(value); + if (destination.contains(value)) + continue; + if (destination.size() + >= detail::maximumCoalescedPresentationIdentities) + return false; + destination.push_back(value); } + return true; } void mergeScope(detail::StateUpdateScope& destination, @@ -37,16 +40,24 @@ void mergeScope(detail::StateUpdateScope& destination, { destination.allThreadsAffected |= source.allThreadsAffected; destination.allInspectorsAffected |= source.allInspectorsAffected; + destination.allSidebarThreadsAffected |= source.allSidebarThreadsAffected; destination.sidebarAffected |= source.sidebarAffected; destination.hasPresentationChange |= source.hasPresentationChange; if (!destination.allThreadsAffected) { - appendUnique(destination.affectedThreadIds, source.affectedThreadIds); - appendUnique(destination.fullyAffectedThreadIds, - source.fullyAffectedThreadIds); + if (!appendUniqueBounded(destination.affectedThreadIds, + source.affectedThreadIds) + || !appendUniqueBounded(destination.fullyAffectedThreadIds, + source.fullyAffectedThreadIds)) + destination.allThreadsAffected = true; } - if (!destination.allInspectorsAffected) - appendUnique(destination.affectedInspectorThreadIds, - source.affectedInspectorThreadIds); + if (!destination.allInspectorsAffected + && !appendUniqueBounded(destination.affectedInspectorThreadIds, + source.affectedInspectorThreadIds)) + destination.allInspectorsAffected = true; + if (!destination.allSidebarThreadsAffected + && !appendUniqueBounded(destination.affectedSidebarThreadIds, + source.affectedSidebarThreadIds)) + destination.allSidebarThreadsAffected = true; const auto sameContent = [](const auto& left, const auto& right) { return left.threadId == right.threadId && left.turnId == right.turnId @@ -60,6 +71,12 @@ void mergeScope(detail::StateUpdateScope& destination, return sameContent(candidate, identity); }); if (existing == destination.affectedItemContents.end()) { + if (static_cast( + destination.affectedItemContents.size()) + >= detail::maximumCoalescedPresentationIdentities) { + destination.allThreadsAffected = true; + break; + } auto bounded = identity; if (bounded.append) { const std::uint64_t bytes = @@ -131,17 +148,21 @@ void mergeScope(detail::StateUpdateScope& destination, } if (destination.affectedThreadIds.size() - > maximumCoalescedPresentationIdentities + > detail::maximumCoalescedPresentationIdentities || destination.fullyAffectedThreadIds.size() - > maximumCoalescedPresentationIdentities + > detail::maximumCoalescedPresentationIdentities || static_cast(destination.affectedItemContents.size()) - > maximumCoalescedPresentationIdentities) { + > detail::maximumCoalescedPresentationIdentities) { destination.allThreadsAffected = true; } if (destination.affectedInspectorThreadIds.size() - > maximumCoalescedPresentationIdentities) { + > detail::maximumCoalescedPresentationIdentities) { destination.allInspectorsAffected = true; } + if (destination.affectedSidebarThreadIds.size() + > detail::maximumCoalescedPresentationIdentities) { + destination.allSidebarThreadsAffected = true; + } if (destination.allThreadsAffected) { destination.affectedThreadIds.clear(); destination.fullyAffectedThreadIds.clear(); @@ -150,6 +171,8 @@ void mergeScope(detail::StateUpdateScope& destination, } if (destination.allInspectorsAffected) destination.affectedInspectorThreadIds.clear(); + if (destination.allSidebarThreadsAffected) + destination.affectedSidebarThreadIds.clear(); } template diff --git a/src/app/FrontendSession.h b/src/app/FrontendSession.h index ecd86af..9d74591 100644 --- a/src/app/FrontendSession.h +++ b/src/app/FrontendSession.h @@ -25,6 +25,7 @@ namespace codexui::detail { // performs an authoritative replacement refresh instead of retaining deltas. inline constexpr std::uint64_t maximumCoalescedContentDeltaBytes = 1024U * 1024U; +inline constexpr qsizetype maximumCoalescedPresentationIdentities = 1'024; struct StateUpdateScope { struct ItemContentAppend { @@ -49,10 +50,12 @@ struct StateUpdateScope { QStringList affectedThreadIds; QStringList fullyAffectedThreadIds; QStringList affectedInspectorThreadIds; + QStringList affectedSidebarThreadIds; std::vector affectedItemContents; std::uint64_t coalescedContentDeltaBytes = 0; bool allThreadsAffected = false; bool allInspectorsAffected = false; + bool allSidebarThreadsAffected = false; bool sidebarAffected = false; bool hasPresentationChange = false; }; diff --git a/src/app/FrontendSessionWorker.cpp b/src/app/FrontendSessionWorker.cpp index 674a503..d460ba6 100644 --- a/src/app/FrontendSessionWorker.cpp +++ b/src/app/FrontendSessionWorker.cpp @@ -55,18 +55,34 @@ StateUpdateScope stateUpdateScope(const sdk::StateUpdate& update) const auto addUnique = [](QStringList& ids, std::string_view id) { const QString threadId = QString::fromUtf8(id.data(), static_cast(id.size())); - if (!ids.contains(threadId)) - ids.append(threadId); + if (ids.contains(threadId)) + return true; + if (ids.size() >= maximumCoalescedPresentationIdentities) + return false; + ids.append(threadId); + return true; }; const auto addThread = [&scope, &addUnique](std::string_view id) { - addUnique(scope.affectedThreadIds, id); + if (!scope.allThreadsAffected + && !addUnique(scope.affectedThreadIds, id)) + scope.allThreadsAffected = true; }; const auto addFullyAffectedThread = [&scope, &addThread, &addUnique](std::string_view id) { addThread(id); - addUnique(scope.fullyAffectedThreadIds, id); + if (!scope.allThreadsAffected + && !addUnique(scope.fullyAffectedThreadIds, id)) + scope.allThreadsAffected = true; }; const auto addInspectorThread = [&scope, &addUnique](std::string_view id) { - addUnique(scope.affectedInspectorThreadIds, id); + if (!scope.allInspectorsAffected + && !addUnique(scope.affectedInspectorThreadIds, id)) + scope.allInspectorsAffected = true; + }; + const auto addSidebarThread = [&scope, &addUnique](std::string_view id) { + scope.sidebarAffected = true; + if (!scope.allSidebarThreadsAffected + && !addUnique(scope.affectedSidebarThreadIds, id)) + scope.allSidebarThreadsAffected = true; }; const auto markThreadAndInspector = [&addFullyAffectedThread, &addInspectorThread](std::string_view id) { addFullyAffectedThread(id); @@ -81,6 +97,8 @@ StateUpdateScope stateUpdateScope(const sdk::StateUpdate& update) const QString turnId = asQString(value.turnId->value); const QString itemId = asQString(value.itemId.value); addThread(value.threadId->value); + if (scope.allThreadsAffected) + return; StateUpdateScope::ItemContentIdentity identity{ threadId, turnId, @@ -98,6 +116,11 @@ StateUpdateScope stateUpdateScope(const sdk::StateUpdate& update) && candidate.channel == identity.channel; }); if (existing == scope.affectedItemContents.end()) { + if (static_cast(scope.affectedItemContents.size()) + >= maximumCoalescedPresentationIdentities) { + scope.allThreadsAffected = true; + return; + } if (identity.append) { const std::uint64_t bytes = static_cast( @@ -131,6 +154,7 @@ StateUpdateScope stateUpdateScope(const sdk::StateUpdate& update) { scope.allThreadsAffected = true; scope.allInspectorsAffected = true; + scope.allSidebarThreadsAffected = true; scope.sidebarAffected = true; scope.hasPresentationChange = true; return scope; @@ -151,25 +175,30 @@ StateUpdateScope stateUpdateScope(const sdk::StateUpdate& update) { scope.allThreadsAffected = true; scope.allInspectorsAffected = true; + scope.allSidebarThreadsAffected = true; scope.sidebarAffected = true; } else if constexpr (std::is_same_v || std::is_same_v) { addFullyAffectedThread(value.threadId.value); - scope.sidebarAffected = true; + addSidebarThread(value.threadId.value); // The selected Inspector can show status/model facts from // a linked subagent thread even when its conversation is - // not selected. - scope.allInspectorsAffected = true; + // not selected. Workbench resolves this identity against + // its retained Inspector dependency set. + addInspectorThread(value.threadId.value); } else if constexpr (std::is_same_v) { - if (const auto* turn = update.state.turn(value.turnId)) + if (const auto* turn = update.state.turn(value.turnId)) { markThreadAndInspector(turn->threadId.value); - else { + addSidebarThread(turn->threadId.value); + } else { scope.allThreadsAffected = true; scope.allInspectorsAffected = true; + scope.allSidebarThreadsAffected = true; + scope.sidebarAffected = true; } } else if constexpr (std::is_same_v) @@ -244,12 +273,15 @@ StateUpdateScope stateUpdateScope(const sdk::StateUpdate& update) // change has no thread identity, especially on removal. scope.allThreadsAffected = true; scope.allInspectorsAffected = true; + scope.allSidebarThreadsAffected = true; + scope.sidebarAffected = true; } else if constexpr (std::is_same_v) { // A list replacement has no per-thread removal identity. scope.allThreadsAffected = true; scope.allInspectorsAffected = true; + scope.allSidebarThreadsAffected = true; scope.sidebarAffected = true; } else @@ -263,6 +295,16 @@ StateUpdateScope stateUpdateScope(const sdk::StateUpdate& update) }, change); } + if (scope.allThreadsAffected) { + scope.affectedThreadIds.clear(); + scope.fullyAffectedThreadIds.clear(); + scope.affectedItemContents.clear(); + scope.coalescedContentDeltaBytes = 0; + } + if (scope.allInspectorsAffected) + scope.affectedInspectorThreadIds.clear(); + if (scope.allSidebarThreadsAffected) + scope.affectedSidebarThreadIds.clear(); return scope; } @@ -353,6 +395,7 @@ FrontendSessionWorker::FrontendSessionWorker(QObject* parent) detail::StateUpdateScope scope; scope.allThreadsAffected = true; scope.allInspectorsAffected = true; + scope.allSidebarThreadsAffected = true; scope.sidebarAffected = true; scope.hasPresentationChange = true; emit stateChanged(scope); @@ -1177,6 +1220,7 @@ void FrontendSessionWorker::finishArchivedThreadRefresh(ArchivedThreadDiscoveryS detail::StateUpdateScope scope; scope.allThreadsAffected = true; scope.allInspectorsAffected = true; + scope.allSidebarThreadsAffected = true; scope.sidebarAffected = true; scope.hasPresentationChange = true; emit stateChanged(scope); diff --git a/src/ui/InspectorWidget.cpp b/src/ui/InspectorWidget.cpp index 2d3a71c..9c7d5b6 100644 --- a/src/ui/InspectorWidget.cpp +++ b/src/ui/InspectorWidget.cpp @@ -666,6 +666,7 @@ void InspectorWidget::renderUnavailable(const QString& title, const QString& det return; unavailablePresentationKey = presentationKey; inspectedThreadId.clear(); + dependentThreadIds.clear(); selectedAgentItemId.clear(); planPresentationKey.clear(); agentsPresentationKey.clear(); @@ -680,6 +681,12 @@ void InspectorWidget::renderUnavailable(const QString& title, const QString& det } } +bool InspectorWidget::dependsOnThread(const QString& threadId) const +{ + return !threadId.isEmpty() + && (threadId == inspectedThreadId || dependentThreadIds.contains(threadId)); +} + void InspectorWidget::setHistoricalTurnMode(bool enabled) { if (historicalTurnMode == enabled) @@ -752,6 +759,7 @@ void InspectorWidget::render(const sdk::State& state, unavailablePresentationKey.clear(); if (inspectedThreadId != threadId) { inspectedThreadId = threadId; + dependentThreadIds.clear(); selectedAgentItemId.clear(); planPresentationKey.clear(); agentsPresentationKey.clear(); @@ -942,6 +950,11 @@ void InspectorWidget::render(const sdk::State& state, agents = agentPresentations(state, *thread, *turn); collaborations = collaborationPresentations(state, *thread, *turn); } + dependentThreadIds.clear(); + for (const AgentPresentation& agent : agents) { + if (!agent.agentThreadId.isEmpty()) + dependentThreadIds.insert(agent.agentThreadId); + } const auto selected = std::find_if(agents.begin(), agents.end(), [this](const AgentPresentation& agent) { return agent.itemIds.contains(selectedAgentItemId); }); diff --git a/src/ui/InspectorWidget.h b/src/ui/InspectorWidget.h index fae1bda..831c73d 100644 --- a/src/ui/InspectorWidget.h +++ b/src/ui/InspectorWidget.h @@ -8,6 +8,7 @@ #include #include #include +#include #include @@ -32,6 +33,7 @@ class InspectorWidget : public QWidget const QString& backendStatus, const QString& selectedTurnId = {}); void updateStateRevision(std::uint64_t revision); + [[nodiscard]] bool dependsOnThread(const QString& threadId) const; void showInfo(); signals: @@ -50,6 +52,7 @@ class InspectorWidget : public QWidget QVBoxLayout* changesContent = nullptr; QVBoxLayout* infoContent = nullptr; QString inspectedThreadId; + QSet dependentThreadIds; QString selectedAgentItemId; QByteArray unavailablePresentationKey; QByteArray planPresentationKey; diff --git a/src/ui/SidebarWidget.cpp b/src/ui/SidebarWidget.cpp index df86b31..b71eb6b 100644 --- a/src/ui/SidebarWidget.cpp +++ b/src/ui/SidebarWidget.cpp @@ -1026,6 +1026,77 @@ SidebarWidget::~SidebarWidget() delete organizationLock; } +SidebarWidget::ThreadPresentation SidebarWidget::threadPresentation( + const ai::openai::codex::frontend::client::State& state, + const ai::openai::codex::frontend::client::ThreadState& thread, + bool awaitingResponse) const +{ + const QString id = QString::fromStdString(thread.id.value); + const QString title = boundedRowText( + thread.title && !thread.title->empty() ? QString::fromStdString(*thread.title) : id); + const detail::ThreadUiStatus uiStatus = detail::threadUiStatus( + state, thread, awaitingResponse); + QStringList secondaryParts; + if (uiStatus.archived) + secondaryParts.append(QStringLiteral("Archived")); + else if (!thread.fullyLoaded) + secondaryParts.append(QStringLiteral("Loading")); + else if (uiStatus.running) + secondaryParts.append(QStringLiteral("Running")); + else if (!ai::openai::codex::frontend::client::threadIsIdle(thread)) + secondaryParts.append(QStringLiteral("Ready to resume")); + else + secondaryParts.append(QStringLiteral("Idle")); + secondaryParts.append(thread.orderedTurns.empty() + ? QStringLiteral("Ready for first turn") + : QStringLiteral("%1 turn%2") + .arg(thread.orderedTurns.size()) + .arg(thread.orderedTurns.size() == 1 + ? QString{} + : QStringLiteral("s"))); + if (thread.ephemeral.value_or(false)) + secondaryParts.append(QStringLiteral("Temporary")); + return {id, + title, + boundedRowText(secondaryParts.join(QStringLiteral(" · "))), + threadStatusColor(thread.status), + uiStatus.actions, + uiStatus.running, + uiStatus.awaitingResponse, + uiStatus.archived}; +} + +void SidebarWidget::rebuildRenderedThreadIndex() +{ + renderedThreadIndexes.clear(); + renderedThreadIndexes.reserve(static_cast(renderedThreads.size())); + for (std::size_t index = 0; index < renderedThreads.size(); ++index) + renderedThreadIndexes.insert(renderedThreads[index].id, static_cast(index)); +} + +void SidebarWidget::updateRenderedRows(const QSet& threadIds) +{ + for (const QString& threadId : threadIds) { + auto* row = static_cast(renderedThreadRows.value(threadId, nullptr)); + if (!row) + continue; + const auto index = renderedThreadIndexes.constFind(threadId); + if (index == renderedThreadIndexes.cend() + || *index < 0 || static_cast(*index) >= renderedThreads.size()) + continue; + const ThreadPresentation& presentation = renderedThreads[static_cast(*index)]; + row->updatePresentation(presentation.title, + presentation.details, + presentation.color, + presentation.actions, + presentation.running, + presentation.attention, + presentation.archived); + row->setSelected(presentation.id == renderedSelection); + row->setInteractionEnabled(threadInteractionEnabled); + } +} + void SidebarWidget::tryAcquireOrganizationLock() { if (organizationWritable || !organizationLock || !organizationLock->tryLock(0)) @@ -1071,39 +1142,8 @@ void SidebarWidget::setThreads(const ai::openai::codex::frontend::client::State& presentations.reserve(threads.size()); for (const auto& thread : threads) { const QString id = QString::fromStdString(thread.id.value); - const QString title = boundedRowText( - thread.title && !thread.title->empty() ? QString::fromStdString(*thread.title) : id); - const detail::ThreadUiStatus uiStatus = detail::threadUiStatus( - state, thread, threadsAwaitingResponse.contains(id)); - QStringList secondaryParts; - if (uiStatus.archived) - secondaryParts.append(QStringLiteral("Archived")); - else if (!thread.fullyLoaded) - secondaryParts.append(QStringLiteral("Loading")); - else if (uiStatus.running) - secondaryParts.append(QStringLiteral("Running")); - else if (!ai::openai::codex::frontend::client::threadIsIdle(thread)) - secondaryParts.append(QStringLiteral("Ready to resume")); - else - secondaryParts.append(QStringLiteral("Idle")); - secondaryParts.append(thread.orderedTurns.empty() - ? QStringLiteral("Ready for first turn") - : QStringLiteral("%1 turn%2") - .arg(thread.orderedTurns.size()) - .arg(thread.orderedTurns.size() == 1 - ? QString{} - : QStringLiteral("s"))); - if (thread.ephemeral.value_or(false)) - secondaryParts.append(QStringLiteral("Temporary")); - const QString secondary = secondaryParts.join(QStringLiteral(" · ")); - presentations.push_back({id, - title, - boundedRowText(secondary), - threadStatusColor(thread.status), - uiStatus.actions, - uiStatus.running, - uiStatus.awaitingResponse, - uiStatus.archived}); + presentations.push_back(threadPresentation( + state, thread, threadsAwaitingResponse.contains(id))); } std::stable_partition(presentations.begin(), presentations.end(), [](const ThreadPresentation& presentation) { @@ -1122,49 +1162,82 @@ void SidebarWidget::setThreads(const ai::openai::codex::frontend::client::State& sameOrder = sameOrder && renderedOrganizationRevision == organization.revision(); if (sameOrder) { renderedThreads = presentations; + rebuildRenderedThreadIndex(); renderedSelection = selectedThreadId; - QHash presentationsById; - presentationsById.reserve(static_cast(renderedThreads.size())); + QSet allThreadIds; + allThreadIds.reserve(static_cast(renderedThreads.size())); for (const ThreadPresentation& presentation : renderedThreads) - presentationsById.insert(presentation.id, &presentation); - std::size_t updatedRows = 0; - QTreeWidgetItemIterator iterator(threadTree); - while (*iterator) { - auto* item = *iterator; - auto* row = dynamic_cast(threadTree->itemWidget(item, 0)); - ++iterator; - if (!row) - continue; - const auto presentation = presentationsById.constFind(row->id()); - if (presentation == presentationsById.cend()) { - sameOrder = false; - break; - } - const ThreadPresentation& value = **presentation; - row->updatePresentation(value.title, - value.details, - value.color, - value.actions, - value.running, - value.attention, - value.archived); - row->setSelected(value.id == selectedThreadId); - row->setInteractionEnabled(threadInteractionEnabled); - ++updatedRows; - } - sameOrder = sameOrder && updatedRows == renderedThreads.size(); - if (sameOrder) + allThreadIds.insert(presentation.id); + if (renderedThreadRows.size() == static_cast(renderedThreads.size())) { + updateRenderedRows(allThreadIds); return; + } } threadsRendered = true; renderedThreads = std::move(presentations); + rebuildRenderedThreadIndex(); renderedSelection = selectedThreadId; renderThreadTree(); } +void SidebarWidget::updateThreads( + const ai::openai::codex::frontend::client::State& state, + const QString& selectedThreadId, + bool allThreadDiscoveryComplete, + const QStringList& affectedThreadIds) +{ + tryAcquireOrganizationLock(); + if (!threadsRendered || selectedThreadId != renderedSelection + || renderedOrganizationRevision != organization.revision()) { + setThreads(state, selectedThreadId, allThreadDiscoveryComplete); + return; + } + + QSet uniqueThreadIds(affectedThreadIds.cbegin(), affectedThreadIds.cend()); + if (uniqueThreadIds.isEmpty()) + return; + + QSet threadsAwaitingResponse; + if (state.hasPendingRequestProjection()) { + for (const auto& request : state.pendingRequests()) { + if (request.threadId) + threadsAwaitingResponse.insert(QString::fromStdString(request.threadId->value)); + } + } + + QSet changedRows; + for (const QString& threadId : uniqueThreadIds) { + const auto existingIndex = renderedThreadIndexes.constFind(threadId); + const auto* thread = state.thread(threadId.toStdString()); + if (!thread || existingIndex == renderedThreadIndexes.cend() + || *existingIndex < 0 + || static_cast(*existingIndex) >= renderedThreads.size()) { + // Insertions/removals and archive-boundary changes can alter the + // tree hierarchy and ordering. Reconcile those uncommon cases + // through the authoritative full path. + setThreads(state, selectedThreadId, allThreadDiscoveryComplete); + return; + } + ThreadPresentation& existing = + renderedThreads[static_cast(*existingIndex)]; + ThreadPresentation next = threadPresentation( + state, *thread, threadsAwaitingResponse.contains(threadId)); + if (next.archived != existing.archived) { + setThreads(state, selectedThreadId, allThreadDiscoveryComplete); + return; + } + if (next != existing) { + existing = std::move(next); + changedRows.insert(threadId); + } + } + updateRenderedRows(changedRows); +} + void SidebarWidget::renderThreadTree() { rebuildingTree = true; + renderedThreadRows.clear(); threadTree->clear(); renderedOrganizationRevision = organization.revision(); @@ -1263,6 +1336,7 @@ void SidebarWidget::renderThreadTree() presentation.attention, presentation.archived, threadTree); + renderedThreadRows.insert(presentation.id, row); row->setSelected(presentation.id == renderedSelection); row->setInteractionEnabled(threadInteractionEnabled); row->clicked = [this](ThreadRow* selected) { emit threadSelected(selected->id()); }; diff --git a/src/ui/SidebarWidget.h b/src/ui/SidebarWidget.h index f86f9cc..6d139a0 100644 --- a/src/ui/SidebarWidget.h +++ b/src/ui/SidebarWidget.h @@ -6,6 +6,7 @@ #include #include #include +#include #include #include @@ -125,6 +126,10 @@ class SidebarWidget : public QWidget void setThreads(const ai::openai::codex::frontend::client::State& state, const QString& selectedThreadId, bool allThreadDiscoveryComplete); + void updateThreads(const ai::openai::codex::frontend::client::State& state, + const QString& selectedThreadId, + bool allThreadDiscoveryComplete, + const QStringList& affectedThreadIds); void setConnectionStatus(const QString& title, const QString& detail, const QString& color); void setNewThreadEnabled(bool enabled); void setThreadInteractionEnabled(bool enabled); @@ -150,6 +155,12 @@ class SidebarWidget : public QWidget }; void renderThreadTree(); + [[nodiscard]] ThreadPresentation threadPresentation( + const ai::openai::codex::frontend::client::State& state, + const ai::openai::codex::frontend::client::ThreadState& thread, + bool awaitingResponse) const; + void rebuildRenderedThreadIndex(); + void updateRenderedRows(const QSet& threadIds); void tryAcquireOrganizationLock(); void persistOrganization(); void createFolder(const QString& parentFolderId = {}); @@ -166,6 +177,8 @@ class SidebarWidget : public QWidget QPushButton* newThread = nullptr; QPushButton* newFolder = nullptr; std::vector renderedThreads; + QHash renderedThreadIndexes; + QHash renderedThreadRows; QString renderedSelection; detail::ThreadOrganization organization; quint64 renderedOrganizationRevision = 0; diff --git a/src/ui/WorkbenchWidget.cpp b/src/ui/WorkbenchWidget.cpp index 43a7c2f..737da9a 100644 --- a/src/ui/WorkbenchWidget.cpp +++ b/src/ui/WorkbenchWidget.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -30,8 +31,10 @@ #include #include #include +#include #include +#include #include namespace codexui { @@ -292,6 +295,11 @@ WorkbenchWidget::WorkbenchWidget(FrontendSession& session, QWidget* parent) refreshState(); } +WorkbenchWidget::~WorkbenchWidget() +{ + cancelAttachmentPreparation(); +} + void WorkbenchWidget::scheduleStateRefresh(const detail::StateUpdateScope& scope) { const bool currentSelectionAffected = scope.affectedThreadIds.contains(selectedThreadId); @@ -308,10 +316,12 @@ void WorkbenchWidget::scheduleStateRefresh(const detail::StateUpdateScope& scope || scope.affectedThreadIds.contains( automaticResumeThreadId)); const bool selectedInspectorAffected = scope.allInspectorsAffected - || scope.affectedInspectorThreadIds.contains(selectedThreadId) - || (!newThreadIdAwaitingState.isEmpty() - && scope.affectedInspectorThreadIds.contains( - newThreadIdAwaitingState)); + || std::ranges::any_of(scope.affectedInspectorThreadIds, + [this](const QString& threadId) { + return inspector->dependsOnThread(threadId) + || (!newThreadIdAwaitingState.isEmpty() + && threadId == newThreadIdAwaitingState); + }); // Keep the factual State revision current without invoking the expensive // Inspector projections when none of their selected semantics changed. if (!selectedInspectorAffected) @@ -328,9 +338,14 @@ void WorkbenchWidget::scheduleStateRefresh(const detail::StateUpdateScope& scope { 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) { @@ -346,37 +361,85 @@ void WorkbenchWidget::scheduleStateRefresh(const detail::StateUpdateScope& scope && update.itemId == identity.itemId && update.channel == identity.channel; }); - ConversationContentUpdate next{ - identity.turnId, - identity.itemId, - identity.channel, - std::nullopt}; - if (identity.append) - { - next.append = ConversationContentAppend{ - identity.append->baseContentBytes, - identity.append->discardPrefixBytes, - static_cast(identity.append->deltaUtf8.size()), - QString::fromUtf8(identity.append->deltaUtf8)}; - } if (existing == selectedContentRefreshPending.end()) { + if (static_cast(selectedContentRefreshPending.size()) + >= detail::maximumCoalescedPresentationIdentities) + { + selectedPresentationFullRefreshPending = true; + selectedContentRefreshPending.clear(); + selectedContentRefreshPendingBytes = 0; + break; + } + ConversationContentUpdate next{ + identity.turnId, + identity.itemId, + identity.channel, + std::nullopt}; + if (identity.append) + { + const std::uint64_t deltaBytes = static_cast( + identity.append->deltaUtf8.size()); + if (selectedContentRefreshPendingBytes + > detail::maximumCoalescedContentDeltaBytes + || deltaBytes + > detail::maximumCoalescedContentDeltaBytes + - selectedContentRefreshPendingBytes) + { + selectedPresentationFullRefreshPending = true; + selectedContentRefreshPending.clear(); + selectedContentRefreshPendingBytes = 0; + break; + } + next.append = ConversationContentAppend{ + identity.append->baseContentBytes, + identity.append->discardPrefixBytes, + deltaBytes, + QString::fromUtf8(identity.append->deltaUtf8)}; + selectedContentRefreshPendingBytes += deltaBytes; + } selectedContentRefreshPending.push_back(std::move(next)); } - else if (existing->append && next.append + else if (existing->append && identity.append && existing->append->discardPrefixBytes == 0 - && next.append->discardPrefixBytes == 0 + && identity.append->discardPrefixBytes == 0 + && existing->append->baseContentBytes + <= std::numeric_limits::max() + - existing->append->deltaUtf8Bytes && existing->append->baseContentBytes + existing->append->deltaUtf8Bytes - == next.append->baseContentBytes) + == identity.append->baseContentBytes) { - existing->append->delta.append(next.append->delta); - existing->append->deltaUtf8Bytes += next.append->deltaUtf8Bytes; + const std::uint64_t deltaBytes = static_cast( + identity.append->deltaUtf8.size()); + if (selectedContentRefreshPendingBytes + > detail::maximumCoalescedContentDeltaBytes + || deltaBytes + > detail::maximumCoalescedContentDeltaBytes + - selectedContentRefreshPendingBytes) + { + selectedPresentationFullRefreshPending = true; + selectedContentRefreshPending.clear(); + selectedContentRefreshPendingBytes = 0; + break; + } + existing->append->delta.append(QString::fromUtf8(identity.append->deltaUtf8)); + existing->append->deltaUtf8Bytes += deltaBytes; + selectedContentRefreshPendingBytes += deltaBytes; } else { // Ambiguous, rolling, or replacement updates retain the // authoritative State fallback instead of guessing a delta. + if (existing->append) + { + selectedContentRefreshPendingBytes = + existing->append->deltaUtf8Bytes + <= selectedContentRefreshPendingBytes + ? selectedContentRefreshPendingBytes + - existing->append->deltaUtf8Bytes + : 0; + } existing->append.reset(); } } @@ -386,11 +449,38 @@ void WorkbenchWidget::scheduleStateRefresh(const detail::StateUpdateScope& scope { selectedPresentationFullRefreshPending = true; selectedContentRefreshPending.clear(); + selectedContentRefreshPendingBytes = 0; } } } inspectorRefreshPending = inspectorRefreshPending || selectedInspectorAffected; sidebarRefreshPending = sidebarRefreshPending || scope.sidebarAffected; + if (scope.sidebarAffected) { + if (scope.allSidebarThreadsAffected) { + sidebarFullRefreshPending = true; + sidebarThreadRefreshPending.clear(); + sidebarThreadRefreshPendingSet.clear(); + } else if (!sidebarFullRefreshPending) { + for (const QString& threadId : scope.affectedSidebarThreadIds) { + if (sidebarThreadRefreshPendingSet.contains(threadId)) + continue; + if (sidebarThreadRefreshPending.size() + >= detail::maximumCoalescedPresentationIdentities) { + sidebarFullRefreshPending = true; + sidebarThreadRefreshPending.clear(); + sidebarThreadRefreshPendingSet.clear(); + break; + } + sidebarThreadRefreshPendingSet.insert(threadId); + sidebarThreadRefreshPending.append(threadId); + } + if (scope.affectedSidebarThreadIds.isEmpty()) { + sidebarFullRefreshPending = true; + sidebarThreadRefreshPending.clear(); + sidebarThreadRefreshPendingSet.clear(); + } + } + } if (stateRefreshPending) return; stateRefreshPending = true; @@ -403,6 +493,8 @@ void WorkbenchWidget::scheduleStateRefresh(const detail::StateUpdateScope& scope const bool refreshSelectedPresentation = selectedPresentationRefreshPending; const bool refreshInspector = inspectorRefreshPending; const bool refreshSidebar = sidebarRefreshPending; + const bool refreshFullSidebar = sidebarFullRefreshPending; + QStringList sidebarThreadChanges = std::move(sidebarThreadRefreshPending); const bool exactContentOnly = refreshSelectedPresentation && !selectedPresentationFullRefreshPending && !selectedContentRefreshPending.empty(); @@ -410,8 +502,12 @@ void WorkbenchWidget::scheduleStateRefresh(const detail::StateUpdateScope& scope selectedPresentationRefreshPending = false; selectedPresentationFullRefreshPending = false; selectedContentRefreshPending.clear(); + selectedContentRefreshPendingBytes = 0; inspectorRefreshPending = false; sidebarRefreshPending = false; + sidebarFullRefreshPending = false; + sidebarThreadRefreshPending.clear(); + sidebarThreadRefreshPendingSet.clear(); if (exactContentOnly && !refreshInspector && !refreshSidebar && turnThreadIdAwaitingState.isEmpty() && automaticResumeThreadId.isEmpty() && conversation->updateExactMessageContent( @@ -420,7 +516,10 @@ void WorkbenchWidget::scheduleStateRefresh(const detail::StateUpdateScope& scope refreshState(refreshSelectedPresentation, refreshInspector, refreshSidebar, - exactContentOnly ? &exactContentChanges : nullptr); + exactContentOnly ? &exactContentChanges : nullptr, + refreshSidebar && !refreshFullSidebar && !sidebarThreadChanges.isEmpty() + ? &sidebarThreadChanges + : nullptr); }); } @@ -472,7 +571,7 @@ void WorkbenchWidget::refreshLifecycle() if (frontendSession.lifecycle() != Lifecycle::Ready) { const bool writeWasPending = pendingAction != PendingAction::None || controllerAcquireInFlight || threadStartInFlight || threadResumeInFlight || turnStartInFlight - || turnSteerInFlight + || turnSteerInFlight || attachmentPreparationInFlight || interruptInFlight || threadMutationInFlight; clearWriteTransients(); if (writeWasPending) @@ -487,14 +586,19 @@ void WorkbenchWidget::refreshLifecycle() void WorkbenchWidget::refreshState(bool refreshSelectedPresentation, bool refreshInspector, bool refreshSidebar, - const ConversationContentUpdates* exactContentChanges) + const ConversationContentUpdates* exactContentChanges, + const QStringList* sidebarThreadChanges) { stateRefreshPending = false; selectedPresentationRefreshPending = false; selectedPresentationFullRefreshPending = false; selectedContentRefreshPending.clear(); + selectedContentRefreshPendingBytes = 0; inspectorRefreshPending = false; sidebarRefreshPending = false; + sidebarFullRefreshPending = false; + sidebarThreadRefreshPending.clear(); + sidebarThreadRefreshPendingSet.clear(); const auto& state = frontendSession.state(); const auto threads = state.threads(); const bool ready = frontendSession.lifecycle() == FrontendSession::Lifecycle::Ready; @@ -528,8 +632,13 @@ void WorkbenchWidget::refreshState(bool refreshSelectedPresentation, refreshInspector = refreshInspector || selectionChanged; refreshSidebar = refreshSidebar || selectionChanged; - if (refreshSidebar) - sidebar->setThreads(state, selectedThreadId, threadDiscoveryComplete); + if (refreshSidebar) { + if (!selectionChanged && sidebarThreadChanges && !sidebarThreadChanges->isEmpty()) + sidebar->updateThreads( + state, selectedThreadId, threadDiscoveryComplete, *sidebarThreadChanges); + else + sidebar->setThreads(state, selectedThreadId, threadDiscoveryComplete); + } // ConversationWidget resolves the stable selection against this exact // immutable State and never retains backend object addresses. @@ -627,7 +736,12 @@ void WorkbenchWidget::refreshState(bool refreshSelectedPresentation, void WorkbenchWidget::selectThread(const QString& threadId) { - ++selectionGeneration; + const bool semanticSelectionChanged = selectedThreadId != threadId + || !projectedAgentThreadId.isEmpty(); + if (semanticSelectionChanged) { + cancelAttachmentPreparation(); + ++selectionGeneration; + } if (automaticResumeThreadId != threadId) automaticResumeAttemptedThreadIds.remove(threadId); if (!newThreadIdAwaitingState.isEmpty() && threadId != newThreadIdAwaitingState) @@ -638,13 +752,19 @@ void WorkbenchWidget::selectThread(const QString& threadId) selectedInspectorTurnId.clear(); selectedThreadId = threadId; projectedAgentThreadId.clear(); - conversation->setWriteStatus({}); + if (semanticSelectionChanged) + conversation->setWriteStatus({}); refreshState(); } void WorkbenchWidget::selectProjectedAgentThread(const QString& threadId) { - ++selectionGeneration; + const bool semanticSelectionChanged = selectedThreadId != threadId + || projectedAgentThreadId != threadId; + if (semanticSelectionChanged) { + cancelAttachmentPreparation(); + ++selectionGeneration; + } if (automaticResumeThreadId != threadId) automaticResumeAttemptedThreadIds.remove(threadId); if (!newThreadIdAwaitingState.isEmpty() && threadId != newThreadIdAwaitingState) @@ -655,7 +775,8 @@ void WorkbenchWidget::selectProjectedAgentThread(const QString& threadId) selectedInspectorTurnId.clear(); selectedThreadId = threadId; projectedAgentThreadId = threadId; - conversation->setWriteStatus({}); + if (semanticSelectionChanged) + conversation->setWriteStatus({}); refreshState(); } @@ -670,7 +791,8 @@ void WorkbenchWidget::refreshControls() const bool pendingControllerWrite = controllerAcquireInFlight || pendingAction != PendingAction::None || requestControllerAcquireInFlight || requestResponseInFlight || threadMutationInFlight; - const bool promptSubmissionInFlight = threadStartInFlight || threadResumeInFlight + const bool promptSubmissionInFlight = attachmentPreparationInFlight + || threadStartInFlight || threadResumeInFlight || turnStartInFlight || turnSteerInFlight; const bool selectedWritable = selected && selected->fullyLoaded && !selected->archived.value_or(false); @@ -697,7 +819,7 @@ bool WorkbenchWidget::writeOperationBusy() const noexcept { return pendingAction != PendingAction::None || controllerAcquireInFlight || threadStartInFlight || threadResumeInFlight || turnStartInFlight - || turnSteerInFlight + || turnSteerInFlight || attachmentPreparationInFlight || interruptInFlight || threadMutationInFlight || requestControllerAcquireInFlight || requestResponseInFlight; } @@ -1105,6 +1227,7 @@ void WorkbenchWidget::sendPrompt(const QString& prompt, bool steerRequested) pendingThreadId = selectedThreadId; pendingTurnId = QString::fromStdString(active->id.value); pendingTurnDraft = {}; + pendingSelectionGeneration = selectionGeneration; conversation->setWriteStatus(QStringLiteral("Preparing steer…")); ensureController(); return; @@ -1130,6 +1253,7 @@ void WorkbenchWidget::sendPrompt(const QString& prompt, bool steerRequested) pendingThreadId = selectedThreadId; pendingTurnId.clear(); pendingTurnDraft = settings; + pendingSelectionGeneration = selectionGeneration; conversation->setWriteStatus(QStringLiteral("Preparing write…")); ensureController(); } @@ -1329,23 +1453,22 @@ void WorkbenchWidget::executePendingAction() { const auto& state = frontendSession.state(); const auto* thread = state.thread(threadId.toStdString()); - if (!thread || !thread->fullyLoaded || thread->archived.value_or(false) + if (selectionGeneration != expectedSelectionGeneration + || selectedThreadId != threadId || !thread || !thread->fullyLoaded + || thread->archived.value_or(false) || activeTurn(state, thread) || turnSettings.threadIdentity != threadId) { showWriteError(QStringLiteral("The target thread changed before the prompt could be sent")); refreshControls(); break; } - const auto submission = prepareTurnSubmission( - threadId, prompt, attachments, attachmentWorkspace); - if (!submission) { - refreshControls(); - break; - } - // Resuming an already attached thread can replay its existing item projection. - if (ai::openai::codex::frontend::client::threadIsIdle(*thread)) - startTurn(threadId, *submission, turnSettings); - else - resumeThread(threadId, *submission, turnSettings); + beginTurnSubmissionPreparation({action, + threadId, + {}, + prompt, + attachments, + attachmentWorkspace, + turnSettings, + expectedSelectionGeneration}); break; } case PendingAction::SteerActiveTurn: @@ -1353,19 +1476,22 @@ void WorkbenchWidget::executePendingAction() const auto& state = frontendSession.state(); const auto* thread = state.thread(threadId.toStdString()); const auto* turn = activeTurn(state, thread); - if (!thread || !thread->fullyLoaded || thread->archived.value_or(false) + if (selectionGeneration != expectedSelectionGeneration + || selectedThreadId != threadId || !thread || !thread->fullyLoaded + || thread->archived.value_or(false) || !turn || QString::fromStdString(turn->id.value) != turnId) { showWriteError(QStringLiteral("The target turn changed before it could be steered")); refreshControls(); break; } - const auto submission = prepareTurnSubmission( - threadId, prompt, attachments, attachmentWorkspace); - if (!submission) { - refreshControls(); - break; - } - steerTurn(threadId, turnId, *submission); + beginTurnSubmissionPreparation({action, + threadId, + turnId, + prompt, + attachments, + attachmentWorkspace, + {}, + expectedSelectionGeneration}); break; } case PendingAction::InterruptTurn: @@ -1489,19 +1615,150 @@ void WorkbenchWidget::startNewThread(const NewThreadSetup& setup, } } -std::optional -WorkbenchWidget::prepareTurnSubmission(const QString& threadId, - const QString& prompt, - const QList& attachments, - const QString& workspace) +void WorkbenchWidget::beginTurnSubmissionPreparation( + TurnSubmissionPreparationRequest request) { - AttachmentPreparation preparation; - QString error; - if (!AttachmentManager::prepare( - attachments, workspace, threadId, &preparation, &error)) { - showWriteError(error); - return std::nullopt; + cancelAttachmentPreparation(); + const bool copiesGenericFiles = std::ranges::any_of( + request.attachments, + [](const AttachmentInfo& attachment) { + return attachment.kind == AttachmentInfo::Kind::File; + }); + const std::uint64_t generation = attachmentPreparationGeneration; + if (!copiesGenericFiles) { + TurnSubmissionPreparationOutcome outcome; + outcome.success = AttachmentManager::prepare(request.attachments, + request.workspace, + request.threadId, + &outcome.preparation, + &outcome.error); + finishTurnSubmissionPreparation(std::move(request), std::move(outcome), generation); + return; + } + + attachmentPreparationInFlight = true; + auto cancellation = std::make_shared(false); + attachmentPreparationCancellation = cancellation; + conversation->setWriteStatus(QStringLiteral("Preparing attachments…")); + refreshControls(); + QList workerAttachments = request.attachments; + QString workerWorkspace = request.workspace; + QString workerThreadId = request.threadId; + auto* watcher = new QFutureWatcher(this); + connect(watcher, + &QFutureWatcher::finished, + this, + [this, watcher, request = std::move(request), generation]() mutable { + TurnSubmissionPreparationOutcome outcome = watcher->result(); + watcher->deleteLater(); + finishTurnSubmissionPreparation( + std::move(request), std::move(outcome), generation); + }); + watcher->setFuture(QtConcurrent::run( + [attachments = std::move(workerAttachments), + workspace = std::move(workerWorkspace), + threadId = std::move(workerThreadId), + cancellation = std::move(cancellation)]() mutable { + TurnSubmissionPreparationOutcome outcome; + outcome.success = AttachmentManager::prepare(attachments, + workspace, + threadId, + &outcome.preparation, + &outcome.error, + [cancellation] { + return cancellation->load( + std::memory_order_relaxed); + }); + return outcome; + })); +} + +void WorkbenchWidget::cancelAttachmentPreparation() noexcept +{ + if (attachmentPreparationCancellation) + attachmentPreparationCancellation->store(true, std::memory_order_relaxed); + attachmentPreparationCancellation.reset(); + attachmentPreparationInFlight = false; + ++attachmentPreparationGeneration; +} + +void WorkbenchWidget::finishTurnSubmissionPreparation( + TurnSubmissionPreparationRequest request, + TurnSubmissionPreparationOutcome outcome, + std::uint64_t preparationGeneration) +{ + if (preparationGeneration != attachmentPreparationGeneration) + return; + attachmentPreparationCancellation.reset(); + attachmentPreparationInFlight = false; + if (!outcome.success) { + showWriteError(outcome.error.isEmpty() + ? QStringLiteral("Unable to prepare attachments") + : outcome.error); + refreshControls(); + return; + } + if (frontendSession.lifecycle() != FrontendSession::Lifecycle::Ready + || selectionGeneration != request.expectedSelectionGeneration + || selectedThreadId != request.threadId) { + showWriteError(QStringLiteral( + "The target thread changed while attachments were being prepared")); + refreshControls(); + return; } + + auto submission = preparedTurnSubmission( + request.prompt, request.attachments, std::move(outcome.preparation)); + if (!submission) { + refreshControls(); + return; + } + + const auto& state = frontendSession.state(); + const auto* thread = state.thread(request.threadId.toStdString()); + if (request.action == PendingAction::SendExistingThread) { + if (!thread || !thread->fullyLoaded || thread->archived.value_or(false) + || activeTurn(state, thread) + || request.settings.threadIdentity != request.threadId) { + showWriteError(QStringLiteral( + "The target thread changed while attachments were being prepared")); + refreshControls(); + return; + } + // Resuming an already attached thread can replay its existing item projection. + if (ai::openai::codex::frontend::client::threadIsIdle(*thread)) + startTurn(request.threadId, *submission, request.settings); + else + resumeThread(request.threadId, *submission, request.settings); + return; + } + if (request.action == PendingAction::SteerActiveTurn) { + if (!thread || !thread->fullyLoaded || thread->archived.value_or(false)) { + showWriteError(QStringLiteral( + "The target turn changed while attachments were being prepared")); + refreshControls(); + return; + } + const auto* turn = activeTurn(state, thread); + if (!turn || QString::fromStdString(turn->id.value) != request.turnId) { + showWriteError(QStringLiteral( + "The target turn changed while attachments were being prepared")); + refreshControls(); + return; + } + steerTurn(request.threadId, request.turnId, *submission); + return; + } + + showWriteError(QStringLiteral("The pending write changed while attachments were being prepared")); + refreshControls(); +} + +std::optional +WorkbenchWidget::preparedTurnSubmission(const QString& prompt, + const QList& attachments, + AttachmentPreparation preparation) +{ const QString effectivePrompt = AttachmentManager::composePrompt(prompt, preparation); if (const auto validationError = FrontendSession::promptValidationError(effectivePrompt)) { if (preparation.stagingLease) @@ -2146,6 +2403,7 @@ void WorkbenchWidget::clearWriteTransients() threadResumeInFlight = false; turnStartInFlight = false; turnSteerInFlight = false; + cancelAttachmentPreparation(); interruptInFlight = false; threadMutationInFlight = false; controllerUnavailable = false; diff --git a/src/ui/WorkbenchWidget.h b/src/ui/WorkbenchWidget.h index 7c19093..cbd8236 100644 --- a/src/ui/WorkbenchWidget.h +++ b/src/ui/WorkbenchWidget.h @@ -14,8 +14,10 @@ #include #include -#include +#include #include +#include +#include class QFrame; class QLabel; @@ -38,6 +40,7 @@ class WorkbenchWidget : public QWidget { public: explicit WorkbenchWidget(FrontendSession& frontendSession, QWidget* parent = nullptr); + ~WorkbenchWidget() override; private: enum class PendingAction { @@ -76,12 +79,30 @@ class WorkbenchWidget : public QWidget AttachmentStagingLeasePtr lease; }; + struct TurnSubmissionPreparationRequest { + PendingAction action = PendingAction::None; + QString threadId; + QString turnId; + QString prompt; + QList attachments; + QString workspace; + UpcomingTurnDraft settings; + std::uint64_t expectedSelectionGeneration = 0; + }; + + struct TurnSubmissionPreparationOutcome { + AttachmentPreparation preparation; + QString error; + bool success = false; + }; + void refreshLifecycle(); void scheduleStateRefresh(const detail::StateUpdateScope& scope); void refreshState(bool refreshSelectedPresentation = true, bool refreshInspector = true, bool refreshSidebar = true, - const ConversationContentUpdates* exactContentChanges = nullptr); + const ConversationContentUpdates* exactContentChanges = nullptr, + const QStringList* sidebarThreadChanges = nullptr); void refreshControls(); void refreshControllerStatus(); [[nodiscard]] bool writeOperationBusy() const noexcept; @@ -107,11 +128,15 @@ class WorkbenchWidget : public QWidget const ResumeWithOptionsSetup& setup, std::uint64_t expectedSelectionGeneration); void mutateThread(PendingAction action, const QString& threadId, const QString& value = {}); - [[nodiscard]] std::optional - prepareTurnSubmission(const QString& threadId, - const QString& prompt, - const QList& attachments, - const QString& workspace); + void beginTurnSubmissionPreparation(TurnSubmissionPreparationRequest request); + void cancelAttachmentPreparation() noexcept; + void finishTurnSubmissionPreparation(TurnSubmissionPreparationRequest request, + TurnSubmissionPreparationOutcome outcome, + std::uint64_t preparationGeneration); + [[nodiscard]] std::optional preparedTurnSubmission( + const QString& prompt, + const QList& attachments, + AttachmentPreparation preparation); void resumeThread(const QString& threadId, const PreparedTurnSubmission& submission, const UpcomingTurnDraft& settings); @@ -188,6 +213,9 @@ class WorkbenchWidget : public QWidget bool threadResumeInFlight = false; bool turnStartInFlight = false; bool turnSteerInFlight = false; + bool attachmentPreparationInFlight = false; + std::uint64_t attachmentPreparationGeneration = 0; + std::shared_ptr attachmentPreparationCancellation; bool interruptInFlight = false; bool threadMutationInFlight = false; bool controllerUnavailable = false; @@ -199,8 +227,12 @@ class WorkbenchWidget : public QWidget bool selectedPresentationRefreshPending = false; bool selectedPresentationFullRefreshPending = false; ConversationContentUpdates selectedContentRefreshPending; + std::uint64_t selectedContentRefreshPendingBytes = 0; bool inspectorRefreshPending = false; bool sidebarRefreshPending = false; + bool sidebarFullRefreshPending = false; + QStringList sidebarThreadRefreshPending; + QSet sidebarThreadRefreshPendingSet; }; } // namespace codexui diff --git a/tests/AttachmentManagerTest.cpp b/tests/AttachmentManagerTest.cpp index 8004dfa..c623dea 100644 --- a/tests/AttachmentManagerTest.cpp +++ b/tests/AttachmentManagerTest.cpp @@ -151,6 +151,23 @@ int main(int argc, char** argv) "explicit terminal cleanup must remove exact staged files and their empty directory"); inFlightLease.reset(); + const QString cancellablePath = writeFile( + QDir(source.path()).filePath(QStringLiteral("cancellable.bin")), + QByteArray(2 * 1024 * 1024, 'x')); + const codexui::AttachmentInfo cancellable = inspect(cancellablePath); + codexui::AttachmentPreparation cancelledPreparation; + error.clear(); + int cancellationChecks = 0; + passed &= expect(!codexui::AttachmentManager::prepare( + {cancellable}, workspace.path(), QStringLiteral("thread-cancelled"), + &cancelledPreparation, &error, + [&cancellationChecks] { return ++cancellationChecks >= 5; }) + && error.contains(QStringLiteral("cancelled"), Qt::CaseInsensitive) + && cancelledPreparation.stagingDirectory.isEmpty() + && QDir(attachmentsPath).entryList( + QDir::NoDotAndDotDot | QDir::Dirs).isEmpty(), + "cooperative cancellation during a chunked copy must remove partial staging"); + QTemporaryDir registryDirectory; passed &= expect(registryDirectory.isValid(), "an isolated staging registry must be available"); diff --git a/tests/ConversationLayoutTest.cpp b/tests/ConversationLayoutTest.cpp index c4864a3..564924f 100644 --- a/tests/ConversationLayoutTest.cpp +++ b/tests/ConversationLayoutTest.cpp @@ -48,6 +48,9 @@ struct MessageFixture bool genericItemTruncatedOnly = false; std::string command; std::string reasoningSummary; + std::string agentPath; + std::string agentThreadId; + std::string agentKind; }; struct TurnFixture @@ -145,6 +148,10 @@ frontend::Json messageJson(const std::string& threadId, {"durationMs", 42}}; if (fixture.status == "completed") data["exitCode"] = 0; + } else if (fixture.kind == frontend::ThreadItemKind::SubAgentActivity) { + data = frontend::Json{{"agentPath", fixture.agentPath}, + {"agentThreadId", fixture.agentThreadId}, + {"kind", fixture.agentKind}}; } const bool carriesCommandOutput = fixture.kind == frontend::ThreadItemKind::CommandExecution || fixture.kind == frontend::ThreadItemKind::FileChange; @@ -1823,6 +1830,43 @@ bool testInspectorRevisionOnlyUpdate() "a revision-only update must preserve every existing Inspector pane widget"); } +bool testInspectorThreadDependencies() +{ + MessageFixture activity; + activity.id = "subagent-activity"; + activity.kind = frontend::ThreadItemKind::SubAgentActivity; + activity.text = "Delegated work"; + activity.agentPath = "agent/reviewer"; + activity.agentThreadId = "inspector-agent-child"; + activity.agentKind = "spawn"; + const client::State state = makeState( + {{"inspector-parent", {{"turn-inspector-parent", {activity}, std::nullopt}}}, + singleTurn("inspector-agent-child", 1)}); + + codexui::InspectorWidget inspector; + inspector.render(state, + QStringLiteral("inspector-parent"), + true, + QStringLiteral("State synced")); + bool passed = expect(inspector.dependsOnThread(QStringLiteral("inspector-parent")) + && inspector.dependsOnThread( + QStringLiteral("inspector-agent-child")) + && !inspector.dependsOnThread(QStringLiteral("unrelated")), + "Inspector invalidation must include its selected parent and linked agent thread only"); + + const client::State withoutActivity = makeState( + {singleTurn("inspector-parent", 1), singleTurn("inspector-agent-child", 1)}); + inspector.render(withoutActivity, + QStringLiteral("inspector-parent"), + true, + QStringLiteral("State synced")); + passed &= expect(inspector.dependsOnThread(QStringLiteral("inspector-parent")) + && !inspector.dependsOnThread( + QStringLiteral("inspector-agent-child")), + "removing subagent activity must discard its stale linked-thread dependency"); + return passed; +} + bool testStructuredPlanPresentation() { ThreadFixture fixture{"structured-plan", @@ -2078,6 +2122,7 @@ int main(int argc, char** argv) passed &= testSegmentReplacementShrink(); passed &= testThreadSwitchWindow(); passed &= testInspectorRevisionOnlyUpdate(); + passed &= testInspectorThreadDependencies(); passed &= testStructuredPlanPresentation(); passed &= testHistoricalTurnDetailsMode(); passed &= testScopedDuplicateTurnIdentity(); diff --git a/tests/FrontendSessionTest.cpp b/tests/FrontendSessionTest.cpp index a928c65..72e6da3 100644 --- a/tests/FrontendSessionTest.cpp +++ b/tests/FrontendSessionTest.cpp @@ -515,17 +515,34 @@ bool testScopedItemPresentationChanges() replacementUpdate.changes.push_back(sdk::StateReplacedChange{}); const auto replacement = codexui::detail::stateUpdateScope(replacementUpdate); + sdk::StateUpdate threadUpdate; + threadUpdate.changes.push_back( + sdk::ThreadUpsertedChange{ai::openai::codex::typed::ThreadId{"target-thread"}}); + const auto threadScoped = codexui::detail::stateUpdateScope(threadUpdate); + sdk::StateUpdate cursorUpdate; cursorUpdate.changes.push_back( sdk::CursorAdvancedChange{ai::openai::codex::frontend::SequenceNumber{43}}); const auto cursor = codexui::detail::stateUpdateScope(cursorUpdate); + sdk::StateUpdate oversizedIdentityUpdate; + for (int index = 0; + index <= codexui::detail::maximumCoalescedPresentationIdentities; + ++index) { + oversizedIdentityUpdate.changes.push_back( + sdk::ThreadUpsertedChange{ai::openai::codex::typed::ThreadId{ + "thread-" + std::to_string(index)}}); + } + const auto boundedIdentities = + codexui::detail::stateUpdateScope(oversizedIdentityUpdate); + bool passed = expect(unresolvedTurn.affectedThreadIds.empty() && unresolvedTurn.fullyAffectedThreadIds.empty() && unresolvedTurn.affectedInspectorThreadIds.empty() && unresolvedTurn.allThreadsAffected && unresolvedTurn.allInspectorsAffected - && !unresolvedTurn.sidebarAffected + && unresolvedTurn.allSidebarThreadsAffected + && unresolvedTurn.sidebarAffected && unresolvedTurn.hasPresentationChange, "a turn upsert without a unique parent lookup must conservatively refresh all threads"); passed &= expect(scoped.affectedThreadIds == QStringList{QStringLiteral("target-thread")} @@ -587,11 +604,34 @@ bool testScopedItemPresentationChanges() && unscoped.hasPresentationChange, "an unscoped item change must conservatively refresh all thread-bound presentations"); passed &= expect(replacement.allThreadsAffected && replacement.allInspectorsAffected + && replacement.allSidebarThreadsAffected && replacement.sidebarAffected && replacement.hasPresentationChange, "a State replacement must conservatively refresh every presentation"); + passed &= expect(threadScoped.affectedThreadIds + == QStringList{QStringLiteral("target-thread")} + && threadScoped.fullyAffectedThreadIds + == QStringList{QStringLiteral("target-thread")} + && threadScoped.affectedInspectorThreadIds + == QStringList{QStringLiteral("target-thread")} + && threadScoped.affectedSidebarThreadIds + == QStringList{QStringLiteral("target-thread")} + && !threadScoped.allThreadsAffected + && !threadScoped.allInspectorsAffected + && !threadScoped.allSidebarThreadsAffected + && threadScoped.sidebarAffected, + "a thread upsert must target only its conversation, Inspector dependencies, and Sidebar row"); passed &= expect(!cursor.allThreadsAffected && !cursor.allInspectorsAffected + && !cursor.allSidebarThreadsAffected && !cursor.sidebarAffected && cursor.hasPresentationChange, "a cursor-only update must dispatch its revision without dirtying broad presentation"); + passed &= expect(boundedIdentities.allThreadsAffected + && boundedIdentities.allInspectorsAffected + && boundedIdentities.allSidebarThreadsAffected + && boundedIdentities.affectedThreadIds.empty() + && boundedIdentities.fullyAffectedThreadIds.empty() + && boundedIdentities.affectedInspectorThreadIds.empty() + && boundedIdentities.affectedSidebarThreadIds.empty(), + "an oversized identity batch must stop at the presentation bound and degrade to full refreshes"); return passed; } @@ -973,6 +1013,7 @@ bool testArchivedThreadRefresh() && discoverySignals == 1 && discoveryScope && discoveryScope->allThreadsAffected && discoveryScope->allInspectorsAffected + && discoveryScope->allSidebarThreadsAffected && discoveryScope->sidebarAffected && discoveryScope->hasPresentationChange, "the terminal page must publish completion and one conservative presentation refresh"); @@ -1595,6 +1636,24 @@ bool testThreadedFacadeMailbox() static_cast( codexui::detail::maximumCoalescedContentDeltaBytes + 1), 'x'))); + { + codexui::detail::StateUpdateScope scope; + scope.affectedSidebarThreadIds = { + QStringLiteral("sidebar-a"), QStringLiteral("sidebar-b")}; + scope.sidebarAffected = true; + scope.hasPresentationChange = true; + codexui::FrontendSessionFacadeTestAccess::enqueueState( + session, 7, std::move(scope)); + } + { + codexui::detail::StateUpdateScope scope; + scope.affectedSidebarThreadIds = { + QStringLiteral("sidebar-b"), QStringLiteral("sidebar-c")}; + scope.sidebarAffected = true; + scope.hasPresentationChange = true; + codexui::FrontendSessionFacadeTestAccess::enqueueState( + session, 7, std::move(scope)); + } for (int index = 0; index < 1'000; ++index) { codexui::detail::StateUpdateScope scope; scope.affectedThreadIds.push_back(QStringLiteral("streaming-thread")); @@ -1679,6 +1738,14 @@ bool testThreadedFacadeMailbox() && !oversized->append && deliveredScope->coalescedContentDeltaBytes == 13, "the one-slot mailbox must merge only bounded contiguous same-channel appends and degrade ambiguous or oversized sequences to replacement"); + passed &= expect( + deliveredScope->sidebarAffected + && !deliveredScope->allSidebarThreadsAffected + && deliveredScope->affectedSidebarThreadIds + == QStringList{QStringLiteral("sidebar-a"), + QStringLiteral("sidebar-b"), + QStringLiteral("sidebar-c")}, + "the one-slot mailbox must merge and deduplicate targeted Sidebar rows"); } passed &= expect(statusSignals == 1 && session.statusText() @@ -1943,11 +2010,29 @@ bool testFacadeScopeBound() session, 1, std::move(scope)); } QCoreApplication::processEvents(); - const bool passed = expect( + bool passed = expect( delivered && delivered->allThreadsAffected && delivered->affectedThreadIds.empty() && delivered->affectedItemContents.empty(), "a blocked GUI must degrade an unbounded exact-scope burst to one bounded full refresh"); + + delivered.reset(); + for (int index = 0; + index <= codexui::detail::maximumCoalescedPresentationIdentities; + ++index) { + codexui::detail::StateUpdateScope scope; + scope.affectedSidebarThreadIds.push_back( + QStringLiteral("sidebar-%1").arg(index)); + scope.sidebarAffected = true; + scope.hasPresentationChange = true; + codexui::FrontendSessionFacadeTestAccess::enqueueState( + session, 1, std::move(scope)); + } + QCoreApplication::processEvents(); + passed &= expect(delivered && delivered->sidebarAffected + && delivered->allSidebarThreadsAffected + && delivered->affectedSidebarThreadIds.empty(), + "a blocked GUI must bound targeted Sidebar identities and let full-refresh dominance clear them"); session.shutdown(); return passed; } diff --git a/tests/Phase1ThreadTurnUxTest.cpp b/tests/Phase1ThreadTurnUxTest.cpp index d6d9a04..6211bbd 100644 --- a/tests/Phase1ThreadTurnUxTest.cpp +++ b/tests/Phase1ThreadTurnUxTest.cpp @@ -960,6 +960,80 @@ bool testScopedDuplicateTurnActionGating() return passed; } +bool testTargetedSidebarRefreshKeepsUnchangedRows() +{ + const sdk::State initial = threadDiscoveryState( + {{"thread-a", false}, {"thread-b", false}}, std::string{"no-active-thread"}); + const sdk::State updated = threadDiscoveryState( + {{"thread-a", false}, {"thread-b", false}}, std::string{"thread-a"}); + codexui::SidebarWidget sidebar; + sidebar.setThreads(initial, QStringLiteral("thread-a"), true); + + const auto rowFor = [&sidebar](const QString& threadId) -> QFrame* { + for (QFrame* row : sidebar.findChildren(QStringLiteral("threadRow"))) { + if (row->property("threadId").toString() == threadId) + return row; + } + return nullptr; + }; + const auto detailsFor = [](QFrame* row) { + if (!row) + return QString{}; + for (QLabel* label : row->findChildren()) { + if (label->property("kind").toString() == QStringLiteral("meta")) + return label->toolTip(); + } + return QString{}; + }; + + QFrame* threadARow = rowFor(QStringLiteral("thread-a")); + QFrame* threadBRow = rowFor(QStringLiteral("thread-b")); + const QString threadBBefore = detailsFor(threadBRow); + sidebar.updateThreads(updated, + QStringLiteral("thread-a"), + true, + {QStringLiteral("thread-a")}); + + bool passed = expect(threadARow && threadBRow + && rowFor(QStringLiteral("thread-a")) == threadARow + && rowFor(QStringLiteral("thread-b")) == threadBRow, + "a targeted Sidebar update must retain existing row widgets"); + passed &= expect(detailsFor(threadARow).contains(QStringLiteral("Running")) + && detailsFor(threadBRow) == threadBBefore, + "a targeted Sidebar update must recompute only the affected row"); + + const sdk::State removed = threadDiscoveryState({{"thread-b", false}}); + sidebar.updateThreads(removed, + QStringLiteral("thread-a"), + true, + {QStringLiteral("thread-a")}); + settleEvents(); + passed &= expect(rowFor(QStringLiteral("thread-a")) == nullptr + && rowFor(QStringLiteral("thread-b")) != nullptr, + "a targeted removal must fall back to authoritative tree reconstruction"); + + sidebar.updateThreads(initial, + QStringLiteral("thread-a"), + true, + {QStringLiteral("thread-a")}); + settleEvents(); + passed &= expect(rowFor(QStringLiteral("thread-a")) != nullptr + && rowFor(QStringLiteral("thread-b")) != nullptr, + "a targeted insertion must fall back to authoritative tree reconstruction"); + + const sdk::State archived = threadDiscoveryState( + {{"thread-a", true}, {"thread-b", false}}); + sidebar.updateThreads(archived, + QStringLiteral("thread-a"), + true, + {QStringLiteral("thread-a")}); + settleEvents(); + passed &= expect(detailsFor(rowFor(QStringLiteral("thread-a"))) + .contains(QStringLiteral("Archived")), + "an archive-boundary change must rebuild the thread hierarchy"); + return passed; +} + bool testThreadOrganizationPersistenceAndSafeMoves() { QTemporaryDir temporaryDirectory; @@ -1249,6 +1323,7 @@ int main(int argc, char** argv) passed &= testThreadSetupResults(); passed &= testThreadActionGating(); passed &= testScopedDuplicateTurnActionGating(); + passed &= testTargetedSidebarRefreshKeepsUnchangedRows(); passed &= testThreadOrganizationPersistenceAndSafeMoves(); passed &= testArchivedThreadAssignmentPruningWaitsForCompleteDiscovery(); return passed ? 0 : 1; From cba07583a74a6002c2cbb39915362833b83e9b35 Mon Sep 17 00:00:00 2001 From: Volker Christian Date: Fri, 21 Aug 2026 20:36:56 +0200 Subject: [PATCH 2/2] Bound presentation refresh accumulation --- CMakeLists.txt | 21 +++ src/ui/PresentationRefreshAccumulator.cpp | 110 ++++++++++++ src/ui/PresentationRefreshAccumulator.h | 31 ++++ src/ui/WorkbenchWidget.cpp | 110 ++---------- tests/PresentationRefreshAccumulatorTest.cpp | 176 +++++++++++++++++++ 5 files changed, 353 insertions(+), 95 deletions(-) create mode 100644 src/ui/PresentationRefreshAccumulator.cpp create mode 100644 src/ui/PresentationRefreshAccumulator.h create mode 100644 tests/PresentationRefreshAccumulatorTest.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 8899659..bd0de4c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -35,6 +35,8 @@ qt_add_executable( src/ui/InteractiveRequestDialog.h src/ui/MainWindow.cpp src/ui/MainWindow.h + src/ui/PresentationRefreshAccumulator.cpp + src/ui/PresentationRefreshAccumulator.h src/ui/SidebarWidget.cpp src/ui/SidebarWidget.h src/ui/ThreadSetupDialog.cpp @@ -111,6 +113,25 @@ if(BUILD_TESTING) ) add_test(NAME CodexUIFrontendSessionTest COMMAND CodexUIFrontendSessionTest) + add_executable( + CodexUIPresentationRefreshAccumulatorTest + tests/PresentationRefreshAccumulatorTest.cpp + src/ui/PresentationRefreshAccumulator.cpp + src/ui/PresentationRefreshAccumulator.h + ) + target_compile_features(CodexUIPresentationRefreshAccumulatorTest PRIVATE cxx_std_20) + target_include_directories(CodexUIPresentationRefreshAccumulatorTest PRIVATE src) + target_link_libraries( + CodexUIPresentationRefreshAccumulatorTest + PRIVATE + AISuite::OpenAICodexFrontendClient + Qt6::Widgets + ) + add_test( + NAME CodexUIPresentationRefreshAccumulatorTest + COMMAND CodexUIPresentationRefreshAccumulatorTest + ) + add_executable( CodexUIConversationLayoutTest tests/ConversationLayoutTest.cpp diff --git a/src/ui/PresentationRefreshAccumulator.cpp b/src/ui/PresentationRefreshAccumulator.cpp new file mode 100644 index 0000000..f8f5112 --- /dev/null +++ b/src/ui/PresentationRefreshAccumulator.cpp @@ -0,0 +1,110 @@ +// SPDX-License-Identifier: LGPL-3.0-or-later OR MIT + +#include "ui/PresentationRefreshAccumulator.h" + +#include +#include + +namespace codexui::detail { +namespace { + +[[nodiscard]] bool canRetainBytes(std::uint64_t retained, + std::uint64_t additional) noexcept +{ + return retained <= maximumCoalescedContentDeltaBytes + && additional <= maximumCoalescedContentDeltaBytes - retained; +} + +} // namespace + +BoundedMergeResult mergeConversationContentUpdate( + ConversationContentUpdates& updates, + std::uint64_t& retainedUtf8Bytes, + const StateUpdateScope::ItemContentIdentity& identity) +{ + auto existing = std::find_if( + updates.begin(), updates.end(), + [&identity](const ConversationContentUpdate& update) + { + return update.turnId == identity.turnId + && update.itemId == identity.itemId + && update.channel == identity.channel; + }); + if (existing == updates.end()) + { + if (static_cast(updates.size()) + >= maximumCoalescedPresentationIdentities) + return BoundedMergeResult::CapacityExceeded; + + ConversationContentUpdate next{ + identity.turnId, + identity.itemId, + identity.channel, + std::nullopt}; + if (identity.append) + { + const std::uint64_t deltaBytes = static_cast( + identity.append->deltaUtf8.size()); + if (!canRetainBytes(retainedUtf8Bytes, deltaBytes)) + return BoundedMergeResult::CapacityExceeded; + next.append = ConversationContentAppend{ + identity.append->baseContentBytes, + identity.append->discardPrefixBytes, + deltaBytes, + QString::fromUtf8(identity.append->deltaUtf8)}; + retainedUtf8Bytes += deltaBytes; + } + updates.push_back(std::move(next)); + return BoundedMergeResult::Retained; + } + + if (existing->append && identity.append + && existing->append->discardPrefixBytes == 0 + && identity.append->discardPrefixBytes == 0 + && existing->append->baseContentBytes + <= std::numeric_limits::max() + - existing->append->deltaUtf8Bytes + && existing->append->baseContentBytes + + existing->append->deltaUtf8Bytes + == identity.append->baseContentBytes) + { + const std::uint64_t deltaBytes = static_cast( + identity.append->deltaUtf8.size()); + if (!canRetainBytes(retainedUtf8Bytes, deltaBytes)) + return BoundedMergeResult::CapacityExceeded; + existing->append->delta.append(QString::fromUtf8(identity.append->deltaUtf8)); + existing->append->deltaUtf8Bytes += deltaBytes; + retainedUtf8Bytes += deltaBytes; + return BoundedMergeResult::Retained; + } + + // A replacement, rolling window, or non-contiguous append is represented + // by an authoritative item refresh. It no longer retains the previous + // optional text delta in this frame accumulator. + if (existing->append) + { + retainedUtf8Bytes = existing->append->deltaUtf8Bytes + <= retainedUtf8Bytes + ? retainedUtf8Bytes + - existing->append->deltaUtf8Bytes + : 0; + } + existing->append.reset(); + return BoundedMergeResult::Retained; +} + +BoundedMergeResult appendUniqueSidebarThread( + QStringList& orderedThreadIds, + QSet& retainedThreadIds, + const QString& threadId) +{ + if (retainedThreadIds.contains(threadId)) + return BoundedMergeResult::Retained; + if (orderedThreadIds.size() >= maximumCoalescedPresentationIdentities) + return BoundedMergeResult::CapacityExceeded; + retainedThreadIds.insert(threadId); + orderedThreadIds.append(threadId); + return BoundedMergeResult::Retained; +} + +} // namespace codexui::detail diff --git a/src/ui/PresentationRefreshAccumulator.h b/src/ui/PresentationRefreshAccumulator.h new file mode 100644 index 0000000..626bd9d --- /dev/null +++ b/src/ui/PresentationRefreshAccumulator.h @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: LGPL-3.0-or-later OR MIT + +#ifndef CODEXUI_UI_PRESENTATIONREFRESHACCUMULATOR_H +#define CODEXUI_UI_PRESENTATIONREFRESHACCUMULATOR_H + +#include "app/FrontendSession.h" +#include "ui/ConversationWidget.h" + +#include +#include +#include + +#include + +namespace codexui::detail { + +enum class BoundedMergeResult { Retained, CapacityExceeded }; + +[[nodiscard]] BoundedMergeResult mergeConversationContentUpdate( + ConversationContentUpdates& updates, + std::uint64_t& retainedUtf8Bytes, + const StateUpdateScope::ItemContentIdentity& identity); + +[[nodiscard]] BoundedMergeResult appendUniqueSidebarThread( + QStringList& orderedThreadIds, + QSet& retainedThreadIds, + const QString& threadId); + +} // namespace codexui::detail + +#endif // CODEXUI_UI_PRESENTATIONREFRESHACCUMULATOR_H diff --git a/src/ui/WorkbenchWidget.cpp b/src/ui/WorkbenchWidget.cpp index 737da9a..ebf0eb4 100644 --- a/src/ui/WorkbenchWidget.cpp +++ b/src/ui/WorkbenchWidget.cpp @@ -6,6 +6,7 @@ #include "ui/ConversationWidget.h" #include "ui/InspectorWidget.h" #include "ui/InteractiveRequestDialog.h" +#include "ui/PresentationRefreshAccumulator.h" #include "ui/SidebarWidget.h" #include "ui/ThreadSetupDialog.h" #include "ui/UpcomingTurnDock.h" @@ -34,7 +35,6 @@ #include #include -#include #include namespace codexui { @@ -352,95 +352,16 @@ void WorkbenchWidget::scheduleStateRefresh(const detail::StateUpdateScope& scope if (identity.threadId != selectedThreadId) continue; foundExactContent = true; - auto existing = std::find_if( - selectedContentRefreshPending.begin(), - selectedContentRefreshPending.end(), - [&identity](const ConversationContentUpdate& update) - { - return update.turnId == identity.turnId - && update.itemId == identity.itemId - && update.channel == identity.channel; - }); - if (existing == selectedContentRefreshPending.end()) - { - if (static_cast(selectedContentRefreshPending.size()) - >= detail::maximumCoalescedPresentationIdentities) - { - selectedPresentationFullRefreshPending = true; - selectedContentRefreshPending.clear(); - selectedContentRefreshPendingBytes = 0; - break; - } - ConversationContentUpdate next{ - identity.turnId, - identity.itemId, - identity.channel, - std::nullopt}; - if (identity.append) - { - const std::uint64_t deltaBytes = static_cast( - identity.append->deltaUtf8.size()); - if (selectedContentRefreshPendingBytes - > detail::maximumCoalescedContentDeltaBytes - || deltaBytes - > detail::maximumCoalescedContentDeltaBytes - - selectedContentRefreshPendingBytes) - { - selectedPresentationFullRefreshPending = true; - selectedContentRefreshPending.clear(); - selectedContentRefreshPendingBytes = 0; - break; - } - next.append = ConversationContentAppend{ - identity.append->baseContentBytes, - identity.append->discardPrefixBytes, - deltaBytes, - QString::fromUtf8(identity.append->deltaUtf8)}; - selectedContentRefreshPendingBytes += deltaBytes; - } - selectedContentRefreshPending.push_back(std::move(next)); - } - else if (existing->append && identity.append - && existing->append->discardPrefixBytes == 0 - && identity.append->discardPrefixBytes == 0 - && existing->append->baseContentBytes - <= std::numeric_limits::max() - - existing->append->deltaUtf8Bytes - && existing->append->baseContentBytes - + existing->append->deltaUtf8Bytes - == identity.append->baseContentBytes) - { - const std::uint64_t deltaBytes = static_cast( - identity.append->deltaUtf8.size()); - if (selectedContentRefreshPendingBytes - > detail::maximumCoalescedContentDeltaBytes - || deltaBytes - > detail::maximumCoalescedContentDeltaBytes - - selectedContentRefreshPendingBytes) - { - selectedPresentationFullRefreshPending = true; - selectedContentRefreshPending.clear(); - selectedContentRefreshPendingBytes = 0; - break; - } - existing->append->delta.append(QString::fromUtf8(identity.append->deltaUtf8)); - existing->append->deltaUtf8Bytes += deltaBytes; - selectedContentRefreshPendingBytes += deltaBytes; - } - else + if (detail::mergeConversationContentUpdate( + selectedContentRefreshPending, + selectedContentRefreshPendingBytes, + identity) + == detail::BoundedMergeResult::CapacityExceeded) { - // Ambiguous, rolling, or replacement updates retain the - // authoritative State fallback instead of guessing a delta. - if (existing->append) - { - selectedContentRefreshPendingBytes = - existing->append->deltaUtf8Bytes - <= selectedContentRefreshPendingBytes - ? selectedContentRefreshPendingBytes - - existing->append->deltaUtf8Bytes - : 0; - } - existing->append.reset(); + selectedPresentationFullRefreshPending = true; + selectedContentRefreshPending.clear(); + selectedContentRefreshPendingBytes = 0; + break; } } // A conversation-affecting update without an exact item identity @@ -462,17 +383,16 @@ void WorkbenchWidget::scheduleStateRefresh(const detail::StateUpdateScope& scope sidebarThreadRefreshPendingSet.clear(); } else if (!sidebarFullRefreshPending) { for (const QString& threadId : scope.affectedSidebarThreadIds) { - if (sidebarThreadRefreshPendingSet.contains(threadId)) - continue; - if (sidebarThreadRefreshPending.size() - >= detail::maximumCoalescedPresentationIdentities) { + if (detail::appendUniqueSidebarThread( + sidebarThreadRefreshPending, + sidebarThreadRefreshPendingSet, + threadId) + == detail::BoundedMergeResult::CapacityExceeded) { sidebarFullRefreshPending = true; sidebarThreadRefreshPending.clear(); sidebarThreadRefreshPendingSet.clear(); break; } - sidebarThreadRefreshPendingSet.insert(threadId); - sidebarThreadRefreshPending.append(threadId); } if (scope.affectedSidebarThreadIds.isEmpty()) { sidebarFullRefreshPending = true; diff --git a/tests/PresentationRefreshAccumulatorTest.cpp b/tests/PresentationRefreshAccumulatorTest.cpp new file mode 100644 index 0000000..ed6c703 --- /dev/null +++ b/tests/PresentationRefreshAccumulatorTest.cpp @@ -0,0 +1,176 @@ +// SPDX-License-Identifier: LGPL-3.0-or-later OR MIT + +#include "ui/PresentationRefreshAccumulator.h" + +#include + +#include + +namespace { + +namespace detail = codexui::detail; +namespace client = ai::openai::codex::frontend::client; + +bool expect(bool condition, const char* message) +{ + if (!condition) + std::cerr << message << '\n'; + return condition; +} + +detail::StateUpdateScope::ItemContentIdentity identity( + int index, + QByteArray delta = {}, + std::uint64_t base = 0, + std::uint64_t discard = 0) +{ + detail::StateUpdateScope::ItemContentIdentity result; + result.threadId = QStringLiteral("thread"); + result.turnId = QStringLiteral("turn"); + result.itemId = QStringLiteral("item-%1").arg(index); + result.channel = client::ItemContentChannel::CommandOutput; + if (!delta.isNull()) + { + result.append = detail::StateUpdateScope::ItemContentAppend{ + base, discard, std::move(delta)}; + } + return result; +} + +bool testIdentityBound() +{ + codexui::ConversationContentUpdates updates; + std::uint64_t retainedBytes = 0; + bool passed = true; + for (qsizetype index = 0; + index < detail::maximumCoalescedPresentationIdentities; + ++index) + { + passed &= detail::mergeConversationContentUpdate( + updates, retainedBytes, identity(static_cast(index))) + == detail::BoundedMergeResult::Retained; + } + const auto retainedSize = updates.size(); + passed &= expect( + detail::mergeConversationContentUpdate( + updates, + retainedBytes, + identity(static_cast(detail::maximumCoalescedPresentationIdentities))) + == detail::BoundedMergeResult::CapacityExceeded + && updates.size() == retainedSize && retainedBytes == 0, + "the frame accumulator must retain exactly 1024 identities and reject the 1025th without mutation"); + return passed; +} + +bool testAggregateByteBound() +{ + codexui::ConversationContentUpdates updates; + std::uint64_t retainedBytes = 0; + QByteArray maximum( + static_cast(detail::maximumCoalescedContentDeltaBytes), 'x'); + bool passed = detail::mergeConversationContentUpdate( + updates, retainedBytes, identity(0, std::move(maximum))) + == detail::BoundedMergeResult::Retained; + const auto retainedSize = updates.size(); + passed &= expect( + detail::mergeConversationContentUpdate( + updates, + retainedBytes, + identity(1, QByteArray(1, 'y'))) + == detail::BoundedMergeResult::CapacityExceeded + && updates.size() == retainedSize + && retainedBytes == detail::maximumCoalescedContentDeltaBytes, + "the frame accumulator must reject the first byte beyond its aggregate 1 MiB bound without mutation"); + return passed; +} + +bool testReplacementReleasesRetainedBytes() +{ + codexui::ConversationContentUpdates updates; + std::uint64_t retainedBytes = 0; + const auto half = detail::maximumCoalescedContentDeltaBytes / 2; + bool passed = detail::mergeConversationContentUpdate( + updates, + retainedBytes, + identity(0, QByteArray(static_cast(half), 'a'))) + == detail::BoundedMergeResult::Retained; + passed &= detail::mergeConversationContentUpdate( + updates, + retainedBytes, + identity(0, QByteArray(1, 'b'), 9'999)) + == detail::BoundedMergeResult::Retained; + passed &= expect(retainedBytes == 0 && updates.size() == 1 + && !updates.front().append, + "a non-contiguous update must become an authoritative replacement and release retained delta bytes"); + passed &= expect( + detail::mergeConversationContentUpdate( + updates, + retainedBytes, + identity(1, + QByteArray( + static_cast(detail::maximumCoalescedContentDeltaBytes), + 'c'))) + == detail::BoundedMergeResult::Retained + && retainedBytes == detail::maximumCoalescedContentDeltaBytes, + "released replacement bytes must be available to a later independent exact append"); + return passed; +} + +bool testContiguousAppendMerge() +{ + codexui::ConversationContentUpdates updates; + std::uint64_t retainedBytes = 0; + bool passed = detail::mergeConversationContentUpdate( + updates, retainedBytes, identity(0, QByteArray("abc"), 10)) + == detail::BoundedMergeResult::Retained; + passed &= detail::mergeConversationContentUpdate( + updates, retainedBytes, identity(0, QByteArray("def"), 13)) + == detail::BoundedMergeResult::Retained; + passed &= expect(updates.size() == 1 && updates.front().append + && updates.front().append->baseContentBytes == 10 + && updates.front().append->deltaUtf8Bytes == 6 + && updates.front().append->delta == QStringLiteral("abcdef") + && retainedBytes == 6, + "contiguous updates for one channel must merge with exact byte accounting"); + return passed; +} + +bool testSidebarIdentityBound() +{ + QStringList ordered; + QSet retained; + bool passed = true; + for (qsizetype index = 0; + index < detail::maximumCoalescedPresentationIdentities; + ++index) + { + const QString id = QStringLiteral("thread-%1").arg(index); + passed &= detail::appendUniqueSidebarThread(ordered, retained, id) + == detail::BoundedMergeResult::Retained; + passed &= detail::appendUniqueSidebarThread(ordered, retained, id) + == detail::BoundedMergeResult::Retained; + } + passed &= expect( + ordered.size() == detail::maximumCoalescedPresentationIdentities + && retained.size() == detail::maximumCoalescedPresentationIdentities + && detail::appendUniqueSidebarThread( + ordered, retained, QStringLiteral("thread-overflow")) + == detail::BoundedMergeResult::CapacityExceeded + && ordered.size() == detail::maximumCoalescedPresentationIdentities, + "sidebar duplicates must not consume capacity and the 1025th unique identity must be rejected"); + return passed; +} + +} // namespace + +int main(int argc, char** argv) +{ + QCoreApplication application(argc, argv); + bool passed = true; + passed &= testIdentityBound(); + passed &= testAggregateByteBound(); + passed &= testReplacementReleasesRetainedBytes(); + passed &= testContiguousAppendMerge(); + passed &= testSidebarIdentityBound(); + return passed ? 0 : 1; +}