From 5b0a76e607bca29dfeeacc72d8af3831a43ad258 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sat, 2 May 2026 01:31:17 +0000 Subject: [PATCH 1/3] perf(chat): decouple ChatView outer-body observation from paginatedVisibleMessages (LUM-1330) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add isPaginatedEmpty stored boolean to ChatPaginationState, updated via write-only-on-change guard in recomputePaginatedSuffix(). ChatView's isEmptyState/shouldShowSkeleton now read this boolean instead of accessing the full paginatedVisibleMessages array. This removes the outer ChatView.body observation dependency on the high-frequency array property, allowing the existing ObservationBoundaryView (from LUM-1273) to actually isolate message-list observation. Previously, shouldShowSkeleton — evaluated in .onChange(of:) outside the boundary — tracked the array, causing outer body re-evaluation on every message mutation during streaming. Closes LUM-1330 Co-Authored-By: ashlee@vellum.ai --- .../vellum-assistant/Features/Chat/ChatView.swift | 4 ++-- .../shared/Features/Chat/ChatPaginationState.swift | 11 +++++++++++ clients/shared/Features/Chat/ChatViewModel.swift | 7 +++++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/clients/macos/vellum-assistant/Features/Chat/ChatView.swift b/clients/macos/vellum-assistant/Features/Chat/ChatView.swift index 6c9f304bd88..dcadc89e2d5 100644 --- a/clients/macos/vellum-assistant/Features/Chat/ChatView.swift +++ b/clients/macos/vellum-assistant/Features/Chat/ChatView.swift @@ -112,11 +112,11 @@ struct ChatView: View { @State private var diskPressureDismissalRefreshTask: Task? = nil private var isEmptyState: Bool { - viewModel.paginatedVisibleMessages.isEmpty && viewModel.isHistoryLoaded + viewModel.isPaginatedEmpty && viewModel.isHistoryLoaded } private var shouldShowSkeleton: Bool { - viewModel.paginatedVisibleMessages.isEmpty && !viewModel.isHistoryLoaded + viewModel.isPaginatedEmpty && !viewModel.isHistoryLoaded } /// Message IDs whose text contains the search query, ordered chronologically. diff --git a/clients/shared/Features/Chat/ChatPaginationState.swift b/clients/shared/Features/Chat/ChatPaginationState.swift index b2ab8ed6cef..aaa6a62831c 100644 --- a/clients/shared/Features/Chat/ChatPaginationState.swift +++ b/clients/shared/Features/Chat/ChatPaginationState.swift @@ -71,6 +71,13 @@ public final class ChatPaginationState { /// Updated when either the message list or `displayedMessageCount` changes. public private(set) var paginatedVisibleMessages: [ChatMessage] = [] + /// Whether the paginated visible messages array is empty. Maintained as a + /// separate stored property so views that only need emptiness (e.g. skeleton + /// and empty-state routing in ChatView) can observe this O(1) boolean + /// without tracking the full array — preventing unnecessary view + /// invalidation on every message mutation during streaming. + public private(set) var isPaginatedEmpty: Bool = true + // MARK: - Daemon History Pagination /// Timestamp of the oldest loaded message (ms since epoch). Used as the @@ -173,6 +180,10 @@ public final class ChatPaginationState { /// `windowOldestIndex` (or the newest slice when that is `nil`); all /// other cases fall back to a grow-only suffix. func recomputePaginatedSuffix() { + defer { + let newEmpty = paginatedVisibleMessages.isEmpty + if newEmpty != isPaginatedEmpty { isPaginatedEmpty = newEmpty } + } let visible = displayedMessages if isShowAllMode { let cap = Self.maxPaginatedWindowSize diff --git a/clients/shared/Features/Chat/ChatViewModel.swift b/clients/shared/Features/Chat/ChatViewModel.swift index 895d134b2ef..130ead07f22 100644 --- a/clients/shared/Features/Chat/ChatViewModel.swift +++ b/clients/shared/Features/Chat/ChatViewModel.swift @@ -1037,6 +1037,13 @@ public final class ChatViewModel: MessageSendCoordinatorDelegate { paginationState.paginatedVisibleMessages } + /// Whether the paginated visible messages are empty. Views that only need + /// emptiness routing (skeleton/empty-state) should read this instead of + /// `paginatedVisibleMessages.isEmpty` to avoid tracking the full array. + public var isPaginatedEmpty: Bool { + paginationState.isPaginatedEmpty + } + public var historyCursor: Double? { get { paginationState.historyCursor } set { paginationState.historyCursor = newValue } From 75564447d31b1279820d13c7e74c379768367b8c Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sat, 2 May 2026 01:39:43 +0000 Subject: [PATCH 2/3] docs: trim docstrings for conciseness Co-Authored-By: ashlee@vellum.ai --- clients/shared/Features/Chat/ChatPaginationState.swift | 8 +++----- clients/shared/Features/Chat/ChatViewModel.swift | 5 ++--- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/clients/shared/Features/Chat/ChatPaginationState.swift b/clients/shared/Features/Chat/ChatPaginationState.swift index aaa6a62831c..c75e91874e2 100644 --- a/clients/shared/Features/Chat/ChatPaginationState.swift +++ b/clients/shared/Features/Chat/ChatPaginationState.swift @@ -71,11 +71,9 @@ public final class ChatPaginationState { /// Updated when either the message list or `displayedMessageCount` changes. public private(set) var paginatedVisibleMessages: [ChatMessage] = [] - /// Whether the paginated visible messages array is empty. Maintained as a - /// separate stored property so views that only need emptiness (e.g. skeleton - /// and empty-state routing in ChatView) can observe this O(1) boolean - /// without tracking the full array — preventing unnecessary view - /// invalidation on every message mutation during streaming. + /// Whether `paginatedVisibleMessages` is empty. Cached O(1) boolean so + /// views needing only emptiness (skeleton/empty-state routing) observe this + /// instead of the full array — avoiding invalidation on every message mutation. public private(set) var isPaginatedEmpty: Bool = true // MARK: - Daemon History Pagination diff --git a/clients/shared/Features/Chat/ChatViewModel.swift b/clients/shared/Features/Chat/ChatViewModel.swift index 130ead07f22..4577220a30d 100644 --- a/clients/shared/Features/Chat/ChatViewModel.swift +++ b/clients/shared/Features/Chat/ChatViewModel.swift @@ -1037,9 +1037,8 @@ public final class ChatViewModel: MessageSendCoordinatorDelegate { paginationState.paginatedVisibleMessages } - /// Whether the paginated visible messages are empty. Views that only need - /// emptiness routing (skeleton/empty-state) should read this instead of - /// `paginatedVisibleMessages.isEmpty` to avoid tracking the full array. + /// Whether `paginatedVisibleMessages` is empty. Prefer over + /// `paginatedVisibleMessages.isEmpty` to avoid observing the full array. public var isPaginatedEmpty: Bool { paginationState.isPaginatedEmpty } From 9307b17ec3569a933a59822791ca9a530ebdae3b Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sat, 2 May 2026 01:40:41 +0000 Subject: [PATCH 3/3] docs(agents): add guideline for caching derived booleans from high-frequency collections Co-Authored-By: ashlee@vellum.ai --- clients/AGENTS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/clients/AGENTS.md b/clients/AGENTS.md index 0bfd15c22d6..7f1c3b1137d 100644 --- a/clients/AGENTS.md +++ b/clients/AGENTS.md @@ -197,6 +197,7 @@ Prefer migrating the parent to `@Observable` so the bridge becomes unnecessary ( - **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. **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. +- **Cache derived booleans from high-frequency collections.** Reading `.isEmpty` or `.count` on an `@Observable` stored property [tracks the property, not the value](https://developer.apple.com/videos/play/wwdc2023/10149/) — any mutation invalidates the view even when the derived boolean hasn't changed. For frequently-mutated collections, maintain a separate stored boolean with a write-only-on-change guard (`if new != old { prop = new }`). This also applies to modifiers like `.onChange(of:)` — if the `of:` expression reads a collection property, the outer body observes it. - **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:)).