From 17855d69ae09e6ea2696920bcaf3154e9fa257cf Mon Sep 17 00:00:00 2001 From: Kaede Akino Date: Wed, 26 Aug 2026 17:18:04 +0800 Subject: [PATCH] Refine Assistant chat UI --- Termind/Design/DesignTokens.swift | 17 +-- .../Features/AI/AgentToolPresentation.swift | 20 ++++ .../AI/ConversationTranscriptView.swift | 52 ++++++--- Termind/Features/AI/ConversationViews.swift | 21 +++- Termind/Features/AI/MessageViews.swift | 109 +++++++++++++----- TermindTests/AgentToolPresentationTests.swift | 38 ++++++ TermindTests/ChatMessageTimestampTests.swift | 67 +++++++++++ 7 files changed, 270 insertions(+), 54 deletions(-) create mode 100644 TermindTests/ChatMessageTimestampTests.swift diff --git a/Termind/Design/DesignTokens.swift b/Termind/Design/DesignTokens.swift index 4221dc4a..b0d8d62e 100644 --- a/Termind/Design/DesignTokens.swift +++ b/Termind/Design/DesignTokens.swift @@ -145,6 +145,12 @@ nonisolated enum Metrics { /// be noticeably larger than `Space.xl` — at ordinary padding the gap reads /// as a wrap, not as a change of speaker. enum Chat { + /// The shared reading column in the full Assistant tab. + /// + /// Transcript, decision forms and composer use one width so the page + /// reads as a single conversation rather than three nested columns. + /// Compact Assistant surfaces continue to use all available width. + static let contentMaxWidth: CGFloat = 840 static let turnSpacing: CGFloat = 20 static let blockSpacing: CGFloat = 10 static let bubbleRadius: CGFloat = 14 @@ -176,16 +182,11 @@ nonisolated enum Metrics { /// composer stand at its maximum with the caret marooned at the top and /// a hand's width of nothing under it. The height is measured from the /// draft instead; these two bound the result. - static let composerMinHeight: CGFloat = 40 + static let composerMinHeight: CGFloat = 48 + /// Sidebar composers keep their denser one-line floor. + static let compactComposerMinHeight: CGFloat = 40 static let composerMaxHeight: CGFloat = 132 - /// How tall the agent's question form may grow before it scrolls. - /// - /// Three questions with four described options each runs past 500pt, - /// and this card sits between the transcript and the composer — past - /// this it would push the conversation it is about off the screen. - static let userInputMaxHeight: CGFloat = 320 - /// How much of the steering queue is shown before it scrolls. /// /// A ceiling, not a height — the strip is measured from its rows and diff --git a/Termind/Features/AI/AgentToolPresentation.swift b/Termind/Features/AI/AgentToolPresentation.swift index 2f96dfa1..e5af7ebb 100644 --- a/Termind/Features/AI/AgentToolPresentation.swift +++ b/Termind/Features/AI/AgentToolPresentation.swift @@ -59,6 +59,22 @@ nonisolated enum AgentToolPresentation { return output(raw, policy: presentation?.output ?? .automatic) } + /// The denial reason is intentionally kept both on the review record and + /// in the tool result: people need the former and the model must receive + /// the latter. The transcript, however, should not print identical text + /// twice. Distinct execution failures remain visible below the review. + static func detailOutput( + from result: AgentToolResult, + presentation: AgentToolDescriptor.Presentation?, + review: SecurityReviewDecision? + ) -> String? { + let rendered = output(from: result, presentation: presentation) + guard result.isError, let review, !review.allowsExecution, + normalized(rendered) == normalized(review.reason) + else { return rendered } + return nil + } + private static func output( _ raw: String, policy: AgentToolDescriptor.Presentation.Output ) -> String { @@ -116,4 +132,8 @@ nonisolated enum AgentToolPresentation { else { return value.encodedString } return String(decoding: data, as: UTF8.self) } + + private static func normalized(_ text: String) -> String { + text.split(whereSeparator: \.isWhitespace).joined(separator: " ") + } } diff --git a/Termind/Features/AI/ConversationTranscriptView.swift b/Termind/Features/AI/ConversationTranscriptView.swift index 30f53680..548f241c 100644 --- a/Termind/Features/AI/ConversationTranscriptView.swift +++ b/Termind/Features/AI/ConversationTranscriptView.swift @@ -25,8 +25,13 @@ struct ConversationView: View { if let interaction = conversation.pendingUserInteraction { switch interaction { case .questions(let input): - Divider() AgentUserInputCard(request: input, conversation: conversation) + .padding(.horizontal, Metrics.Space.l) + .frame( + maxWidth: isCompact ? .infinity : Metrics.Chat.contentMaxWidth + ) + .frame(maxWidth: .infinity) + .padding(.top, Metrics.Space.s) case .sshAuthentication(let authentication): AgentSSHAuthenticationCard( request: authentication, conversation: conversation @@ -36,7 +41,11 @@ struct ConversationView: View { if !conversation.pendingSteering.isEmpty { SteeringQueueView(conversation: conversation) } - ComposerView(conversation: conversation, sessionContext: sessionContext) + ComposerView( + conversation: conversation, + maxContentWidth: isCompact ? nil : Metrics.Chat.contentMaxWidth, + sessionContext: sessionContext + ) } } @@ -68,7 +77,9 @@ struct ConversationView: View { // edge of unframed assistant text. .padding(.horizontal, isCompact ? Metrics.Space.l : Metrics.Space.xl) .padding(.vertical, Metrics.Space.xl) - .frame(maxWidth: isCompact ? .infinity : 760) + .frame( + maxWidth: isCompact ? .infinity : Metrics.Chat.contentMaxWidth + ) .frame(maxWidth: .infinity) } // Keyed on the last row's revision, not on a count. Streaming text @@ -178,13 +189,17 @@ private struct AgentToolRowView: View { } if let result = card.result { - Text(AgentToolPresentation.output( - from: result, presentation: card.descriptor.presentation - )) - .frame(maxWidth: .infinity, alignment: .leading) - .font(.caption.monospaced()) - .foregroundStyle(result.isError ? .red : .secondary) - .textSelection(.enabled) + if let output = AgentToolPresentation.detailOutput( + from: result, + presentation: card.descriptor.presentation, + review: card.review + ) { + Text(output) + .frame(maxWidth: .infinity, alignment: .leading) + .font(.caption.monospaced()) + .foregroundStyle(result.isError ? .red : .secondary) + .textSelection(.enabled) + } if result.isTruncated { Text("Output truncated") @@ -364,22 +379,25 @@ private struct AgentUserInputCard: View { } } - // Three questions with four described options each runs past 500pt, - // and this card sits between the transcript and the composer. But a - // bare `ScrollView` is greedy: capped at the same height it would - // hand a one-question card 320pt of box for 120pt of question. So - // the natural stack is preferred and the scrolling one is the - // fallback, which is what `ViewThatFits` is for. + // Prefer the form's natural height. When the surrounding VStack + // offers less room (including after a live window resize), the + // scrolling fallback accepts that proposal instead. There is no + // fixed card height, so one question never inherits the box needed + // by three long ones. ViewThatFits(in: .vertical) { questionStack ScrollView { questionStack } .scrollBounceBehavior(.basedOnSize) } - .frame(maxHeight: Metrics.Chat.userInputMaxHeight) footer } .padding(Metrics.Space.l) + .background(.bar, in: .rect(cornerRadius: Metrics.Chat.composerRadius)) + .overlay { + RoundedRectangle(cornerRadius: Metrics.Chat.composerRadius) + .stroke(.quaternary, lineWidth: 1) + } } private var questionStack: some View { diff --git a/Termind/Features/AI/ConversationViews.swift b/Termind/Features/AI/ConversationViews.swift index 661262c5..63ede985 100644 --- a/Termind/Features/AI/ConversationViews.swift +++ b/Termind/Features/AI/ConversationViews.swift @@ -120,6 +120,9 @@ struct FlowLayout: Layout { struct ComposerView: View { let conversation: ConversationModel var prompt: LocalizedStringKey = "Ask anything…" + /// The full Assistant tab shares one centered column with its transcript. + /// `nil` keeps compact/sidebar composers fluid. + var maxContentWidth: CGFloat? = nil /// What the surface showing this composer knows about the focused pane. /// Defaulting to `.none` is what makes a composer outside a Workspace — the /// Assistant tab — session-free by construction rather than by remembering. @@ -131,7 +134,7 @@ struct ComposerView: View { @Environment(\.accessibilityReduceMotion) private var reduceMotion @FocusState private var isFocused: Bool @State private var draftBeforeTranscription = "" - @State private var draftHeight = Metrics.Chat.composerMinHeight + @State private var draftHeight = Metrics.Chat.compactComposerMinHeight private var resolved: (provider: ModelProvider, model: AIModel)? { app.aiConfiguration.resolve(conversation: conversation.model) @@ -206,7 +209,13 @@ struct ComposerView: View { .stroke(.quaternary, lineWidth: 1) } .padding(.horizontal, Metrics.Space.l) - .padding(.vertical, Metrics.Space.s) + .frame(maxWidth: maxContentWidth ?? .infinity) + .frame(maxWidth: .infinity) + .padding(.top, Metrics.Space.s) + .padding( + .bottom, + maxContentWidth == nil ? Metrics.Space.s : Metrics.Space.l + ) .onAppear(perform: normalizeReasoningEffort) .onChange(of: reasoningResolution) { _, _ in normalizeReasoningEffort() } .onDisappear { @@ -283,12 +292,18 @@ struct ComposerView: View { } .frame( height: min( - max(draftHeight, Metrics.Chat.composerMinHeight), + max(draftHeight, minimumDraftHeight), Metrics.Chat.composerMaxHeight ) ) } + private var minimumDraftHeight: CGFloat { + maxContentWidth == nil + ? Metrics.Chat.compactComposerMinHeight + : Metrics.Chat.composerMinHeight + } + /// `TextEditor`'s own leading text-container inset, which the placeholder /// and the measuring twin both have to match or the text jumps sideways /// when the field gains content. diff --git a/Termind/Features/AI/MessageViews.swift b/Termind/Features/AI/MessageViews.swift index f365a596..4f54e02e 100644 --- a/Termind/Features/AI/MessageViews.swift +++ b/Termind/Features/AI/MessageViews.swift @@ -1,3 +1,4 @@ +import Foundation import SwiftUI /// A turn the user sent, drawn as a bubble against the trailing edge. @@ -9,25 +10,37 @@ struct UserMessageView: View { let message: ChatMessage var isCompact = false - var body: some View { - HStack(alignment: .top, spacing: 0) { - // A fixed gutter rather than a percentage of the column: no - // `GeometryReader`, and the bubble hugs its content instead of - // being stretched to a fraction of the width. - Spacer(minLength: isCompact ? Metrics.Chat.compactGutter : Metrics.Chat.gutter) + @State private var isHovering = false - MarkdownView(text: message.text, isAssistant: false, isCompact: isCompact) - // The bubble is sized by what is in it. Without this the block - // views claim every point they are offered, and "ok" gets a - // bubble as wide as an essay. - .environment(\.markdownFillsWidth, false) - .padding(.horizontal, Metrics.Space.l) - .padding(.vertical, Metrics.Space.m) - .background( - .quaternary.opacity(0.5), - in: .rect(cornerRadius: Metrics.Chat.bubbleRadius) - ) + var body: some View { + VStack(alignment: .trailing, spacing: Metrics.Chat.blockSpacing) { + HStack(alignment: .top, spacing: 0) { + // A fixed gutter rather than a percentage of the column: no + // `GeometryReader`, and the bubble hugs its content instead of + // being stretched to a fraction of the width. + Spacer(minLength: isCompact ? Metrics.Chat.compactGutter : Metrics.Chat.gutter) + + MarkdownView(text: message.text, isAssistant: false, isCompact: isCompact) + // The bubble is sized by what is in it. Without this the block + // views claim every point they are offered, and "ok" gets a + // bubble as wide as an essay. + .environment(\.markdownFillsWidth, false) + .padding(.horizontal, Metrics.Space.l) + .padding(.vertical, Metrics.Space.m) + .background( + .quaternary.opacity(0.5), + in: .rect(cornerRadius: Metrics.Chat.bubbleRadius) + ) + } + + MessageActionRow(message: message, isUser: true, isVisible: isHovering) } + .frame(maxWidth: .infinity, alignment: .trailing) + // Includes the otherwise-empty gap above the action row. Without a + // rectangular hover region, crossing that gap hides the button before + // the pointer can reach it. + .contentShape(.rect) + .onHover { isHovering = $0 } .accessibilityElement(children: .contain) .accessibilityLabel(Text("You said")) } @@ -63,7 +76,7 @@ struct AssistantMessageView: View { StreamingCaret() } } else { - actions + MessageActionRow(message: message, isUser: false, isVisible: isHovering) } ForEach(message.commands) { command in @@ -71,21 +84,65 @@ struct AssistantMessageView: View { } } .frame(maxWidth: .infinity, alignment: .leading) + .contentShape(.rect) .onHover { isHovering = $0 } .accessibilityElement(children: .contain) .accessibilityLabel(Text("Assistant said")) } +} - /// Copying the whole reply, which a stack of block views cannot be framed - /// and dragged over the way a single `Text` could. Paying that back with a - /// button is deliberate — losing it quietly would not be. - private var actions: some View { +/// Copy and time use opposite reading order for the two speakers, matching the +/// side each message occupies. The row remains in layout while transparent so +/// its hit area bridges the pointer from the message body to the copy button. +private struct MessageActionRow: View { + let message: ChatMessage + let isUser: Bool + let isVisible: Bool + + var body: some View { HStack(spacing: Metrics.Space.s) { - CopyButton(text: message.text) - Spacer(minLength: 0) + if isUser { + Spacer(minLength: 0) + timestamp + CopyButton(text: message.text) + } else { + CopyButton(text: message.text) + timestamp + Spacer(minLength: 0) + } } - .opacity(isHovering ? 1 : 0) - .animation(.easeOut(duration: 0.12), value: isHovering) + .opacity(isVisible ? 1 : 0) + .animation(.easeOut(duration: 0.12), value: isVisible) + } + + private var timestamp: some View { + Text(ChatMessageTimestampFormatter.string(for: message.createdAt)) + .font(.caption2.monospacedDigit()) + .foregroundStyle(.tertiary) + .textSelection(.disabled) + } +} + +/// Localized minute-precision metadata for a chat turn. Today omits the date; +/// older messages include the user's short date so restored conversations do +/// not lose temporal context. +nonisolated enum ChatMessageTimestampFormatter { + static func string( + for date: Date, + now: Date = .now, + calendar: Calendar = .autoupdatingCurrent, + locale: Locale = .autoupdatingCurrent, + timeZone: TimeZone = .autoupdatingCurrent + ) -> String { + let formatter = DateFormatter() + var calendar = calendar + calendar.timeZone = timeZone + formatter.calendar = calendar + formatter.locale = locale + formatter.timeZone = timeZone + formatter.dateStyle = calendar.isDate(date, inSameDayAs: now) ? .none : .short + formatter.timeStyle = .short + return formatter.string(from: date) } } diff --git a/TermindTests/AgentToolPresentationTests.swift b/TermindTests/AgentToolPresentationTests.swift index 23a96d26..b9333a21 100644 --- a/TermindTests/AgentToolPresentationTests.swift +++ b/TermindTests/AgentToolPresentationTests.swift @@ -148,6 +148,44 @@ struct AgentToolPresentationTests { ) } + @Test + func duplicateSecurityReviewDenialIsShownOnlyOnce() { + let reason = "The request was not explicitly authorized." + let review = SecurityReviewDecision( + verdict: .deny, risk: .high, userAuthorization: .insufficient, + reason: reason + ) + let duplicate = AgentToolResult( + callID: "call", content: " The request was not explicitly\nauthorized. ", + isError: true + ) + let distinct = AgentToolResult( + callID: "call", content: "The remote host refused the command.", isError: true + ) + + #expect(AgentToolPresentation.detailOutput( + from: duplicate, presentation: nil, review: review + ) == nil) + #expect(AgentToolPresentation.detailOutput( + from: distinct, presentation: nil, review: review + ) == "The remote host refused the command.") + } + + @Test + func allowedReviewNeverHidesAResult() { + let review = SecurityReviewDecision( + verdict: .approve, risk: .low, userAuthorization: .sufficient, + reason: "Completed with a warning." + ) + let result = AgentToolResult( + callID: "call", content: review.reason, isError: true + ) + + #expect(AgentToolPresentation.detailOutput( + from: result, presentation: nil, review: review + ) == review.reason) + } + @Test func interruptedResultsAreExplicitlyMarked() { let result = AgentToolResult( diff --git a/TermindTests/ChatMessageTimestampTests.swift b/TermindTests/ChatMessageTimestampTests.swift new file mode 100644 index 00000000..3df4d348 --- /dev/null +++ b/TermindTests/ChatMessageTimestampTests.swift @@ -0,0 +1,67 @@ +import Foundation +import Testing + +@testable import Termind + +@Suite +struct ChatMessageTimestampTests { + @Test + func todayUsesOnlyTheLocalizedShortTime() throws { + let timeZone = try #require(TimeZone(identifier: "Asia/Shanghai")) + let locale = Locale(identifier: "zh_CN") + var calendar = Calendar(identifier: .gregorian) + calendar.timeZone = timeZone + let message = try #require(calendar.date(from: DateComponents( + year: 2026, month: 8, day: 26, hour: 9, minute: 5 + ))) + let now = try #require(calendar.date(from: DateComponents( + year: 2026, month: 8, day: 26, hour: 17 + ))) + + #expect(ChatMessageTimestampFormatter.string( + for: message, now: now, calendar: calendar, + locale: locale, timeZone: timeZone + ) == expected( + message, dateStyle: .none, locale: locale, + calendar: calendar, timeZone: timeZone + )) + } + + @Test + func anOlderDayIncludesLocalizedDateAndTime() throws { + let timeZone = try #require(TimeZone(identifier: "America/Los_Angeles")) + let locale = Locale(identifier: "en_US") + var calendar = Calendar(identifier: .gregorian) + calendar.timeZone = timeZone + let message = try #require(calendar.date(from: DateComponents( + year: 2026, month: 8, day: 25, hour: 23, minute: 15 + ))) + let now = try #require(calendar.date(from: DateComponents( + year: 2026, month: 8, day: 26, hour: 8 + ))) + + #expect(ChatMessageTimestampFormatter.string( + for: message, now: now, calendar: calendar, + locale: locale, timeZone: timeZone + ) == expected( + message, dateStyle: .short, locale: locale, + calendar: calendar, timeZone: timeZone + )) + } + + private func expected( + _ date: Date, + dateStyle: DateFormatter.Style, + locale: Locale, + calendar: Calendar, + timeZone: TimeZone + ) -> String { + let formatter = DateFormatter() + formatter.locale = locale + formatter.calendar = calendar + formatter.timeZone = timeZone + formatter.dateStyle = dateStyle + formatter.timeStyle = .short + return formatter.string(from: date) + } +}