From 618e2ccb67e53f42b4b1ec979c4748f80dad3d19 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 28 Apr 2026 14:26:19 +0000 Subject: [PATCH 1/3] fix(macos): decouple VSelectableTextView container from frame on external-sizing path (LUM-1170) Closes LUM-1170 Co-Authored-By: ashlee@vellum.ai --- .../Display/SelectableTextView.swift | 70 +++++++++++++++++-- 1 file changed, 66 insertions(+), 4 deletions(-) diff --git a/clients/shared/DesignSystem/Components/Display/SelectableTextView.swift b/clients/shared/DesignSystem/Components/Display/SelectableTextView.swift index c3392911410..b2bbc119331 100644 --- a/clients/shared/DesignSystem/Components/Display/SelectableTextView.swift +++ b/clients/shared/DesignSystem/Components/Display/SelectableTextView.swift @@ -204,11 +204,35 @@ public struct VSelectableTextView: NSViewRepresentable { // VCodeView / HighlightedTextView still opt in to non-contiguous // layout independently for their own scroll-attachment perf fix. textStorage.addLayoutManager(layoutManager) + + // The container is the text wrapping surface. Its width determines + // where lines break; its configured size is what the layout manager + // lays glyphs into. Whether the container follows the NSTextView's + // frame depends on `widthTracksTextView` / `heightTracksTextView`. + // + // Two sizing paths: + // 1. `useExternalSizing: true` — the caller precomputes size via + // `measureSize` and applies an explicit `.frame(width:height:)`. + // We must decouple the container from the frame so that when + // SwiftUI calls `setFrameSize` on the NSTextView (e.g. during a + // window resize), AppKit does not interpret it as a container- + // width change and invoke `_fillLayoutHoleForCharacterRange` to + // re-lay out every glyph on the main thread — an O(n) cascade + // that produces multi-second hangs on large messages. Size the + // container explicitly from `maxWidth`; propagate changes in + // `updateNSView`. Peer components `VCodeView` and + // `HighlightedTextView` follow the same pattern. + // 2. `useExternalSizing: false` — our `sizeThatFits` drives layout + // and the container tracks the view frame as NSTextView's + // default behavior intends. Used by the Gallery preview, where + // there is only a single instance and no resize storm. + let initialContainerWidth = useExternalSizing ? (maxWidth ?? CGFloat.greatestFiniteMagnitude) : 0 let textContainer = NSTextContainer(size: NSSize( - width: 0, + width: initialContainerWidth, height: CGFloat.greatestFiniteMagnitude )) - textContainer.widthTracksTextView = true + textContainer.widthTracksTextView = !useExternalSizing + textContainer.heightTracksTextView = false textContainer.lineFragmentPadding = 0 layoutManager.addTextContainer(textContainer) @@ -220,8 +244,22 @@ public struct VSelectableTextView: NSViewRepresentable { textView.backgroundColor = .clear textView.drawsBackground = false textView.isVerticallyResizable = true - textView.isHorizontallyResizable = false - textView.autoresizingMask = [.width] + // Allow the text view to grow independently of its frame when + // `useExternalSizing` is true. This reinforces the container + // decoupling above: if `setFrameSize` is invoked with a width + // smaller than the current content, AppKit will not shrink the + // container and trigger a re-layout of all glyphs. + textView.isHorizontallyResizable = useExternalSizing + if useExternalSizing { + textView.maxSize = NSSize( + width: CGFloat.greatestFiniteMagnitude, + height: CGFloat.greatestFiniteMagnitude + ) + } else { + // Match NSTextView's default autoresizing so the container + // width tracks the view frame in the Gallery preview path. + textView.autoresizingMask = [.width] + } textView.textContainerInset = .zero textView.delegate = context.coordinator @@ -232,12 +270,29 @@ public struct VSelectableTextView: NSViewRepresentable { .cursor: NSCursor.pointingHand, ] + context.coordinator.lastContainerWidth = useExternalSizing ? maxWidth : nil context.coordinator.applyAttributedString(attributedString, lineSpacing: lineSpacing, to: textView) return textView } public func updateNSView(_ textView: NSTextView, context: Context) { let coordinator = context.coordinator + + // Propagate container width changes before applying any text update. + // On the external-sizing path, the container is decoupled from the + // view frame, so a new `maxWidth` (e.g. from a window resize) must + // be written onto the container explicitly. Skip when unchanged so + // we do not perturb the current layout. + if useExternalSizing, + let textContainer = textView.textContainer, + coordinator.lastContainerWidth != maxWidth { + textContainer.containerSize = NSSize( + width: maxWidth ?? CGFloat.greatestFiniteMagnitude, + height: CGFloat.greatestFiniteMagnitude + ) + coordinator.lastContainerWidth = maxWidth + } + guard coordinator.lastAttributedString != attributedString || coordinator.lastLineSpacing != lineSpacing else { return } if useExternalSizing { @@ -326,6 +381,12 @@ public struct VSelectableTextView: NSViewRepresentable { var lastMeasuredWidth: CGFloat = -1 var lastMeasuredSize: CGSize = .zero + // Width currently written onto the text container on the external- + // sizing path. `updateNSView` propagates changes when this drifts + // from the incoming `maxWidth`, so the container stays decoupled + // from the view frame without re-sizing on every SwiftUI update. + var lastContainerWidth: CGFloat? + func reset() { lastAttributedString = nil lastLineSpacing = 0 @@ -333,6 +394,7 @@ public struct VSelectableTextView: NSViewRepresentable { pendingLineSpacing = nil pendingTextView = nil hasScheduledApply = false + lastContainerWidth = nil invalidateMeasurementCache() } From 774dfed2787352e555b231ab61b8ba9a9ffdc195 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 28 Apr 2026 15:16:18 +0000 Subject: [PATCH 2/3] docs(clients): add NSTextContainer/frame decoupling rule and tighten SelectableTextView comments Closes LUM-1170 Co-Authored-By: ashlee@vellum.ai --- clients/AGENTS.md | 1 + .../Display/SelectableTextView.swift | 35 ++++++++----------- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/clients/AGENTS.md b/clients/AGENTS.md index 5e75824fc42..14b8d46e206 100644 --- a/clients/AGENTS.md +++ b/clients/AGENTS.md @@ -197,6 +197,7 @@ Prefer migrating the parent to `@Observable` so the bridge becomes unnecessary ( - **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. - **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:)). +- **Decouple `NSTextContainer` from the view frame when an `NSViewRepresentable` hands SwiftUI a precomputed size.** If the caller applies `.frame(width:height:)` from a static measurement helper (`measureSize`), set [`NSTextContainer.widthTracksTextView`](https://developer.apple.com/documentation/uikit/nstextcontainer/widthtrackstextview) to `false`, size the container explicitly from the same `maxWidth` used to measure, and propagate `maxWidth` changes onto `containerSize` in `updateNSView`. Do not pair this path with `autoresizingMask = [.width]`. The default (`widthTracksTextView = true`, autoresize on) causes every SwiftUI-driven `setFrameSize` — window resize, eager `VStack` measurement — to forward onto the layout manager, triggering `_fillLayoutHoleForCharacterRange`: a synchronous main-thread relayout that is O(glyph count). Reference: [`NSTextView.isHorizontallyResizable`](https://developer.apple.com/documentation/appkit/nstextview/ishorizontallyresizable). - **Memoize `sizeThatFits` (and sibling measurement calls) in `NSViewRepresentable` views placed in lazy containers.** Any `NSViewRepresentable` that hosts a TextKit stack (`NSLayoutManager` + `NSTextContainer`) and can appear inside a `LazyVStack` / `LazyHStack` / `List` cell must cache its last measurement. [`NSLayoutManager.ensureLayout(for:)`](https://developer.apple.com/documentation/uikit/nslayoutmanager/ensurelayout(for:)) is O(glyph count) and runs synchronously on the main thread, and SwiftUI calls [`sizeThatFits(_:nsView:context:)`](https://developer.apple.com/documentation/swiftui/nsviewrepresentable/sizethatfits(_:nsview:context:)) on every visible cell on every layout pass (scroll, resize, streaming edit). Cache at minimum on `(textStorage.length, proposalWidth)`; for attribute-sensitive paths (static measurement helpers, highlighted content) key on the `NSAttributedString` itself and fall through to [`NSAttributedString.isEqual(_:)`](https://developer.apple.com/documentation/foundation/nsattributedstring/isequal(_:)) for identity — `NSAttributedString.hash` alone admits collisions and surfaces as wrong-height cells. Invalidate at every text-storage mutation site (text apply, edit callbacks, async highlight/formatting completions). Reference: `SelectableTextView`. When line wrapping is disabled and line height is pinned (as in `VCodeView.VCodeTextView` and `HighlightedTextView.CodeTextView`), prefer the line-count math above over a cached `ensureLayout` call.
Per-entity observation and dictionary patterns diff --git a/clients/shared/DesignSystem/Components/Display/SelectableTextView.swift b/clients/shared/DesignSystem/Components/Display/SelectableTextView.swift index b2bbc119331..c8efa87d349 100644 --- a/clients/shared/DesignSystem/Components/Display/SelectableTextView.swift +++ b/clients/shared/DesignSystem/Components/Display/SelectableTextView.swift @@ -205,27 +205,22 @@ public struct VSelectableTextView: NSViewRepresentable { // layout independently for their own scroll-attachment perf fix. textStorage.addLayoutManager(layoutManager) - // The container is the text wrapping surface. Its width determines - // where lines break; its configured size is what the layout manager - // lays glyphs into. Whether the container follows the NSTextView's - // frame depends on `widthTracksTextView` / `heightTracksTextView`. + // Two container sizing modes: // - // Two sizing paths: - // 1. `useExternalSizing: true` — the caller precomputes size via - // `measureSize` and applies an explicit `.frame(width:height:)`. - // We must decouple the container from the frame so that when - // SwiftUI calls `setFrameSize` on the NSTextView (e.g. during a - // window resize), AppKit does not interpret it as a container- - // width change and invoke `_fillLayoutHoleForCharacterRange` to - // re-lay out every glyph on the main thread — an O(n) cascade - // that produces multi-second hangs on large messages. Size the - // container explicitly from `maxWidth`; propagate changes in - // `updateNSView`. Peer components `VCodeView` and - // `HighlightedTextView` follow the same pattern. - // 2. `useExternalSizing: false` — our `sizeThatFits` drives layout - // and the container tracks the view frame as NSTextView's - // default behavior intends. Used by the Gallery preview, where - // there is only a single instance and no resize storm. + // - `useExternalSizing: true` — the caller precomputes size via + // `measureSize` and applies `.frame(width:height:)`. The container + // is decoupled from the view frame so that `setFrameSize` cannot + // forward a width change onto the layout manager and trigger + // `_fillLayoutHoleForCharacterRange`, an O(glyph-count) main-thread + // relayout. Container width is sized explicitly from `maxWidth` + // here, and propagated in `updateNSView` when `maxWidth` changes. + // + // - `useExternalSizing: false` — `sizeThatFits` drives layout and + // the container tracks the view frame, matching NSTextView's + // default. Used by the design system gallery only. + // + // Reference: NSTextContainer.widthTracksTextView + // https://developer.apple.com/documentation/uikit/nstextcontainer/widthtrackstextview let initialContainerWidth = useExternalSizing ? (maxWidth ?? CGFloat.greatestFiniteMagnitude) : 0 let textContainer = NSTextContainer(size: NSSize( width: initialContainerWidth, From 843fa368c7e14666936e97ad38ebe20f860d318e Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 28 Apr 2026 15:27:47 +0000 Subject: [PATCH 3/3] fix(macos): pass effective wrap width from measurement to VSelectableTextView Both reviewers flagged that VSelectableTextView's container width fell back to .greatestFiniteMagnitude on the useExternalSizing path when maxWidth was nil, while the measurement path fell back to VSpacing.chatBubbleMaxWidth. The two fallbacks must agree or the live container will not wrap at the measured width, producing horizontal clipping until a real width arrives. Carry the resolved effectiveMaxWidth on SelectableRunMeasurementResult and pass it directly to VSelectableTextView so the call site cannot drift from the measurement value. Document the contract on VSelectableTextView so future external-sizing callers do the same. Closes LUM-1170 Co-Authored-By: ashlee@vellum.ai --- .../Features/Chat/MarkdownSegmentView.swift | 13 ++++++++++--- .../Components/Display/SelectableTextView.swift | 6 ++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/clients/macos/vellum-assistant/Features/Chat/MarkdownSegmentView.swift b/clients/macos/vellum-assistant/Features/Chat/MarkdownSegmentView.swift index 0ecd9c74351..21588685628 100644 --- a/clients/macos/vellum-assistant/Features/Chat/MarkdownSegmentView.swift +++ b/clients/macos/vellum-assistant/Features/Chat/MarkdownSegmentView.swift @@ -194,7 +194,7 @@ struct MarkdownSegmentView: View, Equatable { VSelectableTextView( attributedString: measurement.nsAttributedString, - maxWidth: markdownView.maxContentWidth, + maxWidth: measurement.effectiveMaxWidth, lineSpacing: 4, tintColor: NSColor(markdownView.tintColor), useExternalSizing: true @@ -410,6 +410,11 @@ struct MarkdownSegmentView: View, Equatable { let nsAttributedString: NSAttributedString let size: CGSize let hasUnresolvedEmphasis: Bool + /// The wrap width used to produce `size`. Callers rendering with + /// `VSelectableTextView(useExternalSizing: true)` must pass this as + /// `maxWidth` so the live text container wraps at the same width + /// that measurement assumed. + let effectiveMaxWidth: CGFloat } /// Computes or retrieves from cache the `(NSAttributedString, CGSize)` pair @@ -455,7 +460,8 @@ struct MarkdownSegmentView: View, Equatable { return SelectableRunMeasurementResult( nsAttributedString: cached.nsAttributedString, size: cached.size, - hasUnresolvedEmphasis: false + hasUnresolvedEmphasis: false, + effectiveMaxWidth: effectiveMaxWidth ) } @@ -500,7 +506,8 @@ struct MarkdownSegmentView: View, Equatable { return SelectableRunMeasurementResult( nsAttributedString: nsAttributed, size: size, - hasUnresolvedEmphasis: hasUnresolvedEmphasis + hasUnresolvedEmphasis: hasUnresolvedEmphasis, + effectiveMaxWidth: effectiveMaxWidth ) } diff --git a/clients/shared/DesignSystem/Components/Display/SelectableTextView.swift b/clients/shared/DesignSystem/Components/Display/SelectableTextView.swift index c8efa87d349..6c3cfda6048 100644 --- a/clients/shared/DesignSystem/Components/Display/SelectableTextView.swift +++ b/clients/shared/DesignSystem/Components/Display/SelectableTextView.swift @@ -32,6 +32,12 @@ private final class SelectableNSTextView: NSTextView { /// view during the layout pass. This avoids an O(N) layout measurement /// cascade through nested `StackLayout.sizeThatFits` calls. /// +/// **Contract for `useExternalSizing: true`:** `maxWidth` MUST be non-nil +/// and equal to the value passed to `measureSize`. The live text container +/// is sized from `maxWidth`; if it diverges from the measurement width the +/// rendered wrap geometry will not match the precomputed frame, producing +/// horizontal clipping or stale heights. +/// /// For low-instance-count scenarios (e.g., a single thinking block), /// leave `useExternalSizing` at its default (`false`) and let /// `sizeThatFits` compute the size normally.