Skip to content

perf(chat): replace maxHeight with definite height on ScrollViews inside LazyVStack cells #23611

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ashleeradka merged 6 commits into from
Apr 4, 2026
Merged

perf(chat): replace maxHeight with definite height on ScrollViews inside LazyVStack cells #23611

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
59406f7
perf(chat): replace maxHeight with definite height on ScrollViews ins…
ashleeradka Apr 4, 2026
dfe0823
fix: address Codex review — restore content-adaptive height for short…
ashleeradka Apr 4, 2026
2e28d64
fix: revert maxHeight to height for small ScrollViews — cell height c…
ashleeradka Apr 4, 2026
1a8781f
fix: add lineCount > 500 guard to ToolConfirmationBubble.codePreviewB…
ashleeradka Apr 4, 2026
73e5363
fix: use VCodeView.countLines in ToolConfirmationBubble for consistency
ashleeradka Apr 4, 2026
1ea0fe2
refactor: extract adaptiveScrollFrame modifier for LazyVStack height …
ashleeradka Apr 4, 2026
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
25 changes: 25 additions & 0 deletions clients/shared/DesignSystem/Modifiers/LazyVStackScrollFrameModifier.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import SwiftUI

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.
///
/// - 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.
func adaptiveScrollFrame(

@devin-ai-integration devin-ai-integration Bot Apr 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Design system modifier .adaptiveScrollFrame() missing required v prefix

The clients/AGENTS.md mandates: "All design system types — structs, enums, and view modifiers — must use the V prefix" with examples .vCard(), .vTooltip(), .vShimmer(), .vPanelBackground(). The new modifier .adaptiveScrollFrame() is defined in clients/shared/DesignSystem/Modifiers/LazyVStackScrollFrameModifier.swift and does not follow this naming convention. It should be named .vAdaptiveScrollFrame() (or similar v-prefixed name). All three call sites (ToolCallChip.swift:232, ToolCallProgressBar.swift:253, ToolConfirmationBubble.swift:325) would need updating.

Suggested change
func adaptiveScrollFrame(
func vAdaptiveScrollFrame(
Open in Devin Review

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

for text: String,
maxHeight: CGFloat,
lineThreshold: Int = 500
) -> some View {
let isLong = VCodeView.countLines(in: text) > lineThreshold

@devin-ai-integration devin-ai-integration Bot Apr 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 adaptiveScrollFrame recalculates line count O(n) on every render, bypassing the existing cachedResultLineCount

In ToolCallChip.swift, the cachedResultLineCount cache was specifically introduced to avoid O(n) VCodeView.countLines(in:) on every SwiftUI render pass (see the doc comment at line 20-22). The lineCount variable at line 221 correctly uses this cache. However, .adaptiveScrollFrame(for: result, maxHeight: 400) at line 232 internally calls VCodeView.countLines(in: text) again (LazyVStackScrollFrameModifier.swift:20), completely bypassing the cache on every body evaluation. For large tool call results (thousands of lines of terminal output), this reintroduces the O(n) per-render cost that the cache was designed to eliminate. The modifier should accept a pre-computed line count or isLong flag instead of recalculating it.

Prompt for agents 
The adaptiveScrollFrame modifier calls VCodeView.countLines(in: text) every time it is evaluated. In ToolCallChip.swift, there is an explicit cachedResultLineCount @State cache (line 22) that was created to avoid this O(n) computation on every render. The modifier bypasses this cache because it accepts the raw text and recomputes the line count internally.

Two possible fixes:
1. Add an overload that accepts a pre-computed line count (or isLong Bool) so callers with caches can pass the cached value.
2. Change the modifier to accept a Binding or let the caller pass lineCount directly, e.g. func adaptiveScrollFrame(isLong: Bool, maxHeight: CGFloat).

In ToolCallChip.swift line 232, the caller already has lineCount at line 221 which uses the cache. It should pass this value to the modifier rather than having the modifier recompute it.

Also affects ToolCallProgressBar.swift:253 and ToolConfirmationBubble.swift:325 where there is no cache — these introduce new O(n) per-render work that wasn't present before.
Open in Devin Review

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

return self
.frame(height: isLong ? maxHeight : nil)
.frame(maxHeight: isLong ? nil : maxHeight)
}
}
28 changes: 6 additions & 22 deletions clients/shared/Features/Chat/ToolCallChip.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,6 @@ public struct ToolCallChip: View {
return Int(matched[numRange])
}

/// Count lines in a string without allocating an intermediate array.
static func countLines(in text: String) -> Int {
var count = 1
for byte in text.utf8 where byte == UInt8(ascii: "\n") {
count += 1
}
return count
}

/// Whether the tool produces diff-formatted output that benefits from line-level highlighting.
static func isFileEditTool(_ name: String) -> Bool {
switch name {
Expand Down Expand Up @@ -227,25 +218,18 @@ public struct ToolCallChip: View {
.foregroundStyle(VColor.contentSecondary)
}
} else {
let lineCount = cachedResultLineCount ?? Self.countLines(in: result)
let lineCount = cachedResultLineCount ?? VCodeView.countLines(in: result)
if Self.isFileEditTool(toolCall.toolName) {
VDiffView(result, maxHeight: lineCount > 500 ? 400 : nil)
} else if lineCount > 500 {
} else {
ScrollView {
Text(result)
.font(VFont.bodySmallDefault)
.foregroundStyle(VColor.contentSecondary)
.frame(maxWidth: .infinity, alignment: .leading)
.textSelection(.enabled)
}
.frame(maxHeight: 400)
} else {
Text(result)
.font(VFont.bodySmallDefault)
.foregroundStyle(VColor.contentSecondary)
.frame(maxWidth: .infinity, alignment: .leading)
.textSelection(.enabled)
.fixedSize(horizontal: false, vertical: true)
.adaptiveScrollFrame(for: result, maxHeight: 400)
}
Comment on lines 229 to 233

@devin-ai-integration devin-ai-integration Bot Apr 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🚩 Short tool results now always wrapped in ScrollView — behavioral change from plain Text

The old code in ToolCallChip.swift had three branches for non-exit-code, non-command-completed results: (1) file edit → VDiffView, (2) lineCount > 500 → ScrollView with maxHeight, (3) else → plain Text with .fixedSize(horizontal: false, vertical: true). The new code collapses branches 2 and 3 into a single ScrollView with .adaptiveScrollFrame. For short results (≤500 lines), the old code rendered a plain Text with .fixedSize(horizontal: false, vertical: true) which guaranteed the text expanded to its natural height. The new code wraps it in a ScrollView with .frame(maxHeight: 400). In most layout contexts (VStack, LazyVStack), a ScrollView with maxHeight should still collapse to content height, so this is likely fine. However, if the enclosing layout proposes a large height, the ScrollView could be taller than the content, leaving blank space below the text. This is a subtle behavioral change worth verifying visually with short (1-5 line) tool outputs.

(Refers to lines 224-233)

Open in Devin Review

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

}
}
Expand All @@ -266,7 +250,7 @@ public struct ToolCallChip: View {
}
// Cache the result line count so subsequent renders are O(1).
if cachedResultLineCount == nil, let result = toolCall.result {
cachedResultLineCount = Self.countLines(in: result)
cachedResultLineCount = VCodeView.countLines(in: result)
}
// Trigger on-demand rehydration when expanding truncated content.
onRehydrate?()
Expand Down Expand Up @@ -299,7 +283,7 @@ public struct ToolCallChip: View {
}
}
if cachedResultLineCount == nil, let result = toolCall.result {
cachedResultLineCount = Self.countLines(in: result)
cachedResultLineCount = VCodeView.countLines(in: result)
}
}
}
Expand All @@ -310,7 +294,7 @@ public struct ToolCallChip: View {
}
.onChange(of: toolCall.result) {
if isExpanded, let result = toolCall.result {
cachedResultLineCount = Self.countLines(in: result)
cachedResultLineCount = VCodeView.countLines(in: result)
} else {
cachedResultLineCount = nil
}
Expand Down
2 changes: 1 addition & 1 deletion clients/shared/Features/Chat/ToolCallProgressBar.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ public struct ToolCallProgressBar: View {
.frame(maxWidth: .infinity, alignment: .leading)
.textSelection(.enabled)
}
.frame(maxHeight: 200)
.adaptiveScrollFrame(for: result, maxHeight: 200)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion clients/shared/Features/Chat/ToolConfirmationBubble.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ public struct ToolConfirmationBubble: View {
.frame(maxWidth: .infinity, alignment: .leading)
.textSelection(.enabled)
}
.frame(maxHeight: maxHeight)
.adaptiveScrollFrame(for: content, maxHeight: maxHeight)
.padding(VSpacing.sm)
.frame(maxWidth: .infinity, alignment: .leading)
.background(
Expand Down