From 9e733b16302e00580e63a4856147b67831dc49f9 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 16 Apr 2026 21:46:57 +0000 Subject: [PATCH 1/3] fix(macos): prevent Show less button from disappearing on user messages Decouple the user-message collapse decision from observed geometry so clicking Show less collapses the bubble instead of removing the button. .frame(height: 150) is a _FrameLayout, which hard-proposes 150pt to its child. onGeometryChange on the same subtree then reports the clamped height back into @State, flipping isCollapsible to false on the next render and tearing down both the button and the clamp together. The decision is now derived from a conservative estimate of text + per-attachment heights, which is deterministic from the model and not affected by layout. Worst case (estimate too low) gracefully degrades to no collapse button and natural-height rendering, matching existing non-collapsible user messages. Co-Authored-By: ashlee@vellum.ai --- .../Features/Chat/ChatBubble.swift | 71 +++++++++++++------ 1 file changed, 50 insertions(+), 21 deletions(-) diff --git a/clients/macos/vellum-assistant/Features/Chat/ChatBubble.swift b/clients/macos/vellum-assistant/Features/Chat/ChatBubble.swift index 37a2f76a053..68d68390536 100644 --- a/clients/macos/vellum-assistant/Features/Chat/ChatBubble.swift +++ b/clients/macos/vellum-assistant/Features/Chat/ChatBubble.swift @@ -71,7 +71,6 @@ struct ChatBubble: View, Equatable { var isLatestAssistantMessage: Bool = false var typographyGeneration: Int = 0 @State private var isUserMessageExpanded: Bool = false - @State private var userMessageIntrinsicHeight: CGFloat = 0 private let userMessageMaxCollapsedHeight: CGFloat = 150 private static let heuristicUserCollapseCharacterThreshold = 3_000 private static let heuristicUserCollapseLineThreshold = 40 @@ -609,11 +608,27 @@ struct ChatBubble: View, Equatable { return preview == trimmedText ? preview : "\(preview)\n\n..." } - /// Estimates whether the user message text will exceed the collapse - /// threshold when rendered. Used on the first frame before - /// `onGeometryChange` has fired to avoid a full-height flash. - private var estimatedTextExceedsCollapseThreshold: Bool { + /// Estimates whether the user message content will exceed the collapse + /// threshold when rendered. Decides the collapse state deterministically + /// from the model (text + attachments) rather than from observed geometry, + /// which avoids a measurement feedback loop: `.frame(height: 150)` forces + /// the observed subtree to lay out at 150pt, and if an `onGeometryChange` + /// inside that subtree writes the clamped height back to state, the + /// "is this message collapsible?" decision flips to false on the next + /// render — removing the "Show less" button and the clamp together. + /// + /// Per-attachment heights mirror the renderers in `ChatBubbleAttachmentContent.swift`: + /// single images use `AspectFitImageView` (~200pt typical), grid tiles are 120pt, + /// videos/inline previews are ~200pt, and file chips are compact rows. + /// + /// The estimate is intentionally conservative — it only needs to correctly + /// classify content above/below ~150pt. When the estimate is wrong on the + /// low side, the worst-case UX is a missing "Show more" button and the + /// content rendering at its natural height (no clipping), which matches + /// the existing non-collapsible user message behavior. + private var estimatedContentExceedsCollapseThreshold: Bool { guard isUser, !message.isStreaming else { return false } + let text = message.text as NSString let contentWidth = max(bubbleMaxWidth - 2 * VSpacing.lg, 0) let font = NSFont.systemFont(ofSize: 14, weight: .regular) @@ -622,34 +637,48 @@ struct ChatBubble: View, Equatable { options: [.usesLineFragmentOrigin, .usesFontLeading], attributes: [.font: font] ) - return ceil(textRect.height) > userMessageMaxCollapsedHeight + let textHeight = ceil(textRect.height) + + let parts = partitionedAttachments + let imageCount = parts.images.count + let imageHeight: CGFloat + if imageCount == 0 { + imageHeight = 0 + } else if imageCount == 1 { + imageHeight = 200 + } else { + let rows = ceil(Double(imageCount) / 2) + imageHeight = CGFloat(rows) * 120 + } + let videoHeight = CGFloat(parts.videos.count) * 200 + let audioHeight = CGFloat(parts.audios.count) * 80 + let fileHeight = CGFloat(parts.files.count) * 40 + + let totalHeight = textHeight + imageHeight + videoHeight + audioHeight + fileHeight + return totalHeight > userMessageMaxCollapsedHeight } // MARK: - User Message Collapse / Expand // - // .frame(maxHeight:) creates _FlexFrameLayout which recursively measures + // `.frame(maxHeight:)` creates `_FlexFrameLayout` which recursively measures // children and resolves explicitAlignment through the entire LazyVStack - // subtree — O(n × depth) per layout pass, causing 35 s+ hangs. + // subtree — O(n × depth) per layout pass, causing 35 s+ hangs. We use + // `.frame(height:)` instead (`_FrameLayout` — O(1), no alignment cascade). // - // Fix: .frame(height:) creates _FrameLayout — O(1), no alignment cascade. - // When height is nil (expanded / short), _FrameLayout passes through the - // child's natural height. Single view identity is preserved (no - // _ConditionalContent), so withAnimation still drives a smooth height - // transition on expand/collapse. + // The collapsibility decision is driven by a deterministic content + // estimate (see `estimatedContentExceedsCollapseThreshold`). Observing + // the clamped subtree's height and feeding it back into `@State` would + // create a layout/state feedback loop that collapses the button on + // toggle — see https://developer.apple.com/documentation/swiftui/view/ongeometrychange(for:of:action:) + // for the general guidance that geometry observations should not drive + // state that changes the observed layout. @ViewBuilder private func userMessageHeightWrapper(@ViewBuilder _ content: () -> Content) -> some View { - let isCollapsible = userMessageIntrinsicHeight > 0 - ? userMessageIntrinsicHeight > userMessageMaxCollapsedHeight - : estimatedTextExceedsCollapseThreshold + let isCollapsible = estimatedContentExceedsCollapseThreshold let needsCollapse = isCollapsible && !isUserMessageExpanded VStack(alignment: .leading, spacing: 0) { content() - .onGeometryChange(for: CGFloat.self) { proxy in - proxy.size.height - } action: { height in - userMessageIntrinsicHeight = height - } .frame(height: needsCollapse ? userMessageMaxCollapsedHeight : nil, alignment: .top) .clipped() .overlay(alignment: .bottom) { From e9d78b777156f29079b4b91e04583993ceb1d78e Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 16 Apr 2026 21:53:27 +0000 Subject: [PATCH 2/3] docs: update SCROLL_STRATEGY and account for bubble padding in collapse estimate - SCROLL_STRATEGY.md: reflect the purely-deterministic content estimate (removes stale reference to userMessageIntrinsicHeight and estimatedTextExceedsCollapseThreshold, adds explicit no-geometry-in-state guidance with link to Apple docs). - ChatBubble.swift: include bubbleChrome vertical padding (2 * VSpacing.md) and inter-section VStack spacing (VSpacing.sm) in estimatedContentExceedsCollapseThreshold. Without this, messages with intrinsic content between ~126pt and 150pt render as bubbles >150pt but were classified as non-collapsible, hiding the Show more button. Co-Authored-By: ashlee@vellum.ai --- clients/macos/SCROLL_STRATEGY.md | 13 ++++++---- .../Features/Chat/ChatBubble.swift | 25 ++++++++++++++++++- 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/clients/macos/SCROLL_STRATEGY.md b/clients/macos/SCROLL_STRATEGY.md index 8cde550739b..0d4dbaa4754 100644 --- a/clients/macos/SCROLL_STRATEGY.md +++ b/clients/macos/SCROLL_STRATEGY.md @@ -206,16 +206,19 @@ Deep-link scroll uses `.center` anchor for scroll-to-ID via `ScrollPosition` val --- -## User Message Collapse (Prevents First-Frame Flash) +## User Message Collapse -Long user messages collapse at 150pt. The collapse decision uses `NSString.boundingRect` on the first frame (before `onGeometryChange` fires) to avoid a full-height flash: +Long user messages collapse at 150pt. The collapse decision is driven **purely** by a deterministic estimate of text + per-attachment heights, computed from the model: ```swift -let isCollapsible = userMessageIntrinsicHeight > 0 - ? userMessageIntrinsicHeight > userMessageMaxCollapsedHeight - : estimatedTextExceedsCollapseThreshold // NSString.boundingRect estimate +let isCollapsible = estimatedContentExceedsCollapseThreshold +let needsCollapse = isCollapsible && !isUserMessageExpanded ``` +`estimatedContentExceedsCollapseThreshold` combines `NSString.boundingRect` on the message text with conservative per-attachment heights (single image ~200pt, grid tiles 120pt, videos/inline previews ~200pt, audio ~80pt, file chips ~40pt) that mirror the renderers in `ChatBubbleAttachmentContent.swift`. + +**Do not wire `onGeometryChange` (or any layout observation) into the collapsibility state.** `.frame(height: 150)` is a `_FrameLayout` that hard-proposes 150pt to its child; observing the child's height and feeding it back into state creates a feedback loop that flips `isCollapsible` to false on the first toggle, removing the "Show less" button and the frame clamp together. See [`onGeometryChange` docs](https://developer.apple.com/documentation/swiftui/view/ongeometrychange(for:of:action:)) for the general guidance that geometry observations should not drive state that changes the observed layout. + Collapsed messages have: - Gradient fade overlay (transparent -> `VColor.surfaceLift`) - "Show more" button using `VButton(style: .ghost, size: .compact, tintColor: .contentTertiary)`, left-aligned diff --git a/clients/macos/vellum-assistant/Features/Chat/ChatBubble.swift b/clients/macos/vellum-assistant/Features/Chat/ChatBubble.swift index 68d68390536..df739435139 100644 --- a/clients/macos/vellum-assistant/Features/Chat/ChatBubble.swift +++ b/clients/macos/vellum-assistant/Features/Chat/ChatBubble.swift @@ -654,7 +654,30 @@ struct ChatBubble: View, Equatable { let audioHeight = CGFloat(parts.audios.count) * 80 let fileHeight = CGFloat(parts.files.count) * 40 - let totalHeight = textHeight + imageHeight + videoHeight + audioHeight + fileHeight + // `bubbleChrome` adds top+bottom padding around the entire subtree + // (see `bubbleChrome` for user messages: `EdgeInsets(top: VSpacing.md, bottom: VSpacing.md, ...)`), + // and the inner VStack separates sibling content sections by `VSpacing.sm`. + // These contribute to the rendered bubble height that `.frame(height: 150)` + // clamps, so the estimate must include them — otherwise messages with + // intrinsic content between (150 - padding) and 150pt render taller than + // the clamp but are classified as non-collapsible. + let bubbleVerticalPadding: CGFloat = 2 * VSpacing.md + let contentSections = [ + textHeight > 0, + imageHeight > 0, + videoHeight > 0, + audioHeight > 0, + fileHeight > 0 + ].filter { $0 }.count + let interSectionSpacing = CGFloat(max(0, contentSections - 1)) * VSpacing.sm + + let totalHeight = textHeight + + imageHeight + + videoHeight + + audioHeight + + fileHeight + + bubbleVerticalPadding + + interSectionSpacing return totalHeight > userMessageMaxCollapsedHeight } From 9171e471782e372dd16f722c729a8d41fa704a32 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 16 Apr 2026 21:57:19 +0000 Subject: [PATCH 3/3] use VFont.nsChat in collapse estimate to match rendered text font User messages render through MarkdownSegmentView using VFont.chat (16pt DM Sans). The estimate previously used NSFont.systemFont at 14pt, which under-counted text height and classified some messages that actually render > 150pt as non-collapsible. Co-Authored-By: ashlee@vellum.ai --- .../macos/vellum-assistant/Features/Chat/ChatBubble.swift | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/clients/macos/vellum-assistant/Features/Chat/ChatBubble.swift b/clients/macos/vellum-assistant/Features/Chat/ChatBubble.swift index df739435139..34e2066b051 100644 --- a/clients/macos/vellum-assistant/Features/Chat/ChatBubble.swift +++ b/clients/macos/vellum-assistant/Features/Chat/ChatBubble.swift @@ -631,7 +631,10 @@ struct ChatBubble: View, Equatable { let text = message.text as NSString let contentWidth = max(bubbleMaxWidth - 2 * VSpacing.lg, 0) - let font = NSFont.systemFont(ofSize: 14, weight: .regular) + // Match the font used by `MarkdownSegmentView` (VFont.chat = 16pt DM Sans) + // so the estimate tracks the actual rendered text height. Using a smaller + // system font under-counts height and misses messages that render >150pt. + let font = VFont.nsChat let textRect = text.boundingRect( with: NSSize(width: contentWidth, height: .greatestFiniteMagnitude), options: [.usesLineFragmentOrigin, .usesFontLeading],