From 43d0668f818ee1e4753e9bc2dddc0422188b6ac3 Mon Sep 17 00:00:00 2001 From: Terence Pae Date: Fri, 9 Jan 2026 13:19:28 -0800 Subject: [PATCH] fixed issue with rerendering blocks --- .../OsaurusCore/Models/ContentBlock.swift | 45 ++++- Packages/OsaurusCore/Views/ChatView.swift | 143 ++++---------- .../Views/Components/MessageThreadView.swift | 182 ++++++++++++++++++ 3 files changed, 264 insertions(+), 106 deletions(-) create mode 100644 Packages/OsaurusCore/Views/Components/MessageThreadView.swift diff --git a/Packages/OsaurusCore/Models/ContentBlock.swift b/Packages/OsaurusCore/Models/ContentBlock.swift index 69ca63f..a78caa0 100644 --- a/Packages/OsaurusCore/Models/ContentBlock.swift +++ b/Packages/OsaurusCore/Models/ContentBlock.swift @@ -26,7 +26,7 @@ struct ToolCallItem: Equatable { } /// The kind/type of a content block -enum ContentBlockKind { +enum ContentBlockKind: Equatable { case header(role: MessageRole, personaName: String, isFirstInGroup: Bool) case paragraph(index: Int, text: String, isStreaming: Bool, role: MessageRole) case toolCall(call: ToolCall, result: String?) @@ -35,6 +35,46 @@ enum ContentBlockKind { case image(index: Int, imageData: Data) case typingIndicator case groupSpacer + + /// Custom Equatable optimized for performance during streaming. + /// Uses text length comparison as a cheap proxy for content change detection. + static func == (lhs: ContentBlockKind, rhs: ContentBlockKind) -> Bool { + switch (lhs, rhs) { + case let (.header(lRole, lName, lFirst), .header(rRole, rName, rFirst)): + return lRole == rRole && lName == rName && lFirst == rFirst + + case let (.paragraph(lIdx, lText, lStream, lRole), .paragraph(rIdx, rText, rStream, rRole)): + // Compare text length first (O(1)) - if lengths differ, content changed + // Only do full comparison if lengths are equal (rare during streaming) + guard lIdx == rIdx && lStream == rStream && lRole == rRole else { return false } + guard lText.count == rText.count else { return false } + return lText == rText + + case let (.toolCall(lCall, lResult), .toolCall(rCall, rResult)): + return lCall.id == rCall.id && lResult == rResult + + case let (.toolCallGroup(lCalls), .toolCallGroup(rCalls)): + return lCalls == rCalls + + case let (.thinking(lIdx, lText, lStream), .thinking(rIdx, rText, rStream)): + // Same optimization as paragraph + guard lIdx == rIdx && lStream == rStream else { return false } + guard lText.count == rText.count else { return false } + return lText == rText + + case let (.image(lIdx, lData), .image(rIdx, rData)): + return lIdx == rIdx && lData == rData + + case (.typingIndicator, .typingIndicator): + return true + + case (.groupSpacer, .groupSpacer): + return true + + default: + return false + } + } } // MARK: - ContentBlock @@ -56,7 +96,8 @@ struct ContentBlock: Identifiable, Equatable { } static func == (lhs: ContentBlock, rhs: ContentBlock) -> Bool { - lhs.id == rhs.id && lhs.position == rhs.position + // Check id first (cheapest), then position, then kind (most expensive) + lhs.id == rhs.id && lhs.position == rhs.position && lhs.kind == rhs.kind } func withPosition(_ newPosition: BlockPosition) -> ContentBlock { diff --git a/Packages/OsaurusCore/Views/ChatView.swift b/Packages/OsaurusCore/Views/ChatView.swift index 2585945..1c2ab26 100644 --- a/Packages/OsaurusCore/Views/ChatView.swift +++ b/Packages/OsaurusCore/Views/ChatView.swift @@ -1504,124 +1504,45 @@ struct ChatView: View { // MARK: - Message Thread + /// Isolated message thread view to prevent cascading re-renders private func messageThread(_ width: CGFloat) -> some View { // Use flattened content blocks for efficient LazyVStack recycling let blocks = session.visibleBlocks let displayName = windowState.cachedPersonaDisplayName - return ScrollViewReader { proxy in - ScrollView { - LazyVStack(spacing: 0) { - ForEach(blocks) { block in - ContentBlockView( - block: block, - width: width, - personaName: displayName, - onCopy: { turnId in - copyTurnContent(turnId: turnId) - }, - onRegenerate: { turnId in - session.regenerate(turnId: turnId) - } - ) - .padding(.horizontal, 16) - } - - // Bottom padding for visual breathing room - Color.clear.frame(height: 16) - - // Bottom anchor for scroll tracking - Color.clear - .frame(height: 1) - .id("BOTTOM") - .onAppear { isPinnedToBottom = true } - .onDisappear { - // Only unpin if we're not streaming - // During streaming, the bottom marker may temporarily disappear - // due to content growth and LazyVStack recycling - if !session.isStreaming { - isPinnedToBottom = false - } - } - } - .padding(.top, 8) - } - .scrollContentBackground(.hidden) - .scrollIndicators(.hidden) - .overlay(alignment: .bottomTrailing) { - scrollToBottomButton(proxy: proxy) - } - .onChange(of: session.turns.count) { _, _ in - if isPinnedToBottom { - // Defer scroll to next run loop to allow layout to complete - // This is especially important for voice input where the overlay - // is closing at the same time as the message is added - DispatchQueue.main.async { - withAnimation(theme.animationQuick()) { - proxy.scrollTo("BOTTOM", anchor: .bottom) - } - } - } - } - .onChange(of: session.scrollTick) { _, _ in - // During streaming, always scroll to follow content - // Otherwise, only scroll if pinned to bottom - guard isPinnedToBottom || session.isStreaming else { return } - - // Defer scroll to next run loop to allow layout to complete - DispatchQueue.main.async { - // Disable animation during streaming for performance - var transaction = Transaction() - transaction.disablesAnimations = true - withTransaction(transaction) { - proxy.scrollTo("BOTTOM", anchor: .bottom) - } - } - } - // Note: Removed onChange(of: session.turns.last?.content.count) handler - // as it was forcing lazy string joins on every content update. - // scrollTick already handles scroll updates during streaming. + return ZStack { + MessageThreadView( + blocks: blocks, + width: width, + personaName: displayName, + isStreaming: session.isStreaming, + turnsCount: session.turns.count, + scrollTick: session.scrollTick, + onCopy: copyTurnContent, + onRegenerate: regenerateTurn, + onScrolledToBottom: { isPinnedToBottom = true }, + onScrolledAwayFromBottom: { isPinnedToBottom = false } + ) .onReceive(NotificationCenter.default.publisher(for: .chatOverlayActivated)) { _ in - proxy.scrollTo("BOTTOM", anchor: .bottom) isPinnedToBottom = true } - } - } - @ViewBuilder - private func scrollToBottomButton(proxy: ScrollViewProxy) -> some View { - if !isPinnedToBottom && !session.turns.isEmpty { - Button(action: { - withAnimation(theme.springAnimation()) { - proxy.scrollTo("BOTTOM", anchor: .bottom) - } - isPinnedToBottom = true - }) { - Image(systemName: "chevron.down") - .font(.system(size: 12, weight: .semibold)) - .foregroundColor(theme.secondaryText) - .frame(width: 32, height: 32) - .background( - Circle() - .fill(theme.secondaryBackground) - .shadow(color: Color.black.opacity(0.2), radius: 8, x: 0, y: 2) + // Scroll button overlay - isolated from content + VStack { + Spacer() + HStack { + Spacer() + ScrollToBottomButton( + isPinnedToBottom: isPinnedToBottom, + hasTurns: !session.turns.isEmpty, + onTap: { isPinnedToBottom = true } ) + } } - .buttonStyle(.plain) - .padding(20) - .transition(.scale.combined(with: .opacity)) } } - // MARK: - Helpers - - private func displayModelName(_ raw: String?) -> String { - guard let raw else { return "Model" } - if raw.lowercased() == "foundation" { return "Foundation" } - if let last = raw.split(separator: "/").last { return String(last) } - return raw - } - + /// Stable callback for copy action - prevents closure recreation private func copyTurnContent(turnId: UUID) { guard let turn = session.turns.first(where: { $0.id == turnId }) else { return } @@ -1645,6 +1566,20 @@ struct ChatView: View { NSPasteboard.general.setString(textToCopy, forType: .string) } + /// Stable callback for regenerate action - prevents closure recreation + private func regenerateTurn(turnId: UUID) { + session.regenerate(turnId: turnId) + } + + // MARK: - Helpers + + private func displayModelName(_ raw: String?) -> String { + guard let raw else { return "Model" } + if raw.lowercased() == "foundation" { return "Foundation" } + if let last = raw.split(separator: "/").last { return String(last) } + return raw + } + private func resizeWindowForContent(isEmpty: Bool) { guard let window = hostWindow else { return } diff --git a/Packages/OsaurusCore/Views/Components/MessageThreadView.swift b/Packages/OsaurusCore/Views/Components/MessageThreadView.swift new file mode 100644 index 0000000..9a45c5e --- /dev/null +++ b/Packages/OsaurusCore/Views/Components/MessageThreadView.swift @@ -0,0 +1,182 @@ +// +// MessageThreadView.swift +// osaurus +// +// Isolated message thread view to prevent cascading re-renders. +// Observes only the session data it needs, not all ChatView state. +// + +import SwiftUI + +// MARK: - Message Thread View + +/// An isolated view for rendering the message thread. +/// This prevents ChatView state changes (like isPinnedToBottom) from causing +/// all ContentBlockViews to re-render. +struct MessageThreadView: View { + let blocks: [ContentBlock] + let width: CGFloat + let personaName: String + let isStreaming: Bool + let turnsCount: Int + let scrollTick: Int + + // Callbacks - excluded from Equatable comparison in child views + let onCopy: (UUID) -> Void + let onRegenerate: (UUID) -> Void + let onScrolledToBottom: () -> Void + let onScrolledAwayFromBottom: () -> Void + + @Environment(\.theme) private var theme + + var body: some View { + ScrollViewReader { proxy in + ScrollView { + // Use EquatableView to skip body evaluation when blocks haven't changed + EquatableView( + content: MessageBlocksList( + blocks: blocks, + width: width, + personaName: personaName, + onCopy: onCopy, + onRegenerate: onRegenerate + ) + ) + + // Bottom padding for visual breathing room + Color.clear.frame(height: 16) + + // Bottom anchor for scroll tracking + Color.clear + .frame(height: 1) + .id("BOTTOM") + .onAppear { onScrolledToBottom() } + .onDisappear { + // Only unpin if we're not streaming + if !isStreaming { + onScrolledAwayFromBottom() + } + } + } + .scrollContentBackground(.hidden) + .scrollIndicators(.hidden) + .onChange(of: turnsCount) { _, _ in + scrollToBottom(proxy: proxy, animated: true) + } + .onChange(of: scrollTick) { _, _ in + scrollToBottom(proxy: proxy, animated: false) + } + } + } + + private func scrollToBottom(proxy: ScrollViewProxy, animated: Bool) { + DispatchQueue.main.async { + if animated { + withAnimation(theme.animationQuick()) { + proxy.scrollTo("BOTTOM", anchor: .bottom) + } + } else { + var transaction = Transaction() + transaction.disablesAnimations = true + withTransaction(transaction) { + proxy.scrollTo("BOTTOM", anchor: .bottom) + } + } + } + } +} + +// MARK: - Message Blocks List + +/// Isolated list view that only re-renders when blocks change. +/// Uses Equatable conformance to prevent unnecessary updates. +private struct MessageBlocksList: View, Equatable { + let blocks: [ContentBlock] + let width: CGFloat + let personaName: String + let onCopy: (UUID) -> Void + let onRegenerate: (UUID) -> Void + + // Custom Equatable - only compare blocks and width, not closures + // nonisolated to avoid actor isolation issues with SwiftUI views + nonisolated static func == (lhs: MessageBlocksList, rhs: MessageBlocksList) -> Bool { + lhs.blocks == rhs.blocks && lhs.width == rhs.width && lhs.personaName == rhs.personaName + } + + var body: some View { + LazyVStack(spacing: 0) { + ForEach(blocks) { block in + // Use EquatableView to skip body evaluation when block hasn't changed + EquatableView( + content: ContentBlockRow( + block: block, + width: width, + personaName: personaName, + onCopy: onCopy, + onRegenerate: onRegenerate + ) + ) + } + } + .padding(.top, 8) + } +} + +// MARK: - Content Block Row + +/// Individual block row with Equatable conformance to prevent re-renders +/// when the block hasn't changed. +private struct ContentBlockRow: View, Equatable { + let block: ContentBlock + let width: CGFloat + let personaName: String + let onCopy: (UUID) -> Void + let onRegenerate: (UUID) -> Void + + // Custom Equatable - only compare block, not closures + // nonisolated to avoid actor isolation issues with SwiftUI views + nonisolated static func == (lhs: ContentBlockRow, rhs: ContentBlockRow) -> Bool { + lhs.block == rhs.block && lhs.width == rhs.width && lhs.personaName == rhs.personaName + } + + var body: some View { + ContentBlockView( + block: block, + width: width, + personaName: personaName, + onCopy: onCopy, + onRegenerate: onRegenerate + ) + .padding(.horizontal, 16) + } +} + +// MARK: - Scroll to Bottom Button + +/// Isolated scroll button that doesn't trigger re-renders of content blocks +struct ScrollToBottomButton: View { + let isPinnedToBottom: Bool + let hasTurns: Bool + let onTap: () -> Void + + @Environment(\.theme) private var theme + + var body: some View { + if !isPinnedToBottom && hasTurns { + Button(action: onTap) { + Image(systemName: "chevron.down") + .font(.system(size: 12, weight: .semibold)) + .foregroundColor(theme.secondaryText) + .frame(width: 32, height: 32) + .background( + Circle() + .fill(theme.secondaryBackground) + .shadow(color: Color.black.opacity(0.2), radius: 8, x: 0, y: 2) + ) + } + .buttonStyle(.plain) + .padding(20) + .transition(.scale.combined(with: .opacity)) + } + } +}