refactor: reorganize conversation history handling and improve structure

crates/forge_app/src/service/api.rs

@@ -3,12 +3,11 @@ use std::sync::Arc;
 
 use anyhow::Result;
 use forge_domain::{
-    ChatRequest, ChatResponse, Config, ConfigRepository, Context, Conversation, ConversationId,
-    ConversationRepository, Environment, Model, ProviderService, ResultStream, ToolDefinition,
-    ToolService,
+    ChatRequest, ChatResponse, Config, ConfigRepository, Context, Conversation,
+    ConversationHistory, ConversationId, ConversationRepository, Environment, Model,
+    ProviderService, ResultStream, ToolDefinition, ToolService,
 };
 
-use super::chat::ConversationHistory;
 use super::env::EnvironmentService;
 use super::suggestion::{File, SuggestionService};
 use super::ui::UIService;

crates/forge_app/src/service/chat.rs

@@ -3,9 +3,8 @@ use std::sync::Arc;
 use anyhow::Result;
 use forge_domain::{
     ChatRequest, ChatResponse, Context, ContextMessage, FinishReason, ProviderService,
-    ResultStream, Role, ToolCall, ToolCallFull, ToolResult, ToolService,
+    ResultStream, ToolCall, ToolCallFull, ToolResult, ToolService,
 };
-use serde::Serialize;
 use tokio_stream::wrappers::ReceiverStream;
 use tokio_stream::StreamExt;
 
@@ -212,39 +211,6 @@ impl ChatService for Live {
     }
 }
 
-#[derive(Default, Debug, Clone, Serialize)]
-pub struct ConversationHistory {
-    pub messages: Vec<ChatResponse>,
-}
-
-impl From<Context> for ConversationHistory {
-    fn from(request: Context) -> Self {
-        let messages = request
-            .messages
-            .iter()
-            .filter(|message| match message {
-                ContextMessage::ContentMessage(content) => content.role != Role::System,
-                ContextMessage::ToolMessage(_) => true,
-            })
-            .flat_map(|message| match message {
-                ContextMessage::ContentMessage(content) => {
-                    let mut messages = vec![ChatResponse::Text(content.content.clone())];
-                    if let Some(tool_calls) = &content.tool_calls {
-                        for tool_call in tool_calls {
-                            messages.push(ChatResponse::ToolCallStart(tool_call.clone()));
-                        }
-                    }
-                    messages
-                }
-                ContextMessage::ToolMessage(result) => {
-                    vec![ChatResponse::ToolCallEnd(result.clone())]
-                }
-            })
-            .collect();
-        Self { messages }
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use std::sync::{Arc, Mutex};

crates/forge_domain/src/conversation.rs

@@ -6,7 +6,7 @@ use derive_setters::Setters;
 use serde::{Deserialize, Serialize};
 use uuid::Uuid;
 
-use crate::{Context, Error};
+use crate::{ChatResponse, Context, ContextMessage, Error, Role};
 
 #[derive(Debug, Display, Serialize, Deserialize, Clone, PartialEq, Eq, Copy)]
 #[serde(transparent)]
@@ -48,6 +48,11 @@ impl Conversation {
             title: None,
         }
     }
+
+    /// Get the conversation history representation of this conversation
+    pub fn history(&self) -> ConversationHistory {
+        self.context.clone().into()
+    }
 }
 
 #[derive(Debug, Serialize, Deserialize)]
@@ -56,6 +61,43 @@ pub struct ConversationMeta {
     pub updated_at: DateTime<Utc>,
 }
 
+/// Represents a conversation's history including user and assistant messages
+/// and tool interactions. This is a view-oriented representation of a
+/// conversation that is suitable for displaying to users or processing the
+/// conversation flow.
+#[derive(Debug, Clone, Serialize)]
+pub struct ConversationHistory {
+    pub messages: Vec<ChatResponse>,
+}
+
+impl From<Context> for ConversationHistory {
+    fn from(request: Context) -> Self {
+        let messages = request
+            .messages
+            .into_iter()
+            .filter(|message| match message {
+                ContextMessage::ContentMessage(content) => content.role != Role::System,
+                ContextMessage::ToolMessage(_) => true,
+            })
+            .flat_map(|message| match message {
+                ContextMessage::ContentMessage(content) => {
+                    let mut messages = vec![ChatResponse::Text(content.content.clone())];
+                    if let Some(tool_calls) = content.tool_calls {
+                        for tool_call in tool_calls {
+                            messages.push(ChatResponse::ToolCallStart(tool_call.clone()));
+                        }
+                    }
+                    messages
+                }
+                ContextMessage::ToolMessage(result) => {
+                    vec![ChatResponse::ToolCallEnd(result)]
+                }
+            })
+            .collect();
+        Self { messages }
+    }
+}
+
 #[async_trait]
 pub trait ConversationRepository: Send + Sync {
     /// Set a new conversation or update an existing one
@@ -73,3 +115,62 @@ pub trait ConversationRepository: Send + Sync {
     /// Set the title for a conversation
     async fn set_title(&self, id: &ConversationId, title: String) -> Result<Conversation>;
 }
+
+#[cfg(test)]
+mod tests {
+    use serde_json::json;
+
+    use super::*;
+    use crate::{ContextMessage, Role, ToolCallFull, ToolName, ToolResult};
+
+    #[test]
+    fn test_conversation_history_from_context() {
+        let mut context = Context::default();
+
+        // Add system message (should be filtered out)
+        context = context.add_message(ContextMessage::system("System prompt"));
+
+        // Add user message
+        context = context.add_message(ContextMessage::ContentMessage(crate::ContentMessage {
+            role: Role::User,
+            content: "User message".to_string(),
+            tool_calls: None,
+        }));
+
+        // Add assistant message with tool call
+        let tool_call =
+            ToolCallFull::new(ToolName::new("test_tool")).arguments(json!({"arg": "value"}));
+        context = context.add_message(ContextMessage::ContentMessage(crate::ContentMessage {
+            role: Role::Assistant,
+            content: "Assistant message".to_string(),
+            tool_calls: Some(vec![tool_call.clone()]),
+        }));
+
+        // Add tool result
+        let tool_result = ToolResult::new(ToolName::new("test_tool"))
+            .success(json!({"result": "success"}).to_string());
+        context = context.add_message(ContextMessage::ToolMessage(tool_result.clone()));
+
+        let history = ConversationHistory::from(context);
+
+        assert_eq!(history.messages.len(), 4);
+        assert!(matches!(&history.messages[0], ChatResponse::Text(text) if text == "User message"));
+        assert!(
+            matches!(&history.messages[1], ChatResponse::Text(text) if text == "Assistant message")
+        );
+        assert!(
+            matches!(&history.messages[2], ChatResponse::ToolCallStart(call) if call == &tool_call)
+        );
+        assert!(
+            matches!(&history.messages[3], ChatResponse::ToolCallEnd(result) if result == &tool_result)
+        );
+    }
+
+    #[test]
+    fn test_conversation_history_only_system_message() {
+        let context = Context::default().add_message(ContextMessage::system("System prompt"));
+
+        let history = ConversationHistory::from(context);
+        assert!(history.messages.is_empty());
+    }
+}