From 0ff385f2065b77f60815008008b8df404c5815fa Mon Sep 17 00:00:00 2001 From: Kazuki Nakashima <65545348+lynnswap@users.noreply.github.com> Date: Sun, 9 Aug 2026 22:58:14 +0900 Subject: [PATCH] fix(review-ui): distinguish sidebar threads --- Package.resolved | 4 +- Package.swift | 2 +- .../Store/CodexReviewStoreOrderQueries.swift | 10 + .../CodexChats/ReviewMonitorChatRowView.swift | 249 +++++++++++++++--- .../ReviewMonitorSidebarViewController.swift | 31 ++- ...ReviewMonitorPreviewAppServerRuntime.swift | 4 +- .../ReviewMonitorPreviewContent.swift | 76 +++++- ...eviewMonitorCodexSidebarResultsTests.swift | 93 +++++-- Tests/ReviewUITests/ReviewUIShellTests.swift | 39 ++- Tests/ReviewUITests/ReviewUITests.swift | 182 +++++++++++-- .../xcshareddata/swiftpm/Package.resolved | 4 +- 11 files changed, 574 insertions(+), 120 deletions(-) diff --git a/Package.resolved b/Package.resolved index 6845d35..2cca363 100644 --- a/Package.resolved +++ b/Package.resolved @@ -1,12 +1,12 @@ { - "originHash" : "db686e8128086fbeb71f9dc90e21b592d1c55f433180924062def874be45aa52", + "originHash" : "70ce22664981eba030baa03c4d4ef13bee21abf7e2f99ad31df38675cbb241d2", "pins" : [ { "identity" : "codexkit", "kind" : "remoteSourceControl", "location" : "https://github.com/lynnswap/CodexKit.git", "state" : { - "revision" : "99ef48d1306435c0bb801b1b1c233f31685421c6" + "revision" : "ab025ed970d30c7679913951bdb9fff20a9b77b1" } }, { diff --git a/Package.swift b/Package.swift index f5fe9d8..00dca8f 100644 --- a/Package.swift +++ b/Package.swift @@ -8,7 +8,7 @@ let packageDirectory = URL(fileURLWithPath: #filePath) let localCodexKitPath = packageDirectory .appendingPathComponent("dependencies/CodexKit", isDirectory: true) .path -let codexKitFallbackRevision = "99ef48d1306435c0bb801b1b1c233f31685421c6" +let codexKitFallbackRevision = "ab025ed970d30c7679913951bdb9fff20a9b77b1" let codexKitDependency: Package.Dependency = FileManager.default.fileExists(atPath: "\(localCodexKitPath)/Package.swift") ? .package(path: localCodexKitPath) diff --git a/Sources/CodexReviewKit/Store/CodexReviewStoreOrderQueries.swift b/Sources/CodexReviewKit/Store/CodexReviewStoreOrderQueries.swift index 82f82fa..a22a93b 100644 --- a/Sources/CodexReviewKit/Store/CodexReviewStoreOrderQueries.swift +++ b/Sources/CodexReviewKit/Store/CodexReviewStoreOrderQueries.swift @@ -80,6 +80,12 @@ extension CodexReviewStore { } } + package func reviewRun(forReviewChatID chatID: String) -> ReviewRunRecord? { + orderedReviewRuns.first { runRecord in + runRecord.matchesReviewChatID(chatID) + } + } + package func cancellableReviewRun(forChatID chatID: String) -> ReviewRunRecord? { orderedReviewRuns.first { runRecord in guard isCancellableReviewRun(runRecord) else { @@ -92,6 +98,10 @@ extension CodexReviewStore { } private extension ReviewRunRecord { + func matchesReviewChatID(_ chatID: String) -> Bool { + core.attempt?.threadIdentity.activeTurnThreadID.rawValue == chatID + } + func matchesChatID(_ chatID: String) -> Bool { guard let identity = core.attempt?.threadIdentity else { return false diff --git a/Sources/ReviewUI/Sidebar/CodexChats/ReviewMonitorChatRowView.swift b/Sources/ReviewUI/Sidebar/CodexChats/ReviewMonitorChatRowView.swift index b294327..90462e6 100644 --- a/Sources/ReviewUI/Sidebar/CodexChats/ReviewMonitorChatRowView.swift +++ b/Sources/ReviewUI/Sidebar/CodexChats/ReviewMonitorChatRowView.swift @@ -1,38 +1,180 @@ import Foundation import SwiftUI +import CodexAppServerKit import CodexDataKit +import CodexReviewKit + +@MainActor +struct ReviewMonitorChatRowPresentation: Equatable { + enum Timing: Equatable { + case elapsed(since: Date) + case relative(to: Date) + } + + enum Symbol: Equatable { + case progress + case succeeded + case failed + case cancelled + case none + } + + let title: String + let statusText: String + let timing: Timing? + let symbol: Symbol + + init(chat: CodexChat, reviewRun: ReviewRunRecord?) { + title = reviewRun?.targetSummary.trimmedNonEmpty + ?? Self.gitLabel(chat.gitInfo) + ?? chat.title + + guard let reviewRun else { + if chat.status?.isActive == true { + statusText = Self.isReviewSource(chat) ? "Reviewing" : "Running" + timing = chat.activityDate.map(Timing.elapsed) + symbol = .progress + } else { + statusText = Self.sourceLabel(chat) + timing = chat.activityDate.map(Timing.relative) + symbol = .none + } + return + } + + let presentation = reviewRun.presentation + switch presentation.lifecycle { + case .queued: + statusText = "Queued" + symbol = .none + case .starting: + statusText = "Starting" + symbol = .progress + case .running: + statusText = "Reviewing" + symbol = .progress + case .waitingForNetwork: + statusText = "Waiting for network" + symbol = .progress + case .preparingRestart, .restarting: + statusText = "Restarting" + symbol = .progress + case .cancelling: + statusText = "Cancelling" + symbol = .progress + case .succeeded: + statusText = "Review complete" + symbol = .succeeded + case .failed: + statusText = "Review failed" + symbol = .failed + case .cancelled: + statusText = "Cancelled" + symbol = .cancelled + } + + if let endedAt = reviewRun.core.endedAt { + timing = .relative(to: endedAt) + } else if let startedAt = reviewRun.core.startedAt { + timing = .elapsed(since: startedAt) + } else { + timing = chat.activityDate.map(Timing.relative) + } + } + + private static func gitLabel(_ gitInfo: CodexThreadGitInfo?) -> String? { + let branch = gitInfo?.branch?.trimmedNonEmpty + let sha = gitInfo?.sha?.trimmedNonEmpty.map { String($0.prefix(8)) } + switch (branch, sha) { + case (.some(let branch), .some(let sha)): + return "\(branch) · \(sha)" + case (.some(let branch), nil): + return branch + case (nil, .some(let sha)): + return sha + case (nil, nil): + return nil + } + } + + private static func isReviewSource(_ chat: CodexChat) -> Bool { + if let source = chat.source, case .subAgent(.review) = source { + return true + } + return chat.sourceKind == .subAgentReview + } + + private static func sourceLabel(_ chat: CodexChat) -> String { + if let source = chat.source { + switch source { + case .cli: + return "CLI" + case .vscode: + return "VS Code" + case .exec: + return "Exec" + case .appServer: + return "Codex" + case .custom(let value): + guard let value = value.trimmedNonEmpty else { + return "Custom" + } + switch value.lowercased() { + case "atlas": + return "Atlas" + case "chatgpt": + return "ChatGPT" + default: + return value + } + case .subAgent(.review): + return "Review" + case .subAgent(.compact): + return "Compact" + case .subAgent(.threadSpawn), .subAgent(.other): + return "Sub-agent" + case .subAgent(.memoryConsolidation): + return "Memory" + case .unknown: + return "Thread" + } + } + + let sourceKind = chat.sourceKind + if sourceKind == .cli { return "CLI" } + if sourceKind == .vscode { return "VS Code" } + if sourceKind == .exec { return "Exec" } + if sourceKind == .appServer { return "Codex" } + if sourceKind == .subAgentReview { return "Review" } + if sourceKind == .subAgentCompact { return "Compact" } + if sourceKind == .subAgent || sourceKind == .subAgentThreadSpawn + || sourceKind == .subAgentOther + { + return "Sub-agent" + } + return "Thread" + } +} @MainActor struct ReviewMonitorChatRowView: View { var chat: CodexChat + var store: CodexReviewStore var body: some View { - let isRunning = chat.status?.isActive == true - let startedAt = isRunning ? chat.activityDate : nil + let reviewRun = store.reviewRun(forReviewChatID: chat.id.rawValue) + let presentation = ReviewMonitorChatRowPresentation(chat: chat, reviewRun: reviewRun) Label { - VStack { - HStack { - Text(chat.title) - .truncationMode(.tail) - Spacer(minLength: 0) - if let startedAt { - Text( - timerInterval: startedAt...(.distantFuture), - pauseTime: nil, - countsDown: false, - showsHours: true - ) - .monospacedDigit() - .foregroundStyle(.secondary) - .layoutPriority(1) - } - } - .lineLimit(1) - HStack { - Text(chat.modelProvider?.trimmedNonEmpty ?? "") - Text(chat.preview?.trimmedNonEmpty ?? "") - Spacer(minLength: 0) + VStack(alignment: .leading) { + Text(presentation.title) + .truncationMode(.middle) + .frame(maxWidth: .infinity, alignment: .leading) + .lineLimit(1) + HStack(spacing: 4) { + Text(presentation.statusText) + Spacer(minLength: 4) + timingText(presentation.timing) } .textScale(.secondary) .foregroundStyle(.secondary) @@ -42,17 +184,54 @@ struct ReviewMonitorChatRowView: View { ZStack { Image(systemName: "circle.fill") .foregroundStyle(.clear) - if isRunning { + switch presentation.symbol { + case .progress: ProgressView() .controlSize(.mini) + .accessibilityHidden(true) + case .succeeded: + Image(systemName: "checkmark.circle.fill") + .foregroundStyle(.green) + .accessibilityHidden(true) + case .failed: + Image(systemName: "exclamationmark.circle.fill") + .foregroundStyle(.red) + .accessibilityHidden(true) + case .cancelled: + Image(systemName: "xmark.circle") + .foregroundStyle(.secondary) + .accessibilityHidden(true) + case .none: + EmptyView() } } - .animation(.default, value: isRunning) + .animation(.default, value: presentation.symbol) .padding(.leading, SidebarLayout.disclosureGutterWidth) } .transaction(value: chat.id.rawValue) { transaction in transaction.disablesAnimations = true } + .help(presentation.title) + } + + @ViewBuilder + private func timingText(_ timing: ReviewMonitorChatRowPresentation.Timing?) -> some View { + switch timing { + case .elapsed(let startedAt): + Text( + timerInterval: startedAt...(.distantFuture), + pauseTime: nil, + countsDown: false, + showsHours: true + ) + .monospacedDigit() + .layoutPriority(1) + case .relative(let date): + Text(date, style: .relative) + .layoutPriority(1) + case nil: + EmptyView() + } } } @@ -80,10 +259,13 @@ extension ReviewMonitorChatRowView { let hostingView = NSHostingView( rootView: Label { VStack { - HStack { - Text("Uncommitted changes") - .truncationMode(.tail) - Spacer(minLength: 0) + Text("Uncommitted changes") + .truncationMode(.middle) + .frame(maxWidth: .infinity, alignment: .leading) + .lineLimit(1) + HStack(spacing: 4) { + Text("Waiting for network") + Spacer(minLength: 4) Text( timerInterval: Date(timeIntervalSince1970: 0)...(.distantFuture), pauseTime: nil, @@ -91,15 +273,8 @@ extension ReviewMonitorChatRowView { showsHours: true ) .monospacedDigit() - .foregroundStyle(.secondary) .layoutPriority(1) } - .lineLimit(1) - HStack { - Text("gpt-5.5") - Text("Review output preview") - Spacer(minLength: 0) - } .textScale(.secondary) .foregroundStyle(.secondary) .lineLimit(1) diff --git a/Sources/ReviewUI/Sidebar/CodexChats/ReviewMonitorSidebarViewController.swift b/Sources/ReviewUI/Sidebar/CodexChats/ReviewMonitorSidebarViewController.swift index ceee4e7..34d8ac9 100644 --- a/Sources/ReviewUI/Sidebar/CodexChats/ReviewMonitorSidebarViewController.swift +++ b/Sources/ReviewUI/Sidebar/CodexChats/ReviewMonitorSidebarViewController.swift @@ -279,7 +279,8 @@ final class ReviewMonitorSidebarViewController: NSViewController, NSOutlineViewD static var defaultCodexSidebarDescriptor: CodexFetchDescriptor { CodexFetchDescriptor( predicate: #Predicate { $0.isArchived == false }, - sortBy: [CodexSortDescriptor(\.recencyAt, order: .reverse)] + sortBy: [CodexSortDescriptor(\.recencyAt, order: .reverse)], + fetchLimit: 25 ) } @@ -1330,7 +1331,7 @@ final class ReviewMonitorSidebarViewController: NSViewController, NSOutlineViewD else { return false } - cellView.configure(with: chat) + cellView.configure(with: chat, store: store) return true case .section: guard let cellView = cellView as? ReviewMonitorWorkspaceGroupCellView else { @@ -2218,6 +2219,7 @@ private final class ReviewMonitorReviewChatTableRowView: NSTableRowView { private final class ReviewMonitorReviewChatCellView: NSTableCellView { private var hostingView: NSHostingView? private weak var boundChat: CodexChat? + private weak var boundStore: CodexReviewStore? override init(frame frameRect: NSRect) { super.init(frame: frameRect) @@ -2229,30 +2231,33 @@ private final class ReviewMonitorReviewChatCellView: NSTableCellView { nil } - func configure(with chat: CodexChat) { - guard boundChat !== chat else { + func configure(with chat: CodexChat, store: CodexReviewStore) { + guard boundChat !== chat || boundStore !== store else { return } objectValue = chat boundChat = chat - render(chat) + boundStore = store + render(chat, store: store) } private func configureHierarchy() { translatesAutoresizingMaskIntoConstraints = false } - private func render(_ chat: CodexChat) { - toolTip = chat.workspace?.url.path ?? chat.preview ?? chat.title + private func render(_ chat: CodexChat, store: CodexReviewStore) { if let hostingView { if hostingView.rootView.chat !== chat { hostingView.rootView.chat = chat } + if hostingView.rootView.store !== store { + hostingView.rootView.store = store + } return } let hostingView = NSHostingView( - rootView: ReviewMonitorChatRowView(chat: chat) + rootView: ReviewMonitorChatRowView(chat: chat, store: store) ) hostingView.sizingOptions = [] hostingView.translatesAutoresizingMaskIntoConstraints = false @@ -2281,22 +2286,24 @@ private final class ReviewMonitorReviewChatCellView: NSTableCellView { #if DEBUG @MainActor func makeReviewMonitorReviewChatCellViewForTesting( - chat: CodexChat + chat: CodexChat, + store: CodexReviewStore ) -> NSTableCellView { let cellView = ReviewMonitorReviewChatCellView() - cellView.configure(with: chat) + cellView.configure(with: chat, store: store) return cellView } @MainActor func configureReviewMonitorReviewChatCellViewForTesting( _ cellView: NSTableCellView, - chat: CodexChat + chat: CodexChat, + store: CodexReviewStore ) { guard let cellView = cellView as? ReviewMonitorReviewChatCellView else { fatalError("Expected ReviewMonitorReviewChatCellView.") } - cellView.configure(with: chat) + cellView.configure(with: chat, store: store) } #endif diff --git a/Sources/ReviewUIPreviewSupport/ReviewMonitorPreviewAppServerRuntime.swift b/Sources/ReviewUIPreviewSupport/ReviewMonitorPreviewAppServerRuntime.swift index 9e1a68f..ac9c12e 100644 --- a/Sources/ReviewUIPreviewSupport/ReviewMonitorPreviewAppServerRuntime.swift +++ b/Sources/ReviewUIPreviewSupport/ReviewMonitorPreviewAppServerRuntime.swift @@ -675,7 +675,7 @@ private func makePreviewStoredThread( name: title, preview: preview, modelProvider: modelProvider, - sourceKind: .appServer, + sourceKind: .subAgentReview, createdAt: createdAt.previewWholeSecondDate, updatedAt: updatedAt.previewWholeSecondDate, recencyAt: recencyAt?.previewWholeSecondDate, @@ -687,7 +687,7 @@ private func makePreviewStoredThread( metadata: .init( sessionID: "preview-session-\(chatID.rawValue)", cliVersion: "codex-preview", - source: .appServer + source: .subAgentReview ), runtimeMetadata: .init( model: model, diff --git a/Sources/ReviewUIPreviewSupport/ReviewMonitorPreviewContent.swift b/Sources/ReviewUIPreviewSupport/ReviewMonitorPreviewContent.swift index efa0e84..6182bab 100644 --- a/Sources/ReviewUIPreviewSupport/ReviewMonitorPreviewContent.swift +++ b/Sources/ReviewUIPreviewSupport/ReviewMonitorPreviewContent.swift @@ -150,6 +150,11 @@ public enum ReviewMonitorPreviewContent { let chatItems: [PreviewChatLogItemTemplate] } + private struct PreviewContentFixture { + let chatLog: ReviewMonitorPreviewChatLogFixture + let terminalReviewRun: ReviewRunRecord? + } + private enum PreviewReasoningStyle { case raw case summary @@ -242,7 +247,8 @@ public enum ReviewMonitorPreviewContent { } private static func makeStore( - runtimeLifetime: PreviewRuntimeLifetime? + runtimeLifetime: PreviewRuntimeLifetime?, + reviewRuns: [ReviewRunRecord] = [] ) -> CodexReviewStore { let store = CodexReviewStore.makePreviewStore( seed: .init(initialSettingsSnapshot: makePreviewSettingsSnapshot()), @@ -253,18 +259,22 @@ public enum ReviewMonitorPreviewContent { serverState: .running, account: accounts.first, persistedAccounts: accounts, - serverURL: URL(string: "http://localhost:9417/mcp") + serverURL: URL(string: "http://localhost:9417/mcp"), + reviewRuns: reviewRuns ) return store } public static func makeContentSource() -> ReviewMonitorPreviewContentSource { - let chatLogFixtures = makeChatLogFixtures() + let fixtures = makeContentFixtures() let lifetime = PreviewRuntimeLifetime( - fixtures: chatLogFixtures + fixtures: fixtures.map(\.chatLog) ) return ReviewMonitorPreviewContentSource( - store: makeStore(runtimeLifetime: lifetime), + store: makeStore( + runtimeLifetime: lifetime, + reviewRuns: fixtures.compactMap(\.terminalReviewRun) + ), lifetime: lifetime ) } @@ -722,7 +732,7 @@ public enum ReviewMonitorPreviewContent { ] } - private static func makeChatLogFixtures() -> [ReviewMonitorPreviewChatLogFixture] { + private static func makeContentFixtures() -> [PreviewContentFixture] { let now = Date() let workspacePaths = [ "/path/to/workspace-alpha", @@ -730,7 +740,7 @@ public enum ReviewMonitorPreviewContent { "/path/to/workspace-gamma", ] - var chatLogFixtures: [ReviewMonitorPreviewChatLogFixture] = [] + var fixtures: [PreviewContentFixture] = [] for (workspaceIndex, cwd) in workspacePaths.enumerated() { let workspaceName = URL(fileURLWithPath: cwd).lastPathComponent for (chatIndex, definition) in makeChatDefinitions(for: workspaceName).enumerated() { @@ -755,14 +765,58 @@ public enum ReviewMonitorPreviewContent { endedAt: definition.endedOffset.map { now.addingTimeInterval($0) }, chatItems: chatItems ) - chatLogFixtures.append( - makeChatLogFixture( + fixtures.append(PreviewContentFixture( + chatLog: makeChatLogFixture( for: chatFixture, referenceDate: now - )) + ), + terminalReviewRun: makeTerminalReviewRun(for: chatFixture) + )) } } - return chatLogFixtures + return fixtures + } + + private static func makeTerminalReviewRun( + for fixture: PreviewChatFixture + ) -> ReviewRunRecord? { + let status: ReviewRunState + let failure: ReviewBackendFailure? + let cancellation: ReviewCancellation? + switch fixture.lifecycle { + case .queued, .running: + return nil + case .succeeded: + status = .succeeded + failure = nil + cancellation = nil + case .failed: + status = .failed + failure = .turnFailed(.init(message: fixture.summary, code: .other)) + cancellation = nil + case .cancelled: + status = .cancelled + failure = nil + cancellation = .userInterface(message: fixture.summary) + } + + return ReviewRunRecord.makeForTesting( + id: "\(fixture.id)-run", + sessionID: "preview-session-\(fixture.id)", + cwd: fixture.cwd, + targetSummary: fixture.targetSummary, + model: fixture.model, + attemptID: "\(fixture.id)-attempt", + threadID: fixture.chatID.rawValue, + reviewThreadID: fixture.chatID.rawValue, + turnID: fixture.turnID.rawValue, + status: status, + cancellation: cancellation, + startedAt: fixture.startedAt, + endedAt: fixture.endedAt, + summary: fixture.summary, + failure: failure + ) } private static func makeChatLogFixture( diff --git a/Tests/ReviewUITests/ReviewMonitorCodexSidebarResultsTests.swift b/Tests/ReviewUITests/ReviewMonitorCodexSidebarResultsTests.swift index ed79719..44a2230 100644 --- a/Tests/ReviewUITests/ReviewMonitorCodexSidebarResultsTests.swift +++ b/Tests/ReviewUITests/ReviewMonitorCodexSidebarResultsTests.swift @@ -17,7 +17,7 @@ struct ReviewMonitorCodexSidebarResultsTests { let app = try makeDirectory("App", in: repo) let tools = try makeDirectory("Tools", in: repo) - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( @@ -86,7 +86,7 @@ struct ReviewMonitorCodexSidebarResultsTests { let runningThreadID = CodexThreadID(rawValue: "thread-running") let idleThreadID = CodexThreadID(rawValue: "thread-idle") - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( @@ -136,7 +136,7 @@ struct ReviewMonitorCodexSidebarResultsTests { let context = CodexModelContainer(appServer: runtime.server).mainContext let repo = try makeGitRepository() - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( @@ -156,22 +156,40 @@ struct ReviewMonitorCodexSidebarResultsTests { #expect(results.sections.filtered(by: .running).isEmpty) } - @Test func defaultCodexSidebarDescriptorUsesDedicatedHomeWithoutSourceFiltering() async throws { + @Test func defaultCodexSidebarDescriptorUsesDedicatedHomeAndUserVisibleSources() async throws { let runtime = try await CodexAppServerTestRuntime.start() let context = CodexModelContainer(appServer: runtime.server).mainContext - try await runtime.transport.enqueueThreadList(.init(threads: [])) + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite(.init(threads: [])) let results = makeCodexSidebarFetchedResults(context: context) try await results.performFetch() - let request = try #require(await runtime.transport.recordedRequests(for: .threadList).first) - guard case .threadList(let query) = request.request else { - Issue.record("Expected a thread-list request.") + let requests = await runtime.transport.recordedRequests(for: .threadList) + #expect(requests.count == 2) + let interactiveRequest = try #require(requests.first) + let noninteractiveRequest = try #require(requests.dropFirst().first) + guard case .threadList(let interactiveQuery) = interactiveRequest.request, + case .threadList(let noninteractiveQuery) = noninteractiveRequest.request + else { + Issue.record("Expected two thread-list requests.") return } - #expect(query.archived == false) - #expect(query.sourceKinds == nil) + #expect(interactiveQuery.archived == false) + #expect(interactiveQuery.sourceKinds == nil) + #expect(interactiveQuery.limit == 25) + #expect(noninteractiveQuery.archived == false) + #expect( + noninteractiveQuery.sourceKinds == [ + .exec, + .appServer, + .subAgentReview, + .subAgentCompact, + .subAgentThreadSpawn, + .subAgentOther, + .unknown, + ]) + #expect(noninteractiveQuery.limit == 25) } @Test func sidebarIncludesCanonicalWorkspaceChatsWithStableRowIDs() async throws { @@ -179,7 +197,7 @@ struct ReviewMonitorCodexSidebarResultsTests { let context = CodexModelContainer(appServer: runtime.server).mainContext let repo = try makeGitRepository() - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( @@ -230,7 +248,7 @@ struct ReviewMonitorCodexSidebarResultsTests { let repo = try makeGitRepository() let threadID = CodexThreadID(rawValue: "thread-app") - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( @@ -276,7 +294,7 @@ struct ReviewMonitorCodexSidebarResultsTests { let runningThreadID = CodexThreadID(rawValue: "thread-running") let idleThreadID = CodexThreadID(rawValue: "thread-idle") - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( @@ -311,7 +329,7 @@ struct ReviewMonitorCodexSidebarResultsTests { let repo = try makeGitRepository() let threadID = CodexThreadID(rawValue: "thread-app") - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( @@ -364,7 +382,7 @@ struct ReviewMonitorCodexSidebarResultsTests { let visibleThreadID = CodexThreadID(rawValue: "thread-app") let hiddenRunThreadID = CodexThreadID(rawValue: "run-backed-review-thread") - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( @@ -422,7 +440,7 @@ struct ReviewMonitorCodexSidebarResultsTests { let repo = try makeGitRepository() let threadID = CodexThreadID(rawValue: "thread-app") - try await runtime.transport.enqueueThreadList(.init(threads: [])) + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite(.init(threads: [])) let store = CodexReviewStore.makePreviewStore() store.loadForTesting(serverState: .running) @@ -438,7 +456,7 @@ struct ReviewMonitorCodexSidebarResultsTests { sidebar.isShowingEmptyStateForTesting } - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( @@ -463,7 +481,7 @@ struct ReviewMonitorCodexSidebarResultsTests { let selectedThreadID = CodexThreadID(rawValue: "thread-selected") let remainingThreadID = CodexThreadID(rawValue: "thread-remaining") - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( @@ -514,7 +532,7 @@ struct ReviewMonitorCodexSidebarResultsTests { && transport.renderedStateForTesting.snapshot.isShowingEmptyState == false } - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( @@ -545,7 +563,7 @@ struct ReviewMonitorCodexSidebarResultsTests { let context = CodexModelContainer(appServer: runtime.server).mainContext let repo = try makeGitRepository() - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( @@ -581,7 +599,7 @@ struct ReviewMonitorCodexSidebarResultsTests { let repo = try makeGitRepository() let threadID = CodexThreadID(rawValue: "thread-app") - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( @@ -663,7 +681,7 @@ struct ReviewMonitorCodexSidebarResultsTests { let firstRepo = try makeGitRepository() let secondRepo = try makeGitRepository() - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( @@ -729,7 +747,7 @@ struct ReviewMonitorCodexSidebarResultsTests { let firstRepo = try makeGitRepository() let secondRepo = try makeGitRepository() - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( @@ -823,7 +841,7 @@ struct ReviewMonitorCodexSidebarResultsTests { let firstRepo = try makeGitRepository() let secondRepo = try makeGitRepository() - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( @@ -902,7 +920,7 @@ struct ReviewMonitorCodexSidebarResultsTests { let firstThreadID = CodexThreadID(rawValue: "thread-first") let secondThreadID = CodexThreadID(rawValue: "thread-second") - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( @@ -954,7 +972,7 @@ struct ReviewMonitorCodexSidebarResultsTests { let firstThreadID = CodexThreadID(rawValue: "thread-first") let secondThreadID = CodexThreadID(rawValue: "thread-second") - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( @@ -1007,7 +1025,7 @@ struct ReviewMonitorCodexSidebarResultsTests { let activeThreadID = CodexThreadID(rawValue: "thread-active") let previousThreadID = CodexThreadID(rawValue: "thread-previous") - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( @@ -1072,7 +1090,7 @@ struct ReviewMonitorCodexSidebarResultsTests { let firstRecencyAt = Date(timeIntervalSince1970: 5_000) let secondRecencyAt = Date(timeIntervalSince1970: 4_000) - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( @@ -1175,6 +1193,25 @@ private func makeGitRepository() throws -> URL { return repo } +extension CodexAppServerTestTransport { + func enqueueDefaultUserVisibleThreadListComposite( + _ noninteractivePage: CodexAppServerTestThreadPage + ) throws { + // Sidebar fixtures in this file are app-server threads. The default interactive + // partition must terminate before CodexDataKit requests the noninteractive sources. + try enqueueThreadList(.init(threads: [])) + try enqueueThreadList(noninteractivePage) + } + + func enqueueDefaultUserVisibleThreadListComposite( + interactivePage: CodexAppServerTestThreadPage, + noninteractivePage: CodexAppServerTestThreadPage = .init(threads: []) + ) throws { + try enqueueThreadList(interactivePage) + try enqueueThreadList(noninteractivePage) + } +} + private extension CodexAppServerTestStoredThread { init( id: CodexThreadID, diff --git a/Tests/ReviewUITests/ReviewUIShellTests.swift b/Tests/ReviewUITests/ReviewUIShellTests.swift index f635fa4..ec89f3a 100644 --- a/Tests/ReviewUITests/ReviewUIShellTests.swift +++ b/Tests/ReviewUITests/ReviewUIShellTests.swift @@ -95,6 +95,35 @@ extension ReviewUITests { #expect(sidebar.isShowingEmptyStateForTesting == false) #expect(sidebar.sidebarKindForTesting == .chatList) + let activeReviewChat = try #require( + sidebar.codexSidebarSectionsForTesting + .flatMap(\.items) + .first { $0.id == "preview-thread-0-0" } + ) + let presentation = ReviewMonitorChatRowPresentation( + chat: activeReviewChat, + reviewRun: nil + ) + #expect(activeReviewChat.source == .subAgent(.review)) + #expect(presentation.statusText == "Reviewing") + #expect(presentation.symbol == .progress) + } + + @Test func previewContentSeedsOnlyTerminalReviewRunPresentations() throws { + let store = ReviewMonitorPreviewContent.makeContentSource().store + + #expect(store.orderedReviewRuns.count == 12) + #expect(store.reviewRun(forReviewChatID: "preview-thread-0-3")?.presentation.status == .succeeded) + #expect(store.reviewRun(forReviewChatID: "preview-thread-0-4")?.presentation.status == .failed) + #expect(store.reviewRun(forReviewChatID: "preview-thread-0-5")?.presentation.status == .cancelled) + #expect(store.reviewRun(forReviewChatID: "preview-thread-0-6")?.presentation.status == .succeeded) + #expect(store.reviewRun(forReviewChatID: "preview-thread-0-0") == nil) + #expect(store.reviewRun(forReviewChatID: "preview-thread-0-1") == nil) + #expect(store.reviewRun(forReviewChatID: "preview-thread-0-2") == nil) + #expect(store.chatCancellationCapability( + forChatID: "preview-thread-0-0", + isChatActive: true + ).action == .directChat) } @Test func previewChatContextMenuCancelInterruptsActiveFakeAppServerChat() async throws { @@ -228,7 +257,7 @@ extension ReviewUITests { let repo = try makeShellTestGitRepository() let chatID = CodexThreadID(rawValue: "inactive-chat-to-archive") - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( @@ -288,7 +317,7 @@ extension ReviewUITests { let repo = try makeShellTestGitRepository() let chatID = CodexThreadID(rawValue: "active-chat-archive-rejected") - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( @@ -346,7 +375,7 @@ extension ReviewUITests { let repo = try makeShellTestGitRepository() let chatID = CodexThreadID(rawValue: "active-chat-archive-approved") - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( @@ -480,7 +509,7 @@ extension ReviewUITests { let chatID = CodexThreadID(rawValue: "active-chat-with-terminal-run") let turnID = CodexTurnID(rawValue: "active-turn") - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( @@ -571,7 +600,7 @@ extension ReviewUITests { let chatID = CodexThreadID(rawValue: "active-chat-with-pending-run-cancel") let turnID = CodexTurnID(rawValue: "pending-turn") - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init( threads: [ .init( diff --git a/Tests/ReviewUITests/ReviewUITests.swift b/Tests/ReviewUITests/ReviewUITests.swift index a26a64d..7b53963 100644 --- a/Tests/ReviewUITests/ReviewUITests.swift +++ b/Tests/ReviewUITests/ReviewUITests.swift @@ -30,6 +30,8 @@ private extension CodexAppServerTestStoredThread { workspace: URL? = nil, name: String? = nil, preview: String? = nil, + source: CodexAppServerTestSessionSource = .appServer, + gitInfo: CodexThreadGitInfo? = nil, updatedAt: Date = Date(timeIntervalSince1970: 0), recencyAt: Date? = nil, status: CodexThreadStatus = .idle @@ -45,7 +47,8 @@ private extension CodexAppServerTestStoredThread { name: name, preview: preview ?? name ?? id.rawValue, modelProvider: "openai", - sourceKind: .appServer, + sourceKind: source.sourceKind, + gitInfo: gitInfo, createdAt: updatedAt, updatedAt: updatedAt, recencyAt: recencyAt ?? updatedAt, @@ -57,7 +60,8 @@ private extension CodexAppServerTestStoredThread { metadata: .init( sessionID: "session-\(id.rawValue)", cliVersion: "codex-cli-test", - source: .appServer + source: source, + gitInfo: gitInfo ), runtimeMetadata: .init( model: "gpt-5", @@ -514,6 +518,7 @@ struct ReviewUITests { } @Test func reviewChatCellViewUpdatesNativeOwnerStateWhenConfiguredWithNewChat() async throws { + let store = CodexReviewStore.makePreviewStore() let placeholderChat = try await reviewChatCellTestChat( id: "chat-placeholder", title: "Queued review", @@ -525,17 +530,134 @@ struct ReviewUITests { workspaceCWD: "/tmp/loaded" ) - let cellView = makeReviewMonitorReviewChatCellViewForTesting(chat: placeholderChat) + let cellView = makeReviewMonitorReviewChatCellViewForTesting(chat: placeholderChat, store: store) let initialObjectChat = try #require(cellView.objectValue as? CodexChat) #expect(initialObjectChat.id == placeholderChat.id) - #expect(cellView.toolTip == (placeholderChat.workspace?.url.path ?? placeholderChat.preview ?? placeholderChat.title)) - configureReviewMonitorReviewChatCellViewForTesting(cellView, chat: loadedChat) + configureReviewMonitorReviewChatCellViewForTesting(cellView, chat: loadedChat, store: store) let objectChat = try #require(cellView.objectValue as? CodexChat) #expect(objectChat.id == loadedChat.id) - #expect(cellView.toolTip == (loadedChat.workspace?.url.path ?? loadedChat.preview ?? loadedChat.title)) + } + + @Test func reviewChatRowPresentationUsesReviewRunTargetAndResult() async throws { + let chat = try await reviewChatCellTestChat( + id: "reviewer-chat", + title: "Review the code changes against the base branch", + workspaceCWD: "/tmp/reviewer-chat" + ) + let startedAt = Date(timeIntervalSince1970: 100) + let endedAt = Date(timeIntervalSince1970: 200) + let reviewRun = ReviewRunRecord.makeForTesting( + id: "review-run", + targetSummary: "Base branch: main", + attemptID: "review-attempt", + threadID: "source-chat", + reviewThreadID: chat.id.rawValue, + turnID: "review-turn", + status: .succeeded, + startedAt: startedAt, + endedAt: endedAt, + summary: "Review completed." + ) + let store = CodexReviewStore.makePreviewStore() + store.loadReviewCancellationStateForTesting( + serverState: .running, + reviewRuns: [reviewRun] + ) + + let presentation = ReviewMonitorChatRowPresentation( + chat: chat, + reviewRun: store.reviewRun(forReviewChatID: chat.id.rawValue) + ) + + #expect(presentation.title == "Base branch: main") + #expect(presentation.statusText == "Review complete") + #expect(presentation.timing == .relative(to: endedAt)) + #expect(presentation.symbol == .succeeded) + } + + @Test func reviewChatRowPresentationDoesNotApplyReviewLifecycleToSourceChat() async throws { + let sourceChat = try await reviewChatCellTestChat( + id: "source-chat", + title: "Implement sidebar metadata", + workspaceCWD: "/tmp/source-chat" + ) + let reviewRun = ReviewRunRecord.makeForTesting( + id: "review-run", + targetSummary: "Base branch: main", + attemptID: "review-attempt", + threadID: sourceChat.id.rawValue, + reviewThreadID: "review-chat", + turnID: "review-turn", + status: .succeeded, + startedAt: Date(timeIntervalSince1970: 100), + endedAt: Date(timeIntervalSince1970: 200), + summary: "Review completed." + ) + let store = CodexReviewStore.makePreviewStore() + store.loadReviewCancellationStateForTesting( + serverState: .running, + reviewRuns: [reviewRun] + ) + + let presentation = ReviewMonitorChatRowPresentation( + chat: sourceChat, + reviewRun: store.reviewRun(forReviewChatID: sourceChat.id.rawValue) + ) + + #expect(presentation.title == sourceChat.title) + #expect(presentation.statusText == "Codex") + #expect(presentation.symbol == .none) + } + + @Test func reviewChatRowPresentationKeepsPersistedReviewWithoutLiveRun() async throws { + let gitInfo = CodexThreadGitInfo( + sha: "1234567890abcdef", + branch: "feature/sidebar" + ) + let reviewChat = try await reviewChatCellTestChat( + id: "persisted-reviewer-chat", + title: "Persisted review", + workspaceCWD: "/tmp/persisted-reviewer-chat", + source: .subAgentReview, + gitInfo: gitInfo + ) + let vscodeChat = try await reviewChatCellTestChat( + id: "persisted-vscode-chat", + title: "Persisted review", + workspaceCWD: "/tmp/persisted-vscode-chat", + source: .vscode, + gitInfo: gitInfo + ) + + let reviewPresentation = ReviewMonitorChatRowPresentation(chat: reviewChat, reviewRun: nil) + let vscodePresentation = ReviewMonitorChatRowPresentation(chat: vscodeChat, reviewRun: nil) + + #expect(reviewPresentation.title == "feature/sidebar · 12345678") + #expect(vscodePresentation.title == reviewPresentation.title) + #expect(reviewPresentation.statusText == "Review") + #expect(vscodePresentation.statusText == "VS Code") + #expect(reviewPresentation.timing == .relative(to: Date(timeIntervalSince1970: 200))) + #expect(reviewPresentation.symbol == .none) + } + + @Test func reviewChatRowPresentationCanonicalizesKnownCustomSourceLabels() async throws { + let atlasChat = try await reviewChatCellTestChat( + id: "atlas-chat", + title: "Atlas task", + workspaceCWD: "/tmp/atlas-chat", + source: .custom("atlas") + ) + let chatGPTChat = try await reviewChatCellTestChat( + id: "chatgpt-chat", + title: "ChatGPT task", + workspaceCWD: "/tmp/chatgpt-chat", + source: .custom("chatgpt") + ) + #expect(ReviewMonitorChatRowPresentation(chat: atlasChat, reviewRun: nil).statusText == "Atlas") + #expect(ReviewMonitorChatRowPresentation(chat: chatGPTChat, reviewRun: nil).statusText == "ChatGPT") } @Test func accountContextMenuPresentationRestoresResponderStateAfterClosing() throws { @@ -2289,7 +2411,7 @@ struct ReviewUITests { let activeThreadID = CodexThreadID(rawValue: "thread-active") let recentThreadID = CodexThreadID(rawValue: "thread-recent") - try await runtime.transport.enqueueThreadList( + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( .init(threads: [ .init( id: activeThreadID, @@ -5844,7 +5966,9 @@ func makeReviewChatFixtureForTesting( func reviewChatCellTestChat( id: String, title: String, - workspaceCWD: String + workspaceCWD: String, + source: CodexAppServerTestSessionSource = .appServer, + gitInfo: CodexThreadGitInfo? = nil ) async throws -> CodexChat { let chatID = CodexThreadID(rawValue: id) let workspaceURL = URL(fileURLWithPath: workspaceCWD, isDirectory: true) @@ -5855,19 +5979,37 @@ func reviewChatCellTestChat( ) let runtime = try await CodexAppServerTestRuntime.start() let context = CodexModelContainer(appServer: runtime.server).mainContext - try await runtime.transport.enqueueThreadList( - .init( - threads: [ - .init( - id: chatID, - workspace: workspaceURL, - name: title, - updatedAt: Date(timeIntervalSince1970: 200), - status: .idle - ) - ] + let page = CodexAppServerTestThreadPage( + threads: [ + try .init( + id: chatID, + workspace: workspaceURL, + name: title, + source: source, + gitInfo: gitInfo, + updatedAt: Date(timeIntervalSince1970: 200), + status: .idle + ) + ] + ) + if source.filterSourceKind == nil { + try await runtime.transport.enqueueDefaultUserVisibleThreadListComposite( + interactivePage: page + ) + } else { + try await runtime.transport.enqueueThreadList(page) + } + let results: CodexFetchedResults + if let sourceKind = source.filterSourceKind { + let optionalSourceKind: CodexThreadSourceKind? = sourceKind + results = context.fetchedResults(for: CodexFetchDescriptor( + predicate: #Predicate { chat in + chat.isArchived == false && chat.sourceKind == optionalSourceKind + } )) - let results = context.fetchedResults(for: CodexFetchDescriptor()) + } else { + results = context.fetchedResults(for: CodexFetchDescriptor()) + } try await results.performFetch() guard let chat = results.items.first else { throw TestFailure("Expected test CodexChat for \(id).") diff --git a/Tools/ReviewMonitor/CodexReviewMonitor.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/Tools/ReviewMonitor/CodexReviewMonitor.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 1317590..0f4d9a7 100644 --- a/Tools/ReviewMonitor/CodexReviewMonitor.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/Tools/ReviewMonitor/CodexReviewMonitor.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -1,12 +1,12 @@ { - "originHash" : "844e7e3b43d92f89827170165177645f6aa45da42a6028dea22353eb9f9cbdde", + "originHash" : "c6b46fe80620b9641d4c656454fd12dacf29e40336aaa6becfca5411a05280d3", "pins" : [ { "identity" : "codexkit", "kind" : "remoteSourceControl", "location" : "https://github.com/lynnswap/CodexKit.git", "state" : { - "revision" : "99ef48d1306435c0bb801b1b1c233f31685421c6" + "revision" : "ab025ed970d30c7679913951bdb9fff20a9b77b1" } }, {