mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(agent): prevent orphaned tool messages from reaching the provider (#2717)
This commit is contained in:
sanil-23
@@ -795,3 +795,47 @@ fn bound_cached_transcript_messages_without_system_prefix_keeps_tail() {
|
||||
assert_eq!(bounded[1].content, "a2");
|
||||
assert_eq!(bounded[2].content, "u3");
|
||||
}
|
||||
|
||||
/// The cached-transcript resume path operates on wire-form `ChatMessage`s. When
|
||||
/// the window cut lands so the tail opens on a `tool` result whose `tool_calls`
|
||||
/// opener fell outside the window, `bound_cached_transcript_messages` must snap
|
||||
/// past it — a leading `tool` message has no preceding `tool_calls` and the
|
||||
/// provider 400s (surfacing as "Something went wrong").
|
||||
#[test]
|
||||
fn bound_cached_transcript_messages_snaps_past_leading_orphan_tool() {
|
||||
use crate::openhuman::inference::provider::ChatMessage;
|
||||
|
||||
let mut agent = build_minimal_agent_with_definition_name(Some("orchestrator"));
|
||||
agent.config.max_history_messages = 3;
|
||||
|
||||
// 5 messages, cap 3: the tail slice is [tool(a), user(u2), assistant(a2)];
|
||||
// the assistant `tool_calls` opener fell outside the window.
|
||||
let messages = vec![
|
||||
ChatMessage::assistant(
|
||||
r#"{"content":"calling","tool_calls":[{"id":"call_a","name":"shell","arguments":"{}"}]}"#,
|
||||
),
|
||||
ChatMessage::tool(r#"{"tool_call_id":"call_a","content":"orphaned"}"#),
|
||||
ChatMessage::user("u2"),
|
||||
ChatMessage::assistant("a2"),
|
||||
ChatMessage::user("u3"),
|
||||
];
|
||||
|
||||
let bounded = agent.bound_cached_transcript_messages(messages);
|
||||
|
||||
assert!(
|
||||
bounded.first().map(|m| m.role.as_str()) != Some("tool"),
|
||||
"window must not open on an orphaned tool result"
|
||||
);
|
||||
assert!(
|
||||
!bounded.iter().any(|m| m.role == "tool"),
|
||||
"the orphaned tool result must be dropped"
|
||||
);
|
||||
// tail [tool, u2, a2, u3] -> drop leading tool -> [u2, a2, u3].
|
||||
assert_eq!(
|
||||
bounded
|
||||
.iter()
|
||||
.map(|m| m.content.as_str())
|
||||
.collect::<Vec<_>>(),
|
||||
vec!["u2", "a2", "u3"]
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1590,6 +1590,25 @@ impl Agent {
|
||||
other_messages.drain(0..drop_count);
|
||||
}
|
||||
|
||||
// A cut that lands *between* an `AssistantToolCalls` and its
|
||||
// `ToolResults` leaves the window opening on an orphaned `ToolResults`.
|
||||
// Serialized, that is a `tool` message with no preceding `tool_calls`,
|
||||
// which the provider rejects with a 400 (the response streams back
|
||||
// empty and surfaces to the user as "Something went wrong"). Snap the
|
||||
// boundary forward past any leading orphaned results so the window
|
||||
// always starts on a clean turn (a `Chat` or an `AssistantToolCalls`).
|
||||
let orphan_lead = other_messages
|
||||
.iter()
|
||||
.take_while(|m| matches!(m, ConversationMessage::ToolResults(_)))
|
||||
.count();
|
||||
if orphan_lead > 0 {
|
||||
log::debug!(
|
||||
"[agent] trim_history snapped window past {orphan_lead} orphaned ToolResults \
|
||||
(tool-cycle bisected by the {max}-message cap)"
|
||||
);
|
||||
other_messages.drain(0..orphan_lead);
|
||||
}
|
||||
|
||||
self.history = system_messages;
|
||||
self.history.extend(other_messages);
|
||||
}
|
||||
@@ -1610,19 +1629,34 @@ impl Agent {
|
||||
return messages;
|
||||
}
|
||||
|
||||
if matches!(messages.first(), Some(msg) if msg.role == "system") {
|
||||
let keep_tail = max.saturating_sub(1);
|
||||
let start = messages.len().saturating_sub(keep_tail);
|
||||
let mut bounded = Vec::with_capacity(max);
|
||||
bounded.push(messages[0].clone());
|
||||
if keep_tail > 0 {
|
||||
bounded.extend(messages[start..].iter().cloned());
|
||||
}
|
||||
bounded
|
||||
let has_system = matches!(messages.first(), Some(msg) if msg.role == "system");
|
||||
let keep_tail = if has_system {
|
||||
max.saturating_sub(1)
|
||||
} else {
|
||||
let start = messages.len().saturating_sub(max);
|
||||
messages[start..].to_vec()
|
||||
max
|
||||
};
|
||||
let start = messages.len().saturating_sub(keep_tail);
|
||||
|
||||
// Same hazard as `trim_history`: the tail slice can open on a `tool`
|
||||
// message whose `tool_calls` opener fell outside the window, which the
|
||||
// provider rejects. Advance past any leading orphaned `tool` results so
|
||||
// the window starts on a clean turn.
|
||||
let tail = &messages[start..];
|
||||
let orphan_lead = tail.iter().take_while(|m| m.role == "tool").count();
|
||||
if orphan_lead > 0 {
|
||||
log::debug!(
|
||||
"[agent] bound_cached_transcript_messages snapped window past {orphan_lead} \
|
||||
orphaned tool result(s) (tool-cycle bisected by the {max}-message cap)"
|
||||
);
|
||||
}
|
||||
let tail = &tail[orphan_lead..];
|
||||
|
||||
let mut bounded = Vec::with_capacity(tail.len() + usize::from(has_system));
|
||||
if has_system {
|
||||
bounded.push(messages[0].clone());
|
||||
}
|
||||
bounded.extend(tail.iter().cloned());
|
||||
bounded
|
||||
}
|
||||
|
||||
/// Pre-fetches learned context data from memory (observations, patterns, user profile).
|
||||
|
||||
@@ -370,6 +370,53 @@ fn trim_history_preserves_system_and_keeps_latest_non_system_entries() {
|
||||
.any(|msg| matches!(msg, ConversationMessage::Chat(chat) if chat.content == "a2")));
|
||||
}
|
||||
|
||||
/// When the `max_history_messages` cap drops an `AssistantToolCalls` opener but
|
||||
/// keeps its `ToolResults`, the window would otherwise open on an orphaned tool
|
||||
/// result — serialized, a `tool` message with no preceding `tool_calls`, which
|
||||
/// the provider rejects (the 400 that surfaces as "Something went wrong").
|
||||
/// `trim_history` must snap past the orphan so the window starts on a clean turn.
|
||||
#[test]
|
||||
fn trim_history_snaps_past_orphaned_tool_results() {
|
||||
use crate::openhuman::inference::provider::{ToolCall, ToolResultMessage};
|
||||
|
||||
let mut agent = make_agent(None); // max_history_messages = 3
|
||||
agent.history = vec![
|
||||
ConversationMessage::Chat(ChatMessage::system("sys")),
|
||||
// This opener is the oldest non-system entry, so the cap drops it...
|
||||
ConversationMessage::AssistantToolCalls {
|
||||
text: Some("calling".into()),
|
||||
tool_calls: vec![ToolCall {
|
||||
id: "call_x".into(),
|
||||
name: "shell".into(),
|
||||
arguments: "{}".into(),
|
||||
}],
|
||||
},
|
||||
// ...orphaning this result at the head of the kept window.
|
||||
ConversationMessage::ToolResults(vec![ToolResultMessage {
|
||||
tool_call_id: "call_x".into(),
|
||||
content: "result".into(),
|
||||
}]),
|
||||
ConversationMessage::Chat(ChatMessage::user("u2")),
|
||||
ConversationMessage::Chat(ChatMessage::assistant("a2")),
|
||||
];
|
||||
|
||||
agent.trim_history();
|
||||
|
||||
assert!(
|
||||
!agent
|
||||
.history
|
||||
.iter()
|
||||
.any(|m| matches!(m, ConversationMessage::ToolResults(_))),
|
||||
"orphaned ToolResults must be dropped, not left at the window head"
|
||||
);
|
||||
assert!(
|
||||
matches!(agent.history.first(), Some(ConversationMessage::Chat(c)) if c.role == "system"),
|
||||
"system message is preserved"
|
||||
);
|
||||
// system + u2 + a2 (the bisected cycle is gone entirely).
|
||||
assert_eq!(agent.history.len(), 3);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_parent_context_and_sanitize_helpers_cover_snapshot_paths() {
|
||||
let mut agent = make_agent(None);
|
||||
|
||||
@@ -13,9 +13,10 @@
|
||||
//! `ToolResults` envelopes with a placeholder, preserving the
|
||||
//! `AssistantToolCalls ⇔ ToolResults` API invariant.
|
||||
//! 4. **Autocompact** — prose summarisation of older messages.
|
||||
//! OpenHuman's existing `auto_compact_history` lives in
|
||||
//! `agent/loop_/history.rs` and operates on `ChatMessage` (not
|
||||
//! `ConversationMessage`), so we don't call it here — the pipeline
|
||||
//! The summariser (`ProviderSummarizer` in `context/summarizer.rs`,
|
||||
//! optionally wrapped by `segment_recap_summarizer.rs`) operates on
|
||||
//! `ConversationMessage` and issues LLM calls, so we don't run it
|
||||
//! here — the pipeline
|
||||
//! instead signals a `PipelineOutcome::AutocompactionRequested` to
|
||||
//! the caller and trusts the caller to dispatch its own summariser
|
||||
//! when ready. Keeping the pipeline pure (no LLM calls) means the
|
||||
|
||||
@@ -545,79 +545,186 @@ impl OpenAiCompatibleProvider {
|
||||
}
|
||||
|
||||
fn convert_messages_for_native(messages: &[ChatMessage]) -> Vec<NativeMessage> {
|
||||
messages
|
||||
.iter()
|
||||
.map(|message| {
|
||||
if message.role == "assistant" {
|
||||
if let Ok(value) = serde_json::from_str::<serde_json::Value>(&message.content)
|
||||
{
|
||||
if let Some(tool_calls_value) = value.get("tool_calls") {
|
||||
if let Ok(parsed_calls) =
|
||||
serde_json::from_value::<Vec<ProviderToolCall>>(
|
||||
tool_calls_value.clone(),
|
||||
)
|
||||
{
|
||||
let tool_calls = parsed_calls
|
||||
.into_iter()
|
||||
.map(|tc| ToolCall {
|
||||
id: Some(tc.id),
|
||||
kind: Some("function".to_string()),
|
||||
function: Some(Function {
|
||||
name: Some(tc.name),
|
||||
arguments: Some(serde_json::Value::String(
|
||||
tc.arguments,
|
||||
)),
|
||||
}),
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
let converted: Vec<NativeMessage> =
|
||||
messages
|
||||
.iter()
|
||||
.map(|message| {
|
||||
if message.role == "assistant" {
|
||||
if let Ok(value) =
|
||||
serde_json::from_str::<serde_json::Value>(&message.content)
|
||||
{
|
||||
if let Some(tool_calls_value) = value.get("tool_calls") {
|
||||
if let Ok(parsed_calls) =
|
||||
serde_json::from_value::<Vec<ProviderToolCall>>(
|
||||
tool_calls_value.clone(),
|
||||
)
|
||||
{
|
||||
let tool_calls = parsed_calls
|
||||
.into_iter()
|
||||
.map(|tc| ToolCall {
|
||||
id: Some(tc.id),
|
||||
kind: Some("function".to_string()),
|
||||
function: Some(Function {
|
||||
name: Some(tc.name),
|
||||
arguments: Some(serde_json::Value::String(
|
||||
tc.arguments,
|
||||
)),
|
||||
}),
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
let content = value
|
||||
.get("content")
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.map(ToString::to_string);
|
||||
let content = value
|
||||
.get("content")
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.map(ToString::to_string);
|
||||
|
||||
return NativeMessage {
|
||||
role: "assistant".to_string(),
|
||||
content,
|
||||
tool_call_id: None,
|
||||
tool_calls: Some(tool_calls),
|
||||
};
|
||||
return NativeMessage {
|
||||
role: "assistant".to_string(),
|
||||
content,
|
||||
tool_call_id: None,
|
||||
tool_calls: Some(tool_calls),
|
||||
};
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if message.role == "tool" {
|
||||
if let Ok(value) =
|
||||
serde_json::from_str::<serde_json::Value>(&message.content)
|
||||
{
|
||||
let tool_call_id = value
|
||||
.get("tool_call_id")
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.map(ToString::to_string);
|
||||
let content = value
|
||||
.get("content")
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.map(ToString::to_string)
|
||||
.or_else(|| Some(message.content.clone()));
|
||||
|
||||
return NativeMessage {
|
||||
role: "tool".to_string(),
|
||||
content,
|
||||
tool_call_id,
|
||||
tool_calls: None,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
NativeMessage {
|
||||
role: message.role.clone(),
|
||||
content: Some(message.content.clone()),
|
||||
tool_call_id: None,
|
||||
tool_calls: None,
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
|
||||
Self::enforce_tool_message_invariants(converted)
|
||||
}
|
||||
|
||||
/// Enforce the OpenAI-compatible tool-message ordering invariants on the
|
||||
/// fully-serialized wire array, immediately before it goes on the wire.
|
||||
///
|
||||
/// Several upstream defects can leave the array malformed and trip a 400
|
||||
/// (`messages with role 'tool' must be a response to a preceding message
|
||||
/// with 'tool_calls'`). That 400 streams back as an empty completion, which
|
||||
/// the agent loop collapses to "The model returned an empty response" and
|
||||
/// the chat surface shows as a generic "Something went wrong":
|
||||
///
|
||||
/// * **(A)** History tail-trimming (`session::turn::trim_history` /
|
||||
/// `bound_cached_transcript_messages`) cuts *between* an
|
||||
/// `assistant(tool_calls)` and its `tool` result, dropping the assistant
|
||||
/// and orphaning the result at the head of the window.
|
||||
/// * **(B)** A persisted assistant tool-call message whose `content` no
|
||||
/// longer deserializes as `tool_calls` (format drift) falls through the
|
||||
/// parser above and is emitted as plain text with its `tool_calls`
|
||||
/// stripped — again orphaning the following `tool` result.
|
||||
/// * **(C)** An `assistant(tool_calls)` whose results never arrived (an
|
||||
/// aborted / max-iteration turn, or a partially-answered multi-call
|
||||
/// cycle) leaves dangling tool-call ids with no matching `tool` response.
|
||||
///
|
||||
/// This pass makes the contract hold *by construction* regardless of which
|
||||
/// path produced the array. It is **position-aware**: each
|
||||
/// `assistant(tool_calls)` is paired with the *contiguous run of `tool`
|
||||
/// messages that immediately follows it* (the only place valid responses can
|
||||
/// live in the OpenAI wire format), then:
|
||||
///
|
||||
/// * `tool_calls` entries with no matching response *in that run* are pruned
|
||||
/// (C); if none survive, the field is dropped so the message serializes as
|
||||
/// plain assistant text rather than an empty tool-call block.
|
||||
/// * `tool` messages that are **not** part of such a run — a leading orphan
|
||||
/// from trimming (A), or one stranded after an assistant whose `tool_calls`
|
||||
/// were stripped (B) — are dropped.
|
||||
///
|
||||
/// Pairing by adjacency (rather than a global "is this id answered anywhere"
|
||||
/// set) is what keeps **sequential** cycles (`asst(A)→tool(A)`,
|
||||
/// `asst(B)→tool(B)`, …) and **parallel** calls (one `asst([X,Y,Z])` answered
|
||||
/// by `tool(X) tool(Y) tool(Z)`) correct, and makes the result well-formed
|
||||
/// even if responses are reordered or a cycle is bisected mid-sequence — no
|
||||
/// causal-ordering assumption required.
|
||||
fn enforce_tool_message_invariants(messages: Vec<NativeMessage>) -> Vec<NativeMessage> {
|
||||
use std::collections::HashSet;
|
||||
|
||||
let mut out: Vec<NativeMessage> = Vec::with_capacity(messages.len());
|
||||
let mut dropped_orphans = 0usize;
|
||||
let mut pruned_calls = 0usize;
|
||||
|
||||
let mut iter = messages.into_iter().peekable();
|
||||
while let Some(mut msg) = iter.next() {
|
||||
if msg.role == "assistant" && msg.tool_calls.is_some() {
|
||||
// Gather the contiguous run of `tool` messages that answer this
|
||||
// block (responses must immediately follow, in any order).
|
||||
let mut run: Vec<NativeMessage> = Vec::new();
|
||||
while iter.peek().is_some_and(|m| m.role == "tool") {
|
||||
run.push(iter.next().expect("peeked tool message"));
|
||||
}
|
||||
let responded: HashSet<String> =
|
||||
run.iter().filter_map(|t| t.tool_call_id.clone()).collect();
|
||||
|
||||
if message.role == "tool" {
|
||||
if let Ok(value) =
|
||||
serde_json::from_str::<serde_json::Value>(&message.content)
|
||||
{
|
||||
let tool_call_id = value
|
||||
.get("tool_call_id")
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.map(ToString::to_string);
|
||||
let content = value
|
||||
.get("content")
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.map(ToString::to_string)
|
||||
.or_else(|| Some(message.content.clone()));
|
||||
// (C) keep only tool_calls answered within this run.
|
||||
let calls = msg.tool_calls.take().unwrap_or_default();
|
||||
let before = calls.len();
|
||||
let kept: Vec<ToolCall> = calls
|
||||
.into_iter()
|
||||
.filter(|c| c.id.as_deref().is_some_and(|id| responded.contains(id)))
|
||||
.collect();
|
||||
pruned_calls += before - kept.len();
|
||||
let kept_ids: HashSet<String> = kept.iter().filter_map(|c| c.id.clone()).collect();
|
||||
msg.tool_calls = if kept.is_empty() { None } else { Some(kept) };
|
||||
out.push(msg);
|
||||
|
||||
return NativeMessage {
|
||||
role: "tool".to_string(),
|
||||
content,
|
||||
tool_call_id,
|
||||
tool_calls: None,
|
||||
};
|
||||
// Emit the run's responses that map to a surviving call; drop the
|
||||
// rest (e.g. a stray tool whose id wasn't in this block).
|
||||
for tool_msg in run {
|
||||
let kept = tool_msg
|
||||
.tool_call_id
|
||||
.as_deref()
|
||||
.is_some_and(|id| kept_ids.contains(id));
|
||||
if kept {
|
||||
out.push(tool_msg);
|
||||
} else {
|
||||
dropped_orphans += 1;
|
||||
}
|
||||
}
|
||||
} else if msg.role == "tool" {
|
||||
// (A, B) a `tool` not consumed by a preceding assistant block.
|
||||
dropped_orphans += 1;
|
||||
} else {
|
||||
out.push(msg);
|
||||
}
|
||||
}
|
||||
|
||||
NativeMessage {
|
||||
role: message.role.clone(),
|
||||
content: Some(message.content.clone()),
|
||||
tool_call_id: None,
|
||||
tool_calls: None,
|
||||
}
|
||||
})
|
||||
.collect()
|
||||
if dropped_orphans > 0 || pruned_calls > 0 {
|
||||
log::warn!(
|
||||
"[provider] sanitized malformed tool-message ordering before send: \
|
||||
dropped {dropped_orphans} orphaned tool result(s), pruned {pruned_calls} \
|
||||
unanswered tool_call(s)"
|
||||
);
|
||||
}
|
||||
|
||||
out
|
||||
}
|
||||
|
||||
fn with_prompt_guided_tool_instructions(
|
||||
|
||||
@@ -659,15 +659,242 @@ fn parse_native_response_preserves_tool_call_id() {
|
||||
|
||||
#[test]
|
||||
fn convert_messages_for_native_maps_tool_result_payload() {
|
||||
let input = vec![ChatMessage::tool(
|
||||
r#"{"tool_call_id":"call_abc","content":"done"}"#,
|
||||
)];
|
||||
// A `tool` result must be opened by a preceding `assistant(tool_calls)`,
|
||||
// else the invariant sanitizer drops it as an orphan (see `tool_invariants_*`).
|
||||
// Pair it with its opener so this test exercises payload mapping only.
|
||||
let input = vec![
|
||||
ChatMessage::assistant(
|
||||
r#"{"content":"on it","tool_calls":[{"id":"call_abc","name":"shell","arguments":"{}"}]}"#,
|
||||
),
|
||||
ChatMessage::tool(r#"{"tool_call_id":"call_abc","content":"done"}"#),
|
||||
];
|
||||
|
||||
let converted = OpenAiCompatibleProvider::convert_messages_for_native(&input);
|
||||
assert_eq!(converted.len(), 1);
|
||||
assert_eq!(converted[0].role, "tool");
|
||||
assert_eq!(converted[0].tool_call_id.as_deref(), Some("call_abc"));
|
||||
assert_eq!(converted[0].content.as_deref(), Some("done"));
|
||||
assert_eq!(converted.len(), 2);
|
||||
assert_eq!(converted[1].role, "tool");
|
||||
assert_eq!(converted[1].tool_call_id.as_deref(), Some("call_abc"));
|
||||
assert_eq!(converted[1].content.as_deref(), Some("done"));
|
||||
}
|
||||
|
||||
/// Helper: roles in serialized order.
|
||||
fn roles(messages: &[NativeMessage]) -> Vec<&str> {
|
||||
messages.iter().map(|m| m.role.as_str()).collect()
|
||||
}
|
||||
|
||||
/// Mechanism (A): history tail-trimming dropped an `assistant(tool_calls)` but
|
||||
/// kept its `tool` result, orphaning the result at the head of the window. The
|
||||
/// sanitizer must drop the orphan so the wire array never starts a tool block
|
||||
/// without a preceding `tool_calls`.
|
||||
#[test]
|
||||
fn tool_invariants_drop_orphaned_tool_result_from_trim(/* A */) {
|
||||
let input = vec![
|
||||
ChatMessage::system("system prompt"),
|
||||
// assistant(tool_calls=call_orphan) was sliced off by trim_history;
|
||||
// only its result survived as the first non-system message.
|
||||
ChatMessage::tool(r#"{"tool_call_id":"call_orphan","content":"stale result"}"#),
|
||||
ChatMessage::user("and then?"),
|
||||
];
|
||||
|
||||
let converted = OpenAiCompatibleProvider::convert_messages_for_native(&input);
|
||||
|
||||
assert_eq!(roles(&converted), vec!["system", "user"]);
|
||||
assert!(
|
||||
converted.iter().all(|m| m.role != "tool"),
|
||||
"orphaned tool result must be dropped"
|
||||
);
|
||||
}
|
||||
|
||||
/// Mechanism (B): a persisted assistant tool-call message whose `content` no
|
||||
/// longer parses as `{tool_calls: [...]}` is emitted as plain assistant text
|
||||
/// with its `tool_calls` stripped, orphaning the following `tool` result. The
|
||||
/// assistant message stays; the now-orphaned tool result is dropped.
|
||||
#[test]
|
||||
fn tool_invariants_drop_tool_after_unparseable_assistant_call(/* B */) {
|
||||
let input = vec![
|
||||
// Plain text, not the JSON tool-call shape -> tool_calls stripped on convert.
|
||||
ChatMessage::assistant("let me check that for you"),
|
||||
ChatMessage::tool(r#"{"tool_call_id":"call_b","content":"tool ran"}"#),
|
||||
];
|
||||
|
||||
let converted = OpenAiCompatibleProvider::convert_messages_for_native(&input);
|
||||
|
||||
assert_eq!(roles(&converted), vec!["assistant"]);
|
||||
assert!(converted[0].tool_calls.is_none());
|
||||
assert!(
|
||||
converted.iter().all(|m| m.role != "tool"),
|
||||
"tool result with no opening tool_calls must be dropped"
|
||||
);
|
||||
}
|
||||
|
||||
/// Mechanism (C): an `assistant(tool_calls=[answered, missing])` whose second
|
||||
/// call never received a `tool` response (aborted / max-iteration turn, or a
|
||||
/// partially-answered multi-call cycle). The sanitizer prunes the dangling
|
||||
/// tool-call entry while keeping the answered one and its result.
|
||||
#[test]
|
||||
fn tool_invariants_prune_unanswered_tool_call(/* C */) {
|
||||
let input = vec![
|
||||
ChatMessage::assistant(
|
||||
r#"{"content":"on it","tool_calls":[{"id":"call_done","name":"shell","arguments":"{}"},{"id":"call_missing","name":"shell","arguments":"{}"}]}"#,
|
||||
),
|
||||
ChatMessage::tool(r#"{"tool_call_id":"call_done","content":"finished"}"#),
|
||||
// call_missing never gets a tool response.
|
||||
];
|
||||
|
||||
let converted = OpenAiCompatibleProvider::convert_messages_for_native(&input);
|
||||
|
||||
let assistant = converted
|
||||
.iter()
|
||||
.find(|m| m.role == "assistant")
|
||||
.expect("assistant message present");
|
||||
let calls = assistant
|
||||
.tool_calls
|
||||
.as_ref()
|
||||
.expect("answered tool_call retained");
|
||||
assert_eq!(calls.len(), 1, "dangling tool_call must be pruned");
|
||||
assert_eq!(calls[0].id.as_deref(), Some("call_done"));
|
||||
assert!(
|
||||
converted
|
||||
.iter()
|
||||
.any(|m| m.role == "tool" && m.tool_call_id.as_deref() == Some("call_done")),
|
||||
"answered tool result must survive"
|
||||
);
|
||||
}
|
||||
|
||||
/// (C) extreme: an `assistant(tool_calls)` with NO response at all collapses to
|
||||
/// a plain assistant message (tool_calls dropped) rather than a dangling block.
|
||||
#[test]
|
||||
fn tool_invariants_collapse_fully_unanswered_assistant_call() {
|
||||
let input = vec![
|
||||
ChatMessage::assistant(
|
||||
r#"{"content":"on it","tool_calls":[{"id":"call_x","name":"shell","arguments":"{}"}]}"#,
|
||||
),
|
||||
ChatMessage::assistant("never mind, here's the answer"),
|
||||
];
|
||||
|
||||
let converted = OpenAiCompatibleProvider::convert_messages_for_native(&input);
|
||||
|
||||
assert_eq!(roles(&converted), vec!["assistant", "assistant"]);
|
||||
assert!(
|
||||
converted[0].tool_calls.is_none(),
|
||||
"fully-unanswered tool_calls must be dropped"
|
||||
);
|
||||
assert_eq!(converted[0].content.as_deref(), Some("on it"));
|
||||
}
|
||||
|
||||
/// Regression guard: a well-formed tool cycle is passed through untouched —
|
||||
/// the sanitizer must not strip or reorder valid messages.
|
||||
#[test]
|
||||
fn tool_invariants_preserve_well_formed_cycle() {
|
||||
let input = vec![
|
||||
ChatMessage::system("system prompt"),
|
||||
ChatMessage::user("run it"),
|
||||
ChatMessage::assistant(
|
||||
r#"{"content":"on it","tool_calls":[{"id":"call_ok","name":"shell","arguments":"{}"}]}"#,
|
||||
),
|
||||
ChatMessage::tool(r#"{"tool_call_id":"call_ok","content":"done"}"#),
|
||||
ChatMessage::assistant("all set"),
|
||||
];
|
||||
|
||||
let converted = OpenAiCompatibleProvider::convert_messages_for_native(&input);
|
||||
|
||||
assert_eq!(
|
||||
roles(&converted),
|
||||
vec!["system", "user", "assistant", "tool", "assistant"]
|
||||
);
|
||||
assert_eq!(converted[2].tool_calls.as_ref().unwrap().len(), 1);
|
||||
assert_eq!(
|
||||
converted[2].tool_calls.as_ref().unwrap()[0].id.as_deref(),
|
||||
Some("call_ok")
|
||||
);
|
||||
assert_eq!(converted[3].tool_call_id.as_deref(), Some("call_ok"));
|
||||
}
|
||||
|
||||
/// Sequential tool cycles — successive agent iterations, each its own
|
||||
/// `assistant(tool_calls)` → `tool` block. Distinct ids, opened then immediately
|
||||
/// consumed. All survive untouched.
|
||||
#[test]
|
||||
fn tool_invariants_preserve_sequential_cycles() {
|
||||
let input = vec![
|
||||
ChatMessage::user("go"),
|
||||
ChatMessage::assistant(
|
||||
r#"{"content":"step 1","tool_calls":[{"id":"call_a","name":"shell","arguments":"{}"}]}"#,
|
||||
),
|
||||
ChatMessage::tool(r#"{"tool_call_id":"call_a","content":"a done"}"#),
|
||||
ChatMessage::assistant(
|
||||
r#"{"content":"step 2","tool_calls":[{"id":"call_b","name":"shell","arguments":"{}"}]}"#,
|
||||
),
|
||||
ChatMessage::tool(r#"{"tool_call_id":"call_b","content":"b done"}"#),
|
||||
ChatMessage::assistant(
|
||||
r#"{"content":"step 3","tool_calls":[{"id":"call_c","name":"shell","arguments":"{}"}]}"#,
|
||||
),
|
||||
ChatMessage::tool(r#"{"tool_call_id":"call_c","content":"c done"}"#),
|
||||
ChatMessage::assistant("all done"),
|
||||
];
|
||||
|
||||
let converted = OpenAiCompatibleProvider::convert_messages_for_native(&input);
|
||||
|
||||
assert_eq!(
|
||||
roles(&converted),
|
||||
vec![
|
||||
"user",
|
||||
"assistant",
|
||||
"tool",
|
||||
"assistant",
|
||||
"tool",
|
||||
"assistant",
|
||||
"tool",
|
||||
"assistant"
|
||||
]
|
||||
);
|
||||
for idx in [1usize, 3, 5] {
|
||||
assert_eq!(
|
||||
converted[idx].tool_calls.as_ref().unwrap().len(),
|
||||
1,
|
||||
"cycle at index {idx} must keep its call"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Parallel tool calls — one `assistant` issuing N calls, answered by N `tool`
|
||||
/// messages arriving out of order. All survive; pairing is by membership, not
|
||||
/// position, so order does not matter.
|
||||
#[test]
|
||||
fn tool_invariants_preserve_parallel_calls() {
|
||||
let input = vec![
|
||||
ChatMessage::assistant(
|
||||
r#"{"content":"fanning out","tool_calls":[{"id":"call_x","name":"shell","arguments":"{}"},{"id":"call_y","name":"shell","arguments":"{}"},{"id":"call_z","name":"shell","arguments":"{}"}]}"#,
|
||||
),
|
||||
ChatMessage::tool(r#"{"tool_call_id":"call_y","content":"y"}"#),
|
||||
ChatMessage::tool(r#"{"tool_call_id":"call_z","content":"z"}"#),
|
||||
ChatMessage::tool(r#"{"tool_call_id":"call_x","content":"x"}"#),
|
||||
];
|
||||
|
||||
let converted = OpenAiCompatibleProvider::convert_messages_for_native(&input);
|
||||
|
||||
assert_eq!(roles(&converted), vec!["assistant", "tool", "tool", "tool"]);
|
||||
assert_eq!(converted[0].tool_calls.as_ref().unwrap().len(), 3);
|
||||
}
|
||||
|
||||
/// Trim bisecting a sequence: the window opens inside cycle A (its assistant was
|
||||
/// sliced off), followed by an intact cycle B. The orphaned A result is dropped;
|
||||
/// cycle B survives — proving adjacency-pairing localizes the damage.
|
||||
#[test]
|
||||
fn tool_invariants_drop_orphan_but_keep_following_cycle() {
|
||||
let input = vec![
|
||||
// assistant(call_a) was sliced off by trim; only its result remains.
|
||||
ChatMessage::tool(r#"{"tool_call_id":"call_a","content":"orphaned"}"#),
|
||||
ChatMessage::assistant(
|
||||
r#"{"content":"step 2","tool_calls":[{"id":"call_b","name":"shell","arguments":"{}"}]}"#,
|
||||
),
|
||||
ChatMessage::tool(r#"{"tool_call_id":"call_b","content":"b done"}"#),
|
||||
ChatMessage::assistant("done"),
|
||||
];
|
||||
|
||||
let converted = OpenAiCompatibleProvider::convert_messages_for_native(&input);
|
||||
|
||||
assert_eq!(roles(&converted), vec!["assistant", "tool", "assistant"]);
|
||||
assert_eq!(converted[0].tool_calls.as_ref().unwrap().len(), 1);
|
||||
assert_eq!(converted[1].tool_call_id.as_deref(), Some("call_b"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user