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
2 changes: 1 addition & 1 deletion clients/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:)).
Expand Down
130 changes: 72 additions & 58 deletions clients/macos/vellum-assistant/Features/Chat/ChatView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -744,6 +696,68 @@ struct ChatView: View {
}
}

// MARK: - BtwOverlayView

/// "/btw" response overlay with its own observation scope.
///
/// 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

var body: some View {
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")
}

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")
}
}
.animation(VAnimation.fast, value: viewModel.btwResponse != nil)
}
}

// MARK: - Bootstrap Loading View

/// Minimal loading panel shown during first-launch bootstrap while the
Expand Down