Skip to content

Commit e09e0ae

Browse files
authored
fix(context): strip orphaned tool_result messages after hard compaction (#3256)
compact_context() and compact_context_with_budget() drained messages[1..compact_end] without checking whether the first message of the preserved tail was a tool_result whose paired tool_use assistant fell inside the drained range, producing an invalid message sequence that caused 400 invalid_request_error on the next LLM call. Add adjust_compact_end_for_tool_pairs() that walks the compaction boundary backward past any leading tool_result-only user messages and their paired tool_use assistant, ensuring the preserved tail always starts on a valid message. Apply in both compact_context() and compact_context_with_budget() before the drain. Fixes #3255
1 parent 4f38786 commit e09e0ae

1 file changed

Lines changed: 219 additions & 2 deletions

File tree

crates/zeph-core/src/agent/context/summarization.rs

Lines changed: 219 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,55 @@ impl<C: Channel> Agent<C> {
380380
Self::archive_tool_outputs(archive_enabled, memory, cid, to_compact.to_vec()).await
381381
}
382382

383+
/// Adjust `compact_end` backward so the preserved tail begins on a clean message boundary.
384+
///
385+
/// If `messages[compact_end]` is a `Role::User` message that contains only `ToolResult`
386+
/// parts, its paired `Role::Assistant` `ToolUse` message is at `compact_end - 1` and would
387+
/// be drained, leaving an orphaned `tool_result`. This function walks the boundary backward
388+
/// until the first message of the tail is not a `ToolResult`-only user message, absorbing all
389+
/// consecutive `ToolResult` messages and the single preceding `ToolUse` assistant message.
390+
///
391+
/// Returns the adjusted `compact_end`. The minimum returned value is `1` (drain nothing
392+
/// beyond the system message).
393+
fn adjust_compact_end_for_tool_pairs(messages: &[Message], compact_end: usize) -> usize {
394+
let mut end = compact_end;
395+
loop {
396+
// Nothing left to drain — stop.
397+
if end <= 1 {
398+
return 1;
399+
}
400+
// compact_end may equal messages.len() when preserve_tail = 0.
401+
if end >= messages.len() {
402+
break;
403+
}
404+
let first_tail = &messages[end];
405+
let is_tool_result_msg = first_tail.role == Role::User
406+
&& !first_tail.parts.is_empty()
407+
&& first_tail
408+
.parts
409+
.iter()
410+
.all(|p| matches!(p, MessagePart::ToolResult { .. }));
411+
if !is_tool_result_msg {
412+
break;
413+
}
414+
// Absorb this ToolResult message into the tail.
415+
end -= 1;
416+
}
417+
// If we moved the boundary, also absorb the preceding ToolUse assistant message.
418+
if end < compact_end && end > 1 {
419+
let preceding = &messages[end - 1];
420+
let is_tool_use_msg = preceding.role == Role::Assistant
421+
&& preceding
422+
.parts
423+
.iter()
424+
.any(|p| matches!(p, MessagePart::ToolUse { .. }));
425+
if is_tool_use_msg {
426+
end -= 1;
427+
}
428+
}
429+
end.max(1)
430+
}
431+
383432
#[allow(clippy::too_many_lines)]
384433
pub(in crate::agent) async fn compact_context(
385434
&mut self,
@@ -393,7 +442,14 @@ impl<C: Channel> Agent<C> {
393442
return Ok(CompactionOutcome::NoChange);
394443
}
395444

396-
let compact_end = self.msg.messages.len() - preserve_tail;
445+
let compact_end = {
446+
let raw = self.msg.messages.len() - preserve_tail;
447+
Self::adjust_compact_end_for_tool_pairs(&self.msg.messages, raw)
448+
};
449+
450+
if compact_end <= 1 {
451+
return Ok(CompactionOutcome::NoChange);
452+
}
397453

398454
// S1 fix: extract focus-pinned messages before draining so they survive compaction.
399455
// These are Knowledge block messages created by the Focus Agent (#1850).
@@ -2048,7 +2104,15 @@ impl<C: Channel> Agent<C> {
20482104
return Ok(());
20492105
}
20502106

2051-
let compact_end = self.msg.messages.len() - preserve_tail;
2107+
let compact_end = {
2108+
let raw = self.msg.messages.len() - preserve_tail;
2109+
Self::adjust_compact_end_for_tool_pairs(&self.msg.messages, raw)
2110+
};
2111+
2112+
if compact_end <= 1 {
2113+
return Ok(());
2114+
}
2115+
20522116
let to_compact = &self.msg.messages[1..compact_end];
20532117
if to_compact.is_empty() {
20542118
return Ok(());
@@ -3531,3 +3595,156 @@ mod subgoal_extraction_tests {
35313595
assert_eq!(result.completed, None);
35323596
}
35333597
}
3598+
3599+
#[cfg(test)]
3600+
mod compact_end_tool_pair_tests {
3601+
use zeph_llm::provider::{Message, MessagePart, Role};
3602+
3603+
use super::super::super::Agent;
3604+
use crate::agent::tests::agent_tests::MockChannel;
3605+
3606+
fn tool_use_msg() -> Message {
3607+
Message {
3608+
role: Role::Assistant,
3609+
content: String::new(),
3610+
parts: vec![MessagePart::ToolUse {
3611+
id: "tu1".into(),
3612+
name: "shell".into(),
3613+
input: serde_json::json!({}),
3614+
}],
3615+
metadata: Default::default(),
3616+
}
3617+
}
3618+
3619+
fn tool_result_msg() -> Message {
3620+
Message {
3621+
role: Role::User,
3622+
content: String::new(),
3623+
parts: vec![MessagePart::ToolResult {
3624+
tool_use_id: "tu1".into(),
3625+
content: "ok".into(),
3626+
is_error: false,
3627+
}],
3628+
metadata: Default::default(),
3629+
}
3630+
}
3631+
3632+
fn text_msg(role: Role, text: &str) -> Message {
3633+
Message {
3634+
role,
3635+
content: text.into(),
3636+
parts: vec![MessagePart::Text { text: text.into() }],
3637+
metadata: Default::default(),
3638+
}
3639+
}
3640+
3641+
fn system_msg() -> Message {
3642+
Message {
3643+
role: Role::System,
3644+
content: "system".into(),
3645+
parts: vec![],
3646+
metadata: Default::default(),
3647+
}
3648+
}
3649+
3650+
// Alias for the static helper under test.
3651+
fn adjust(messages: &[Message], compact_end: usize) -> usize {
3652+
Agent::<MockChannel>::adjust_compact_end_for_tool_pairs(messages, compact_end)
3653+
}
3654+
3655+
#[test]
3656+
fn no_tool_pair_at_boundary_unchanged() {
3657+
// [sys, user, assistant_text, user2] preserve_tail=1 → compact_end=3
3658+
// messages[3] = user2 (plain text) → no adjustment
3659+
let msgs = vec![
3660+
system_msg(),
3661+
text_msg(Role::User, "hi"),
3662+
text_msg(Role::Assistant, "ok"),
3663+
text_msg(Role::User, "bye"),
3664+
];
3665+
assert_eq!(adjust(&msgs, 3), 3);
3666+
}
3667+
3668+
#[test]
3669+
fn tool_result_at_boundary_absorbs_pair() {
3670+
// [sys, user, tool_use, tool_result, user2] preserve_tail=2 → compact_end=3
3671+
// messages[3] = tool_result → adjust to 2, then absorb tool_use → compact_end=2
3672+
let msgs = vec![
3673+
system_msg(),
3674+
text_msg(Role::User, "hi"),
3675+
tool_use_msg(),
3676+
tool_result_msg(),
3677+
text_msg(Role::User, "bye"),
3678+
];
3679+
assert_eq!(adjust(&msgs, 3), 2);
3680+
}
3681+
3682+
#[test]
3683+
fn multiple_tool_results_absorbed() {
3684+
// Parallel tool calls: one tool_use, two tool_results (two result messages)
3685+
let tool_result2 = Message {
3686+
role: Role::User,
3687+
content: String::new(),
3688+
parts: vec![MessagePart::ToolResult {
3689+
tool_use_id: "tu2".into(),
3690+
content: "ok2".into(),
3691+
is_error: false,
3692+
}],
3693+
metadata: Default::default(),
3694+
};
3695+
// [sys, user, tool_use, tool_result, tool_result2, assistant_reply]
3696+
// preserve_tail=2 → compact_end=4
3697+
// messages[4] = tool_result2 → absorb; messages[3] = tool_result → absorb;
3698+
// messages[2] = tool_use → absorb; compact_end=2
3699+
let msgs = vec![
3700+
system_msg(),
3701+
text_msg(Role::User, "hi"),
3702+
tool_use_msg(),
3703+
tool_result_msg(),
3704+
tool_result2,
3705+
text_msg(Role::Assistant, "done"),
3706+
];
3707+
assert_eq!(adjust(&msgs, 4), 2);
3708+
}
3709+
3710+
#[test]
3711+
fn preserve_tail_zero_no_tool_result_unchanged() {
3712+
// [sys, user, assistant] compact_end=3 (preserve_tail=0)
3713+
// We only call with compact_end < len in practice; test a valid boundary.
3714+
let msgs = vec![
3715+
system_msg(),
3716+
text_msg(Role::User, "hi"),
3717+
text_msg(Role::Assistant, "ok"),
3718+
];
3719+
assert_eq!(adjust(&msgs, 2), 2);
3720+
}
3721+
3722+
#[test]
3723+
fn compact_end_equals_len_does_not_panic() {
3724+
// preserve_tail=0 → compact_end = messages.len(); must not panic on out-of-bounds.
3725+
let msgs = vec![
3726+
system_msg(),
3727+
text_msg(Role::User, "hi"),
3728+
tool_use_msg(),
3729+
tool_result_msg(),
3730+
];
3731+
// compact_end = 4 = msgs.len(); bounds guard must fire and return unchanged.
3732+
assert_eq!(adjust(&msgs, msgs.len()), msgs.len());
3733+
}
3734+
3735+
#[test]
3736+
fn compact_end_already_one_returns_one() {
3737+
let msgs = vec![system_msg(), tool_result_msg()];
3738+
assert_eq!(adjust(&msgs, 1), 1);
3739+
}
3740+
3741+
#[test]
3742+
fn only_tool_pairs_degenerate_returns_one() {
3743+
// All non-system messages are tool pairs; compact_end points into the pair.
3744+
let msgs = vec![system_msg(), tool_use_msg(), tool_result_msg()];
3745+
// compact_end=2 → messages[2]=tool_result → absorb → end=1; tool_use at [1]
3746+
// preceding = messages[0] = system (not tool_use) so no extra absorption
3747+
// end = 1, max(1) = 1
3748+
assert_eq!(adjust(&msgs, 2), 1);
3749+
}
3750+
}

0 commit comments

Comments
 (0)