feat: 添加工具调用序列的前向检查,确保工具消息紧随助手消息后
This commit is contained in:
parent
cde41e32a8
commit
303f6d83e3
@ -2453,6 +2453,57 @@ mod tests {
|
|||||||
// Verify no tool messages remain
|
// Verify no tool messages remain
|
||||||
assert!(messages.iter().all(|m| m.role != "tool"));
|
assert!(messages.iter().all(|m| m.role != "tool"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_sanitize_strips_tool_calls_when_tool_results_not_immediately_following() {
|
||||||
|
// [assistant(tool_calls=[A]), user, tool(A)]
|
||||||
|
// Tool result exists but is NOT immediately after assistant → strip tool_calls
|
||||||
|
// This is the scenario that triggers DeepSeek 400 "insufficient tool messages
|
||||||
|
// following tool_calls message" — the reverse scan sees tool(A) as resolved,
|
||||||
|
// but the API requires it to be IMMEDIATELY after the assistant.
|
||||||
|
let mut messages = vec![
|
||||||
|
ChatMessage::assistant_with_tool_calls(
|
||||||
|
"calling tool",
|
||||||
|
vec![ToolCall {
|
||||||
|
id: "call_A".to_string(),
|
||||||
|
name: "search".to_string(),
|
||||||
|
arguments: serde_json::json!({}),
|
||||||
|
}],
|
||||||
|
),
|
||||||
|
ChatMessage::user("interrupting message"),
|
||||||
|
ChatMessage::tool("call_A", "search", "result"),
|
||||||
|
];
|
||||||
|
|
||||||
|
let removed = crate::bus::message::sanitize_incomplete_tool_call_sequences(&mut messages);
|
||||||
|
// The assistant should be removed (tool_calls stripped via removal)
|
||||||
|
// and the orphaned tool(A) should also be removed
|
||||||
|
assert!(removed >= 2, "should remove both the assistant and orphaned tool message, got {}", removed);
|
||||||
|
assert_eq!(messages.len(), 1, "only the user message should remain");
|
||||||
|
assert_eq!(messages[0].role, "user");
|
||||||
|
assert!(messages.iter().all(|m| m.tool_calls.as_ref().map_or(true, |c| c.is_empty())),
|
||||||
|
"no assistant should have tool_calls remaining");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_sanitize_preserves_tool_calls_when_immediately_followed() {
|
||||||
|
// [assistant(tool_calls=[A]), tool(A), user] — valid, tool result is immediate
|
||||||
|
let mut messages = vec![
|
||||||
|
ChatMessage::assistant_with_tool_calls(
|
||||||
|
"calling tool",
|
||||||
|
vec![ToolCall {
|
||||||
|
id: "call_A".to_string(),
|
||||||
|
name: "search".to_string(),
|
||||||
|
arguments: serde_json::json!({}),
|
||||||
|
}],
|
||||||
|
),
|
||||||
|
ChatMessage::tool("call_A", "search", "result"),
|
||||||
|
ChatMessage::user("next message after tool result"),
|
||||||
|
];
|
||||||
|
|
||||||
|
let removed = crate::bus::message::sanitize_incomplete_tool_call_sequences(&mut messages);
|
||||||
|
assert_eq!(removed, 0, "should not remove anything — tool result immediately follows");
|
||||||
|
assert_eq!(messages.len(), 3);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Debug)]
|
#[derive(Debug)]
|
||||||
|
|||||||
@ -316,6 +316,86 @@ pub(crate) fn sanitize_incomplete_tool_call_sequences(messages: &mut Vec<ChatMes
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Phase 1.5: Forward-order check — verify tool messages IMMEDIATELY follow
|
||||||
|
// the assistant(tool_calls). If any non-tool message appears between the
|
||||||
|
// assistant and its tool results, the API rejects with
|
||||||
|
// "insufficient tool messages following tool_calls message".
|
||||||
|
//
|
||||||
|
// The reverse scan in Phase 1 only checks existence (tool result appears
|
||||||
|
// somewhere after assistant), NOT immediacy. This pass catches cases like:
|
||||||
|
// [assistant(tool_calls=[A]), user, tool(A)]
|
||||||
|
// ^ Phase 1 sees tool(A) after assistant → "resolved"
|
||||||
|
// but API requires tool(A) to be IMMEDIATELY after assistant
|
||||||
|
{
|
||||||
|
let mut pending_tool_ids: HashSet<String> = HashSet::new();
|
||||||
|
let mut pending_assistant_idx: Option<usize> = None;
|
||||||
|
|
||||||
|
for (i, m) in messages.iter().enumerate() {
|
||||||
|
// If we have pending tool_ids and encounter a non-tool message,
|
||||||
|
// the assistant's tool results were NOT immediately following.
|
||||||
|
if !pending_tool_ids.is_empty() && m.role != "tool" {
|
||||||
|
if let Some(idx) = pending_assistant_idx {
|
||||||
|
if !remove_indices.contains(&idx) {
|
||||||
|
tracing::warn!(
|
||||||
|
message_index = idx,
|
||||||
|
interrupted_by_index = i,
|
||||||
|
interrupted_by_role = %m.role,
|
||||||
|
pending_tool_call_count = pending_tool_ids.len(),
|
||||||
|
"Removing assistant with tool_calls — tool results \
|
||||||
|
not immediately following (interrupted by non-tool message)"
|
||||||
|
);
|
||||||
|
// Remove this assistant's tool_call_ids from with_parent
|
||||||
|
// so Phase 2 cleans up the now-orphaned tool messages
|
||||||
|
if let Some(calls) = messages[idx].tool_calls.as_ref() {
|
||||||
|
for tc in calls.iter() {
|
||||||
|
with_parent.remove(&tc.id);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
remove_indices.push(idx);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
pending_tool_ids.clear();
|
||||||
|
pending_assistant_idx = None;
|
||||||
|
}
|
||||||
|
|
||||||
|
if m.role == "assistant"
|
||||||
|
&& m.tool_calls.as_ref().map_or(false, |calls| !calls.is_empty())
|
||||||
|
{
|
||||||
|
let already_marked = remove_indices.contains(&i);
|
||||||
|
if !already_marked {
|
||||||
|
pending_tool_ids = m.tool_calls.as_ref().unwrap()
|
||||||
|
.iter().map(|tc| tc.id.clone()).collect();
|
||||||
|
pending_assistant_idx = Some(i);
|
||||||
|
}
|
||||||
|
} else if m.role == "tool" {
|
||||||
|
if let Some(ref tc_id) = m.tool_call_id {
|
||||||
|
pending_tool_ids.remove(tc_id);
|
||||||
|
if pending_tool_ids.is_empty() {
|
||||||
|
pending_assistant_idx = None;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Handle trailing assistant with unresolved immediate tool results
|
||||||
|
if !pending_tool_ids.is_empty() {
|
||||||
|
if let Some(idx) = pending_assistant_idx {
|
||||||
|
if !remove_indices.contains(&idx) {
|
||||||
|
tracing::warn!(
|
||||||
|
message_index = idx,
|
||||||
|
"Removing trailing assistant with incomplete immediate tool results"
|
||||||
|
);
|
||||||
|
if let Some(calls) = messages[idx].tool_calls.as_ref() {
|
||||||
|
for tc in calls.iter() {
|
||||||
|
with_parent.remove(&tc.id);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
remove_indices.push(idx);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Remove in descending index order to avoid shifting
|
// Remove in descending index order to avoid shifting
|
||||||
for &idx in &remove_indices {
|
for &idx in &remove_indices {
|
||||||
messages.remove(idx);
|
messages.remove(idx);
|
||||||
|
|||||||
@ -219,8 +219,6 @@ impl AgentExecutionService {
|
|||||||
}
|
}
|
||||||
let enriched_content =
|
let enriched_content =
|
||||||
enrich_user_content_with_media_refs(request.content, &media_refs)?;
|
enrich_user_content_with_media_refs(request.content, &media_refs)?;
|
||||||
enrich_user_content_with_media_refs(request.content, &media_refs)?;
|
|
||||||
enrich_user_content_with_media_refs(request.content, &media_refs)?;
|
|
||||||
|
|
||||||
// 先计算 user_message_count(在添加新消息之前)
|
// 先计算 user_message_count(在添加新消息之前)
|
||||||
let history_before = session_guard.get_or_create_history(request.chat_id).clone();
|
let history_before = session_guard.get_or_create_history(request.chat_id).clone();
|
||||||
|
|||||||
@ -672,6 +672,82 @@ impl OpenAIProvider {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Forward-order check: verify tool messages IMMEDIATELY follow the
|
||||||
|
// assistant(tool_calls). If any non-tool message appears between the
|
||||||
|
// assistant and its tool results, the API rejects with
|
||||||
|
// "insufficient tool messages following tool_calls message".
|
||||||
|
//
|
||||||
|
// The reverse scan above only checks existence (tool result appears
|
||||||
|
// somewhere after assistant), NOT immediacy. This forward pass catches:
|
||||||
|
// [assistant(tool_calls=[A]), user, tool(A)]
|
||||||
|
// ^ reverse scan sees tool(A) after assistant → "resolved"
|
||||||
|
// but API requires tool(A) to be IMMEDIATELY after assistant
|
||||||
|
{
|
||||||
|
let mut pending_tool_ids: std::collections::HashSet<&str> = std::collections::HashSet::new();
|
||||||
|
let mut pending_assistant_idx: Option<usize> = None;
|
||||||
|
|
||||||
|
for (i, m) in request.messages.iter().enumerate() {
|
||||||
|
// If we have pending tool_ids and encounter a non-tool message,
|
||||||
|
// the assistant's tool results were NOT immediately following.
|
||||||
|
if !pending_tool_ids.is_empty() && m.role != "tool" {
|
||||||
|
if let Some(idx) = pending_assistant_idx {
|
||||||
|
skip_assistant_indices.insert(idx);
|
||||||
|
tracing::warn!(
|
||||||
|
message_index = idx,
|
||||||
|
interrupted_by_index = i,
|
||||||
|
interrupted_by_role = %m.role,
|
||||||
|
pending_tool_call_count = pending_tool_ids.len(),
|
||||||
|
"build_request_body: assistant tool_calls not immediately \
|
||||||
|
followed by tool results — stripping tool_calls"
|
||||||
|
);
|
||||||
|
// Remove this assistant's tool_call_ids from with_parent
|
||||||
|
// so orphaned tool messages are dropped during serialization
|
||||||
|
if let Some(calls) = &request.messages[idx].tool_calls {
|
||||||
|
for tc in calls.iter() {
|
||||||
|
with_parent.remove(tc.id.as_str());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
pending_tool_ids.clear();
|
||||||
|
pending_assistant_idx = None;
|
||||||
|
}
|
||||||
|
|
||||||
|
if m.role == "assistant" {
|
||||||
|
if let Some(ref calls) = m.tool_calls {
|
||||||
|
if !calls.is_empty() && !skip_assistant_indices.contains(&i) {
|
||||||
|
pending_tool_ids = calls.iter().map(|tc| tc.id.as_str()).collect();
|
||||||
|
pending_assistant_idx = Some(i);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} else if m.role == "tool" {
|
||||||
|
if let Some(ref tc_id) = m.tool_call_id {
|
||||||
|
pending_tool_ids.remove(tc_id.as_str());
|
||||||
|
if pending_tool_ids.is_empty() {
|
||||||
|
pending_assistant_idx = None;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Handle trailing assistant with unresolved immediate tool results
|
||||||
|
if !pending_tool_ids.is_empty() {
|
||||||
|
if let Some(idx) = pending_assistant_idx {
|
||||||
|
skip_assistant_indices.insert(idx);
|
||||||
|
tracing::warn!(
|
||||||
|
message_index = idx,
|
||||||
|
pending_tool_call_count = pending_tool_ids.len(),
|
||||||
|
"build_request_body: trailing assistant tool_calls without \
|
||||||
|
immediately following tool results — stripping tool_calls"
|
||||||
|
);
|
||||||
|
if let Some(calls) = &request.messages[idx].tool_calls {
|
||||||
|
for tc in calls.iter() {
|
||||||
|
with_parent.remove(tc.id.as_str());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// valid_tool_call_parent_ids = with_parent (assistant tool_call_ids
|
// valid_tool_call_parent_ids = with_parent (assistant tool_call_ids
|
||||||
// whose parent assistant has ALL results after it)
|
// whose parent assistant has ALL results after it)
|
||||||
let valid_tool_call_parent_ids = &with_parent;
|
let valid_tool_call_parent_ids = &with_parent;
|
||||||
@ -795,6 +871,48 @@ impl OpenAIProvider {
|
|||||||
body["tools"] = json!(tools);
|
body["tools"] = json!(tools);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Diagnostic: log the final message sequence when tool_calls are involved.
|
||||||
|
// This captures the exact sequence sent to the API, making 400 errors
|
||||||
|
// like "insufficient tool messages following tool_calls message" easy to
|
||||||
|
// diagnose.
|
||||||
|
let has_tool_calls = body["messages"].as_array()
|
||||||
|
.map(|msgs| msgs.iter().any(|m| m.get("tool_calls").is_some()))
|
||||||
|
.unwrap_or(false);
|
||||||
|
if has_tool_calls {
|
||||||
|
let sequence: Vec<String> = body["messages"].as_array()
|
||||||
|
.map(|msgs| msgs.iter().enumerate().map(|(i, m)| {
|
||||||
|
let role = m.get("role").and_then(|r| r.as_str()).unwrap_or("?");
|
||||||
|
match role {
|
||||||
|
"assistant" => {
|
||||||
|
let tc_count = m.get("tool_calls")
|
||||||
|
.and_then(|t| t.as_array())
|
||||||
|
.map(|a| a.len())
|
||||||
|
.unwrap_or(0);
|
||||||
|
if tc_count > 0 {
|
||||||
|
format!("[{}] assistant(tool_calls={})", i, tc_count)
|
||||||
|
} else {
|
||||||
|
format!("[{}] assistant", i)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
"tool" => {
|
||||||
|
let tcid = m.get("tool_call_id")
|
||||||
|
.and_then(|t| t.as_str())
|
||||||
|
.unwrap_or("??");
|
||||||
|
format!("[{}] tool(id={})", i, tcid)
|
||||||
|
}
|
||||||
|
_ => format!("[{}] {}", i, role),
|
||||||
|
}
|
||||||
|
}).collect())
|
||||||
|
.unwrap_or_default();
|
||||||
|
tracing::info!(
|
||||||
|
provider = %self.name,
|
||||||
|
model = %self.model_id,
|
||||||
|
message_count = sequence.len(),
|
||||||
|
sequence = ?sequence,
|
||||||
|
"build_request_body: final message sequence with tool_calls"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
body
|
body
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -1496,4 +1614,131 @@ mod tests {
|
|||||||
// custom_param 应该保留
|
// custom_param 应该保留
|
||||||
assert_eq!(body["custom_param"], Value::String("value".to_string()));
|
assert_eq!(body["custom_param"], Value::String("value".to_string()));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_build_request_body_strips_tool_calls_when_not_immediately_followed() {
|
||||||
|
// [assistant(tool_calls=[A]), user, tool(A)] → should strip tool_calls
|
||||||
|
// The tool result exists but a user message interrupts between assistant
|
||||||
|
// and tool result. The API would reject this with
|
||||||
|
// "insufficient tool messages following tool_calls message".
|
||||||
|
let provider = OpenAIProvider::new(
|
||||||
|
"test".to_string(),
|
||||||
|
"key".to_string(),
|
||||||
|
"https://example.com/v1".to_string(),
|
||||||
|
HashMap::new(),
|
||||||
|
120,
|
||||||
|
"gpt-test".to_string(),
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
HashMap::new(),
|
||||||
|
);
|
||||||
|
|
||||||
|
let request = ChatCompletionRequest {
|
||||||
|
messages: vec![
|
||||||
|
Message {
|
||||||
|
role: "assistant".to_string(),
|
||||||
|
content: vec![ContentBlock::text("calling tool")],
|
||||||
|
reasoning_content: None,
|
||||||
|
tool_call_id: None,
|
||||||
|
name: None,
|
||||||
|
tool_calls: Some(vec![ToolCall {
|
||||||
|
id: "call_A".to_string(),
|
||||||
|
name: "search".to_string(),
|
||||||
|
arguments: json!({}),
|
||||||
|
}]),
|
||||||
|
},
|
||||||
|
Message {
|
||||||
|
role: "user".to_string(),
|
||||||
|
content: vec![ContentBlock::text("interrupting message")],
|
||||||
|
reasoning_content: None,
|
||||||
|
tool_call_id: None,
|
||||||
|
name: None,
|
||||||
|
tool_calls: None,
|
||||||
|
},
|
||||||
|
Message {
|
||||||
|
role: "tool".to_string(),
|
||||||
|
content: vec![ContentBlock::text("result")],
|
||||||
|
reasoning_content: None,
|
||||||
|
tool_call_id: Some("call_A".to_string()),
|
||||||
|
name: Some("search".to_string()),
|
||||||
|
tool_calls: None,
|
||||||
|
},
|
||||||
|
],
|
||||||
|
temperature: None,
|
||||||
|
max_tokens: None,
|
||||||
|
tools: None,
|
||||||
|
};
|
||||||
|
|
||||||
|
let body = provider.build_request_body(&request);
|
||||||
|
let messages = body["messages"].as_array().unwrap();
|
||||||
|
|
||||||
|
// Assistant should NOT have tool_calls (stripped because not immediately followed)
|
||||||
|
assert!(
|
||||||
|
messages[0].get("tool_calls").is_none(),
|
||||||
|
"tool_calls should be stripped when tool results are not immediately following"
|
||||||
|
);
|
||||||
|
// Tool message should be dropped (orphaned after stripping)
|
||||||
|
assert_eq!(
|
||||||
|
messages.len(),
|
||||||
|
2,
|
||||||
|
"tool message should be dropped as orphaned, got {} messages",
|
||||||
|
messages.len()
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_build_request_body_preserves_tool_calls_when_immediately_followed() {
|
||||||
|
// [assistant(tool_calls=[A]), tool(A)] → should keep tool_calls (valid sequence)
|
||||||
|
let provider = OpenAIProvider::new(
|
||||||
|
"test".to_string(),
|
||||||
|
"key".to_string(),
|
||||||
|
"https://example.com/v1".to_string(),
|
||||||
|
HashMap::new(),
|
||||||
|
120,
|
||||||
|
"gpt-test".to_string(),
|
||||||
|
None,
|
||||||
|
None,
|
||||||
|
HashMap::new(),
|
||||||
|
);
|
||||||
|
|
||||||
|
let request = ChatCompletionRequest {
|
||||||
|
messages: vec![
|
||||||
|
Message {
|
||||||
|
role: "assistant".to_string(),
|
||||||
|
content: vec![ContentBlock::text("calling tool")],
|
||||||
|
reasoning_content: None,
|
||||||
|
tool_call_id: None,
|
||||||
|
name: None,
|
||||||
|
tool_calls: Some(vec![ToolCall {
|
||||||
|
id: "call_A".to_string(),
|
||||||
|
name: "search".to_string(),
|
||||||
|
arguments: json!({}),
|
||||||
|
}]),
|
||||||
|
},
|
||||||
|
Message {
|
||||||
|
role: "tool".to_string(),
|
||||||
|
content: vec![ContentBlock::text("result")],
|
||||||
|
reasoning_content: None,
|
||||||
|
tool_call_id: Some("call_A".to_string()),
|
||||||
|
name: Some("search".to_string()),
|
||||||
|
tool_calls: None,
|
||||||
|
},
|
||||||
|
],
|
||||||
|
temperature: None,
|
||||||
|
max_tokens: None,
|
||||||
|
tools: None,
|
||||||
|
};
|
||||||
|
|
||||||
|
let body = provider.build_request_body(&request);
|
||||||
|
let messages = body["messages"].as_array().unwrap();
|
||||||
|
|
||||||
|
// Assistant should keep tool_calls (valid immediate sequence)
|
||||||
|
let tool_calls = messages[0].get("tool_calls")
|
||||||
|
.and_then(|t| t.as_array())
|
||||||
|
.expect("tool_calls should be preserved when immediately followed");
|
||||||
|
assert_eq!(tool_calls.len(), 1);
|
||||||
|
// Tool message should be present
|
||||||
|
assert_eq!(messages.len(), 2);
|
||||||
|
assert_eq!(messages[1]["role"], "tool");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user