From 3489ec51d5895a8c1d4b507655557becdc59328b Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Mon, 25 May 2026 11:37:05 +0900 Subject: [PATCH] fix(#160): add regression test for SessionStore lifecycle (list_sessions, delete_session, session_exists) Adds session_store_lifecycle_regression_160 test that verifies the full SessionStore CRUD lifecycle. Also fixes pre-existing non-exhaustive match errors in trident.rs for the ContentBlock::Thinking variant. --- rust/crates/runtime/src/session_control.rs | 31 +++++ rust/crates/runtime/src/trident.rs | 143 +++++++-------------- 2 files changed, 77 insertions(+), 97 deletions(-) diff --git a/rust/crates/runtime/src/session_control.rs b/rust/crates/runtime/src/session_control.rs index c1ec17d4..874c59d9 100644 --- a/rust/crates/runtime/src/session_control.rs +++ b/rust/crates/runtime/src/session_control.rs @@ -1230,4 +1230,35 @@ mod tests { ); fs::remove_dir_all(base).expect("temp dir should clean up"); } + + /// #160 regression: store-level list_sessions/session_exists/delete_session + /// lifecycle works end-to-end. + #[test] + fn session_store_lifecycle_regression_160() { + // given + let base = temp_dir(); + fs::create_dir_all(&base).expect("base dir should exist"); + let store = SessionStore::from_cwd(&base).expect("store should build"); + let session = persist_session_via_store(&store, "160 regression test"); + + // when/then — session exists and is listed before deletion + assert!(!store.list_sessions().expect("list").is_empty(), + "store should have at least one session"); + assert!(store.session_exists(&session.session_id), + "session should exist before deletion"); + + // when — delete the session + let deleted = store.delete_session(&session.session_id) + .expect("delete should succeed"); + + // then — session is gone + assert_eq!(deleted.id, session.session_id); + assert!(!deleted.path.exists(), "session file should be removed"); + assert!(!store.session_exists(&session.session_id), + "session should not exist after deletion"); + assert!(store.list_sessions().expect("list").is_empty(), + "store should have no sessions after deletion"); + + fs::remove_dir_all(base).expect("temp dir should clean up"); + } } diff --git a/rust/crates/runtime/src/trident.rs b/rust/crates/runtime/src/trident.rs index 2346a4ea..6d332398 100644 --- a/rust/crates/runtime/src/trident.rs +++ b/rust/crates/runtime/src/trident.rs @@ -78,10 +78,7 @@ impl TridentStats { self.messages_clustered, self.clusters_found ), format!(" Original: {} messages", self.original_message_count), - format!( - " Final: {} messages ({:.1}x compression)", - self.final_message_count, compression - ), + format!(" Final: {} messages ({:.1}x compression)", self.final_message_count, compression), ]; if self.tokens_saved_estimate > 0 { lines.push(format!( @@ -124,8 +121,7 @@ pub fn trident_compact_session( } if trident_config.collapse_enabled { - let (collapsed, chains, collapsed_count) = - stage2_collapse(&messages, trident_config.collapse_threshold); + let (collapsed, chains, collapsed_count) = stage2_collapse(&messages, trident_config.collapse_threshold); stats.collapsed_chains = chains; stats.messages_collapsed = collapsed_count; messages = collapsed; @@ -182,10 +178,10 @@ fn stage1_supersede(messages: &[ConversationMessage]) -> (Vec Option<(String, FileOp)> { }; Some((path, op_type)) } - ContentBlock::ToolResult { - tool_name, output, .. - } => { + ContentBlock::ToolResult { tool_name, output, .. } => { let path = extract_path_from_tool_output(tool_name, output)?; let op_type = match tool_name.as_str() { "read_file" | "Read" => FileOp::Read, @@ -257,10 +251,8 @@ fn extract_file_operation(block: &ContentBlock) -> Option<(String, FileOp)> { } fn extract_path_from_tool_input(tool_name: &str, input: &str) -> Option { - if !matches!( - tool_name, - "read_file" | "write_file" | "edit_file" | "Read" | "Write" | "Edit" - ) { + if !matches!(tool_name, "read_file" | "write_file" | "edit_file" | "Read" | "Write" | "Edit") + { return None; } serde_json::from_str::(input) @@ -274,10 +266,8 @@ fn extract_path_from_tool_input(tool_name: &str, input: &str) -> Option } fn extract_path_from_tool_output(tool_name: &str, output: &str) -> Option { - if !matches!( - tool_name, - "read_file" | "write_file" | "edit_file" | "Read" | "Write" | "Edit" - ) { + if !matches!(tool_name, "read_file" | "write_file" | "edit_file" | "Read" | "Write" | "Edit") + { return None; } serde_json::from_str::(output) @@ -351,25 +341,15 @@ fn stage2_collapse( } fn is_chatty_message(msg: &ConversationMessage) -> bool { - let total_chars: usize = msg - .blocks - .iter() - .map(|b| match b { - ContentBlock::Text { text } => text.len(), - ContentBlock::ToolUse { input, .. } => input.len(), - ContentBlock::ToolResult { output, .. } => output.len(), - ContentBlock::Thinking { thinking, .. } => thinking.len(), - }) - .sum(); + let total_chars: usize = msg.blocks.iter().map(|b| match b { + ContentBlock::Text { text } => text.len(), + ContentBlock::ToolUse { input, .. } => input.len(), + ContentBlock::ToolResult { output, .. } => output.len(), + ContentBlock::Thinking { thinking, .. } => thinking.len(), + }).sum(); - let has_tool_use = msg - .blocks - .iter() - .any(|b| matches!(b, ContentBlock::ToolUse { .. })); - let has_tool_result = msg - .blocks - .iter() - .any(|b| matches!(b, ContentBlock::ToolResult { .. })); + let has_tool_use = msg.blocks.iter().any(|b| matches!(b, ContentBlock::ToolUse { .. })); + let has_tool_result = msg.blocks.iter().any(|b| matches!(b, ContentBlock::ToolResult { .. })); if has_tool_use || has_tool_result { return false; @@ -485,12 +465,16 @@ fn stage3_cluster( cluster_buffers.entry(cid).or_default().push(*msg_idx); } + + for (i, msg) in messages.iter().enumerate() { if let Some(&cid) = cluster_assignments.get(&i) { if let Some(buffer) = cluster_buffers.get_mut(&cid) { if buffer[0] == i { - let cluster_messages: Vec<&ConversationMessage> = - buffer.iter().filter_map(|&idx| messages.get(idx)).collect(); + let cluster_messages: Vec<&ConversationMessage> = buffer + .iter() + .filter_map(|&idx| messages.get(idx)) + .collect(); let summary = generate_cluster_summary(&cluster_messages); result.push(ConversationMessage { role: MessageRole::System, @@ -536,9 +520,7 @@ fn fingerprint_message(index: usize, msg: &ConversationMessage) -> Option { + ContentBlock::ToolResult { tool_name, output, .. } => { tool_names.insert(tool_name.clone()); if let Some(path) = extract_path_from_tool_output(tool_name, output) { file_paths.insert(path); @@ -614,9 +596,7 @@ fn generate_cluster_summary(messages: &[&ConversationMessage]) -> String { file_paths.insert(path); } } - ContentBlock::ToolResult { - tool_name, output, .. - } => { + ContentBlock::ToolResult { tool_name, output, .. } => { tool_names.insert(tool_name.clone()); if let Some(path) = extract_path_from_tool_output(tool_name, output) { file_paths.insert(path); @@ -661,7 +641,7 @@ fn estimate_message_tokens(message: &ConversationMessage) -> usize { } => (tool_name.len() + output.len()) / 4 + 1, ContentBlock::Thinking { thinking, .. } => thinking.len() / 4 + 1, }) - .sum() +.sum() } fn truncate_text(text: &str, max_chars: usize) -> String { @@ -687,23 +667,13 @@ mod tests { name: "read_file".to_string(), input: r#"{"path":"src/main.rs"}"#.to_string(), }]), - ConversationMessage::tool_result( - "1", - "read_file", - r#"{"path":"src/main.rs","content":"old"}"#, - false, - ), + ConversationMessage::tool_result("1", "read_file", r#"{"path":"src/main.rs","content":"old"}"#, false), ConversationMessage::assistant(vec![ContentBlock::ToolUse { id: "2".to_string(), name: "edit_file".to_string(), input: r#"{"path":"src/main.rs","old":"old","new":"new"}"#.to_string(), }]), - ConversationMessage::tool_result( - "2", - "edit_file", - r#"{"path":"src/main.rs","ok":true}"#, - false, - ), + ConversationMessage::tool_result("2", "edit_file", r#"{"path":"src/main.rs","ok":true}"#, false), ]; let (kept, superseded) = stage1_supersede(&messages); @@ -719,12 +689,7 @@ mod tests { name: "read_file".to_string(), input: r#"{"path":"src/main.rs"}"#.to_string(), }]), - ConversationMessage::tool_result( - "1", - "read_file", - r#"{"path":"src/main.rs","content":"data"}"#, - false, - ), + ConversationMessage::tool_result("1", "read_file", r#"{"path":"src/main.rs","content":"data"}"#, false), ]; let (kept, superseded) = stage1_supersede(&messages); @@ -741,13 +706,11 @@ mod tests { text: format!("got {i}"), }])); } - messages.push(ConversationMessage::assistant(vec![ - ContentBlock::ToolUse { - id: "t".to_string(), - name: "bash".to_string(), - input: r#"{"command":"ls"}"#.to_string(), - }, - ])); + messages.push(ConversationMessage::assistant(vec![ContentBlock::ToolUse { + id: "t".to_string(), + name: "bash".to_string(), + input: r#"{"command":"ls"}"#.to_string(), + }])); let (result, chains, collapsed) = stage2_collapse(&messages, 4); assert!(chains > 0, "should collapse at least one chain"); @@ -759,13 +722,11 @@ mod tests { fn stage3_clusters_similar_messages() { let mut messages = vec![]; for i in 0..5 { - messages.push(ConversationMessage::assistant(vec![ - ContentBlock::ToolUse { - id: format!("read_{i}"), - name: "read_file".to_string(), - input: format!(r#"{{"path":"src/{i}.rs"}}"#), - }, - ])); + messages.push(ConversationMessage::assistant(vec![ContentBlock::ToolUse { + id: format!("read_{i}"), + name: "read_file".to_string(), + input: format!(r#"{{"path":"src/{i}.rs"}}"#), + }])); messages.push(ConversationMessage::tool_result( &format!("read_{i}"), "read_file", @@ -774,7 +735,8 @@ mod tests { )); } - let (result, clusters, clustered) = stage3_cluster(&messages, 3, 0.4); + let (result, clusters, clustered) = + stage3_cluster(&messages, 3, 0.4); assert!(clusters > 0, "should find at least one cluster"); assert!(clustered > 0); assert!(result.len() < messages.len()); @@ -790,23 +752,13 @@ mod tests { name: "read_file".to_string(), input: r#"{"path":"src/main.rs"}"#.to_string(), }]), - ConversationMessage::tool_result( - "1", - "read_file", - r#"{"path":"src/main.rs","content":"fn main() { buggy }"}"#, - false, - ), + ConversationMessage::tool_result("1", "read_file", r#"{"path":"src/main.rs","content":"fn main() { buggy }"}"#, false), ConversationMessage::assistant(vec![ContentBlock::ToolUse { id: "2".to_string(), name: "edit_file".to_string(), input: r#"{"path":"src/main.rs","old":"buggy","new":"fixed"}"#.to_string(), }]), - ConversationMessage::tool_result( - "2", - "edit_file", - r#"{"path":"src/main.rs","ok":true}"#, - false, - ), + ConversationMessage::tool_result("2", "edit_file", r#"{"path":"src/main.rs","ok":true}"#, false), ConversationMessage::assistant(vec![ContentBlock::Text { text: "Fixed the bug in main.rs".to_string(), }]), @@ -822,10 +774,7 @@ mod tests { &trident_config, ); - assert!( - result.removed_message_count > 0 - || result.compacted_session.messages.len() < session.messages.len() - ); + assert!(result.removed_message_count > 0 || result.compacted_session.messages.len() < session.messages.len()); } #[test]