LUM-799: Replace GeometryReader with .onGeometryChange(for:) in ChatView#24423
Merged
ashleeradka merged 2 commits intoApr 8, 2026
Merged
Conversation
Contributor
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
cd0cf90 to
d63db85
Compare
GeometryReader at ChatView.swift:124 wrapped the entire chat view hierarchy as a layout container, forcing full O(n × depth) measurement cascades through LazyVStack on every attribute graph dirty (hover, window focus, animation tick), causing 48-83 second hangs. Replace with @State containerWidth + .onGeometryChange(for:) modifier, which observes geometry after layout completes without creating a layout node that participates in measurement. The action closure only fires when the extracted width value actually changes (built-in deduplication via Equatable), so hover/focus events cause zero state writes. Data flow is unchanged: containerWidth flows through mainContentStack(containerWidth:) -> activeConversationContent -> MessageListView -> bubbleMaxWidth environment key. Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
d63db85 to
bfbff69
Compare
ashleeradka
previously approved these changes
Apr 8, 2026
Contributor
ashleeradka
left a comment
There was a problem hiding this comment.
✦ Reviewed ChatView.swift in full context.
GeometryReader removed, replaced with @State containerWidth + .onGeometryChange(for: CGFloat.self). containerWidth=0 initial state handled gracefully — existing ternary in MessageListContentView falls back to VSpacing.chatBubbleMaxWidth until first callback. Same behavior as before.
Clean single-file change, no conflicts with main.
Ship it.
With @State containerWidth starting at 0, the first .onGeometryChange callback triggers handleContainerWidthChanged() which would start a spurious 100ms resize stabilization during initial load. This never happened with GeometryReader because containerWidth was always the real width from the first render. Add an early return when lastHandledContainerWidth is 0, recording the width without triggering stabilization. Subsequent real resizes still trigger normally. Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
ashleeradka
approved these changes
Apr 8, 2026
devin-ai-integration Bot
added a commit
that referenced
this pull request
Apr 9, 2026
Replace .frame(maxWidth:).frame(maxWidth: .infinity) patterns with .frame(width: min(containerWidth, maxWidth)) in ChatView banners, ComposerView, and ChatLoadingSkeleton. These FlexFrame modifiers in the VStack siblings of MessageListView triggered explicitAlignment queries that cascaded through _FrameLayout into the LazyVStack subtree, causing 77s hangs (Spindump-4, build BE7C2D43). Changes: - ChatView: Replace 3 banner .frame(maxWidth:) pairs with .frame(width:) - ChatView: Replace read-only label .frame(maxWidth: .infinity) with Spacer centering - ComposerView: Replace .frame(maxWidth:).frame(maxWidth: .infinity) with .frame(width:) - ComposerSection: Thread containerWidth from ChatView - ChatLoadingSkeleton: Accept containerWidth and use .frame(width:) containerWidth is already captured via .onGeometryChange in ChatView (PR #24423) and threaded through activeConversationContent. This extends that flow to ComposerSection -> ComposerView and ChatLoadingSkeleton. Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
5 tasks
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the
GeometryReaderwrapper inChatView.bodywith a@Stateproperty +.onGeometryChange(for:)modifier to eliminate 48–83 second performance hangs.GeometryReaderas a layout container creates aGeometryReaderLayoutnode whose child measurement runs duringplaceSubviews. Any attribute graph invalidation (hover, window focus, animation tick) forces a full O(n × depth) re-measurement cascade through theLazyVStack..onGeometryChangeis a post-layout observer — it reads geometry after layout completes without participating in the measurement chain, and its action only fires when the extracted value actually changes (Apple docs).The downstream data flow is unchanged:
containerWidthstill flows throughmainContentStack→activeConversationContent→MessageListView→bubbleMaxWidthenvironment key.Because
@State containerWidthinitializes to0(unlikeGeometryReaderwhich provided the real width from the first render), the first.onGeometryChangecallback produces a 0→real transition. Without mitigation, this would trigger spurious scroll resize stabilization during initial load. A guard inhandleContainerWidthChanged()now absorbs this first measurement (whenlastHandledContainerWidth == 0) by recording the width without starting stabilization. Subsequent real resizes still trigger normally.Review & Testing Checklist for Human
lastHandledContainerWidth > 0guard skips stabilization on first mount, so verify this doesn't regress the initial-load scroll-to-bottom behaviorhandleConversationSwitched()seedslastHandledContainerWidth = containerWidth— confirm this still works when the width was previously recordedRecommended test plan: Open a long conversation, resize the window several times (including very narrow), hover over messages, switch window focus, switch conversations, and verify no hangs or scroll position issues.
Notes
.onGeometryChange(for:of:action:)requires macOS 14+; deployment target is macOS 15.0 — safebf7d13875(PR refactor: ChatView uses GeometryReader instead of @State for container width #23855) which introduced theGeometryReader, contradicting AGENTS.md guidance. The@Statewrite concern that motivated that commit is moot because.onGeometryChangededuplicates viaEquatable.onGeometryChangefires with the real width — verify whether this causes a visible layout jumpLink to Devin session: https://app.devin.ai/sessions/14a69bbf31b145c987e81d2aee0c2612
Requested by: @ashleeradka