perf(chat): use FixedWidthLayout for centeredChatColumn (LUM-1276)

[ashleeradka]

Prompt / plan

centeredChatColumn (used six times inside ChatView.activeConversationContent for chrome banners, the read-only sentinel, and the composer) wraps its content in .frame(width: width). clients/macos/AGENTS.md § "No _FlexFrameLayout inside LazyVStack/LazyHStack/LazyVGrid cell hierarchy" already documents that .frame(width:) "queries child alignment via commonPlacement → ViewDimensions[guide]" during placeSubviews and is "not safe as a cascade barrier" when the subtree contains a LazyVStack or deep view hierarchy — recommending .fixedWidth() instead. The MessageListView work in #29212 and the PinnedLatestTurnSection change in #29199 applied that rule to the message-list ScrollView ancestor chain. This PR applies the same rule to the chat-chrome ancestor chain, which is the wrapper for MessageListView and the chrome banners. Tracking ticket: LUM-1276 (Sentry MACOS-Q9, 1 event on macOS 26 beta — _FlexFrameLayout ancestor chain reaching CreditsExhaustedBanner).

Test plan

CI does not run an Xcode build for this repo, so Swift typechecking and visual verification need to happen locally in Xcode. .fixedWidth(_:) and FixedWidthLayout are already public in clients/shared/DesignSystem/Modifiers/FixedWidthLayout.swift and used elsewhere in MessageListView.swift and MessageCellView.

Visual checks to verify locally:

• All five chrome banners (CreditsExhaustedBanner, DiskPressureBanner, MissingApiKeyBanner, CompactionCircuitOpenBanner, RecoveryModeBanner) render at the same width and centering as before.
• CompactionCircuitOpenBanner (the only banner with natural-width content and no internal Spacer) stays horizontally centered inside the chat column.
• Composer column and the read-only sentinel are unchanged.

---

What changed

// before
HStack(spacing: 0) {
    Spacer(minLength: 0)
    content().frame(width: width)
    Spacer(minLength: 0)
}

// after
HStack(spacing: 0) {
    Spacer(minLength: 0)
    HStack(spacing: 0) {
        Spacer(minLength: 0)
        content()
        Spacer(minLength: 0)
    }
    .fixedWidth(width)
    Spacer(minLength: 0)
}

The outer HStack { Spacer; …; Spacer } continues to center the column inside the available width. The inner HStack { Spacer; content; Spacer } reproduces the visual centering that .frame(width:)'s default .center alignment used to apply to non-filling content (for filling content, the inner spacers collapse to zero and have no layout effect).

Why this change is needed

FixedWidthLayout is a Layout-protocol wrapper used elsewhere in the codebase (MessageListView.swift:150, :183, :199, and across MessageCellView). Compared to .frame(width:):

• sizeThatFits returns the same width — identical visual sizing.
• placeSubviews calls place(at:bounds.origin, anchor: .topLeading, …) — UnitPoint-based placement that does not query alignment guides on the child subtree.
• Both explicitAlignment overloads return nil, blocking parent-initiated alignment queries from propagating through the subtree.

This makes the helper a true cascade barrier for the chat-column subtree, which contains MessageListView (a LazyVStack-backed deep hierarchy) and chrome banners that themselves have nested layout.

Why it's safe

FixedWidthLayout returns the same width as .frame(width:) from sizeThatFits, so column width is unchanged. The inner HStack { Spacer; content; Spacer } reproduces .frame(width:)'s default .center alignment for content that does not fill its proposal:

Banner	Fills horizontally?	Behavior under .fixedWidth + inner Spacers
CreditsExhaustedBanner	Yes (VStack.frame(maxWidth: .infinity, alignment: .leading))	Spacers collapse to 0; identical
DiskPressureBanner	Yes (interior Spacer(minLength:))	Spacers collapse to 0; identical
MissingApiKeyBanner	Yes (VButton.frame(maxWidth: .infinity))	Spacers collapse to 0; identical
RecoveryModeBanner	Yes (.frame(maxWidth: .infinity, alignment: .leading))	Spacers collapse to 0; identical
CompactionCircuitOpenBanner	No (natural-width HStack)	Inner spacers split remainder, content is centered — matches previous .frame(width:) .center default
ComposerSection	Yes (ComposerView.frame(maxWidth: .infinity))	Spacers collapse to 0; identical
Read-only sentinel	Yes (interior Spacer(minLength: 0) on both sides)	Spacers collapse to 0; identical

Because FixedWidthLayout returns nil from both explicitAlignment overloads, the OUTER HStack { Spacer; centeredColumn; Spacer } (column-centering wrapper) continues to work — it computes its own positioning and never queries the inner subtree's alignment.

Alternatives NOT taken

1. Add .frame(maxWidth: .infinity) inside each banner. Reintroduces _FlexFrameLayout, which is the explicit anti-pattern enforced by clients/scripts/check-flexframe.sh and clients/macos/AGENTS.md. Wrong direction.
2. Drop the inner HStack { Spacer; content; Spacer } wrapper. CompactionCircuitOpenBanner's natural-width HStack would shift from center to leading inside the chat column — visible regression. The inner spacers add zero layout cost when content fills (Spacers collapse to 0), so the wrapper is strictly safer than not having it.
3. Wrap each banner site individually instead of changing the helper. Six near-identical call-site rewrites versus one helper change. The helper is the architectural source.
4. Hoist the column-centering HStack into FixedWidthLayout directly (e.g. a new CenteredFixedWidthLayout). Possible, but would add a new shared Layout type for one consumer. Defer until a second consumer needs it.

Root cause analysis

1. How did the code get into this state? centeredChatColumn predates the design-system migration that introduced FixedWidthLayout and the cascade-barrier rule. When .fixedWidth() was applied to the inner ScrollView positions in MessageListView (#29212, MessageListView.swift:150, :183, :199), the chat-chrome wrapper that hosts those scroll surfaces was not on the radar.
2. What mistakes or decisions led to it? .frame(width:) looks like an unambiguous "constrain to a width" primitive; _FrameLayout's placement-time alignment query is not surfaced in its public API. The cascade-vs-not distinction was learned incrementally as the chat surfaces hit production hangs, rather than being an upfront design choice. Once the rule was articulated, retroactive sweeps fixed the message-list and pinned-turn ancestor chains but not the chrome-banner wrapper.
3. Were there warning signs we missed? Yes — the prior cascade fixes (#28870, #29199, #29212) all targeted the same family of width-constraint helpers. A grep for .frame(width: in clients/macos/vellum-assistant/Features/Chat/ would have surfaced this helper at any of those points.
4. What can we do to prevent this pattern from recurring? The substantive rule already exists in clients/macos/AGENTS.md § "No _FlexFrameLayout …" (line 304–309), including the explicit warning that .frame(width:) is "not safe as a cascade barrier". The lint script clients/scripts/check-flexframe.sh mechanically enforces the _FlexFrameLayout half of the rule (.frame(maxWidth:), .frame(minHeight:), etc.) but does not flag .frame(width:) because the modifier is fine in most contexts and only problematic when used as a cascade barrier above a deep subtree. Extending the lint to flag .frame(width:) inside clients/macos/vellum-assistant/Features/Chat/ (the directory where deep-hierarchy cascade barriers live) would close the gap; doing it more broadly would generate false positives.
5. Should we add/remove/change a guideline to AGENTS.md? No new rule needed — the existing rule at clients/macos/AGENTS.md line 304–309 already says exactly what this PR implements. The improvement is in tooling/manual-review reach, not docs. (If a follow-up PR extends check-flexframe.sh to flag .frame(width:) inside chat directories, that would be a concrete next step, but is intentionally out of scope here.)

Closes LUM-1276

Link to Devin session: https://app.devin.ai/sessions/9a108e31a87d4ebcb19aeefaff7f529a
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

[linear]

LUM-1276 Hang: _FlexFrameLayout in CreditsExhaustedBannerV ancestor chain (ChatView.activeConversationContent)

LUM-785 App Hang (2s+): ChatPaginationState.hasMoreMessages computed prop registers 4 @Observable deps on ChatView.body (MACOS-H7)

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

[devin-ai-integration]

Devin Review found 2 new potential issues.

View 3 additional findings in Devin Review.

[devin-ai-integration]

🔴 centeredChatColumn inner HStack+Spacer wrapping causes flexible content to compete with spacers instead of filling full width

The old centeredChatColumn applied .frame(width: width) directly to content, which unconditionally constrained the content to exactly width pixels — both for fixed-size content (centered via default .center alignment) and for flexible content (proposed width and filling it). The new implementation wraps content in HStack(spacing: 0) { Spacer(minLength: 0); content(); Spacer(minLength: 0) }.fixedWidth(width), which proposes width to the inner HStack. The inner HStack then distributes that width among three children: two Spacer(minLength: 0) and the content. For fixed-size content this correctly centers it (spacers expand equally). However, for flexible content — views that expand to fill available space — the content now competes with the two spacers for surplus space rather than being directly constrained to width.

Multiple callers pass flexible content:

• CreditsExhaustedBanner uses .frame(maxWidth: .infinity, alignment: .leading) (ChatErrorToastView.swift:223)
• DiskPressureBanner has Spacer(minLength:) in its HStack (ChatErrorToastView.swift:263)
• MissingApiKeyBanner uses .frame(maxWidth: .infinity) (ChatErrorToastView.swift:405) and Spacer() (ChatErrorToastView.swift:384)
• ComposerSection → ComposerView uses .frame(maxWidth: .infinity) (ComposerView.swift:332)
• The read-only label passes HStack { Spacer; Icon; Text; Spacer } (ChatView.swift:436)

The docstring claims flexible content "collapses the inner spacers to zero and occupies the full column width," but this depends on undocumented SwiftUI HStack surplus-distribution behavior. If the surplus is shared equally among all flexible children (which is the standard understanding of HStack layout), banners and the composer would render narrower than the intended column width — a visible layout regression.

Prompt for agents

The centeredChatColumn function replaced `.frame(width: width)` with `HStack { Spacer; content; Spacer }.fixedWidth(width)`. The intent was to reproduce centering while using FixedWidthLayout instead of _FrameLayout to avoid the alignment cascade. However, the inner HStack forces flexible content (banners with .frame(maxWidth: .infinity), composer, read-only label with Spacers) to compete with the wrapper Spacers for surplus space, instead of being directly proposed `width`.

The simplest correct fix is to make FixedWidthLayout support centered placement natively (e.g. add an alignment parameter that defaults to .center), so content can be directly wrapped with .fixedWidth(width) without needing surrounding Spacers. Alternatively, apply .fixedWidth(width) directly to content() and accept topLeading placement (the outer HStack { Spacer; ...; Spacer } already handles page-level centering), since all current callers either have internal centering (read-only label) or want full-width fill (banners, composer).

Key files: FixedWidthLayout.swift (the Layout implementation), ChatView.swift:543-557 (centeredChatColumn), and all callers listed in ChatView.swift lines 368-448 plus composerSection at line 461.

---

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

[devin-ai-integration]

Confirmed and addressed in follow-up #29232. The PR was merged before the fix landed, so the change is on a separate branch.

You're right that relying on HStack surplus distribution is undocumented. The follow-up adopts the MessageListView.swift:147-182 shape — .fixedWidth(width) applied directly to content(), no inner Spacer wrapper. Five of the six centeredChatColumn callers are horizontally flexible and fill the column on their own; CompactionCircuitOpenBanner is the lone natural-width caller and gets an explicit HStack { Spacer; banner; Spacer } wrap at its call site to preserve centering. Resolved.

[devin-ai-integration]

🚩 Existing MessageListView fixedWidth usage pattern differs from new centeredChatColumn pattern

In MessageListView.swift:150, .fixedWidth(widths.chatColumnWidth) is applied directly to scrollViewContent without wrapper spacers, while the outer centering is handled by HStack { Spacer; content.fixedWidth(); Spacer }. This is the pattern used when content should fill the full column width (no centering within the fixed-width region needed). The new centeredChatColumn adds an additional inner HStack { Spacer; content; Spacer } layer before applying .fixedWidth(), creating a different layout structure. This inconsistency between the two patterns is worth noting — if all centeredChatColumn content should fill the full width (which appears to be the case for banners and the composer), the MessageListView pattern of direct .fixedWidth() application would be more appropriate.

---

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

[devin-ai-integration]

Agreed — the inconsistency is real. Follow-up #29232 brings centeredChatColumn in line with the MessageListView.swift:147-182 pattern (Spacer + content.fixedWidth(W) + Spacer, no inner wrapper). The one natural-width caller (CompactionCircuitOpenBanner) takes an explicit centering wrap at its call site rather than baking it into the helper. Resolved.

[vex-assistant-bot]

✦ APPROVE

Value: Completes the _FrameLayout → FixedWidthLayout migration across ChatView's full layout hierarchy — all 6 chrome banner + composer call sites now barrier against alignment-guide cascade in a single helper change.

What this does: Replaces .frame(width: width) inside centeredChatColumn with .fixedWidth(width) + inner HStack { Spacer; content; Spacer } centering pattern. One function change covers all call sites simultaneously.

---

Verified — FixedWidthLayout is a correct barrier:
Read FixedWidthLayout.swift directly. Both explicitAlignment overloads explicitly return nil, and placeSubviews uses .topLeading anchor — no alignment guide query at all. The PR description's claim is accurate and backed by the source.

Centering behavior preserved correctly:
The inner HStack { Spacer(minLength: 0); content; Spacer(minLength: 0) } reproduces what .frame(width:, alignment: .center) did. For flexible content (banners with internal .frame(maxWidth: .infinity) or Spacers), the inner Spacers collapse to zero and content fills the full column width — exactly as before.

Scope is complete:
All 6 centeredChatColumn call sites (5 chrome banners at chatColumnWidth - 2 * VSpacing.xl, 1 read-only sentinel at chatColumnWidth) are covered by the single helper change. No missed instances.

Minor (non-blocking): data/codebase/anti-patterns.md line 50 still documents the old content.frame(width: chatColumnWidth) pattern for centeredChatColumn. Will update the KB.

Devin approved. Merge criteria met once CI is green.