Eliminate explicitAlignment recursion in message list layout (LUM-800)

[devin-ai-integration]

Summary

Adds AlignmentBarrierLayout — a custom SwiftUI Layout that returns nil for explicitAlignment queries while passing sizing and placement through unchanged. Wrapping the ScrollView content in MessageListView with this barrier prevents _FlexFrameLayout alignment queries from cascading into the LazyVStack, eliminating the O(depth × children) recursive measureEstimates that caused 50+ second main-thread hangs during hover and window resize.

The original .frame(maxWidth:).frame(maxWidth: .infinity) on the ScrollView is preserved as-is — the barrier sits between those outer FlexFrames and the LazyVStack, so the alignment cascade stops at the barrier and never reaches the lazy container.

As defense-in-depth, also converts 23 .frame(maxWidth:, alignment:) sites across 10 cell-hierarchy files to HStack+Spacer or .frame(width:) — none of which create FlexFrame nodes or trigger alignment cascades. Documents both the anti-pattern rule and the AlignmentBarrierLayout convention in AGENTS.md. See WWDC23: Demystify SwiftUI performance.

Why this is safe: AlignmentBarrierLayout only overrides explicitAlignment → nil; sizing and placement pass through unchanged. The outer ScrollView frame pattern is identical to main. Removed .frame(maxWidth: chatBubbleMaxWidth) caps are redundant — the parent layout chain (chatColumnMaxWidth 808pt − 2 × VSpacing.xl 48pt = 760pt = chatBubbleMaxWidth) already constrains content.

Alternatives not taken:

• Fix all 46+ .frame(maxWidth:) sites individually — higher risk (15+ files), not future-proof. AlignmentBarrierLayout is 1 file + 1 wrapper.
• Replace ScrollView .frame(maxWidth:) with .frame(width:) — caused sidebar squeeze on narrow windows.
• Replace with .containerRelativeFrame — caused layout regressions.
• Remove ScrollView width constraint entirely — content expanded to full window width.

Review & Testing Checklist for Human

CI skips macOS builds — local Xcode is the only compilation and runtime check. The AlignmentBarrierLayout has not been verified at runtime.

• Spindump during window resize — open a long conversation with tool calls, resize the window. Should show near-zero explicitAlignment hits in the LazyVStack subtree (was 3730 pre-fix). This is the critical validation — if hits persist, the barrier placement may be wrong.
• Build succeeds in Xcode — AlignmentBarrierLayout.swift is in clients/shared/DesignSystem/Layout/ (SPM auto-pickup), used from clients/macos/
• Error bubble background width — .containerRelativeFrame(.horizontal) in ChatBubble.swift resolves against the ScrollView, not the padded content area. Error background may extend ~24pt beyond normal content on each side. Verify this looks acceptable. (HStack+Spacer was tried first but the parent VStack is content-sized, causing Spacer to collapse to zero; .frame(maxWidth:) is banned.)
• Bubble alignment and width — assistant left-aligned, user right-aligned, max width ~760pt, column centered at 808pt. Verify removed .frame(maxWidth: chatBubbleMaxWidth) caps don't cause overflow.
• Sidebar and narrow window layout — verify sidebar is not squeezed and content reflows at various window widths.

Notes

• optionalMaxWidth changed from .frame(maxWidth:) to .frame(width:) — on macOS, text uses SelectableRunView with pre-measured width and never hits this modifier. Post-PR callers are only code blocks and horizontal rules, which should fill to exact width. The function name is now slightly misleading but was kept to minimize diff churn.
• ChatMarkdownParser:394 and MarkdownSegmentView:915 retain alignment: .leading on _FrameLayout — O(depth) not O(n × depth), no hang risk.
• Several unmodified files (ChatBubbleAttachmentContent, ChatLoadingSkeleton, ChatEmptyStateView) still use .frame(maxWidth:) — these are protected by the AlignmentBarrierLayout barrier and can be addressed in a follow-up if spindump shows residual hits.

Link to Devin session: https://app.devin.ai/sessions/ef66aadc67724413b3b5ede76cf20ca9
Requested by: @ashleeradka

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment and CI monitoring

[ashleeradka]

Ship it ✦

Reviewed all 6 commits across 10 source files + AGENTS.md. Every .frame(maxWidth:, alignment:) in the cell layout hierarchy is eliminated — zero remaining in code (only ⚠️ warning comments).

What this fixes

The first commit (c2fd37b) fixed the 3 outer wrapper files but missed 16 .frame(maxWidth:, alignment:) sites inside the cell content views. Spindump on that build showed 3730 explicitAlignment hits — identical to pre-fix. The second commit (30d1feb) converts the remaining 16 sites in 7 inner content files (AssistantProgressView, ChatBubbleInterleavedContent, ChatMarkdownParser, ChatBubbleToolStatusView, MarkdownSegmentView, ThinkingBlockView, MessageListContentView).

QA notes

• Must verify: Build, open a long conversation with tool calls, resize window. Should be zero perceptible lag. Spindump should show near-zero explicitAlignment hits.
• Minor visual check: Some rows (AssistantProgressView, indicator rows) lost their explicit chatBubbleMaxWidth constraint and may be ~48px wider (filling chatColumnMaxWidth instead). Should be barely visible since it's in the padding zone, but worth a glance.

Nitpicks (non-blocking)

• ChatMarkdownParser:396 uses .frame(width: maxWidth.isFinite ? maxWidth : nil, alignment: .leading) — the alignment: .leading on _FrameLayout still propagates alignment queries, but at O(depth) not O(n × depth). Won't cause hangs.
• MarkdownSegmentView:912 same pattern, same verdict.
• Both could drop the alignment: parameter since their content already uses HStack+Spacer for alignment, but not worth a follow-up.

[devin-ai-integration]

Thanks for the thorough review! Agreed on both nitpicks — the alignment: .leading on _FrameLayout is O(depth) not O(n × depth), so no hang risk. Could drop them since the content already uses HStack+Spacer, but not worth churn.

[devin-ai-integration]

Devin Review found 3 new potential issues.

View 7 additional findings in Devin Review.

[devin-ai-integration]

🚩 Remaining .frame(maxWidth:) usages in non-modified files

The PR removes .frame(maxWidth:) from LazyVStack cell hierarchies in the modified files, but several unmodified files still use the pattern within the same LazyVStack: ChatBubbleAttachmentContent.swift:144,269,408, ChatLoadingSkeleton.swift:37,45, ChatEmptyStateView.swift:124,154,167,400. If the AGENTS.md rule about no .frame(maxWidth:) in LazyVStack cells is to be enforced consistently, these would need follow-up. However, attachment views with fixed max widths (not alignment: parameter) may produce _FlexFrameLayout without the problematic alignment queries, so the performance impact may vary.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

These are all outside the scope of this PR and protected by the AlignmentBarrierLayout barrier — alignment queries from the outer .frame(maxWidth:) chain stop at the barrier and never reach the LazyVStack. The remaining sites in ChatBubbleAttachmentContent, ChatLoadingSkeleton, and ChatEmptyStateView can be addressed in a follow-up if spindump shows any residual hits after the barrier is verified.

[devin-ai-integration]

🔴 .containerRelativeFrame(.horizontal) on error bubbles resolves against ScrollView, overflowing LazyVStack padding

The error bubble branch in bubbleChrome replaced .frame(maxWidth: .infinity) with .containerRelativeFrame(.horizontal). .containerRelativeFrame resolves against the nearest scroll container (the ScrollView in MessageListView.swift:113), returning the full ScrollView visible width. However, the LazyVStack content has horizontal padding of VSpacing.xl (24pt) per side applied at MessageListContentView.swift:272-273. The old .frame(maxWidth: .infinity) expanded within the parent's proposed width (already accounting for padding), while .containerRelativeFrame(.horizontal) ignores the padding and returns the raw ScrollView width. This makes error bubbles ~48pt wider than the content area, causing horizontal overflow.

Layout chain showing the mismatch

ScrollView (width W) → AlignmentBarrierLayout → LazyVStack.padding(.horizontal, VSpacing.xl) → cell proposed (W − 48pt) → HStack → VStack → bubbleChrome → .containerRelativeFrame(.horizontal) resolves to W (not W − 48pt)

Prompt for agents

The error bubble in bubbleChrome uses .containerRelativeFrame(.horizontal) which resolves against the ScrollView, ignoring the LazyVStack's horizontal padding (VSpacing.xl per side, applied in MessageListContentView). This causes error bubbles to be ~48pt wider than the content area.

The fix needs to replace .containerRelativeFrame(.horizontal) with a pattern that expands to the parent's proposed width (which already accounts for padding) without using .frame(maxWidth:) (which is banned by AGENTS.md for LazyVStack cells). Options:

1. Use an HStack + Spacer pattern like other views in this PR:
   HStack(spacing: 0) { content(); Spacer(minLength: 0) }
   This provides leading-aligned full-width expansion without FlexFrame.

2. Use .containerRelativeFrame with explicit size computation that subtracts the known padding.

3. Use a GeometryReader in .background to measure and constrain, though this is more complex.

Option 1 (HStack+Spacer) is most consistent with the rest of this PR's approach and should be the preferred fix. The error background fill (bubbleChromeBackground) is already applied via .background so it will naturally extend to the HStack's width.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

This is a known tradeoff already documented in the PR's testing checklist. The alternatives were evaluated:

1. HStack+Spacer — already tried and rejected. The Codex bot correctly identified (comment ci: add terraform apply workflow on platform changes #3) that the error bubble sits inside a content-sized VStack, so the Spacer collapses to zero and the error background shrinks to content width instead of spanning full-width.
2. .frame(maxWidth: .infinity) — the original code, banned by AGENTS.md for LazyVStack cells (creates _FlexFrameLayout with alignment queries).
3. .containerRelativeFrame(.horizontal) — resolves against the ScrollView, which is ~48pt wider than the padded content area. The error background may extend beyond the normal content bounds.

Option 3 is the least-bad choice. The AlignmentBarrierLayout is the primary hang fix — this .containerRelativeFrame is defense-in-depth. The user flagged this for visual verification during local Xcode testing.

[devin-ai-integration]

🚩 optionalMaxWidth semantic change from .frame(maxWidth:) to .frame(width:) forces exact widths

The optionalMaxWidth helper at clients/macos/vellum-assistant/Features/Chat/MarkdownSegmentView.swift:914-920 changed from .frame(maxWidth: width, alignment: .leading) to .frame(width: width, alignment: .leading). This is a meaningful behavioral change: maxWidth allows the view to be narrower than the cap if the parent proposes less, while width forces the view to report exactly that width regardless of the parent's proposal.

This affects code blocks (line 869) and horizontal rules (line 110). For code blocks, this means they will always be exactly maxContentWidth wide instead of shrinking to fit. Since maxContentWidth flows from bubbleMaxWidth which is already computed as min(chatBubbleMaxWidth, containerWidth - 2*xl) at MessageListContentView.swift:274-276, overflow is unlikely in normal usage. However, during transient states (sidebar resize, window narrowing before the environment updates), the fixed width could briefly exceed the available space. The function name optionalMaxWidth is now misleading since it applies an exact width, not a maximum.

(Refers to lines 914-920)

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Acknowledged — the function name optionalMaxWidth is now semantically inaccurate since it applies .frame(width:) not .frame(maxWidth:). However, on macOS (the target platform), text goes through SelectableRunView which uses a pre-measured .frame(width: measurement.size.width) — it never hits optionalMaxWidth. The only callers post-PR are code blocks (MarkdownSegmentView.swift:869) and horizontal rules (MarkdownSegmentView.swift:110), both of which should fill to exact width. The transient-state overflow risk is theoretical since bubbleMaxWidth is already computed from containerWidth which updates via onGeometryChange.

A rename to optionalWidth would be more accurate, but keeping the existing name to minimize diff churn in this performance PR.

[ashleeradka]

Re-approving after final 5 commits (AlignmentBarrierLayout + width fix iterations). Reviewed the full 12-file diff — architecturally sound. AlignmentBarrierLayout is the right fix. Ship it ✦