From 16ddf29e0143b2d710e86134dc75215c64a4fc2a Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 28 Apr 2026 22:56:08 +0000 Subject: [PATCH 1/3] fix(macos): isolate ChatView.body observation scope to prevent window-focus hang (LUM-1273) Co-Authored-By: ashlee@vellum.ai --- .../Features/Chat/ChatView.swift | 131 ++++++++++-------- 1 file changed, 73 insertions(+), 58 deletions(-) diff --git a/clients/macos/vellum-assistant/Features/Chat/ChatView.swift b/clients/macos/vellum-assistant/Features/Chat/ChatView.swift index 9a9f6bc4e69..aa7310b564d 100644 --- a/clients/macos/vellum-assistant/Features/Chat/ChatView.swift +++ b/clients/macos/vellum-assistant/Features/Chat/ChatView.swift @@ -137,15 +137,16 @@ struct ChatView: View { // children (808pt fallback) or when rapid drag updates are batched. GeometryReader { proxy in ZStack { - mainContentStack(containerWidth: proxy.size.width) - .background(alignment: .bottom) { - chatBackground - } - .background(VColor.surfaceBase) - .overlay(alignment: .bottom) { - btwOverlay - } - .animation(VAnimation.fast, value: viewModel.btwResponse != nil) + ObservationBoundaryView { + mainContentStack(containerWidth: proxy.size.width) + .background(alignment: .bottom) { + chatBackground + } + .background(VColor.surfaceBase) + } + .overlay(alignment: .bottom) { + BtwOverlayView(viewModel: viewModel) + } dropTargetOverlay } @@ -563,61 +564,12 @@ struct ChatView: View { } } - @Environment(\.colorScheme) private var colorScheme - @ViewBuilder private var chatBackground: some View { EmptyView() } - @ViewBuilder - private var btwOverlay: some View { - if let btwText = viewModel.btwResponse { - VStack(alignment: .leading, spacing: VSpacing.xs) { - HStack { - Text("/btw") - .font(VFont.labelDefault) - .foregroundStyle(VColor.contentTertiary) - Spacer() - Button(action: { viewModel.dismissBtw() }) { - VIconView(.x, size: 12) - .foregroundStyle(VColor.contentTertiary) - } - .buttonStyle(.plain) - .accessibilityLabel("Dismiss btw response") - } - if viewModel.btwLoading && btwText.isEmpty { - Text("Thinking...") - .font(VFont.bodyMediumLighter) - .foregroundStyle(VColor.contentTertiary) - } else if !viewModel.btwLoading && btwText.isEmpty { - Text("No response received.") - .font(VFont.bodyMediumLighter) - .foregroundStyle(VColor.contentTertiary) - } else { - Text(btwText) - .font(VFont.bodyMediumLighter) - .foregroundStyle(VColor.contentDefault) - .textSelection(.enabled) - } - - if !viewModel.btwLoading { - Text("Press Escape to dismiss") - .font(VFont.labelSmall) - .foregroundStyle(VColor.contentTertiary) - } - } - .padding(VSpacing.md) - .background(VColor.surfaceBase) - .cornerRadius(VRadius.md) - .vShadow(VShadow.sm) - .padding(.horizontal, VSpacing.lg) - .padding(.bottom, VSpacing.xxxl + VSpacing.xxl) - .transition(.opacity.combined(with: .move(edge: .bottom))) - .layoutHangSignpost("chat.btwOverlay") - } - } // MARK: - Internal Drag Detection @@ -744,6 +696,69 @@ struct ChatView: View { } } +// MARK: - BtwOverlayView + +/// Standalone View struct for the "/btw" response overlay, extracted from +/// `ChatView` to create a separate observation scope. With `@Observable`, +/// each View's `body` gets its own observation tracking context — property +/// reads inside this struct are tracked here, not in the parent `ChatView.body`. +/// This prevents `viewModel.btwResponse` / `viewModel.btwLoading` changes +/// from triggering a full `ChatView.body` re-evaluation. +/// +/// Ref: [WWDC23 — Discover Observation in SwiftUI](https://developer.apple.com/videos/play/wwdc2023/10149/) +private struct BtwOverlayView: View { + @Bindable var viewModel: ChatViewModel + + var body: some View { + if let btwText = viewModel.btwResponse { + VStack(alignment: .leading, spacing: VSpacing.xs) { + HStack { + Text("/btw") + .font(VFont.labelDefault) + .foregroundStyle(VColor.contentTertiary) + Spacer() + Button(action: { viewModel.dismissBtw() }) { + VIconView(.x, size: 12) + .foregroundStyle(VColor.contentTertiary) + } + .buttonStyle(.plain) + .accessibilityLabel("Dismiss btw response") + } + + if viewModel.btwLoading && btwText.isEmpty { + Text("Thinking...") + .font(VFont.bodyMediumLighter) + .foregroundStyle(VColor.contentTertiary) + } else if !viewModel.btwLoading && btwText.isEmpty { + Text("No response received.") + .font(VFont.bodyMediumLighter) + .foregroundStyle(VColor.contentTertiary) + } else { + Text(btwText) + .font(VFont.bodyMediumLighter) + .foregroundStyle(VColor.contentDefault) + .textSelection(.enabled) + } + + if !viewModel.btwLoading { + Text("Press Escape to dismiss") + .font(VFont.labelSmall) + .foregroundStyle(VColor.contentTertiary) + } + } + .padding(VSpacing.md) + .background(VColor.surfaceBase) + .cornerRadius(VRadius.md) + .vShadow(VShadow.sm) + .padding(.horizontal, VSpacing.lg) + .padding(.bottom, VSpacing.xxxl + VSpacing.xxl) + .transition(.opacity.combined(with: .move(edge: .bottom))) + .animation(VAnimation.fast, value: viewModel.btwResponse != nil) + .layoutHangSignpost("chat.btwOverlay") + } + } +} + // MARK: - Bootstrap Loading View /// Minimal loading panel shown during first-launch bootstrap while the From 50e6ad75ee63621c00227532b976e32582eca53e Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 28 Apr 2026 23:00:40 +0000 Subject: [PATCH 2/3] docs: clean up BtwOverlayView docstring, clarify @ViewBuilder observation scope in AGENTS.md Co-Authored-By: ashlee@vellum.ai --- clients/AGENTS.md | 2 +- .../vellum-assistant/Features/Chat/ChatView.swift | 11 ++++------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/clients/AGENTS.md b/clients/AGENTS.md index 0254902b52c..da08c28f923 100644 --- a/clients/AGENTS.md +++ b/clients/AGENTS.md @@ -194,7 +194,7 @@ Prefer migrating the parent to `@Observable` so the bridge becomes unnecessary ( - **View bodies must be lightweight.** Never perform I/O, network calls, or heavy computation inside a SwiftUI view body. This includes filtering, sorting, and data transformation — do that work in the ViewModel (asynchronously), not inline during render. Cheap display-time formatting (e.g., `Date.formatted(...)`, locale-aware unit strings) is fine in the view since it must react to locale/calendar changes. Defer expensive work to `task {}`, `onAppear`, or background actors. - **Lazy containers for large collections.** Use `LazyVStack`, `LazyHStack`, `LazyVGrid` instead of eager equivalents when the item count is unbounded or large. In particular, avoid `VStack`/`HStack` inside a `ScrollView` for large or unbounded data-driven lists — eager loading kills scroll performance. Small, fixed-size lists where visibility-sensitive logic (e.g., `onAppear` pagination triggers) matters may use eager containers intentionally. See `clients/macos/AGENTS.md` §§ "No `.frame(maxWidth:)` …" for the FlexFrame anti-pattern that compounds with lazy containers (mechanically enforced in CI via `clients/scripts/check-flexframe.sh`). - **Keep modifier chains lean.** Every SwiftUI modifier wraps its content in a new view value. Long chains of redundant or duplicated modifiers deepen the view tree and increase diffing cost. Group related modifiers, remove redundant ones (e.g., don't set `.font` on every row when the parent already sets it), and extract heavily-modified subtrees into standalone `@ViewBuilder` methods. -- **Scope observation narrowly.** Only observe the specific properties a view needs. Prefer granular `@Observable` properties or `withObservationTracking` over observing an entire store that publishes on unrelated changes. When a view reads `@Observable` properties from many different objects, extract logical sections (toolbar, overlays, content) into standalone child `View` structs — each child creates its own [observation tracking scope](https://developer.apple.com/videos/play/wwdc2023/10149/) so changes to one section's state don't re-evaluate the entire parent body. +- **Scope observation narrowly.** Only observe the specific properties a view needs. Prefer granular `@Observable` properties or `withObservationTracking` over observing an entire store that publishes on unrelated changes. When a view reads `@Observable` properties from many different objects, extract logical sections (toolbar, overlays, content) into standalone child `View` structs — each child creates its own [observation tracking scope](https://developer.apple.com/videos/play/wwdc2023/10149/) so changes to one section's state don't re-evaluate the entire parent body. **Note:** private `@ViewBuilder` functions do NOT create separate observation scopes — they execute during the caller's `body` evaluation, so all their `@Observable` reads are tracked in the caller's scope. Use standalone `View` structs or [`ObservationBoundaryView`](https://developer.apple.com/videos/play/wwdc2023/10149/) (deferred `@escaping @ViewBuilder` closure) when extraction into a full `View` struct is impractical. - **Use `.task` + async sequences instead of `.onReceive` for `NotificationCenter`.** Each `.onReceive` modifier adds a generic wrapper layer to the view type and exists only for side effects — no rendering value. Use a single [`.task`](https://developer.apple.com/documentation/swiftui/view/task(priority:_:)) with [`NotificationCenter.notifications(named:)`](https://developer.apple.com/documentation/foundation/notificationcenter/notifications(named:object:)) async sequences instead. `.task` has identical lifecycle semantics (auto-cancelled on view disappear) without inflating the view type. - **Enable non-contiguous layout on TextKit 1 stacks in `NSViewRepresentable`.** When constructing an `NSLayoutManager` for an `NSTextView` hosted via `NSViewRepresentable`, set `layoutManager.allowsNonContiguousLayout = true`. The default (`false`) forces full-document glyph layout from character 0 on the main thread whenever a glyph range is queried — which happens the first time the text view is attached to its hosting view (`setDocumentView:` / `addSubview:` → `_setSuperview:` → `setNeedsDisplayInRect:` → `_glyphRangeForBoundingRect:`). Reference: [`NSLayoutManager.allowsNonContiguousLayout`](https://developer.apple.com/documentation/appkit/nslayoutmanager/allowsnoncontiguouslayout). **Exception — streaming text with a separate measurement stack:** when the representable measures height via a sibling TextKit stack that always fully lays out every glyph (e.g. `ensureLayout(for:)` + `usedRect(for:)`), the measured frame assumes the render stack will also lay out every glyph in that rect. Non-contiguous layout leaves glyphs pending until the view scrolls or draws them, which races with streaming updates — producing a gap (render paints a smaller laid-out region inside the correct frame) and then overlap (the next sibling is placed via the measured frame and the lazy glyphs later paint outside it). In that case set `allowsNonContiguousLayout = false` and document why. See `clients/shared/DesignSystem/Components/Display/SelectableTextView.swift` (lines 180–205) for the canonical example; `VCodeView` / `HighlightedTextView` still opt into non-contiguous layout independently because they do not share this measurement model. - **Prefer line-count math over `ensureLayout(for:)` for `sizeThatFits` when line wrapping is disabled and line height is pinned.** If the TextKit 1 stack uses an unbounded-width `NSTextContainer` (so lines never wrap) and the paragraph style pins `minimumLineHeight == maximumLineHeight`, the total height is exactly `lineCount * lineHeight + insets` — no glyph layout required. This avoids an O(glyph count) main-thread layout pass that SwiftUI would otherwise re-run on every cell during `LazyVStack` layout. Reference: [`NSLayoutManager.defaultLineHeight(for:)`](https://developer.apple.com/documentation/appkit/nslayoutmanager/defaultlineheight(for:)). diff --git a/clients/macos/vellum-assistant/Features/Chat/ChatView.swift b/clients/macos/vellum-assistant/Features/Chat/ChatView.swift index aa7310b564d..812a8644c17 100644 --- a/clients/macos/vellum-assistant/Features/Chat/ChatView.swift +++ b/clients/macos/vellum-assistant/Features/Chat/ChatView.swift @@ -698,14 +698,11 @@ struct ChatView: View { // MARK: - BtwOverlayView -/// Standalone View struct for the "/btw" response overlay, extracted from -/// `ChatView` to create a separate observation scope. With `@Observable`, -/// each View's `body` gets its own observation tracking context — property -/// reads inside this struct are tracked here, not in the parent `ChatView.body`. -/// This prevents `viewModel.btwResponse` / `viewModel.btwLoading` changes -/// from triggering a full `ChatView.body` re-evaluation. +/// "/btw" response overlay with its own observation scope. /// -/// Ref: [WWDC23 — Discover Observation in SwiftUI](https://developer.apple.com/videos/play/wwdc2023/10149/) +/// As a standalone `View` struct, `@Observable` property reads +/// (`btwResponse`, `btwLoading`) are tracked in this view's body +/// rather than the parent's — narrowing the invalidation surface. private struct BtwOverlayView: View { @Bindable var viewModel: ChatViewModel From 9c28839f3e2697b629bd61df6d606e8ba2922826 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 28 Apr 2026 23:06:05 +0000 Subject: [PATCH 3/3] fix: move .animation outside conditional in BtwOverlayView to preserve removal transition Co-Authored-By: ashlee@vellum.ai --- .../Features/Chat/ChatView.swift | 80 ++++++++++--------- 1 file changed, 41 insertions(+), 39 deletions(-) diff --git a/clients/macos/vellum-assistant/Features/Chat/ChatView.swift b/clients/macos/vellum-assistant/Features/Chat/ChatView.swift index 812a8644c17..855545f19f3 100644 --- a/clients/macos/vellum-assistant/Features/Chat/ChatView.swift +++ b/clients/macos/vellum-assistant/Features/Chat/ChatView.swift @@ -707,52 +707,54 @@ private struct BtwOverlayView: View { @Bindable var viewModel: ChatViewModel var body: some View { - if let btwText = viewModel.btwResponse { - VStack(alignment: .leading, spacing: VSpacing.xs) { - HStack { - Text("/btw") - .font(VFont.labelDefault) - .foregroundStyle(VColor.contentTertiary) - Spacer() - Button(action: { viewModel.dismissBtw() }) { - VIconView(.x, size: 12) + Group { + if let btwText = viewModel.btwResponse { + VStack(alignment: .leading, spacing: VSpacing.xs) { + HStack { + Text("/btw") + .font(VFont.labelDefault) .foregroundStyle(VColor.contentTertiary) + Spacer() + Button(action: { viewModel.dismissBtw() }) { + VIconView(.x, size: 12) + .foregroundStyle(VColor.contentTertiary) + } + .buttonStyle(.plain) + .accessibilityLabel("Dismiss btw response") } - .buttonStyle(.plain) - .accessibilityLabel("Dismiss btw response") - } - if viewModel.btwLoading && btwText.isEmpty { - Text("Thinking...") - .font(VFont.bodyMediumLighter) - .foregroundStyle(VColor.contentTertiary) - } else if !viewModel.btwLoading && btwText.isEmpty { - Text("No response received.") - .font(VFont.bodyMediumLighter) - .foregroundStyle(VColor.contentTertiary) - } else { - Text(btwText) - .font(VFont.bodyMediumLighter) - .foregroundStyle(VColor.contentDefault) - .textSelection(.enabled) - } + if viewModel.btwLoading && btwText.isEmpty { + Text("Thinking...") + .font(VFont.bodyMediumLighter) + .foregroundStyle(VColor.contentTertiary) + } else if !viewModel.btwLoading && btwText.isEmpty { + Text("No response received.") + .font(VFont.bodyMediumLighter) + .foregroundStyle(VColor.contentTertiary) + } else { + Text(btwText) + .font(VFont.bodyMediumLighter) + .foregroundStyle(VColor.contentDefault) + .textSelection(.enabled) + } - if !viewModel.btwLoading { - Text("Press Escape to dismiss") - .font(VFont.labelSmall) - .foregroundStyle(VColor.contentTertiary) + if !viewModel.btwLoading { + Text("Press Escape to dismiss") + .font(VFont.labelSmall) + .foregroundStyle(VColor.contentTertiary) + } } + .padding(VSpacing.md) + .background(VColor.surfaceBase) + .cornerRadius(VRadius.md) + .vShadow(VShadow.sm) + .padding(.horizontal, VSpacing.lg) + .padding(.bottom, VSpacing.xxxl + VSpacing.xxl) + .transition(.opacity.combined(with: .move(edge: .bottom))) + .layoutHangSignpost("chat.btwOverlay") } - .padding(VSpacing.md) - .background(VColor.surfaceBase) - .cornerRadius(VRadius.md) - .vShadow(VShadow.sm) - .padding(.horizontal, VSpacing.lg) - .padding(.bottom, VSpacing.xxxl + VSpacing.xxl) - .transition(.opacity.combined(with: .move(edge: .bottom))) - .animation(VAnimation.fast, value: viewModel.btwResponse != nil) - .layoutHangSignpost("chat.btwOverlay") } + .animation(VAnimation.fast, value: viewModel.btwResponse != nil) } }