mirror of
https://github.com/awizemann/scarf.git
synced 2026-05-10 02:26:37 +00:00
perf(chat): stop O(n)-per-token re-render of settled bubbles (#46)
Long chats progressively bog down and eventually crash because every streamed ACP token triggers a full messageGroups rebuild plus a body re-evaluation of every MessageGroupView and RichMessageBubble — even the n-1 settled groups that haven't changed. Three changes cap per-chunk work at "patch the trailing group + re-render the streaming bubble": - MessageGroupView and RichMessageBubble are now Equatable, applied via .equatable() in the ForEach. Settled groups (no streaming message inside) short-circuit body re-evaluation entirely; the streaming group compares content/reasoning/toolCalls.count so it still redraws on every chunk. - RichChatViewModel.upsertStreamingMessage no longer calls buildMessageGroups() per chunk. New patchTrailingGroupForStreaming mutates only the trailing group's assistant entry in place. The 9 other call sites of buildMessageGroups() are untouched — they cover structural events (user message, tool-call complete, finalize, session resume) where group boundaries can actually change, and a full rebuild is correct there. - MessageGroup.toolKindCounts is now a model property (was a MessageGroupView computed prop that re-walked O(m × k) per body render). Lives behind the Equatable short-circuit. - ToolCallCard.formatJSON cached via .task(id: call.callId) so JSON pretty-printing runs once per card lifetime instead of on every expand/collapse + every neighbour's re-render. Seeded with raw arguments to avoid a first-frame empty-text flicker. - ToolResultContent.lines/preview cached via .task(id: content) — the prior pair of computed properties split content on \n twice per render, expensive on long command/file output. Skipped from the original plan: the per-message parse cache (rendered moot once Equatable already short-circuits settled bubbles) and the LazyVStack switch (deferred — RichChatMessageList comments flag scroll-anchor regression risk; revisit separately if needed). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -27,6 +27,21 @@ public struct MessageGroup: Identifiable {
|
||||
public var toolCallCount: Int {
|
||||
assistantMessages.reduce(0) { $0 + $1.toolCalls.count }
|
||||
}
|
||||
|
||||
/// Aggregated `ToolKind → count` over all assistant tool calls in
|
||||
/// this group. Lives on the model so SwiftUI's Equatable
|
||||
/// short-circuit (issue #46) covers it — previously this was a
|
||||
/// `MessageGroupView` computed property that re-walked O(m × k)
|
||||
/// per group on every body re-evaluation.
|
||||
public var toolKindCounts: [ToolKind: Int] {
|
||||
var counts: [ToolKind: Int] = [:]
|
||||
for msg in assistantMessages where msg.isAssistant {
|
||||
for call in msg.toolCalls {
|
||||
counts[call.toolKind, default: 0] += 1
|
||||
}
|
||||
}
|
||||
return counts
|
||||
}
|
||||
}
|
||||
|
||||
@Observable
|
||||
@@ -759,7 +774,42 @@ public final class RichChatViewModel {
|
||||
} else {
|
||||
messages.append(msg)
|
||||
}
|
||||
buildMessageGroups()
|
||||
patchTrailingGroupForStreaming(streamingMsg: msg)
|
||||
}
|
||||
|
||||
/// Per-chunk fast path for `messageGroups` (issue #46). Mutates
|
||||
/// only the trailing group's assistant entry instead of rebuilding
|
||||
/// the entire `messageGroups` array via `buildMessageGroups()` on
|
||||
/// every streamed token.
|
||||
///
|
||||
/// Falls back to a full rebuild whenever it can't safely patch:
|
||||
/// - no trailing group exists yet (e.g. first chunk after `reset`)
|
||||
/// - the trailing group is a user-only group (the very first chunk
|
||||
/// of a brand-new turn — we need a full rebuild so the assistant
|
||||
/// is grouped under the right user message)
|
||||
///
|
||||
/// Other call sites of `buildMessageGroups()` are intentionally
|
||||
/// untouched: they handle structural events (user message, tool
|
||||
/// call complete, finalize, session resume) where group boundaries
|
||||
/// can change, and a full rebuild is the right move there.
|
||||
private func patchTrailingGroupForStreaming(streamingMsg: HermesMessage) {
|
||||
guard let lastIdx = messageGroups.indices.last else {
|
||||
buildMessageGroups()
|
||||
return
|
||||
}
|
||||
let trailing = messageGroups[lastIdx]
|
||||
var assistants = trailing.assistantMessages
|
||||
if let i = assistants.firstIndex(where: { $0.id == Self.streamingId }) {
|
||||
assistants[i] = streamingMsg
|
||||
} else {
|
||||
assistants.append(streamingMsg)
|
||||
}
|
||||
messageGroups[lastIdx] = MessageGroup(
|
||||
id: trailing.id,
|
||||
userMessage: trailing.userMessage,
|
||||
assistantMessages: assistants,
|
||||
toolResults: trailing.toolResults
|
||||
)
|
||||
}
|
||||
|
||||
/// Convert the streaming message (id=0) into a permanent message and reset streaming state.
|
||||
|
||||
@@ -59,6 +59,7 @@ struct RichChatMessageList: View {
|
||||
|
||||
ForEach(groups) { group in
|
||||
MessageGroupView(group: group, turnDurations: turnDurations)
|
||||
.equatable()
|
||||
.id("group-\(group.id)")
|
||||
}
|
||||
|
||||
@@ -136,7 +137,7 @@ struct RichChatMessageList: View {
|
||||
}
|
||||
}
|
||||
|
||||
struct MessageGroupView: View {
|
||||
struct MessageGroupView: View, Equatable {
|
||||
let group: MessageGroup
|
||||
/// Wall-clock turn durations keyed by assistant-message id (v2.5).
|
||||
/// Forwarded into `RichMessageBubble` so the metadata footer can
|
||||
@@ -144,10 +145,47 @@ struct MessageGroupView: View {
|
||||
/// that haven't been updated yet still compile.
|
||||
var turnDurations: [Int: TimeInterval] = [:]
|
||||
|
||||
/// Equatable short-circuit for SwiftUI: when the trailing group's
|
||||
/// streaming bubble grows, only that group's `==` returns false.
|
||||
/// All earlier groups skip body re-evaluation, dropping per-chunk
|
||||
/// render work from O(n) to O(1) for settled groups (issue #46).
|
||||
///
|
||||
/// What participates:
|
||||
/// - `group.id` (primary key — stable sequential index).
|
||||
/// - assistant-message id list (additions / finalize-id-flip).
|
||||
/// - For the streaming message (id == 0): content, reasoning,
|
||||
/// reasoningContent, toolCalls.count — the only fields that
|
||||
/// mutate while streaming.
|
||||
/// - `turnDurations[msg.id]` for assistants in this group only —
|
||||
/// the dict is large and shared across groups, but each group
|
||||
/// only renders its own entries.
|
||||
/// - `group.toolResults.count` — append-only within a group.
|
||||
static func == (lhs: MessageGroupView, rhs: MessageGroupView) -> Bool {
|
||||
guard lhs.group.id == rhs.group.id else { return false }
|
||||
guard lhs.group.userMessage?.id == rhs.group.userMessage?.id else { return false }
|
||||
guard lhs.group.userMessage?.content == rhs.group.userMessage?.content else { return false }
|
||||
guard lhs.group.assistantMessages.count == rhs.group.assistantMessages.count else { return false }
|
||||
for (l, r) in zip(lhs.group.assistantMessages, rhs.group.assistantMessages) {
|
||||
if l.id != r.id { return false }
|
||||
if l.id == 0 {
|
||||
if l.content != r.content { return false }
|
||||
if l.reasoning != r.reasoning { return false }
|
||||
if l.reasoningContent != r.reasoningContent { return false }
|
||||
if l.toolCalls.count != r.toolCalls.count { return false }
|
||||
}
|
||||
}
|
||||
if lhs.group.toolResults.count != rhs.group.toolResults.count { return false }
|
||||
for msg in lhs.group.assistantMessages where msg.isAssistant && msg.id != 0 {
|
||||
if lhs.turnDurations[msg.id] != rhs.turnDurations[msg.id] { return false }
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
var body: some View {
|
||||
VStack(alignment: .leading, spacing: 8) {
|
||||
if let user = group.userMessage {
|
||||
RichMessageBubble(message: user, toolResults: [:])
|
||||
.equatable()
|
||||
}
|
||||
|
||||
// Identify by array offset rather than `message.id`. The
|
||||
@@ -166,6 +204,7 @@ struct MessageGroupView: View {
|
||||
toolResults: group.toolResults,
|
||||
turnDuration: turnDurations[message.id]
|
||||
)
|
||||
.equatable()
|
||||
}
|
||||
|
||||
if group.toolCallCount > 1 {
|
||||
@@ -176,7 +215,7 @@ struct MessageGroupView: View {
|
||||
|
||||
@ViewBuilder
|
||||
private var toolSummary: some View {
|
||||
let kinds = toolKindCounts
|
||||
let kinds = group.toolKindCounts
|
||||
if !kinds.isEmpty {
|
||||
HStack(spacing: 4) {
|
||||
Image(systemName: "wrench")
|
||||
@@ -190,16 +229,6 @@ struct MessageGroupView: View {
|
||||
}
|
||||
}
|
||||
|
||||
private var toolKindCounts: [ToolKind: Int] {
|
||||
var counts: [ToolKind: Int] = [:]
|
||||
for msg in group.assistantMessages where msg.isAssistant {
|
||||
for call in msg.toolCalls {
|
||||
counts[call.toolKind, default: 0] += 1
|
||||
}
|
||||
}
|
||||
return counts
|
||||
}
|
||||
|
||||
private func summaryText(_ kinds: [ToolKind: Int]) -> String {
|
||||
let total = kinds.values.reduce(0, +)
|
||||
let parts = kinds.sorted(by: { $0.value > $1.value })
|
||||
|
||||
@@ -2,7 +2,7 @@ import SwiftUI
|
||||
import ScarfCore
|
||||
import ScarfDesign
|
||||
|
||||
struct RichMessageBubble: View {
|
||||
struct RichMessageBubble: View, Equatable {
|
||||
let message: HermesMessage
|
||||
let toolResults: [String: HermesMessage]
|
||||
/// Wall-clock duration of the agent turn this assistant message
|
||||
@@ -14,6 +14,29 @@ struct RichMessageBubble: View {
|
||||
|
||||
@Environment(ChatViewModel.self) private var chatViewModel
|
||||
|
||||
/// SwiftUI body short-circuit (issue #46). Settled bubbles
|
||||
/// (`message.id != 0`) are immutable — id equality plus a couple
|
||||
/// of cheap stored-field comparisons is sufficient. The streaming
|
||||
/// bubble (id == 0) gets a content + reasoning + toolCalls.count
|
||||
/// comparison so it correctly redraws on every chunk.
|
||||
/// `toolResults` is compared by count: results are append-only
|
||||
/// within a group, so a count change implies a new tool result.
|
||||
static func == (lhs: RichMessageBubble, rhs: RichMessageBubble) -> Bool {
|
||||
guard lhs.message.id == rhs.message.id else { return false }
|
||||
if lhs.message.id == 0 {
|
||||
return lhs.message.content == rhs.message.content
|
||||
&& lhs.message.reasoning == rhs.message.reasoning
|
||||
&& lhs.message.reasoningContent == rhs.message.reasoningContent
|
||||
&& lhs.message.toolCalls.count == rhs.message.toolCalls.count
|
||||
&& lhs.turnDuration == rhs.turnDuration
|
||||
&& lhs.toolResults.count == rhs.toolResults.count
|
||||
}
|
||||
return lhs.turnDuration == rhs.turnDuration
|
||||
&& lhs.toolResults.count == rhs.toolResults.count
|
||||
&& lhs.message.tokenCount == rhs.message.tokenCount
|
||||
&& lhs.message.finishReason == rhs.message.finishReason
|
||||
}
|
||||
|
||||
var body: some View {
|
||||
if message.isUser {
|
||||
userBubble
|
||||
|
||||
@@ -16,6 +16,12 @@ struct ToolCallCard: View {
|
||||
var onFocus: (() -> Void)? = nil
|
||||
|
||||
@State private var expanded = false
|
||||
/// Pretty-printed `call.arguments`. Computed once per `call.callId`
|
||||
/// via `.task(id:)` instead of on every card re-render (issue #46).
|
||||
/// Seeded with the raw arguments so the first frame after expand
|
||||
/// shows readable text instead of a flicker of empty space while
|
||||
/// the task runs.
|
||||
@State private var formattedArgs: String = ""
|
||||
|
||||
var body: some View {
|
||||
VStack(alignment: .leading, spacing: 6) {
|
||||
@@ -77,7 +83,7 @@ struct ToolCallCard: View {
|
||||
Text("ARGUMENTS")
|
||||
.scarfStyle(.captionUppercase)
|
||||
.foregroundStyle(ScarfColor.foregroundMuted)
|
||||
Text(formatJSON(call.arguments))
|
||||
Text(formattedArgs.isEmpty ? call.arguments : formattedArgs)
|
||||
.font(ScarfFont.monoSmall)
|
||||
.foregroundStyle(ScarfColor.foregroundPrimary)
|
||||
.textSelection(.enabled)
|
||||
@@ -102,6 +108,9 @@ struct ToolCallCard: View {
|
||||
.padding(.leading, 4)
|
||||
}
|
||||
}
|
||||
.task(id: call.callId) {
|
||||
formattedArgs = formatJSON(call.arguments)
|
||||
}
|
||||
}
|
||||
|
||||
private var toolLabel: String {
|
||||
@@ -141,13 +150,18 @@ struct ToolResultContent: View {
|
||||
let content: String
|
||||
|
||||
@State private var showAll = false
|
||||
|
||||
private var lines: [String] { content.components(separatedBy: "\n") }
|
||||
private var isLong: Bool { lines.count > 8 }
|
||||
/// Cached line split. The previous computed-property pair
|
||||
/// (`lines` + `isLong`) split `content` twice on every render —
|
||||
/// once for the count check, once for the prefix join. With long
|
||||
/// tool outputs (file contents, command output) this was O(n)
|
||||
/// per render, repeated for every settled card on every chunk
|
||||
/// (issue #46). Now split once per content change via `.task(id:)`.
|
||||
@State private var lines: [String] = []
|
||||
@State private var preview: String = ""
|
||||
|
||||
var body: some View {
|
||||
VStack(alignment: .leading, spacing: 4) {
|
||||
Text(showAll ? content : lines.prefix(8).joined(separator: "\n"))
|
||||
Text(showAll ? content : preview)
|
||||
.font(ScarfFont.monoSmall)
|
||||
.foregroundStyle(ScarfColor.foregroundPrimary)
|
||||
.textSelection(.enabled)
|
||||
@@ -162,7 +176,7 @@ struct ToolResultContent: View {
|
||||
)
|
||||
)
|
||||
|
||||
if isLong {
|
||||
if lines.count > 8 {
|
||||
Button(showAll ? "Show less" : "Show all \(lines.count) lines") {
|
||||
withAnimation { showAll.toggle() }
|
||||
}
|
||||
@@ -171,5 +185,10 @@ struct ToolResultContent: View {
|
||||
.buttonStyle(.plain)
|
||||
}
|
||||
}
|
||||
.task(id: content) {
|
||||
let split = content.components(separatedBy: "\n")
|
||||
lines = split
|
||||
preview = split.prefix(8).joined(separator: "\n")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user