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
7 changes: 1 addition & 6 deletions clients/macos/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -259,12 +259,7 @@ All design system types use the `V` prefix (VButton, VColor, VFont, etc.). Alway
- [`.containerRelativeFrame(.horizontal)`](https://developer.apple.com/documentation/swiftui/view/containerrelativeframe(_:alignment:)) — width constraint without FlexFrame.

Never trade `HStack+Spacer` for `.frame(alignment:)` in lazy containers — fewer layout nodes is not worth O(n) recursive alignment queries per node.
- **`AlignmentBarrierLayout` for LazyVStack protection**: Wrap the `ScrollView` (from the **outside**) with `AlignmentBarrierLayout { ScrollView { ... } }` to block `explicitAlignment` queries from cascading into the lazy container. The barrier returns `nil` for all alignment queries while passing through sizing and placement unchanged. Three critical constraints:
1. **The barrier MUST wrap the ScrollView, not its content.** Placing a custom `Layout` _inside_ a `ScrollView` (between the `ScrollView` and a `LazyVStack`) breaks `LazyVStack`'s viewport tracking — cells fail to materialize on resize, send, or content changes until the user scrolls. Custom `Layout` containers disrupt the internal communication channel `ScrollView` uses to tell `LazyVStack` which cells are visible.
2. **`.id()` MUST be outside the barrier, not inside.** A custom `Layout` container does not propagate child identity changes to the outer modifier chain. If `.id(conversationId)` is inside the `Layout`, `.onAppear`/`.onDisappear` on the outer chain will not fire on identity changes (e.g. conversation switch), breaking lifecycle handlers like `handleAppear()`. Place `.id()` after the barrier in the modifier chain: `AlignmentBarrierLayout { ScrollView { ... } }.id(conversationId)`.
3. **Scroll-specific modifiers MUST be inside the barrier, on the ScrollView.** `.scrollPosition`, `.defaultScrollAnchor`, `.onScrollPhaseChange`, and `.scrollIndicators` must be applied directly to the `ScrollView` inside the barrier's content closure. Placing them outside forces SwiftUI to traverse the custom `Layout` container to find the `ScrollView`, which can intermittently fail and cause cells to not materialize.

See [`Layout.explicitAlignment`](https://developer.apple.com/documentation/swiftui/layout/explicitalignment(of:in:proposal:subviews:cache:)-3iqmu) and `clients/shared/DesignSystem/Layout/AlignmentBarrierLayout.swift`.
- **Use `.frame(width:)` (not `.frame(maxWidth:)`) on ScrollView containers with LazyVStack**: `.frame(width:)` creates [`_FrameLayout`](https://developer.apple.com/documentation/swiftui/view/frame(width:height:alignment:)) which returns `bounds.midX` for alignment without querying children — the alignment cascade stops here. `.frame(maxWidth:)` creates [`_FlexFrameLayout`](https://developer.apple.com/documentation/swiftui/view/frame(minwidth:idealwidth:maxwidth:minheight:idealheight:maxheight:alignment:)) which queries `explicitAlignment` on children, cascading O(n) through the entire `LazyVStack` subtree on every layout pass. Use a computed width (e.g. `min(containerWidth, maxWidth)`) to get responsive behavior without `_FlexFrameLayout`. Do NOT use a custom `Layout` barrier — custom `Layout` containers between the outer modifier chain and the `ScrollView` disrupt SwiftUI's internal scroll infrastructure, causing intermittent cell materialization failures.
- **Gallery**: When adding or modifying a design system primitive/component, update the corresponding Gallery section file (`Gallery/Sections/`) so the visual catalog stays current.
- **Accessibility**: See `clients/AGENTS.md` § [Accessibility](../AGENTS.md#accessibility) for the full checklist (labels, hidden elements, custom interactions, AppKit panels). All rules there apply to macOS components.

Expand Down
105 changes: 44 additions & 61 deletions clients/macos/vellum-assistant/Features/Chat/MessageListView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -110,70 +110,53 @@ struct MessageListView: View {
#if DEBUG
let _ = os_signpost(.event, log: PerfSignposts.log, name: "MessageListView.body")
#endif
// AlignmentBarrierLayout stops explicitAlignment queries from the
// outer .frame(maxWidth:) modifiers from cascading into the
// LazyVStack. It MUST wrap the ScrollView from the outside —
// placing it inside the ScrollView breaks LazyVStack's viewport
// tracking, causing cells to not materialize until scrolled.
// See AGENTS.md and AlignmentBarrierLayout.swift for details.
//
// Scroll-specific modifiers (.scrollPosition, .defaultScrollAnchor,
// .onScrollPhaseChange, .scrollIndicators) are applied INSIDE the
// barrier directly on the ScrollView. Placing them outside forces
// SwiftUI to traverse a custom Layout container to find the
// ScrollView, which can intermittently fail and cause cells to
// not materialize.
AlignmentBarrierLayout {
ScrollView {
scrollViewContent
.background(
MessageListScrollObserver { newState in
enqueueScrollGeometryUpdate(newState)
}
)
}
.scrollContentBackground(.hidden)
.scrollDisabled(messages.isEmpty && !isSending)
// Apply only to .initialOffset — where the scroll view starts
// when first displayed (including .id() recreation on switch).
// Deliberately NOT using the all-roles overload (.sizeChanges)
// because it fights user scroll-up during streaming: SwiftUI's
// definition of "at bottom" for anchor purposes can differ from
// our hysteresis-based isAtBottom, causing the viewport to snap
// back to bottom on every content-height change even after the
// user has entered freeBrowsing. Our explicit content-height
// auto-follow handles streaming growth with proper mode checks.
// https://developer.apple.com/documentation/swiftui/view/defaultscrollanchor(_:for:)
.defaultScrollAnchor(.bottom, for: .initialOffset)
.scrollPosition($scrollPosition)
.environment(\.suppressAutoScroll, { [self] in
os_signpost(.event, log: PerfSignposts.log, name: "scrollSuppressionChanged", "on reason=manualExpansionDetach")
let intents = scrollCoordinator.handle(.manualExpansion)
executeCoordinatorIntents(intents)
// Keep scrollState in sync as runtime executor.
scrollState.handleManualExpansionInteraction()
})
.onScrollPhaseChange { oldPhase, newPhase in
let coordinatorPhase = ScrollCoordinator.Phase.from(newPhase)
let intents = scrollCoordinator.handle(.scrollPhaseChanged(phase: coordinatorPhase))
executeCoordinatorIntents(intents)
// Keep scrollState in sync as runtime executor.
scrollState.scrollPhase = newPhase
if newPhase == .idle && oldPhase != .idle && scrollState.isAtBottom {
scrollState.handleReachedBottom()
}
// .frame(width:) creates _FrameLayout (not _FlexFrameLayout). FrameLayout
// returns bounds.midX for alignment without querying children, stopping the
// alignment cascade. The old .frame(maxWidth:) pattern created FlexFrameLayout
// which queried explicitAlignment on the entire LazyVStack subtree — O(n) per
// layout pass, causing 34-70s hangs. See AGENTS.md.
ScrollView {
scrollViewContent
.background(
MessageListScrollObserver { newState in
enqueueScrollGeometryUpdate(newState)
}
)
}
.scrollContentBackground(.hidden)
.scrollDisabled(messages.isEmpty && !isSending)
// Apply only to .initialOffset — where the scroll view starts
// when first displayed (including .id() recreation on switch).
// Deliberately NOT using the all-roles overload (.sizeChanges)
// because it fights user scroll-up during streaming: SwiftUI's
// definition of "at bottom" for anchor purposes can differ from
// our hysteresis-based isAtBottom, causing the viewport to snap
// back to bottom on every content-height change even after the
// user has entered freeBrowsing. Our explicit content-height
// auto-follow handles streaming growth with proper mode checks.
// https://developer.apple.com/documentation/swiftui/view/defaultscrollanchor(_:for:)
.defaultScrollAnchor(.bottom, for: .initialOffset)
.scrollPosition($scrollPosition)
.environment(\.suppressAutoScroll, { [self] in
os_signpost(.event, log: PerfSignposts.log, name: "scrollSuppressionChanged", "on reason=manualExpansionDetach")
let intents = scrollCoordinator.handle(.manualExpansion)
executeCoordinatorIntents(intents)
// Keep scrollState in sync as runtime executor.
scrollState.handleManualExpansionInteraction()
})
.onScrollPhaseChange { oldPhase, newPhase in
let coordinatorPhase = ScrollCoordinator.Phase.from(newPhase)
let intents = scrollCoordinator.handle(.scrollPhaseChanged(phase: coordinatorPhase))
executeCoordinatorIntents(intents)
// Keep scrollState in sync as runtime executor.
scrollState.scrollPhase = newPhase
if newPhase == .idle && oldPhase != .idle && scrollState.isAtBottom {
scrollState.handleReachedBottom()
}
.scrollIndicators(scrollState.scrollIndicatorsHidden ? .hidden : .automatic)
}
// .id() MUST be outside AlignmentBarrierLayout. A custom Layout
// container does not propagate child identity changes to the outer
// modifier chain — .onAppear would not fire on conversation switch
// if .id() were inside the Layout, breaking handleAppear() and all
// scroll state that depends on it (configureScrollCallbacks,
// handleConversationSwitched). See MessageListView+Lifecycle.swift.
.scrollIndicators(scrollState.scrollIndicatorsHidden ? .hidden : .automatic)
.id(conversationId)
.frame(maxWidth: VSpacing.chatColumnMaxWidth)
.frame(maxWidth: .infinity)
.frame(width: containerWidth > 0 ? min(containerWidth, VSpacing.chatColumnMaxWidth) : VSpacing.chatColumnMaxWidth)
.overlay(alignment: .bottom) {
ScrollToLatestOverlayView(scrollState: scrollState)
}
Expand Down
51 changes: 0 additions & 51 deletions clients/shared/DesignSystem/Layout/AlignmentBarrierLayout.swift

This file was deleted.