Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 15 additions & 14 deletions clients/macos/SCROLL_STRATEGY.md
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,18 @@ This fills the viewport below the user message, so after scroll-to-bottom the us

## MinHeight Calculation (Critical)

Three-path formula depending on the last user message:

```swift
let estimatedUserHeight = min(NSString.boundingRect(text) + 100, 260)
// Path 1: No user message
estimatedUserHeight = 80

// Path 2: Heuristic-collapsed (text.count > 3,000 or > 40 lines)
estimatedUserHeight = NSString.boundingRect(previewText) + 60 + 30 + attachmentHeight

// Path 3: Normal (renders at full height)
estimatedUserHeight = NSString.boundingRect(fullText) + 60 + attachmentHeight

let composerHeight: CGFloat = 80 // static — composer is empty after send
let layoutPadding = VSpacing.md * 3 + 1 // top + bottom + inter-item + anchor
let turnMinHeight = containerHeight - composerHeight - estimatedUserHeight - layoutPadding
Expand All @@ -124,7 +134,7 @@ let turnMinHeight = containerHeight - composerHeight - estimatedUserHeight - lay
### Key decisions:
- **Uses `containerHeight`** (full chat pane from GeometryReader), NOT `scrollState.viewportHeight`. The viewport height fluctuates when the composer resizes — the container height is stable.
- **Composer is static 80pt.** We only care about the composer height when it's empty (after the user hits send). It grows when typing, but by then minHeight doesn't matter.
- **User message estimated via `NSString.boundingRect`** for word-wrap accuracy. Cell overhead is 100pt (bubble padding 24 + timestamp 24 + spacing 12 + show more button 30 + gradient 10). Capped at 260pt (collapse threshold + overhead).
- **User message estimated via `NSString.boundingRect`** for word-wrap accuracy. Cell overhead is 60pt (bubble padding 24 + timestamp 24 + spacing 12). Attachment height is estimated per-type (images use grid layout at ~130pt/row, videos ~200pt, audio ~60pt, files ~40pt). No fixed cap — moderate messages render at full height; only heuristic-collapsed messages (> 3,000 chars / > 40 lines) use preview text height + 30pt "Show more" button.
- **MinHeight applies when `row.isLatestAssistant && row.message.id == state.rows.last?.message.id`.** No `isActiveTurn` gate — the minHeight persists after streaming ends so the viewport doesn't jump.

---
Expand Down Expand Up @@ -196,20 +206,11 @@ withAnimation(VAnimation.spring) {

---

## 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:

```swift
let isCollapsible = userMessageIntrinsicHeight > 0
? userMessageIntrinsicHeight > userMessageMaxCollapsedHeight
: estimatedTextExceedsCollapseThreshold // NSString.boundingRect estimate
```
Only extremely large user messages (>3,000 characters or >40 lines) are collapsed. These use a text-truncation heuristic: the preview is limited to 1,200 characters / 24 lines with a trailing "..." indicator. A "Show more" / "Show less" button toggles between the truncated preview and full text.

Collapsed messages have:
- Gradient fade overlay (transparent → `VColor.surfaceLift`)
- "Show more" button using `VButton(style: .ghost, size: .compact, tintColor: .contentTertiary)`, left-aligned
- Button is inside the bubble container (rounded corners, surfaceLift background)
Moderate-length user messages (under the heuristic threshold) render at full height with no collapse or height clipping. The previous height-based collapse (150pt cap with `onGeometryChange` measurement) was removed because it aggressively truncated typical user messages to ~7-8 lines, causing content to appear cut off (LUM-833).

---

Expand Down
104 changes: 6 additions & 98 deletions clients/macos/vellum-assistant/Features/Chat/ChatBubble.swift
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,9 @@ 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

static let heuristicUserCollapseCharacterThreshold = 3_000
static let heuristicUserCollapseLineThreshold = 40
private static let heuristicUserPreviewCharacterLimit = 1_200
private static let heuristicUserPreviewLineLimit = 24

Expand Down Expand Up @@ -558,7 +557,7 @@ struct ChatBubble: View, Equatable {
Self.collapsedPreviewText(from: message.text)
}

private static func exceedsLineLimit(_ text: String, limit: Int) -> Bool {
static func exceedsLineLimit(_ text: String, limit: Int) -> Bool {
guard limit > 0 else { return !text.isEmpty }
var lineCount = 1
for character in text {
Expand All @@ -571,7 +570,7 @@ struct ChatBubble: View, Equatable {
return false
}

private static func collapsedPreviewText(from text: String) -> String {
static func collapsedPreviewText(from text: String) -> String {
let trimmedText = text.trimmingCharacters(in: .whitespacesAndNewlines)
guard !trimmedText.isEmpty else { return text }

Expand All @@ -591,101 +590,10 @@ 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 {
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)
let textRect = text.boundingRect(
with: NSSize(width: contentWidth, height: .greatestFiniteMagnitude),
options: [.usesLineFragmentOrigin, .usesFontLeading],
attributes: [.font: font]
)
return ceil(textRect.height) > userMessageMaxCollapsedHeight
}

// MARK: - User Message Collapse / Expand
//
// .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.
//
// 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.

@ViewBuilder
private func userMessageHeightWrapper<Content: View>(@ViewBuilder _ content: () -> Content) -> some View {
let isCollapsible = userMessageIntrinsicHeight > 0
? userMessageIntrinsicHeight > userMessageMaxCollapsedHeight
: estimatedTextExceedsCollapseThreshold
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) {
if needsCollapse {
LinearGradient(
colors: [
VColor.surfaceLift.opacity(0),
VColor.surfaceLift
],
startPoint: .init(x: 0.5, y: 0),
endPoint: .init(x: 0.5, y: 1)
)
.frame(height: 40)
.allowsHitTesting(false)
}
}

if isCollapsible {
collapseToggleButton
.padding(.horizontal, VSpacing.lg)
.padding(.bottom, VSpacing.sm)
}
}
.if(isCollapsible) { view in
view
.background(
RoundedRectangle(cornerRadius: VRadius.lg)
.fill(VColor.surfaceLift)
)
}
}

@ViewBuilder
private func heuristicUserMessageCollapseWrapper<Content: View>(@ViewBuilder _ content: () -> Content) -> some View {
VStack(alignment: .leading, spacing: 0) {
content()
// Clip to same height as the measurement-based collapse path
// so both produce a consistent collapsed height.
.frame(height: isUserMessageExpanded ? nil : userMessageMaxCollapsedHeight, alignment: .top)
.clipped()
.overlay(alignment: .bottom) {
if !isUserMessageExpanded {
LinearGradient(
colors: [
VColor.surfaceLift.opacity(0),
VColor.surfaceLift
],
startPoint: .init(x: 0.5, y: 0),
endPoint: .init(x: 0.5, y: 1)
)
.frame(height: 40)
.allowsHitTesting(false)
}
}
collapseToggleButton
.padding(.horizontal, VSpacing.lg)
.padding(.bottom, VSpacing.sm)
Comment on lines 593 to 599
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Heuristic collapse wrapper no longer clips content height — relies entirely on text truncation

The old heuristicUserMessageCollapseWrapper had .frame(height: isUserMessageExpanded ? nil : userMessageMaxCollapsedHeight) + .clipped() + gradient overlay. The new version removes all of these, relying entirely on effectiveRenderingText being set to collapsedUserMessagePreviewText (1200 chars / 24 lines + ...) at ChatBubble.swift:669-670. This means the collapsed height is now determined by the preview text's natural rendered height rather than a fixed 150pt cap. The visual result is different: no gradient fade, and the collapsed height varies based on preview text content. The "Show more" button is always visible (not gated on isCollapsible), which is correct since the wrapper is only used when shouldUseHeuristicCollapse is true. One side effect: since attachments (images, files, videos, audio) are rendered inside bubbleChrome at ChatBubble.swift:708-743, they are always displayed at full size even when the text is collapsed — the old height clipping would have hidden them if they were below the 150pt line.

(Refers to lines 593-605)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct — the heuristic collapse wrapper now relies entirely on text truncation (1,200 chars / 24 lines) rather than height clipping. This is intentional: the text truncation provides a proper expand flow with "Show more" button, while the old height clipping just cut content off visually. The attachment visibility change is also intentional — attachments should be visible even when the text portion is collapsed, since they're the primary content the user shared.

Expand Down Expand Up @@ -848,7 +756,7 @@ struct ChatBubble: View, Equatable {
if shouldUseHeuristicCollapse {
heuristicUserMessageCollapseWrapper { chrome }
} else {
userMessageHeightWrapper { chrome }
chrome
Comment on lines 756 to +759
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep attachment bubbles in active-turn height calculation

Switching non-heuristic user messages to render chrome directly means attachment messages are no longer collapsed by default, but MessageListContentView.estimatedUserHeight still assumes any attachment message is capped at 260. For large image/file previews this underestimates the real user-row height and inflates turnMinHeight, which can break the active-turn scroll behavior (extra blank space / user message no longer pinned as intended) whenever the latest user message includes attachments.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b2286e3. Removed the stale 260pt early return for attachment messages and added a per-attachment height estimate (~200pt each) to the minHeight calculation for both heuristic-collapsed and non-collapsed paths.

}
} else {
chrome
Expand Down
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -288,10 +288,6 @@ struct MessageListContentView: View, Equatable {
}) else {
return 80 + markerHeight
}
// Messages with attachments are always collapsed — use max height
if !lastUser.message.attachments.isEmpty {
return 260 + markerHeight
}
let text = lastUser.message.text as NSString
let contentWidth = max(layoutMetrics.bubbleMaxWidth - 2 * VSpacing.lg, 0)
let font = NSFont.systemFont(ofSize: 14, weight: .regular)
Expand All @@ -301,10 +297,51 @@ struct MessageListContentView: View, Equatable {
attributes: [.font: font]
)
let textHeight = ceil(textRect.height)
// Bubble padding (24) + timestamp (24) + spacing (12) + show more button (30) + gradient (10)
let cellOverhead: CGFloat = 100
// Cap at collapsed bubble height (150pt content + overhead)
return min(textHeight + cellOverhead, 260) + markerHeight
// Bubble padding (24) + timestamp (24) + spacing (12)
let cellOverhead: CGFloat = 60
// Estimate attachment height by type
let attachmentHeight: CGFloat = {
guard !lastUser.message.attachments.isEmpty else { return 0 }
var imageCount = 0
var videoCount = 0
var audioCount = 0
var fileCount = 0
for attachment in lastUser.message.attachments {
if attachment.mimeType.hasPrefix("image/") {
imageCount += 1
} else if attachment.mimeType.hasPrefix("video/") {
videoCount += 1
} else if attachment.mimeType.hasPrefix("audio/") {
audioCount += 1
} else {
fileCount += 1
}
}
// Image grid: adaptive columns at min 160px width
let columnsPerRow = max(1, Int(contentWidth / 160))
let imageRows = imageCount > 0 ? CGFloat((imageCount + columnsPerRow - 1) / columnsPerRow) : 0
let imageHeight = imageRows * 130 // 120pt cell + spacing
let videoHeight = CGFloat(videoCount) * 200
let audioHeight = CGFloat(audioCount) * 60
let fileHeight = CGFloat(fileCount) * 40
return imageHeight + videoHeight + audioHeight + fileHeight
}()
let isHeuristicCollapse = lastUser.message.text.count > ChatBubble.heuristicUserCollapseCharacterThreshold
|| ChatBubble.exceedsLineLimit(lastUser.message.text, limit: ChatBubble.heuristicUserCollapseLineThreshold)
if isHeuristicCollapse {
// Heuristic-collapsed messages show truncated preview text
// (24 lines / 1,200 chars) + "Show more" button (30pt).
let previewText = ChatBubble.collapsedPreviewText(from: lastUser.message.text) as NSString
let previewRect = previewText.boundingRect(
with: NSSize(width: contentWidth, height: .greatestFiniteMagnitude),
options: [.usesLineFragmentOrigin, .usesFontLeading],
attributes: [.font: font]
)
let showMoreButton: CGFloat = 30
return ceil(previewRect.height) + cellOverhead + showMoreButton + attachmentHeight + markerHeight
}
// Non-collapsed messages render at full height
return textHeight + cellOverhead + attachmentHeight + markerHeight
}()
// Precise minHeight: fill the space between user message and composer.
// containerHeight = full chat pane (stable, from GeometryReader)
Expand Down