perf(macos): eliminate redundant recomputeDerivedProperties during messaging

[ashleeradka]

Prompt / plan

Sentry hang analysis identified 7 issues (303 events, ~57 users) all rooted in ConversationListStore.recomputeDerivedProperties() blocking the main thread during messaging. The method runs synchronously in conversations.didSet and performs O(N log N) sorts, 8+ array equality comparisons (each O(N×18) for ConversationModel), and triggers synchronous SwiftUI graph invalidation via @Observable property writes.

Two prior PRs addressed the bulk case (snapshot-coalescing for batch handlers, cached derived properties with setIfChanged). This PR targets the remaining hot paths: individual-mutation callers that fire during normal messaging flow.

Changes

1. Guard conversations.didSet against no-op mutations

didSet {
    guard conversations != oldValue else { return }
    recomputeDerivedProperties()
}

Array subscript assignment (conversations[idx] = value) fires didSet even when the new value equals the old. During streaming, handleAssistantMessageArrival fires on every AssistantActivitySnapshot change — but most invocations don't actually modify the conversation (e.g., active conversation is already marked seen). The guard eliminates these redundant recomputes entirely.

2. Coalesce handleAssistantMessageArrival into a single write

Previously this method called listStore.updateLastInteracted() (write #1 → recompute) then listStore.conversations[index] = conversation (write #2 → recompute) — 2 full recomputes per new assistant message.

Extracted applyLastInteracted(into:) as a value-level helper (same pattern as applyAssistantAttention(from:into:)) so the caller applies last-interacted logic to a local snapshot and writes back once.

3. Guard individual writebacks at call sites

Added early-return guards to updateConversationTitle, updateConversationInferenceProfile, renameConversation, handleConversationTitleUpdated, and mergeAssistantAttention — all sites where the daemon may push unchanged data that would otherwise trigger a full recompute.

4. AGENTS.md guideline

Added didSet/writeback guard pattern to the coalescing guideline, documenting that array subscript assignment fires didSet unconditionally and how to guard against it.

Alternatives not taken

• RunLoop-coalesced recompute (defer recomputeDerivedProperties to next RunLoop turn): Risk of stale derived properties — callers like appendConversations → onConversationsAppended?() may read derived state synchronously after writing.
• Lazy recompute on access: Would require replacing all derived properties with manual @ObservationIgnored + access(keyPath:) management — high complexity, error-prone.
• Per-group granular sidebar properties: Split sidebarGroupEntries into per-group properties to reduce notification blast radius — large refactor for marginal gain since most changes affect the "All" group.

Root cause analysis

1. How did the code get into this state? conversations.didSet always called recomputeDerivedProperties() unconditionally. Swift's value-type array semantics mean subscript writes (array[i] = x) trigger didSet even when x == array[i]. The snapshot-coalescing PR fixed batch handlers but individual-mutation callers (especially handleAssistantMessageArrival) were left untouched.
2. What led to it? The handleAssistantMessageArrival path wasn't included in the coalescing PR because it was a single-element write — but it actually does two writes (one via updateLastInteracted, one direct), and fires on every activity snapshot during streaming.
3. Warning signs? MACOS-18Z (v0.8.3, post-coalescing) showed sidebarGroupEntries.setter → SwiftUI graph walk during recomputeDerivedProperties — confirming even a single recompute is expensive enough to hang for 2s+ with large conversation lists.
4. Prevention: The new AGENTS.md guideline codifies the didSet guard pattern.

Test plan

• CI passes (CI skips macOS builds — Xcode build verification needed locally)
• Changes are purely additive guards — they only skip work that produces no observable change
• The conversations != oldValue guard preserves existing behavior: when content actually changes, recomputeDerivedProperties runs exactly as before
• Existing ConversationListStoreObservationTests verify observation still fires on real mutations

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

[vex-assistant-bot]

✦ APPROVE

Value: Eliminates the root cause of 7 confirmed Sentry hang issues (303 events, ~57 users) by cutting redundant recomputeDerivedProperties() calls during streaming — the most frequent path in normal messaging flow.

What this does: Three-layer guard strategy — (1) didSet guard on the array, (2) value-level helper extraction to collapse 2× writes into 1× during handleAssistantMessageArrival, (3) call-site guards on individual write methods that fire during steady-state streaming.

---

ConversationModel.== — verified comprehensive: The guard conversations != oldValue guard's safety depends entirely on == covering all mutable stored properties. Verified at HEAD: the custom == compares all 16 stored fields (id, title, createdAt, conversationId, isArchived, groupId, displayOrder, lastInteractedAt, source, conversationType, inferenceProfile, scheduleJobId, hasUnseenLatestAssistantMessage, latestAssistantMessageAt, lastSeenAssistantMessageAt, forkParent expanded to 3 sub-fields, originChannel). All computed properties (isPinned, isBackgroundConversation, etc.) derive from stored fields — captured indirectly. No false-negative skip risk. ✅

Anti-patterns KB — directly on-point: The KB's Observation rule explicitly states "DO NOT write to @observable properties without first checking != old value — Observation fires on any withMutation regardless of whether value changed." Every change in this PR enforces exactly that rule, at both the didSet boundary and the write sites.

applyLastInteracted(into:) extraction — correct:

• @discardableResult is appropriate — updateLastInteracted (the wrapper) calls it without needing the return value, while handleAssistantMessageArrival (the coalescing caller) captures it for sendPinChange.
• sendPinChange call moved from ConversationListStore to ConversationManager after the single writeback — ordering preserved (fires after conversations[index] = conversation). Typecheck clean confirms accessibility.

Individual call-site guards — each verified:

• updateConversationTitle / renameConversation / handleConversationTitleUpdated: guard against daemon pushing the same title again (common during sync on reconnect).
• updateConversationInferenceProfile: same pattern, same reasoning.
• mergeAssistantAttention: guard conversation != conversations[index] is the most general form — applies the full == after applyAssistantAttention mutates the local copy, before writeback. Correct.

Concurrency — clean: All methods are synchronous on @MainActor. No new async work introduced. applyLastInteracted(into:) takes inout ConversationModel — pure local mutation, no shared state access. No race conditions introduced.

Non-blocking: groups.didSet still lacks the != oldValue guard. groups mutates far less often than conversations (no streaming path touches it), so it's not a hot path concern. Worth a follow-on if groups ever shows up in Sentry.

Vellum Constitution — Trust-seeking: 303 events of "app appears frozen during messaging" replaced with a single idempotency check that the framework's own docs say is the correct pattern.

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