Skip to content

Commit ef5390f

Browse files
committed
feat(agent): P1.3 — structured tool-error signal to the model (is_error)
Tool failures previously reached the model only as an "Error:" text prefix; the computed is_error boolean was used for telemetry but dropped before the provider. Now a failed tool call carries the failure through to the API: - ChatMessage gains an internal-only `is_error: Option<bool>` (#[serde(default, skip_serializing)]), so the OpenAI-compatible wire is byte-identical and strict endpoints can't reject an unknown message field. - The Anthropic message builder (convert_messages) sets the native `is_error: true` on the tool_result content block on failure (absent on success). - ChatMessage::tool_with_error sets it; both the parallel and sequential tool-dispatch paths now use it. The "Error:" text prefix is preserved as the OpenAI-compatible signal. Self-correction (the heart of loop robustness): Anthropic models get a first-class failure signal instead of inferring it from text. Tests: native is_error only on failure (success/legacy omit it); is_error never on the OpenAI wire. cargo test --lib green (302); clippy adds no new warnings.
1 parent 2194258 commit ef5390f

7 files changed

Lines changed: 114 additions & 7 deletions

File tree

src/agent/compaction.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -772,6 +772,7 @@ mod tests {
772772
tool_calls: None,
773773
tool_call_id: None,
774774
reasoning_content: None,
775+
is_error: None,
775776
}
776777
}
777778

@@ -783,6 +784,7 @@ mod tests {
783784
tool_calls: None,
784785
tool_call_id: Some("call_0".to_string()),
785786
reasoning_content: None,
787+
is_error: None,
786788
}
787789
}
788790

@@ -1045,6 +1047,7 @@ mod tests {
10451047
tool_calls: None,
10461048
tool_call_id: Some(id.to_string()),
10471049
reasoning_content: None,
1050+
is_error: None,
10481051
}
10491052
}
10501053

@@ -1099,6 +1102,7 @@ mod tests {
10991102
tool_calls: None,
11001103
tool_call_id: None,
11011104
reasoning_content: None,
1105+
is_error: None,
11021106
}];
11031107
let (swapped, cached) = swap_all_tool_results_in_place(&mut ctx);
11041108
assert_eq!(swapped, 0);
@@ -1171,6 +1175,7 @@ mod tests {
11711175
tool_calls: None,
11721176
tool_call_id: None,
11731177
reasoning_content: None,
1178+
is_error: None,
11741179
},
11751180
),
11761181
(3, user_msg("u2")),
@@ -1204,6 +1209,7 @@ mod tests {
12041209
tool_calls: None,
12051210
tool_call_id: Some("c1".to_string()),
12061211
reasoning_content: None,
1212+
is_error: None,
12071213
},
12081214
),
12091215
(3, user_msg("u2")),

src/agent/doom_loop.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ mod tests {
145145
tool_calls: Some(calls),
146146
tool_call_id: None,
147147
reasoning_content: None,
148+
is_error: None,
148149
}
149150
}
150151

src/agent/mod.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2527,6 +2527,7 @@ impl AgentLogic {
25272527
tool_calls: Some(tool_calls.clone()),
25282528
tool_call_id: None,
25292529
reasoning_content: response.reasoning_content.clone(),
2530+
is_error: None,
25302531
};
25312532
mem.add_message(assistant_msg).await?;
25322533

@@ -2665,10 +2666,11 @@ impl AgentLogic {
26652666
};
26662667
let _ = outbound_tx.send(BusMessage::Telemetry(tfin.clone())).await;
26672668
hook_observe_telemetry(hook_tool_ctx.as_ref(), &inbound, is_subagent, tfin);
2668-
mem.add_message(crate::utils::ChatMessage::tool(
2669+
mem.add_message(crate::utils::ChatMessage::tool_with_error(
26692670
&tool_result_text,
26702671
&tc.id,
26712672
Some(tool_name.as_str()),
2673+
is_error,
26722674
))
26732675
.await?;
26742676
}
@@ -2770,10 +2772,11 @@ impl AgentLogic {
27702772
let _ = outbound_tx.send(BusMessage::Telemetry(tfin.clone())).await;
27712773
hook_observe_telemetry(hook_tool_ctx.as_ref(), &inbound, is_subagent, tfin);
27722774

2773-
mem.add_message(crate::utils::ChatMessage::tool(
2775+
mem.add_message(crate::utils::ChatMessage::tool_with_error(
27742776
&tool_result_text,
27752777
&tc.id,
27762778
Some(tool_name.as_str()),
2779+
is_error,
27772780
))
27782781
.await?;
27792782
tool_invoked = true;

src/channels/terminal_ui/history_cells.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ mod tests {
110110
tool_calls: None,
111111
tool_call_id: None,
112112
reasoning_content: None,
113+
is_error: None,
113114
},
114115
];
115116
let cells = chat_messages_to_terminal_cells(&messages);

src/memory.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -886,6 +886,9 @@ impl ActorLogic<MemoryMessage> for SqliteMemoryActor {
886886
tool_calls,
887887
tool_call_id: row.get(4)?,
888888
reasoning_content: row.get(5)?,
889+
// Not persisted (skip_serializing internal field); a reloaded
890+
// message keeps only the "Error:" text already in `content`.
891+
is_error: None,
889892
})
890893
})
891894
.map_err(|e| e.to_string())?;
@@ -1388,6 +1391,7 @@ impl ActorLogic<MemoryMessage> for SqliteMemoryActor {
13881391
tool_calls,
13891392
tool_call_id,
13901393
reasoning_content,
1394+
is_error: None,
13911395
},
13921396
))
13931397
})

src/provider.rs

Lines changed: 69 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -229,13 +229,20 @@ impl AnthropicProvider {
229229
.unwrap_or_default();
230230
let tool_call_id = msg.tool_call_id.as_deref().unwrap_or("");
231231

232+
let mut tool_result = json!({
233+
"type": "tool_result",
234+
"tool_use_id": tool_call_id,
235+
"content": result_text
236+
});
237+
// Surface Anthropic's native failure flag so the model gets a structured
238+
// error signal, not just an "Error:" text prefix. Only emit on failure
239+
// (Anthropic treats an absent flag as success).
240+
if msg.is_error == Some(true) {
241+
tool_result["is_error"] = json!(true);
242+
}
232243
anthropic_messages.push(json!({
233244
"role": "user",
234-
"content": [{
235-
"type": "tool_result",
236-
"tool_use_id": tool_call_id,
237-
"content": result_text
238-
}]
245+
"content": [tool_result]
239246
}));
240247
}
241248
_ => {}
@@ -452,3 +459,60 @@ impl Provider for AnthropicProvider {
452459
})
453460
}
454461
}
462+
463+
#[cfg(test)]
464+
mod is_error_tests {
465+
use super::AnthropicProvider;
466+
use crate::utils::ChatMessage;
467+
468+
/// Collect every `tool_result` content block across the converted Anthropic messages.
469+
fn tool_result_blocks(msgs: &[serde_json::Value]) -> Vec<serde_json::Value> {
470+
let mut blocks = Vec::new();
471+
for m in msgs {
472+
if let Some(content) = m.get("content").and_then(|c| c.as_array()) {
473+
for b in content {
474+
if b.get("type").and_then(|t| t.as_str()) == Some("tool_result") {
475+
blocks.push(b.clone());
476+
}
477+
}
478+
}
479+
}
480+
blocks
481+
}
482+
483+
#[test]
484+
fn anthropic_tool_result_sets_is_error_only_on_failure() {
485+
let msgs = vec![
486+
ChatMessage::tool_with_error("Error: boom", "call_1", Some("exec"), true),
487+
ChatMessage::tool_with_error("ok output", "call_2", Some("exec"), false),
488+
ChatMessage::tool("legacy output", "call_3", Some("exec")), // is_error == None
489+
];
490+
let (_system, anthropic) = AnthropicProvider::convert_messages(&msgs);
491+
let blocks = tool_result_blocks(&anthropic);
492+
assert_eq!(blocks.len(), 3, "expected 3 tool_result blocks, got {anthropic:?}");
493+
494+
let by_id = |id: &str| {
495+
blocks
496+
.iter()
497+
.find(|b| b["tool_use_id"] == id)
498+
.unwrap_or_else(|| panic!("missing tool_result for {id}"))
499+
};
500+
// Failure -> native is_error: true.
501+
assert_eq!(by_id("call_1").get("is_error"), Some(&serde_json::json!(true)));
502+
// Success and legacy(None) -> NO is_error key (Anthropic treats absence as success).
503+
assert!(by_id("call_2").get("is_error").is_none());
504+
assert!(by_id("call_3").get("is_error").is_none());
505+
}
506+
507+
#[test]
508+
fn is_error_is_never_serialized_to_openai_wire() {
509+
// The OpenAI-compatible request serializes ChatMessage directly; `is_error` must NOT
510+
// appear (strict endpoints reject unknown message fields).
511+
let msg = ChatMessage::tool_with_error("Error: boom", "call_1", Some("exec"), true);
512+
let v = serde_json::to_value(&msg).expect("serialize");
513+
assert!(
514+
v.get("is_error").is_none(),
515+
"is_error leaked onto the OpenAI-compatible wire: {v}"
516+
);
517+
}
518+
}

src/utils.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,14 @@ pub struct ChatMessage {
9191
/// providers that ignore the field see no change.
9292
#[serde(skip_serializing_if = "Option::is_none")]
9393
pub reasoning_content: Option<String>,
94+
/// For `role == "tool"` messages: whether the tool call failed. INTERNAL-ONLY — never
95+
/// serialized via this struct's serde (`skip_serializing`), so the OpenAI-compatible wire
96+
/// format is byte-identical and strict endpoints can't reject an unknown field. It is read
97+
/// directly by the Anthropic message builder, which sets the native `is_error: true` on the
98+
/// `tool_result` content block so the model gets a structured failure signal (not just an
99+
/// `"Error:"` text prefix). `default` so older persisted messages still deserialize.
100+
#[serde(default, skip_serializing)]
101+
pub is_error: Option<bool>,
94102
}
95103

96104
#[derive(Debug, Clone, Serialize, Deserialize)]
@@ -145,6 +153,7 @@ impl ChatMessage {
145153
tool_calls: None,
146154
tool_call_id: None,
147155
reasoning_content: None,
156+
is_error: None,
148157
}
149158
}
150159

@@ -165,6 +174,7 @@ impl ChatMessage {
165174
tool_calls: None,
166175
tool_call_id: None,
167176
reasoning_content: None,
177+
is_error: None,
168178
}
169179
}
170180

@@ -176,6 +186,7 @@ impl ChatMessage {
176186
tool_calls: None,
177187
tool_call_id: None,
178188
reasoning_content: None,
189+
is_error: None,
179190
}
180191
}
181192

@@ -187,6 +198,7 @@ impl ChatMessage {
187198
tool_calls: None,
188199
tool_call_id: None,
189200
reasoning_content: None,
201+
is_error: None,
190202
}
191203
}
192204

@@ -198,6 +210,22 @@ impl ChatMessage {
198210
tool_calls: None,
199211
tool_call_id: Some(tool_call_id.to_string()),
200212
reasoning_content: None,
213+
is_error: None,
214+
}
215+
}
216+
217+
/// Like [`ChatMessage::tool`] but records whether the tool call failed, so the Anthropic
218+
/// message builder can set the native `is_error` on the `tool_result` block. `content`
219+
/// still carries the human/OpenAI-readable text (typically `"Error: ..."` on failure).
220+
pub fn tool_with_error(
221+
content: &str,
222+
tool_call_id: &str,
223+
name: Option<&str>,
224+
is_error: bool,
225+
) -> Self {
226+
Self {
227+
is_error: Some(is_error),
228+
..Self::tool(content, tool_call_id, name)
201229
}
202230
}
203231
}

0 commit comments

Comments
 (0)