Skip to content

fix(macos): narrow ChatView.body observation scope to prevent window-focus hang (LUM-1273)#28686

Merged
vex-assistant-bot[bot] merged 4 commits into
mainfrom
devin/1777416849-chatview-observation-scope-lum-1273
Apr 28, 2026
Merged

fix(macos): narrow ChatView.body observation scope to prevent window-focus hang (LUM-1273)#28686
vex-assistant-bot[bot] merged 4 commits into
mainfrom
devin/1777416849-chatview-observation-scope-lum-1273

Conversation

@ashleeradka
Copy link
Copy Markdown
Contributor

@ashleeradka ashleeradka commented Apr 28, 2026

ChatView.body tracked ~42 @Observable properties in a single observation scope because all @ViewBuilder helper functions (mainContentStack, activeConversationContent, composerSection, btwOverlay) execute during the parent's body evaluation. When the window regains focus via TitleBarZoomableWindow.becomeKey, deferred observation changes flush simultaneously, forcing a synchronous full body re-evaluation in one AttributeGraph pass — causing 2000ms+ main-thread hangs (Sentry: VELLUM-ASSISTANT-MACOS-Q7).

This narrows the scope to ~3 reads by wrapping mainContentStack in ObservationBoundaryView and extracting btwOverlay into a standalone BtwOverlayView struct. Also updates clients/AGENTS.md to explicitly document the @ViewBuilder observation scope anti-pattern.

Why this is safe

Pure observation scope reorganization — same properties are read, same handlers fire, same view hierarchy is rendered. No new state, no new modifiers, no removed functionality. The .onKeyPress(.escape) handler reads viewModel.btwResponse only in its action closure (on key press), not during body evaluation — no observation scope issue.

What was NOT done and why

  • Full View struct extraction of mainContentStack: The Apple-recommended approach and what clients/AGENTS.md line 197 prescribes. However, mainContentStackactiveConversationContentcomposerSection together require threading 20+ parameters (viewModel + callbacks, flags, bindings). ObservationBoundaryView achieves identical observation isolation with a single wrapper — same pragmatic tradeoff made for MainWindowView in LUM-841.
  • Wrapping entire ChatView.body in one boundary: Too coarse — collapses all observation into one node, defeating property-level tracking and breaking SwiftUI structural diffing.
  • Adding @State window-focus tracking: Adds a new observation source rather than narrowing the existing scope.
  • DispatchQueue.main.async / withAnimation workarounds: Don't address the root cause (scope width); just defer or mask the cascade.

Root cause analysis

  1. How did the code get into this state? ChatView accumulated @ViewBuilder helper functions incrementally as features were added (message list, composer, btw overlay, error banners, queued messages). Each function was the simplest decomposition approach, but @ViewBuilder functions execute in the caller's body scope — all reads are tracked as if they were inline in ChatView.body.

  2. What mistakes led to it? The @ViewBuilder function pattern was appropriate in the ObservableObject era (whole-object invalidation anyway), but defeats @Observable's property-level tracking. The @Observable migration didn't narrow observation scopes to match the new tracking granularity — the body structure was carried over as-is.

  3. Were there warning signs? Yes — the identical pattern was fixed in MainWindowView (LUM-841) using the same ObservationBoundaryView technique. ChatView wasn't addressed in that fix because it was a separate scope.

  4. What can we do to prevent this? When adding new @Observable property reads to a view, audit whether the view's observation scope is still appropriately narrow. The key signal is a @ViewBuilder private function that reads @Observable properties — those reads are tracked in the parent, not in a separate scope.

  5. AGENTS.md update: Added a note to the "Scope observation narrowly" guideline in clients/AGENTS.md explicitly calling out that @ViewBuilder functions do NOT create separate observation scopes, and recommending standalone View structs or ObservationBoundaryView as alternatives.

References

Apple refs checked (2026-04-28): WWDC23-10149, Managing-model-data-in-your-app



Open in Devin Review

…-focus hang (LUM-1273)

Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
@devin-ai-integration
Copy link
Copy Markdown
Contributor

🤖 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
Copy link
Copy Markdown

linear Bot commented Apr 28, 2026

LUM-1273 App Hanging: App hanging for at least 2000 ms.

devin-ai-integration Bot and others added 2 commits April 28, 2026 22:59
…tion scope in AGENTS.md

Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
@devin-ai-integration devin-ai-integration Bot changed the title fix(macos): isolate ChatView.body observation scope to prevent window-focus hang (LUM-1273) fix(macos): narrow ChatView.body observation scope to prevent window-focus hang (LUM-1273) Apr 28, 2026
devin-ai-integration[bot]

This comment was marked as resolved.

…e removal transition

Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>
Copy link
Copy Markdown
Contributor

@vex-assistant-bot vex-assistant-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE

Value: Eliminates 2000ms+ window-focus hangs (Sentry VELLUM-ASSISTANT-MACOS-Q7) by applying the same observation scope narrowing that fixed MainWindowView in LUM-841. Verified independently against source — approach is correct and consistent with the established pattern.

What this does: Wraps mainContentStack in ObservationBoundaryView and extracts the btwOverlay @ViewBuilder into a standalone BtwOverlayView struct. This isolates ~42 @Observable property reads from ChatView.body — changes to btwResponse/btwLoading no longer cascade a full ChatView.body re-evaluation. Also removes the dead @Environment(\.colorScheme) property that was tracking color scheme changes for no reason (adjacent to the empty chatBackground).

Verified:

  • ObservationBoundaryView definition confirmed: @escaping @ViewBuilder closure deferred to its own body, creating a separate AttributeGraph node — exactly as described. Same struct used in MainWindowView LUM-841 fix. ✅
  • BtwOverlayView uses @Bindable var viewModel: ChatViewModel — consistent with the established pattern in TopBarView, ACPSessionsPanel, iOS views. ✅
  • Both ObservationBoundaryView and ChatView are in clients/macos/vellum-assistant/ (same Swift module, internal access works). ✅
  • .animation(VAnimation.fast, value: viewModel.btwResponse != nil) moved from the parent .overlay modifier chain into BtwOverlayView.body — semantically equivalent: the animation still fires on btwResponse presence change, and now it's scoped to the subview's own body so it doesn't contribute to parent re-evaluation. ✅
  • Removed @Environment(\.colorScheme) confirmed dead — chatBackground is EmptyView() and doesn't reference it. Correct deletion. ✅
  • AGENTS.md update: @ViewBuilder functions don't create separate observation scopes, and ObservationBoundaryView is the recommended alternative when full View struct extraction is impractical. Clear, accurate, well-placed. ✅

One note (non-blocking): ObservationBoundaryView lives in Features/MainWindow/ while ChatView is in Features/Chat/. Both are in the same module so it compiles fine, but if this pattern spreads further it may be worth relocating ObservationBoundaryView to a shared location (e.g. DesignSystem/ or a SwiftUIHelpers/ file). Not a blocker — just worth tracking if it gets adopted in 3+ features.

Root cause analysis quality: The PR description correctly identifies the deferred observation flush mechanism (becomeKey → pending obs flush → full ChatView.body cascade) and the ConversationArtifactsButton v0.6.6 amplifier. The AGENTS.md callout on @ViewBuilder scope is exactly what's needed to prevent this class of issue from recurring.

@vex-assistant-bot vex-assistant-bot Bot merged commit 6de785b into main Apr 28, 2026
7 checks passed
@vex-assistant-bot vex-assistant-bot Bot deleted the devin/1777416849-chatview-observation-scope-lum-1273 branch April 28, 2026 23:09
vex-assistant-bot Bot pushed a commit that referenced this pull request May 2, 2026
* perf(chat): replace .frame(width:) with .fixedWidth() in centeredChatColumn (LUM-1276)

Eliminates the _FrameLayout alignment cascade in the helper that wraps
all chat-chrome banners (CreditsExhaustedBanner, DiskPressureBanner,
MissingApiKeyBanner, CompactionCircuitOpenBanner, RecoveryModeBanner)
and the composer/read-only sentinel inside ChatView.activeConversationContent.

Same recipe as PR #29212 (MessageListView) and PR #28870. .frame(width:)
compiles to _FrameLayout, whose placeSubviews calls commonPlacement →
ViewDimensions[guide], which queries explicitAlignment on every
descendant — O(n × depth) on every layout pass. Inside this hot stack
(activeConversationContent → MessageListView etc.) it shows up in
Sentry MACOS-H7 / LUM-1276 as 2s+ App Hangs whose stack ends at the
ChatView body symbol.

FixedWidthLayout achieves the same width constraint and visual result
without the cascade: placeSubviews places the child at the bounds
origin via a UnitPoint anchor and both explicitAlignment overloads
return nil. The inner HStack { Spacer; content; Spacer } reproduces
the centered positioning that .frame(width:)'s default .center
alignment used to provide for non-filling content (e.g.,
CompactionCircuitOpenBanner's natural-width HStack); flexible content
collapses the inner spacers to zero and is visually unchanged.

Closes LUM-1276
Part of LUM-785 (the observation-cascade hypothesis was already
addressed by LUM-1273 / PR #28686; the residual layout cascade is
the actual cause of the 2s+ MACOS-H7 hangs)

Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>

* docs: soften centeredChatColumn docstring (drop unverified _FrameLayout cascade assertion)

Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant