fix(chat): use synchronous Combine publish for optimistic message inserts

[devin-ai-integration]

Summary

Fixes LUM-929: sent messages not appearing instantly in the macOS conversation.

The two optimistic insert sites in MessageSendCoordinator.sendMessage() used messageManager.messages.append(), which goes through the _modify accessor on ChatMessageManager. That accessor defers the Combine publish to a Task on the next cooperative executor turn via scheduleDeferredPublish(). Since ChatPaginationState subscribes to this Combine publisher to update paginatedVisibleMessages (the array the view renders), the new message doesn't appear until the deferred Task fires — or worse, until another event triggers a synchronous publish (worst case: the 60-second isSending watchdog).

Replacing with batchUpdateMessages { $0.append(msg) } publishes synchronously, ensuring paginatedVisibleMessages is current before SwiftUI re-evaluates. This is the same pattern already used at 14 other mutation sites across ChatActionHandler, ChatViewModel, and MessageSendCoordinator.

Why the _modify deferred publish exists (and why it doesn't help here)

PR #23464 introduced scheduleDeferredPublish() to coalesce rapid multi-element mutation loops (e.g. stopGenerating). However, that same PR also converted all those loops to batchUpdateMessages. The remaining _modify callers — including these two .append() sites — are single-mutation operations that don't benefit from coalescing. All streaming/action-handler mutations go through vm.messages[idx] (the get+set path on ChatViewModel), which already publishes synchronously.

Prompt / plan

Investigated the full display pipeline: messageManager.messages → ChatPaginationState.recomputeVisibleMessages() → paginatedVisibleMessages → SwiftUI view. Traced every mutation site to determine which use _modify (deferred) vs get+set (synchronous). Confirmed only MessageSendCoordinator accesses messageManager.messages directly.

Test plan

• Static analysis of all mutation paths through _modify vs get+set
• CI checks
• Manual verification on macOS app needed to confirm the fix resolves the visible delay

Human review checklist

• Confirm _modify accessor theory: that messageManager.messages.append() triggers deferred publish while batchUpdateMessages publishes synchronously (see ChatMessageManager.swift lines 59–67 vs 84–94)
• Verify the closure capture of local userMsg/userMessage is safe (synchronous, non-escaping closure)
• Consider whether remaining direct messageManager.messages subscript mutations in MessageSendCoordinator (status changes on existing messages, lines 429/454/701/733/743–745/823) should also be converted as a follow-up

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

[devin-ai-integration]

Devin Review found 1 potential issue.

View 1 additional finding in Devin Review.

[devin-ai-integration]

🚩 Two messages.append calls remain in ChatActionHandler

While this PR migrates the two .messages.append() calls in MessageSendCoordinator.swift to batchUpdateMessages, there are two remaining .messages.append() calls in ChatActionHandler.swift:159 and ChatActionHandler.swift:1128 that still use the _modify accessor path (deferred Combine publish). These may be intentional — the _modify coalescing is appropriate when followed by additional rapid subscript mutations — but if the goal of this PR is to standardize on batchUpdateMessages for all append operations, these two sites should also be considered for migration.

---

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

[devin-ai-integration]

Those two sites (ChatActionHandler.swift:159 and :1128) use vm.messages.append(), not messageManager.messages.append().

vm.messages is a computed property on ChatViewModel that does NOT have a _modify accessor — it only has get/set. So .append() on it goes through get (copy) + set (synchronous Combine publish). These are already on the synchronous publish path and don't have the deferred publish issue.

Only direct messageManager.messages mutations trigger the _modify accessor with deferred publish, and those only exist in MessageSendCoordinator.

[vex-assistant-bot]

Verified the full publish pipeline. Clean, minimal, correct fix.

Confirmed the _modify theory:

• ChatMessageManager.messages has three accessors: get (read), set (synchronous publish via _messagesSubject.send()), and _modify (defers publish via scheduleDeferredPublish() → Task)
• .append() on a COW array triggers _modify, NOT set — so the Combine publish gets deferred to the next cooperative executor turn
• batchUpdateMessages publishes synchronously (_messagesSubject.send() inline), cancels any pending deferred publish, and emits exactly one Observation notification
• ChatPaginationState subscribes to _messagesSubject → recomputeVisibleMessages() → paginatedVisibleMessages — this is the array SwiftUI renders. Deferred publish = deferred display.

Closure capture is safe: Both sites capture local ChatMessage structs (userMsg/userMessage). batchUpdateMessages takes a synchronous, non-escaping (inout [ChatMessage]) -> Void. No retain cycle or lifetime issue.

No circular fix risk: PR #23464 introduced both scheduleDeferredPublish and batchUpdateMessages together. It converted all loop mutations to batchUpdateMessages but left these two single-append sites on the raw _modify path. This PR finishes that migration for the append sites.

Follow-up consideration: The PR description correctly notes remaining _modify callers at lines 429, 454, 701, 733, 743-745, 823 (subscript status mutations like messages[idx].status = .sent). These also defer publish but are less user-visible since they update existing messages rather than inserting new ones. Worth a follow-up if status transitions feel laggy.

Ship it. This directly fixes the P2 LUM-929 user-visible regression.