From b764175be49fe906eb7bd878f82dfdb294baafc0 Mon Sep 17 00:00:00 2001 From: Shawn Edwards Date: Fri, 29 May 2026 19:01:20 -0500 Subject: [PATCH] spark-server: avoid orphan tool messages in F32 --- crates/spark-server/src/api/chat_phases.rs | 5 +- .../spark-server/src/api/failures/circuit.rs | 10 ++- .../src/api/failures/circuit_tests.rs | 85 ++++++++++++++++++- 3 files changed, 94 insertions(+), 6 deletions(-) diff --git a/crates/spark-server/src/api/chat_phases.rs b/crates/spark-server/src/api/chat_phases.rs index 561fd13a..88a7a60b 100644 --- a/crates/spark-server/src/api/chat_phases.rs +++ b/crates/spark-server/src/api/chat_phases.rs @@ -136,9 +136,10 @@ pub(super) fn apply_failure_guards(req: &mut ChatCompletionRequest) -> F23Progre ); } - // F32: duplicate failed tool_result at conversation tail. + // F32: surface the most recent failed tool_result without creating + // an orphan role=tool message that vendor templates reject. if f32_reposition_failed_tool_result(&mut req.messages) { - tracing::info!("F32: duplicated most-recent failed tool_result at conversation tail"); + tracing::info!("F32: surfaced most-recent failed tool_result in a runtime reminder"); } // F39: cross-turn permanent-failure circuit breaker. diff --git a/crates/spark-server/src/api/failures/circuit.rs b/crates/spark-server/src/api/failures/circuit.rs index 1cf532a5..26c91d95 100644 --- a/crates/spark-server/src/api/failures/circuit.rs +++ b/crates/spark-server/src/api/failures/circuit.rs @@ -90,8 +90,14 @@ pub fn f32_reposition_failed_tool_result( { return false; } - let dup = messages[idx].clone(); - messages.push(dup); + let failed_tool_result = messages[idx].content.text.trim(); + let reminder = format!( + "\n\n\nThe most recent failed tool result is still relevant. \ + Treat it as an observed tool failure and do not retry the same call unless \ + the approach materially changes.\n\n{failed_tool_result}\n\ + \n" + ); + append_f7_reminder_to_last_user(messages, &reminder); true } diff --git a/crates/spark-server/src/api/failures/circuit_tests.rs b/crates/spark-server/src/api/failures/circuit_tests.rs index 0c998b16..5e4b1be3 100644 --- a/crates/spark-server/src/api/failures/circuit_tests.rs +++ b/crates/spark-server/src/api/failures/circuit_tests.rs @@ -5,11 +5,92 @@ //! `circuit.rs` stays under the 500-LoC file-size-cap. use super::circuit::{ - F39PermanentFailureMatch, f39_build_circuit_breaker_banner, f39_class_label, - f39_extract_binary_name, + F39PermanentFailureMatch, f32_reposition_failed_tool_result, f39_build_circuit_breaker_banner, + f39_class_label, f39_extract_binary_name, }; use super::classification::F37FailureClass; +fn msg(role: &str, text: &str) -> crate::openai::IncomingMessage { + crate::openai::IncomingMessage { + role: role.to_string(), + content: crate::openai::ParsedContent { + text: text.to_string(), + images: Vec::new(), + }, + tool_calls: None, + tool_call_id: None, + name: None, + } +} + +fn assistant_tool_call(id: &str) -> crate::openai::IncomingMessage { + crate::openai::IncomingMessage { + role: "assistant".to_string(), + content: crate::openai::ParsedContent::default(), + tool_calls: Some(vec![crate::tool_parser::IncomingToolCall { + id: Some(id.to_string()), + function: crate::tool_parser::IncomingFunction { + name: "Bash".to_string(), + arguments: r#"{"command":"cargo"}"#.to_string(), + }, + }]), + tool_call_id: None, + name: None, + } +} + +fn tool_result(id: &str, text: &str) -> crate::openai::IncomingMessage { + crate::openai::IncomingMessage { + role: "tool".to_string(), + content: crate::openai::ParsedContent { + text: text.to_string(), + images: Vec::new(), + }, + tool_calls: None, + tool_call_id: Some(id.to_string()), + name: None, + } +} + +// ── f32_reposition_failed_tool_result ──────────────────────────── + +#[test] +fn f32_surfaces_error_without_orphan_tool_message() { + let mut messages = vec![ + msg("user", "do it"), + assistant_tool_call("toolu_1"), + tool_result("toolu_1", "[tool error]\ncargo: command not found"), + msg("user", "intervening 1"), + msg("user", "intervening 2"), + ]; + let before = messages.len(); + + assert!(f32_reposition_failed_tool_result(&mut messages)); + + assert_eq!(messages.len(), before); + let last = messages.last().unwrap(); + assert_eq!(last.role, "user"); + assert!(last.content.text.contains("")); + assert!( + last.content + .text + .contains("[tool error]\ncargo: command not found") + ); +} + +#[test] +fn f32_no_op_when_failed_tool_is_already_fresh() { + let mut messages = vec![ + msg("user", "do it"), + assistant_tool_call("toolu_1"), + tool_result("toolu_1", "[tool error]\ncargo: command not found"), + ]; + let before = messages.len(); + + assert!(!f32_reposition_failed_tool_result(&mut messages)); + assert_eq!(messages.len(), before); +} + // ── f39_extract_binary_name ─────────────────────────────────────── #[test]