Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions crates/tinyagents-harness/src/agent_loop/mod_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3693,6 +3693,76 @@ async fn provider_invalid_arguments_recoverable_by_relaxed_json_are_repaired_and
);
}

/// The host stack installs `ArgRecoveryMiddleware`, whose `before_tool` runs
/// before admission's relaxed-JSON repair. It used to coerce a provider-invalid
/// call's raw string to `{}` (the tool's schema is unknown to it here, so it
/// counts as permissive), which left repair nothing to fix and answered a
/// repairable call with the bare parse error.
#[tokio::test]
async fn arg_recovery_middleware_leaves_provider_invalid_arguments_for_repair() {
let tool = Arc::new(crate::testkit::FakeTool::returning("lookup", "found it"));
let mut harness: AgentHarness<()> = AgentHarness::new();
harness.with_model_resolver(Arc::new(crate::host::FixedModelResolver::named(
"mock",
Arc::new(MockModel::with_responses(vec![
invalid_tool_call_response("call-x", "lookup", "{query:\"weather\"}"),
text_response("found it", 1, 1),
])),
)));
harness.register_tool(tool.clone());
harness.push_middleware(Arc::new(crate::middleware::ArgRecoveryMiddleware::new(
Vec::new(),
)));

let run = harness
.invoke_in_context(
&(),
RunContext::new(RunConfig::new("arg-recovery-repair"), ()),
vec![Message::user("lookup the weather")],
)
.await
.expect("repaired arguments let the call execute");

assert_eq!(run.text().as_deref(), Some("found it"));
assert_eq!(tool.calls(), vec![json!({"query": "weather"})]);
}

/// An unrepairable call is answered with the parse detail plus a corrective
/// the model can act on, not the bare serde error alone.
#[tokio::test]
async fn unrepairable_arguments_are_answered_with_an_actionable_error() {
let tool = Arc::new(crate::testkit::FakeTool::returning("lookup", "found it"));
let mut harness: AgentHarness<()> = AgentHarness::new();
harness.with_model_resolver(Arc::new(crate::host::FixedModelResolver::named(
"mock",
Arc::new(MockModel::with_responses(vec![
invalid_tool_call_response("call-x", "lookup", "{\"query\": [1 2]}"),
text_response("gave up", 1, 1),
])),
)));
harness.register_tool(tool.clone());

let run = harness
.invoke_in_context(
&(),
RunContext::new(RunConfig::new("unrepairable-args"), ()),
vec![Message::user("lookup")],
)
.await
.expect("unparseable arguments do not fail the run");

assert!(tool.calls().is_empty());
let transcript = format!("{:?}", run.messages);
assert!(
transcript.contains("invalid JSON arguments"),
"{transcript}"
);
assert!(
transcript.contains("matches the tool's argument schema"),
"{transcript}"
);
}

#[tokio::test]
async fn structured_output_is_extracted() {
let mut harness: AgentHarness<()> = AgentHarness::new();
Expand Down
17 changes: 16 additions & 1 deletion crates/tinyagents-harness/src/agent_loop/tools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -869,7 +869,7 @@ impl<State: Send + Sync, Ctx: Send + Sync> AgentHarness<State, Ctx> {
});
status.set_last_event(record.id);
return Ok(ResolvedToolCall::Answered(tinytools::ToolResult::error(
detail,
unparseable_tool_call_message(&call.name, &detail),
)));
}

Expand Down Expand Up @@ -2342,6 +2342,21 @@ fn repeated_truncated_tool_call_message(tool_name: &str) -> String {
)
}

/// The corrective for a call whose arguments the provider could not parse and
/// relaxed-JSON repair could not fix. A bare parse error ("expected `,` or `]`
/// at line 1 column 911") gave the model nothing to act on: production traces
/// show it re-sending the same large inline document until the failure breaker
/// halted the run.
fn unparseable_tool_call_message(tool_name: &str, detail: &str) -> String {
format!(
"Tool call `{tool_name}` was not run: its arguments are not valid JSON ({detail}). \
Re-send it as well-formed JSON that matches the tool's argument schema. If the \
arguments carry a large document, do not inline it again: build it in smaller \
incremental calls (for example with an editing tool that returns a handle or id) and \
pass that reference instead."
)
}

/// Appends a batch's follow-up user messages (B2) after its last tool row,
/// in the calls' original order.
///
Expand Down
27 changes: 22 additions & 5 deletions crates/tinyagents-harness/src/middleware/library/arg_recovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@ use tinytools::Tool;
/// 1. Already a JSON object → leave it (the common, valid case).
/// 2. A JSON-encoded string (optionally fenced) that decodes to an object →
/// decode and use it.
/// 3. Otherwise a non-object whose tool schema declares **no** required fields →
/// 3. A call the provider marked invalid (`ToolCall::invalid`) keeps its raw
/// string, so admission can run its relaxed-JSON repair on it.
/// 4. Otherwise a non-object whose tool schema declares **no** required fields →
/// coerce to `{}` (legacy-engine parity: the tool runs and produces its own
/// recoverable error).
/// 4. Otherwise (non-object + schema has required fields) → leave the arguments
/// 5. Otherwise (non-object + schema has required fields) → leave the arguments
/// untouched so the crate's `InvalidArgsPolicy::ReturnToolError` path reports
/// the original validation failure. Coercing to `{}` would discard the raw
/// malformed value without improving recovery.
Expand Down Expand Up @@ -66,7 +68,9 @@ impl<C: Send + Sync> Middleware<(), C> for ArgRecoveryMiddleware {
}

// (2) JSON-encoded-string arguments (optionally markdown-fenced): decode
// and adopt the inner object.
// and adopt the inner object. A provider marks a fenced object invalid,
// so the decode also clears that mark; otherwise admission would answer
// the now-valid call with the stale parse error.
if let Some(raw) = call.arguments.as_str()
&& let Some(obj) = recover_object_from_json_string(raw)
{
Expand All @@ -75,10 +79,23 @@ impl<C: Send + Sync> Middleware<(), C> for ArgRecoveryMiddleware {
"[tinyagents::mw] arg_recovery: decoded JSON-encoded-string tool arguments to object"
);
call.arguments = obj;
call.invalid = None;
return Ok(());
}

// (3) Non-object with a permissive schema (no required fields): coerce to
// (3) A call the provider marked unparseable keeps its raw string: admission
// runs the relaxed-JSON repair on it next, and otherwise answers with the
// parse error. Coercing it to `{}` here destroyed the input that repair
// needs while the call was still answered as invalid.
if call.invalid.is_some() {
Comment thread
M3gA-Mind marked this conversation as resolved.
tracing::debug!(
tool = call.name.as_str(),
"[tinyagents::mw] arg_recovery: leaving provider-invalid tool arguments for admission repair"
);
return Ok(());
}

// (4) Non-object with a permissive schema (no required fields): coerce to
// `{}` so the tool runs and produces its own recoverable error — engine
// parity for tools that predate the schema gate.
let has_required = self
Expand All @@ -94,7 +111,7 @@ impl<C: Send + Sync> Middleware<(), C> for ArgRecoveryMiddleware {
return Ok(());
}

// (4) Non-object + schema has required fields: leave untouched. The
// (5) Non-object + schema has required fields: leave untouched. The
// crate admission policy surfaces a descriptive, recoverable tool
// result without executing the real tool.
tracing::debug!(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,23 @@ async fn a_non_object_for_a_schema_with_required_fields_is_left_for_the_crate_ga
async fn an_unknown_tool_is_treated_as_permissive() {
assert_eq!(recover("nope", json!(3)).await, json!({}));
}

#[tokio::test]
async fn provider_invalid_arguments_keep_their_raw_string_for_admission_repair() {
let mut ctx: RunContext = RunContext::new(RunConfig::new("mw-test"), ());
let raw = "{query:\"weather\"}";
let mut call = ToolCall::invalid("c1", "loose", raw, "key must be a string");
mw().before_tool(&mut ctx, &(), &mut call).await.unwrap();
assert_eq!(call.arguments, json!(raw));
assert!(call.invalid.is_some());
}

#[tokio::test]
async fn a_provider_invalid_fenced_object_is_decoded_and_cleared() {
let mut ctx: RunContext = RunContext::new(RunConfig::new("mw-test"), ());
let raw = "```json\n{\"q\": \"x\"}\n```";
let mut call = ToolCall::invalid("c1", "strict", raw, "expected value");
mw().before_tool(&mut ctx, &(), &mut call).await.unwrap();
assert_eq!(call.arguments, json!({"q": "x"}));
assert!(call.invalid.is_none());
}
Loading