fix(chat): don't strip and refetch inactive conversation history on thread switch (LUM-1107)

[ashleeradka]

Why

Noa reported thread switching feeling clunky in v0.6.5. Three distinct issues producing a single user-visible symptom — all fixed here:

1. Every switch to a previously-loaded conversation hit a blocking /history refetch, and cached tool-call / image bubbles rendered empty for several seconds while the fetch was in flight.
2. Clicking a thread in the sidebar provided no visible click feedback until the conversation fully loaded — the highlight stayed on the old thread, then jumped to the new thread together with the content swap.
3. The chat input disappeared during the loading skeleton, then popped back in when the conversation loaded — making the layout visibly jumpy even when the load itself was fast.

Fix 1 — Don't strip and refetch cached conversations on switch

Root cause

ConversationManager.selectConversation / activateConversation ran trimPreviousConversationIfNeeded on every switch, which called ChatViewModel.trimForBackground() on the outgoing VM:

1. Sliced messages to the latest 50
2. Ran stripHeavyContent() on each retained message — cleared tool-call results, attachment base64, and inline-surface payloads in place (text was preserved)
3. Set isHistoryLoaded = false

Step 3 failed the !viewModel.isHistoryLoaded guard in ConversationRestorer.loadHistoryIfNeeded, firing a blocking /history round-trip on re-activation. Step 2 meant the cached UI rendered empty tool/media bubbles until the fetch returned.

Change

• Remove the two trimPreviousConversationIfNeeded call-sites in ConversationManager
• Delete the now-unused helper in ConversationSelectionStore
• Delete the now-unused trimForBackground method in ChatViewModel
• Simplify populateFromHistory's post-trim dedupe path to drop its isContentStripped branches (they became unreachable once trimForBackground was gone — trimOldMessagesIfNeeded strips and removes in the same batchUpdateMessages, so stripped messages never persist to readers). Raised by Devin review on this PR.

Fix 2 — Yield before heavy VM work on selection change

Root cause

The sidebar click handler set windowState.selection synchronously and then called conversationManager.selectConversation(id:) directly. On a cache-miss the second call builds a new ChatViewModel, wires three Combine observers, and fires performActivation. SwiftUI can't commit the windowState.selection change until the synchronous handler returns — so the sidebar highlight stayed on the old thread for ~200–500 ms and appeared to update only when the content loaded.

Change

Move the yield into .onChange(of: windowState.selection) in MainWindowView.body rather than at the sidebar call-site. That handler is the convergence point for all selection sources (sidebar, nav-history replay, overlay dismissal, deep links), so the yield applies uniformly. Validation + applySelectionCorrection stay synchronous so an invalid selection never commits to the current frame; conversationManager.selectConversation(id:) and the dependent surface/dock sync move into a Task { @MainActor in ... }. The sidebar's direct call to conversationManager.selectConversation is dropped — the onChange handler now owns the sync.

An earlier version of this PR placed the Task { @MainActor in ... } wrapper at the sidebar call-site. Devin review caught that it was a no-op: onChange ran synchronously during the same SwiftUI view-update pass and did all the heavy work before the enqueued Task got a chance. By the time the Task fired, performActivation's conversationId != activeConversationId guard skipped the work. Addressed in 968c9908c5.

Matches Apple's event-handler guidance (WWDC25 — Embracing Swift concurrency, Apple HIG — Feedback target of <100 ms click-to-paint).

Fix 3 — Keep the composer on screen during the loading skeleton

Root cause

ChatView.mainContentStack branched between mutually-exclusive states (skeleton / bootstrap / empty / active). Only the non-skeleton branches rendered a composer, so switching to a cold conversation produced a visible layout pop: composer disappears during the skeleton window, then re-enters when the conversation loads. Behavior predates the skeleton PR (#14906) — the earlier spinner also rendered without a composer — but became more visually disruptive once the skeleton replaced the small spinner.

Change

Extract a single composerSection(width:isInteractionEnabled:) helper and render it in the skeleton branch with isInteractionEnabled: false. The active branch uses the same helper with the caller's current isInteractionEnabled. Composer keeps a stable screen position across load → active transitions. Typing into the disabled composer during load is blocked, but the input is still anchored — no layout jump.

Empty-state views (ChatEmptyStateView, ChatTemporaryChatEmptyStateView) still own their own inline composers. Unifying those into the extracted helper is tracked as LUM-1114.

Why all three are safe

Fix 1 — client memory is still bounded by four existing safeguards this PR does not touch:

Mechanism	Trigger	Behavior
Daemon light history (mode: \"light\")	Every /history fetch	Truncates text and tool-result payloads at source
VM LRU cache, cap 10	Cache overflow	Evicts oldest non-active, non-busy VM entirely
ChatViewModel.trimOldMessagesIfNeeded	Single VM >150 messages	Removes oldest, keeps 75 recent with heavy content
MemoryPressureMonitor	Dispatch memory-pressure warning / critical	Prunes active VMs to 75 recent

Upper bound: 10 VMs × 150 messages ≈ 1,500 messages in-memory, each already daemon-truncated. Stripped or daemon-truncated messages rehydrate on demand via ChatViewModel.rehydrateMessage(id:) (wired through MessageCellView.onRehydrate).

Fix 2 — the deferral only shifts conversationManager.selectConversation(id:) by one main-actor tick. windowState.selection — the source of truth for the sidebar highlight — is still set synchronously. Validation runs synchronously so invalid selections never commit. Surface/dock sync lives in the same Task as selectConversation because it reads activeViewModel, which only reflects the new conversation after selectConversation has run.

Fix 3 — uses the same ComposerSection view with isInteractionEnabled: false while the skeleton is visible. No new code paths, no state mutation risk.

References

• Dispatch — DispatchSource.makeMemoryPressureSource: OS-level API wrapped by MemoryPressureMonitor.
• WWDC23 — Demystify SwiftUI performance: informs the "keep the view's data cheap to read" principle already cited in ChatPaginationState.
• WWDC25 — Embracing Swift concurrency: Task { @MainActor in ... } as the canonical yield-before-work pattern.
• Apple HIG — Feedback: <100 ms click-to-paint target.
• Parent tracking issue #9272 — M1–M4 milestones that established daemon truncation + on-demand rehydration, making M5's trim-on-switch redundant.

Alternatives considered and rejected

• Drop only isHistoryLoaded = false from trimForBackground, keep the strip. Eliminates the refetch but leaves cached bubbles rendering empty until the user interacts with each one. Worse UX than doing nothing.
• Inactivity-based trim timer. Adds state and a timer for a speculative memory win the existing safeguards already cover. YAGNI.
• Optimistic rendering + silent background refresh (LUM-1107 "Fix Direction" 3/4). Correct long-term design but requires coordinated changes across loading state, pagination, and rehydration. Too large for a same-week ship; deferred.
• Keep trimForBackground as potential future reuse. Dead code. If we want timer-based eviction later, it's four lines to rewrite.
• Task wrapper at the sidebar call-site (Fix 2, original attempt). Bypassed by the synchronous onChange handler — ineffective. See Devin review discussion thread.
• Skip-flag on the sidebar handler to suppress onChange (Fix 2, Devin's option 2). Adds cross-file state coupling that is easy to forget when adding new selection sources. Moving the yield into onChange uniformly covers all callers without additional state.
• Move makeViewModel + observer setup off the main thread entirely. Observers are @MainActor-bound; moving them is a larger refactor. The Task-based yield captures 95% of the UX win for one onChange handler change.
• Unify the composer across skeleton/empty/active states in a single always-rendered instance (Fix 3, full refactor). The correct long-term architecture — kills all composer-position jumps — but requires refactoring two shared empty-state views (ChatEmptyStateView, ChatTemporaryChatEmptyStateView) to externalize their composer. Tracked as LUM-1114; this PR ships the narrow composer-during-skeleton fix first.

Root cause analysis

How did the code get into this state?

Fix 1 origin. The aggressive trim landed Feb 25 2026 in 5b73ce12a9 as Milestone 5 of #9272. M1–M4 had already built:

• M1: daemon truncates text/tool-result payloads at source
• M2: client on-demand rehydration (rehydrateMessage(id:), message_content_request)
• M3: stripHeavyContent() for in-place trim of old messages
• M4: markdown-cache guardrails against huge messages

M5 was the last piece: "evict memory from inactive threads." In isolation it seemed fine. Stacked on M1–M4 it was redundant and, worse, bypassed M2's lazy rehydration by forcing a blanket reload.

Fix 2 origin. The sidebar click handler followed the natural Swift pattern of calling conversationManager.selectConversation directly after mutating windowState.selection. Nothing about the pattern looked wrong — the problem only surfaces when the callee does meaningful synchronous work on a cache miss. The redundant call at the sidebar duplicated work that the onChange handler was already doing.

Fix 3 origin. The skeleton was introduced by #14906 as a direct replacement for a ProgressView(); the earlier spinner also had no composer next to it, so the visible-gap regression was inherited, not introduced.

What uncovered all three. c7186d7828 (@observable migration of MainWindowView managers) fixed a 2 s+ MainWindowView.body re-evaluation hang that had been masking all three regressions. Once that hang was gone, the strip-then-refetch latency, the event-handler blocking, and the composer pop all became visible bottlenecks.

What decisions led to it?

• Redundancy blindness. M5 was added without re-checking whether M1–M4 had already solved the memory goal.
• Second mechanism added alongside an existing one. isHistoryLoaded = false forces a full-conversation refetch, but rehydrateMessage(id:) already existed for exactly this purpose.
• Unverified latency assumption. The design assumed daemon refetch was fast enough to be invisible. True for small text-only conversations, false for ones with tool calls / images.
• Implicit assumption that event-handler work is always cheap. ConversationManager.selectConversation grew over time; no one revisited whether it was still acceptable to run synchronously from a tap gesture.
• Spinner → skeleton swap preserved a pre-existing shortcut. The earlier spinner rendered alone in its branch, and the skeleton inherited that structure without reconsidering whether the composer should stay put.

Warning signs we missed

• PR #9314 had to patch a pagination-corruption bug caused by the trim racing in-flight history loads. That was a signal the mechanism was fragile on the hot path.
• No test asserted "switching between loaded threads stays in memory" or "clicking a conversation updates the sidebar selection synchronously."
• The trim-on-switch state machine was spread across three files (ConversationManager → ConversationSelectionStore → ChatViewModel.trimForBackground). Spread-out state is harder to reason about and easier to regress unknowingly.
• mainContentStack had four mutually-exclusive branches each owning their own composer (or lacking one). A single always-rendered composer was never considered because there was no forcing function.

Prevention

• When adding behavior that runs on a hot path (every switch, every message, every render), require a test that exercises the hot path and asserts bounded work.
• Before flipping a cached-state flag (isLoaded = false, isStale = true) to force a reload, check whether an on-demand lazy API already exists and prefer it.
• When a downstream PR has to paper over a side effect of an earlier "optimization," treat that as a prompt to revisit the optimization itself.
• In event handlers that mutate observable state driving a visible UI change, keep the synchronous work minimal and defer heavier work (VM init, observer wiring) to the next main-actor tick.
• When a component has multiple mutually-exclusive render branches, audit whether any UI elements should remain stable across branch transitions rather than being duplicated or missing per-branch.

AGENTS.md rules added in this PR

clients/AGENTS.md → Performance and Resource Management → Memory Management:

Don't invalidate cached state on a hot path to force a refetch. When an isLoaded-style flag already gates a load, flipping it to false on every thread switch / tab switch / reselection runs a full refetch each time and leaves the cached UI rendering stale (or empty, if you also strip content) until the network round-trip resolves. If the data really does need refreshing, prefer an on-demand per-item rehydration API (see ChatViewModel.rehydrateMessage(id:)) over a blanket reload. The LRU cache, trimOldMessagesIfNeeded, and the memory-pressure handler are sufficient memory safeguards on their own — don't stack a preemptive trim on top that defeats them.

clients/AGENTS.md → Performance and Resource Management → Concurrency and Task Management:

Yield to the run loop before heavy synchronous work in event handlers. When a .onTapGesture, Button action, or similar user-input handler both mutates state that drives a visible UI change and calls into expensive synchronous work (ViewModel init, Combine observer wiring, large parse), wrap the heavy call in Task { @MainActor in ... }. SwiftUI commits invalidated views only after the synchronous handler returns — without the yield, the user sees no response to the click until the slow work finishes, even though the state change itself was instant. Target <100 ms click-to-paint latency per Apple HIG — Feedback.

Both rules are durable (encode general principles, not specific APIs that rename), sit next to related guidance, and reference live canonical APIs/WWDC material rather than content that will drift.

Testing

New file ConversationManagerThreadSwitchCacheTests.swift covers three invariants for Fix 1:

• testSwitchingAwayPreservesIsHistoryLoaded
• testSwitchingAwayDoesNotStripMessageContent
• testRapidRoundTripPreservesBothConversations

All three pass. ./build.sh lint is clean.

Fixes 2 and 3 are not unit-tested here — SwiftUI paint-commit timing and layout stability during transitions aren't observable from XCTest without UI instrumentation. The AGENTS.md rules plus the inline comments at each call-site are the forward-looking guard. Manual verification is in the test plan.

Test plan

• ./build.sh test --filter ConversationManagerThreadSwitchCacheTests
• Manual: click a previously-loaded conversation — content renders instantly with tool calls / images intact, no spinner
• Manual: click a cold (first-visit or LRU-evicted) conversation — sidebar highlight moves immediately (on the same frame as the click), skeleton appears inside the chat area within 200 ms with the composer visible below it, content replaces skeleton when fetch completes, composer stays in the same position throughout
• Manual: rapidly click between 3+ conversations — each click produces instant highlight feedback in the sidebar
• Manual: long conversation >150 messages still caps correctly via trimOldMessagesIfNeeded

🤖 Generated with Claude Code

[linear]

LUM-1107 Thread switching feels slow/clunky — messages not cached, reload via API on every switch

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

[vex-assistant-bot]

✦ APPROVE

Value: Directly fixes the thread-switch regression from standup — removes the trimForBackground() path that was (1) stripping cached tool-call/media content in-place and (2) resetting isHistoryLoaded, which forced a blocking /history round-trip on every re-activation.

Verified the safety argument:

The four remaining memory safeguards are untouched and sufficient:

1. Daemon mode: "light" — truncates payloads at source ✅
2. VM LRU cache cap 10 (scheduleEvictionIfNeeded) — evicts oldest VMs entirely ✅
3. trimOldMessagesIfNeeded at 150 messages per VM — hard-deletes oldest, keeps 75 ✅
4. MemoryPressureMonitor — OS-level signal → prunes active VMs ✅

The removed trim was the most aggressive AND most user-impactful of the five defenses — it ran on every switch and defeated the cache that the other four were designed to protect.

Code review:

• Removal is clean — both activateConversation and selectConversation call sites, the helper, and the method itself. No orphaned references.
• stripHeavyContent() correctly stays (still called by trimOldMessagesIfNeeded on messages immediately before removal).
• ChatMessage.isContentStripped correctly stays (defensive guard in history dedup).
• Updated comment in populateFromHistory accurately reflects the remaining use case.
• AGENTS.md rule is durable and well-worded.

Tests:
The three regression tests (preservesIsHistoryLoaded, doesNotStripMessageContent, rapidRoundTrip) directly encode the invariant. Good use of makeLoadedConversation helper to skip network bootstrap.

Re: remaining perceived slowness — this PR eliminates the data-layer regression (refetch + empty bubbles). Any remaining sluggishness on switch is likely the SwiftUI view-diff cost when displayedMessageCount has escalated to Int.max on conversations that were scrolled to the top (the LUM-1022 issue). That's a separate fix.

No concerns. Ship it.

[vex-assistant-bot]

✦ Re-approving after new commits. Original review still applies — clean removal of the trim-on-switch path, four remaining memory safeguards are sufficient.

[devin-ai-integration]

Devin Review found 1 new potential issue.

View 7 additional findings in Devin Review.

[devin-ai-integration]

🟡 Skeleton branch shows composer for readonly conversations, causing layout jump on transition

In mainContentStack, the skeleton loading branch unconditionally renders composerSection(width: layoutMetrics.chatColumnWidth, isInteractionEnabled: false) at line 274. However, when the skeleton clears and activeConversationContent takes over, readonly conversations show a "Read-only conversation" banner instead of the composer (ChatView.swift:473). This means the skeleton→active transition for readonly conversations swaps a full-height composer for a single-line banner — producing the exact layout jump the PR aims to eliminate. The fix should check isReadonly in the skeleton branch and render the read-only banner (or matching-height placeholder) instead of the composer when the conversation is readonly.

Prompt for agents

In ChatView.mainContentStack, the skeleton branch at line 274 unconditionally renders composerSection(width:isInteractionEnabled:). But activeConversationContent (line 473) conditionally renders either the read-only banner (if isReadonly) or the composer. To keep layout stable across the skeleton-to-active transition for readonly conversations, the skeleton branch should mirror this conditional: show composerSection when !isReadonly, and show the same centeredChatColumn read-only banner when isReadonly. Look at lines 473-489 in activeConversationContent for the if/else pattern to replicate.

---

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

[vex-assistant-bot]

✦ Re-approving after latest updates. Core fix unchanged — trim-on-switch removal with four remaining memory safeguards intact.