diff --git a/Cargo.lock b/Cargo.lock index b4aac988..3accf2c3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1644,9 +1644,9 @@ dependencies = [ "sha2", "thiserror", "tinyinference-core", + "tinytools-agent", "tokio", "tracing", - "url", ] [[package]] @@ -1661,7 +1661,7 @@ dependencies = [ [[package]] name = "tinytools" -version = "0.2.0" +version = "0.3.0" dependencies = [ "anyhow", "async-trait", @@ -1671,7 +1671,7 @@ dependencies = [ [[package]] name = "tinytools-agent" -version = "0.2.0" +version = "0.3.0" dependencies = [ "regex", "serde", diff --git a/Cargo.toml b/Cargo.toml index aa93c250..ecd4dc71 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,3 +27,12 @@ all = { level = "warn", priority = -1 } lto = "thin" codegen-units = 1 strip = "debuginfo" + +# `tinyinference-llm` names `tinytools-agent` as a git dependency (it is not on +# crates.io). Inside this workspace that crate is the vendored submodule, so the +# git source is redirected onto the path — one copy of the protocol crate in the +# graph, and the harness's `path` dependency and the provider's dependency are +# the same type. +[patch."https://github.com/tinyhumansai/tinytools"] +tinytools-agent = { path = "vendor/tinytools/crates/tinytools-agent" } +tinytools = { path = "vendor/tinytools/crates/tinytools" } diff --git a/crates/tinyagents-graph/Cargo.toml b/crates/tinyagents-graph/Cargo.toml index 80eef596..325b2fac 100644 --- a/crates/tinyagents-graph/Cargo.toml +++ b/crates/tinyagents-graph/Cargo.toml @@ -18,7 +18,7 @@ tinyagents-harness = { path = "../tinyagents-harness", version = "2.1.2", defaul tinyagents-language = { path = "../tinyagents-language", version = "2.1.2" } tinyagents-tracing = { path = "../tinyagents-tracing", version = "2.1.2", default-features = false } tinyinference-llm = { path = "../../vendor/tinyinference/crates/tinyinference-llm", version = "0.3.0" } -tinytools = { path = "../../vendor/tinytools/crates/tinytools", version = "0.2.0" } +tinytools = { path = "../../vendor/tinytools/crates/tinytools", version = "0.3.0" } tokio = { version = "1", default-features = false, features = ["sync", "time", "macros", "rt", "rt-multi-thread", "fs"] } [features] diff --git a/crates/tinyagents-harness/Cargo.toml b/crates/tinyagents-harness/Cargo.toml index 165db73c..32e5de72 100644 --- a/crates/tinyagents-harness/Cargo.toml +++ b/crates/tinyagents-harness/Cargo.toml @@ -28,10 +28,10 @@ sha2 = "0.11" thiserror = "2" tinyagents-tracing = { path = "../tinyagents-tracing", version = "2.1.2", default-features = false } tinyagents-definition = { path = "../tinyagents-definition", version = "2.1.2" } -tinytools-agent = { path = "../../vendor/tinytools/crates/tinytools-agent", version = "0.2.0", default-features = false } +tinytools-agent = { path = "../../vendor/tinytools/crates/tinytools-agent", version = "0.3.0", default-features = false } tinyinference-llm = { path = "../../vendor/tinyinference/crates/tinyinference-llm", version = "0.3.0" } tinyinference-embeddings = { path = "../../vendor/tinyinference/crates/tinyinference-embeddings", version = "0.3.0" } -tinytools = { path = "../../vendor/tinytools/crates/tinytools", version = "0.2.0" } +tinytools = { path = "../../vendor/tinytools/crates/tinytools", version = "0.3.0" } tokio = { version = "1", default-features = false, features = ["sync", "time", "macros", "rt", "rt-multi-thread", "fs", "io-util", "process"] } tempfile = "3" wait-timeout = "0.2" diff --git a/crates/tinyagents-harness/src/README.md b/crates/tinyagents-harness/src/README.md index b03220ad..03f970d5 100644 --- a/crates/tinyagents-harness/src/README.md +++ b/crates/tinyagents-harness/src/README.md @@ -38,7 +38,6 @@ the module map below. | `observability` | Durable observability — journals, status stores, sinks — making the live event stream persistent. See [`observability/README.md`](observability/README.md). | | `prompt` | Prompt assembly — templates and `PromptBuilder` turning runtime values into the final request. | | `providers` | Model adapters whose behavior depends on TinyAgents-specific prompt dialects (e.g. Claude Code/Agent SDK). | -| `relaxed_json` (crate-private) | Best-effort repair of malformed JSON small local models emit for tool-call arguments. | | `retriever` | Provider-neutral retrieval contracts (`Retriever`) for injecting ranked context into a prompt. See [`retriever/README.md`](retriever/README.md). | | `retry` | Retry/backoff, model fallback, and rate-limiting policies applied uniformly to every model call. See [`retry/README.md`](retry/README.md). | | `run_queue` | A generic multi-lane FIFO queue (steer/followup/collect) for messages arriving during an active run. See [`run_queue/README.md`](run_queue/README.md). | diff --git a/crates/tinyagents-harness/src/agent_loop/dialect.rs b/crates/tinyagents-harness/src/agent_loop/dialect.rs new file mode 100644 index 00000000..6d25bba0 --- /dev/null +++ b/crates/tinyagents-harness/src/agent_loop/dialect.rs @@ -0,0 +1,316 @@ +//! Host-side selection and application of the tool dialect for one run. +//! +//! The protocol itself — how a call is rendered, parsed, repaired, and +//! scrubbed from a stream — is owned by `tinytools-agent`. What is decided +//! *here* is the host's part: which dialect a run speaks +//! ([`RunPolicy::tool_dialect`]), the rewrite of a request onto a text +//! protocol when one is forced, the minting of call ids for calls recovered +//! from text, and the fallback that reads a native model's narrated call out +//! of its visible text. +//! +//! [`RunPolicy::tool_dialect`]: crate::runtime::RunPolicy::tool_dialect + +use std::sync::Arc; + +use tinyinference_llm::message::ContentBlock; +use tinyinference_llm::model::{ModelRequest, ModelResponse, ToolChoice}; +use tinyinference_llm::tool::{ToolCall, ToolSchema}; +use tinytools_agent::dialect::PFormatDialect; +use tinytools_agent::types::{ParseOptions, ParsedToolCall}; +use tinytools_agent::{PFormatRegistry, StreamScrubber}; + +use crate::config::ToolDispatcher; +use crate::ids::CallId; + +/// The dialect a run speaks, resolved once from policy. +#[derive(Debug, Clone)] +pub(super) enum RunDialect { + /// Schemas on the wire; the provider adapter owns any text fallback. + Native, + /// JSON-in-tag, rendered into the system prompt by the host. + Xml, + /// Positional P-Format, rendered into the system prompt by the host. + PFormat(Arc), +} + +impl RunDialect { + /// Resolves the policy against the tools this run offers. + pub(super) fn resolve(dispatcher: ToolDispatcher, tools: &[ToolSchema]) -> Self { + match dispatcher { + ToolDispatcher::Auto | ToolDispatcher::Native => Self::Native, + ToolDispatcher::Xml => Self::Xml, + ToolDispatcher::Pformat => Self::PFormat(Arc::new(tinytools_agent::build_registry( + tools + .iter() + .map(|schema| (schema.name.clone(), schema.parameters.clone())), + ))), + } + } + + /// Whether the host renders the protocol and parses the answer itself. + pub(super) fn is_text(&self) -> bool { + !matches!(self, Self::Native) + } + + /// The P-Format registry for one call, extended with any tool in `tools` + /// beyond the run-level set the registry was built from. + /// + /// The run-level registry is built once from the schemas offered at the + /// start of the run (see [`Self::resolve`]); a synthetic per-turn tool — + /// the structured-output fallback schema pushed onto `request.tools` + /// after that — is advertised in the P-Format catalogue (rendered fresh + /// from the final tool list on every call) but would otherwise have no + /// positional layout to decode a call against. Extending here, rather + /// than rebuilding from scratch every call, keeps the common case (no + /// new tool this turn) a cheap `Arc::clone`. + pub(super) fn registry_for(&self, tools: &[ToolSchema]) -> Option> { + match self { + Self::PFormat(registry) => { + let extra: Vec<&ToolSchema> = tools + .iter() + .filter(|schema| !registry.contains_key(&schema.name)) + .collect(); + if extra.is_empty() { + return Some(Arc::clone(registry)); + } + let mut merged = (**registry).clone(); + merged.extend(tinytools_agent::build_registry( + extra + .into_iter() + .map(|schema| (schema.name.clone(), schema.parameters.clone())), + )); + Some(Arc::new(merged)) + } + _ => None, + } + } + + /// Rewrites `request` onto this dialect's text protocol: the transcript + /// is folded into forms a prompt-guided model can read, the protocol block + /// and catalogue go into the system prompt, and no schema goes on the + /// wire. A no-op for [`Self::Native`] or when no tools are offered. + pub(super) fn apply_to_request(&self, request: &mut ModelRequest) { + if !self.is_text() || request.tools.is_empty() || request.tool_choice == ToolChoice::None { + return; + } + use tinyinference_llm::prompt_tools; + + let tools = std::mem::take(&mut request.tools); + let messages = prompt_tools::coalesce_tool_results(&request.messages); + let messages = prompt_tools::ensure_resolvable_user_turn(&messages); + request.messages = match self { + Self::Xml | Self::Native => { + prompt_tools::with_tool_instructions(&messages, &tools, &request.tool_choice) + } + Self::PFormat(_) => { + let specs: Vec = tools + .iter() + .map(|schema| tinytools_agent::tinytools::ToolSpec { + name: schema.name.clone(), + description: schema.description.clone(), + parameters: schema.parameters.clone(), + }) + .collect(); + let mut block = PFormatDialect::instructions(); + block.push_str(&tinytools_agent::render::render_pformat_catalogue(&specs)); + // The XML branch renders `tool_choice` into its instructions + // via `prompt_tools::tool_instructions`; P-Format has no + // schema on the wire either (the wire choice is reset to + // `Auto` below), so a forced choice has to be said in plain + // English here too or `Required`/`Tool(name)` silently loses + // its meaning — in particular the sole synthetic + // structured-output tool would no longer be forced, and a + // plain-text response would make extraction fail. + match &request.tool_choice { + ToolChoice::Required => { + block.push_str("\nYou must emit at least one tool call.\n"); + } + ToolChoice::Tool(name) => { + block.push_str(&format!("\nYou must call the `{name}` tool.\n")); + } + ToolChoice::Auto | ToolChoice::None => {} + } + prompt_tools::append_system_block(&messages, &block) + } + }; + request.tool_choice = ToolChoice::Auto; + } +} + +/// What a model call needs in order to recover text-dialect calls: the +/// tools that were offered (which a text-dialect request no longer carries +/// on the wire) and the P-Format registry, when there is one. +#[derive(Debug, Clone, Default)] +pub(super) struct TextRecovery { + /// The tools offered this turn, before any dialect rewrite. + pub(super) offered: Arc>, + /// The P-Format layouts, for [`RunDialect::PFormat`]. + pub(super) registry: Option>, +} + +impl TextRecovery { + /// A scrubber for one streamed model call, or `None` when no tools were + /// offered and there is nothing to recover. + pub(super) fn scrubber(&self, model_call_id: &CallId) -> Option { + (!self.offered.is_empty()).then(|| { + DeltaScrubber::new(model_call_id.clone(), &self.offered, self.registry.clone()) + }) + } +} + +/// How one model call is made: streamed or unary, and what it needs to +/// recover text-dialect calls from the answer. +#[derive(Debug, Clone, Default)] +pub(super) struct CallShape { + /// Whether the provider's streaming path is used. + pub(super) streaming: bool, + /// Offered tools and P-Format registry for text recovery. + pub(super) recovery: TextRecovery, +} + +/// Converts a recovered call into the harness's [`ToolCall`], minting an id +/// scoped to the model call it came from. +/// +/// `{model_call_id}-tool-{n}` is unique per run by construction — model call +/// ids already are — and visibly distinct from any provider's, so a +/// recovered call can never be confused with a native one in a transcript. +fn to_tool_call(call: ParsedToolCall, model_call_id: &CallId, slot: usize) -> ToolCall { + // `call.id` is intentionally never used, even when a grammar or a future + // change to `tinytools-agent` happens to populate one: this function's + // whole contract (see its doc comment) is that a text-recovered call's id + // is always host-minted and unique per run, so it can never collide with + // another recovered call or be confused with a native provider one. A + // parser-supplied id would be model-controlled input; trusting it here + // would let two calls collide on an id the model chose, or let a + // narrated call impersonate a specific native one. + let id = format!("{model_call_id}-tool-{slot}"); + ToolCall::new(id, call.name, call.arguments) +} + +/// Reads text-dialect calls out of a response that carries no structured +/// ones, through every grammar `tinytools-agent` knows, with the offered +/// tools enabling name repair. Non-text content blocks (reasoning) survive. +pub(super) fn recover_text_calls( + response: &mut ModelResponse, + model_call_id: &CallId, + offered: &[ToolSchema], + registry: Option<&PFormatRegistry>, +) { + if offered.is_empty() { + return; + } + let known: Vec = offered.iter().map(|tool| tool.name.clone()).collect(); + let mut options = ParseOptions::new().with_known_tools(&known); + if let Some(registry) = registry { + options = options.with_registry(registry); + } + let text = response.text(); + let outcome = tinytools_agent::parse_text(&text, &options); + if outcome.calls.is_empty() { + return; + } + for diagnostic in &outcome.diagnostics { + tinyagents_tracing::debug!(?diagnostic, "[agent_loop] text-dialect recovery"); + } + // Appended, not assigned: a provider can legitimately return one native + // structured call *and* narrate a second one as text in the same + // response (this is deliberately parsed even when `tool_calls` was + // already non-empty — see above), and overwriting the collection here + // used to silently drop whichever set ran second. + let recovered = outcome + .calls + .into_iter() + .enumerate() + .map(|(index, call)| to_tool_call(call, model_call_id, index + 1)); + response.message.tool_calls.extend(recovered); + response.message.content = + replace_text_blocks(std::mem::take(&mut response.message.content), outcome.text); +} + +/// Keeps every non-text block in place and substitutes one cleaned text at +/// the first text block's position; an empty `cleaned` emits no text block. +fn replace_text_blocks(content: Vec, cleaned: String) -> Vec { + let mut out = Vec::with_capacity(content.len()); + let mut inserted = false; + for block in content { + match block { + ContentBlock::Text(_) => { + if !inserted { + if !cleaned.is_empty() { + out.push(ContentBlock::Text(cleaned.clone())); + } + inserted = true; + } + } + other => out.push(other), + } + } + if !inserted && !cleaned.is_empty() { + out.push(ContentBlock::Text(cleaned)); + } + out +} + +/// Scrubs tool-call markup from streamed visible text and collects the +/// calls it completes, minting harness ids for them. +/// +/// Consumers of [`AgentEvent::ModelDelta`](crate::events::AgentEvent::ModelDelta) +/// never see a partial ``; the calls surface on the terminal +/// response instead, exactly once. +pub(super) struct DeltaScrubber { + inner: StreamScrubber, + model_call_id: CallId, + calls: Vec, +} + +impl DeltaScrubber { + /// A scrubber for one model call, knowing the tools it offered. + pub(super) fn new( + model_call_id: CallId, + offered: &[ToolSchema], + registry: Option>, + ) -> Self { + let known = offered.iter().map(|tool| tool.name.clone()).collect(); + let mut inner = StreamScrubber::new().with_known_tools(known); + if let Some(registry) = registry { + inner = inner.with_registry(registry); + } + Self { + inner, + model_call_id, + calls: Vec::new(), + } + } + + /// Feeds one text delta; returns the text safe to forward. + pub(super) fn feed(&mut self, text: &str) -> String { + let step = self.inner.feed(text); + self.collect(step.calls); + step.text + } + + /// Drains the remainder at end of stream. + pub(super) fn flush(&mut self) -> String { + let step = self.inner.flush(); + self.collect(step.calls); + step.text + } + + fn collect(&mut self, calls: Vec) { + for call in calls { + let slot = self.calls.len() + 1; + self.calls + .push(to_tool_call(call, &self.model_call_id, slot)); + } + } + + /// The calls completed during the stream, in order. + pub(super) fn into_calls(self) -> Vec { + self.calls + } + + /// Whether any call was completed during the stream. + pub(super) fn has_calls(&self) -> bool { + !self.calls.is_empty() + } +} diff --git a/crates/tinyagents-harness/src/agent_loop/mod.rs b/crates/tinyagents-harness/src/agent_loop/mod.rs index 7d2f3003..beaa97c7 100644 --- a/crates/tinyagents-harness/src/agent_loop/mod.rs +++ b/crates/tinyagents-harness/src/agent_loop/mod.rs @@ -115,6 +115,7 @@ use tinyinference_llm::model::{ }; use tinyinference_llm::tool::{ToolCall, ToolSchema}; +mod dialect; mod entry; mod model_call; mod run_loop; diff --git a/crates/tinyagents-harness/src/agent_loop/model_call.rs b/crates/tinyagents-harness/src/agent_loop/model_call.rs index 091b33e0..0070d30a 100644 --- a/crates/tinyagents-harness/src/agent_loop/model_call.rs +++ b/crates/tinyagents-harness/src/agent_loop/model_call.rs @@ -142,8 +142,9 @@ impl AgentHarness { request: &ModelRequest, call_id: &CallId, binding: ResolvedModelBinding, - streaming: bool, + shape: &super::dialect::CallShape, ) -> Result { + let streaming = shape.streaming; let policy = self.effective_cache_policy(request); // The identity of the model that is actually about to be called — known // only *after* resolution, which is why the key cannot be finalized by @@ -269,7 +270,7 @@ impl AgentHarness { }; let response = self - .invoke_model_resolving(state, ctx, effective_request, call_id, binding, streaming) + .invoke_model_resolving(state, ctx, effective_request, call_id, binding, shape) .await?; if let Some((cache, key)) = decision.as_ref() { @@ -484,8 +485,9 @@ impl AgentHarness { request: &ModelRequest, call_id: &CallId, binding: ResolvedModelBinding, - streaming: bool, + shape: &super::dialect::CallShape, ) -> Result { + let streaming = shape.streaming; let mut current_name = binding.resolved.name.clone(); let mut model = binding.model; let mut resolved = binding.resolved; @@ -534,6 +536,7 @@ impl AgentHarness { request, call_id, &mut deltas_emitted, + shape, ); Self::with_call_budget(remaining, run_id.as_str(), "model call", bound, fut) .await @@ -824,6 +827,10 @@ impl AgentHarness { /// `deltas_emitted` is incremented for every delta actually handed to /// consumers, so the retry path can tell whether a failed attempt already /// published output that now has to be discarded. + // `deltas_emitted` must stay an out-parameter: on the error path the + // retry logic reads how much output already reached consumers, which a + // return value could not carry alongside the error. + #[allow(clippy::too_many_arguments)] async fn invoke_model_streaming_once( &self, state: &State, @@ -832,9 +839,15 @@ impl AgentHarness { request: &ModelRequest, call_id: &CallId, deltas_emitted: &mut usize, + shape: &super::dialect::CallShape, ) -> Result { + let recovery = &shape.recovery; let mut stream = model.stream(state, request.clone()).await?; let mut accumulator = StreamAccumulator::new(); + // Tool-call markup a model narrates as text is held back from live + // consumers and turned into calls on the terminal response instead. + // Runs for every provider: native models narrate calls often enough. + let mut text_scrubber = recovery.scrubber(call_id); // A terminal `Completed` response usually has richer provider metadata // than deltas (message id, usage, tool calls, and route information), // but its text is still the raw provider payload. Keep the text and @@ -867,6 +880,67 @@ impl AgentHarness { }, }; + // Scrub tool-call markup from visible text before anything else + // sees it; a delta the scrubber empties carries nothing to emit. + if let (Some(scrubber), ModelStreamItem::MessageDelta(delta)) = + (text_scrubber.as_mut(), &mut item) + && !delta.text.is_empty() + { + delta.text = scrubber.feed(&delta.text); + if delta.text.is_empty() && delta.reasoning.is_empty() && delta.tool_call.is_none() + { + continue; + } + } + if let (Some(scrubber), ModelStreamItem::Completed(_)) = (text_scrubber.as_mut(), &item) + { + let tail = scrubber.flush(); + if !tail.is_empty() { + // The held-back remainder is ordinary text after all. + // Route it through the same delta middleware pipeline as + // every other streamed delta (below): a naive direct + // emit skipped `run_on_model_delta` and host progress, so + // redaction/policy/transformation middleware could not + // inspect or suppress this tail and consumers saw it + // behave differently from every other delta. + let mut model_delta = ModelDelta { + call_id: call_id.as_str().to_string(), + content: tail, + reasoning: String::new(), + tool_call: None, + }; + self.middleware + .run_on_model_delta(ctx, state, &mut model_delta) + .await?; + // Unconditional, not gated on the post-middleware content: + // the pre-middleware tail here is always non-empty (the + // surrounding `if` already checked it), matching the + // ordinary delta path below, which ORs the *pre*-middleware + // text against the post-middleware one. Gating on + // `model_delta.content` alone meant a middleware that + // suppressed the whole tail to `""` left + // `saw_streamed_content` false, which skipped terminal + // reconciliation and let the provider's raw (unscrubbed) + // `Completed` content silently restore the exact text the + // middleware had just suppressed. + saw_streamed_content = true; + streamed_text.push_str(&model_delta.content); + ctx.emit(AgentEvent::ModelDelta { + run_id: ctx.config.run_id.clone(), + call_id: call_id.clone(), + delta: MessageDelta::text(model_delta.content.clone()), + }); + crate::runtime::emit_host_progress::( + ctx, + crate::host::ProgressEvent::Token { + run: ctx.run_id().clone(), + text: model_delta.content, + }, + ); + *deltas_emitted += 1; + } + } + // Surface incremental message/tool-call fragments through events and // the `on_model_delta` middleware hook before merging them. let message_delta = match &item { @@ -949,8 +1023,19 @@ impl AgentHarness { *deltas_emitted += 1; } + // Reconcile even when nothing ordinary streamed: a response that + // is *purely* text-dialect tool-call markup suppresses every + // delta (so `saw_streamed_content` stays false) but still needs + // its raw ``-style text replaced — otherwise that raw + // markup survives in the terminal response's content block + // alongside the structured calls the scrubber recovered below, + // and gets persisted into the transcript to be replayed back to + // the model next turn. + let scrubber_recovered_calls = text_scrubber + .as_ref() + .is_some_and(super::dialect::DeltaScrubber::has_calls); if let ModelStreamItem::Completed(response) = &mut item - && saw_streamed_content + && (saw_streamed_content || scrubber_recovered_calls) { // Deltas represent only text/thinking, so preserve terminal // blocks that cannot be streamed as a `ModelDelta` (JSON, @@ -978,6 +1063,21 @@ impl AgentHarness { })); response.message.content = content; } + if let ModelStreamItem::Completed(response) = &mut item + && text_scrubber + .as_ref() + .is_some_and(super::dialect::DeltaScrubber::has_calls) + && let Some(scrubber) = text_scrubber.take() + { + // The streamed text held complete tool-call blocks, scrubbed + // from the reconciled text above, so this is the only place + // they can be dispatched from. Appended, not assigned: a + // provider can legitimately return a native structured call + // *and* narrate a second one as text in the same turn, and + // gating this on `tool_calls.is_empty()` used to silently + // drop the narrated one whenever a native call was present. + response.message.tool_calls.extend(scrubber.into_calls()); + } if let ModelStreamItem::Completed(response) = &mut item && saw_tool_delta { @@ -1022,7 +1122,7 @@ pub(super) struct ModelCallBase<'h, State: Send + Sync, Ctx: Send + Sync> { pub(super) resolved: ResolvedModel, pub(super) model: Arc>, pub(super) required_capabilities: Option, - pub(super) streaming: bool, + pub(super) shape: super::dialect::CallShape, } impl ModelCallBase<'_, State, Ctx> { @@ -1117,14 +1217,7 @@ impl ModelBaseCall super::run_loop::refresh_prompt_cache_fingerprint(&mut request); let binding = self.rebind(ctx, &request).await?; self.harness - .invoke_model_with_retry( - state, - ctx, - &request, - &self.call_id, - binding, - self.streaming, - ) + .invoke_model_with_retry(state, ctx, &request, &self.call_id, binding, &self.shape) .await }) } diff --git a/crates/tinyagents-harness/src/agent_loop/run_loop.rs b/crates/tinyagents-harness/src/agent_loop/run_loop.rs index 5b0bbc24..2976ed2d 100644 --- a/crates/tinyagents-harness/src/agent_loop/run_loop.rs +++ b/crates/tinyagents-harness/src/agent_loop/run_loop.rs @@ -206,6 +206,15 @@ impl AgentHarness { } } } + // The dialect itself is resolved per turn, once the model for that + // turn is known (see the `run_dialect` binding below, right after + // `binding`): `Auto` needs the model's capability to decide between + // `Native` and the documented `Xml` fallback, and that capability is + // not known this early. `tool_schemas` — what the text protocols need + // a registry built from (the *prepared* direct set plus the discovery + // bridge, i.e. exactly what is rendered into the catalogue and can + // come back as a call) — is fixed for the whole run and captured here. + // Fail closed on a structured-output schema whose name collides with a // registered tool *or* the intrinsic discovery bridge. Under the // tool-call strategy the schema is sent as an extra `function` entry, @@ -262,6 +271,9 @@ impl AgentHarness { // records the original cap so growth stays clamped at 4x, and the counter // bounds how many times we re-issue the call. let mut truncated_empty_retries_used: u32 = 0; + // Consecutive "you said tool_calls but sent none" re-prompts + // (see `RunPolicy::dropped_tool_call_nudges`). + let mut dropped_tool_call_nudges_used: u32 = 0; let mut boosted_max_tokens: Option = None; let mut truncation_base: Option = None; @@ -415,6 +427,55 @@ impl AgentHarness { .run_before_model(ctx, state, &mut request) .await?; + // `ToolDispatcher::Native` is documented as *forcing* provider-native + // tool calls, unlike `Auto`'s "native when available, else Xml". + // `RunDialect::resolve` maps both to the same `Native` variant (it + // only decides whether *this* host renders a text protocol), so + // without a capability requirement that promise was unenforceable: + // a model profile lacking `tool_calling` could still be resolved, + // and a provider adapter is free to fall back to its own text + // encoding for such a profile. Requiring the capability makes + // resolution itself fail closed for an incapable model. An + // adapter's own *runtime* degrade after a live "tools not + // supported" provider response is a separate, adapter-internal + // reliability behavior this host-side dialect selection has no + // visibility into or control over. + // + // Checked against `request.tools` (the *effective* tool set), + // not the pre-`before_model` `tool_schemas` snapshot: a run that + // starts with no tools but whose `before_model` middleware adds + // some must still be gated — checking the earlier snapshot would + // silently let those middleware-added tools reach an + // incapable-of-native-tool-calling model. + // + // Also gated on an `Auto` structured-output format even when + // `request.tools` is still empty here: `StructuredStrategy` + // resolution (below, after `binding`) only ever appends a + // synthetic tool-call schema for a model whose profile already + // has `tool_calling` (`StructuredStrategy::for_profile`'s + // `ToolCall` arm), so requiring it up front is what makes that + // later fact true rather than merely hoped for — by the time + // structured planning knows whether a schema tool is needed the + // model is already resolved, too late to gate resolution on. + // Requiring the capability here is conservatively broader than + // strictly necessary for a model that would have used + // `ProviderSchema` instead, but never wrong: a fail-closed + // requirement narrowing the candidate pool is the point of this + // gate. + let structured_output_may_need_tool_calling = matches!( + self.policy.default_response_format, + Some(ResponseFormat::Auto { .. }) + ); + if matches!( + self.policy.tool_dialect, + crate::config::ToolDispatcher::Native + ) && (!request.tools.is_empty() || structured_output_may_need_tool_calling) + { + let mut required = request.required_capabilities.clone().unwrap_or_default(); + required.tool_calling = true; + request.required_capabilities = Some(required); + } + // Resolve the model for the event/log name before invoking. // Hosted turns install their routing decision against this live // `RunContext`; explicit-model SDK calls continue to resolve only @@ -436,6 +497,42 @@ impl AgentHarness { }; let model_name = binding.resolved.name.clone(); + // Resolved per turn (not once for the whole run) because `Auto` + // needs the *resolved* model's capability, known only now: + // `ToolDispatcher::Auto` is documented as "provider-native tool + // calls when the provider supports them, otherwise Xml", but + // mapping it to the same host-side-no-op behavior as `Native` + // (as an earlier version of this dialect resolution did) left + // that fallback unenforced — a model with `tool_calling: false` + // selected under `Auto` would receive a request that still + // depended on provider-native tools, with no host-rendered text + // protocol and no adapter guaranteed to supply one. `Native` + // stays forced regardless of capability (it fails closed at + // resolution instead, via the capability requirement above); + // `Xml`/`Pformat` stay forced as explicit opt-ins. + let effective_dispatcher = match self.policy.tool_dialect { + crate::config::ToolDispatcher::Auto => { + // A model with *no declared profile at all* is unknown, + // not incapable — treated as capable (the historical + // behavior, and correct for hosts/tests that never + // bother declaring a profile). Only an explicit + // `tool_calling: false` triggers the documented Xml + // fallback. + if binding + .model + .profile() + .is_none_or(|profile| profile.tool_calling) + { + crate::config::ToolDispatcher::Native + } else { + crate::config::ToolDispatcher::Xml + } + } + other => other, + }; + let run_dialect = + super::dialect::RunDialect::resolve(effective_dispatcher, &tool_schemas); + // An explicit request override that resolution skipped (unknown // name, missing capability, or provider-retired) falls through to // a lower-priority candidate by documented fail-closed semantics; @@ -518,11 +615,58 @@ impl AgentHarness { _ => None, }; + // What was offered is fixed here, before a text dialect strips + // the schemas off the wire: recovery and the stream scrubber need + // the names, and the structured-output schema tool counts. The + // registry is extended (not just the run-level one) so a + // per-turn synthetic tool — the structured-output fallback + // schema just pushed above — has a positional layout to decode + // a recovered call against; the catalogue already advertises it + // because it is rendered fresh from `tools` on every call. + let offered_tool_count = request.tools.len(); + // An empty recovery when the effective choice is `None`: a + // `before_model` middleware asking for no tool calls this turn + // must actually get none. `apply_to_request` below already skips + // its rewrite for `None`, but that alone left recovery/the + // stream scrubber still treating every offered name as + // recognizable — so a model that narrated `` markup + // as plain text anyway would still have it parsed and dispatched + // as a real, side-effecting call despite the explicit + // prohibition. An empty `offered` list makes every grammar in + // `tinytools-agent` decline to recognize anything as a call. + let recovery = if request.tool_choice == ToolChoice::None { + super::dialect::TextRecovery::default() + } else { + super::dialect::TextRecovery { + offered: Arc::new(request.tools.clone()), + registry: run_dialect.registry_for(&request.tools), + } + }; + // Whether this turn could possibly have accepted a tool call at + // all — reuses `recovery.offered`, which is already empty + // exactly when no tools were offered or the effective choice was + // `None`. Read below by the dropped-tool-call nudge: nudging a + // model to "issue the call" when no call could ever have been + // accepted wastes up to `dropped_tool_call_nudges` model calls + // asking for something impossible before falling through. + let tools_available_this_turn = !recovery.offered.is_empty(); + // Applied before budget preflight below: for a text dialect this + // rewrite folds the protocol block and full tool catalogue into + // `request.messages` and clears `request.tools`, and that is the + // request whose size the budget estimate has to reflect. Doing + // this after preflight (as before) let a prompt near + // `max_input_tokens` pass admission on the small structured + // request and then send a materially larger rendered-text one, + // defeating the pre-call budget limit. + run_dialect.apply_to_request(&mut request); + // A host budget is acquired only for an explicit host-driven run. - // Do it after structured-output planning: a synthetic schema tool - // is part of the provider request and must be included in its - // estimate. The permit remains alive through response accounting, - // so cancellation or a provider error still releases it through + // Do it after structured-output planning and the dialect + // rewrite: a synthetic schema tool and, for a text dialect, the + // rendered protocol/catalogue text are both part of the actual + // provider request and must be included in its estimate. The + // permit remains alive through response accounting, so + // cancellation or a provider error still releases it through // Drop. let host_budget = if let Some(host_run) = crate::runtime::host_invocation_binding::(ctx)? @@ -558,7 +702,7 @@ impl AgentHarness { .cloned() .unwrap_or_else(|| ctx.run_id().as_str().into()), ) - .with_tool_count(request.tools.len()); + .with_tool_count(offered_tool_count); let permit = match self.call_budget(ctx) { Some(remaining) => tokio::select! { biased; @@ -587,7 +731,6 @@ impl AgentHarness { } else { None }; - let request_has_tools = !request.tools.is_empty(); let call_id = CallId::new(format!("{}-model-{}", ctx.run_id(), run.model_calls + 1)); status.mark_running(HarnessPhase::Model); @@ -606,13 +749,18 @@ impl AgentHarness { // already ran above; the wrap onion runs here; lifecycle // `after_model` runs below — so ordering is: // before_model -> wrap onion (outer..inner..base) -> after_model. + // `recovery` and the dialect rewrite were computed above, before + // budget preflight (see the comment there for why). let base = ModelCallBase { harness: self, call_id: call_id.clone(), resolved: binding.resolved, model: binding.model, required_capabilities: request.required_capabilities.clone(), - streaming, + shape: super::dialect::CallShape { + streaming, + recovery: recovery.clone(), + }, }; // Snapshot the request messages for observability before `request` // is moved into the model-wrap onion, gated by the capture policy so @@ -633,11 +781,16 @@ impl AgentHarness { .into_response(); // Providers occasionally put a text-dialect call in visible - // content even when a native tool channel was offered. Use the - // canonical TinyTools-Agent parser rather than the retired - // harness prompt parser, and only recover when the provider did - // not already supply structured calls. - recover_text_dialect_calls(&mut response, &call_id, request_has_tools); + // content even when a native tool channel was offered, and a + // forced text dialect always does. Read the response through + // every grammar the protocol crate knows, but only when the + // provider did not already supply structured calls. + super::dialect::recover_text_calls( + &mut response, + &call_id, + &recovery.offered, + recovery.registry.as_deref(), + ); // Account for the completed provider response before fallible // response middleware. A middleware rejection must not erase @@ -774,6 +927,14 @@ impl AgentHarness { )); } + // A mixed turn (structured payload alongside real tool calls) + // is a resolved turn exactly like an ordinary tool-calling one + // (see the reset below at the non-mixed path): it must not + // leave a spent `dropped_tool_call_nudges_used` counter to + // leak into a later, unrelated dropped-call turn, which would + // otherwise receive fewer than the policy's configured number + // of consecutive re-prompts. + dropped_tool_call_nudges_used = 0; reset_truncated_empty_recovery( &mut truncated_empty_retries_used, &mut boosted_max_tokens, @@ -832,6 +993,26 @@ impl AgentHarness { continue; } + // Dropped tool call: the provider says the model stopped to + // call a tool, but nothing arrived — structured or in text. + // A bounded re-prompt asks for the call itself. The assistant + // row stays on the transcript so the model sees what it did. + if tool_calls.is_empty() + && response.finish_reason.as_deref() == Some("tool_calls") + && tools_available_this_turn + && dropped_tool_call_nudges_used < self.policy.dropped_tool_call_nudges + { + dropped_tool_call_nudges_used += 1; + messages.push(Message::user(DROPPED_TOOL_CALL_NUDGE)); + let record = ctx.emit(AgentEvent::RetryScheduled { + call_id: call_id.clone(), + attempt: dropped_tool_call_nudges_used as usize, + }); + status.set_last_event(record.id); + continue; + } + dropped_tool_call_nudges_used = 0; + // This turn resolved without scheduling a truncated-empty // retry, so the recovery state must not leak into later turns: // a stale `boosted_max_tokens` would override the caller's @@ -888,6 +1069,7 @@ impl AgentHarness { // A tool-calling response is a resolved turn too: clear the // recovery state before the tools run so the next turn starts from // the caller's configured cap and a full retry budget. + dropped_tool_call_nudges_used = 0; reset_truncated_empty_recovery( &mut truncated_empty_retries_used, &mut boosted_max_tokens, @@ -1147,63 +1329,12 @@ fn apply_host_budget_compression( Ok(()) } -/// Recovers XML/text-dialect calls through `tinytools-agent` while preserving -/// every non-text provider content block (notably reasoning blocks). -fn recover_text_dialect_calls( - response: &mut tinyinference_llm::model::ModelResponse, - model_call_id: &CallId, - has_tools: bool, -) { - if !has_tools || !response.message.tool_calls.is_empty() { - return; - } - - use tinytools_agent::dialect::{DialectResponse, ToolDialect, XmlDialect}; - - let dialect_response = DialectResponse { - text: Some(response.text()), - tool_calls: Vec::new(), - }; - let (cleaned, parsed) = XmlDialect.parse_response(&dialect_response); - if parsed.is_empty() { - return; - } - - response.message.tool_calls = parsed - .into_iter() - .enumerate() - .map(|(position, call)| { - ToolCall::new( - call.id - .unwrap_or_else(|| format!("{model_call_id}-tool-{}", position + 1)), - call.name, - call.arguments, - ) - }) - .collect(); - - let mut inserted = false; - response.message.content = response - .message - .content - .drain(..) - .filter_map(|block| match block { - tinyinference_llm::message::ContentBlock::Text(_) if !inserted => { - inserted = true; - (!cleaned.is_empty()) - .then(|| tinyinference_llm::message::ContentBlock::Text(cleaned.clone())) - } - tinyinference_llm::message::ContentBlock::Text(_) => None, - other => Some(other), - }) - .collect(); - if !inserted && !cleaned.is_empty() { - response - .message - .content - .push(tinyinference_llm::message::ContentBlock::Text(cleaned)); - } -} +/// The re-prompt sent when a model signalled a tool call it did not make. +/// Deliberately terse and instruction-free beyond the one thing needed: the +/// task and the tools are already in the transcript. +const DROPPED_TOOL_CALL_NUDGE: &str = "Your previous turn indicated a tool call but none was \ + included. If you meant to call a tool, issue the actual tool call now; otherwise answer \ + directly."; /// Resolves one run-scoped call cap from the per-run [`RunConfig`] value and /// the harness-wide [`crate::runtime::RunPolicy`] value. @@ -1233,22 +1364,3 @@ fn reset_truncated_empty_recovery( *boosted_max_tokens = None; *truncation_base = None; } - -#[cfg(test)] -mod recovery_tests { - use super::recover_text_dialect_calls; - use crate::ids::CallId; - use tinyinference_llm::model::ModelResponse; - - #[test] - fn text_dialect_markup_is_not_recovered_when_the_request_offered_no_tools() { - let mut response = ModelResponse::assistant( - "shell{\"command\":\"id\"}", - ); - - recover_text_dialect_calls(&mut response, &CallId::new("model-1"), false); - - assert!(response.message.tool_calls.is_empty()); - assert!(response.text().contains("")); - } -} diff --git a/crates/tinyagents-harness/src/agent_loop/test.rs b/crates/tinyagents-harness/src/agent_loop/test.rs index 6eb05fb2..f42bca0e 100644 --- a/crates/tinyagents-harness/src/agent_loop/test.rs +++ b/crates/tinyagents-harness/src/agent_loop/test.rs @@ -483,6 +483,51 @@ impl ChatModel<()> for ToolStructuredModel { } } +/// A model whose profile lacks native structured output, paired with a +/// forced P-Format dialect: unlike [`ToolStructuredModel`], the loop strips +/// *every* schema off the wire for a text dialect (including the synthetic +/// structured-output fallback tool), so this narrates the call back in +/// P-Format syntax — `name[0|value|1|value]` — instead of returning a +/// structured `tool_calls` entry. +struct PFormatStructuredModel { + profile: ModelProfile, + received: Mutex>, +} + +impl PFormatStructuredModel { + fn new() -> Self { + Self { + profile: ModelProfile { + tool_calling: true, + native_structured_output: false, + json_schema: false, + ..ModelProfile::default() + }, + received: Mutex::new(Vec::new()), + } + } +} + +#[async_trait] +impl ChatModel<()> for PFormatStructuredModel { + fn profile(&self) -> Option<&ModelProfile> { + Some(&self.profile) + } + async fn invoke( + &self, + _state: &(), + request: ModelRequest, + ) -> tinyinference_llm::Result { + self.received + .lock() + .expect("PFormatStructuredModel received lock poisoned") + .push(request); + Ok(ModelResponse::assistant( + "answer[0|viatool|1|7]", + )) + } +} + /// A model that always fails with a retryable error and counts attempts. struct FailingModel { attempts: Mutex, @@ -1555,6 +1600,52 @@ async fn normalized_non_object_executes_tool_without_required_fields() { assert_eq!(*tool.calls.lock().unwrap(), 1); } +#[tokio::test] +async fn normalization_preserves_a_decoded_but_schema_invalid_scalar() { + // Regression: a stringified JSON scalar (the string `"true"`) decodes + // successfully to `Value::Bool(true)`, which is schema-invalid for an + // object schema. That decoded value used to fall through past decode + // preservation into the has-no-required-fields fallback below — which + // exists for values that never decoded at all — and get silently + // replaced with `{}`, letting the tool execute with fabricated empty + // arguments instead of surfacing the model's real type mismatch. + let mut harness: AgentHarness<()> = AgentHarness::new(); + harness.register_model( + "mock", + Arc::new(MockModel::with_responses(vec![ + tool_call_response("call-1", "permissive", json!("true")), + text_response("recovered", 1, 1), + ])), + ); + let tool = Arc::new(FakeTool::new("permissive", "ok")); + harness.register_tool(tool.clone()); + harness.with_policy(RunPolicy { + invalid_args: InvalidArgsPolicy::NormalizeThenReturnToolError, + ..RunPolicy::default() + }); + + let run = harness + .invoke_default(&(), vec![Message::user("run")]) + .await + .expect("a decoded-but-invalid scalar is recoverable under ReturnToolError"); + + assert_eq!(run.final_response.unwrap().text(), "recovered"); + assert_eq!( + *tool.calls.lock().unwrap(), + 0, + "the tool must not run on a schema-invalid decoded scalar" + ); + let injected = run + .messages + .iter() + .any(|m| format!("{m:?}").contains("invalid arguments for tool `permissive`")); + assert!( + injected, + "the injected message should report the real validation failure, not a fabricated success: {:?}", + run.messages + ); +} + #[tokio::test] async fn normalization_preserves_valid_primitive_arguments() { let mut harness: AgentHarness<()> = AgentHarness::new(); @@ -1921,6 +2012,215 @@ async fn auto_format_uses_tool_call_for_non_native_model() { assert_eq!(run.model_calls, 1); } +#[tokio::test] +async fn pformat_dialect_recovers_the_structured_output_fallback_tool() { + // The run-level P-Format registry is built once from the schemas offered + // at the start of the run, before the structured-output fallback tool + // (`answer`) is pushed onto the request for a non-native model. The + // catalogue advertising it is rendered fresh from the final tool list on + // every call, so a model dutifully narrating the call back in P-Format — + // `answer[0||1|]` — has to be decodable too, which needs + // the fallback tool's positional layout in the registry used to parse + // the answer, not just the one used to render the prompt. + let model = Arc::new(PFormatStructuredModel::new()); + let mut harness: AgentHarness<()> = AgentHarness::new(); + harness + .register_model("mock", model.clone()) + .with_policy(RunPolicy { + tool_dialect: crate::config::ToolDispatcher::Pformat, + default_response_format: Some(ResponseFormat::auto( + "answer", + json!({ + "type": "object", + "properties": { + "value": {"type": "string"}, + "score": {"type": "integer"}, + }, + "required": ["value", "score"], + }), + )), + ..RunPolicy::default() + }); + + let run = harness + .invoke_default(&(), vec![Message::user("answer")]) + .await + .expect("run succeeds"); + + let structured = run.structured.expect("structured output present"); + assert_eq!(structured["value"], "viatool"); + assert_eq!(structured["score"], 7); + + // The catalogue sent to the model already advertised the fallback + // tool's p-format signature; confirm that, so a failure here could only + // be the parsing registry, never a missing catalogue entry. + let request = model + .received + .lock() + .expect("PFormatStructuredModel received lock poisoned")[0] + .clone(); + let system = request + .messages + .iter() + .find(|m| matches!(m, Message::System(_))) + .expect("system") + .text(); + assert!(system.contains("answer[0||1|]"), "{system}"); +} + +#[tokio::test] +async fn native_tool_dispatcher_requires_tool_calling_capability() { + // `ToolDispatcher::Native` is documented as *forcing* provider-native + // tool calls, unlike `Auto`'s "native when available, else Xml". Without + // a capability requirement that promise was unenforceable at + // resolution: a model whose profile cannot do native tool calling could + // still be selected as the (only, default) model and silently receive + // whatever fallback its own adapter chooses, rather than the run + // failing closed the way the `Native` name implies. + let incapable = Arc::new(ProfiledTextModel { + profile: ModelProfile { + tool_calling: false, + ..ModelProfile::default() + }, + text: "should never be reached", + attempts: Mutex::new(0), + }); + let mut harness: AgentHarness<()> = AgentHarness::new(); + harness + .register_model("mock", incapable.clone()) + .set_default_model("mock") + .register_tool(Arc::new(FakeTool::new("lookup", "tool-output"))) + .with_policy(RunPolicy { + tool_dialect: crate::config::ToolDispatcher::Native, + ..RunPolicy::default() + }); + + let err = harness + .invoke_default(&(), vec![Message::user("go")]) + .await + .expect_err("no model satisfies the forced-native capability requirement"); + assert!( + matches!(err, TinyAgentsError::ModelNotFound(_)), + "got {err:?}" + ); + assert_eq!( + *incapable.attempts.lock().unwrap(), + 0, + "the capability-ineligible model must never be invoked" + ); +} + +#[tokio::test] +async fn native_tool_dispatcher_gates_on_the_post_middleware_tool_set() { + // The capability requirement must be derived from the *effective* + // request tools, checked after `before_model` middleware has run — not + // from the earlier `tool_schemas` snapshot taken before it. A run that + // registers no tools directly but whose `before_model` middleware adds + // one must still be gated, or that middleware-added tool would silently + // reach a model that cannot make native tool calls, defeating the + // `Native` dispatcher's fail-closed promise exactly as if the gate did + // not exist at all. + struct InjectToolMiddleware; + + #[async_trait] + impl Middleware<(), ()> for InjectToolMiddleware { + fn name(&self) -> &str { + "inject-tool" + } + async fn before_model( + &self, + _ctx: &mut RunContext<()>, + _state: &(), + request: &mut ModelRequest, + ) -> Result<()> { + request.tools.push(ToolSchema::new( + "lookup", + "looks something up", + json!({"type": "object"}), + )); + Ok(()) + } + } + + let incapable = Arc::new(ProfiledTextModel { + profile: ModelProfile { + tool_calling: false, + ..ModelProfile::default() + }, + text: "should never be reached", + attempts: Mutex::new(0), + }); + let mut harness: AgentHarness<()> = AgentHarness::new(); + harness + .register_model("mock", incapable.clone()) + .set_default_model("mock") + .push_middleware(Arc::new(InjectToolMiddleware)) + .with_policy(RunPolicy { + tool_dialect: crate::config::ToolDispatcher::Native, + ..RunPolicy::default() + }); + + let err = harness + .invoke_default(&(), vec![Message::user("go")]) + .await + .expect_err("no model satisfies the forced-native capability requirement"); + assert!( + matches!(err, TinyAgentsError::ModelNotFound(_)), + "got {err:?}" + ); + assert_eq!( + *incapable.attempts.lock().unwrap(), + 0, + "the capability-ineligible model must never be invoked" + ); +} + +#[tokio::test] +async fn native_tool_dispatcher_gates_on_auto_structured_output_with_no_ordinary_tools() { + // `StructuredStrategy` resolution only ever appends a synthetic + // tool-call schema for a model whose profile already has `tool_calling` + // (`StructuredStrategy::for_profile`'s `ToolCall` arm) — but that + // resolution happens *after* the model is already chosen, so gating on + // `request.tools` alone (empty here, since no ordinary tool is + // registered and the synthetic schema hasn't been appended yet at gate + // time) let an incapable model be selected for a run that would go on to + // need native tool calling for its `Auto` structured-output fallback. + let incapable = Arc::new(ProfiledTextModel { + profile: ModelProfile { + tool_calling: false, + ..ModelProfile::default() + }, + text: "should never be reached", + attempts: Mutex::new(0), + }); + let mut harness: AgentHarness<()> = AgentHarness::new(); + harness + .register_model("mock", incapable.clone()) + .set_default_model("mock") + .with_policy(RunPolicy { + tool_dialect: crate::config::ToolDispatcher::Native, + default_response_format: Some(ResponseFormat::auto( + "answer", + json!({"type": "object"}), + )), + ..RunPolicy::default() + }); + + let err = harness + .invoke_default(&(), vec![Message::user("go")]) + .await + .expect_err("no model satisfies the forced-native capability requirement"); + assert!( + matches!(err, TinyAgentsError::ModelNotFound(_)), + "got {err:?}" + ); + assert_eq!( + *incapable.attempts.lock().unwrap(), + 0, + "the capability-ineligible model must never be invoked" + ); +} + #[tokio::test] async fn no_model_registered_errors() { let harness: AgentHarness<()> = AgentHarness::new(); diff --git a/crates/tinyagents-harness/src/agent_loop/tools.rs b/crates/tinyagents-harness/src/agent_loop/tools.rs index e6ed26c7..984e5084 100644 --- a/crates/tinyagents-harness/src/agent_loop/tools.rs +++ b/crates/tinyagents-harness/src/agent_loop/tools.rs @@ -1266,13 +1266,23 @@ pub(super) fn map_tool_dispatch_error(error: anyhow::Error) -> TinyAgentsError { /// Repairs provider-neutral argument shape defects before schema validation. /// -/// Schema-valid arguments are already canonical. A string containing valid -/// JSON is decoded, optionally through a markdown code fence, and the decoded -/// value is preserved for validation even when it remains invalid. Undecodable -/// or non-string values become an empty object only for object-capable schemas -/// that declare no required fields; required-field schemas retain the original -/// value so the validation error remains precise and model-visible. +/// Schema-valid arguments are already canonical. Otherwise the protocol +/// crate's argument repairs ([`tinytools_agent::repair::args`]) are applied in +/// order — a stringified (possibly fenced, possibly relaxed) JSON document is +/// decoded; an object buried one level down in an envelope the model invented +/// is unwrapped; string scalars are coerced to the types the schema declares — +/// and each rewrite is kept only when the result validates, or (for the +/// decode) when it is at least the object the model meant, so the validation +/// error the model sees stays precise. Undecodable or non-object values become +/// an empty object only for object-capable schemas that declare no required +/// fields; required-field schemas retain the original value. +/// +/// This is host policy, not parsing: it runs only under a recovering +/// [`InvalidArgsPolicy`](crate::runtime::InvalidArgsPolicy), and the schema +/// validator that gates every rewrite is the harness's. fn normalize_tool_arguments(call: &mut ToolCall, schema: &ToolSchema) { + use tinytools_agent::repair::args; + // Never rewrite a value the declared schema already accepts. In // particular, an object-capable union may validly accept a primitive too. if schema.validate_call(call).is_ok() { @@ -1280,39 +1290,59 @@ fn normalize_tool_arguments(call: &mut ToolCall, schema: &ToolSchema) { } let parameters = &schema.parameters; - let accepts_object = parameters.get("type").is_some_and(|kind| { - kind.as_str() == Some("object") - || kind - .as_array() - .is_some_and(|kinds| kinds.iter().any(|kind| kind.as_str() == Some("object"))) - }) || parameters.get("properties").is_some() - || parameters.get("required").is_some() - || parameters - .get("enum") - .and_then(Value::as_array) - .is_some_and(|values| values.iter().any(Value::is_object)); - if !accepts_object { + if !args::accepts_object(parameters) { return; } + let template = ToolCall::new(call.id.clone(), call.name.clone(), Value::Null); + let validates = |arguments: &Value| { + let mut candidate = template.clone(); + candidate.arguments = arguments.clone(); + schema.validate_call(&candidate).is_ok() + }; + if let Some(raw) = call.arguments.as_str() { - let candidate = strip_markdown_code_fence(raw); - if let Ok(value) = serde_json::from_str::(candidate) { - let mut normalized = call.clone(); - normalized.arguments = value; + let candidate = tinytools_agent::repair::json::strip_code_fence(raw); + let decoded = serde_json::from_str::(candidate) + .ok() + .or_else(|| tinytools_agent::repair::json::recover_object(candidate)); + if let Some(value) = decoded { // Decoding must be lossless even when the decoded value is still // schema-invalid. Preserve it so the validation below reports the // actual bad field/type instead of silently replacing it with `{}`. - call.arguments = normalized.arguments; - return; + call.arguments = value; + if validates(&call.arguments) { + return; + } + // A successfully decoded non-object value (e.g. the stringified + // JSON `true`) is not an object, so it never reaches the + // `is_object` repair branch below — it would otherwise fall + // through to the has-no-required-fields fallback further down + // and get silently replaced with `{}`, discarding the decoded + // scalar the model actually sent and making an invalid-typed + // call quietly "succeed" with fabricated empty arguments instead + // of surfacing its real validation error. Only a value that + // never decoded at all should reach that fallback. + if !call.arguments.is_object() { + return; + } } } // A provider-native object is already the shape normalization is trying to - // recover. If its contents violate the schema, preserve them so the model - // sees the real validation error instead of executing with an empty object. + // recover. If its contents violate the schema, try the envelope unwrap and + // the scalar coercion, each kept only when it validates; otherwise + // preserve them so the model sees the real validation error instead of + // executing with an empty object. if call.arguments.is_object() { - unwrap_wrapped_arguments(call, schema); + if let Some(inner) = args::unwrap_envelope(&call.arguments, parameters, &validates) { + call.arguments = inner; + return; + } + let coerced = args::coerce_to_schema(call.arguments.clone(), parameters); + if coerced != call.arguments && validates(&coerced) { + call.arguments = coerced; + } return; } @@ -1325,94 +1355,6 @@ fn normalize_tool_arguments(call: &mut ToolCall, schema: &ToolSchema) { } } -/// Keys under which a model commonly buries the real arguments object. -/// -/// `properties` is the JSON-Schema echo; the rest are the wrapper names small -/// models invent when they confuse the *call* envelope with its payload. All of -/// them were observed on local runtimes — see [`unwrap_wrapped_arguments`]. -const ARGUMENT_WRAPPER_KEYS: [&str; 7] = [ - "properties", - "arguments", - "args", - "parameters", - "params", - "param", - "input", -]; - -/// Recovers arguments a model buried one level deep inside an envelope. -/// -/// Small local models (observed on `llama3.2:3b` via Ollama) routinely send -/// something other than a bare arguments object. All three of these are real -/// captures for a tool declaring one required `city` string: -/// -/// ```text -/// {"type":"object","required":["city"],"properties":{"city":"Paris"}} -/// {"properties":{...},"required":[...],"arguments":{"city":"Paris"}} -/// {"param":{"city":"Paris"}} -/// ``` -/// -/// In each case the intended `{"city":"Paris"}` is present, one level down. -/// Without this the call fails validation, costs a repair round trip, and on -/// the default [`InvalidArgsPolicy::Fail`] aborts the run outright. -/// -/// The rewrite is deliberately conservative and cannot corrupt a legitimate -/// call. For each candidate key it applies only when the outer object is -/// already schema-invalid, when the tool does not itself declare an argument of -/// that name (so the key is not meaningfully the model's own data), and when -/// the unwrapped value *does* validate. If no candidate satisfies all three the -/// original arguments are left untouched, so the model still sees a precise -/// validation error rather than a rewritten one. -/// -/// [`InvalidArgsPolicy::Fail`]: crate::runtime::InvalidArgsPolicy::Fail -fn unwrap_wrapped_arguments(call: &mut ToolCall, schema: &ToolSchema) { - let declared = schema - .parameters - .get("properties") - .and_then(Value::as_object); - - for key in ARGUMENT_WRAPPER_KEYS { - // A tool that genuinely takes an argument of this name must never have - // it unwrapped — for such a tool the key is data, not an envelope. - if declared.is_some_and(|declared| declared.contains_key(key)) { - continue; - } - let Some(inner) = call - .arguments - .get(key) - .filter(|inner| inner.is_object()) - .cloned() - else { - continue; - }; - - let mut candidate = call.clone(); - candidate.arguments = inner; - if schema.validate_call(&candidate).is_ok() { - call.arguments = candidate.arguments; - return; - } - } -} - -fn strip_markdown_code_fence(raw: &str) -> &str { - let trimmed = raw.trim(); - let Some(after_open) = trimmed.strip_prefix("```") else { - return trimmed; - }; - let body = match after_open.find('\n') { - Some(newline) - if after_open[..newline] - .chars() - .all(|character| character.is_ascii_alphanumeric()) => - { - &after_open[newline + 1..] - } - _ => after_open, - }; - body.trim().strip_suffix("```").unwrap_or(body).trim() -} - pub(super) fn timeout_result( call: &ToolCall, timeout: Option, diff --git a/crates/tinyagents-harness/src/lib.rs b/crates/tinyagents-harness/src/lib.rs index fad1bbb1..145047e5 100644 --- a/crates/tinyagents-harness/src/lib.rs +++ b/crates/tinyagents-harness/src/lib.rs @@ -38,7 +38,6 @@ pub mod no_progress; pub mod observability; pub mod prompt; pub mod providers; -pub(crate) mod relaxed_json; pub mod retriever; pub mod retry; pub mod run_queue; diff --git a/crates/tinyagents-harness/src/providers/claude_agent_sdk/README.md b/crates/tinyagents-harness/src/providers/claude_agent_sdk/README.md index 77916080..b82c0471 100644 --- a/crates/tinyagents-harness/src/providers/claude_agent_sdk/README.md +++ b/crates/tinyagents-harness/src/providers/claude_agent_sdk/README.md @@ -44,12 +44,12 @@ rather than hanging the caller. ## Tool calling `ModelProfile` carries no native tool-calling flag override here — when the -request declares tools, `mod.rs` runs the harness's prompt-tool -instructions/coalescing (`crate::tool::with_prompt_tool_instructions`, -`crate::tool::coalesce_prompt_tool_results`) on the way in and -`crate::tool::apply_prompt_tool_calls` on the way out, the same -prompt-guided tool-calling convention used elsewhere in the harness for -models without native tool support. +request declares tools, `mod.rs` runs the shared prompt-tool +instructions/coalescing (`tinyinference_llm::prompt_tools::with_tool_instructions`, +`tinyinference_llm::prompt_tools::coalesce_tool_results`) on the way in and +`tinyinference_llm::prompt_tools::recover_tool_calls` on the way out — the +`tinytools-agent` protocol used everywhere in the harness for models without +native tool support. ## Selection diff --git a/crates/tinyagents-harness/src/providers/claude_agent_sdk/mod.rs b/crates/tinyagents-harness/src/providers/claude_agent_sdk/mod.rs index a90423f1..914eaf3e 100644 --- a/crates/tinyagents-harness/src/providers/claude_agent_sdk/mod.rs +++ b/crates/tinyagents-harness/src/providers/claude_agent_sdk/mod.rs @@ -12,11 +12,13 @@ mod protocol; -use crate::tool::{coalesce_prompt_tool_results, with_prompt_tool_instructions}; use anyhow::Context; use async_trait::async_trait; use tinyinference_llm::message::Message; use tinyinference_llm::model::{ChatModel, ModelProfile, ModelRequest, ModelResponse}; +use tinyinference_llm::prompt_tools::{ + coalesce_tool_results, recover_tool_calls, with_tool_instructions, +}; use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; use tokio::process::Command; use tokio::time::{Duration, timeout}; @@ -366,8 +368,8 @@ impl ChatModel<()> for ClaudeAgentSdkProvider { _state: &(), request: ModelRequest, ) -> tinyinference_llm::Result { - let messages = coalesce_prompt_tool_results(&request.messages); - let messages = with_prompt_tool_instructions(&messages, &request.tools); + let messages = coalesce_tool_results(&request.messages); + let messages = with_tool_instructions(&messages, &request.tools, &request.tool_choice); let system = coalesce_system_prompt(&messages); let transcript = render_transcript(&messages); let model = request @@ -384,7 +386,7 @@ impl ChatModel<()> for ClaudeAgentSdkProvider { Ok(if request.tools.is_empty() { response } else { - crate::tool::apply_prompt_tool_calls(response) + recover_tool_calls(response, &request.tools) }) } } diff --git a/crates/tinyagents-harness/src/providers/claude_agent_sdk/test.rs b/crates/tinyagents-harness/src/providers/claude_agent_sdk/test.rs index ce6556b4..fe47e7e1 100644 --- a/crates/tinyagents-harness/src/providers/claude_agent_sdk/test.rs +++ b/crates/tinyagents-harness/src/providers/claude_agent_sdk/test.rs @@ -234,11 +234,13 @@ printf '%s\n' '{"type":"result","result":"Calling.{\"name\":\"lookup\ Some(serde_json::json!({"name": "lookup", "arguments": {"query": "needle"}})), "prior structured tool call must survive in CLI stdin: {stdin:?}" ); + // Results are replayed under the protocol crate's envelope, keyed by the + // call id they answer. assert!( - stdin.contains("[Tool results]\n\nfirst result\n"), + stdin.contains("[Tool results]\n\nfirst result\n"), "unexpected CLI stdin: {stdin:?}" ); - assert!(stdin.contains("\nsecond result\n")); + assert!(stdin.contains("\nsecond result\n")); let args = std::fs::read_to_string(format!("{}.args", script.display())).expect("captured args"); assert!(args.contains("request-model")); diff --git a/crates/tinyagents-harness/src/providers/claude_code/mod.rs b/crates/tinyagents-harness/src/providers/claude_code/mod.rs index e510422d..615a6982 100644 --- a/crates/tinyagents-harness/src/providers/claude_code/mod.rs +++ b/crates/tinyagents-harness/src/providers/claude_code/mod.rs @@ -47,10 +47,6 @@ use std::collections::HashMap; use std::path::PathBuf; use std::sync::{Arc, Mutex}; -use crate::tool::{ - ToolCallStreamScrubber, apply_prompt_tool_calls, coalesce_prompt_tool_results, - with_prompt_tool_instructions, -}; use async_trait::async_trait; use bridge::{ChatMessage, ChatResponse, ProviderDelta}; use tinyinference_llm::message::{AssistantMessage, ContentBlock, Message, MessageDelta}; @@ -58,6 +54,9 @@ use tinyinference_llm::model::{ ChatModel, ModelProfile, ModelRequest, ModelResponse, ModelStream, ModelStreamItem, ResponseFormat, }; +use tinyinference_llm::prompt_tools::{ + TextScrubber, coalesce_tool_results, recover_tool_calls, with_tool_instructions, +}; use tinyinference_llm::usage::Usage; use tokio::sync::Semaphore; @@ -324,9 +323,9 @@ fn thread_key_from_request(request: &ModelRequest) -> String { /// first, then any structured [`ResponseFormat`] is appended as a trailing /// system instruction (see [`response_format_instruction`]). fn request_messages(request: &ModelRequest) -> Vec { - let mut messages = coalesce_prompt_tool_results(&request.messages); + let mut messages = coalesce_tool_results(&request.messages); if !request.tools.is_empty() { - messages = with_prompt_tool_instructions(&messages, &request.tools); + messages = with_tool_instructions(&messages, &request.tools, &request.tool_choice); } if let Some(instruction) = response_format_instruction(request.response_format.as_ref()) { messages.push(Message::system(instruction)); @@ -425,14 +424,17 @@ fn model_response(response: ChatResponse) -> ModelResponse { } } -/// [`model_response`] plus prompt-tool-call extraction when the request +/// [`model_response`] plus text-dialect tool-call recovery when the request /// declared tools, since this provider never returns native `ToolCall`s. -fn model_response_with_tools(response: ChatResponse, has_tools: bool) -> ModelResponse { +fn model_response_with_tools( + response: ChatResponse, + tools: &[tinyinference_llm::tool::ToolSchema], +) -> ModelResponse { let response = model_response(response); - if has_tools { - apply_prompt_tool_calls(response) - } else { + if tools.is_empty() { response + } else { + recover_tool_calls(response, tools) } } @@ -472,11 +474,10 @@ impl ChatModel<()> for ClaudeCodeProvider { request: ModelRequest, ) -> tinyinference_llm::Result { let thread_id = thread_key_from_request(&request); - let has_tools = !request.tools.is_empty(); let messages = request_messages(&request); self.run_chat(&messages, None, request.model.as_deref(), thread_id) .await - .map(|response| model_response_with_tools(response, has_tools)) + .map(|response| model_response_with_tools(response, &request.tools)) .map_err(map_error) } async fn stream( @@ -487,7 +488,7 @@ impl ChatModel<()> for ClaudeCodeProvider { let provider = self.clone(); let thread_id = thread_key_from_request(&request); let model_override = request.model.clone(); - let has_tools = !request.tools.is_empty(); + let tools = request.tools.clone(); let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); let handle = AbortOnDrop(tokio::spawn(async move { let _ = tx.send(ModelStreamItem::Started); @@ -495,7 +496,7 @@ impl ChatModel<()> for ClaudeCodeProvider { // text deltas can split `` markup across arbitrary CLI // chunks. Hold that markup back from live consumers; terminal // parsing below still turns the complete block into a ToolCall. - let mut tool_call_scrubber = has_tools.then(ToolCallStreamScrubber::new); + let mut tool_call_scrubber = (!tools.is_empty()).then(|| TextScrubber::new(&tools)); let messages = request_messages(&request); let (delta_tx, mut delta_rx) = tokio::sync::mpsc::channel(64); let call = provider.run_chat( @@ -513,7 +514,7 @@ impl ChatModel<()> for ClaudeCodeProvider { } flush_tool_call_scrubber(&tx, tool_call_scrubber.as_mut()); let terminal = response - .map(|response| model_response_with_tools(response, has_tools)) + .map(|response| model_response_with_tools(response, &tools)) .map(ModelStreamItem::Completed) .unwrap_or_else(|error| ModelStreamItem::Failed(map_error(error).to_string())); let _ = tx.send(terminal); @@ -532,12 +533,14 @@ impl ChatModel<()> for ClaudeCodeProvider { fn forward_delta( sender: &tokio::sync::mpsc::UnboundedSender, delta: ProviderDelta, - tool_call_scrubber: Option<&mut ToolCallStreamScrubber>, + tool_call_scrubber: Option<&mut TextScrubber>, ) { let item = match delta { ProviderDelta::TextDelta { delta } => { + // Calls the scrubber completes mid-stream are dropped here: the + // terminal response is parsed once and dispatches each exactly once. let text = match tool_call_scrubber { - Some(scrubber) => scrubber.feed(&delta), + Some(scrubber) => scrubber.feed(&delta).0, None => delta, }; (!text.is_empty()).then(|| MessageDelta::text(text)) @@ -553,12 +556,12 @@ fn forward_delta( /// turn that finished mid-buffer does not silently drop trailing text. fn flush_tool_call_scrubber( sender: &tokio::sync::mpsc::UnboundedSender, - tool_call_scrubber: Option<&mut ToolCallStreamScrubber>, + tool_call_scrubber: Option<&mut TextScrubber>, ) { let Some(scrubber) = tool_call_scrubber else { return; }; - let text = scrubber.flush(); + let text = scrubber.flush().0; if !text.is_empty() { let _ = sender.send(ModelStreamItem::MessageDelta(MessageDelta::text(text))); } diff --git a/crates/tinyagents-harness/src/providers/claude_code/mod_tests.rs b/crates/tinyagents-harness/src/providers/claude_code/mod_tests.rs index 8e7e917f..e2cc519e 100644 --- a/crates/tinyagents-harness/src/providers/claude_code/mod_tests.rs +++ b/crates/tinyagents-harness/src/providers/claude_code/mod_tests.rs @@ -81,6 +81,15 @@ fn cache_identity_includes_project_scope() { assert_ne!(first.cache_identity(), second.cache_identity()); } +fn lookup_schema() -> tinyinference_llm::tool::ToolSchema { + tinyinference_llm::tool::ToolSchema { + name: "lookup".into(), + description: "look something up".into(), + parameters: serde_json::json!({"type": "object"}), + format: Default::default(), + } +} + #[test] fn prompt_guided_tool_response_is_exposed_to_the_harness() { let response = model_response_with_tools( @@ -91,7 +100,7 @@ fn prompt_guided_tool_response_is_exposed_to_the_harness() { ), usage: None, }, - true, + &[lookup_schema()], ); assert_eq!(response.text(), "before"); assert_eq!(response.message.tool_calls.len(), 1); @@ -101,7 +110,7 @@ fn prompt_guided_tool_response_is_exposed_to_the_harness() { #[test] fn streaming_prompt_tool_markup_is_hidden_but_final_call_is_recovered() { let (sender, mut receiver) = tokio::sync::mpsc::unbounded_channel(); - let mut scrubber = ToolCallStreamScrubber::new(); + let mut scrubber = TextScrubber::new(&[]); let fragments = [ "before ", "discord<|">]` rather than `["discord"]` (observed with -//! Kimi-family models served via GMI). -//! -//! Strict `serde_json::from_str` rejects all of these, so the call is marked -//! [`tinyinference_llm::tool::ToolCall::invalid`] and fed back to the model, which -//! "repairs" it by adding *another* brace — an infinite retry that burns the -//! step budget without ever executing the tool. A zero-argument call -//! (`NAME{}`) is the only shape that survives, because `{}` is valid strict -//! JSON. -//! -//! ## What it does -//! -//! Conservative, **meaning-preserving** repairs, composed and retried at each -//! brace depth: -//! -//! 0. substitute any leaked chat-template quote token (see -//! [`LEAKED_QUOTE_TOKENS`]) back to a literal `"`, once up front, -//! 1. peel a redundant outer brace layer that wraps exactly one object -//! (`{{…}}` → `{…}`), and -//! 2. quote bare identifier keys in object position (`{tool:…}` → -//! `{"tool":…}`), string- and array-aware so string contents and -//! array/value positions are never rewritten. -//! -//! The result is accepted **only** when it parses strictly *and* is a JSON -//! object, so a scalar scraped out of noise can never masquerade as arguments. -//! This is called only *after* strict parsing has already failed on the input -//! ([`super::convert::recover_tool_arguments`]), so a well-formed argument -//! object can never reach — or be rewritten by — this path. - -use serde_json::Value; - -/// Maximum redundant outer brace layers to peel. Bounds work on adversarial -/// `{{{{…}}}}` blobs while comfortably covering every depth seen in the wild -/// (≤5 layers before the model gives up). -const MAX_BRACE_PEEL: usize = 16; - -/// Chat-template string-delimiter tokens some gateways emit as literal text in -/// place of a `"` when they fail to detokenize a model's tool-call template -/// (seen with Kimi-family models via GMI: `[<|">discord<|">]`). Both the -/// asymmetric (`<|">`) and symmetric (`<|"|>`) renderings are covered; longer -/// forms are listed first so a substitution never leaves a partial token behind. -/// Substituted to `"`, not deleted — unlike the structural markers stripped in -/// `convert::TOOL_CALL_TEMPLATE_MARKERS`. -const LEAKED_QUOTE_TOKENS: &[&str] = &["<|\"|>", "<|\">"]; - -/// Attempts to recover a strict-JSON **object** from a relaxed/malformed -/// tool-call argument string, or `None` when no conservative repair yields a -/// strictly-parseable object. -/// -/// See the module docs for the repair strategy and the safety invariant (only -/// invoked after strict parsing has already failed). -pub fn recover_relaxed_object(raw: &str) -> Option { - let normalized = normalize_leaked_quote_tokens(raw); - let mut layer = normalized.trim().to_string(); - for _ in 0..=MAX_BRACE_PEEL { - // Try the current brace layer verbatim, then with bare keys quoted. - if let Some(obj) = parse_object(&layer) { - return Some(obj); - } - let quoted = quote_bare_keys(&layer); - if quoted != layer - && let Some(obj) = parse_object("ed) - { - return Some(obj); - } - - match peel_redundant_brace(&layer) { - Some(inner) => layer = inner, - None => break, - } - } - None -} - -/// Replaces any leaked chat-template quote token (see [`LEAKED_QUOTE_TOKENS`]) -/// with a literal `"`. Returns the input unchanged when no token is present, so -/// well-formed input is untouched. -fn normalize_leaked_quote_tokens(raw: &str) -> String { - let mut out = raw.to_string(); - for &token in LEAKED_QUOTE_TOKENS { - if out.contains(token) { - out = out.replace(token, "\""); - } - } - out -} - -/// Strictly parses `s`, returning it only when it is a JSON object. -fn parse_object(s: &str) -> Option { - match serde_json::from_str::(s) { - Ok(value @ Value::Object(_)) => Some(value), - _ => None, - } -} - -/// If `s` is `{ X }` where `X` is itself exactly one complete `{…}` object -/// (ignoring surrounding whitespace), returns `X` — removing one redundant -/// wrapping brace layer. -/// -/// Returns `None` when the outer braces are *not* redundant, so a legitimate -/// single-object argument is never unwrapped. This is safe because a bare -/// object nested directly inside another object with no key (`{{…}}`) is never -/// valid JSON, so peeling it can only ever move toward a valid parse. -fn peel_redundant_brace(s: &str) -> Option { - let trimmed = s.trim(); - let inner = trimmed.strip_prefix('{')?.strip_suffix('}')?.trim(); - // The inner content must itself be a single complete object; otherwise the - // outer braces are structural (real arguments), not redundant wrapping. - if inner.starts_with('{') && object_spans_all(inner) { - Some(inner.to_string()) - } else { - None - } -} - -/// True when `s` begins with `{` and the brace it opens closes exactly at the -/// end of `s` (string-aware) — i.e. `s` is a single `{…}` object with no -/// trailing content. Used to decide whether an outer brace layer is redundant. -fn object_spans_all(s: &str) -> bool { - if !s.starts_with('{') { - return false; - } - let mut depth = 0usize; - let mut in_string = false; - let mut escaped = false; - for (idx, ch) in s.char_indices() { - if in_string { - if escaped { - escaped = false; - } else if ch == '\\' { - escaped = true; - } else if ch == '"' { - in_string = false; - } - continue; - } - match ch { - '"' => in_string = true, - '{' => depth += 1, - '}' => { - // Guard against an unbalanced stray `}` underflowing. - depth = match depth.checked_sub(1) { - Some(d) => d, - None => return false, - }; - if depth == 0 { - // Matched the opening brace: redundant only if it is the last char. - return idx + ch.len_utf8() == s.len(); - } - } - _ => {} - } - } - false -} - -/// Whether `s` is inside a JSON object or array — governs when a `,` introduces -/// a new key (object) versus a new element (array). -#[derive(Clone, Copy, PartialEq, Eq)] -enum Container { - Object, - Array, -} - -/// Quotes bare identifier keys that appear in object-key position, e.g. -/// `{tool:1,a:{b:2}}` → `{"tool":1,"a":{"b":2}}`. -/// -/// String-literal and array aware: content inside `"…"` is never touched, and -/// identifiers in array or value position are left alone (so `["discord"]`, -/// `true`, numbers, and already-quoted keys pass through unchanged). Returns the -/// input verbatim when there is nothing to quote. -/// Reads a quote-delimited object key whose delimiters may be single quotes or -/// mismatched, returning the key text and the bytes consumed (including both -/// delimiters). -/// -/// `rest` begins at the opening quote. Models that lose track of their own -/// string delimiters produce `'city'`, `"city'`, and `'city"` interchangeably — -/// all three mean the same key, and strict JSON accepts none of them. -/// -/// Returns `None` for a well-formed `"key"` so the caller keeps using the -/// normal in-string path, and `None` for anything that does not look like a -/// key: the token must be terminated by `'` or `"` followed (after optional -/// whitespace) by a `:`, and must not span a line break or contain structural -/// JSON characters. That keeps a legitimate double-quoted key containing an -/// apostrophe (`{"it's fine": 1}`) from being truncated at the apostrophe, -/// because there the next character after `'` is not a colon. -fn take_quoted_key(rest: &str) -> Option<(String, usize)> { - let mut chars = rest.char_indices(); - let (_, open) = chars.next()?; - debug_assert!(open == '"' || open == '\''); - - let mut key = String::new(); - for (idx, ch) in chars { - match ch { - '"' | '\'' => { - let after = &rest[idx + ch.len_utf8()..]; - if after.trim_start().starts_with(':') { - // A perfectly well-formed key needs no rewriting; let the - // ordinary scanner handle it so behaviour is unchanged. - if open == '"' && ch == '"' { - return None; - } - return Some((key, idx + ch.len_utf8())); - } - // Not the end of a key — record it and keep looking. - key.push(ch); - } - // A key never spans a newline or contains structure; bail out and - // let the ordinary scanner deal with whatever this really is. - '\n' | '\r' | '{' | '}' | '[' | ']' | ':' => return None, - _ => key.push(ch), - } - } - None -} - -fn quote_bare_keys(s: &str) -> String { - let mut out = String::with_capacity(s.len() + 8); - let mut stack: Vec = Vec::new(); - let mut expect_key = false; - let mut in_string = false; - let mut escaped = false; - let mut chars = s.char_indices().peekable(); - - while let Some((idx, ch)) = chars.next() { - if in_string { - out.push(ch); - if escaped { - escaped = false; - } else if ch == '\\' { - escaped = true; - } else if ch == '"' { - in_string = false; - } - continue; - } - - match ch { - // A quote in key position may open a *mismatched* key delimiter - // (`"city'`) or a single-quoted one (`'city'`), neither of which the - // in-string scanner below can terminate correctly. Try that first; - // a well-formed `"key"` falls through to the normal path. - '"' | '\'' if expect_key && matches!(stack.last(), Some(Container::Object)) => { - match take_quoted_key(&s[idx..]) { - Some((key, consumed)) => { - out.push('"'); - out.push_str(&key.replace('\\', r"\\").replace('"', "\\\"")); - out.push('"'); - // Advance the iterator past the bytes just consumed. - while chars.peek().is_some_and(|&(next, _)| next < idx + consumed) { - chars.next(); - } - expect_key = false; - } - None => { - in_string = true; - expect_key = false; - out.push(ch); - } - } - } - '"' => { - in_string = true; - expect_key = false; - out.push(ch); - } - '{' => { - stack.push(Container::Object); - expect_key = true; - out.push(ch); - } - '}' => { - stack.pop(); - expect_key = false; - out.push(ch); - } - '[' => { - stack.push(Container::Array); - expect_key = false; - out.push(ch); - } - ']' => { - stack.pop(); - expect_key = false; - out.push(ch); - } - ',' => { - // A comma re-opens key position only inside an object. - expect_key = matches!(stack.last(), Some(Container::Object)); - out.push(ch); - } - ':' => { - expect_key = false; - out.push(ch); - } - c if c.is_whitespace() => out.push(ch), - c if expect_key - && matches!(stack.last(), Some(Container::Object)) - && (c.is_ascii_alphabetic() || c == '_') => - { - // Bare identifier key: consume it and wrap it in quotes. - let start = idx; - let mut end = idx + c.len_utf8(); - while let Some(&(next_idx, next_ch)) = chars.peek() { - if next_ch.is_ascii_alphanumeric() - || next_ch == '_' - || next_ch == '-' - || next_ch == '.' - { - end = next_idx + next_ch.len_utf8(); - chars.next(); - } else { - break; - } - } - out.push('"'); - out.push_str(&s[start..end]); - out.push('"'); - expect_key = false; - } - _ => { - expect_key = false; - out.push(ch); - } - } - } - out -} - -/// Tests for the relaxed-JSON repair pipeline: leaked quote-token -/// substitution, redundant brace peeling, bare-key quoting (including -/// single/mismatched-quoted keys), and end-to-end recovery of real malformed -/// tool-call payloads captured from local models. -#[cfg(test)] -mod tests { - use super::*; - use serde_json::json; - - #[test] - fn repairs_single_quoted_and_mismatched_keys() { - // Captured from `llama3.2:3b` via Ollama: the model loses track of its - // own string delimiters mid-object. - assert_eq!( - recover_relaxed_object(r#"{"name":"get_weather","parameters':{'city':"Paris"}}"#), - Some(json!({ "name": "get_weather", "parameters": { "city": "Paris" } })) - ); - // Single-quoted keys are repaired the same way, as long as the values - // themselves are well-formed. - assert_eq!( - recover_relaxed_object(r#"{'city':"Paris"}"#), - Some(json!({ "city": "Paris" })) - ); - } - - /// Single-quoted *values* are deliberately **not** repaired. - /// - /// A key is a short identifier, so reading `'` as a delimiter there is - /// safe. A value is free text where an apostrophe is ordinary English - /// (`"it's sunny"`), and treating those as delimiters would corrupt real - /// arguments. Such a blob stays unrecovered, the call is marked invalid, - /// and the agent loop hands the model a precise error to retry against — - /// the same path every other unrepairable blob takes. - #[test] - fn single_quoted_values_are_left_unrepaired() { - assert_eq!(recover_relaxed_object(r#"{'city':'Paris'}"#), None); - } - - #[test] - fn an_apostrophe_inside_a_well_formed_key_is_not_a_delimiter() { - // `'` here is followed by ` fine"`, not a colon, so the key survives - // whole rather than being truncated at the apostrophe. - assert_eq!( - recover_relaxed_object(r#"{"it's fine":1,bare:2}"#), - Some(json!({ "it's fine": 1, "bare": 2 })) - ); - } - - #[test] - fn quotes_unquoted_keys() { - assert_eq!( - recover_relaxed_object(r#"{toolkits:["discord"]}"#), - Some(json!({ "toolkits": ["discord"] })) - ); - } - - #[test] - fn quotes_multiple_unquoted_keys_and_bool_value() { - assert_eq!( - recover_relaxed_object(r#"{include_unconnected:true,toolkits:["discord"]}"#), - Some(json!({ "include_unconnected": true, "toolkits": ["discord"] })) - ); - } - - #[test] - fn substitutes_leaked_quote_tokens_in_values() { - assert_eq!( - recover_relaxed_object(r#"{toolkits:[<|">discord<|">]}"#), - Some(json!({ "toolkits": ["discord"] })) - ); - } - - #[test] - fn substitutes_symmetric_leaked_quote_token_variant() { - assert_eq!( - recover_relaxed_object(r#"{toolkits:[<|"|>discord<|"|>]}"#), - Some(json!({ "toolkits": ["discord"] })) - ); - } - - #[test] - fn peels_one_redundant_brace_layer() { - assert_eq!( - recover_relaxed_object(r#"{{"tool":"X","arguments":{"guild_id":"1"}}}"#), - Some(json!({ "tool": "X", "arguments": { "guild_id": "1" } })) - ); - } - - #[test] - fn peels_and_quotes_together() { - assert_eq!( - recover_relaxed_object( - r#"{{tool:"DISCORD_LIST_CHANNELS",arguments:{"guild_id":"1470856511193616498"}}}"# - ), - Some(json!({ - "tool": "DISCORD_LIST_CHANNELS", - "arguments": { "guild_id": "1470856511193616498" } - })) - ); - } - - #[test] - fn recovers_full_composio_execute_with_leaked_quote_tokens() { - assert_eq!( - recover_relaxed_object( - r#"{arguments:{guild_id:<|">1470856511193616498<|">},tool:<|">DISCORD_GET_GUILD_CHANNELS<|">}"# - ), - Some(json!({ - "arguments": { "guild_id": "1470856511193616498" }, - "tool": "DISCORD_GET_GUILD_CHANNELS" - })) - ); - } - - #[test] - fn peels_several_redundant_layers() { - assert_eq!( - recover_relaxed_object(r#"{{{{tool:"X",arguments:{"guild_id":"1"}}}}}"#), - Some(json!({ "tool": "X", "arguments": { "guild_id": "1" } })) - ); - } - - #[test] - fn handles_reordered_relaxed_keys() { - assert_eq!( - recover_relaxed_object(r#"{{arguments:{guild_id:"1"},tool:"X"}}"#), - Some(json!({ "arguments": { "guild_id": "1" }, "tool": "X" })) - ); - } - - #[test] - fn preserves_brace_inside_string_value() { - assert_eq!( - recover_relaxed_object(r#"{{note:"see {ref:1}"}}"#), - Some(json!({ "note": "see {ref:1}" })) - ); - } - - #[test] - fn does_not_quote_array_elements() { - assert_eq!(recover_relaxed_object(r#"{tags:[hi,bye]}"#), None); - } - - #[test] - fn rejects_keyless_nested_object() { - assert_eq!(recover_relaxed_object(r#"{tool:"X",{guild_id:"Y"}}"#), None); - } - - #[test] - fn rejects_non_object_scalar() { - assert_eq!(recover_relaxed_object("42"), None); - assert_eq!(recover_relaxed_object(r#""just a string""#), None); - assert_eq!(recover_relaxed_object("[1,2,3]"), None); - } - - #[test] - fn rejects_unrecoverable_garbage() { - assert_eq!(recover_relaxed_object(r#"{"a":1]"#), None); - assert_eq!(recover_relaxed_object("not json at all"), None); - } - - #[test] - fn already_valid_object_passes_through() { - assert_eq!( - recover_relaxed_object(r#"{"a":1,"b":{"c":2}}"#), - Some(json!({ "a": 1, "b": { "c": 2 } })) - ); - } - - #[test] - fn does_not_unwrap_legitimate_single_object() { - assert_eq!( - recover_relaxed_object(r#"{guild_id:"1",limit:50}"#), - Some(json!({ "guild_id": "1", "limit": 50 })) - ); - } - - #[test] - fn quote_bare_keys_leaves_quoted_keys_untouched() { - assert_eq!(quote_bare_keys(r#"{"a":1,"b":2}"#), r#"{"a":1,"b":2}"#); - } - - #[test] - fn normalize_leaked_quote_tokens_is_noop_without_tokens() { - assert_eq!(normalize_leaked_quote_tokens(r#"{"a":1}"#), r#"{"a":1}"#); - } - - #[test] - fn object_spans_all_respects_strings_and_trailing() { - assert!(object_spans_all(r#"{"a":"}"}"#)); - assert!(!object_spans_all(r#"{"a":1},{"b":2}"#)); - assert!(!object_spans_all(r#"{"a":1}trailing"#)); - } -} diff --git a/crates/tinyagents-harness/src/runtime/test.rs b/crates/tinyagents-harness/src/runtime/test.rs index 59089994..1bb1a99a 100644 --- a/crates/tinyagents-harness/src/runtime/test.rs +++ b/crates/tinyagents-harness/src/runtime/test.rs @@ -106,6 +106,41 @@ struct RecordingBudget { records: Mutex>, } +/// A permissive budget that records every `estimate.estimated_input_tokens` +/// it is asked to admit, so a test can assert the preflight estimate saw the +/// request the provider actually receives — not a smaller one taken before a +/// later rewrite grew it. +struct EstimateRecordingBudget { + estimates: Mutex>, +} + +impl EstimateRecordingBudget { + fn new() -> Self { + Self { + estimates: Mutex::new(Vec::new()), + } + } +} + +#[async_trait] +impl BudgetGate for EstimateRecordingBudget { + async fn acquire(&self, estimate: &CallEstimate) -> crate::error::Result { + self.estimates + .lock() + .expect("estimate budget lock") + .push(estimate.estimated_input_tokens); + Ok(Permit::unlimited()) + } + + async fn record(&self, _usage: &Usage) -> crate::error::Result<()> { + Ok(()) + } + + fn compression_hint(&self, _state: &ContextState) -> CompressionHint { + CompressionHint::None + } +} + /// Per-invocation trace used by the overlap test below. Each adapter writes /// both its capability name and the identity-bearing value it received, so a /// capability bundle accidentally borrowed from the other root is observable @@ -2542,6 +2577,109 @@ async fn hard_budget_compression_hint_reduces_context_before_the_provider_call() assert_eq!(budget.records.lock().expect("budget lock").len(), 1); } +/// A tool with a deliberately long description, so folding its catalogue +/// entry into the system prompt (as a forced text dialect does) is a large, +/// easily distinguished jump in estimated prompt size. +struct VerboseTool; + +#[async_trait] +impl Tool for VerboseTool { + fn name(&self) -> &str { + "verbose_lookup" + } + + fn description(&self) -> &str { + // ~2 KiB: large enough that folding it into the system prompt moves + // the token estimate by hundreds of tokens under the `chars / 4` + // heuristic `token_estimation::estimate_slice_tokens` uses. + "look something up in the verbose index. " + } + + fn parameters_schema(&self) -> serde_json::Value { + json!({ + "type": "object", + "properties": { "q": { "type": "string", "description": "x".repeat(2000) } }, + "required": ["q"] + }) + } + + async fn execute(&self, _arguments: serde_json::Value) -> anyhow::Result { + Ok(ToolResult::success("verbose-output")) + } +} + +#[tokio::test] +async fn budget_preflight_estimate_reflects_the_dialect_rewritten_request() { + // A forced text dialect (`Xml` here) folds the protocol block and full + // tool catalogue into `request.messages` and clears `request.tools`. + // `token_estimation::estimate_slice_tokens` only looks at + // `request.messages`, so the host budget preflight estimate has to be + // taken *after* that rewrite or it silently estimates a request far + // smaller than the one actually sent to the provider — the whole point + // of a pre-call budget limit defeated by the rewrite arriving late. + let model = Arc::new(ScriptedModel::replies(vec!["done"])); + let budget = Arc::new(EstimateRecordingBudget::new()); + let host = crate::host::HostCapabilities::new( + Arc::new(StaticContextComposer::empty()), + Arc::new(InMemoryDefinitionRegistry::new(vec![AgentDefinition::new( + "helper", + "Helper", + "test helper", + )])), + Arc::new(AllowAllSecurityGate), + Arc::new(FixedModelResolver::new(model.clone())), + ) + .with_budget(budget.clone()); + let mut harness: AgentHarness<()> = AgentHarness::new(); + harness + .register_tool(Arc::new(VerboseTool)) + .with_policy(RunPolicy { + tool_dialect: crate::config::ToolDispatcher::Xml, + ..RunPolicy::default() + }); + + let run = harness + .invoke_agent( + AgentInvocation::new( + host, + AgentTurnRequest::new( + "helper", + vec![tinyinference_llm::message::Message::user("go")], + ), + RunContext::new(RunConfig::new("dialect-budget"), ()), + ), + &(), + ) + .await + .expect("run succeeds"); + assert_eq!(run.text().as_deref(), Some("done")); + + // The request the provider actually received carries the rendered + // catalogue, not a schema — confirming the dialect rewrite did happen + // before this call. + let request = model.requests().pop().expect("provider was called once"); + assert!(request.tools.is_empty(), "no schema goes on the wire"); + let system = request + .messages + .iter() + .find(|m| matches!(m, tinyinference_llm::message::Message::System(_))) + .expect("a system turn carries the protocol") + .text(); + assert!(system.contains("verbose_lookup"), "{system}"); + + let estimates = budget.estimates.lock().expect("estimate budget lock"); + assert_eq!(estimates.len(), 1); + // The bare user turn ("go") alone estimates to a handful of tokens; the + // rewritten request additionally carries the ~2 KiB tool description + // folded into the system prompt. A stale pre-rewrite estimate would stay + // near the former; this asserts it reflects the latter. + assert!( + estimates[0] > 300, + "preflight estimate ({}) does not reflect the dialect-rewritten request", + estimates[0] + ); +} + #[tokio::test] async fn public_hosted_stream_sanitizes_provider_middleware_and_budget_failures() { fn host(model: Arc>) -> crate::host::HostCapabilities<()> { diff --git a/crates/tinyagents-harness/src/runtime/types.rs b/crates/tinyagents-harness/src/runtime/types.rs index ab831f3b..7a5d0075 100644 --- a/crates/tinyagents-harness/src/runtime/types.rs +++ b/crates/tinyagents-harness/src/runtime/types.rs @@ -17,6 +17,7 @@ //! `crate::runtime` directly. Implementations and tests live in the //! sibling `mod.rs` and `test.rs`. +pub use crate::config::ToolDispatcher; use std::collections::HashSet; use std::sync::Arc; @@ -238,6 +239,35 @@ pub struct RunPolicy { /// rely on empty finals; opt in to turn a silent blank success into a typed /// error the caller can re-prompt on. pub error_on_empty_response: bool, + /// How tools are spoken to the model: through the provider's native + /// channel, or through one of the text protocols owned by + /// `tinytools-agent`. + /// + /// [`ToolDispatcher::Auto`] (the default) sends tool schemas on the wire + /// and lets the provider adapter decide — the OpenAI-compatible adapter + /// switches to the JSON-in-tag protocol by itself for a profile without + /// native tool calling. [`ToolDispatcher::Xml`] and + /// [`ToolDispatcher::Pformat`] force a text protocol regardless of + /// provider: the schemas are rendered into the system prompt, nothing goes + /// on the wire as `tools`, and the answer is parsed here. P-Format is the + /// cheapest on tokens and the most demanding on the model, which is why + /// it is opt-in only. + /// + /// Whatever the dispatcher, a response with no structured calls is still + /// read through every text grammar, because native models narrate calls + /// as text often enough to matter. + pub tool_dialect: ToolDispatcher, + /// Maximum consecutive re-prompts when a model signals a tool call it did + /// not make: `finish_reason == "tool_calls"` with no structured call and + /// no text-recoverable one. + /// + /// Some routers rewrite finish reasons, and some models emit the + /// intention without the call. Treating that as the final answer ends the + /// turn on an empty promise; re-prompting once with "issue the actual + /// tool call now" recovers it far more often than not. Each re-prompt is a + /// model call and counts against `limits.max_model_calls`. Defaults to + /// `3`; `0` disables it. + pub dropped_tool_call_nudges: u32, /// Number of automatic retries when a model call returns a *truncated /// empty* completion — `finish_reason == "length"` with no visible text, no /// tool calls, and no structured output. @@ -292,6 +322,8 @@ impl Default for RunPolicy { }, // Opt-in: preserve the historical blank-final behavior by default. error_on_empty_response: false, + tool_dialect: ToolDispatcher::Auto, + dropped_tool_call_nudges: 3, // On by default: a truncated-empty completion is useless to every // caller, so one stochastic-failure retry is strictly better than a // blank final. diff --git a/crates/tinyagents-harness/src/structured/README.md b/crates/tinyagents-harness/src/structured/README.md index 08fb3918..21e8c17f 100644 --- a/crates/tinyagents-harness/src/structured/README.md +++ b/crates/tinyagents-harness/src/structured/README.md @@ -90,6 +90,6 @@ it, each owned by its own submodule: `response_format_for_strategy` supplies the `ResponseFormat` for a `tinyinference_llm::model::ModelRequest`; `repair::parse_lenient` reuses -`crate::relaxed_json::recover_relaxed_object`, the same relaxed-JSON repair +`tinytools_agent::repair::json::recover_object`, the same relaxed-JSON repair tool-call argument parsing already uses, rather than a second divergent implementation. diff --git a/crates/tinyagents-harness/src/structured/repair.rs b/crates/tinyagents-harness/src/structured/repair.rs index 656950c3..4f4e7122 100644 --- a/crates/tinyagents-harness/src/structured/repair.rs +++ b/crates/tinyagents-harness/src/structured/repair.rs @@ -6,12 +6,10 @@ //! assistant text, and a single malformed brace on the final turn discarded the //! whole run — every tool call and token already spent. Meanwhile the crate //! already carried a repair ladder for the *other* place a model emits JSON: -//! tool-call arguments, repaired by -//! [`recover_tool_arguments`][rta] over -//! [`relaxed_json`][rj]. Structured output got none of it. +//! tool-call arguments, repaired by the protocol crate's +//! [`recover_object`][rj]. Structured output got none of it. //! -//! [rta]: tinyinference_llm::providers::openai -//! [rj]: tinyinference_llm::providers::openai::relaxed_json +//! [rj]: tinytools_agent::repair::json::recover_object //! //! # The ladder //! @@ -25,7 +23,7 @@ //! | `Strict` | nothing — the input was already valid | — | //! | `CodeFence` | ```` ```json … ``` ```` wrappers | ubiquitous | //! | `Slice` | prose around the value (`Here is the JSON: {…}`) | — | -//! | `Relaxed` | unquoted keys, doubled braces, leaked chat-template quote tokens | [`relaxed_json`][rj] | +//! | `Relaxed` | unquoted keys, doubled braces, leaked chat-template quote tokens | [`recover_object`][rj] | //! | `Closed` | truncated output: unterminated strings and unclosed brackets | LangChain `parse_partial_json` | //! //! # What it deliberately does not do @@ -111,10 +109,10 @@ pub fn parse_lenient(raw: &str) -> Option<(Value, JsonRepair)> { return Some((value, JsonRepair::Slice)); } - // Reuses the crate's existing relaxed-JSON repairs rather than a second, + // Reuses the protocol crate's repair ladder rather than a second, // divergent implementation. It only yields objects, which is the shape a // JSON-Schema structured output almost always declares. - if let Some(value) = crate::relaxed_json::recover_relaxed_object(unfenced) { + if let Some(value) = tinytools_agent::repair::json::recover_object(unfenced) { tinyagents_tracing::debug!( "[structured::repair] recovered JSON through the relaxed-JSON repairs" ); diff --git a/crates/tinyagents-harness/src/tool/README.md b/crates/tinyagents-harness/src/tool/README.md index d3c927fe..983e1287 100644 --- a/crates/tinyagents-harness/src/tool/README.md +++ b/crates/tinyagents-harness/src/tool/README.md @@ -4,10 +4,13 @@ Harness-side registration, projection, and execution support for canonical tools. Tool *vocabulary* (`Tool`, `ToolCall`, `ToolResult`, `ToolPolicy`, `ToolTimeout`, `WorkspaceDescriptor`, ...) belongs to the `tinytools` crate; this module owns only the host concerns that sit between a declared tool and -a live agent run: name lookup, provider-schema projection, prompt-guided -(text-mode) tool calling, injected-argument enforcement, timeout resolution, -and the explicit recursive-dispatch handoff for tools that must see the typed -parent run. +a live agent run: name lookup, provider-schema projection, injected-argument +enforcement, timeout resolution, and the explicit recursive-dispatch handoff +for tools that must see the typed parent run. Prompt-guided (text-mode) tool +calling is *not* owned here: the protocol (render, parse, repair, stream +scrub) lives in `tinytools-agent`, reached through +`tinyinference_llm::prompt_tools`, and the host-side dialect choice lives in +`agent_loop/dialect.rs`. ## Public surface @@ -74,30 +77,17 @@ path, not by this module itself. - `require_all_properties` / `set_additional_properties_false` — the two halves of OpenAI strict-mode sanitization. -### Prompt-guided (text-mode) tool calling (`prompt.rs`) - -For provider adapters whose model profile has no native tool calling: - -- `prompt_tool_instructions` / `with_prompt_tool_instructions` — embed the - `` protocol and the tool catalogue into the system prompt. -- `coalesce_prompt_tool_results` — renders structured assistant tool calls - back into `` text and folds consecutive tool results into one - `[Tool results]` user turn. -- `ensure_resolvable_user_turn` — inserts a content-free continuation user - turn when none is present, so chat templates that hard-require a locatable - user query (e.g. Qwen 3's) don't reject the request outright. -- `parse_prompt_tool_calls_from_text` — extracts `` blocks (and - DeepSeek's native delimiter) from completed text into `ToolCall`s. -- `ToolCallStreamScrubber` — the streaming counterpart: scrubs `` - markup out of live text deltas as fragments arrive, holding back any tail - that could still grow into an opening delimiter. -- `should_recover` / `apply_prompt_tool_calls` — decide whether text-mode - recovery should run over a completed `ModelResponse` (always for - prompt-guided models, as a fallback for native models that returned no - structured calls) and perform it. -- `SYNTHETIC_CALL_ID_PREFIX` / `next_synthetic_call_id` — mint - process-unique, human-readable ids (`ptc_{sequence}_{slot}`) for a recovered - call, since a per-response counter collides across turns. +### Prompt-guided (text-mode) tool calling + +Not implemented in this module. The `` / P-Format protocols — +instructions, catalogue rendering, coalescing of tool results into a user +turn, `ensure_resolvable_user_turn`, parsing, argument repair, and the +streaming scrubber — are owned by `tinytools-agent` and exposed to adapters +through `tinyinference_llm::prompt_tools` (`with_tool_instructions`, +`coalesce_tool_results`, `recover_tool_calls`, `TextScrubber`). The agent +loop selects the run's dialect (`RunPolicy::tool_dialect`) and mints +`{model_call_id}-tool-{n}` ids for calls recovered from text in +`agent_loop/dialect.rs`. See `docs/modules/harness/tool-dialect.md`. ### Timeouts (`timeout.rs`) @@ -124,7 +114,6 @@ the model. Re-exported here as `pub mod select` and via `pub use select::*`. | `injected.rs` | Injected (host-only) argument stripping and schema projection. | | `schema.rs` | `SchemaCleanr`, `CleaningStrategy`; low-level JSON Schema cleaning. | | `schema_prepare.rs` | Provider projection seam built on `schema.rs`; strict-mode sanitizer. | -| `prompt.rs` | Prompt-guided tool-call protocol: instructions, coalescing, parsing, streaming scrub. | | `timeout.rs` | `ToolTimeoutSettings`, `ResolvedToolTimeout`. | | `select/` | Prompt-driven tool ranking (own submodule; see its README/module doc). | | `*_test.rs`, `test.rs` | Unit tests colocated by concern, listed via `#[path = "..."]` or `mod ..._test;`. | @@ -138,13 +127,10 @@ the model. Re-exported here as `pub mod select` and via `pub use select::*`. - **Schema cleaning must run before strict-mode sanitization** (`prepare_parameters`), so `required` is computed from the resolved property set rather than one still hidden behind an unresolved `$ref`. -- **`ToolCallStreamScrubber` is stateful and per-stream.** Create one per - response stream, feed every fragment through `feed`, and call `flush` once - at the end to drain the final safe remainder; reusing one across streams or - skipping `flush` will misplace or drop trailing text. -- **Synthetic call ids are process-global, not per-response**, specifically - so two recovered calls in different turns of the same run never collide; - do not reset or shard `SYNTHETIC_CALL_SEQUENCE`. +- **Never re-add tool-call markup matching here.** Model-specific + render/parse/scrub logic belongs in `tinytools-agent`; this crate only + consumes it, so a new dialect quirk is fixed upstream, not by a harness + regex. - Canonical tool vocabulary, execution, and policy enforcement itself remain in `tinytools`; this module never redeclares them, only bridges them to a live harness run. diff --git a/crates/tinyagents-harness/src/tool/mod.rs b/crates/tinyagents-harness/src/tool/mod.rs index 39be2186..0f114451 100644 --- a/crates/tinyagents-harness/src/tool/mod.rs +++ b/crates/tinyagents-harness/src/tool/mod.rs @@ -5,7 +5,6 @@ //! routing, and the explicit recursive-dispatch handoff. pub mod discover; -mod prompt; mod schema; mod schema_compact; mod schema_prepare; @@ -20,7 +19,6 @@ use std::sync::Arc; use async_trait::async_trait; use serde_json::Value; -pub use prompt::*; pub use schema::*; pub use schema_compact::*; pub use schema_prepare::*; diff --git a/crates/tinyagents-harness/src/tool/prompt.rs b/crates/tinyagents-harness/src/tool/prompt.rs deleted file mode 100644 index b495a90d..00000000 --- a/crates/tinyagents-harness/src/tool/prompt.rs +++ /dev/null @@ -1,688 +0,0 @@ -//! Provider-neutral prompt-guided (text-mode) tool calling for models without native tool support. -//! -//! Provider adapters whose model profile has `tool_calling = false` can use these -//! helpers to embed tool specs **in the system prompt** as a small protocol and -//! parse the model's `…` blocks back into -//! [`ToolCall`]s — so the agent loop sees tool calls identically to the native -//! path, without changing the harness loop. -//! -//! The `{"name":…,"arguments":…}` convention matches the -//! long-standing OpenHuman host format so models already prompted for it behave -//! identically after the crate cutover. - -use std::fmt::Write as _; - -use serde_json::{Map, Value}; - -use tinyinference_llm::message::{ContentBlock, Message}; -use tinyinference_llm::model::ModelResponse; -use tinyinference_llm::tool::{ToolCall, ToolSchema}; - -/// Opening / closing delimiters for a text-mode tool call. -const OPEN_TAG: &str = ""; -const CLOSE_TAG: &str = ""; - -/// Prefix of the opening delimiter, matched tolerantly so the attribute form -/// (``, as emitted by Hermes / DeepSeek chat templates) -/// and the pipe variant (``) are recognized — not just the bare -/// `` literal. The scanner matches this prefix, verifies the tag name -/// is properly delimited, and then consumes up to the tag's closing `>`. -const OPEN_PREFIX: &str = "` convention and lists -/// each tool's name, description, and a compact TypeScript-style argument -/// signature (see [`super::type_signature`]) with one note per described -/// top-level argument — a fraction of the tokens of the raw JSON Schema, which -/// used to be pasted here verbatim for every tool on every request. -pub fn prompt_tool_instructions(tools: &[ToolSchema]) -> String { - let mut out = String::new(); - out.push_str("## Tool Use Protocol\n\n"); - out.push_str("To use a tool, wrap a JSON object in tags:\n\n"); - out.push_str(OPEN_TAG); - out.push('\n'); - out.push_str(r#"{"name": "tool_name", "arguments": {"param": "value"}}"#); - out.push('\n'); - out.push_str(CLOSE_TAG); - out.push_str("\n\n"); - out.push_str("You may emit multiple tool calls in a single response. "); - out.push_str("After execution, results appear in tags. "); - out.push_str("Continue reasoning with the results until you can give a final answer.\n\n"); - out.push_str("### Available Tools\n\n"); - out.push_str("Arguments are shown as `{name: type, optional?: type}`.\n\n"); - for tool in tools { - // Infallible: writing to a String never errors. - let _ = writeln!(out, "**{}**: {}", tool.name, tool.description); - let _ = writeln!( - out, - "Arguments: `{}`", - super::signature::type_signature(&tool.parameters) - ); - for note in super::signature::argument_notes(&tool.parameters) { - let _ = writeln!(out, " - {note}"); - } - out.push('\n'); - } - out -} - -/// Return `messages` with the tool-use protocol appended to the system prompt: -/// the instructions are added as a trailing block on the first system message, or -/// a new leading system message when the request carries none. `tools` empty → -/// `messages` is returned unchanged (cloned). -pub fn with_prompt_tool_instructions(messages: &[Message], tools: &[ToolSchema]) -> Vec { - if tools.is_empty() { - return messages.to_vec(); - } - let block = prompt_tool_instructions(tools); - let mut out = messages.to_vec(); - if let Some(Message::System(system)) = out.iter_mut().find(|m| matches!(m, Message::System(_))) - { - // Append as a distinct text block so the original system prompt is intact. - system - .content - .push(ContentBlock::Text(format!("\n\n{block}"))); - } else { - out.insert(0, Message::system(block)); - } - out -} - -/// Convert structured assistant tool calls and native tool-result messages into -/// prompt-guided turns. -/// -/// Models without native tool calling cannot consume provider assistant -/// `tool_calls` fields or a `tool` role. Assistant calls are rendered back into -/// `` blocks and cleared from the structured field; consecutive -/// results are folded into one `[Tool results]` user message with each result -/// wrapped in the advertised `` protocol. Other messages keep -/// their original order and type. -pub fn coalesce_prompt_tool_results(messages: &[Message]) -> Vec { - let mut out = Vec::with_capacity(messages.len()); - let mut pending = Vec::new(); - - fn flush(out: &mut Vec, pending: &mut Vec) { - if !pending.is_empty() { - out.push(Message::user(format!( - "{TOOL_RESULTS_MARKER}\n{}", - std::mem::take(pending).join("\n") - ))); - } - } - - for message in messages { - match message { - Message::Tool(_) => { - pending.push(format!("\n{}\n", message.text())); - } - Message::Assistant(assistant) if !assistant.tool_calls.is_empty() => { - flush(&mut out, &mut pending); - let mut assistant = assistant.clone(); - let mut rendered = String::new(); - if !assistant.content.is_empty() && !message.text().trim().is_empty() { - rendered.push('\n'); - } - for call in &assistant.tool_calls { - let body = serde_json::json!({ - "name": &call.name, - "arguments": &call.arguments, - }); - let body = serde_json::to_string(&body).unwrap_or_else(|_| "{}".to_string()); - let _ = writeln!(rendered, "{OPEN_TAG}{body}{CLOSE_TAG}"); - } - assistant.content.push(ContentBlock::Text(rendered)); - assistant.tool_calls.clear(); - out.push(Message::Assistant(assistant)); - } - _ => { - flush(&mut out, &mut pending); - out.push(message.clone()); - } - } - } - flush(&mut out, &mut pending); - out -} - -/// Whether this message is a user turn a chat template can resolve as "the user -/// query". -/// -/// A folded tool-result turn does not count. It carries the -/// [`TOOL_RESULTS_MARKER`] prefix, and templates that look for a user query are -/// looking for a request to answer, not for the transcript of a tool the model -/// itself invoked — Qwen 3's template makes the same distinction, skipping user -/// turns that are wholly a tool response. Neither does an empty or -/// whitespace-only turn. Non-text content (JSON, an image) does count: it is a -/// real user input the model is being asked about. -fn is_resolvable_user_query(message: &Message) -> bool { - let Message::User(user) = message else { - return false; - }; - if message.text().trim_start().starts_with(TOOL_RESULTS_MARKER) { - return false; - } - user.content.iter().any(|block| match block { - ContentBlock::Text(text) => !text.trim().is_empty(), - ContentBlock::Json(_) | ContentBlock::Image(_) => true, - // Reasoning replay and opaque provider payloads are not user input. - ContentBlock::Thinking { .. } - | ContentBlock::RedactedThinking { .. } - | ContentBlock::ProviderExtension(_) => false, - }) -} - -/// Guarantee the outgoing list contains a user turn a chat template can resolve, -/// inserting one only when none is present. -/// -/// Models without native tool calling are driven through their **own** chat -/// template by the serving runtime (LM Studio, llama.cpp, Ollama), and several -/// widely used templates hard-require a locatable user query. Qwen 3's raises -/// outright: -/// -/// ```text -/// {%- if ns.multi_step_tool %}{{- raise_exception('No user query found in messages.') }} -/// ``` -/// -/// A prompt-guided tool loop can reach that state legitimately: once the real -/// user turn has aged out of the window — summarization, a resumed transcript, -/// a task delivered entirely through the system prompt — every remaining -/// non-system turn is an assistant continuation or a folded tool result, and the -/// template aborts the request with a 400 before the model is ever called -/// (tinyhumansai/openhuman#5291). Native-tool models are unaffected: they are -/// served through the provider's own tool protocol, not a Jinja template with -/// this guard. -/// -/// The inserted turn goes directly after any leading system messages, so the -/// transcript still reads system → user → assistant. A request that already has -/// a real user turn is returned unchanged. -pub fn ensure_resolvable_user_turn(messages: &[Message]) -> Vec { - if messages.iter().any(is_resolvable_user_query) { - return messages.to_vec(); - } - let mut out = messages.to_vec(); - let insert_at = out - .iter() - .position(|message| !matches!(message, Message::System(_))) - .unwrap_or(out.len()); - out.insert(insert_at, Message::user(CONTINUATION_USER_TURN)); - out -} - -/// Extract `…` blocks from `text`, parsing each inner JSON -/// object (`{"name":…,"arguments":…}`) into a [`ToolCall`]. Returns the text with -/// the blocks removed (trimmed) plus the parsed calls, in order. -/// -/// The opening delimiter is matched tolerantly: the bare `` literal, -/// the attribute form `` and pipe variant `` -/// emitted by Hermes / DeepSeek chat templates, and the DeepSeek -/// `<｜tool▁call▁begin｜>` delimiter all open a block. -/// -/// Robust to noise: a block whose inner text is not a JSON object with a string -/// `name` is dropped (its raw markup never leaks back into the text); a dangling -/// open tag with no close is left verbatim in the returned text; a prose mention -/// of `` (or the plural ``) is not -/// treated as an opening tag. -pub fn parse_prompt_tool_calls_from_text(text: &str) -> (String, Vec) { - let mut calls = Vec::new(); - let mut cleaned = String::new(); - let mut rest = text; - - while let Some(open) = next_open(rest) { - cleaned.push_str(&rest[..open.start]); - let after_open = &rest[open.body_start..]; - let Some(end) = after_open.find(open.close) else { - // Unterminated block: keep it (and everything after) as plain text. - cleaned.push_str(&rest[open.start..]); - return (cleaned.trim().to_string(), calls); - }; - let inner = after_open[..end].trim(); - if let Some(call) = parse_one(inner, calls.len() + 1) { - calls.push(call); - } - rest = &after_open[end + open.close.len()..]; - } - cleaned.push_str(rest); - (cleaned.trim().to_string(), calls) -} - -/// A located opening tool-call delimiter. -struct OpenMatch { - /// Byte offset of the opening `<`. - start: usize, - /// Byte offset where the inner body begins (just past the opening tag's `>` - /// for the `` family, or past the DeepSeek open delimiter). - body_start: usize, - /// Closing delimiter that terminates this block. - close: &'static str, -} - -/// Find the earliest opening tool-call delimiter in `text`, tolerant of the -/// attribute form (``), the pipe variant (``), and -/// the DeepSeek `<｜tool▁call▁begin｜>` delimiter. Returns `None` when no complete -/// opening tag is present. A bare `` (prose) -/// is not treated as an opening tag. -fn next_open(text: &str) -> Option { - let mut best: Option = None; - - // DeepSeek native delimiter (literal open/close pair). - if let Some(start) = text.find(DS_OPEN) { - best = Some(OpenMatch { - start, - body_start: start + DS_OPEN.len(), - close: DS_CLOSE, - }); - } - - // `` family: the first prefix occurrence whose tag name is - // properly delimited (`>`, whitespace, or `|` — so `` / - // `` do not match) and is closed by a `>`. - let mut from = 0; - while let Some(rel) = text[from..].find(OPEN_PREFIX) { - let start = from + rel; - let after_prefix = &text[start + OPEN_PREFIX.len()..]; - let delimited = match after_prefix.chars().next() { - Some('>') | Some('|') => true, - Some(c) => c.is_whitespace(), - None => false, - }; - if delimited && let Some(gt) = after_prefix.find('>') { - let candidate = OpenMatch { - start, - body_start: start + OPEN_PREFIX.len() + gt + 1, - close: CLOSE_TAG, - }; - best = match best { - Some(b) if b.start <= candidate.start => Some(b), - _ => Some(candidate), - }; - break; - } - // Not a usable open tag here (prose mention, or no closing `>`): keep - // scanning past this occurrence. Advances by a non-zero amount. - from = start + OPEN_PREFIX.len(); - } - - best -} - -/// Byte index in `buf` from which the trailing bytes must be held back because -/// they could still grow into a tool-call open delimiter once more text arrives. -/// Returns `buf.len()` when the whole buffer is provably safe to surface now. -/// -/// Two shapes are held: an in-progress `` -/// has not arrived yet (so [`next_open`] cannot see it), and a trailing byte run -/// that is a proper prefix of an open delimiter (` usize { - // In-progress `` has not yet arrived. `` (name not delimited) is not openable. - let mut from = 0; - let mut incomplete: Option = None; - while let Some(rel) = buf[from..].find(OPEN_PREFIX) { - let start = from + rel; - let after = &buf[start + OPEN_PREFIX.len()..]; - let openable = match after.chars().next() { - None => true, - Some('>') | Some('|') => true, - Some(c) => c.is_whitespace(), - }; - if openable && !after.contains('>') { - incomplete = Some(start); - } - from = start + OPEN_PREFIX.len(); - } - if let Some(start) = incomplete { - return start; - } - - // Trailing proper prefix of an open delimiter (a full delimiter would have - // been reported by `next_open` or the in-progress branch above). - let len = buf.len(); - for marker in [OPEN_PREFIX, DS_OPEN] { - // A held tail can be as long as the whole buffer when the buffer is - // shorter than the marker, so the range is inclusive of `max`. - let max = marker.len().min(len); - for k in (1..=max).rev() { - if marker.is_char_boundary(k) - && buf.is_char_boundary(len - k) - && buf[len - k..] == marker[..k] - { - return len - k; - } - } - } - len -} - -/// Streaming counterpart to [`parse_prompt_tool_calls_from_text`]: strips -/// `…` markup from a text stream *as fragments arrive*. -/// -/// The terminal-response recovery ([`apply_prompt_tool_calls`]) only cleans the -/// aggregated answer, so a consumer that renders live -/// [`MessageDelta`](tinyinference_llm::message::MessageDelta) text would -/// still see raw markup stream through. This scrubber closes that gap: it emits -/// only the text that is provably not part of a tool-call block, holding back any -/// tail that could still become one (a partial `` or matching close has not arrived, or a partial DeepSeek delimiter) until -/// more input resolves it. -/// -/// It is stateful across fragments — feed each fragment through [`feed`](Self::feed) -/// and call [`flush`](Self::flush) once when the stream ends to drain the final -/// safe remainder. Only the visible-text channel is scrubbed; reasoning and -/// structured tool-call channels are unaffected. -#[derive(Debug, Default)] -pub struct ToolCallStreamScrubber { - buf: String, -} - -impl ToolCallStreamScrubber { - /// Creates an empty scrubber. - pub fn new() -> Self { - Self::default() - } - - /// Feeds the next text fragment and returns the portion that is safe to emit - /// now. Complete `` blocks are dropped; text that could still be - /// the start of one is buffered for a later fragment. - pub fn feed(&mut self, fragment: &str) -> String { - self.buf.push_str(fragment); - let mut out = String::new(); - loop { - match next_open(&self.buf) { - Some(open) => { - let after_open = &self.buf[open.body_start..]; - if let Some(end) = after_open.find(open.close) { - // Complete block: the prefix before it is safe; drop the - // block and keep scanning the remainder. - out.push_str(&self.buf[..open.start]); - let next = open.body_start + end + open.close.len(); - self.buf.drain(..next); - continue; - } - // Open delimiter located but not yet closed: emit the prefix, - // hold the unterminated block for later fragments. - out.push_str(&self.buf[..open.start]); - self.buf.drain(..open.start); - break; - } - None => { - // No complete open delimiter: emit everything except the tail - // that could still grow into one. - let hold = hold_from(&self.buf); - out.push_str(&self.buf[..hold]); - self.buf.drain(..hold); - break; - } - } - } - out - } - - /// Drains the final safe remainder once no more fragments will arrive. A - /// complete tool-call block still buffered is dropped; a dangling partial - /// delimiter is surfaced verbatim (with the stream ended it was never a real - /// call). Unlike [`parse_prompt_tool_calls_from_text`], the remainder is not - /// trimmed — streamed whitespace is preserved. - pub fn flush(&mut self) -> String { - let mut out = String::new(); - loop { - match next_open(&self.buf) { - Some(open) => { - let after_open = &self.buf[open.body_start..]; - if let Some(end) = after_open.find(open.close) { - out.push_str(&self.buf[..open.start]); - let next = open.body_start + end + open.close.len(); - self.buf.drain(..next); - continue; - } - // Unterminated at end of stream: real text, emit verbatim. - out.push_str(&self.buf); - break; - } - None => { - out.push_str(&self.buf); - break; - } - } - } - self.buf.clear(); - out - } -} - -/// Whether text-mode `` recovery should run over a completed response. -/// -/// * Prompt-guided models (`native == false`) always recover — the whole point of -/// the mode is that tool calls arrive as text. -/// * Native models recover only as a **fallback**: when they were offered tools -/// but returned an empty structured `tool_calls` array, some OpenAI-compatible -/// routes (Hermes / DeepSeek chat templates via OpenRouter) emit the call as -/// `…` text instead of the structured field — recovering -/// it keeps the raw markup from leaking to the caller as assistant content. -/// * When native tool calls came back structured (`structured_calls > 0`), -/// recovery is skipped so the native path stays byte-for-byte unchanged. -/// * When no tools were offered, there is nothing to recover. -pub fn should_recover(native: bool, has_tools: bool, structured_calls: usize) -> bool { - has_tools && (!native || structured_calls == 0) -} - -/// Extract prompt-guided `` blocks from a completed [`ModelResponse`]'s -/// text into `message.tool_calls`, replacing the message content with the cleaned -/// prose. No-op when the text carries no blocks — so a plain text answer is -/// untouched. Provider adapters should apply this to each completed response after -/// using [`with_prompt_tool_instructions`]. -pub fn apply_prompt_tool_calls(mut response: ModelResponse) -> ModelResponse { - let text = response.text(); - let (cleaned, mut calls) = parse_prompt_tool_calls_from_text(&text); - if calls.is_empty() { - // No delimited block. A small local model may still have emitted the - // call as a bare object with no markup at all — see - // `parse_bare_tool_call`. That path consumes the whole content, so the - // cleaned prose is empty by construction. - if let Some(call) = parse_bare_tool_call(&text) { - calls.push(call); - response.message.tool_calls.extend(calls); - // The object was the whole visible text, so nothing survives as - // prose — but a reasoning model's `Thinking` block must, hence - // `replace_text_blocks` with empty text rather than clearing the - // content outright. - response.message.content = replace_text_blocks(response.message.content, String::new()); - return response; - } - return response; - } - response.message.tool_calls.extend(calls); - response.message.content = replace_text_blocks(response.message.content, cleaned); - response -} - -/// Rebuild a content vector, keeping every non-[`ContentBlock::Text`] block (e.g. -/// `Thinking`) in place and substituting the single cleaned text at the position -/// of the first original `Text` block. If the original content had no `Text` -/// block, the cleaned text (when non-empty) is appended; if `cleaned` is empty, -/// no text block is emitted at all. -fn replace_text_blocks(content: Vec, cleaned: String) -> Vec { - let mut out = Vec::with_capacity(content.len()); - let mut inserted = false; - for block in content { - match block { - ContentBlock::Text(_) => { - if !inserted { - if !cleaned.is_empty() { - out.push(ContentBlock::Text(cleaned.clone())); - } - inserted = true; - } - } - other => out.push(other), - } - } - if !inserted && !cleaned.is_empty() { - out.push(ContentBlock::Text(cleaned)); - } - out -} - -/// Keys a model may put its arguments under inside a tool-call object. -/// -/// `arguments` is the OpenAI spelling; `parameters` is what a model copying the -/// *schema* vocabulary reaches for, and is what `llama3.2:3b` emits. -const CALL_ARGUMENT_KEYS: [&str; 4] = ["arguments", "parameters", "args", "input"]; - -/// Parse a single tool-call body into a [`ToolCall`] with a synthetic id. -/// -/// `slot` is the call's 1-based position within the response it was recovered -/// from; it appears in the id only for readability. Uniqueness comes from the -/// process-wide counter in [`next_synthetic_call_id`], not from `slot`. -fn parse_one(inner: &str, slot: usize) -> Option { - let value = parse_relaxed_object(inner)?; - tool_call_from_object(&value, slot) -} - -/// Monotonic source of unique synthetic tool-call ids. -/// -/// # Why a global counter and not a per-response index -/// -/// The recovered id previously came from the call's position **within one -/// response** (`call_1`, `call_2`, …), which resets on every model turn. That is -/// wrong for anything but a single-turn run: two turns of the same run both emit -/// `call_1`, so the next request contains two assistant messages declaring the -/// same tool-call id and two tool messages answering it. The pairing is then -/// unresolvable — a provider cannot tell which result answers which call, and -/// neither can the harness's own pairing repair. -/// -/// This is **not** confined to prompt-guided models. -/// [`should_recover`] returns `true` for a *native* profile whenever tools were -/// offered and the response carried no structured calls, so a native run that -/// hits the text-mode fallback twice collides exactly the same way. -/// -/// A process-wide `AtomicU64` makes every recovered id unique for the lifetime -/// of the process, which is strictly stronger than per-run uniqueness and needs -/// no run context threaded into a pure parsing function. -static SYNTHETIC_CALL_SEQUENCE: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(1); - -/// Prefix of every synthetic id minted here. -/// -/// Deliberately **not** `call_` and **not** `tool-`: those are the shapes real -/// providers emit and the shape the OpenAI adapter mints for its own -/// positional fallback (`tool-{slot}`), so a distinct prefix makes a collision -/// between the two schemes impossible by construction and makes a synthetic id -/// obvious in a transcript or a log. -pub const SYNTHETIC_CALL_ID_PREFIX: &str = "ptc"; - -/// Returns a fresh, process-unique synthetic tool-call id of the form -/// `ptc_{sequence}_{slot}` — "prompt tool call". -/// -/// `slot` is the 1-based position of the call within its response and is -/// included only so a human reading a transcript can see the ordering; the -/// `sequence` component is what guarantees uniqueness. -pub fn next_synthetic_call_id(slot: usize) -> String { - let sequence = SYNTHETIC_CALL_SEQUENCE.fetch_add(1, std::sync::atomic::Ordering::Relaxed); - let id = format!("{SYNTHETIC_CALL_ID_PREFIX}_{sequence}_{slot}"); - tinyagents_tracing::trace!("[tool::prompt] minted synthetic tool-call id {id}"); - id -} - -/// Parses a JSON object, repairing the relaxed spellings small local models -/// emit (unquoted keys, redundant braces, leaked quote tokens) when strict -/// parsing fails. -fn parse_relaxed_object(raw: &str) -> Option { - match serde_json::from_str::(raw) { - Ok(value) if value.is_object() => Some(value), - // A non-object parsed strictly is not a tool call; do not try to - // "repair" it into one. - Ok(_) => None, - Err(_) => crate::relaxed_json::recover_relaxed_object(raw), - } -} - -/// Builds a [`ToolCall`] from an already-parsed call object, or `None` when the -/// object does not name a tool. -/// -/// The id is minted by [`next_synthetic_call_id`] and is unique for the life of -/// the process, so two calls recovered in different turns of the same run can -/// never share one. -fn tool_call_from_object(value: &Value, slot: usize) -> Option { - let name = value.get("name")?.as_str()?.trim().to_string(); - if name.is_empty() { - return None; - } - let arguments = CALL_ARGUMENT_KEYS - .iter() - .find_map(|key| value.get(*key).cloned()) - .unwrap_or_else(|| Value::Object(Map::new())); - Some(ToolCall { - id: next_synthetic_call_id(slot), - name, - arguments, - invalid: None, - }) -} - -/// Recovers a tool call a model emitted as a **bare object**, with no -/// `` markup of any kind. -/// -/// Observed on `llama3.2:3b` via Ollama under `tool_choice: "required"`: rather -/// than populating the wire's `tool_calls` array, roughly one response in a -/// dozen puts the call in `content` as -/// -/// ```text -/// {"name":"get_weather","parameters':{'city':"Paris"}} -/// ``` -/// -/// — note the mismatched quotes, which strict JSON also rejects. Without -/// recovery the agent loop sees an assistant message with no tool calls, treats -/// it as the final answer, and silently returns JSON-looking prose to the user -/// instead of running the tool. -/// -/// # Why this cannot swallow a genuine text answer -/// -/// The recovery requires the **entire** message content (trimmed, and with a -/// surrounding markdown fence removed) to parse as a single JSON object -/// carrying a string `name`. Prose that merely mentions or quotes JSON has text -/// outside the object and is left untouched, as is any object that does not -/// name a tool. The caller only reaches this path when the request declared -/// tools and the response carried no structured tool calls. -fn parse_bare_tool_call(text: &str) -> Option { - let candidate = strip_code_fence(text.trim()); - if !(candidate.starts_with('{') && candidate.ends_with('}')) { - return None; - } - let value = parse_relaxed_object(candidate)?; - tool_call_from_object(&value, 1) -} - -/// Strips one surrounding markdown code fence, with or without a language tag. -fn strip_code_fence(raw: &str) -> &str { - let Some(after_open) = raw.strip_prefix("```") else { - return raw; - }; - let body = match after_open.find('\n') { - Some(newline) => &after_open[newline + 1..], - None => return raw, - }; - body.trim_end().strip_suffix("```").map_or(raw, str::trim) -} - -#[cfg(test)] -#[path = "prompt_test.rs"] -mod tests; diff --git a/crates/tinyagents-harness/src/tool/prompt_test.rs b/crates/tinyagents-harness/src/tool/prompt_test.rs deleted file mode 100644 index f25e2092..00000000 --- a/crates/tinyagents-harness/src/tool/prompt_test.rs +++ /dev/null @@ -1,678 +0,0 @@ -//! Tests for the prompt-guided tool-call protocol. - -use super::*; -use tinyinference_llm::message::{ContentBlock, ImageRef, Message}; -use tinyinference_llm::model::ModelResponse; - -fn schema(name: &str) -> ToolSchema { - ToolSchema { - name: name.to_string(), - description: format!("{name} description"), - parameters: serde_json::json!({"type": "object"}), - format: Default::default(), - } -} - -#[test] -fn prompt_instructions_list_each_tool() { - let text = prompt_tool_instructions(&[schema("read_file"), schema("write_file")]); - assert!(text.contains("## Tool Use Protocol")); - assert!(text.contains("")); - assert!(text.contains("**read_file**")); - assert!(text.contains("**write_file**")); -} - -#[test] -fn prompt_instructions_append_to_system() { - let msgs = vec![Message::system("You are helpful."), Message::user("hi")]; - let out = with_prompt_tool_instructions(&msgs, &[schema("read_file")]); - assert_eq!(out.len(), 2); - let Message::System(system) = &out[0] else { - panic!("first message should stay system") - }; - let joined: String = system - .content - .iter() - .filter_map(|block| match block { - ContentBlock::Text(text) => Some(text.as_str()), - _ => None, - }) - .collect(); - assert!(joined.contains("You are helpful.")); - assert!(joined.contains("Tool Use Protocol")); -} - -#[test] -fn prompt_instructions_insert_system_when_absent() { - let msgs = vec![Message::user("hi")]; - let out = with_prompt_tool_instructions(&msgs, &[schema("read_file")]); - assert_eq!(out.len(), 2); - assert!(matches!(out[0], Message::System(_))); -} - -#[test] -fn empty_tools_leave_messages_unchanged() { - let msgs = vec![Message::user("hi")]; - assert_eq!(with_prompt_tool_instructions(&msgs, &[]), msgs); -} - -#[test] -fn prompt_results_coalesce_consecutive_tool_messages() { - let messages = vec![ - Message::user("question"), - Message::assistant("calling tools"), - Message::tool("call-1", "first"), - Message::tool("call-2", "second"), - Message::assistant("done"), - ]; - - let out = coalesce_prompt_tool_results(&messages); - - assert_eq!(out.len(), 4); - assert!(matches!(out[0], Message::User(_))); - assert!(matches!(out[1], Message::Assistant(_))); - assert!(matches!(out[2], Message::User(_))); - assert_eq!( - out[2].text(), - "[Tool results]\n\nfirst\n\n\nsecond\n" - ); - assert!(matches!(out[3], Message::Assistant(_))); -} - -#[test] -fn prompt_result_coalescing_without_tools_is_identity() { - let messages = vec![Message::system("system"), Message::user("question")]; - assert_eq!(coalesce_prompt_tool_results(&messages), messages); -} - -#[test] -fn user_turn_normalization_leaves_a_real_query_alone() { - let messages = vec![ - Message::system("system"), - Message::user("question"), - Message::assistant("answer"), - ]; - assert_eq!(ensure_resolvable_user_turn(&messages), messages); -} - -#[test] -fn user_turn_normalization_inserts_after_leading_system_turns() { - // openhuman#5291: the real user turn aged out of the window, leaving a - // system prompt and an assistant continuation. Qwen 3's template raises - // `No user query found in messages.` on exactly this shape. - let messages = vec![ - Message::system("system"), - Message::system("tool protocol"), - Message::assistant("continuing"), - ]; - - let out = ensure_resolvable_user_turn(&messages); - - assert_eq!(out.len(), 4); - assert!(matches!(out[0], Message::System(_))); - assert!(matches!(out[1], Message::System(_))); - assert!(matches!(out[2], Message::User(_)), "user turn is inserted"); - assert!(matches!(out[3], Message::Assistant(_))); -} - -#[test] -fn user_turn_normalization_does_not_count_folded_tool_results() { - // The only user-role turns are coalesced tool results, which is not a query - // the template can answer — the model asked for those itself. - let coalesced = coalesce_prompt_tool_results(&[ - Message::system("system"), - Message::assistant("calling"), - Message::tool("call-1", "result"), - ]); - assert!( - coalesced.iter().any(|m| matches!(m, Message::User(_))), - "coalescing produces a user-role turn" - ); - - let out = ensure_resolvable_user_turn(&coalesced); - - assert_eq!(out.len(), coalesced.len() + 1); - assert!(matches!(out[1], Message::User(_))); - assert!(!out[1].text().starts_with("[Tool results]")); -} - -#[test] -fn user_turn_normalization_ignores_a_blank_user_turn() { - let messages = vec![Message::system("system"), Message::user(" ")]; - let out = ensure_resolvable_user_turn(&messages); - assert_eq!(out.len(), 3); - assert!(!out[1].text().trim().is_empty()); -} - -#[test] -fn user_turn_normalization_accepts_a_non_text_user_turn() { - // An image-only turn carries no text but is still a real user input. - let mut messages = vec![Message::system("system"), Message::user("")]; - let Message::User(user) = &mut messages[1] else { - unreachable!() - }; - user.content = vec![ContentBlock::Image(ImageRef { - url: "https://example.invalid/a.png".to_string(), - mime_type: None, - })]; - - assert_eq!(ensure_resolvable_user_turn(&messages), messages); -} - -#[test] -fn user_turn_normalization_inserts_first_when_there_is_no_system_turn() { - let messages = vec![Message::assistant("continuing")]; - let out = ensure_resolvable_user_turn(&messages); - assert_eq!(out.len(), 2); - assert!(matches!(out[0], Message::User(_))); -} - -#[test] -fn prompt_replay_renders_assistant_calls_before_results() { - let mut assistant = Message::assistant("I will inspect both files."); - let Message::Assistant(message) = &mut assistant else { - unreachable!() - }; - message.tool_calls = vec![ - ToolCall::new("call-1", "read_file", serde_json::json!({"path":"a.txt"})), - ToolCall::new("call-2", "read_file", serde_json::json!({"path":"b.txt"})), - ]; - let messages = vec![ - Message::user("compare them"), - assistant, - Message::tool("call-1", "first"), - Message::tool("call-2", "second"), - ]; - - let out = coalesce_prompt_tool_results(&messages); - - let Message::Assistant(replayed) = &out[1] else { - panic!("assistant call turn should remain an assistant turn") - }; - assert!(replayed.tool_calls.is_empty()); - assert!(out[1].text().contains("I will inspect both files.")); - assert!( - out[1].text().contains( - r#"{"arguments":{"path":"a.txt"},"name":"read_file"}"# - ) - ); - assert!( - out[1].text().contains( - r#"{"arguments":{"path":"b.txt"},"name":"read_file"}"# - ) - ); - assert!( - out[2] - .text() - .contains("\nfirst\n") - ); - assert!( - out[2] - .text() - .contains("\nsecond\n") - ); -} - -#[test] -fn prompt_parser_extracts_single_tool_call() { - let text = r#"Let me read it. - -{"name": "read_file", "arguments": {"path": "a.txt"}} -"#; - let (cleaned, calls) = parse_prompt_tool_calls_from_text(text); - assert_eq!(cleaned, "Let me read it."); - assert_eq!(calls.len(), 1); - assert_eq!(calls[0].name, "read_file"); - // Ids are process-unique, not a per-response index: only the shape and the - // slot suffix are stable. See `next_synthetic_call_id`. - assert!( - calls[0] - .id - .starts_with(&format!("{SYNTHETIC_CALL_ID_PREFIX}_")), - "unexpected synthetic id {}", - calls[0].id - ); - assert!( - calls[0].id.ends_with("_1"), - "slot suffix lost: {}", - calls[0].id - ); - assert_eq!(calls[0].arguments, serde_json::json!({"path": "a.txt"})); -} - -#[test] -fn prompt_parser_extracts_multiple_calls_and_keeps_prose() { - let text = r#"a{"name":"one","arguments":{}}b{"name":"two","arguments":{"x":1}}c"#; - let (cleaned, calls) = parse_prompt_tool_calls_from_text(text); - assert_eq!(cleaned, "abc"); - assert_eq!(calls.len(), 2); - assert_eq!(calls[0].name, "one"); - assert_eq!(calls[1].name, "two"); - assert!( - calls[1].id.ends_with("_2"), - "slot suffix lost: {}", - calls[1].id - ); - assert_ne!(calls[0].id, calls[1].id); -} - -#[test] -fn prompt_parser_defaults_missing_arguments_to_empty_object() { - let (_, calls) = - parse_prompt_tool_calls_from_text(r#"{"name":"noargs"}"#); - assert_eq!(calls.len(), 1); - assert_eq!(calls[0].arguments, serde_json::json!({})); -} - -#[test] -fn prompt_parser_drops_malformed_block() { - let (cleaned, calls) = parse_prompt_tool_calls_from_text("not jsondone"); - assert!(calls.is_empty()); - assert_eq!(cleaned, "done"); -} - -#[test] -fn prompt_parser_keeps_unterminated_block_as_text() { - let text = "text {\"name\":\"x\"}"; - let (cleaned, calls) = parse_prompt_tool_calls_from_text(text); - assert!(calls.is_empty()); - assert_eq!(cleaned, "text {\"name\":\"x\"}"); -} - -#[test] -fn prompt_parser_returns_plain_text_verbatim() { - let (cleaned, calls) = parse_prompt_tool_calls_from_text("just a normal answer"); - assert!(calls.is_empty()); - assert_eq!(cleaned, "just a normal answer"); -} - -// --- Attribute / variant-tolerant matching (Hermes / DeepSeek templates) --- - -#[test] -fn prompt_parser_matches_attribute_form_open_tag() { - // Regression for the exact-literal miss: `` must match so a - // native model that leaks the call as text doesn't dump raw markup. - let text = r#"{"name":"foo","arguments":{"a":1}}"#; - let (cleaned, calls) = parse_prompt_tool_calls_from_text(text); - assert_eq!(calls.len(), 1); - assert_eq!(calls[0].name, "foo"); - assert_eq!(calls[0].arguments, serde_json::json!({"a": 1})); - assert!(cleaned.is_empty()); - assert!(!cleaned.contains("{"name":"foo","arguments":{}}"#; - let (_, calls) = parse_prompt_tool_calls_from_text(text); - assert_eq!(calls.len(), 1); - assert_eq!(calls[0].name, "foo"); -} - -#[test] -fn prompt_parser_matches_deepseek_delimiters() { - let text = "<｜tool▁call▁begin｜>{\"name\":\"foo\",\"arguments\":{}}<｜tool▁call▁end｜>"; - let (cleaned, calls) = parse_prompt_tool_calls_from_text(text); - assert_eq!(calls.len(), 1); - assert_eq!(calls[0].name, "foo"); - assert!(cleaned.is_empty()); -} - -#[test] -fn prompt_parser_drops_attribute_form_no_name_body_without_leak() { - // The reported bug: `{}` has no `name`. - // It must be dropped — never echoed back as assistant content. - let text = "prefix \n{}\n suffix"; - let (cleaned, calls) = parse_prompt_tool_calls_from_text(text); - assert!(calls.is_empty()); - assert!(!cleaned.contains("` must not be mistaken for an opening ``. - let text = "see the list"; - let (cleaned, calls) = parse_prompt_tool_calls_from_text(text); - assert!(calls.is_empty()); - assert_eq!(cleaned, "see the list"); -} - -#[test] -fn prompt_parser_does_not_misparse_prose_open_tag_without_close() { - let text = "You can emit a block to call a tool."; - let (cleaned, calls) = parse_prompt_tool_calls_from_text(text); - assert!(calls.is_empty()); - assert_eq!(cleaned, text); -} - -// --- apply_prompt_tool_calls recovery + native-mode fallback gating --- - -#[test] -fn apply_prompt_tool_calls_recovers_attribute_markup() { - // A native model emitted the call as text with EMPTY structured tool_calls: - // recovery yields a structured call and the raw markup does NOT survive. - let resp = tinyinference_llm::model::ModelResponse::assistant( - r#"{"name":"foo","arguments":{}}"#, - ); - let out = apply_prompt_tool_calls(resp); - assert_eq!(out.message.tool_calls.len(), 1); - assert_eq!(out.message.tool_calls[0].name, "foo"); - assert!(!out.text().contains(" String { - let mut s = ToolCallStreamScrubber::new(); - let mut out = String::new(); - for f in fragments { - out.push_str(&s.feed(f)); - } - out.push_str(&s.flush()); - out -} - -#[test] -fn scrubber_passes_plain_text_through_unchanged() { - // No markup: the concatenated emissions equal the input exactly. - assert_eq!(scrub_all(&["hello ", "world", " done"]), "hello world done"); -} - -#[test] -fn scrubber_drops_a_complete_block_in_one_fragment() { - let out = scrub_all(&[r#"before {"name":"x","arguments":{}} after"#]); - assert_eq!(out, "before after"); -} - -#[test] -fn scrubber_suppresses_markup_split_across_fragments() { - // The open tag, body, and close arrive in separate fragments — no raw markup - // may appear in any emission, and the surrounding prose survives. - let out = scrub_all(&[ - "answer: ", - "{\"name\":\"x\",", - "\"arguments\":{}} end", - ]); - assert_eq!(out, "answer: end"); - assert!(!out.contains("{\"name\":\"x\",\"arguments\":{}}!"); - assert_eq!(second, "!"); - assert_eq!(s.flush(), ""); -} - -#[test] -fn scrubber_handles_attribute_open_form_split() { - // The Hermes/DeepSeek attribute form `` split mid-tag. - let out = scrub_all(&[ - "ok ", - "{\"name\":\"x\",\"arguments\":{}}", - ]); - assert_eq!(out, "ok "); -} - -#[test] -fn scrubber_handles_deepseek_delimiters_split() { - let out = scrub_all(&[ - "r ", - "<｜tool▁call▁be", - "gin｜>{\"name\":\"x\",\"arguments\":{}}<｜tool▁call▁end｜>", - " s", - ]); - assert_eq!(out, "r s"); - assert!(!out.contains("tool▁call")); -} - -#[test] -fn scrubber_does_not_hold_plural_tool_calls_prose() { - // `` (name not delimiter-terminated) is prose, not an open tag. - assert_eq!( - scrub_all(&["see below"]), - "see below" - ); -} - -#[test] -fn scrubber_flush_surfaces_a_dangling_open_verbatim_untrimmed() { - // A `{"name":"a","arguments":{}} mid {"name":"b","arguments":{"k":1}} tail"#; - let (batch, calls) = parse_prompt_tool_calls_from_text(full); - assert_eq!(calls.len(), 2); - // Fragment the input into single-byte-ish chunks at char boundaries. - let frags: Vec = full.chars().map(|c| c.to_string()).collect(); - let refs: Vec<&str> = frags.iter().map(String::as_str).collect(); - assert_eq!(scrub_all(&refs).trim(), batch); -} - -#[test] -fn apply_prompt_tool_calls_preserves_a_leading_thinking_block() { - // A prompt-guided reasoning model emits a `Thinking` block followed by the - // `` text. Recovering the call must not discard the reasoning. - let mut response = ModelResponse::assistant( - r#"reply {"name":"search","arguments":{"q":"x"}}"#, - ); - response.message.content.insert( - 0, - ContentBlock::Thinking { - text: "chain of thought".to_string(), - signature: None, - }, - ); - - let out = apply_prompt_tool_calls(response); - - assert_eq!(out.message.tool_calls.len(), 1); - assert_eq!(out.message.tool_calls[0].name, "search"); - assert_eq!( - out.message.content[0], - ContentBlock::Thinking { - text: "chain of thought".to_string(), - signature: None, - }, - "the thinking block must survive the content rebuild" - ); - assert_eq!( - out.message.content[1], - ContentBlock::Text("reply".to_string()) - ); -} -// --------------------------------------------------------------------------- -// Bare (undelimited) tool calls -// -// Captured from `llama3.2:3b` via Ollama with `tool_choice: "required"`: the -// model puts the call in `content` instead of the wire's `tool_calls` array, -// with no `` markup and frequently with malformed JSON. -// --------------------------------------------------------------------------- - -#[test] -fn apply_prompt_tool_calls_recovers_a_bare_object_with_relaxed_json() { - // The exact capture: `parameters'` and `{'city'` use mismatched quotes, so - // strict JSON rejects it outright. - let resp = tinyinference_llm::model::ModelResponse::assistant( - r#"{"name":"get_weather","parameters':{'city':"Paris"}}"#, - ); - let out = apply_prompt_tool_calls(resp); - - assert_eq!(out.message.tool_calls.len(), 1); - assert_eq!(out.message.tool_calls[0].name, "get_weather"); - assert_eq!( - out.message.tool_calls[0].arguments, - serde_json::json!({ "city": "Paris" }) - ); - // The raw markup must not also survive as prose, or the user sees the JSON. - assert!( - out.text().is_empty(), - "the consumed object should not remain as text: {}", - out.text() - ); -} - -#[test] -fn apply_prompt_tool_calls_recovers_a_bare_object_inside_a_code_fence() { - let resp = tinyinference_llm::model::ModelResponse::assistant( - "```json\n{\"name\":\"get_weather\",\"arguments\":{\"city\":\"Paris\"}}\n```", - ); - let out = apply_prompt_tool_calls(resp); - - assert_eq!(out.message.tool_calls.len(), 1); - assert_eq!(out.message.tool_calls[0].name, "get_weather"); -} - -#[test] -fn a_tool_call_object_may_name_its_arguments_parameters() { - let resp = tinyinference_llm::model::ModelResponse::assistant( - r#"{"name":"get_weather","parameters":{"city":"Paris"}}"#, - ); - let out = apply_prompt_tool_calls(resp); - - assert_eq!(out.message.tool_calls.len(), 1); - assert_eq!( - out.message.tool_calls[0].arguments, - serde_json::json!({ "city": "Paris" }) - ); -} - -#[test] -fn bare_object_recovery_never_swallows_a_genuine_text_answer() { - // Prose, prose that merely quotes JSON, a JSON object that names no tool, - // and a bare JSON scalar must all pass through untouched. - for text in [ - "The weather in Paris is mild today.", - r#"You could send {"name":"get_weather"} to that endpoint."#, - r#"{"city":"Paris","temperature":17}"#, - r#"{"name":42}"#, - r#""just a string""#, - "[1, 2, 3]", - ] { - let out = apply_prompt_tool_calls(tinyinference_llm::model::ModelResponse::assistant(text)); - assert!( - out.message.tool_calls.is_empty(), - "{text:?} must not be recovered as a tool call" - ); - assert_eq!(out.text(), text, "{text:?} must survive as text"); - } -} - -#[test] -fn bare_tool_call_recovery_preserves_a_thinking_block() { - // A local *reasoning* model emits its chain of thought and then the bare - // call object as the whole visible text. Consuming the object must not take - // the reasoning with it. - let mut response = ModelResponse::assistant(r#"{"name":"search","arguments":{"q":"x"}}"#); - response.message.content.insert( - 0, - ContentBlock::Thinking { - text: "chain of thought".to_string(), - signature: None, - }, - ); - - let out = apply_prompt_tool_calls(response); - - assert_eq!(out.message.tool_calls.len(), 1); - assert_eq!(out.message.tool_calls[0].name, "search"); - assert_eq!( - out.message.content, - vec![ContentBlock::Thinking { - text: "chain of thought".to_string(), - signature: None, - }], - "the reasoning must survive while the consumed object does not" - ); -} - -/// TOOL-2: two turns of the same run must not both mint `call_1`. -/// -/// The recovered id used to be the call's index *within one response*, which -/// resets every turn. A two-turn run therefore produced a transcript with two -/// assistant messages declaring the same tool-call id and two tool messages -/// answering it — a pairing no provider (and no pairing repair) can resolve. -#[test] -fn synthetic_call_ids_are_unique_across_responses() { - let text = r#"{"name":"one","arguments":{}}"#; - let (_, first) = parse_prompt_tool_calls_from_text(text); - let (_, second) = parse_prompt_tool_calls_from_text(text); - - assert_eq!(first.len(), 1); - assert_eq!(second.len(), 1); - assert_ne!( - first[0].id, second[0].id, - "a second turn reused the first turn's synthetic tool-call id" - ); -} - -/// The synthetic scheme must be visibly distinct from real provider ids and -/// from the OpenAI adapter's own positional fallback (`tool-{slot}`), so the -/// two can never collide. -#[test] -fn synthetic_call_ids_do_not_look_like_provider_ids() { - let id = next_synthetic_call_id(1); - assert!(id.starts_with("ptc_"), "{id}"); - assert!(!id.starts_with("call_"), "{id}"); - assert!(!id.starts_with("tool-"), "{id}"); -} - -/// The bare-object recovery path mints ids from the same counter, so a model -/// that alternates between markup and bare objects still cannot collide. -#[test] -fn bare_object_recovery_also_mints_unique_ids() { - let body = r#"{"name":"one","arguments":{}}"#; - let first = apply_prompt_tool_calls(ModelResponse::assistant(body)); - let second = apply_prompt_tool_calls(ModelResponse::assistant(body)); - - let first_id = &first.message.tool_calls[0].id; - let second_id = &second.message.tool_calls[0].id; - assert_ne!(first_id, second_id); -} diff --git a/crates/tinyagents-integration-tests/Cargo.toml b/crates/tinyagents-integration-tests/Cargo.toml index fa21acff..2509d06b 100644 --- a/crates/tinyagents-integration-tests/Cargo.toml +++ b/crates/tinyagents-integration-tests/Cargo.toml @@ -26,7 +26,7 @@ tinyagents-registry = { path = "../tinyagents-registry" } tinyagents-session = { path = "../tinyagents-session" } tinyinference-llm = { path = "../../vendor/tinyinference/crates/tinyinference-llm", version = "0.3.0" } tinyinference-embeddings = { path = "../../vendor/tinyinference/crates/tinyinference-embeddings", version = "0.3.0" } -tinytools = { path = "../../vendor/tinytools/crates/tinytools", version = "0.2.0" } +tinytools = { path = "../../vendor/tinytools/crates/tinytools", version = "0.3.0" } tokio = { version = "1", features = ["macros", "rt-multi-thread", "time", "test-util"] } [features] diff --git a/crates/tinyagents-integration-tests/tests/dependency_boundary.rs b/crates/tinyagents-integration-tests/tests/dependency_boundary.rs index e6da1e9c..8d264908 100644 --- a/crates/tinyagents-integration-tests/tests/dependency_boundary.rs +++ b/crates/tinyagents-integration-tests/tests/dependency_boundary.rs @@ -135,23 +135,23 @@ const KNOWN_GENERIC_CLAUDE_CODE_CHAT_MESSAGE_DEBT: &[(&str, usize)] = &[ ), ( "crates/tinyagents-harness/src/providers/claude_code/mod.rs", - 55, + 51, ), ( "crates/tinyagents-harness/src/providers/claude_code/mod.rs", - 220, + 219, ), ( "crates/tinyagents-harness/src/providers/claude_code/mod.rs", - 283, + 282, ), ( "crates/tinyagents-harness/src/providers/claude_code/mod.rs", - 326, + 325, ), ( "crates/tinyagents-harness/src/providers/claude_code/mod.rs", - 349, + 348, ), ( "crates/tinyagents-harness/src/providers/claude_code/mod_tests.rs", diff --git a/crates/tinyagents-integration-tests/tests/e2e_tool_dialects.rs b/crates/tinyagents-integration-tests/tests/e2e_tool_dialects.rs new file mode 100644 index 00000000..6e68b169 --- /dev/null +++ b/crates/tinyagents-integration-tests/tests/e2e_tool_dialects.rs @@ -0,0 +1,1090 @@ +//! End-to-end coverage for tool dialects through the harness. +//! +//! The protocol crate (`tinytools-agent`) owns how a call is rendered and +//! parsed; these tests pin the *host* half: a native model that narrates a +//! call as text — in any grammar — still dispatches it with a harness-minted +//! id; a forced text dialect strips the schemas off the wire and renders the +//! protocol instead; streamed text never shows tool-call markup to a +//! consumer; and a P-Format run parses positional calls. + +use std::sync::{Arc, Mutex}; + +use async_trait::async_trait; +use serde_json::json; + +use tinyagents_harness::config::ToolDispatcher; +use tinyagents_harness::context::RunContext; +use tinyagents_harness::events::{AgentEvent, RecordingListener}; +use tinyagents_harness::middleware::Middleware; +use tinyagents_harness::runtime::{AgentHarness, RunPolicy}; +use tinyagents_harness::testkit::{FakeTool, ScriptedModel, StreamingMock}; +use tinyinference_llm::message::{Message, MessageDelta}; +use tinyinference_llm::model::{ + ChatModel, ModelDelta, ModelProfile, ModelRequest, ModelResponse, ModelStreamItem, + ResponseFormat, ToolChoice, +}; +use tinyinference_llm::providers::MockModel; +use tinyinference_llm::tool::ToolCall; +use tinytools::{Tool, ToolResult}; + +struct CaptureMiddleware { + listener: Arc, +} + +#[async_trait] +impl Middleware<(), ()> for CaptureMiddleware { + fn name(&self) -> &str { + "capture" + } + + async fn before_agent( + &self, + ctx: &mut RunContext<()>, + _state: &(), + ) -> tinyagents_harness::Result<()> { + ctx.events.subscribe(self.listener.clone()); + Ok(()) + } +} + +/// A tool with a real parameter, so P-Format has a slot to render. +struct Lookup; + +#[async_trait] +impl Tool for Lookup { + fn name(&self) -> &str { + "lookup" + } + + fn description(&self) -> &str { + "Looks something up." + } + + fn parameters_schema(&self) -> serde_json::Value { + json!({ + "type": "object", + "properties": { "q": { "type": "string" } }, + "required": ["q"] + }) + } + + async fn execute(&self, _arguments: serde_json::Value) -> anyhow::Result { + Ok(ToolResult::success("tool-output")) + } +} + +/// Every tool-call id the run dispatched, from the tool-started events. +fn dispatched_ids(listener: &RecordingListener) -> Vec { + listener + .events() + .into_iter() + .filter_map(|record| match record.event { + AgentEvent::ToolStarted { call_id, .. } => Some(call_id.to_string()), + _ => None, + }) + .collect() +} + +/// A native-profile model that answers with text only, once, then finishes. +fn narrating_model(text: &str) -> MockModel { + MockModel::with_responses(vec![ + ModelResponse::assistant(text), + ModelResponse::assistant("done"), + ]) +} + +fn harness_with( + model: Arc>, + listener: &Arc, +) -> AgentHarness<()> { + let mut harness: AgentHarness<()> = AgentHarness::new(); + harness + .register_model("mock", model) + .set_default_model("mock") + .register_tool(Arc::new(FakeTool::returning("lookup", "tool-output"))) + .push_middleware(Arc::new(CaptureMiddleware { + listener: listener.clone(), + })); + harness +} + +#[tokio::test] +async fn a_native_model_narrating_a_call_in_any_grammar_dispatches_it() { + for text in [ + "Let me check. {\"name\":\"lookup\",\"arguments\":{\"q\":\"x\"}}", + "<｜DSML｜tool_calls><｜DSML｜invoke name=\"lookup\">{\"q\":\"x\"}", + "<｜tool▁call▁begin｜>lookup<｜tool▁sep｜>{\"q\":\"x\"}<｜tool▁call▁end｜>", + "<|channel|>commentary to=functions.lookup<|message|>{\"q\":\"x\"}<|call|>", + "{\"name\":\"functions.lookup\",\"arguments\":{\"q\":\"x\"}}", + ] { + let listener = Arc::new(RecordingListener::new()); + let harness = harness_with(Arc::new(narrating_model(text)), &listener); + let run = harness + .invoke_default(&(), vec![Message::user("go")]) + .await + .expect("run succeeds"); + assert_eq!(run.tool_calls, 1, "{text}"); + let ids = dispatched_ids(&listener); + assert_eq!(ids.len(), 1, "{text}"); + assert!( + ids[0].ends_with("-tool-1"), + "harness-minted id, got {}: {text}", + ids[0] + ); + } +} + +#[tokio::test] +async fn an_unknown_narrated_tool_is_not_invented_into_a_known_one() { + let listener = Arc::new(RecordingListener::new()); + let harness = harness_with( + Arc::new(narrating_model( + "{\"name\":\"launch_missiles\",\"arguments\":{}}", + )), + &listener, + ); + let run = harness + .invoke_default(&(), vec![Message::user("go")]) + .await + .expect("run completes under the default unknown-tool policy"); + // The name reaches the unknown-tool policy as written; it is never + // fuzzed onto the one registered tool. + let started: Vec = listener + .events() + .into_iter() + .filter_map(|record| match record.event { + AgentEvent::ToolStarted { tool_name, .. } => Some(tool_name), + _ => None, + }) + .collect(); + assert!(!started.iter().any(|name| name == "lookup"), "{started:?}"); + assert!(run.model_calls >= 1); +} + +#[tokio::test] +async fn a_narrated_call_is_not_dispatched_when_tool_choice_is_forced_to_none() { + // `apply_to_request` already skips its own dialect rewrite for + // `ToolChoice::None`, but that alone did not stop recovery: the offered + // tool names were still recorded for the scrubber/`recover_text_calls` + // regardless of the effective choice, so a model that narrated + // `` markup as plain text anyway still had it parsed and + // dispatched as a real, side-effecting call despite the caller's + // explicit "no tool calls this turn". + let listener = Arc::new(RecordingListener::new()); + let mut harness = harness_with( + Arc::new(narrating_model( + "{\"name\":\"lookup\",\"arguments\":{\"q\":\"x\"}}", + )), + &listener, + ); + harness.push_middleware(Arc::new(ForceToolChoice(ToolChoice::None))); + + let run = harness + .invoke_default(&(), vec![Message::user("go")]) + .await + .expect("run completes"); + + assert_eq!( + run.tool_calls, 0, + "a narrated call must not be dispatched when the effective tool_choice is None" + ); + let started: Vec = listener + .events() + .into_iter() + .filter_map(|record| match record.event { + AgentEvent::ToolStarted { tool_name, .. } => Some(tool_name), + _ => None, + }) + .collect(); + assert!(started.is_empty(), "{started:?}"); +} + +#[tokio::test] +async fn a_forced_xml_dialect_renders_the_protocol_and_sends_no_schemas() { + let model = Arc::new(ScriptedModel::replies(vec![ + "{\"name\":\"lookup\",\"arguments\":{\"q\":\"x\"}}", + "done", + ])); + let listener = Arc::new(RecordingListener::new()); + let mut harness = harness_with(model.clone(), &listener); + harness.with_policy(RunPolicy { + tool_dialect: ToolDispatcher::Xml, + ..RunPolicy::default() + }); + + let run = harness + .invoke_default(&(), vec![Message::user("go")]) + .await + .expect("run succeeds"); + assert_eq!(run.tool_calls, 1); + + let requests = model.requests(); + assert_eq!(requests.len(), 2); + for request in &requests { + assert!(request.tools.is_empty(), "no schema goes on the wire"); + let system = request + .messages + .iter() + .find(|m| matches!(m, Message::System(_))) + .expect("a system turn carries the protocol") + .text(); + assert!(system.contains("## Tool Use Protocol")); + assert!(system.contains("**lookup**")); + } + // The second request replays the call as text and folds the result. + let replay = &requests[1]; + let assistant = replay + .messages + .iter() + .find(|m| matches!(m, Message::Assistant(_))) + .expect("assistant turn replayed") + .text(); + assert!(assistant.contains(""), "{assistant}"); + assert!( + replay + .messages + .iter() + .any(|m| m.text().contains("lookup[0|needle]", + "done", + ])); + let listener = Arc::new(RecordingListener::new()); + let mut harness: AgentHarness<()> = AgentHarness::new(); + harness + .register_model("mock", model.clone()) + .set_default_model("mock") + .register_tool(Arc::new(Lookup)) + .push_middleware(Arc::new(CaptureMiddleware { + listener: listener.clone(), + })) + .with_policy(RunPolicy { + tool_dialect: ToolDispatcher::Pformat, + ..RunPolicy::default() + }); + + let run = harness + .invoke_default(&(), vec![Message::user("go")]) + .await + .expect("run succeeds"); + assert_eq!(run.tool_calls, 1); + let system = model.requests()[0] + .messages + .iter() + .find(|m| matches!(m, Message::System(_))) + .expect("system") + .text(); + assert!(system.contains("P-Format"), "{system}"); + assert!(system.contains("lookup[0|]"), "{system}"); +} + +/// Middleware that forces `tool_choice` before the dialect rewrite runs, the +/// same shape a caller or another middleware forcing a specific tool would +/// produce. +struct ForceToolChoice(ToolChoice); + +#[async_trait] +impl Middleware<(), ()> for ForceToolChoice { + fn name(&self) -> &str { + "force-tool-choice" + } + + async fn before_model( + &self, + _ctx: &mut RunContext<()>, + _state: &(), + request: &mut ModelRequest, + ) -> tinyagents_harness::Result<()> { + request.tool_choice = self.0.clone(); + Ok(()) + } +} + +#[tokio::test] +async fn a_forced_pformat_dialect_preserves_a_forced_required_tool_choice() { + // Unlike the XML branch (`prompt_tools::with_tool_instructions`, which + // renders `tool_choice` into its instructions), P-Format has no schema on + // the wire either — a forced choice has to survive as plain English in + // the rendered prompt or it silently loses its meaning once the wire + // `tool_choice` is reset to `Auto`. + let model = Arc::new(ScriptedModel::replies(vec![ + "lookup[0|needle]", + "done", + ])); + let listener = Arc::new(RecordingListener::new()); + let mut harness: AgentHarness<()> = AgentHarness::new(); + harness + .register_model("mock", model.clone()) + .set_default_model("mock") + .register_tool(Arc::new(Lookup)) + .push_middleware(Arc::new(CaptureMiddleware { + listener: listener.clone(), + })) + .push_middleware(Arc::new(ForceToolChoice(ToolChoice::Required))) + .with_policy(RunPolicy { + tool_dialect: ToolDispatcher::Pformat, + ..RunPolicy::default() + }); + + let run = harness + .invoke_default(&(), vec![Message::user("go")]) + .await + .expect("run succeeds"); + assert_eq!(run.tool_calls, 1); + + let system = model.requests()[0] + .messages + .iter() + .find(|m| matches!(m, Message::System(_))) + .expect("system") + .text(); + assert!( + system.contains("You must emit at least one tool call."), + "{system}" + ); + // The wire choice is reset to `Auto` (no schema is on the wire for a + // text dialect), so this asserts the prompt carries the constraint + // instead, not that the wire field kept it. + assert_eq!(model.requests()[0].tool_choice, ToolChoice::Auto); +} + +#[tokio::test] +async fn a_forced_pformat_dialect_preserves_a_forced_named_tool_choice() { + let model = Arc::new(ScriptedModel::replies(vec![ + "lookup[0|needle]", + "done", + ])); + let listener = Arc::new(RecordingListener::new()); + let mut harness: AgentHarness<()> = AgentHarness::new(); + harness + .register_model("mock", model.clone()) + .set_default_model("mock") + .register_tool(Arc::new(Lookup)) + .push_middleware(Arc::new(CaptureMiddleware { + listener: listener.clone(), + })) + .push_middleware(Arc::new(ForceToolChoice(ToolChoice::Tool( + "lookup".to_string(), + )))) + .with_policy(RunPolicy { + tool_dialect: ToolDispatcher::Pformat, + ..RunPolicy::default() + }); + + let run = harness + .invoke_default(&(), vec![Message::user("go")]) + .await + .expect("run succeeds"); + assert_eq!(run.tool_calls, 1); + + let system = model.requests()[0] + .messages + .iter() + .find(|m| matches!(m, Message::System(_))) + .expect("system") + .text(); + assert!( + system.contains("You must call the `lookup` tool."), + "{system}" + ); +} + +/// Middleware recording every visible text delta the harness emits. +struct DeltaRecorder { + seen: Arc>>, +} + +#[async_trait] +impl Middleware<(), ()> for DeltaRecorder { + fn name(&self) -> &str { + "deltas" + } + + async fn on_model_delta( + &self, + _ctx: &mut RunContext<()>, + _state: &(), + delta: &mut ModelDelta, + ) -> tinyagents_harness::Result<()> { + self.seen.lock().unwrap().push(delta.content.clone()); + Ok(()) + } +} + +/// Middleware that redacts every occurrence of `"too"` from a visible delta, +/// standing in for any redaction/policy/transformation middleware a host +/// might install on [`Middleware::on_model_delta`]. +struct RedactMiddleware; + +#[async_trait] +impl Middleware<(), ()> for RedactMiddleware { + fn name(&self) -> &str { + "redact" + } + + async fn on_model_delta( + &self, + _ctx: &mut RunContext<()>, + _state: &(), + delta: &mut ModelDelta, + ) -> tinyagents_harness::Result<()> { + delta.content = delta.content.replace("too", "REDACTED"); + Ok(()) + } +} + +#[tokio::test] +async fn a_flushed_stream_tail_that_was_only_a_marker_false_alarm_still_hits_delta_middleware() { + // A fragment such as ` for SuppressAllMiddleware { + fn name(&self) -> &str { + "suppress-all" + } + + async fn on_model_delta( + &self, + _ctx: &mut RunContext<()>, + _state: &(), + delta: &mut ModelDelta, + ) -> tinyagents_harness::Result<()> { + delta.content.clear(); + Ok(()) + } +} + +#[tokio::test] +async fn a_fully_suppressed_flushed_tail_does_not_restore_raw_provider_content() { + // The whole stream is a single fragment that looks like it could be + // opening a tool-call marker (`{\"name\":\"lookup\",", + "\"arguments\":{\"q\":\"x\"}}", + " checking.", + ]; + let full: String = chunks.concat(); + let mut items = vec![ModelStreamItem::Started]; + items.extend( + chunks + .iter() + .map(|chunk| ModelStreamItem::MessageDelta(MessageDelta::text(*chunk))), + ); + items.push(ModelStreamItem::Completed(ModelResponse::assistant(full))); + // The scripted stream replays the same call every turn; one model call is + // enough to observe the dispatch and the scrubbed deltas. + let model = Arc::new(StreamingMock::new(items)); + let seen = Arc::new(Mutex::new(Vec::new())); + let listener = Arc::new(RecordingListener::new()); + let mut harness = harness_with(model, &listener); + harness + .push_middleware(Arc::new(DeltaRecorder { seen: seen.clone() })) + .with_policy(RunPolicy { + limits: tinyagents_harness::limits::RunLimits { + max_model_calls: 1, + ..tinyagents_harness::limits::RunLimits::default() + }, + ..RunPolicy::default() + }); + + let _ = harness + .invoke_streaming_default(&(), vec![Message::user("go")]) + .await; + + let deltas = seen.lock().unwrap().clone(); + let joined = deltas.concat(); + assert!(!joined.contains(""), + "markup leaked: {deltas:?}" + ); + assert!(joined.contains("Sure, "), "{deltas:?}"); + assert!(joined.contains(" checking."), "{deltas:?}"); + let ids = dispatched_ids(&listener); + assert_eq!(ids.len(), 1, "the scrubbed call still dispatches once"); + assert!(ids[0].ends_with("-tool-1")); +} + +#[tokio::test] +async fn a_pure_tool_call_stream_leaves_no_raw_markup_in_the_terminal_response() { + // A response that is *only* tool-call markup, with no ordinary text + // around it, suppresses every delta (the scrubber holds all of it back), + // so terminal-content reconciliation must not depend on having seen any + // *ordinary* streamed text — only on the scrubber having recovered a + // call. Otherwise the raw `` text produced by the provider + // (not the scrubbed one) survives in the terminal response's content + // block, gets persisted into the transcript, and is replayed to the + // model on the very next turn alongside the structured call. + let markup = "{\"name\":\"lookup\",\"arguments\":{\"q\":\"x\"}}"; + let items = vec![ + ModelStreamItem::Started, + ModelStreamItem::MessageDelta(MessageDelta::text(markup)), + ModelStreamItem::Completed(ModelResponse::assistant(markup)), + ]; + let model = Arc::new(StreamingMock::new(items)); + let seen = Arc::new(Mutex::new(Vec::new())); + let listener = Arc::new(RecordingListener::new()); + let mut harness = harness_with(model, &listener); + harness + .push_middleware(Arc::new(DeltaRecorder { seen: seen.clone() })) + .with_policy(RunPolicy { + limits: tinyagents_harness::limits::RunLimits { + max_model_calls: 1, + behavior: tinyagents_harness::limits::LimitBehavior::StopWithPartial, + ..tinyagents_harness::limits::RunLimits::default() + }, + ..RunPolicy::default() + }); + + let run = harness + .invoke_streaming_default(&(), vec![Message::user("go")]) + .await + .expect("run stops cleanly with the partial transcript at the call cap"); + + let deltas = seen.lock().unwrap().clone(); + let joined = deltas.concat(); + assert!(joined.is_empty(), "no ordinary text streamed: {deltas:?}"); + + let ids = dispatched_ids(&listener); + assert_eq!(ids.len(), 1, "the scrubbed call still dispatches once"); + + // The persisted transcript must not carry the raw markup anywhere, + // including on the assistant turn the terminal response became. + for message in &run.messages { + assert!( + !message.text().contains(""; + let mut mixed = ModelResponse::assistant(markup); + mixed + .message + .tool_calls + .push(ToolCall::new("native-1", "lookup", json!({"q": "x"}))); + let model = Arc::new(ScriptedModel::new(vec![ + mixed, + ModelResponse::assistant("done"), + ])); + let listener = Arc::new(RecordingListener::new()); + let mut harness: AgentHarness<()> = AgentHarness::new(); + harness + .register_model("mock", model.clone()) + .set_default_model("mock") + .register_tool(Arc::new(FakeTool::returning("lookup", "lookup-output"))) + .register_tool(Arc::new(FakeTool::returning("second", "second-output"))) + .push_middleware(Arc::new(CaptureMiddleware { + listener: listener.clone(), + })); + + let run = harness + .invoke_default(&(), vec![Message::user("go")]) + .await + .expect("run succeeds"); + + assert_eq!(run.tool_calls, 2, "{:?}", run.messages); + let started: Vec = listener + .events() + .into_iter() + .filter_map(|record| match record.event { + AgentEvent::ToolStarted { tool_name, .. } => Some(tool_name), + _ => None, + }) + .collect(); + assert!(started.contains(&"lookup".to_string()), "{started:?}"); + assert!(started.contains(&"second".to_string()), "{started:?}"); +} + +#[tokio::test] +async fn a_native_call_and_a_narrated_text_call_are_both_dispatched_when_streamed() { + // The streaming counterpart of the non-streaming test above: the + // `DeltaScrubber`-recovered call attach point in `model_call.rs` had the + // identical bug (gated on `tool_calls.is_empty()`), and fixing only + // `recover_text_calls` would not cover it — by the time the terminal + // response reaches `recover_text_calls`, the streaming reconciliation + // path has already scrubbed the narrated markup out of the visible + // text, so there is nothing left in `response.text()` for + // `recover_text_calls` to recover a second time. + let markup = "{\"name\":\"second\",\"arguments\":{\"q\":\"y\"}}"; + let mut completed = ModelResponse::assistant(markup); + completed + .message + .tool_calls + .push(ToolCall::new("native-1", "lookup", json!({"q": "x"}))); + let items = vec![ + ModelStreamItem::Started, + ModelStreamItem::MessageDelta(MessageDelta::text(markup)), + ModelStreamItem::Completed(completed), + ]; + let model = Arc::new(StreamingMock::new(items)); + let listener = Arc::new(RecordingListener::new()); + let mut harness: AgentHarness<()> = AgentHarness::new(); + harness + .register_model("mock", model.clone()) + .set_default_model("mock") + .register_tool(Arc::new(FakeTool::returning("lookup", "lookup-output"))) + .register_tool(Arc::new(FakeTool::returning("second", "second-output"))) + .push_middleware(Arc::new(CaptureMiddleware { + listener: listener.clone(), + })) + .with_policy(RunPolicy { + limits: tinyagents_harness::limits::RunLimits { + max_model_calls: 1, + behavior: tinyagents_harness::limits::LimitBehavior::StopWithPartial, + ..tinyagents_harness::limits::RunLimits::default() + }, + ..RunPolicy::default() + }); + + let run = harness + .invoke_streaming_default(&(), vec![Message::user("go")]) + .await + .expect("run stops cleanly"); + + assert_eq!(run.tool_calls, 2, "{:?}", run.messages); + let started: Vec = listener + .events() + .into_iter() + .filter_map(|record| match record.event { + AgentEvent::ToolStarted { tool_name, .. } => Some(tool_name), + _ => None, + }) + .collect(); + assert!(started.contains(&"lookup".to_string()), "{started:?}"); + assert!(started.contains(&"second".to_string()), "{started:?}"); +} + +#[tokio::test] +async fn a_signalled_but_missing_tool_call_is_re_prompted_then_recovered() { + let mut promised = ModelResponse::assistant(""); + promised.finish_reason = Some("tool_calls".into()); + let model = Arc::new(ScriptedModel::new(vec![ + promised, + ModelResponse::assistant( + "{\"name\":\"lookup\",\"arguments\":{\"q\":\"x\"}}", + ), + ModelResponse::assistant("done"), + ])); + let listener = Arc::new(RecordingListener::new()); + let harness = harness_with(model.clone(), &listener); + + let run = harness + .invoke_default(&(), vec![Message::user("go")]) + .await + .expect("run succeeds"); + + assert_eq!(run.model_calls, 3, "one nudge, one call, one final"); + assert_eq!(run.tool_calls, 1); + let second = &model.requests()[1]; + let last = second.messages.last().expect("nudge appended").text(); + assert!(last.contains("issue the actual tool call now"), "{last}"); +} + +#[tokio::test] +async fn dropped_tool_call_nudges_are_bounded() { + let mut promised = ModelResponse::assistant(""); + promised.finish_reason = Some("tool_calls".into()); + let model = Arc::new(ScriptedModel::new(vec![ + promised.clone(), + promised.clone(), + promised.clone(), + promised, + ])); + let listener = Arc::new(RecordingListener::new()); + let harness = harness_with(model, &listener); + + let run = harness + .invoke_default(&(), vec![Message::user("go")]) + .await + .expect("run ends instead of looping"); + assert_eq!( + run.model_calls, 4, + "three nudges, then the answer is taken as final" + ); + assert_eq!(run.tool_calls, 0); +} + +#[tokio::test] +async fn no_dropped_call_nudge_is_issued_when_the_turn_could_not_accept_a_tool_call() { + // A provider/router can report `finish_reason == "tool_calls"` with no + // actual call even when this turn's effective `tool_choice` is `None` + // (set by a `before_model` middleware) — nudging the model to "issue the + // call" in that situation asks for something that could never have been + // accepted, wasting `dropped_tool_call_nudges` model calls before + // falling through to the same terminal outcome a single call would have + // reached immediately. + let mut promised = ModelResponse::assistant(""); + promised.finish_reason = Some("tool_calls".into()); + let model = Arc::new(ScriptedModel::new(vec![promised])); + let listener = Arc::new(RecordingListener::new()); + let mut harness = harness_with(model, &listener); + harness.push_middleware(Arc::new(ForceToolChoice(ToolChoice::None))); + + let run = harness + .invoke_default(&(), vec![Message::user("go")]) + .await + .expect("run ends on the first call instead of nudging"); + assert_eq!( + run.model_calls, 1, + "no nudge should be spent on a turn that could not accept a tool call" + ); + let nudges = listener + .events() + .into_iter() + .filter(|record| matches!(record.event, AgentEvent::RetryScheduled { .. })) + .count(); + assert_eq!(nudges, 0, "{:?}", listener.events()); +} + +/// A queued model with a fixed, caller-chosen profile, so a test can force +/// `StructuredStrategy::ToolCall` (a profile with `tool_calling` but not +/// `native_structured_output && json_schema`) while still scripting a +/// specific sequence of responses. [`ScriptedModel`] cannot do this: it +/// advertises no profile at all, which `StructuredStrategy::for_profile` +/// resolves to `ProviderSchema`, not `ToolCall`. +struct ProfiledScriptedModel { + profile: ModelProfile, + queue: Mutex>, + received: Mutex>, +} + +impl ProfiledScriptedModel { + fn new(profile: ModelProfile, responses: Vec) -> Self { + Self { + profile, + queue: Mutex::new(responses.into()), + received: Mutex::new(Vec::new()), + } + } + + /// Every request received so far, in order. + fn requests(&self) -> Vec { + self.received.lock().unwrap().clone() + } +} + +#[async_trait] +impl ChatModel<()> for ProfiledScriptedModel { + fn profile(&self) -> Option<&ModelProfile> { + Some(&self.profile) + } + + async fn invoke( + &self, + _state: &(), + request: ModelRequest, + ) -> tinyinference_llm::Result { + self.received.lock().unwrap().push(request); + self.queue + .lock() + .unwrap() + .pop_front() + .ok_or_else(|| tinyinference_llm::Error::Model("queue exhausted".to_string())) + } +} + +/// Builds an assistant response carrying several tool calls in one turn, with +/// no text. +fn multi_tool_call_response(calls: Vec<(&str, &str)>) -> ModelResponse { + let mut response = ModelResponse::assistant(""); + response.finish_reason = Some("tool_calls".to_string()); + response.message.tool_calls = calls + .into_iter() + .map(|(id, name)| ToolCall::new(id, name, json!({}))) + .collect(); + response +} + +#[tokio::test] +async fn dropped_call_nudge_budget_resets_after_a_mixed_structured_and_tool_turn() { + // Regression: the mixed-turn branch (a structured payload alongside real + // tool calls in the same response) runs its real tools and continues the + // loop, but — unlike the ordinary tool-calling path and the + // dropped-call-recovered path, both of which do — it used to leave + // `dropped_tool_call_nudges_used` unreset. A nudge spent before a mixed + // turn would then leak into a later, unrelated dropped-call turn and + // receive fewer than the policy's configured number of re-prompts. + let mut promised = ModelResponse::assistant(""); + promised.finish_reason = Some("tool_calls".into()); + + let profile = ModelProfile { + tool_calling: true, + native_structured_output: false, + json_schema: false, + ..ModelProfile::default() + }; + let model = Arc::new(ProfiledScriptedModel::new( + profile, + vec![ + promised.clone(), // dropped call #1: spends the only nudge. + multi_tool_call_response(vec![("s1", "answer"), ("t1", "lookup")]), // mixed turn: must reset the nudge budget. + promised, // dropped call #2: must be nudged again, not treated + // as already out of budget. + multi_tool_call_response(vec![("s2", "answer")]), // final: satisfies structured extraction. + ], + )); + let listener = Arc::new(RecordingListener::new()); + let mut harness: AgentHarness<()> = AgentHarness::new(); + harness + .register_model("mock", model.clone()) + .set_default_model("mock") + .register_tool(Arc::new(FakeTool::returning("lookup", "tool-output"))) + .push_middleware(Arc::new(CaptureMiddleware { + listener: listener.clone(), + })) + .with_policy(RunPolicy { + dropped_tool_call_nudges: 1, + default_response_format: Some(ResponseFormat::auto( + "answer", + json!({"type": "object"}), + )), + ..RunPolicy::default() + }); + + let run = harness + .invoke_default(&(), vec![Message::user("go")]) + .await + .expect("both dropped-call turns are recoverable under their own nudge budget"); + + assert_eq!( + run.model_calls, 4, + "dropped -> nudged -> mixed turn -> dropped -> nudged again -> final" + ); + let nudges = listener + .events() + .into_iter() + .filter(|record| matches!(record.event, AgentEvent::RetryScheduled { .. })) + .count(); + assert_eq!( + nudges, 2, + "each dropped-call turn gets its own full nudge budget, proving the \ + mixed turn reset the counter rather than leaving it spent" + ); +} + +#[tokio::test] +async fn auto_dialect_falls_back_to_xml_for_a_model_that_cannot_make_native_tool_calls() { + // `ToolDispatcher::Auto` is documented as "provider-native tool calls + // when the provider supports them, otherwise Xml". Resolving Auto to the + // host-side no-op `RunDialect::Native` unconditionally (regardless of + // the resolved model's capability) left that fallback unenforced: a + // model with `tool_calling: false` would receive a request that kept + // depending on provider-native tools, with no host-rendered text + // protocol to fall back to. + let model = Arc::new(ProfiledScriptedModel::new( + ModelProfile { + tool_calling: false, + ..ModelProfile::default() + }, + vec![ + ModelResponse::assistant( + "{\"name\":\"lookup\",\"arguments\":{\"q\":\"x\"}}", + ), + ModelResponse::assistant("done"), + ], + )); + let listener = Arc::new(RecordingListener::new()); + let mut harness: AgentHarness<()> = AgentHarness::new(); + harness + .register_model("mock", model.clone()) + .set_default_model("mock") + .register_tool(Arc::new(FakeTool::returning("lookup", "tool-output"))) + .push_middleware(Arc::new(CaptureMiddleware { + listener: listener.clone(), + })); + // Default policy: `tool_dialect` defaults to `Auto`. + + let run = harness + .invoke_default(&(), vec![Message::user("go")]) + .await + .expect("run succeeds"); + assert_eq!(run.tool_calls, 1); + + let requests = model.requests(); + assert!(!requests.is_empty()); + let first = &requests[0]; + assert!( + first.tools.is_empty(), + "Auto must fall back to the text dialect (no schema on the wire) \ + for a model that cannot make native tool calls" + ); + let system = first + .messages + .iter() + .find(|m| matches!(m, Message::System(_))) + .expect("a system turn carries the protocol") + .text(); + assert!(system.contains("Tool Use Protocol"), "{system}"); +} + +#[tokio::test] +async fn a_terminal_only_stream_with_no_preceding_deltas_still_recovers_the_call() { + // A provider may emit a single `Completed` item with no preceding + // `MessageDelta`s at all (e.g. a short response sent in one frame). The + // per-delta `DeltaScrubber` in `model_call.rs` never sees this text, so + // it cannot flag it as recovered — but that scrubber is not the only + // recovery path: `run_loop.rs` unconditionally runs + // `dialect::recover_text_calls` on the returned response afterward, + // regardless of whether anything streamed. This pins that second pass as + // the safety net for exactly this case, rather than assuming (as a + // superficial read of `model_call.rs` alone might) that terminal-only + // content without any preceding delta is unrecoverable. + let text = "{\"name\":\"lookup\",\"arguments\":{\"q\":\"x\"}}"; + let items = vec![ + ModelStreamItem::Started, + ModelStreamItem::Completed(ModelResponse::assistant(text)), + ]; + let model = Arc::new(StreamingMock::new(items)); + let listener = Arc::new(RecordingListener::new()); + let mut harness = harness_with(model, &listener); + harness.with_policy(RunPolicy { + limits: tinyagents_harness::limits::RunLimits { + max_model_calls: 1, + behavior: tinyagents_harness::limits::LimitBehavior::StopWithPartial, + ..tinyagents_harness::limits::RunLimits::default() + }, + ..RunPolicy::default() + }); + + let run = harness + .invoke_streaming_default(&(), vec![Message::user("go")]) + .await + .expect("run stops cleanly"); + + assert_eq!(run.tool_calls, 1, "{:?}", run.messages); + let assistant = run + .messages + .iter() + .find(|m| matches!(m, Message::Assistant(_))) + .expect("assistant turn"); + assert!( + !assistant.text().contains("` in every spelling, DeepSeek DSML and R1 markup, Kimi sentinels, +gpt-oss Harmony and Mistral blocks — see +[tool-dialect.md](tool-dialect.md). ### Invalid arguments abort the run by default diff --git a/docs/modules/harness/tool-dialect.md b/docs/modules/harness/tool-dialect.md index aebb4b75..67fa36a4 100644 --- a/docs/modules/harness/tool-dialect.md +++ b/docs/modules/harness/tool-dialect.md @@ -1,6 +1,66 @@ # Tool Dialects -Canonical API: `tinytools_agent::dialect` from the vendored TinyTools workspace. +Canonical API: `tinytools_agent` from the vendored TinyTools workspace — +`parse`, `repair`, `stream`, `render`, and `dialect`. The harness owns only the +host half, in `agent_loop/dialect.rs`. + +## Who owns what + +| Concern | Owner | +| --- | --- | +| Grammars a model may write a call in (`` spellings, Claude / DeepSeek DSML ``, DeepSeek-R1 and Kimi sentinel tokens, gpt-oss Harmony, Mistral `[TOOL_CALLS]`, GLM lines, bare JSON, P-Format) | `tinytools_agent::parse` — one file per grammar under `parse/grammar/` | +| JSON, tool-name, and argument-shape repair | `tinytools_agent::repair` | +| Scrubbing markup from a live text stream | `tinytools_agent::stream::StreamScrubber` | +| Protocol block, catalogue, `` envelope, replay | `tinytools_agent::render` | +| Mapping `tinyinference_llm::Message` onto the text protocol; the OpenAI-compatible adapter's own prompt-guided mode | `tinyinference_llm::prompt_tools` | +| Which dialect a run speaks, minting call ids, argument validation policy, the unknown-tool policy, re-prompt nudges | `tinyagents_harness` (`agent_loop/dialect.rs`, `RunPolicy`) | + +A model-specific marker string appears in exactly one grammar file. If a +consumer finds itself matching one, that is a bug to fix in `tinytools-agent`, +where every consumer — this harness, the inference adapters, any host loop — +picks the fix up. + +## Selecting a dialect + +`RunPolicy::tool_dialect` (a `ToolDispatcher`) is resolved once per run: + +| Value | Request | Response | +| --- | --- | --- | +| `Auto` / `Native` | schemas on the wire; the provider adapter decides (the OpenAI-compatible adapter switches to the JSON protocol by itself for a profile without native tool calling, or after a "tools unsupported" 400) | structured calls, else every text grammar as a fallback | +| `Xml` | the transcript is folded into text forms (assistant calls → `` markup, `tool` results → one `[Tool results]` turn), a continuation user turn is inserted when no user query is resolvable, the JSON protocol block plus catalogue goes into the system prompt, **no** schema goes on the wire | every text grammar | +| `Pformat` | as `Xml`, with the P-Format block and signature catalogue | every text grammar, with the positional registry built from the run's schemas | + +Whatever the dialect, a response carrying no structured call is read through +every grammar with the offered tool names supplied, so a damaged name +(`terminal" parameter=…`, `functions.read_file`, `Read File`) resolves to the +offered tool and an unknown one reaches the unknown-tool policy as written. + +## Ids and streaming + +`tinytools-agent` never mints call ids. The harness mints +`{model_call_id}-tool-{n}` for every call recovered from text — unique per run +by construction and visibly distinct from any provider's. (The +OpenAI-compatible adapter mints `text-{seq}-{slot}` for calls it recovers +itself; the harness leaves those alone.) + +Streamed visible text passes through a `StreamScrubber` whenever tools were +offered, so a consumer of `AgentEvent::ModelDelta` never sees a partial +``. Calls the scrubber completes surface on the terminal response, +exactly once; the reconciled terminal text is the scrubbed text. + +## Dropped tool calls + +A response with `finish_reason == "tool_calls"` and no call — structured or +recoverable — is re-prompted with a one-line nudge, at most +`RunPolicy::dropped_tool_call_nudges` (default 3) times in a row. Each nudge is +a model call and counts against `RunLimits::max_model_calls`. + +## Two pairing repairs, deliberately + +`tinytools_agent::dialect::pair_tool_cycles` drops incomplete tool cycles at +wire-replay time for hosts using `TranscriptEntry`. The harness's +`summarization/pairing.rs` chooses a compaction cut-off that does not bisect a +cycle. They answer different questions and are not duplicates. ## What a dialect is diff --git a/docs/modules/harness/tool-discovery.md b/docs/modules/harness/tool-discovery.md index 3073a38f..0a3d04a1 100644 --- a/docs/modules/harness/tool-discovery.md +++ b/docs/modules/harness/tool-discovery.md @@ -121,11 +121,15 @@ quarter earlier instead of overflowing. ## Prompt-guided models For models without native tool calling the tool list is rendered into the -system prompt. It now uses a compact TypeScript-style signature per tool — -`Arguments: {path: string, limit?: integer}` plus one note per described -top-level argument — instead of the raw JSON Schema (`tool::type_signature`, -`tool::argument_notes`). Constraints and nested descriptions are dropped; the -native path is unaffected. +system prompt by `tinytools-agent` (see [tool-dialect.md](tool-dialect.md)), +not by this crate: the JSON-in-tag dialect lists each tool's parameter +schema, and the P-Format dialect lists a compact positional call signature +(`read_file[0||1|]`). The bridge schemas above go through the same +renderer as any other tool, so `tool_search` / `tool_call` are callable from +either text dialect. `tool::type_signature` / `tool::argument_notes` remain +available as compact TypeScript-style formatters +(`{path: string, limit?: integer}`) for hosts that build their own prompt +text. The native path is unaffected. ## Live proof diff --git a/docs/modules/harness/tool.md b/docs/modules/harness/tool.md index 89e78333..7ee411a1 100644 --- a/docs/modules/harness/tool.md +++ b/docs/modules/harness/tool.md @@ -186,27 +186,30 @@ preference: ## Prompt-Guided Message Shape -A model whose profile reports `tool_calling = false` is driven through its own -Jinja chat template by the serving runtime (LM Studio, llama.cpp, Ollama), so -the outgoing message list has to satisfy that template, not just the wire -schema. Two helpers in `harness::tool` normalize it: - -- `coalesce_prompt_tool_results` renders assistant `tool_calls` back into +A model without a native tool channel is driven through its own Jinja chat +template by the serving runtime (LM Studio, llama.cpp, Ollama), so the outgoing +message list has to satisfy that template, not just the wire schema. Three +helpers in `tinyinference_llm::prompt_tools` normalize it, and both the +OpenAI-compatible adapter (for a profile with `tool_calling = false`, or after a +"tools unsupported" 400) and the harness (for a forced `Xml` / `Pformat` +dialect) apply them: + +- `coalesce_tool_results` renders assistant `tool_calls` back into `` text and folds consecutive `tool`-role results into one - `[Tool results]` user turn — the `tool` role and structured `tool_calls` - field are not consumable by these models. + `[Tool results]` user turn under the `` envelope — the + `tool` role and structured `tool_calls` field are not consumable by these + models, and a result body cannot forge a closing tag. - `ensure_resolvable_user_turn` guarantees the list contains a user turn the template can resolve as "the user query", inserting one after any leading - system turns when none is present. Several widely used templates hard-require - one: Qwen 3's raises `No user query found in messages.` and the runtime - returns a 400 before the model is called. A prompt-guided tool loop reaches - that state legitimately once the real user turn ages out of the window - (summarization, a resumed transcript, a task carried entirely by the system - prompt), leaving only assistant continuations and folded tool results — which - do not count as a query, since the model requested them itself. - -Both are applied by the OpenAI-compatible adapter before wire translation. -Native-tool models keep their message list untouched. + system turns when none is present. Qwen 3's template raises + `No user query found in messages.` otherwise, and a prompt-guided tool loop + reaches that state legitimately once the real user turn ages out of the + window. +- `with_tool_instructions` appends the protocol block and catalogue to the + system prompt. + +The answer is read back through `tinytools_agent::parse`; see +[tool-dialect.md](tool-dialect.md). ## Execution Lifecycle diff --git a/vendor/tinyinference b/vendor/tinyinference index 219b0ea6..92445ea5 160000 --- a/vendor/tinyinference +++ b/vendor/tinyinference @@ -1 +1 @@ -Subproject commit 219b0ea646c37376efcdcc32a4bfc41468d3a62d +Subproject commit 92445ea582c9b1c5dc78891ebbff4a817f9e8fa9 diff --git a/vendor/tinytools b/vendor/tinytools index a14e24d5..e347becd 160000 --- a/vendor/tinytools +++ b/vendor/tinytools @@ -1 +1 @@ -Subproject commit a14e24d55b699e812bfced96d0ff56e2f30d544e +Subproject commit e347becd7b83bd9a07e65b69b4294c485fc86e26