diff --git a/crates/tinyagents-harness/src/agent_loop/dialect.rs b/crates/tinyagents-harness/src/agent_loop/dialect.rs index b68a4989a..5a1627e70 100644 --- a/crates/tinyagents-harness/src/agent_loop/dialect.rs +++ b/crates/tinyagents-harness/src/agent_loop/dialect.rs @@ -12,8 +12,10 @@ use std::sync::Arc; -use tinyinference_llm::message::ContentBlock; -use tinyinference_llm::model::{ModelRequest, ModelResponse, ToolChoice}; +use tinyinference_llm::message::{ContentBlock, Message}; +use tinyinference_llm::model::{ + ModelRequest, ModelResponse, PromptSegment, SegmentRole, ToolChoice, +}; use tinyinference_llm::tool::{ToolCall, ToolSchema}; use tinytools_agent::dialect::{CodeDialect, CodeStyle, PFormatDialect}; use tinytools_agent::types::{ParseOptions, ParsedToolCall}; @@ -101,7 +103,24 @@ impl RunDialect { /// 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) { + /// + /// With `host_renders_catalogue` the schemas still leave the wire (the + /// registry built from them before this call is what parses the answer), + /// and nothing from the run's ordinary catalogue is appended: the host's + /// own prompt already carries the protocol block and the catalogue for + /// this dialect. `synthesized` is the exception — tool schemas minted + /// *this turn*, after the host's static prompt was already composed (the + /// structured-output fallback tool `StructuredStrategy::ToolCall` / + /// `ToolCallUnion` push onto `request.tools`). The host cannot have + /// rendered a schema it did not know about yet, so their catalogue + /// entries are appended here even in the host-rendered case, or the + /// model never learns the shape it is being forced to call. + pub(super) fn apply_to_request( + &self, + request: &mut ModelRequest, + host_renders_catalogue: bool, + synthesized: &[ToolSchema], + ) { if !self.is_text() || request.tools.is_empty() || request.tool_choice == ToolChoice::None { return; } @@ -110,6 +129,34 @@ impl RunDialect { 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); + let had_leading_system = matches!(messages.first(), Some(Message::System(_))); + if host_renders_catalogue { + let mut block = String::new(); + if !synthesized.is_empty() { + block.push_str(&self.render_catalogue(synthesized)); + } + // Only a forced choice still has to be said, since the host's + // prompt was composed before the choice was known. + match &request.tool_choice { + ToolChoice::Required => block.push_str("You must emit at least one tool call.\n"), + ToolChoice::Tool(name) => { + block.push_str(&format!("You must call the `{name}` tool.\n")); + } + ToolChoice::Auto | ToolChoice::None => {} + } + request.messages = if block.is_empty() { + // Nothing to say: no synthesized tool to advertise and no + // forced choice, so this rewrite leaves `messages` — and in + // particular whether a leading system message exists — + // exactly as it already was. + messages + } else { + prompt_tools::append_system_block(&messages, &block) + }; + request.tool_choice = ToolChoice::Auto; + sync_stripped_tools_cache_segment(request, had_leading_system); + return; + } request.messages = match self { Self::Xml | Self::Native => { prompt_tools::with_tool_instructions(&messages, &tools, &request.tool_choice) @@ -158,7 +205,118 @@ impl RunDialect { } }; request.tool_choice = ToolChoice::Auto; + sync_stripped_tools_cache_segment(request, had_leading_system); + } + + /// Renders `tools` into this dialect's catalogue shape alone (no + /// protocol instructions): the `Self::Xml` full-schema form, the + /// `Self::PFormat` positional-signature form, or the `Self::Code` + /// function-signature form. Used to advertise a schema the host's own + /// static catalogue could not have carried — see + /// [`Self::apply_to_request`]'s `synthesized` parameter. + fn render_catalogue(&self, tools: &[ToolSchema]) -> String { + let specs: Vec = tools + .iter() + .map(|schema| tinytools_agent::tinytools::ToolSpec { + name: schema.name.clone(), + description: schema.description.clone(), + parameters: schema.parameters.clone(), + }) + .collect(); + match self { + Self::Xml | Self::Native => tinytools_agent::render::render_json_catalogue(&specs), + Self::PFormat(_) => tinytools_agent::render::render_pformat_catalogue(&specs), + Self::Code(style, _) => tinytools_agent::render::render_code_catalogue(&specs, *style), + } + } +} + +/// Keeps a harness-declared `cache_segments` layout in sync with a text +/// dialect's rewrite, using the one thing only this call site still knows +/// for certain: whether `pre_rewrite_messages` already had a leading system +/// message *before* the protocol block gets folded in below. +/// +/// `request.cache_segments` may declare a trailing canonical tools segment +/// (`{id: "tools", role: Tools, cacheable: true}`) that is about to +/// disappear once `request.tools` is cleared. When a leading system message +/// already existed, dropping that trailing segment is all that is needed — +/// the declared head still names the same messages it always did, and later +/// fingerprinting (`refresh_prompt_cache_fingerprint`) can verify that by +/// simple equality. But when none existed yet, +/// `tinyinference_llm::prompt_tools::append_system_block` (used by both the +/// host-rendered and ordinary rewrite paths below) synthesizes exactly one +/// new leading system message for the protocol block — a segment no +/// declaration could have named in advance. That case is resolved *here*, +/// with certain knowledge of the pre-rewrite shape, rather than left for +/// `refresh_prompt_cache_fingerprint` to guess from the rewritten request +/// alone: reconstructing it after the fact from the post-rewrite shape alone +/// cannot tell an actually-synthesized segment apart from a custom +/// declaration that deliberately left an already-present system message out +/// of the cache key, and conflating the two would silently widen what a +/// middleware asked to keep out of the stable prefix. +/// +/// A declaration that is not exactly `[.., tools_segment]` — anything with a +/// head that does not otherwise account for the messages, or no declaration +/// at all — is left untouched, so `refresh_prompt_cache_fingerprint` keeps +/// taking the conservative whole-request digest for it. +fn sync_stripped_tools_cache_segment(request: &mut ModelRequest, had_leading_system: bool) { + let canonical_tools_segment = PromptSegment { + id: "tools".to_string(), + role: SegmentRole::Tools, + cacheable: true, + }; + let Some((last, head)) = request.cache_segments.split_last() else { + return; + }; + if *last != canonical_tools_segment { + return; + } + // The real, post-rewrite leading-system-message count — the exact same + // thing `refresh_prompt_cache_fingerprint` will independently derive + // from `request.messages` a moment later. Deriving the comparison + // against this, rather than against `head`'s own declared shape, is + // what lets every case below be a plain equality check instead of a + // guess: whatever the rewrite actually did to the messages is the one + // fact this function can trust. + let final_system_end = request + .messages + .iter() + .take_while(|message| matches!(message, Message::System(_))) + .count(); + let canonical_head: Vec = (0..final_system_end) + .map(|index| PromptSegment { + id: crate::prompt::system_segment_id(index), + role: SegmentRole::System, + cacheable: true, + }) + .collect(); + if head == canonical_head { + // The declared head already names exactly the messages that are + // really there (including the trivial `final_system_end == 0` + // case, where both sides are empty because this rewrite turned out + // to touch nothing — e.g. a host-rendered request with no + // synthesized tool and an `Auto` choice leaves `messages` + // untouched); only the now-gone trailing tools segment is stale. + request.cache_segments = canonical_head; + } else if head.is_empty() && !had_leading_system && final_system_end == 1 { + // No declared head and no existing leading system message before + // this call: the rewrite is the sole source of the new leading + // segment (`prompt_tools::append_system_block` inserts exactly one + // when none exists), so this is unambiguously the harness's own + // synthesis rather than something a declaration could have named in + // advance. + request.cache_segments = canonical_head; } + // Every other combination is left completely untouched, including the + // trailing tools segment: `head.is_empty() && had_leading_system` is a + // declaration that deliberately named nothing ahead of the tools + // segment even though a system message already existed (dropping to + // `head` would leave an *empty* `cache_segments`, which + // `refresh_prompt_cache_fingerprint` reads as "nothing declared yet" + // and promotes just the same); a non-empty `head` that does not match + // `canonical_head` is a custom declaration (a middleware-owned id, or a + // stale count) that must not be silently rewritten out from under it, + // partially or otherwise. } /// Builds the positional layout registry the P-Format and code dialects diff --git a/crates/tinyagents-harness/src/agent_loop/dialect/test.rs b/crates/tinyagents-harness/src/agent_loop/dialect/test.rs index 30fa93808..8f2505166 100644 --- a/crates/tinyagents-harness/src/agent_loop/dialect/test.rs +++ b/crates/tinyagents-harness/src/agent_loop/dialect/test.rs @@ -53,3 +53,167 @@ fn code_dialects_are_opt_in_and_share_the_positional_registry() { RunDialect::Code(CodeStyle::TypeScript, _) )); } + +#[test] +fn a_host_that_renders_the_catalogue_gets_the_schemas_stripped_but_nothing_appended() { + use tinyinference_llm::message::Message; + use tinyinference_llm::model::{ModelRequest, ToolChoice}; + use tinyinference_llm::tool::ToolSchema; + + let tools = vec![ToolSchema::new( + "lookup", + "Looks something up.", + serde_json::json!({ + "type": "object", + "properties": {"q": {"type": "string"}}, + "required": ["q"] + }), + )]; + let dialect = RunDialect::resolve(ToolDispatcher::Python, &tools, Some(true)); + let messages = vec![ + Message::system("host prompt with its own ## Tools block"), + Message::user("hi"), + ]; + + // Default: the loop appends the protocol block and the catalogue. + let mut appended = ModelRequest::new(messages.clone()).with_tools(tools.clone()); + dialect.apply_to_request(&mut appended, false, &[]); + assert!(appended.tools.is_empty()); + let system = appended.messages[0].text(); + assert!(system.contains("def lookup("), "{system}"); + + // Host-rendered: schemas still leave the wire, the prompt is untouched. + let mut host = ModelRequest::new(messages.clone()).with_tools(tools.clone()); + dialect.apply_to_request(&mut host, true, &[]); + assert!(host.tools.is_empty()); + assert_eq!(host.messages, messages); + assert_eq!(host.tool_choice, ToolChoice::Auto); + + // A forced choice is the one thing the host could not have said. + let mut forced = ModelRequest::new(messages).with_tools(tools); + forced.tool_choice = ToolChoice::Tool("lookup".into()); + dialect.apply_to_request(&mut forced, true, &[]); + let system = forced.messages[0].text(); + assert!( + system.contains("You must call the `lookup` tool."), + "{system}" + ); + assert!(!system.contains("def lookup(")); + assert_eq!(forced.tool_choice, ToolChoice::Auto); +} + +/// Every case above starts from a transcript that already has a leading +/// system message, so `prompt_tools::append_system_block`'s *other* branch — +/// inserting a brand-new leading message when none exists yet — is never +/// exercised. A regression there (failing to insert, inserting more than +/// one, or inserting it somewhere other than the front) would pass every +/// other test in this file undetected. +#[test] +fn a_run_with_no_leading_system_message_gets_exactly_one_synthesized_by_the_rewrite() { + use tinyinference_llm::message::Message; + use tinyinference_llm::model::{ModelRequest, ToolChoice}; + use tinyinference_llm::tool::ToolSchema; + + let tools = vec![ToolSchema::new( + "lookup", + "Looks something up.", + serde_json::json!({ + "type": "object", + "properties": {"q": {"type": "string"}}, + "required": ["q"] + }), + )]; + let dialect = RunDialect::resolve(ToolDispatcher::Python, &tools, Some(true)); + let no_leading_system = vec![Message::user("hi")]; + + // Ordinary rewrite: the loop's own protocol block and catalogue become + // the sole, newly-inserted leading system message. + let mut appended = ModelRequest::new(no_leading_system.clone()).with_tools(tools.clone()); + dialect.apply_to_request(&mut appended, false, &[]); + assert_eq!( + appended.messages.len(), + 2, + "exactly one system message must be inserted, not folded into an \ + existing one or duplicated: {:?}", + appended.messages + ); + assert!( + matches!(appended.messages[0], Message::System(_)), + "the synthesized message must be the new leading one: {:?}", + appended.messages + ); + let system = appended.messages[0].text(); + assert!(system.contains("def lookup("), "{system}"); + assert_eq!(appended.messages[1], Message::user("hi")); + + // Host-rendered with a forced choice: the host had no prompt at all to + // predate the synthesis, so the forced-choice sentence is what lands in + // the newly-inserted message; the catalogue stays the host's job. + let mut forced = ModelRequest::new(no_leading_system).with_tools(tools); + forced.tool_choice = ToolChoice::Tool("lookup".into()); + dialect.apply_to_request(&mut forced, true, &[]); + assert_eq!(forced.messages.len(), 2, "{:?}", forced.messages); + assert!(matches!(forced.messages[0], Message::System(_))); + let system = forced.messages[0].text(); + assert!( + system.contains("You must call the `lookup` tool."), + "{system}" + ); + assert!(!system.contains("def lookup(")); + assert_eq!(forced.tool_choice, ToolChoice::Auto); +} + +#[test] +fn a_host_that_renders_the_catalogue_still_learns_a_turn_synthesized_tool() { + use tinyinference_llm::message::Message; + use tinyinference_llm::model::{ModelRequest, ToolChoice}; + use tinyinference_llm::tool::ToolSchema; + + // The base tool the host's own static prompt already advertises. + let base = ToolSchema::new( + "lookup", + "Looks something up.", + serde_json::json!({ + "type": "object", + "properties": {"q": {"type": "string"}}, + "required": ["q"] + }), + ); + // The structured-output fallback tool minted for this turn only, after + // the host's prompt was already composed — see + // `RunPolicy::host_renders_tool_catalogue` and + // `StructuredStrategy::ToolCall`. + let synthesized = ToolSchema::new( + "emit_result", + "Return the result as `emit_result`.", + serde_json::json!({ + "type": "object", + "properties": {"total": {"type": "number"}}, + "required": ["total"] + }), + ); + let dialect = RunDialect::resolve( + ToolDispatcher::Python, + std::slice::from_ref(&base), + Some(true), + ); + let messages = vec![ + Message::system("host prompt with its own ## Tools block"), + Message::user("hi"), + ]; + + let mut request = + ModelRequest::new(messages).with_tools(vec![base.clone(), synthesized.clone()]); + request.tool_choice = ToolChoice::Tool("emit_result".into()); + dialect.apply_to_request(&mut request, true, std::slice::from_ref(&synthesized)); + + assert!(request.tools.is_empty()); + let system = request.messages[0].text(); + // The base tool is left to the host's own (untouched) catalogue... + assert!(!system.contains("def lookup("), "{system}"); + // ...but the synthesized one, the host could never have known about, is + // appended so the forced call has a schema to answer against. + assert!(system.contains("def emit_result("), "{system}"); + assert!(system.contains("You must call the `emit_result` tool.")); + assert_eq!(request.tool_choice, ToolChoice::Auto); +} diff --git a/crates/tinyagents-harness/src/agent_loop/run_loop.rs b/crates/tinyagents-harness/src/agent_loop/run_loop.rs index a91cfa559..fd9bfda1d 100644 --- a/crates/tinyagents-harness/src/agent_loop/run_loop.rs +++ b/crates/tinyagents-harness/src/agent_loop/run_loop.rs @@ -734,6 +734,10 @@ impl AgentHarness { // mode versus a tool-call fallback; an explicit `JsonSchema` always // uses provider-native mode. The chosen strategy drives extraction of // the final response below. + // Marks where any structured-output fallback tool gets pushed + // below, so it can be told apart afterward from what was already + // on `request.tools` — see `synthesized_tools`. + let tools_before_structured_plan = request.tools.len(); let structured_plan: Option<(StructuredStrategy, String, Value)> = match request.response_format.clone() { Some(ResponseFormat::Auto { name, schema }) @@ -899,6 +903,16 @@ impl AgentHarness { _ => None, }; + // Tool schemas minted by the structured-output plan above (the + // `ToolCall` / `ToolCallUnion` fallback tools), pushed onto + // `request.tools` after `tools_before_structured_plan` was + // recorded. A host that renders its own static tool catalogue + // composed it before this turn's structured-output planning ran, + // so it cannot have advertised these; `RunDialect::apply_to_request` + // appends their catalogue entries even in the host-rendered case. + let synthesized_tools: Vec = + request.tools[tools_before_structured_plan..].to_vec(); + // 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 @@ -944,7 +958,11 @@ impl AgentHarness { // `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. - dialect.apply_to_request(&mut request); + dialect.apply_to_request( + &mut request, + self.policy.host_renders_tool_catalogue, + &synthesized_tools, + ); // A host budget is acquired only for an explicit host-driven run. // Do it after structured-output planning: a synthetic schema tool @@ -2033,8 +2051,44 @@ pub(super) fn refresh_prompt_cache_fingerprint(request: &mut ModelRequest) { cacheable: true, }); } - let harness_layout = - request.cache_segments.is_empty() || request.cache_segments == expected_layout; + // The canonical harness-owned trailing tools segment: only *this* exact + // segment (including `cacheable: true`) is recognized as the harness's + // own below, so middleware that deliberately annotated its own trailing + // `tools` segment `cacheable: false` keeps that opt-out instead of being + // silently promoted to cacheable once a text dialect strips the schemas. + let canonical_tools_segment = PromptSegment { + id: "tools".to_string(), + role: SegmentRole::Tools, + cacheable: true, + }; + // A text dialect (`RunDialect::apply_to_request`) folds the catalogue + // into the system prompt and clears `tools` *after* `before_model` ran, + // so a middleware that declared the harness layout while the schemas + // were still on the request legitimately carries a trailing `tools` + // segment the rebuilt layout no longer has. That is still the harness + // layout, not a custom annotation: demoting it to the whole-request + // digest below would re-roll the provider routing key on every call. + // + // The declared head has to equal the rebuilt system-segment prefix + // exactly. The one case that legitimately would not — no leading system + // message at declare time, so the dialect synthesizes one — is already + // resolved before this function ever runs, by + // `RunDialect::sync_stripped_tools_cache_segment`, which has the + // pre-rewrite message shape this function does not: reconstructing that + // distinction from the rewritten request alone cannot tell an + // actually-synthesized leading segment apart from a custom declaration + // that deliberately left an already-present system message out of the + // cache key. + let declared_with_stripped_tools = request.tools.is_empty() + && request + .cache_segments + .split_last() + .is_some_and(|(last, head)| { + *last == canonical_tools_segment && head == expected_layout + }); + let harness_layout = request.cache_segments.is_empty() + || request.cache_segments == expected_layout + || declared_with_stripped_tools; if harness_layout { request.cache_segments = expected_layout; diff --git a/crates/tinyagents-harness/src/agent_loop/test.rs b/crates/tinyagents-harness/src/agent_loop/test.rs index 1fd8ad08b..4afd22e70 100644 --- a/crates/tinyagents-harness/src/agent_loop/test.rs +++ b/crates/tinyagents-harness/src/agent_loop/test.rs @@ -6798,3 +6798,371 @@ async fn tiered_system_messages_become_one_cacheable_segment_each() { ); assert!(request.provider_options[PROMPT_CACHE_KEY_OPTION].is_string()); } + +/// A middleware that declares the harness layout while the schemas are still +/// on the request (`system`, `tools`) must keep its stable-prefix fingerprint +/// under a text dialect, which folds the catalogue into the prompt and clears +/// `tools` afterwards. Before, the rebuilt layout (`system` only) no longer +/// matched the declaration, the request fell through to the whole-request +/// digest, and the provider routing key changed on every call of a thread. +#[test] +fn stripped_tools_segment_still_counts_as_the_harness_layout() { + use tinyinference_llm::model::{PromptSegment, SegmentRole}; + let system = Message::system("identity and rules"); + let declared = |request: &mut ModelRequest| { + request.cache_segments = vec![ + PromptSegment { + id: "system".to_string(), + role: SegmentRole::System, + cacheable: true, + }, + PromptSegment { + id: "tools".to_string(), + role: SegmentRole::Tools, + cacheable: true, + }, + ]; + }; + + // Same declaration, two turns of one thread, schemas already stripped. + let mut turn_one = ModelRequest::new(vec![system.clone(), Message::user("hi")]); + declared(&mut turn_one); + super::run_loop::refresh_prompt_cache_fingerprint(&mut turn_one); + let mut turn_two = ModelRequest::new(vec![ + system.clone(), + Message::user("hi"), + Message::assistant("hello"), + Message::user("later"), + ]); + declared(&mut turn_two); + super::run_loop::refresh_prompt_cache_fingerprint(&mut turn_two); + + let ids: Vec<&str> = turn_one + .cache_segments + .iter() + .map(|segment| segment.id.as_str()) + .collect(); + assert_eq!(ids, vec!["system"], "the stripped tools segment is dropped"); + let mut expected = crate::prompt::PromptBuilder::new(); + expected.push_system_messages(std::slice::from_ref(&system)); + assert_eq!( + turn_one.prompt_fingerprint, + expected.build(Vec::new()).prompt_fingerprint, + "the fingerprint is the stable-prefix one, not a whole-request digest" + ); + assert_eq!(turn_one.prompt_fingerprint, turn_two.prompt_fingerprint); + + // A genuinely custom annotation still takes the conservative path. + let mut custom = ModelRequest::new(vec![system, Message::user("hi")]); + custom.cache_segments = vec![PromptSegment { + id: "system:abc".to_string(), + role: SegmentRole::System, + cacheable: true, + }]; + super::run_loop::refresh_prompt_cache_fingerprint(&mut custom); + assert_ne!(custom.prompt_fingerprint, turn_one.prompt_fingerprint); +} + +/// A text-dialect run that starts with *no* leading system message declares +/// only the `tools` segment (`PromptBuilder` has no system prefix to name +/// yet). The dialect then synthesizes exactly one new leading system message +/// for its protocol block (`prompt_tools::append_system_block` inserts one +/// when none exists) and clears `tools`. That single synthesized segment is +/// still the harness's own dialect rewrite, not a custom annotation, and +/// must keep the stable-prefix fingerprint rather than falling through to +/// the whole-request digest — which would re-roll the provider routing key +/// as the transcript grows even though the leading system content itself +/// (the dialect's protocol block) never changes. +/// +/// Runs the actual dialect rewrite (`RunDialect::apply_to_request`), not a +/// hand-constructed post-rewrite `cache_segments`: the synthesis is resolved +/// there (`sync_stripped_tools_cache_segment`), using the pre-rewrite +/// message shape this test needs to be real for. +#[test] +fn a_dialect_synthesized_first_system_segment_still_counts_as_the_harness_layout() { + use tinyinference_llm::model::{PromptSegment, SegmentRole, ToolChoice}; + use tinyinference_llm::tool::ToolSchema; + + let tool = ToolSchema::new( + "lookup", + "Looks something up.", + serde_json::json!({ + "type": "object", + "properties": {"q": {"type": "string"}}, + "required": ["q"] + }), + ); + let dialect = super::dialect::RunDialect::resolve( + crate::config::ToolDispatcher::Xml, + std::slice::from_ref(&tool), + Some(false), + ); + let build_turn = |history: Vec| { + let mut request = ModelRequest::new(history).with_tools(vec![tool.clone()]); + request.tool_choice = ToolChoice::Auto; + // Declared before the rewrite: no leading system message exists yet + // (`history` is user-only), so only the tools segment is named. + request.cache_segments = vec![PromptSegment { + id: "tools".to_string(), + role: SegmentRole::Tools, + cacheable: true, + }]; + dialect.apply_to_request(&mut request, false, &[]); + super::run_loop::refresh_prompt_cache_fingerprint(&mut request); + request + }; + + let turn_one = build_turn(vec![Message::user("hi")]); + let turn_two = build_turn(vec![ + Message::user("hi"), + Message::assistant("hello"), + Message::user("later"), + ]); + + let ids: Vec<&str> = turn_one + .cache_segments + .iter() + .map(|segment| segment.id.as_str()) + .collect(); + assert_eq!(ids, vec!["system"], "the stripped tools segment is dropped"); + assert!(turn_one.prompt_fingerprint.is_some()); + assert_eq!( + turn_one.prompt_fingerprint, turn_two.prompt_fingerprint, + "the routing key must not change as the transcript grows, since the \ + synthesized protocol block is identical every turn" + ); +} + +/// The counterexample to the synthesis case above: a leading system message +/// *already exists* at declare time, but middleware deliberately names only +/// the trailing `tools` segment — omitting that system message from the +/// cache key on purpose, e.g. because it carries per-request volatile +/// content. The dialect rewrite folds its protocol block into that existing +/// message in place (`prompt_tools::append_system_block` only ever inserts a +/// *new* leading message when none exists), so nothing was synthesized here. +/// The declaration must be left exactly as the middleware wrote it, not +/// promoted to a fresh cacheable system segment the middleware never named. +#[test] +fn a_custom_layout_omitting_an_existing_system_message_is_not_promoted_by_the_dialect_rewrite() { + use tinyinference_llm::model::{PromptSegment, SegmentRole, ToolChoice}; + use tinyinference_llm::tool::ToolSchema; + + let tool = ToolSchema::new( + "lookup", + "Looks something up.", + serde_json::json!({ + "type": "object", + "properties": {"q": {"type": "string"}}, + "required": ["q"] + }), + ); + let dialect = super::dialect::RunDialect::resolve( + crate::config::ToolDispatcher::Xml, + std::slice::from_ref(&tool), + Some(false), + ); + let system = Message::system("volatile per-request content middleware keeps out of the key"); + let mut request = ModelRequest::new(vec![system, Message::user("hi")]).with_tools(vec![tool]); + request.tool_choice = ToolChoice::Auto; + request.cache_segments = vec![PromptSegment { + id: "tools".to_string(), + role: SegmentRole::Tools, + cacheable: true, + }]; + + dialect.apply_to_request(&mut request, false, &[]); + + assert!( + !request + .cache_segments + .iter() + .any(|segment| segment.role == SegmentRole::System), + "the rewrite must not invent a system segment the declaration never named: {:?}", + request.cache_segments + ); + + super::run_loop::refresh_prompt_cache_fingerprint(&mut request); + // Falls through to the conservative whole-request digest, not the + // stable-prefix fingerprint a harness-owned layout would get. + let mut harness_owned = request.clone(); + harness_owned.cache_segments = vec![ + PromptSegment { + id: "system".to_string(), + role: SegmentRole::System, + cacheable: true, + }, + PromptSegment { + id: "tools".to_string(), + role: SegmentRole::Tools, + cacheable: true, + }, + ]; + super::run_loop::refresh_prompt_cache_fingerprint(&mut harness_owned); + assert_ne!(request.prompt_fingerprint, harness_owned.prompt_fingerprint); +} + +/// A middleware that deliberately opts a custom trailing `tools` segment out +/// of caching (`cacheable: false`) is not the harness's own canonical +/// segment, even though its role and id match. Stripping the schemas off the +/// wire must not silently promote that opt-out to cacheable by matching it +/// against the harness layout on role/id alone. +#[test] +fn a_custom_tools_segment_opted_out_of_caching_is_not_mistaken_for_the_harness_layout() { + use tinyinference_llm::model::{PromptSegment, SegmentRole}; + + let system = Message::system("identity and rules"); + let mut request = ModelRequest::new(vec![system.clone(), Message::user("hi")]); + request.cache_segments = vec![ + PromptSegment { + id: "system".to_string(), + role: SegmentRole::System, + cacheable: true, + }, + PromptSegment { + id: "tools".to_string(), + role: SegmentRole::Tools, + cacheable: false, + }, + ]; + super::run_loop::refresh_prompt_cache_fingerprint(&mut request); + + // Declared segments are left untouched: this is treated as a genuinely + // custom annotation, not silently rewritten to the harness's stripped + // layout. + assert_eq!(request.cache_segments.len(), 2); + assert!(!request.cache_segments[1].cacheable); + + // And the fingerprint takes the conservative whole-request digest path, + // not the stable-prefix one a harness-owned layout would get. + let mut harness_owned = ModelRequest::new(vec![system, Message::user("hi")]); + harness_owned.cache_segments = vec![ + PromptSegment { + id: "system".to_string(), + role: SegmentRole::System, + cacheable: true, + }, + PromptSegment { + id: "tools".to_string(), + role: SegmentRole::Tools, + cacheable: true, + }, + ]; + super::run_loop::refresh_prompt_cache_fingerprint(&mut harness_owned); + assert_ne!(request.prompt_fingerprint, harness_owned.prompt_fingerprint); +} + +/// A second custom-layout counterexample: middleware names a *non-empty* +/// but middleware-owned head ahead of the canonical trailing `tools` +/// segment — `[system:tenant, tools]` — while a leading system message +/// really does exist. `head` is non-empty here (unlike the omission case +/// above), so an earlier, less careful version of the sync helper dropped +/// the trailing tools segment unconditionally whenever `head` was +/// non-empty, silently mutating this declaration down to `[system:tenant]` +/// before `refresh_prompt_cache_fingerprint` ever got a chance to recognize +/// it as custom and take the conservative path over the *original* bytes. +/// The declaration must survive completely intact — trailing tools segment +/// included — since it never matched the harness's own canonical shape in +/// the first place. +#[test] +fn a_custom_head_that_does_not_match_the_canonical_shape_is_left_completely_untouched() { + use tinyinference_llm::model::{PromptSegment, SegmentRole, ToolChoice}; + use tinyinference_llm::tool::ToolSchema; + + let tool = ToolSchema::new( + "lookup", + "Looks something up.", + serde_json::json!({ + "type": "object", + "properties": {"q": {"type": "string"}}, + "required": ["q"] + }), + ); + let dialect = super::dialect::RunDialect::resolve( + crate::config::ToolDispatcher::Xml, + std::slice::from_ref(&tool), + Some(false), + ); + let system = Message::system("tenant-scoped instructions"); + let original_segments = vec![ + PromptSegment { + id: "system:tenant".to_string(), + role: SegmentRole::System, + cacheable: true, + }, + PromptSegment { + id: "tools".to_string(), + role: SegmentRole::Tools, + cacheable: true, + }, + ]; + let mut request = ModelRequest::new(vec![system, Message::user("hi")]).with_tools(vec![tool]); + request.tool_choice = ToolChoice::Auto; + request.cache_segments = original_segments.clone(); + + dialect.apply_to_request(&mut request, false, &[]); + + assert_eq!( + request.cache_segments, original_segments, + "a non-canonical custom head must not be partially rewritten" + ); +} + +/// The exact edge case tinysweeper flagged: a host-rendered, no-tools- +/// synthesized, `Auto`-choice turn with no leading system message leaves +/// `request.messages` completely untouched (see the `block.is_empty()` +/// branch of `apply_to_request`) — no system message is ever inserted. The +/// sync helper must not synthesize a leading system cache segment for a +/// message that was never created, or `refresh_prompt_cache_fingerprint` +/// mismatches the (fictitious) declared segment against the real, empty +/// message layout and falls back to the conservative digest for a turn that +/// is actually the trivial empty-declaration case. +#[test] +fn host_rendered_with_nothing_to_say_drops_the_stale_tools_segment_without_inventing_one() { + use tinyinference_llm::model::{ModelRequest, PromptSegment, SegmentRole, ToolChoice}; + use tinyinference_llm::tool::ToolSchema; + + let tool = ToolSchema::new( + "lookup", + "Looks something up.", + serde_json::json!({ + "type": "object", + "properties": {"q": {"type": "string"}}, + "required": ["q"] + }), + ); + let dialect = super::dialect::RunDialect::resolve( + crate::config::ToolDispatcher::Xml, + std::slice::from_ref(&tool), + Some(false), + ); + let mut request = ModelRequest::new(vec![Message::user("hi")]).with_tools(vec![tool]); + request.tool_choice = ToolChoice::Auto; + request.cache_segments = vec![PromptSegment { + id: "tools".to_string(), + role: SegmentRole::Tools, + cacheable: true, + }]; + + // host_renders_catalogue = true, no synthesized tools, tool_choice::Auto: + // `block` stays empty, so the rewrite leaves `messages` untouched. + dialect.apply_to_request(&mut request, true, &[]); + + assert!( + !matches!(request.messages[0], Message::System(_)), + "no system message was actually inserted: {:?}", + request.messages + ); + assert!( + request.cache_segments.is_empty(), + "the stale tools segment must be dropped without inventing a system \ + segment for a message that does not exist: {:?}", + request.cache_segments + ); + + super::run_loop::refresh_prompt_cache_fingerprint(&mut request); + assert_eq!( + request.prompt_fingerprint, None, + "an empty declared layout with no system messages fingerprints as \ + nothing (the trivial stable-prefix case), not a whole-request digest" + ); +} diff --git a/crates/tinyagents-harness/src/runtime/types.rs b/crates/tinyagents-harness/src/runtime/types.rs index f01f6a110..d994696b9 100644 --- a/crates/tinyagents-harness/src/runtime/types.rs +++ b/crates/tinyagents-harness/src/runtime/types.rs @@ -268,6 +268,19 @@ pub struct RunPolicy { /// the same read is the fallback for a model that narrated a call as /// text, gated by [`RunPolicy::text_dialect_recovery`]. pub tool_dialect: ToolDispatcher, + /// Under a text dialect, whether the host already rendered the protocol + /// block and the tool catalogue into its own system prompt. + /// + /// `false` (the default): the loop folds `tinytools-agent`'s protocol + /// instructions and a catalogue of the advertised tools into the system + /// prompt right before dispatch, so a host that only sets + /// [`RunPolicy::tool_dialect`] gets a complete text-protocol prompt. + /// `true`: the loop still strips the schemas off the wire and still binds + /// the positional registry that parses calls back, but appends nothing — + /// a host that composes its prompt from the same dialect (so the model + /// sees one catalogue, in the place the host chose, inside the cacheable + /// prefix) sets this to avoid shipping every signature twice. + pub host_renders_tool_catalogue: bool, /// 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. @@ -529,6 +542,7 @@ impl Default for RunPolicy { // Opt-in: preserve the historical blank-final behavior by default. error_on_empty_response: false, tool_dialect: ToolDispatcher::Auto, + host_renders_tool_catalogue: false, 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 diff --git a/docs/modules/harness/tool-dialect.md b/docs/modules/harness/tool-dialect.md index 7bedd3bdc..437bb2345 100644 --- a/docs/modules/harness/tool-dialect.md +++ b/docs/modules/harness/tool-dialect.md @@ -30,6 +30,22 @@ picks the fix up. | `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 | +A host that composes its own system prompt from the same dialect — the +protocol block and the catalogue already in place, inside its cacheable +prefix — sets `RunPolicy::host_renders_tool_catalogue`. The text dialects +then still fold the transcript, strip the schemas off the wire and bind the +positional registry, but append nothing from the run's *ordinary* catalogue; +only a forced `tool_choice` (`Required` / `Tool(name)`) is still spelled out, +since the host's prompt predates it. Without the flag the loop appends the +block itself, and a host that also rendered one ships every signature twice. + +One exception: a structured-output fallback tool synthesized for *this turn* +(`StructuredStrategy::ToolCall` / `ToolCallUnion`, pushed onto the request +after the host's static prompt was already composed) is not something the +host could ever have advertised in its own catalogue. Its schema and +signature are appended anyway, even under `host_renders_tool_catalogue`, or +the model has nothing to answer the forced call against. + 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 @@ -87,10 +103,15 @@ The agent loop selects one per run from `RunPolicy::tool_dialect` (`ToolDispatcher::{Auto, Native, Xml, Pformat, Python, Typescript}`). `Auto` resolves to native when the model profile supports it and to XML otherwise; P-Format and the code dialects are opt-in. When a host has already composed a -tool protocol into the system prompt, the loop still appends its authoritative -block from the final post-middleware tool set. A heading in arbitrary prompt -text cannot prove that the host block matches the selected dialect, current -catalogue, or effective tool choice. +tool protocol into the system prompt but has not set +`RunPolicy::host_renders_tool_catalogue`, the loop still appends its own +authoritative block from the final post-middleware tool set — a heading in +arbitrary prompt text cannot prove that the host block matches the selected +dialect, current catalogue, or effective tool choice, so the loop does not +trust it and the host ends up shipping the catalogue twice. Setting the flag +is the host's explicit assertion that its own block *is* that authoritative +one (see "Selecting a dialect" above for what the loop still does — and does +not — append once it is set). ## Which surface to use diff --git a/vendor/tinytools b/vendor/tinytools index aa811fe9b..cfb3a155e 160000 --- a/vendor/tinytools +++ b/vendor/tinytools @@ -1 +1 @@ -Subproject commit aa811fe9b7cc8de36ee206d17ce6c66b4cad049d +Subproject commit cfb3a155e5821fe6ec19d01936205f0acef54520