perf(chat): cache isPaginatedEmpty to decouple outer ChatView.body from message-array observation (LUM-1330)

[ashleeradka]

Adds isPaginatedEmpty — a cached boolean on ChatPaginationState — so ChatView's outer body observes a low-frequency boolean instead of the high-frequency paginatedVisibleMessages array, eliminating the main-thread stall during streaming that triggered Sentry hang MACOS-RJ.

Why needed: The outer ChatView.body evaluated .onChange(of: shouldShowSkeleton, initial: true) which read paginatedVisibleMessages.isEmpty. With @Observable, this tracked the full array property — any message mutation (every streamed token) invalidated the outer body. This also negated the ObservationBoundaryView added in LUM-1273: since the outer body was invalidated, it reconstructed the boundary view struct, and SwiftUI re-evaluated it anyway (closures aren't Equatable).

Benefits: During streaming, the outer body is no longer invalidated on every message mutation. Only the ObservationBoundaryView body re-evaluates (which is correct — the message list needs updating). This eliminates redundant reconstruction of the GeometryReader, ZStack, and full modifier chain (~12 modifiers including .onDrop, .onKeyPress, .overlay, .animation, multiple .onChange/.onReceive) that previously ran on every token during streaming.

Safety: isPaginatedEmpty is updated via defer in recomputePaginatedSuffix() — the same synchronous function that assigns paginatedVisibleMessages — preserving the consistency invariant from PR #24095 (no timing gap between the two). The write-only-on-change guard (if new != old) ensures observation fires only on actual empty↔non-empty transitions. Both properties live on the same @MainActor @Observable class, updated in the same synchronous call.

AGENTS.md update: Adds a "Cache derived booleans from high-frequency collections" guideline to clients/AGENTS.md § Performance, documenting the pattern to prevent recurrence.

References:

• WWDC23 — Discover Observation in SwiftUI — property-level tracking semantics, observation scope isolation
• WWDC23 — Demystify SwiftUI performance — view invalidation, avoiding unnecessary body re-evaluations
• Observation framework documentation — access(keyPath:) / withMutation(keyPath:) mechanics

Alternatives NOT taken:

• Move shouldShowSkeleton .onChange inside the ObservationBoundaryView — Can't attach modifiers inside a @ViewBuilder closure without restructuring the entire body. The modifier semantically belongs on the outer view (it controls @State showSkeleton).
• Add equality guard on paginatedVisibleMessages itself (if new != old { prop = new }) — [ChatMessage] comparison is O(n), and during streaming the arrays ARE different (last message text changes on every token), so the guard would always fail. Net cost increase with zero benefit.
• Make ObservationBoundaryView Equatable — It stores a () -> Content closure, which can't conform to Equatable.
• Use withObservationTracking bridge — Overly complex machinery for what is fundamentally a "cache a derived bool" problem.

Root cause analysis:

1. How did the code get into this state? — PR fix(chat): use paginatedVisibleMessages for empty/skeleton routing to fix blank chat on load #24095 correctly switched isEmptyState/shouldShowSkeleton from messages.isEmpty to paginatedVisibleMessages.isEmpty to fix a blank-screen bug caused by a timing gap between two @Observable objects connected via Combine. This was the right fix for consistency, but inadvertently coupled the outer body to a high-frequency property.

2. What decisions led to it? — PR fix(macos): narrow ChatView.body observation scope to prevent window-focus hang (LUM-1273) #28686 (LUM-1273) added ObservationBoundaryView to isolate mainContentStack, but focused only on reads inside the boundary — missing that shouldShowSkeleton (evaluated in .onChange(of:) outside the boundary) still read the array in the outer scope. The fix addressed the symptom (message list reads) without auditing all outer-scope reads.

3. Were there warning signs? — Yes. The .onChange(of: shouldShowSkeleton, initial: true) modifier visually appears to only "use" a boolean, but its implementation evaluates the computed property during every body pass to compare old/new values. This indirection made the observation dependency non-obvious during code review.

4. What prevents recurrence? — The new AGENTS.md guideline ("Cache derived booleans from high-frequency collections") makes this pattern explicit. It specifically calls out .onChange(of:) as a hidden observation registration site. Future developers/agents will see this before adding .isEmpty/.count checks on collection properties in view bodies or modifiers.

5. Systemic observation: When narrowing observation scope (e.g., adding ObservationBoundaryView), audit ALL reads in the outer scope — not just the content inside the boundary. Modifiers like .onChange(of:), .overlay {}, and .animation(_, value:) evaluate their expressions during body construction and register observation dependencies in the caller's scope.

---

• Requested by: @ashleeradka
• Session: https://app.devin.ai/sessions/c7ff076105f540abb4b1717864c12bcf

[linear]

LUM-1330 App Hang (2s+): ChatViewModel.paginatedVisibleMessages — eager computation via isEmptyState in ChatView.body (MACOS-RJ)

[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

[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 2 additional findings.

[vex-assistant-bot]

✦ APPROVE

Value: Eliminates the outer ChatView.body re-evaluation on every streamed token — stops redundant reconstruction of the GeometryReader, ZStack, and ~12-modifier chain that was firing for every message mutation during streaming. Direct fix for Sentry MACOS-RJ / LUM-1330.

What this does: Adds isPaginatedEmpty: Bool as a cached stored property on ChatPaginationState, updated via defer in recomputePaginatedSuffix() with a write-only-on-change guard. Replaces the two paginatedVisibleMessages.isEmpty reads in ChatView with isPaginatedEmpty — so the outer body only observes a boolean that changes on empty↔non-empty transitions instead of the full message array that mutates on every streamed token.

Correctness verified:

• All 4 mutation paths covered. paginatedVisibleMessages is private(set) and is only assigned inside recomputePaginatedSuffix() at lines 180, 194, 198, 200 — all are within the same function, so the defer fires after every path including early returns. ✅
• defer ordering correct. Registered at the top of the function, executes after all assignments, reads the final value of paginatedVisibleMessages on every exit path. ✅
• Consistency invariant preserved. isPaginatedEmpty is updated synchronously in the same call that updates paginatedVisibleMessages — no observer can read them in a split state. ✅
• Initial value correct. isPaginatedEmpty = true matches paginatedVisibleMessages = [] at init time. The init call to recomputeVisibleMessages will sync it immediately if seeded messages are non-empty. ✅
• Write-only-on-change guard effective. During streaming, the array is never empty, so newEmpty == isPaginatedEmpty == false → the guard short-circuits, zero observation events emitted. The outer body stays silent for the entire streaming window. ✅
• No remaining raw .isEmpty call sites. The grep confirms the two replaced sites in ChatView.swift are the only ones. ✅
• ObservationBoundaryView fix validated. Since closures aren't Equatable, the boundary view's closure was being treated as changed every time the outer body reconstructed — negating LUM-1273's boundary. This fix prevents outer body reconstruction during streaming, restoring the boundary's intended effect. ✅

AGENTS.md addition is accurate and precisely worded. The note that .onChange(of:) with a collection of: expression also tracks the outer body is a non-obvious gotcha worth documenting.

No anti-patterns. No layout/LazyVStack concerns. Devin reviewed and found no issues.