Skip to content
Merged
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
1 change: 1 addition & 0 deletions clients/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:)).
Expand Down
4 changes: 2 additions & 2 deletions clients/macos/vellum-assistant/Features/Chat/ChatView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,11 @@ struct ChatView: View {
@State private var diskPressureDismissalRefreshTask: Task<Void, Never>? = 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.
Expand Down
9 changes: 9 additions & 0 deletions clients/shared/Features/Chat/ChatPaginationState.swift
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ public final class ChatPaginationState {
/// Updated when either the message list or `displayedMessageCount` changes.
public private(set) var paginatedVisibleMessages: [ChatMessage] = []

/// 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

/// Timestamp of the oldest loaded message (ms since epoch). Used as the
Expand Down Expand Up @@ -173,6 +178,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
Expand Down
6 changes: 6 additions & 0 deletions clients/shared/Features/Chat/ChatViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1037,6 +1037,12 @@ public final class ChatViewModel: MessageSendCoordinatorDelegate {
paginationState.paginatedVisibleMessages
}

/// Whether `paginatedVisibleMessages` is empty. Prefer over
/// `paginatedVisibleMessages.isEmpty` to avoid observing the full array.
public var isPaginatedEmpty: Bool {
paginationState.isPaginatedEmpty
}

public var historyCursor: Double? {
get { paginationState.historyCursor }
set { paginationState.historyCursor = newValue }
Expand Down