From 222b2cbeb2bffae54e739671f25312c5e1eae298 Mon Sep 17 00:00:00 2001 From: Kent Overstreet Date: Sun, 5 Apr 2026 19:41:16 -0400 Subject: [PATCH] chat: PartialEq on ConversationEntry for proper diff Add PartialEq to Message, FunctionCall, ToolCall, ContentPart, ImageUrl, MessageContent, ConversationEntry. Sync now compares entries directly instead of content lengths. Phase 1 pops mismatched tail entries using PartialEq comparison. Phase 2 pushes new entries with clone into last_entries buffer. TODO: route_entry needs to handle multiple tool calls per entry. Co-Authored-By: Kent Overstreet --- src/agent/api/types.rs | 12 ++-- src/agent/context.rs | 2 +- src/user/chat.rs | 124 +++++++++++++++++++---------------------- 3 files changed, 64 insertions(+), 74 deletions(-) diff --git a/src/agent/api/types.rs b/src/agent/api/types.rs index 0eaa1df..6954c4a 100644 --- a/src/agent/api/types.rs +++ b/src/agent/api/types.rs @@ -10,7 +10,7 @@ use chrono::Utc; use serde::{Deserialize, Serialize}; /// Function call within a tool call — name + JSON arguments. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct FunctionCall { pub name: String, pub arguments: String, @@ -24,7 +24,7 @@ pub struct FunctionCallDelta { } /// A tool call requested by the model. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct ToolCall { pub id: String, #[serde(rename = "type")] @@ -45,7 +45,7 @@ pub struct ToolCallDelta { /// Message content — either plain text or an array of content parts /// (for multimodal messages with images). Serializes as a JSON string /// for text-only, or a JSON array for multimodal. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[serde(untagged)] pub enum MessageContent { Text(String), @@ -70,7 +70,7 @@ impl MessageContent { } /// A single content part within a multimodal message. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[serde(tag = "type")] pub enum ContentPart { #[serde(rename = "text")] @@ -80,13 +80,13 @@ pub enum ContentPart { } /// Image URL — either a real URL or a base64 data URI. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct ImageUrl { pub url: String, } /// A chat message in the conversation. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct Message { pub role: Role, pub content: Option, diff --git a/src/agent/context.rs b/src/agent/context.rs index bbbfb98..3ca6b1b 100644 --- a/src/agent/context.rs +++ b/src/agent/context.rs @@ -149,7 +149,7 @@ pub fn is_stream_error(err: &anyhow::Error) -> bool { /// Conversation entry — either a regular message or memory content. /// Memory entries preserve the original message for KV cache round-tripping. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub enum ConversationEntry { Message(Message), Memory { key: String, message: Message }, diff --git a/src/user/chat.rs b/src/user/chat.rs index 8130295..7800be6 100644 --- a/src/user/chat.rs +++ b/src/user/chat.rs @@ -25,46 +25,6 @@ enum PaneTarget { ToolResult, } -/// Route an agent entry to the appropriate pane. -/// Returns None for entries that shouldn't be displayed (memory, system). -fn route_entry(entry: &crate::agent::context::ConversationEntry) -> Option<(PaneTarget, String, Marker)> { - use crate::agent::api::types::Role; - use crate::agent::context::ConversationEntry; - - if let ConversationEntry::Memory { .. } = entry { - return None; - } - - let msg = entry.message(); - let text = msg.content_text().to_string(); - - match msg.role { - Role::User => { - if text.starts_with("") { return None; } - Some((PaneTarget::Conversation, text, Marker::User)) - } - Role::Assistant => { - // Tool calls → tools pane - if let Some(ref calls) = msg.tool_calls { - for call in calls { - let line = format!("[{}] {}", - call.function.name, - call.function.arguments.chars().take(80).collect::()); - // TODO: return multiple targets — for now just return first tool call - return Some((PaneTarget::Tools, line, Marker::None)); - } - } - if text.is_empty() { return None; } - Some((PaneTarget::ConversationAssistant, text, Marker::Assistant)) - } - Role::Tool => { - if text.is_empty() { return None; } - Some((PaneTarget::ToolResult, text, Marker::None)) - } - Role::System => None, - } -} - pub(crate) struct InteractScreen { pub(crate) autonomous: PaneState, pub(crate) conversation: PaneState, @@ -80,8 +40,7 @@ pub(crate) struct InteractScreen { pub(crate) call_timeout_secs: u64, // State sync with agent — double buffer last_generation: u64, - /// Content lengths of rendered entries — for detecting changes - last_entry_lengths: Vec, + last_entries: Vec, /// Reference to agent for state sync agent: std::sync::Arc>, } @@ -102,11 +61,47 @@ impl InteractScreen { call_started: None, call_timeout_secs: 60, last_generation: 0, - last_entry_lengths: Vec::new(), + last_entries: Vec::new(), agent, } } + /// Route an agent entry to the appropriate pane. + /// Returns None for entries that shouldn't be displayed (memory, system). + fn route_entry(&mut self, entry: &crate::agent::context::ConversationEntry) -> Option<&mut PaneState> { + use crate::agent::api::types::Role; + use crate::agent::context::ConversationEntry; + + if let ConversationEntry::Memory { .. } = entry { + return None; + } + + let msg = entry.message(); + let text = msg.content_text().to_string(); + + match msg.role { + if text.is_empty() { return None; } + if text.starts_with("") { return None; } + + Role::User => Some(&mut self.conversation), + Role::Assistant => { + // Tool calls → tools pane + if let Some(ref calls) = msg.tool_calls { + for call in calls { + let line = format!("[{}] {}", + call.function.name, + call.function.arguments.chars().take(80).collect::()); + // TODO: return multiple targets — for now just return first tool call + return Some((PaneTarget::Tools, line, Marker::None)); + } + } + Some((PaneTarget::ConversationAssistant, text, Marker::Assistant)) + } + Role::Tool => Some(&mut self.tools), + Role::System => None, + } + } + /// Sync conversation display from agent entries. fn sync_from_agent(&mut self) { let agent = self.agent.blocking_lock(); @@ -118,34 +113,30 @@ impl InteractScreen { self.conversation = PaneState::new(true); self.autonomous = PaneState::new(true); self.tools = PaneState::new(false); - self.last_entry_lengths.clear(); + self.last_entries.clear(); } else { - // Pop entries from the tail that were removed or changed - while self.last_entry_lengths.len() > entries.len() { - self.last_entry_lengths.pop(); - // TODO: pop from correct pane - } - // Check if last entry changed (streaming) - if let (Some(&last_len), Some(entry)) = ( - self.last_entry_lengths.last(), - entries.get(self.last_entry_lengths.len() - 1), - ) { - let cur_len = entry.message().content_text().len(); - if cur_len != last_len { - // Last entry changed — pop and re-render - self.last_entry_lengths.pop(); - self.conversation.pop_line(); + // Pop entries from the tail that don't match + while !self.last_entries.is_empty() { + let i = self.last_entries.len() - 1; + if entries.get(i) == Some(&self.last_entries[i]) { + break; + } + let popped = self.last_entries.pop().unwrap(); + if let Some((target, _, _)) = Self::route_entry(&popped) { + match target { + PaneTarget::Conversation | PaneTarget::ConversationAssistant + => self.conversation.pop_line(), + PaneTarget::Tools | PaneTarget::ToolResult + => self.tools.pop_line(), + } } } } - // Phase 2: push new/changed entries - let start = self.last_entry_lengths.len(); + // Phase 2: push new entries + let start = self.last_entries.len(); for entry in entries.iter().skip(start) { - let msg = entry.message(); - let text_len = msg.content_text().len(); - - if let Some((target, text, marker)) = route_entry(entry) { + if let Some((target, text, marker)) = Self::route_entry(entry) { match target { PaneTarget::Conversation => { self.conversation.push_line_with_marker(text, Color::Cyan, marker); @@ -163,8 +154,7 @@ impl InteractScreen { } } } - - self.last_entry_lengths.push(text_len); + self.last_entries.push(entry.clone()); } self.last_generation = gen;