Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion clients/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,8 @@ Prefer migrating the parent to `@Observable` so the bridge becomes unnecessary (
- **Keep modifier chains lean.** Every SwiftUI modifier wraps its content in a new view value. Long chains of redundant or duplicated modifiers deepen the view tree and increase diffing cost. Group related modifiers, remove redundant ones (e.g., don't set `.font` on every row when the parent already sets it), and extract heavily-modified subtrees into standalone `@ViewBuilder` methods.
- **Scope observation narrowly.** Only observe the specific properties a view needs. Prefer granular `@Observable` properties or `withObservationTracking` over observing an entire store that publishes on unrelated changes. When a view reads `@Observable` properties from many different objects, extract logical sections (toolbar, overlays, content) into standalone child `View` structs — each child creates its own [observation tracking scope](https://developer.apple.com/videos/play/wwdc2023/10149/) so changes to one section's state don't re-evaluate the entire parent body. **Note:** private `@ViewBuilder` functions do NOT create separate observation scopes — they execute during the caller's `body` evaluation, so all their `@Observable` reads are tracked in the caller's scope. Use standalone `View` structs or [`ObservationBoundaryView`](https://developer.apple.com/videos/play/wwdc2023/10149/) (deferred `@escaping @ViewBuilder` closure) when extraction into a full `View` struct is impractical.
- **Don't double-track scroll viewport geometry.** When an ancestor scroll container already publishes its visible height (e.g. via [`onScrollGeometryChange`](https://developer.apple.com/documentation/swiftui/view/onscrollgeometrychange(for:of:_:))), expose it to descendants through a file-local [`EnvironmentKey`](https://developer.apple.com/documentation/swiftui/environmentkey) instead of re-measuring with `containerRelativeFrame` + [`onGeometryChange`](https://developer.apple.com/documentation/swiftui/view/ongeometrychange(for:of:action:)) inside descendant views. Each independent `@State` viewport probe inside a large view hierarchy adds another layout pass that re-traverses the hierarchy on every viewport change.
- **Coalesce N writes into one when mutating `@Observable` stored properties in a loop.** When a stored property's `didSet` runs derived work (e.g. `ConversationListStore.conversations` triggers `recomputeDerivedProperties`), build a local `var snapshot = property`, mutate elements in the snapshot, and write `property = snapshot` once after the loop. N independent writes fire N `didSet` invocations — each doing the full derived-property recompute — scaling as O(N × work) on the main actor. The snapshot pattern collapses this to a single `didSet`. See `markConversationsSeenImpl`, `restoreUnseen`, `appendConversations`, and `deleteGroup` in `ConversationListStore.swift` for the reference implementations. When a value-level helper exists (e.g. `applyAssistantAttention(from:into:)`), prefer it over the single-row convenience wrapper so the caller retains control over when the writeback fires.
- **Coalesce N writes into one when mutating `@Observable` stored properties in a loop.** When a stored property's `didSet` runs derived work (e.g. `ConversationListStore.conversations` triggers `recomputeDerivedProperties`), build a local `var snapshot = property`, mutate elements in the snapshot, and write `property = snapshot` once after the loop. N independent writes fire N `didSet` invocations — each doing the full derived-property recompute — scaling as O(N × work) on the main actor. The snapshot pattern collapses this to a single `didSet`. See `markConversationsSeenImpl`, `restoreUnseen`, `appendConversations`, and `deleteGroup` in `ConversationListStore.swift` for the reference implementations. When a value-level helper exists (e.g. `applyAssistantAttention(from:into:)`, `applyLastInteracted(into:)`), prefer it over the single-row convenience wrapper so the caller retains control over when the writeback fires.
- **Guard `didSet` and writebacks against no-op mutations.** Array subscript assignment (`array[i] = value`) and whole-array assignment both fire `didSet` even when the new value equals the old. For properties whose `didSet` triggers expensive derived work, add `guard property != oldValue else { return }` in the `didSet`. For copy-modify-writeback sites, add `guard modified != original` before writing back. This eliminates redundant recomputes during high-frequency callbacks (e.g. streaming activity snapshots) where most invocations don't actually change the model.
- **Cache derived booleans from high-frequency collections.** Reading `.isEmpty`, `.count`, `.first`, or `.contains` on an `@Observable` stored property [tracks the property, not the value](https://developer.apple.com/videos/play/wwdc2023/10149/) — any mutation invalidates the view even when the derived value hasn't changed, including reads inside `.onChange(of:)`. For frequently-mutated collections, maintain a separate stored scalar / lookup updated in `didSet` behind a `!=` guard, and route view-body and `.onChange` reads through the cached form. To prevent regression, mark the underlying collection [`@ObservationIgnored`](https://developer.apple.com/documentation/observation/observationignored()) so accidental view-body reads silently stop reacting (caught at code review) rather than working but expensive.
- **Use `.task` + async sequences instead of `.onReceive` for `NotificationCenter`.** Each `.onReceive` modifier adds a generic wrapper layer to the view type and exists only for side effects — no rendering value. Use a single [`.task`](https://developer.apple.com/documentation/swiftui/view/task(priority:_:)) with [`NotificationCenter.notifications(named:)`](https://developer.apple.com/documentation/foundation/notificationcenter/notifications(named:object:)) async sequences instead. `.task` has identical lifecycle semantics (auto-cancelled on view disappear) without inflating the view type.
- **Enable non-contiguous layout on TextKit 1 stacks in `NSViewRepresentable`.** When constructing an `NSLayoutManager` for an `NSTextView` hosted via `NSViewRepresentable`, set `layoutManager.allowsNonContiguousLayout = true`. The default (`false`) forces full-document glyph layout from character 0 on the main thread whenever a glyph range is queried — which happens the first time the text view is attached to its hosting view (`setDocumentView:` / `addSubview:` → `_setSuperview:` → `setNeedsDisplayInRect:` → `_glyphRangeForBoundingRect:`). Reference: [`NSLayoutManager.allowsNonContiguousLayout`](https://developer.apple.com/documentation/appkit/nslayoutmanager/allowsnoncontiguouslayout). **Exception — streaming text with a separate measurement stack:** when the representable measures height via a sibling TextKit stack that always fully lays out every glyph (e.g. `ensureLayout(for:)` + `usedRect(for:)`), the measured frame assumes the render stack will also lay out every glyph in that rect. Non-contiguous layout leaves glyphs pending until the view scrolls or draws them, which races with streaming updates — producing a gap (render paints a smaller laid-out region inside the correct frame) and then overlap (the next sibling is placed via the measured frame and the lazy glyphs later paint outside it). In that case set `allowsNonContiguousLayout = false` and document why. See `clients/shared/DesignSystem/Components/Display/SelectableTextView.swift` (lines 180–205) for the canonical example; `VCodeView` / `HighlightedTextView` still opt into non-contiguous layout independently because they do not share this measurement model.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@ final class ConversationListStore {
///
/// Reference: [`@ObservationIgnored`](https://developer.apple.com/documentation/observation/observationignored()).
@ObservationIgnored var conversations: [ConversationModel] = [] {
didSet { recomputeDerivedProperties() }
didSet {
guard conversations != oldValue else { return }
recomputeDerivedProperties()
}
}
var groups: [ConversationGroup] = [] {
didSet { recomputeDerivedProperties() }
Expand Down Expand Up @@ -531,11 +534,13 @@ final class ConversationListStore {

func updateConversationTitle(id: UUID, title: String) {
guard let index = conversations.firstIndex(where: { $0.id == id }) else { return }
guard conversations[index].title != title else { return }
conversations[index].title = title
}

func updateConversationInferenceProfile(id: UUID, profile: String?) {
guard let index = conversations.firstIndex(where: { $0.id == id }) else { return }
guard conversations[index].inferenceProfile != profile else { return }
conversations[index].inferenceProfile = profile
}

Expand All @@ -546,6 +551,7 @@ final class ConversationListStore {
let trimmed = title.trimmingCharacters(in: .whitespacesAndNewlines)
guard !trimmed.isEmpty else { return }
guard let index = conversations.firstIndex(where: { $0.id == id }) else { return }
guard conversations[index].title != trimmed else { return }
conversations[index].title = trimmed
if let conversationId = conversations[index].conversationId {
Task { await conversationListClient.renameConversation(conversationId: conversationId, name: trimmed) }
Expand All @@ -556,20 +562,28 @@ final class ConversationListStore {

// MARK: - Recency

func updateLastInteracted(conversationId: UUID) {
guard let index = conversations.firstIndex(where: { $0.id == conversationId }) else { return }
var conversation = conversations[index]
/// Value-level helper: apply last-interacted timestamp to a local snapshot.
/// Returns `true` if a pin-change notification is needed (displayOrder was
/// cleared for a Recents conversation). Callers that process multiple
/// mutations can use this to coalesce writes into a single `conversations`
/// writeback — same pattern as `applyAssistantAttention(from:into:)`.
@discardableResult
func applyLastInteracted(into conversation: inout ConversationModel) -> Bool {
conversation.lastInteractedAt = Date()
if conversation.groupId == ConversationGroup.all.id {
// Clear explicit displayOrder so Recents conversations revert to recency-based sorting.
// Grouped conversations keep their manual ordering.
let hadOrder = conversation.displayOrder != nil
if hadOrder { conversation.displayOrder = nil }
conversations[index] = conversation
if hadOrder { sendPinChange(for: conversation) }
} else {
conversations[index] = conversation
return hadOrder
}
return false
}

func updateLastInteracted(conversationId: UUID) {
guard let index = conversations.firstIndex(where: { $0.id == conversationId }) else { return }
var conversation = conversations[index]
let needsPinChange = applyLastInteracted(into: &conversation)
conversations[index] = conversation
if needsPinChange { sendPinChange(for: conversation) }
}

// MARK: - Pinning & Ordering
Expand Down Expand Up @@ -1210,6 +1224,7 @@ final class ConversationListStore {
) {
var conversation = conversations[index]
applyAssistantAttention(from: item, into: &conversation)
guard conversation != conversations[index] else { return }
conversations[index] = conversation
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1579,10 +1579,14 @@ final class ConversationManager: ConversationRestorerDelegate {
}
guard let index = listStore.conversations.firstIndex(where: { $0.id == conversationId }) else { return }
let isNewMessage = previousSnapshot?.messageId != currentSnapshot.messageId
if isNewMessage && !listStore.conversations[index].isBackgroundConversation {
listStore.updateLastInteracted(conversationId: conversationId)
}

// Snapshot-mutate-writeback: coalesce updateLastInteracted + attention
// mutations into a single conversations write to avoid 2× recompute.
var conversation = listStore.conversations[index]
var needsPinChange = false
if isNewMessage && !conversation.isBackgroundConversation {
needsPinChange = listStore.applyLastInteracted(into: &conversation)
}
if conversation.latestAssistantMessageAt == nil || isNewMessage {
conversation.latestAssistantMessageAt = Date()
}
Expand All @@ -1599,6 +1603,7 @@ final class ConversationManager: ConversationRestorerDelegate {
conversation.hasUnseenLatestAssistantMessage = true
}
listStore.conversations[index] = conversation
if needsPinChange { listStore.sendPinChange(for: conversation) }
if shouldEmitSeenSignal, let daemonId = conversation.conversationId {
listStore.emitConversationSeenSignal(conversationId: daemonId)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,7 @@ final class ConversationRestorer {
func handleConversationTitleUpdated(_ response: ConversationTitleUpdatedMessage) {
guard let delegate else { return }
guard let index = delegate.conversations.firstIndex(where: { $0.conversationId == response.conversationId }) else { return }
guard delegate.conversations[index].title != response.title else { return }
delegate.conversations[index].title = response.title
}

Expand Down