diff --git a/clients/macos/vellum-assistant/Features/Chat/AssistantProgressView.swift b/clients/macos/vellum-assistant/Features/Chat/AssistantProgressView.swift index a09d85443d1..9d4acacbb20 100644 --- a/clients/macos/vellum-assistant/Features/Chat/AssistantProgressView.swift +++ b/clients/macos/vellum-assistant/Features/Chat/AssistantProgressView.swift @@ -792,6 +792,12 @@ private struct StepDetailRow: View { var skillLabel: String? var onRehydrate: (() -> Void)? @State private var isHovered = false + /// Cached colored AttributedString for the tool call result — computed once + /// on first expand / result change to avoid rebuilding on every render. + @State private var cachedColoredResult: AttributedString? + /// Cached line count + isLong flag for resolvedInputFull — avoids O(n) + /// byte scan in the view body on every render. + @State private var cachedInputIsLong: Bool? @Environment(\.displayScale) private var displayScale @Environment(\.suppressAutoScroll) private var suppressAutoScroll @@ -918,6 +924,16 @@ private struct StepDetailRow: View { .animation(VAnimation.fast, value: isDetailExpanded) .onChange(of: isDetailExpanded) { _, newValue in if newValue { + // Eagerly populate caches before the expanded body evaluates + // so the first render has colored output and correct input sizing. + if cachedColoredResult == nil, + let result = toolCall.result, !result.isEmpty { + cachedColoredResult = coloredOutput(result, isError: toolCall.isError) + } + if cachedInputIsLong == nil && !resolvedInputFull.isEmpty { + let lines = resolvedInputFull.utf8.reduce(1) { c, b in b == 0x0A ? c + 1 : c } + cachedInputIsLong = lines > 30 || (lines == 1 && resolvedInputFull.utf8.count > 50_000) + } Task { @MainActor in onRehydrate?() } @@ -949,10 +965,22 @@ private struct StepDetailRow: View { .font(VFont.labelDefault) .foregroundStyle(VColor.contentSecondary) if !resolvedInputFull.isEmpty { - Text(resolvedInputFull) - .font(VFont.bodySmallDefault) - .foregroundStyle(VColor.contentSecondary) - .fixedSize(horizontal: false, vertical: true) + let inputIsLong = cachedInputIsLong ?? false + + if inputIsLong { + ScrollView { + Text(resolvedInputFull) + .font(VFont.bodySmallDefault) + .foregroundStyle(VColor.contentSecondary) + .frame(maxWidth: .infinity, alignment: .leading) + } + .frame(height: 300) + .clipShape(RoundedRectangle(cornerRadius: VRadius.sm)) + } else { + Text(resolvedInputFull) + .font(VFont.bodySmallDefault) + .foregroundStyle(VColor.contentSecondary) + } } } } @@ -988,11 +1016,14 @@ private struct StepDetailRow: View { .foregroundStyle(VColor.contentTertiary) .textCase(.uppercase) + // Cached colored output — populated eagerly in + // .onChange(of: isDetailExpanded) or .onAppear. outputBlock( - text: nil, - attributedText: coloredOutput(result, isError: toolCall.isError), + text: cachedColoredResult == nil ? result : nil, + attributedText: cachedColoredResult, copyText: result, - copyLabel: "Copy output" + copyLabel: "Copy output", + isError: toolCall.isError ) } .padding(.horizontal, VSpacing.lg) @@ -1000,40 +1031,51 @@ private struct StepDetailRow: View { } .padding(.bottom, VSpacing.sm) .textSelection(.enabled) + .onAppear { + if cachedColoredResult == nil, + let result = toolCall.result, !result.isEmpty { + cachedColoredResult = coloredOutput(result, isError: toolCall.isError) + } + if cachedInputIsLong == nil && !resolvedInputFull.isEmpty { + let lines = resolvedInputFull.utf8.reduce(1) { c, b in b == 0x0A ? c + 1 : c } + cachedInputIsLong = lines > 30 || (lines == 1 && resolvedInputFull.utf8.count > 50_000) + } + } + .onChange(of: toolCall.result) { _, newResult in + if let result = newResult, !result.isEmpty { + cachedColoredResult = coloredOutput(result, isError: toolCall.isError) + } else { + cachedColoredResult = nil + } + } } // MARK: - Output Block - /// Reusable output block with a height-bounded ScrollView for long outputs. + /// Reusable output block with copy button. + /// Long content (>30 lines) gets a definite-height ScrollView so LazyVStack + /// skips content measurement. Short content renders directly with no ScrollView. @ViewBuilder private func outputBlock( text: String?, attributedText: AttributedString?, copyText: String, - copyLabel: String + copyLabel: String, + isError: Bool = false ) -> some View { - let lineCount = copyText.components(separatedBy: "\n").count + let lines = copyText.utf8.reduce(1) { count, byte in byte == 0x0A ? count + 1 : count } + let isLong = lines > 30 || (lines == 1 && copyText.utf8.count > 50_000) ZStack(alignment: .topTrailing) { VStack(alignment: .leading, spacing: VSpacing.xs) { - if lineCount > 500 { - // Content at 500+ lines always exceeds 400pt, so a fixed - // height lets sizeThatFits return without measuring content. + if isLong { + // Definite height — LazyVStack never measures content inside. ScrollView { - outputTextView(text: text, attributedText: attributedText) + outputTextView(text: text, attributedText: attributedText, isError: isError) } .frame(height: 400) - } else if let attrText = attributedText { - Text(attrText) - .font(VFont.bodySmallDefault) - .frame(maxWidth: .infinity, alignment: .leading) - .fixedSize(horizontal: false, vertical: true) - } else if let plainText = text { - Text(plainText) - .font(VFont.bodySmallDefault) - .foregroundStyle(VColor.contentSecondary) - .frame(maxWidth: .infinity, alignment: .leading) - .fixedSize(horizontal: false, vertical: true) + } else { + outputTextView(text: text, attributedText: attributedText, isError: isError) } } .padding(EdgeInsets(top: VSpacing.sm, leading: VSpacing.sm, bottom: VSpacing.sm, trailing: VSpacing.sm + VSpacing.xl)) @@ -1066,11 +1108,12 @@ private struct StepDetailRow: View { } } - /// Text view used inside the ScrollView for long outputs. + /// Text view for output content, used by both the ScrollView (long) and direct (short) paths. @ViewBuilder private func outputTextView( text: String?, - attributedText: AttributedString? + attributedText: AttributedString?, + isError: Bool = false ) -> some View { if let attrText = attributedText { Text(attrText) @@ -1079,7 +1122,7 @@ private struct StepDetailRow: View { } else if let plainText = text { Text(plainText) .font(VFont.bodySmallDefault) - .foregroundStyle(VColor.contentSecondary) + .foregroundStyle(isError ? VColor.systemNegativeStrong : VColor.contentSecondary) .frame(maxWidth: .infinity, alignment: .leading) } } diff --git a/clients/shared/DesignSystem/Modifiers/LazyVStackScrollFrameModifier.swift b/clients/shared/DesignSystem/Modifiers/LazyVStackScrollFrameModifier.swift index 6f1d87fa459..9fa001b05a4 100644 --- a/clients/shared/DesignSystem/Modifiers/LazyVStackScrollFrameModifier.swift +++ b/clients/shared/DesignSystem/Modifiers/LazyVStackScrollFrameModifier.swift @@ -4,23 +4,36 @@ extension View { /// Applies an adaptive height constraint to a `ScrollView` inside a `LazyVStack` cell. /// /// For content exceeding `lineThreshold` lines, a definite `frame(height:)` is used so - /// `LazyVStack` can skip scroll-content measurement during cell sizing. For shorter content - /// `frame(maxHeight:)` is used so the view collapses to its natural height instead of - /// rendering with blank space. + /// `LazyVStack` can skip scroll-content measurement during cell sizing. When content is a + /// single line, the `charThreshold` catches mega-strings (e.g. base64 data, minified JSON) + /// that would otherwise trigger an expensive Core Text width measurement — the char check + /// is skipped for multi-line content since `lineThreshold` already covers that case. + /// Short content gets no height constraint at all — the ScrollView collapses to its + /// natural content height. Do NOT use `.frame(maxHeight:)` for the short path — it + /// creates a `_FlexFrameLayout` that recursively measures children inside LazyVStack cells. /// /// - Parameters: - /// - text: The string whose line count determines which constraint is applied. - /// - maxHeight: The height cap applied in both branches. - /// - lineThreshold: Line count above which the fixed height is used. Default: 500. + /// - text: The string whose size determines which constraint is applied. + /// - maxHeight: The definite height applied when content is long. + /// - lineThreshold: Line count above which the fixed height is used. Default: 30. + /// - charThreshold: UTF-8 byte count above which the fixed height is used. Default: 50 000. + /// - lineCount: Pre-computed line count. When provided, the modifier skips its + /// internal `countLines` scan. Use this when the caller caches the line count + /// via `@State` to avoid redundant O(n) work on re-render. func adaptiveScrollFrame( for text: String, maxHeight: CGFloat, - lineThreshold: Int = 500 + lineThreshold: Int = 30, + charThreshold: Int = 50_000, + lineCount: Int? = nil ) -> some View { - let isLong = countLines(in: text) > lineThreshold + let lines = lineCount ?? countLines(in: text) + let isLong = lines > lineThreshold || (lines == 1 && text.utf8.count > charThreshold) return self .frame(height: isLong ? maxHeight : nil) - .frame(maxHeight: isLong ? nil : maxHeight) + // Short content: no height constraint — ScrollView collapses to + // content height naturally. Do NOT use .frame(maxHeight:) here — + // it creates a _FlexFrameLayout that recursively measures children. } } diff --git a/clients/shared/Features/Chat/ToolCallChip.swift b/clients/shared/Features/Chat/ToolCallChip.swift index 3871c54b528..c2f8d8dfa4b 100644 --- a/clients/shared/Features/Chat/ToolCallChip.swift +++ b/clients/shared/Features/Chat/ToolCallChip.swift @@ -237,7 +237,7 @@ public struct ToolCallChip: View { .frame(maxWidth: .infinity, alignment: .leading) .textSelection(.enabled) } - .adaptiveScrollFrame(for: result, maxHeight: 400) + .adaptiveScrollFrame(for: result, maxHeight: 400, lineCount: lineCount) } } } diff --git a/clients/shared/Features/Chat/ToolCallProgressBar.swift b/clients/shared/Features/Chat/ToolCallProgressBar.swift index da7dd4bdf35..232c5d88789 100644 --- a/clients/shared/Features/Chat/ToolCallProgressBar.swift +++ b/clients/shared/Features/Chat/ToolCallProgressBar.swift @@ -5,6 +5,9 @@ import SwiftUI public struct ToolCallProgressBar: View { public let toolCalls: [ToolCallData] @State private var expandedStepId: UUID? + /// Cached line count for the expanded tool call's result text — avoids O(n) + /// byte scan on every SwiftUI render pass when a step is expanded. + @State private var cachedResultLineCount: Int? public init(toolCalls: [ToolCallData]) { self.toolCalls = toolCalls @@ -63,6 +66,7 @@ public struct ToolCallProgressBar: View { if expandedStepId == toolCall.id { expandedStepId = nil } else if toolCall.isComplete { + cachedResultLineCount = nil expandedStepId = toolCall.id } } @@ -250,12 +254,38 @@ public struct ToolCallProgressBar: View { .frame(maxWidth: .infinity, alignment: .leading) .textSelection(.enabled) } - .adaptiveScrollFrame(for: result, maxHeight: 200) + .adaptiveScrollFrame(for: result, maxHeight: 200, lineThreshold: 12, lineCount: cachedResultLineCount) } } } } .padding(VSpacing.md) + .onAppear { + if cachedResultLineCount == nil, + let expandedId = expandedStepId, + let expandedCall = toolCalls.first(where: { $0.id == expandedId }), + let result = expandedCall.result { + cachedResultLineCount = ToolCallChip.countLines(in: result) + } + } + .onChange(of: expandedStepId) { + if let expandedId = expandedStepId, + let expandedCall = toolCalls.first(where: { $0.id == expandedId }), + let result = expandedCall.result { + cachedResultLineCount = ToolCallChip.countLines(in: result) + } else { + cachedResultLineCount = nil + } + } + .onChange(of: toolCalls.first(where: { $0.id == expandedStepId })?.resultLength) { + if let expandedId = expandedStepId, + let expandedCall = toolCalls.first(where: { $0.id == expandedId }), + let result = expandedCall.result { + cachedResultLineCount = ToolCallChip.countLines(in: result) + } else { + cachedResultLineCount = nil + } + } .background( RoundedRectangle(cornerRadius: VRadius.md) .fill(VColor.surfaceOverlay) diff --git a/clients/shared/Features/Chat/ToolConfirmationBubble.swift b/clients/shared/Features/Chat/ToolConfirmationBubble.swift index f4d0854b848..64f3fd07b60 100644 --- a/clients/shared/Features/Chat/ToolConfirmationBubble.swift +++ b/clients/shared/Features/Chat/ToolConfirmationBubble.swift @@ -322,7 +322,7 @@ public struct ToolConfirmationBubble: View { .frame(maxWidth: .infinity, alignment: .leading) .textSelection(.enabled) } - .adaptiveScrollFrame(for: content, maxHeight: maxHeight) + .adaptiveScrollFrame(for: content, maxHeight: maxHeight, lineThreshold: Int(maxHeight / 16)) .padding(VSpacing.sm) .frame(maxWidth: .infinity, alignment: .leading) .background(