diff --git a/Cargo.lock b/Cargo.lock index a57be9027..1105a7d39 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1629,6 +1629,7 @@ version = "0.3.0" dependencies = [ "anyhow", "httpdate", + "regex", "url", ] diff --git a/crates/tinyagents-graph/src/goals/mod.rs b/crates/tinyagents-graph/src/goals/mod.rs index ec869f968..6888c430e 100644 --- a/crates/tinyagents-graph/src/goals/mod.rs +++ b/crates/tinyagents-graph/src/goals/mod.rs @@ -24,7 +24,7 @@ mod types; pub use budget::{BudgetVerdict, GoalBudgetGuard, account_turn, accrues_usage, turn_tokens}; pub use continuation::{goal_gate_node, note_user_turn, run_continuation_tick}; pub use prompt::active_goal_context_block; -pub use tool::{GoalTool, GoalToolKind, goal_tools, register_goal_tools}; +pub use tool::{GoalTool, GoalToolKind, GoalUpdateHook, goal_tools, register_goal_tools}; pub use types::{GoalProgress, ThreadGoal, ThreadGoalStatus, TurnOutcome}; #[cfg(test)] diff --git a/crates/tinyagents-graph/src/goals/test.rs b/crates/tinyagents-graph/src/goals/test.rs index 40e691c67..9a884a42a 100644 --- a/crates/tinyagents-graph/src/goals/test.rs +++ b/crates/tinyagents-graph/src/goals/test.rs @@ -325,7 +325,7 @@ mod tool_tests { // Bare call (no context) errors. let res = set.execute(json!({ "objective": "x" })).await.unwrap(); assert!(res.is_error); - assert!(res.output().contains("active thread")); + assert!(res.output().contains("active chat thread")); // Context with no thread id also errors. let set = GoalTool::new(GoalToolKind::Set, store()); let res = run(&set, None, json!({ "objective": "x" })).await; @@ -340,10 +340,92 @@ mod tool_tests { } #[tokio::test] - async fn set_missing_objective_is_a_soft_error() { + async fn set_missing_objective_is_an_error_result() { let set = GoalTool::new(GoalToolKind::Set, store()); let res = run(&set, Some("t"), json!({})).await; - assert!(res.output().contains("missing 'objective'")); + assert!(res.is_error); + assert!(res.output().contains("Missing 'objective'")); + } + + #[tokio::test] + async fn tools_answer_with_goal_and_text_payload() { + let s = store(); + let context = Some("thread-payload"); + + let get = GoalTool::new(GoalToolKind::Get, s.clone()); + let res = run(&get, context, json!({})).await; + let empty = match &res.content[0] { + tinytools::ToolContent::Json { data } => data, + other => panic!("expected structured JSON content, got {other:?}"), + }; + assert!(empty["goal"].is_null()); + assert_eq!(empty["text"], "no goal set for this thread"); + + let set = GoalTool::new(GoalToolKind::Set, s.clone()); + let res = run( + &set, + context, + json!({ "objective": "land the PR", "token_budget": 5000 }), + ) + .await; + assert!(!res.is_error, "{res:?}"); + let payload = match &res.content[0] { + tinytools::ToolContent::Json { data } => data, + other => panic!("expected structured JSON content, got {other:?}"), + }; + assert_eq!(payload["goal"]["objective"], "land the PR"); + assert_eq!(payload["goal"]["status"], "active"); + assert_eq!(payload["goal"]["tokenBudget"], 5000); + assert_eq!(payload["goal"]["tokensUsed"], 0); + let text = payload["text"].as_str().unwrap(); + assert!(text.starts_with("Goal set."), "{text}"); + assert!(text.contains("objective: land the PR"), "{text}"); + assert_eq!(res.markdown_formatted.as_deref(), Some(text)); + } + + #[tokio::test] + async fn update_hook_fires_on_writes_only() { + use std::sync::Mutex; + + let s = store(); + let seen: Arc>> = Arc::default(); + let hook: super::super::tool::GoalUpdateHook = { + let seen = seen.clone(); + Arc::new(move |goal| seen.lock().unwrap().push(goal.status.as_str().to_string())) + }; + let tool = |kind| GoalTool::new(kind, s.clone()).with_update_hook(hook.clone()); + + run(&tool(GoalToolKind::Get), Some("t"), json!({})).await; + assert!(seen.lock().unwrap().is_empty()); + + run( + &tool(GoalToolKind::Set), + Some("t"), + json!({ "objective": "ship" }), + ) + .await; + run(&tool(GoalToolKind::Get), Some("t"), json!({})).await; + run(&tool(GoalToolKind::Complete), Some("t"), json!({})).await; + assert_eq!(*seen.lock().unwrap(), vec!["active", "complete"]); + + // A failed write (no goal to complete on another thread) does not fire. + run(&tool(GoalToolKind::Complete), Some("other"), json!({})).await; + assert_eq!(seen.lock().unwrap().len(), 2); + } + + #[test] + fn permission_levels_follow_read_only() { + use tinytools::PermissionLevel; + + for kind in GoalToolKind::ALL { + let tool = GoalTool::new(kind, store()); + let expected = if kind == GoalToolKind::Get { + PermissionLevel::ReadOnly + } else { + PermissionLevel::Write + }; + assert_eq!(tool.permission_level(), expected, "{}", kind.name()); + } } } diff --git a/crates/tinyagents-graph/src/goals/tool.rs b/crates/tinyagents-graph/src/goals/tool.rs index 5ceeea4df..7dd3b1a2c 100644 --- a/crates/tinyagents-graph/src/goals/tool.rs +++ b/crates/tinyagents-graph/src/goals/tool.rs @@ -7,6 +7,12 @@ //! that wants to expose them, but not part of the default model-facing set //! returned by [`goal_tools`]. //! +//! Every control answers with the JSON `{ "goal": , "text": +//! }`: `goal` is the structured camelCase goal a UI can draw +//! a banner from, `text` the model-readable rendering (also attached as the +//! markdown form). A host that must react to a change (publish an event, refresh +//! a chip) registers a hook with [`GoalTool::with_update_hook`]. +//! //! The target thread is resolved from //! [`ToolExecutionContext::thread_id`](tinyagents_harness::tool::ToolExecutionContext), //! the harness analogue of an ambient thread id: a tool never takes a @@ -21,10 +27,15 @@ use serde_json::{Value, json}; use super::store; use super::types::ThreadGoal; -use tinyagents_harness::error::Result; use tinyagents_harness::store::Store; use tinyagents_harness::tool::ToolRegistry; -use tinytools::{Tool, ToolPolicy, ToolResult, ToolRunContext, ToolSideEffects}; +use tinytools::{PermissionLevel, Tool, ToolPolicy, ToolResult, ToolRunContext, ToolSideEffects}; + +/// Callback a host registers to observe a goal a tool just wrote. +/// +/// Called after a successful `goal_set` / `goal_complete` / `goal_pause` / +/// `goal_resume` with the persisted goal (never for `goal_get` or `goal_clear`). +pub type GoalUpdateHook = Arc; /// Which thread-goal control a [`GoalTool`] implements. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] @@ -78,9 +89,11 @@ impl GoalToolKind { usage. Returns 'no goal set' when the thread has none." } Self::Set => { - "Set (or replace) this thread's goal — the durable objective you should keep \ - pursuing across turns until it's complete. Changing the objective resets \ - usage counters. Optionally set a token_budget; when reached, work halts." + "Set (or replace) this thread's goal — the durable objective you should \ + keep pursuing across turns until it's complete. Use at the start of a \ + non-trivial request, or to refine the objective as it sharpens. Changing \ + the objective resets usage counters. Optionally set a token_budget; when \ + reached, the goal pauses with a progress summary." } Self::Complete => { "Mark this thread's goal complete. Only call this when concrete evidence \ @@ -126,12 +139,24 @@ impl GoalToolKind { pub struct GoalTool { kind: GoalToolKind, store: Arc, + on_update: Option, } impl GoalTool { /// Creates one goal tool of `kind` backed by `store`. pub fn new(kind: GoalToolKind, store: Arc) -> Self { - Self { kind, store } + Self { + kind, + store, + on_update: None, + } + } + + /// Registers a [`GoalUpdateHook`] invoked after each successful write. + #[must_use] + pub fn with_update_hook(mut self, hook: GoalUpdateHook) -> Self { + self.on_update = Some(hook); + self } /// The control kind this tool implements. @@ -139,51 +164,75 @@ impl GoalTool { self.kind } - /// Dispatches the control against `thread_id`, returning the model-facing - /// content and a structured `raw` payload. - async fn dispatch(&self, thread_id: &str, args: &Value) -> Result<(String, Option)> { + /// Dispatches the control against `thread_id`, returning the goal (when + /// one exists after the call) and a model-facing note. + async fn dispatch( + &self, + thread_id: &str, + args: &Value, + ) -> std::result::Result<(Option, String), String> { + let stringify = |error: tinyagents_harness::error::TinyAgentsError| error.to_string(); match self.kind { - GoalToolKind::Get => match store::get(&self.store, thread_id).await? { - Some(goal) => Ok((render_goal(&goal), Some(serde_json::to_value(&goal)?))), - None => Ok(("no goal set for this thread".to_string(), None)), + GoalToolKind::Get => match store::get(&self.store, thread_id) + .await + .map_err(stringify)? + { + Some(goal) => Ok((Some(goal), String::new())), + None => Ok((None, "no goal set for this thread".to_string())), }, GoalToolKind::Set => { let Some(objective) = args.get("objective").and_then(Value::as_str) else { - return Ok(("error: missing 'objective' parameter".to_string(), None)); + return Err("Missing 'objective' parameter".to_string()); }; let token_budget = args.get("token_budget").and_then(Value::as_u64); - let goal = store::set(&self.store, thread_id, objective, token_budget).await?; - Ok(( - format!("Goal set.\n{}", render_goal(&goal)), - Some(serde_json::to_value(&goal)?), - )) + let goal = store::set(&self.store, thread_id, objective, token_budget) + .await + .map_err(stringify)?; + Ok((Some(goal), "Goal set.".to_string())) } GoalToolKind::Complete => { - let goal = store::complete(&self.store, thread_id).await?; - Ok(( - format!("Goal marked complete.\n{}", render_goal(&goal)), - Some(serde_json::to_value(&goal)?), - )) + let goal = store::complete(&self.store, thread_id) + .await + .map_err(stringify)?; + Ok((Some(goal), "Goal marked complete.".to_string())) } GoalToolKind::Pause => { - let goal = store::pause(&self.store, thread_id).await?; - Ok((render_goal(&goal), Some(serde_json::to_value(&goal)?))) + let goal = store::pause(&self.store, thread_id) + .await + .map_err(stringify)?; + Ok((Some(goal), String::new())) } GoalToolKind::Resume => { - let goal = store::resume(&self.store, thread_id).await?; - Ok((render_goal(&goal), Some(serde_json::to_value(&goal)?))) + let goal = store::resume(&self.store, thread_id) + .await + .map_err(stringify)?; + Ok((Some(goal), String::new())) } GoalToolKind::Clear => { - let removed = store::clear(&self.store, thread_id).await?; - Ok(( - format!("Goal cleared (removed={removed})."), - Some(json!({ "removed": removed })), - )) + let removed = store::clear(&self.store, thread_id) + .await + .map_err(stringify)?; + Ok((None, format!("Goal cleared (removed={removed})."))) } } } } +/// Builds the `{ goal, text }` payload every goal control answers with. +/// `text` is `note` followed by the rendered goal block (or just `note` when +/// there is no goal). +fn goal_payload(goal: Option<&ThreadGoal>, note: &str) -> Value { + let text = match goal { + Some(goal) if note.is_empty() => render_goal(goal), + Some(goal) => format!("{note}\n{}", render_goal(goal)), + None => note.to_string(), + }; + json!({ + "goal": goal.map(|goal| serde_json::to_value(goal).unwrap_or(Value::Null)), + "text": text, + }) +} + /// Renders a goal as a compact, model-readable block. fn render_goal(goal: &ThreadGoal) -> String { let budget = match goal.token_budget { @@ -243,6 +292,14 @@ impl Tool for GoalTool { true } + fn permission_level(&self) -> PermissionLevel { + if self.kind.read_only() { + PermissionLevel::ReadOnly + } else { + PermissionLevel::Write + } + } + fn policy(&self) -> ToolPolicy { ToolPolicy { classified: true, @@ -256,7 +313,7 @@ impl Tool for GoalTool { async fn execute(&self, _args: Value) -> anyhow::Result { Ok(error_result( - "goal tools require an active thread (no thread_id in tool context)", + "thread goal tools require an active chat thread", )) } @@ -268,13 +325,25 @@ impl Tool for GoalTool { ) -> anyhow::Result { let Some(thread_id) = context.and_then(ToolRunContext::thread_id) else { return Ok(error_result( - "goal tools require an active thread (no thread_id in tool context)", + "thread goal tools require an active chat thread", )); }; - let (content, raw) = self.dispatch(thread_id, &args).await?; - Ok(match raw { - Some(raw) => ToolResult::json(raw).with_markdown(content), - None => ToolResult::success(content), - }) + tracing::debug!( + tool = self.kind.name(), + thread_id, + "[thread_goals] goal tool execute" + ); + let (goal, note) = match self.dispatch(thread_id, &args).await { + Ok(outcome) => outcome, + Err(message) => return Ok(error_result(message)), + }; + if let (Some(goal), Some(hook)) = (goal.as_ref(), self.on_update.as_ref()) + && !self.kind.read_only() + { + hook(goal); + } + let payload = goal_payload(goal.as_ref(), ¬e); + let text = payload["text"].as_str().unwrap_or_default().to_string(); + Ok(ToolResult::json(payload).with_markdown(text)) } } diff --git a/crates/tinyagents-graph/src/lib.rs b/crates/tinyagents-graph/src/lib.rs index 40eab8c7b..3f836b69d 100644 --- a/crates/tinyagents-graph/src/lib.rs +++ b/crates/tinyagents-graph/src/lib.rs @@ -84,10 +84,10 @@ pub use export::{ }; pub use goals::store as goal_store; pub use goals::{ - BudgetVerdict, GoalBudgetGuard, GoalProgress, GoalTool, GoalToolKind, ThreadGoal, - ThreadGoalStatus, TurnOutcome, account_turn, accrues_usage, active_goal_context_block, - goal_gate_node, goal_tools, note_user_turn, register_goal_tools, run_continuation_tick, - turn_tokens, + BudgetVerdict, GoalBudgetGuard, GoalProgress, GoalTool, GoalToolKind, GoalUpdateHook, + ThreadGoal, ThreadGoalStatus, TurnOutcome, account_turn, accrues_usage, + active_goal_context_block, goal_gate_node, goal_tools, note_user_turn, register_goal_tools, + run_continuation_tick, turn_tokens, }; pub use observability::{ GraphEventJournal, GraphHealthSummary, GraphLangfuseExporter, GraphLatencyMetrics, diff --git a/crates/tinyagents-graph/src/todos/test.rs b/crates/tinyagents-graph/src/todos/test.rs index e1b745abd..292c05aaa 100644 --- a/crates/tinyagents-graph/src/todos/test.rs +++ b/crates/tinyagents-graph/src/todos/test.rs @@ -504,6 +504,21 @@ mod tool_tests { } } + /// A model that sends the retired `{"cards": ...}` shape gets a tool error + /// naming the replacement, never a dispatch error that would end the run. + #[tokio::test] + async fn retired_cards_shape_is_a_soft_error_naming_todos() { + let tool = TodoTool::new(store()); + let res = run( + &tool, + Some("t"), + json!({ "cards": [{ "content": "x", "status": "todo" }] }), + ) + .await; + assert!(res.is_error); + assert!(res.output().contains("pass `todos`"), "{}", res.output()); + } + #[tokio::test] async fn invariant_violation_is_a_soft_error() { let tool = TodoTool::new(store()); diff --git a/crates/tinyagents-harness/src/config/README.md b/crates/tinyagents-harness/src/config/README.md index e27ac325b..b1fb74298 100644 --- a/crates/tinyagents-harness/src/config/README.md +++ b/crates/tinyagents-harness/src/config/README.md @@ -47,6 +47,7 @@ question by existing. | File | Role | | ---------- | ---------------------------------------------------------------------- | | `mod.rs` | Module overview and re-exports. | +| `required_output.rs` | Pure primitives over `RequiredOutput`: `output_satisfies_contract`, `find_required_block`, `synthesize_block`, `repair_instruction`. | | `types.rs` | All types: `SessionConfig`, `TurnConfig`, `ToolConfig`, `MemoryLimits`, `ToolDispatcher`, `RequiredOutput`, and their serde defaults. | | `test.rs` | Unit tests: defaulting, fallback rules, inert-contract behaviour, serde round-tripping. | diff --git a/crates/tinyagents-harness/src/config/mod.rs b/crates/tinyagents-harness/src/config/mod.rs index 91c8275a3..eb7d9cb11 100644 --- a/crates/tinyagents-harness/src/config/mod.rs +++ b/crates/tinyagents-harness/src/config/mod.rs @@ -23,13 +23,20 @@ //! and are separable so a caller can override one turn's limits without //! rebuilding the session. //! -//! Everything here is inert — `serde` + `std` only, see `types`. Capabilities +//! The types are inert — `serde` + `std` only, see `types`; `required_output` +//! adds pure validate/repair functions over [`RequiredOutput`]. Capabilities //! (memory, security, budget, progress) are trait objects supplied separately; //! nothing in this module is a behaviour seam. +mod required_output; mod types; +pub use required_output::{ + find_required_block, output_satisfies_contract, repair_instruction, synthesize_block, +}; pub use types::*; +#[cfg(test)] +mod required_output_test; #[cfg(test)] mod test; diff --git a/crates/tinyagents-harness/src/config/required_output.rs b/crates/tinyagents-harness/src/config/required_output.rs new file mode 100644 index 000000000..76fe3dfe3 --- /dev/null +++ b/crates/tinyagents-harness/src/config/required_output.rs @@ -0,0 +1,92 @@ +//! Required structured-output validation and repair for a [`RequiredOutput`]. +//! +//! Some agent contracts mandate a JSON block — e.g. a `thoughts` block like +//! `{"thoughts": "…", "next_action": "…"}` — on **every** turn, because +//! downstream parsing/routing depends on it. Models frequently omit the block +//! entirely, leaving those consumers with nothing. +//! +//! This module supplies the pure primitives a turn engine uses to guarantee a +//! well-formed block on every accepted turn: +//! +//! * [`output_satisfies_contract`] — validate presence + shape of the block, +//! * [`repair_instruction`] — the corrective re-prompt that asks the model to +//! re-emit its reply with the block, and +//! * [`synthesize_block`] — a minimal, schema-valid block used as a deterministic +//! fallback when the re-prompt also omits it. +//! +//! The orchestration that ties these together (validate, re-prompt, synthesize) +//! belongs to the host so it can drive the extra provider call and reconcile +//! with streaming; keeping the logic here pure keeps it unit-testable without +//! a provider. + +use super::RequiredOutput; + +/// Whether `text` satisfies `contract`: it contains a JSON object carrying every +/// required key with a non-null value, in the expected leading position. An +/// inert contract (no non-blank keys) is treated as always satisfied so +/// enforcement is a no-op. +#[must_use] +pub fn output_satisfies_contract(text: &str, contract: &RequiredOutput) -> bool { + if !contract.is_active() { + return true; + } + find_required_block(text, contract).is_some() +} + +/// The required block when it appears in the expected **leading position** — +/// the *first* JSON value in `text` must be an object carrying every required +/// key with a non-null value — else `None`. +/// +/// The contract mandates the block in a predictable position so downstream +/// parsing/routing can rely on it. Prose before the block is fine (prose is not +/// JSON, so the block is still the first extracted value), but a block buried +/// after *another* JSON object is rejected and gets repaired rather than +/// silently accepted. Reuses the harness JSON extractor so fenced, inline, and +/// whole-object replies are all recognised. +#[must_use] +pub fn find_required_block(text: &str, contract: &RequiredOutput) -> Option { + let keys = contract.all_keys(); + if keys.is_empty() { + return None; + } + let first = tinytools_agent::extract_json_values(text) + .into_iter() + .next()?; + let obj = first.as_object()?; + let has_all = keys + .iter() + .all(|key| obj.get(key).is_some_and(|v| !v.is_null())); + if has_all { Some(first) } else { None } +} + +/// A minimal, schema-valid block synthesised when the model omits the block and +/// a corrective re-prompt fails to recover it. Every required key maps to an +/// empty string so downstream parsing always has a well-formed object to +/// consume. Returns `"{}"` only for an inert contract (which enforcement never +/// reaches). +#[must_use] +pub fn synthesize_block(contract: &RequiredOutput) -> String { + let mut obj = serde_json::Map::new(); + for key in contract.all_keys() { + obj.insert(key, serde_json::Value::String(String::new())); + } + serde_json::to_string(&serde_json::Value::Object(obj)).unwrap_or_else(|_| "{}".to_string()) +} + +/// The corrective instruction that re-prompts the model to re-emit its reply +/// with the required block. It is a self-contained user turn appended after the omitting reply. +/// +/// The model is asked to lead with the JSON object so the repaired reply +/// satisfies the leading-position rule directly, whether the caller keeps the +/// re-prompt as the whole reply (the non-streamed *replace* path) or appends it +/// after prose that was already streamed (the *append* path). +#[must_use] +pub fn repair_instruction(contract: &RequiredOutput) -> String { + let keys = contract.all_keys().join("\", \""); + format!( + "Your previous reply omitted the required JSON `{}` block that every turn must include. \ +Reply again, leading with a single valid JSON object containing the keys \"{}\" — all present \ +and non-null — then continue with your answer. Do not call any tools.", + contract.block_key, keys + ) +} diff --git a/crates/tinyagents-harness/src/config/required_output_test.rs b/crates/tinyagents-harness/src/config/required_output_test.rs new file mode 100644 index 000000000..842b987e5 --- /dev/null +++ b/crates/tinyagents-harness/src/config/required_output_test.rs @@ -0,0 +1,129 @@ +//! Tests for required structured-output validation and repair. + +use super::*; + +fn thoughts_contract() -> RequiredOutput { + RequiredOutput { + block_key: "thoughts".into(), + required_keys: vec!["next_action".into()], + } +} + +#[test] +fn present_well_formed_block_satisfies_contract() { + let contract = thoughts_contract(); + let text = "Sure! {\"thoughts\": \"planning\", \"next_action\": \"call tool\"}"; + assert!(output_satisfies_contract(text, &contract)); + assert!(find_required_block(text, &contract).is_some()); +} + +#[test] +fn prose_only_reply_fails_validation() { + let contract = thoughts_contract(); + assert!(!output_satisfies_contract( + "Sure, I'll handle that.", + &contract + )); +} + +#[test] +fn block_missing_a_required_sibling_key_fails() { + let contract = thoughts_contract(); + // Has `thoughts` but not `next_action`. + let text = "{\"thoughts\": \"planning\"}"; + assert!(!output_satisfies_contract(text, &contract)); +} + +#[test] +fn null_valued_required_key_fails() { + let contract = RequiredOutput::new("thoughts"); + assert!(!output_satisfies_contract( + "{\"thoughts\": null}", + &contract + )); +} + +#[test] +fn synthesized_block_satisfies_its_own_contract() { + let contract = thoughts_contract(); + let synthesized = synthesize_block(&contract); + assert!( + output_satisfies_contract(&synthesized, &contract), + "synthesized fallback must satisfy the contract it was built from: {synthesized}" + ); +} + +#[test] +fn leading_block_after_prose_is_accepted() { + let contract = thoughts_contract(); + // Prose before the block is fine — prose is not JSON, so the block is + // still the first extracted value. + let text = "Here is my plan.\n{\"thoughts\": \"x\", \"next_action\": \"y\"}"; + assert!(output_satisfies_contract(text, &contract)); +} + +#[test] +fn synthesized_block_prepended_to_prose_leads_correctly() { + // The non-streamed *replace* fallback prepends a synthesized block to the + // original prose; the block must be the first JSON value so the reply + // validates. + let contract = thoughts_contract(); + let repaired = format!("{}\n\n{}", synthesize_block(&contract), "Working on it."); + assert!(output_satisfies_contract(&repaired, &contract)); +} + +#[test] +fn block_buried_after_another_json_object_is_rejected() { + let contract = thoughts_contract(); + // A different JSON object leads; the required block is second → rejected + // so it gets repaired rather than silently accepted (issue #4117). + let text = "{\"foo\": 1}\n{\"thoughts\": \"x\", \"next_action\": \"y\"}"; + assert!(!output_satisfies_contract(text, &contract)); +} + +#[test] +fn blank_block_key_makes_contract_inert() { + // A blank block key is inert even when sibling keys are listed — the + // contract's defining key can never be enforced, so enforcement is + // skipped instead of accepting a block missing that key. + let contract = RequiredOutput { + block_key: " ".into(), + required_keys: vec!["next_action".into()], + }; + assert!(!contract.is_active()); + assert!(output_satisfies_contract( + "{\"next_action\": \"y\"}", + &contract + )); +} + +#[test] +fn inert_contract_is_always_satisfied() { + let contract = RequiredOutput::default(); + assert!(!contract.is_active()); + assert!(output_satisfies_contract("no block here", &contract)); + assert!(find_required_block("no block here", &contract).is_none()); +} + +#[test] +fn all_keys_trims_and_dedupes() { + let contract = RequiredOutput { + block_key: " thoughts ".into(), + required_keys: vec![ + "thoughts".into(), + " next_action ".into(), + "next_action".into(), + ], + }; + // block_key trimmed; duplicate `thoughts` and repeated `next_action` + // collapse to a single occurrence each, order-preserving. + assert_eq!(contract.all_keys(), vec!["thoughts", "next_action"]); +} + +#[test] +fn repair_instruction_names_every_required_key() { + let contract = thoughts_contract(); + let instruction = repair_instruction(&contract); + assert!(instruction.contains("thoughts")); + assert!(instruction.contains("next_action")); +} diff --git a/crates/tinyagents-harness/src/lib.rs b/crates/tinyagents-harness/src/lib.rs index 998c768bf..ea9a9ed3c 100644 --- a/crates/tinyagents-harness/src/lib.rs +++ b/crates/tinyagents-harness/src/lib.rs @@ -94,6 +94,7 @@ pub mod stream; pub mod structured; pub mod summarization; pub mod testkit; +pub mod title; pub mod token_estimation; pub mod tool; #[cfg(feature = "builtin-tools")] diff --git a/crates/tinyagents-harness/src/summarization/mod.rs b/crates/tinyagents-harness/src/summarization/mod.rs index a7102835d..0dce0fb1a 100644 --- a/crates/tinyagents-harness/src/summarization/mod.rs +++ b/crates/tinyagents-harness/src/summarization/mod.rs @@ -15,13 +15,17 @@ //! - [`Summarizer`] — async trait for condensing messages into a [`SummaryRecord`]. //! - [`ConcatSummarizer`] — deterministic concatenation stand-in (no LLM). //! - [`SummarizationPolicy`] — decides when to summarize and how to split the slice. +//! - [`ModelSummarizer`] / [`FaultTolerantCachingSummarizer`] — LLM-backed summarization +//! that never aborts a turn on a summarizer outage. //! //! All policy decisions are explicit data types, never hidden behaviour. Callers //! choose when to call, what to pass, and how to handle the result. pub mod compaction; +mod model_summarizer; pub mod pairing; mod render; +mod resilient; mod trim; mod types; @@ -29,11 +33,16 @@ pub use compaction::{ CompactionContext, CompactionDecision, CutPoint, OverflowClassifier, OverflowInfo, OverflowProbe, find_cut_point, summarize_with_split, }; +pub use model_summarizer::{ + DEFAULT_SUMMARIZE_KEEP_LAST, DEFAULT_SUMMARIZE_THRESHOLD_FRACTION, summarization_policy, + summarization_policy_with, +}; pub use pairing::{ advance_past_orphan_tools, find_safe_cutoff_point, is_tool_calling_assistant, retract_orphan_tool_calls, tool_pairing_is_intact, }; pub use render::render_message_for_summary; +pub use resilient::FaultTolerantCachingSummarizer; pub use trim::{trim_messages, trim_messages_to_token_budget_with, trim_messages_with}; pub use types::*; @@ -273,5 +282,7 @@ impl SummarizationPolicy { } } +#[cfg(test)] +mod model_summarizer_test; #[cfg(test)] mod test; diff --git a/crates/tinyagents-harness/src/summarization/model_summarizer.rs b/crates/tinyagents-harness/src/summarization/model_summarizer.rs new file mode 100644 index 000000000..0ab4e3a1e --- /dev/null +++ b/crates/tinyagents-harness/src/summarization/model_summarizer.rs @@ -0,0 +1,225 @@ +//! LLM-backed conversation summarization. +//! +//! [`ModelSummarizer`] is a [`Summarizer`] that condenses the older slice of a +//! transcript into a single system message with a [`ChatModel`] call, and +//! [`summarization_policy`] builds the context-window-aware +//! [`SummarizationPolicy`] that decides when to run it. The trigger is keyed to +//! the **current model's** context window: compaction fires once the running +//! token estimate crosses `threshold_fraction` of it +//! ([`DEFAULT_SUMMARIZE_THRESHOLD_FRACTION`] by default), and the most recent +//! `keep_last` non-system messages stay verbatim +//! ([`DEFAULT_SUMMARIZE_KEEP_LAST`] by default). +//! +//! Pair it with [`FaultTolerantCachingSummarizer`](super::FaultTolerantCachingSummarizer) +//! and [`ContextCompressionMiddleware`](crate::middleware::ContextCompressionMiddleware) +//! so a summarizer outage never aborts a turn. + +use std::sync::Arc; + +use async_trait::async_trait; +use tinyinference_llm::message::Message; +use tinyinference_llm::model::{ChatModel, ModelRequest}; + +use super::types::ModelSummarizer; +use super::{ + CompressionProvenance, SummarizationPolicy, Summarizer, SummaryRecord, SummaryRequest, + estimate_tokens, render_message_for_summary, +}; +use crate::error::{Result, TinyAgentsError}; +use crate::token_estimation::estimate_slice_tokens; + +/// Default fraction of the model's context window at which summarization fires. +pub const DEFAULT_SUMMARIZE_THRESHOLD_FRACTION: f64 = 0.90; + +/// Default number of most-recent non-system messages kept verbatim after a +/// compaction. The older head is folded into the summary; this tail stays +/// untouched so the model retains the live working context. +pub const DEFAULT_SUMMARIZE_KEEP_LAST: usize = 8; + +impl ModelSummarizer { + /// Build a summarizer over `model` (its id/temperature pinned). + pub fn new(model: Arc>, model_id: impl Into) -> Self { + Self { + model, + model_id: model_id.into(), + threshold_fraction: DEFAULT_SUMMARIZE_THRESHOLD_FRACTION, + } + } + + /// Override the threshold fraction recorded in summary provenance. Use the + /// same value passed to [`summarization_policy_with`]. + #[must_use] + pub fn with_threshold_fraction(mut self, fraction: f64) -> Self { + self.threshold_fraction = fraction; + self + } +} + +#[async_trait] +impl Summarizer for ModelSummarizer { + async fn summarize(&self, messages: &[Message]) -> Result { + self.summarize_messages(messages, None).await + } + + async fn summarize_request(&self, request: &SummaryRequest) -> Result { + self.summarize_messages(&request.messages, request.previous_summary.as_deref()) + .await + } +} + +impl ModelSummarizer { + async fn summarize_messages( + &self, + messages: &[Message], + previous_summary: Option<&str>, + ) -> Result { + if messages.is_empty() { + return Err(TinyAgentsError::Validation( + "cannot summarize an empty message list".into(), + )); + } + + let original_token_estimate = estimate_slice_tokens(messages); + let source_ids: Vec = (0..messages.len()).map(|i| format!("msg-{i}")).collect(); + + let transcript = messages + .iter() + .map(render_message_for_summary) + .collect::>() + .join("\n"); + let transcript = match previous_summary { + Some(previous) => format!( + "=== Previous Summary (background context) ===\n{previous}\n\n=== Messages to Summarize ===\n{transcript}" + ), + None => transcript, + }; + + tracing::info!( + model = %self.model_id, + head_messages = messages.len(), + approx_input_tokens = original_token_estimate, + "[tinyagents::summarize] dispatching context-window summary" + ); + + let request = ModelRequest::new(vec![ + Message::system(SUMMARIZER_SYSTEM_PROMPT), + Message::user(transcript), + ]); + let summary = self + .model + .invoke(&(), request) + .await + .map_err(|e| { + tracing::warn!(error = %e, "[tinyagents::summarize] summarizer model call failed"); + TinyAgentsError::Model(format!("summarizer model call failed: {e}")) + })? + .text(); + + let summary = summary.trim(); + if summary.is_empty() { + return Err(TinyAgentsError::Model( + "summarizer returned empty response".into(), + )); + } + + let body = format!("=== Conversation Summary (compacted) ===\n{summary}"); + let summary_token_estimate = estimate_tokens(&body); + + tracing::info!( + model = %self.model_id, + summary_tokens = summary_token_estimate, + freed_tokens = original_token_estimate.saturating_sub(summary_token_estimate), + "[tinyagents::summarize] context-window summary complete" + ); + + Ok(SummaryRecord { + summary: Message::system(body), + provenance: CompressionProvenance { + source_ids, + original_token_estimate, + summary_token_estimate, + reason: format!( + "ModelSummarizer via {} (LLM compaction at {:.0}% of context window)", + self.model_id, + self.threshold_fraction * 100.0 + ), + }, + }) + } +} + +/// Build the context-window-aware [`SummarizationPolicy`] for a model whose +/// input window is `context_window` tokens, with the default threshold +/// ([`DEFAULT_SUMMARIZE_THRESHOLD_FRACTION`]) and tail +/// ([`DEFAULT_SUMMARIZE_KEEP_LAST`]). +#[must_use] +pub fn summarization_policy(context_window: u64) -> SummarizationPolicy { + summarization_policy_with( + context_window, + DEFAULT_SUMMARIZE_THRESHOLD_FRACTION, + DEFAULT_SUMMARIZE_KEEP_LAST, + ) +} + +/// Like [`summarization_policy`] with an explicit trigger `threshold_fraction` +/// of the context window and number of recent messages to keep verbatim. +/// +/// The policy triggers once the estimated transcript tokens reach +/// `context_window * threshold_fraction`; all system messages plus the last +/// `keep_last` non-system messages are kept verbatim. +#[must_use] +pub fn summarization_policy_with( + context_window: u64, + threshold_fraction: f64, + keep_last: usize, +) -> SummarizationPolicy { + let mut policy = SummarizationPolicy::default() + .with_context_window(context_window) + .with_threshold_fraction(threshold_fraction); + policy.keep_last = keep_last; + policy +} + +/// System prompt for the context-window summarizer. +const SUMMARIZER_SYSTEM_PROMPT: &str = "You are a summarization agent creating a context \ +checkpoint for an AI assistant whose conversation has grown too long to fit its context window. \ +You are given the earlier portion of a chronological conversation (user, assistant, and tool \ +messages). Compress it into a dense, structured handoff note that the assistant will read as \ +BACKGROUND REFERENCE — not as new instructions.\n\ +\n\ +Rules:\n\ +- Write ONLY the structured summary below. No greeting, no preamble, no closing remarks.\n\ +- This is reference material describing turns that ALREADY happened. Do NOT answer any question \ +or perform any task mentioned in it. The assistant acts only on the live messages that appear \ +AFTER this summary; if a later message contradicts or changes topic, the later message wins.\n\ +- Redact secrets: replace any API keys, tokens, passwords, or credentials with [REDACTED] (note \ +that a credential was present).\n\ +- Be specific and information-dense: prefer concrete facts (paths, names, values, decisions) over \ +narration. Drop greetings, small talk, and redundant acknowledgements.\n\ +\n\ +Produce exactly these sections (write \"None\" when a section is empty):\n\ +\n\ +## Goal\n\ +What the user is ultimately trying to accomplish.\n\ +\n\ +## Completed Actions\n\ +Numbered list of what has already been done, with key results/outputs.\n\ +\n\ +## Active State\n\ +The current state of the work right now: files touched, systems configured, what is true.\n\ +\n\ +## Key Decisions\n\ +Decisions made and the reasoning, so they are not relitigated.\n\ +\n\ +## Resolved Questions\n\ +Questions already answered — include the answer so it is not repeated.\n\ +\n\ +## Pending / Open (reference only)\n\ +Requests or work outstanding in the compacted turns. These are STALE — do NOT act on them unless \ +the latest live message explicitly asks.\n\ +\n\ +## Relevant Files\n\ +Files read, created, or modified, with a one-line note on each.\n\ +\n\ +## Critical Context\n\ +Anything else essential to continue correctly (constraints, environment facts, gotchas)."; diff --git a/crates/tinyagents-harness/src/summarization/model_summarizer_test.rs b/crates/tinyagents-harness/src/summarization/model_summarizer_test.rs new file mode 100644 index 000000000..e33cae489 --- /dev/null +++ b/crates/tinyagents-harness/src/summarization/model_summarizer_test.rs @@ -0,0 +1,215 @@ +//! Tests for [`ModelSummarizer`], [`FaultTolerantCachingSummarizer`] and the +//! context-window-aware policy builders. + +use std::sync::Arc; +use std::sync::atomic::{AtomicUsize, Ordering}; + +use async_trait::async_trait; +use serde_json::json; +use tinyinference_llm::message::{AssistantMessage, ContentBlock, Message, ToolMessage}; +use tinyinference_llm::tool::ToolCall; + +use super::{ + DEFAULT_SUMMARIZE_KEEP_LAST, DEFAULT_SUMMARIZE_THRESHOLD_FRACTION, + FaultTolerantCachingSummarizer, ModelSummarizer, SummarizationPolicy, Summarizer, + SummaryRecord, SummaryRequest, summarization_policy, summarization_policy_with, +}; +use crate::error::{Result, TinyAgentsError}; +use crate::testkit::ScriptedModel; + +fn tool_call_messages(arguments: serde_json::Value) -> Vec { + vec![ + Message::Assistant(AssistantMessage { + id: None, + content: vec![ContentBlock::Thinking { + text: "I should inspect the matching records.".into(), + signature: None, + }], + tool_calls: vec![ToolCall::new("lookup-1", "lookup", arguments)], + usage: None, + origin: None, + }), + Message::Tool(ToolMessage { + tool_call_id: "lookup-1".into(), + content: vec![ContentBlock::Json(json!({"matches": 2}))], + trusted_verbatim: false, + artifact: None, + }), + ] +} + +#[test] +fn policy_is_context_window_aware_at_the_default_threshold() { + let policy = summarization_policy(200_000); + assert_eq!(policy.context_window, Some(200_000)); + assert_eq!( + policy.threshold_fraction, + DEFAULT_SUMMARIZE_THRESHOLD_FRACTION + ); + assert_eq!(policy.keep_last, DEFAULT_SUMMARIZE_KEEP_LAST); +} + +#[test] +fn default_threshold_leaves_headroom_below_the_window() { + let policy = summarization_policy(100_000); + let effective = (policy.context_window.unwrap() as f64 * policy.threshold_fraction) as u64; + assert_eq!(effective, 90_000); +} + +#[test] +fn explicit_threshold_and_tail_override_the_defaults() { + let policy = summarization_policy_with(10_000, 0.5, 3); + assert_eq!(policy.threshold_fraction, 0.5); + assert_eq!(policy.keep_last, 3); +} + +#[tokio::test] +async fn model_summarizer_wraps_the_reply_and_records_provenance() { + let model = Arc::new(ScriptedModel::replies(vec![" the gist "])); + let summarizer = ModelSummarizer::new(model.clone(), "m-1").with_threshold_fraction(0.8); + let messages = vec![Message::user("hello there"), Message::assistant("hi back")]; + let record = summarizer.summarize(&messages).await.unwrap(); + + assert!( + record + .summary + .text() + .contains("=== Conversation Summary (compacted) ===") + ); + assert!(record.summary.text().contains("the gist")); + assert_eq!(record.provenance.source_ids, vec!["msg-0", "msg-1"]); + assert!(record.provenance.reason.contains("80%")); + assert!(record.provenance.reason.contains("m-1")); + let requests = model.requests(); + assert_eq!(requests.len(), 1); + assert!(requests[0].messages[1].text().contains("user: hello there")); +} + +#[tokio::test] +async fn model_summarizer_rejects_empty_input_and_empty_replies() { + let summarizer = ModelSummarizer::new(Arc::new(ScriptedModel::replies(vec![" "])), "m"); + assert!(summarizer.summarize(&[]).await.is_err()); + let err = summarizer + .summarize(&[Message::user("x")]) + .await + .unwrap_err(); + assert!(err.to_string().contains("empty response")); +} + +#[tokio::test] +async fn model_summarizer_renders_structured_messages_and_prior_summary() { + let model = Arc::new(ScriptedModel::replies(vec!["combined context"])); + let summarizer = ModelSummarizer::new(model.clone(), "m"); + let messages = tool_call_messages(json!({"query": "open issues"})); + let request = + SummaryRequest::new(messages.clone()).with_previous_summary("Earlier result: 4 issues"); + + let record = summarizer.summarize_request(&request).await.unwrap(); + let transcript = model.requests()[0].messages[1].text(); + assert!(transcript.contains("Earlier result: 4 issues")); + assert!(transcript.contains("I should inspect the matching records.")); + assert!(transcript.contains("")); + assert!(transcript.contains("{\"matches\":2}")); + assert_eq!( + record.provenance.original_token_estimate, + crate::token_estimation::estimate_slice_tokens(&messages) + ); +} + +struct CountingFailing(Arc); + +#[async_trait] +impl Summarizer for CountingFailing { + async fn summarize(&self, _messages: &[Message]) -> Result { + self.0.fetch_add(1, Ordering::SeqCst); + Err(TinyAgentsError::Model("boom".into())) + } +} + +fn long_slice() -> Vec { + (0..6) + .map(|i| Message::user(format!("message {i} {}", "word ".repeat(40)))) + .collect() +} + +#[tokio::test] +async fn failure_falls_back_to_a_deterministic_trim_and_trips_the_breaker() { + let calls = Arc::new(AtomicUsize::new(0)); + let policy = SummarizationPolicy::default().with_context_window(1_000); + let guarded = + FaultTolerantCachingSummarizer::new(Box::new(CountingFailing(calls.clone())), &policy); + + let first = guarded.summarize(&long_slice()).await.unwrap(); + assert!(first.summary.text().contains("deterministic trim")); + assert_eq!(calls.load(Ordering::SeqCst), 1); + + // A different slice: the breaker is open, so the inner summarizer is skipped. + let other = vec![Message::user("something else entirely")]; + let second = guarded.summarize(&other).await.unwrap(); + assert!(second.provenance.reason.contains("circuit breaker open")); + assert_eq!(calls.load(Ordering::SeqCst), 1); +} + +#[tokio::test] +async fn an_identical_slice_is_served_from_the_cache() { + let model = Arc::new(ScriptedModel::replies(vec!["one summary"])); + let policy = SummarizationPolicy::default().with_context_window(1_000); + let guarded = FaultTolerantCachingSummarizer::new( + Box::new(ModelSummarizer::new(model.clone(), "m")), + &policy, + ); + let slice = long_slice(); + let a = guarded.summarize(&slice).await.unwrap(); + let b = guarded.summarize(&slice).await.unwrap(); + assert_eq!(a.summary.text(), b.summary.text()); + assert_eq!( + model.requests().len(), + 1, + "second call must not reach the model" + ); +} + +#[tokio::test] +async fn cache_key_includes_structured_messages_and_previous_summary() { + let model = Arc::new(ScriptedModel::replies(vec!["first", "second", "third"])); + let policy = SummarizationPolicy::default().with_context_window(1_000); + let guarded = FaultTolerantCachingSummarizer::new( + Box::new(ModelSummarizer::new(model.clone(), "m")), + &policy, + ); + let first_messages = tool_call_messages(json!({"query": "one"})); + let second_messages = tool_call_messages(json!({"query": "two"})); + + guarded + .summarize_request(&SummaryRequest::new(first_messages.clone())) + .await + .unwrap(); + guarded + .summarize_request(&SummaryRequest::new(second_messages)) + .await + .unwrap(); + guarded + .summarize_request( + &SummaryRequest::new(first_messages).with_previous_summary("prior checkpoint"), + ) + .await + .unwrap(); + + let requests = model.requests(); + assert_eq!(requests.len(), 3); + assert!(requests[2].messages[1].text().contains("prior checkpoint")); +} + +#[tokio::test] +async fn the_fallback_front_drops_oldest_messages_to_fit_its_budget() { + let policy = SummarizationPolicy::default().with_context_window(1_000); + let guarded = + FaultTolerantCachingSummarizer::new(Box::new(CountingFailing(Arc::default())), &policy); + // A 1_000-token window gives a floor budget of 1_024 tokens; oversize the slice. + let big: Vec = (0..40) + .map(|i| Message::user(format!("m{i} {}", "x".repeat(400)))) + .collect(); + let record = guarded.summarize(&big).await.unwrap(); + assert!(record.summary.text().contains("older message(s) dropped")); + assert!(record.summary.text().contains("m39")); +} diff --git a/crates/tinyagents-harness/src/summarization/resilient.rs b/crates/tinyagents-harness/src/summarization/resilient.rs new file mode 100644 index 000000000..f706dde0f --- /dev/null +++ b/crates/tinyagents-harness/src/summarization/resilient.rs @@ -0,0 +1,255 @@ +//! Fault-tolerant, per-turn-caching wrapper around any [`Summarizer`]. + +use std::hash::{Hash, Hasher}; +use std::sync::Mutex; +use std::sync::atomic::{AtomicBool, Ordering}; + +use async_trait::async_trait; +use tinyinference_llm::message::Message; + +use super::{ + CompressionProvenance, SummarizationPolicy, Summarizer, SummaryRecord, SummaryRequest, + estimate_tokens, render_message_for_summary, +}; +use crate::error::Result; +use crate::token_estimation::estimate_slice_tokens; + +/// Token budget for the deterministic-trim fallback summary, as a fraction of +/// the policy's summarization trigger budget. The fallback must actually *free* +/// tokens (so the turn shrinks below the window), so it targets a small slice of +/// the trigger point rather than echoing the whole compacted head back. +const FALLBACK_TRIM_TRIGGER_FRACTION: f64 = 0.25; +/// Hard floor / ceiling (tokens) for the deterministic-trim fallback budget, so +/// tiny windows still keep *something* and huge windows don't defeat the point. +const FALLBACK_TRIM_MIN_TOKENS: u64 = 1_024; +const FALLBACK_TRIM_MAX_TOKENS: u64 = 8_192; + +/// A single cached summary keyed by the shape of its input slice. +/// +/// `key` is a content hash of the exact `to_summarize` slice the crate handed us +/// (message count folded in). Repeat calls within a turn that present the same +/// slice (retries, re-planning, or a stalled tool loop that re-issues an +/// identical model request) reuse the cached [`SummaryRecord`] instead of +/// re-dispatching the summarizer LLM. +struct CachedSummary { + key: u64, + record: SummaryRecord, +} + +/// Fault-tolerant, per-turn-caching [`Summarizer`] adapter (issue #4461). +/// +/// Wraps the real (LLM-backed) [`super::ModelSummarizer`] the turn hands the +/// crate [`ContextCompressionMiddleware`][crate::middleware::ContextCompressionMiddleware] +/// and hardens two regressions the crate introduced versus the legacy engine: +/// +/// 1. **Failure no longer aborts the turn.** The crate's `before_model` does +/// `self.summarizer.summarize(..).await?`, so any provider hiccup maps to +/// [`crate::TinyAgentsError::Model`] and fails the whole run — on exactly the +/// longest, most valuable threads. This adapter instead catches the error, +/// logs a `warn`, trips a **per-turn circuit breaker**, and returns a +/// deterministic (LLM-free) trim of the input. The turn continues, matching +/// the legacy `warn! + circuit-breaker + deterministic-trim` fallback. Once +/// the breaker is tripped, every later compaction in the turn skips the +/// known-bad LLM and trims directly. +/// +/// 2. **No re-summarizing identical input.** The crate rebuilds the request from +/// `messages.clone()` each loop iteration and rewrites only that per-call +/// clone, so the working transcript never shrinks. Any call that presents the +/// same `to_summarize` slice (retries, re-planning, an identical re-issued +/// request) would otherwise spend a fresh full-transcript summarizer LLM call. +/// A single-slot content-hash cache makes those repeat calls free until the +/// transcript actually grows past the threshold again. +/// +/// Constructed fresh per turn inside the turn assembly, +/// so the breaker flag and cache are naturally per-turn state — no task-locals. +pub struct FaultTolerantCachingSummarizer { + /// The real LLM-backed summarizer we guard. + inner: Box, + /// Per-turn circuit breaker: set once `inner` fails, thereafter every + /// compaction trims deterministically without touching the LLM. + breaker_tripped: AtomicBool, + /// Single-slot cache of the last produced summary, keyed by input-slice hash. + cache: Mutex>, + /// Token budget for the deterministic-trim fallback (derived from the + /// policy's context window at construction). + fallback_trim_budget: u64, +} + +impl FaultTolerantCachingSummarizer { + /// Wrap `inner` with per-turn fault tolerance + caching, sizing the + /// deterministic-trim fallback budget from `policy`'s trigger budget. + pub fn new(inner: Box, policy: &SummarizationPolicy) -> Self { + let fallback_trim_budget = ((policy.trigger_budget() as f64 + * FALLBACK_TRIM_TRIGGER_FRACTION) as u64) + .clamp(FALLBACK_TRIM_MIN_TOKENS, FALLBACK_TRIM_MAX_TOKENS); + tracing::debug!( + fallback_trim_budget, + trigger_budget = policy.trigger_budget(), + "[tinyagents::summarize] installing fault-tolerant caching summarizer adapter" + ); + Self { + inner, + breaker_tripped: AtomicBool::new(false), + cache: Mutex::new(None), + fallback_trim_budget, + } + } + + /// Content hash of the complete request, including structured message data + /// and any prior checkpoint, so either kind of change busts the cache. + fn request_key(request: &SummaryRequest) -> u64 { + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + request.messages.len().hash(&mut hasher); + for message in &request.messages { + match serde_json::to_vec(message) { + Ok(encoded) => encoded.hash(&mut hasher), + Err(_) => format!("{message:?}").hash(&mut hasher), + } + } + request.previous_summary.hash(&mut hasher); + hasher.finish() + } + + /// Deterministic, LLM-free fallback: front-drop the oldest messages until the + /// remaining slice fits [`fallback_trim_budget`][Self::fallback_trim_budget] + /// tokens (the same front-drop semantics as + /// [`MessageTrimMiddleware`][crate::middleware::MessageTrimMiddleware] + /// with [`TrimStrategy::MaxTokens`][crate::summarization::TrimStrategy]), + /// then render the survivors into a single system checkpoint message. Never + /// fails, spends no tokens, and produces the same [`SummaryRecord`] shape the + /// LLM path does so provenance still surfaces downstream. + fn deterministic_trim( + &self, + messages: &[Message], + previous_summary: Option<&str>, + cause: &str, + ) -> SummaryRecord { + let original_token_estimate = + estimate_slice_tokens(messages) + previous_summary.map_or(0, estimate_tokens); + let source_ids: Vec = (0..messages.len()).map(|i| format!("msg-{i}")).collect(); + + // Front-drop oldest messages until the tail fits the budget. Keep at + // least the single most-recent message so the summary is never empty. + let mut start = 0usize; + let previous_token_estimate = previous_summary.map_or(0, estimate_tokens); + loop { + let remaining = previous_token_estimate + estimate_slice_tokens(&messages[start..]); + if remaining <= self.fallback_trim_budget || start + 1 >= messages.len() { + break; + } + start += 1; + } + let dropped = start; + + let mut body = String::from( + "=== Conversation Summary (deterministic trim — summarizer unavailable) ===\n", + ); + if dropped > 0 { + body.push_str(&format!( + "[{dropped} older message(s) dropped to fit the context budget]\n", + )); + } + if let Some(previous) = previous_summary { + body.push_str(&format!("Previous summary (older context):\n{previous}\n")); + } + for msg in &messages[start..] { + body.push_str(&format!("{}\n", render_message_for_summary(msg))); + } + let summary_token_estimate = estimate_tokens(&body); + + tracing::warn!( + cause, + head_messages = messages.len(), + dropped, + from_tokens = original_token_estimate, + to_tokens = summary_token_estimate, + "[tinyagents::summarize] deterministic-trim fallback (no LLM); turn continues" + ); + + SummaryRecord { + summary: Message::system(body), + provenance: CompressionProvenance { + source_ids, + original_token_estimate, + summary_token_estimate, + reason: format!( + "deterministic-trim fallback (summarizer LLM unavailable: {cause}); \ + front-dropped {dropped} message(s) to a {}-token budget", + self.fallback_trim_budget + ), + }, + } + } +} + +#[async_trait] +impl Summarizer for FaultTolerantCachingSummarizer { + async fn summarize(&self, messages: &[Message]) -> Result { + self.summarize_request(&SummaryRequest::new(messages.to_vec())) + .await + } + + async fn summarize_request(&self, request: &SummaryRequest) -> Result { + let key = Self::request_key(request); + + // Cache hit: an identical slice was already summarized this turn. + if let Ok(guard) = self.cache.lock() + && let Some(cached) = guard.as_ref() + && cached.key == key + { + tracing::debug!( + key, + head_messages = request.messages.len(), + "[tinyagents::summarize] reusing cached summary (identical input slice; \ + no summarizer LLM call)" + ); + return Ok(cached.record.clone()); + } + + // Circuit open from an earlier failure this turn: skip the known-bad LLM + // and trim deterministically without even attempting a call. + let record = if self.breaker_tripped.load(Ordering::Relaxed) { + tracing::debug!( + key, + head_messages = request.messages.len(), + "[tinyagents::summarize] circuit breaker open; trimming deterministically \ + (skipping summarizer LLM)" + ); + self.deterministic_trim( + &request.messages, + request.previous_summary.as_deref(), + "circuit breaker open (earlier summarizer failure)", + ) + } else { + match self.inner.summarize_request(request).await { + Ok(record) => record, + Err(err) => { + // Trip the per-turn breaker and fall back — never propagate, + // so compaction failure can no longer abort the turn. + self.breaker_tripped.store(true, Ordering::Relaxed); + tracing::warn!( + error = %err, + key, + head_messages = request.messages.len(), + "[tinyagents::summarize] summarizer failed; tripping per-turn circuit \ + breaker and falling back to deterministic trim" + ); + self.deterministic_trim( + &request.messages, + request.previous_summary.as_deref(), + &err.to_string(), + ) + } + } + }; + + // Cache the result (LLM or fallback) so a repeat identical slice is free. + if let Ok(mut guard) = self.cache.lock() { + *guard = Some(CachedSummary { + key, + record: record.clone(), + }); + } + Ok(record) + } +} diff --git a/crates/tinyagents-harness/src/summarization/types.rs b/crates/tinyagents-harness/src/summarization/types.rs index 9ee76664f..b773ddf12 100644 --- a/crates/tinyagents-harness/src/summarization/types.rs +++ b/crates/tinyagents-harness/src/summarization/types.rs @@ -4,11 +4,35 @@ //! to record — are expressed as data types so they can be inspected, tested, //! and audited without coupling to any particular LLM provider. +use std::sync::Arc; + use async_trait::async_trait; use serde::{Deserialize, Serialize}; use crate::error::Result; use tinyinference_llm::message::Message; +use tinyinference_llm::model::ChatModel; + +/// An LLM-backed [`Summarizer`] that condenses messages with the active model. +/// +/// The model id is retained for provenance, while the wrapped model carries +/// the actual provider configuration. +pub struct ModelSummarizer { + pub(super) model: Arc>, + /// Model id, kept for logging and provenance. + pub(super) model_id: String, + /// Threshold reported in summary provenance; the policy owns the trigger. + pub(super) threshold_fraction: f64, +} + +impl std::fmt::Debug for ModelSummarizer { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ModelSummarizer") + .field("model_id", &self.model_id) + .field("threshold_fraction", &self.threshold_fraction) + .finish_non_exhaustive() + } +} // --------------------------------------------------------------------------- // Token estimation diff --git a/crates/tinyagents-harness/src/title/mod.rs b/crates/tinyagents-harness/src/title/mod.rs new file mode 100644 index 000000000..65abd52c6 --- /dev/null +++ b/crates/tinyagents-harness/src/title/mod.rs @@ -0,0 +1,240 @@ +//! Pure helpers for generating and validating conversation thread titles. +//! +//! Parsing / sanitisation rules and the title-generation request, kept free of +//! any host config, provider runtime, or RPC wiring so a host can unit-test and +//! reuse them. + +use std::hash::{Hash, Hasher}; + +use tinyinference_llm::message::Message; +use tinyinference_llm::model::ModelRequest; + +pub const THREAD_TITLE_SYSTEM_PROMPT: &str = "You name chat threads from the first user message and the assistant reply. Return only the name: at most 3 words, like Fix session handoff or Gmail OAuth retry. Lead with the verb or the subject and drop filler words. No quotes. No markdown. No punctuation."; + +/// Words a title carries at most. Three is the whole point of the shape: a +/// thread list is scanned, not read, and a fourth word is always the one that +/// pushes the specific words off the end of a narrow row. +pub const THREAD_TITLE_MAX_WORDS: usize = 3; +/// Hard character ceiling on a title, so one very long word cannot widen a row. +pub const THREAD_TITLE_MAX_CHARS: usize = 48; + +/// Filler a title is better off without. +/// +/// Prompts open with conversational scaffolding ("okay so can you please…"), +/// and taking the first three words verbatim would spend the whole title on it. +/// Only words that never identify a thread on their own are listed; a filtered +/// title that comes out empty falls back to the unfiltered words, so a message +/// made entirely of these still gets a name. +const FILLER_WORDS: &[&str] = &[ + "a", "about", "an", "and", "are", "as", "at", "be", "but", "by", "can", "could", "do", "does", + "for", "from", "hey", "hi", "how", "i", "if", "in", "into", "is", "it", "its", "just", "let", + "lets", "like", "me", "my", "of", "ok", "okay", "on", "or", "our", "please", "so", "thanks", + "that", "the", "their", "then", "there", "these", "they", "this", "to", "uh", "um", "us", + "was", "we", "well", "what", "when", "which", "will", "with", "would", "you", "your", +]; + +/// Stable 16-hex-char fingerprint of a title — safe for structured logs +/// where we want to correlate events without leaking the raw title text. +pub fn title_log_fingerprint(title: &str) -> String { + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + title.hash(&mut hasher); + format!("{:016x}", hasher.finish()) +} + +/// Returns `true` when the title matches the auto-generated placeholder +/// shape used by `thread_create_new` (`"Chat Mon 1 1:23 AM"` / `...PM"`). +/// +/// Only placeholder titles are eligible for replacement by the LLM-generated +/// title; user-renamed threads are left untouched. +pub fn is_auto_generated_thread_title(title: &str) -> bool { + let trimmed = title.trim(); + let bytes = trimmed.as_bytes(); + if bytes.len() < 16 || !trimmed.starts_with("Chat ") { + return false; + } + + let month_end = 8; + if bytes.len() <= month_end || !bytes[5..month_end].iter().all(|b| b.is_ascii_alphabetic()) { + return false; + } + if bytes.get(month_end) != Some(&b' ') { + return false; + } + + let mut idx = month_end + 1; + let day_start = idx; + while idx < bytes.len() && bytes[idx].is_ascii_digit() { + idx += 1; + } + if idx == day_start || idx - day_start > 2 { + return false; + } + if bytes.get(idx) != Some(&b' ') { + return false; + } + idx += 1; + + let hour_start = idx; + while idx < bytes.len() && bytes[idx].is_ascii_digit() { + idx += 1; + } + if idx == hour_start || idx - hour_start > 2 { + return false; + } + if bytes.get(idx) != Some(&b':') { + return false; + } + idx += 1; + + if idx + 2 >= bytes.len() + || !bytes[idx].is_ascii_digit() + || !bytes[idx + 1].is_ascii_digit() + || bytes[idx + 2] != b' ' + { + return false; + } + idx += 3; + + matches!(&trimmed[idx..], "AM" | "PM") +} + +/// Collapses any run of whitespace (including newlines/tabs) into single +/// ASCII spaces and trims the result. +pub fn collapse_whitespace(input: &str) -> String { + input.split_whitespace().collect::>().join(" ") +} + +/// Reduces any text to the thread-title shape: at most +/// [`THREAD_TITLE_MAX_WORDS`] words, e.g. `Fix session handoff`. +/// +/// This is the shape enforcer, not a request: the model is asked for a short +/// name in [`THREAD_TITLE_SYSTEM_PROMPT`], but a title that reaches storage as +/// a whole sentence because one completion ignored the instruction is exactly +/// the bug the shape is meant to remove, so every path runs through here. +/// +/// Rules applied (in order): +/// - split on anything that is not alphanumeric (punctuation, quotes, markdown, +/// and whitespace all become word breaks) +/// - drop [`FILLER_WORDS`], unless that would leave nothing +/// - keep the first [`THREAD_TITLE_MAX_WORDS`] words, and stop early rather +/// than exceed [`THREAD_TITLE_MAX_CHARS`] +/// +/// A word's own spelling is left alone — `OAuth` and `Gmail` read wrong +/// lowercased, and the model is the only thing here that knows which is which. +/// +/// Returns `None` when no word survives. +pub fn shorten_title(text: &str) -> Option { + let words: Vec<&str> = text + .split(|c: char| !c.is_alphanumeric()) + .filter(|word| !word.is_empty()) + .collect(); + if words.is_empty() { + return None; + } + let meaningful: Vec<&str> = words + .iter() + .copied() + .filter(|word| !FILLER_WORDS.contains(&word.to_lowercase().as_str())) + .collect(); + // All-filler input ("can you please") still names its thread, badly-but- + // stably, rather than leaving the placeholder title in place. + let source = if meaningful.is_empty() { + words + } else { + meaningful + }; + + let mut title = String::new(); + for word in source.into_iter().take(THREAD_TITLE_MAX_WORDS) { + let separator = usize::from(!title.is_empty()); + let room = THREAD_TITLE_MAX_CHARS.saturating_sub(title.chars().count() + separator); + if room == 0 { + break; + } + if !title.is_empty() { + title.push(' '); + } + // A single word longer than the ceiling is truncated rather than + // dropped: dropping it can empty an otherwise usable title. + title.extend(word.chars().take(room)); + } + (!title.is_empty()).then_some(title) +} + +/// Sanitises a raw LLM title completion into a stored thread title. +/// +/// Takes the first non-empty line — a chatty model that adds a second line of +/// commentary should not have it folded into the name — and shortens it with +/// [`shorten_title`], which absorbs the quote/markdown/punctuation stripping +/// the older sentence-shaped title needed done by hand. +/// +/// Returns `None` if the result is empty. +pub fn sanitize_generated_title(raw: &str) -> Option { + let line = raw + .lines() + .find(|line| !line.trim().is_empty()) + .unwrap_or(raw); + shorten_title(line) +} + +/// Derives a stable display title directly from the first useful user message. +/// +/// This is the no-provider fallback used while a conversation only has user +/// context, or when model-based title generation is unavailable. It keeps the +/// title meaningful without repeatedly renaming the thread later. +pub fn title_from_user_message(message: &str) -> Option { + let collapsed = collapse_whitespace(message); + if collapsed.is_empty() { + return None; + } + + // Only the first sentence describes the ask; what follows is context the + // title has no room for anyway. + let first_sentence = collapsed + .split(['.', '!', '?', '\n']) + .find(|part| !part.trim().is_empty()) + .unwrap_or(&collapsed); + shorten_title(first_sentence) +} + +/// Builds the user-visible prompt passed to the title-generation model. +pub fn build_title_prompt(user_message: &str, assistant_message: &str) -> String { + format!( + "First user message:\n{user_message}\n\nAssistant reply:\n{assistant_message}\n\nReturn the best thread name." + ) +} + +/// Builds the whole title-generation request. +/// +/// # It deliberately sets no model +/// +/// The caller has already resolved the model by building the provider for the +/// `summarization` role, and the resolved model is the one that should +/// dispatch. `ModelRequest::model` is a *per-request override* that the +/// managed backend resolves verbatim, so anything set here replaces that +/// correct model on the wire. +/// +/// This used to override it with `"hint:summarize"`, which no lookup table in +/// the tree defines — every hint-alias table spells the alias `summarization`. +/// The string matched nothing, survived translation unchanged, and reached the +/// backend as a literal model id, which answered +/// `400 Model 'hint:summarize' is not available` on every call. Title +/// generation then fell back to a keyword title for four months without +/// anything escalating (#5637). +/// +/// Leaving `model` unset is also the only form that is correct for **every** +/// provider. `create_chat_model` resolves the `summarization` role to the +/// managed backend, a Claude Agent SDK / Claude Code model, a local runtime, +/// or a BYOK cloud slug; pinning any concrete tier id here would be wrong for +/// the four non-managed branches. Unset means each provider uses its own +/// construction-time default. +pub fn build_title_request(user_message: &str, assistant_message: &str) -> ModelRequest { + ModelRequest::new(vec![ + Message::system(THREAD_TITLE_SYSTEM_PROMPT), + Message::user(build_title_prompt(user_message, assistant_message)), + ]) + .with_temperature(0.2) +} + +#[cfg(test)] +mod test; diff --git a/crates/tinyagents-harness/src/title/test.rs b/crates/tinyagents-harness/src/title/test.rs new file mode 100644 index 000000000..f70235cdc --- /dev/null +++ b/crates/tinyagents-harness/src/title/test.rs @@ -0,0 +1,256 @@ +use super::*; + +// ── title_log_fingerprint ───────────────────────────────────── + +#[test] +fn fingerprint_is_stable_for_same_input() { + assert_eq!( + title_log_fingerprint("hello"), + title_log_fingerprint("hello") + ); +} + +#[test] +fn fingerprint_differs_for_different_input() { + assert_ne!( + title_log_fingerprint("hello"), + title_log_fingerprint("world") + ); +} + +#[test] +fn fingerprint_is_sixteen_hex_chars() { + let fp = title_log_fingerprint("anything"); + assert_eq!(fp.len(), 16); + // Lowercase hex specifically, so grep-friendly debug logs stay + // consistent (folded in from the former threads/ops_tests copy). + assert!( + fp.chars() + .all(|c| c.is_ascii_hexdigit() && !c.is_ascii_uppercase()), + "fingerprint must be lowercase hex, got: {fp}" + ); +} + +// ── is_auto_generated_thread_title ──────────────────────────── + +#[test] +fn accepts_canonical_placeholder() { + assert!(is_auto_generated_thread_title("Chat Jan 1 1:23 AM")); + assert!(is_auto_generated_thread_title("Chat Dec 31 11:59 PM")); +} + +#[test] +fn accepts_single_digit_day_and_hour() { + assert!(is_auto_generated_thread_title("Chat Mar 5 9:07 AM")); +} + +#[test] +fn accepts_two_digit_day_and_hour() { + assert!(is_auto_generated_thread_title("Chat Feb 28 10:45 PM")); +} + +#[test] +fn tolerates_surrounding_whitespace() { + assert!(is_auto_generated_thread_title(" Chat Jan 1 1:23 AM ")); +} + +#[test] +fn rejects_empty_and_short_titles() { + assert!(!is_auto_generated_thread_title("")); + assert!(!is_auto_generated_thread_title("Chat")); + assert!(!is_auto_generated_thread_title("Chat Jan 1")); +} + +#[test] +fn rejects_non_chat_prefix() { + assert!(!is_auto_generated_thread_title("Thread Jan 1 1:23 AM")); + assert!(!is_auto_generated_thread_title("chat Jan 1 1:23 AM")); // case matters +} + +#[test] +fn rejects_numeric_month() { + assert!(!is_auto_generated_thread_title("Chat 01 1 1:23 AM")); +} + +#[test] +fn rejects_missing_am_pm() { + assert!(!is_auto_generated_thread_title("Chat Jan 1 1:23")); + assert!(!is_auto_generated_thread_title("Chat Jan 1 1:23 XM")); +} + +#[test] +fn rejects_user_renamed_titles() { + assert!(!is_auto_generated_thread_title("Planning the launch party")); + assert!(!is_auto_generated_thread_title( + "Chat with Alice about deploys" + )); +} + +#[test] +fn rejects_malformed_minutes() { + // Minutes must be exactly two digits followed by a space. + assert!(!is_auto_generated_thread_title("Chat Jan 1 1:2 AM")); + assert!(!is_auto_generated_thread_title("Chat Jan 1 1:234 AM")); +} + +// ── collapse_whitespace ──────────────────────────────────────── + +#[test] +fn collapse_whitespace_normalises_runs() { + assert_eq!(collapse_whitespace(" hello world "), "hello world"); +} + +#[test] +fn collapse_whitespace_handles_tabs_and_newlines() { + assert_eq!(collapse_whitespace("a\tb\nc d"), "a b c d"); +} + +#[test] +fn collapse_whitespace_empty_returns_empty() { + assert_eq!(collapse_whitespace(""), ""); + assert_eq!(collapse_whitespace(" "), ""); +} + +// ── shorten_title ───────────────────────────────────────────── + +#[test] +fn shorten_keeps_at_most_three_words() { + assert_eq!( + shorten_title("Fix session handoff flow and pointer").unwrap(), + "Fix session handoff" + ); + assert_eq!(shorten_title("Launch Plan").unwrap(), "Launch Plan"); +} + +#[test] +fn shorten_leaves_a_words_own_spelling_alone() { + // Lowercasing would turn these into something nobody would type. + assert_eq!( + shorten_title("Gmail OAuth retry loop").unwrap(), + "Gmail OAuth retry" + ); +} + +#[test] +fn shorten_drops_leading_filler() { + assert_eq!( + shorten_title("okay so can you please fix the session handoff").unwrap(), + "fix session handoff" + ); +} + +#[test] +fn shorten_falls_back_to_filler_when_that_is_all_there_is() { + assert_eq!(shorten_title("can you please").unwrap(), "can you please"); +} + +#[test] +fn shorten_treats_punctuation_and_markdown_as_word_breaks() { + assert_eq!( + shorten_title("**Debugging deploys:** retry").unwrap(), + "Debugging deploys retry" + ); + assert_eq!( + shorten_title("\"gmail/oauth retry\"").unwrap(), + "gmail oauth retry" + ); +} + +#[test] +fn shorten_bounds_total_length() { + let long = format!("{} {} {}", "a".repeat(30), "b".repeat(30), "c".repeat(30)); + let out = shorten_title(&long).unwrap(); + assert!(out.chars().count() <= THREAD_TITLE_MAX_CHARS); + // A word that does not fit whole is truncated, never dropped. + assert!(out.starts_with(&"a".repeat(30))); +} + +#[test] +fn shorten_counts_chars_not_bytes() { + // Each ✨ is 3 bytes in UTF-8, and is not alphanumeric — a title made + // only of them has no word to keep. + assert!(shorten_title(&"✨".repeat(90)).is_none()); + let out = shorten_title(&"é".repeat(90)).unwrap(); + assert_eq!(out.chars().count(), THREAD_TITLE_MAX_CHARS); +} + +#[test] +fn shorten_returns_none_without_a_word() { + assert!(shorten_title("").is_none()); + assert!(shorten_title(" \n\t ").is_none()); + assert!(shorten_title("///").is_none()); +} + +// ── sanitize_generated_title ────────────────────────────────── + +#[test] +fn sanitize_shortens_a_sentence_shaped_completion() { + assert_eq!( + sanitize_generated_title("\"Planning the launch party\"").unwrap(), + "Planning launch party" + ); + // "are"/"we" are filler, so a question collapses to its one real word. + assert_eq!(sanitize_generated_title("Where are we?").unwrap(), "Where"); +} + +#[test] +fn sanitize_passes_an_already_short_completion_through() { + assert_eq!( + sanitize_generated_title("Fix session handoff").unwrap(), + "Fix session handoff" + ); +} + +#[test] +fn sanitize_picks_first_nonempty_line() { + let raw = "\n\n First real line \nsecond line\n"; + assert_eq!(sanitize_generated_title(raw).unwrap(), "First real line"); +} + +#[test] +fn sanitize_returns_none_for_empty_or_whitespace() { + assert!(sanitize_generated_title("").is_none()); + assert!(sanitize_generated_title(" \n\t ").is_none()); + assert!(sanitize_generated_title("\"\"").is_none()); +} + +#[test] +fn sanitize_bounds_length() { + let long = "a".repeat(200); + let out = sanitize_generated_title(&long).unwrap(); + assert_eq!(out.chars().count(), THREAD_TITLE_MAX_CHARS); +} + +// ── title_from_user_message ────────────────────────────────── + +#[test] +fn title_from_user_message_uses_first_specific_words() { + assert_eq!( + title_from_user_message("Can you retrieve my latest 5 emails and summarize them?").unwrap(), + "retrieve latest 5" + ); +} + +#[test] +fn title_from_user_message_removes_command_prefix_and_punctuation() { + assert_eq!( + title_from_user_message("/briefing Morning update, please. Then check email").unwrap(), + "briefing Morning update" + ); +} + +#[test] +fn title_from_user_message_returns_none_for_empty_context() { + assert!(title_from_user_message(" \n\t ").is_none()); + assert!(title_from_user_message("///").is_none()); +} + +// ── build_title_prompt ──────────────────────────────────────── + +#[test] +fn prompt_contains_both_messages_and_instruction() { + let prompt = build_title_prompt("hello", "hi there"); + assert!(prompt.contains("First user message:\nhello")); + assert!(prompt.contains("Assistant reply:\nhi there")); + assert!(prompt.contains("Return the best thread name")); +} diff --git a/crates/tinyagents-harness/src/tool/README.md b/crates/tinyagents-harness/src/tool/README.md index b07ff7fb0..a0a1dd3ee 100644 --- a/crates/tinyagents-harness/src/tool/README.md +++ b/crates/tinyagents-harness/src/tool/README.md @@ -119,6 +119,8 @@ 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. | +| `schema_walk.rs` | Vendor-neutral schema/value walkers: primary array path, response fields, missing and unsupported argument names. | +| `shared/` | `CanonicalSharedToolAdapter` (a `tinytools::Tool` over shared, non-cloneable registries) and `EarlyExitHook` (pause the run when a designated tool succeeds). | | `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;`. | diff --git a/crates/tinyagents-harness/src/tool/mod.rs b/crates/tinyagents-harness/src/tool/mod.rs index 1113432d5..5d95762e8 100644 --- a/crates/tinyagents-harness/src/tool/mod.rs +++ b/crates/tinyagents-harness/src/tool/mod.rs @@ -11,7 +11,9 @@ mod prompt; mod schema; mod schema_compact; mod schema_prepare; +mod schema_walk; pub mod select; +pub mod shared; mod signature; mod timeout; pub mod toolset; @@ -32,7 +34,9 @@ pub use prompt::*; pub use schema::*; pub use schema_compact::*; pub use schema_prepare::*; +pub use schema_walk::*; pub use select::*; +pub use shared::{CanonicalSharedToolAdapter, EarlyExit, EarlyExitHook}; pub use signature::*; pub use timeout::*; pub use toolset::{ToolExposureExplanation, ToolSet}; @@ -405,4 +409,6 @@ mod canonical_test; #[cfg(test)] mod context_test; #[cfg(test)] +mod schema_walk_test; +#[cfg(test)] mod timeout_test; diff --git a/crates/tinyagents-harness/src/tool/schema_walk.rs b/crates/tinyagents-harness/src/tool/schema_walk.rs new file mode 100644 index 000000000..71f424fdf --- /dev/null +++ b/crates/tinyagents-harness/src/tool/schema_walk.rs @@ -0,0 +1,285 @@ +//! Vendor-neutral JSON Schema and JSON value walking for tool shapes. +//! +//! These five helpers answer questions about a tool's shape — where its primary +//! array lives, what fields its response declares, which required arguments are +//! missing, which supplied argument names the schema does not recognize — using +//! nothing but JSON Schema and `serde_json::Value`. +//! +//! **Nothing here names a vendor** — not a provider, not a tool-slug prefix, not +//! a response-envelope field. The one piece of provider knowledge these walks +//! legitimately need — which root keys belong to a response envelope rather than +//! the tool's own payload — is a **parameter** +//! ([`compute_primary_array_path_from_value`]'s `skip_root_keys`), supplied by +//! the caller that owns that knowledge. + +use serde_json::Value; + +/// Dotted path to the shallowest array-typed property declared by `schema`. +/// +/// Breadth-first, so the shallowest match wins: every property at one level is +/// checked for `type: "array"` before descending, which matters when a schema +/// declares both a top-level list and a deeper one and a binding should prefer +/// the outer. +/// +/// `None` when `schema` is absent, declares no `properties`, or names no array +/// anywhere. +#[must_use] +pub fn compute_primary_array_path(schema: Option<&Value>) -> Option { + let root = schema?; + let mut queue: std::collections::VecDeque<(String, &Value)> = std::collections::VecDeque::new(); + queue.push_back((String::new(), root)); + + while let Some((path, node)) = queue.pop_front() { + let Some(props) = node.get("properties").and_then(Value::as_object) else { + continue; + }; + // Check every property at THIS level for an array before descending + // to the next level — guarantees the shallowest match wins. + for (key, prop_schema) in props { + let is_array = prop_schema.get("type").is_some_and(|kind| { + kind.as_str() == Some("array") + || kind.as_array().is_some_and(|types| { + types.iter().any(|value| value.as_str() == Some("array")) + }) + }); + if is_array { + let prop_path = if path.is_empty() { + key.clone() + } else { + format!("{path}.{key}") + }; + return Some(prop_path); + } + } + for (key, prop_schema) in props { + let prop_path = if path.is_empty() { + key.clone() + } else { + format!("{path}.{key}") + }; + queue.push_back((prop_path, prop_schema)); + } + } + None +} + +/// [`compute_primary_array_path`]'s counterpart for a REAL runtime value rather +/// than a schema — walks the value breadth-first for the first array-typed +/// property. +/// +/// `skip_root_keys` names properties to ignore **at the root only**. A caller +/// whose values arrive wrapped in a response envelope passes the envelope's own +/// metadata keys here, so an envelope field can never masquerade as (or shadow, +/// via the shallowest-wins tie) the tool's real array. Pass `&[]` when the value +/// has no envelope. +/// +/// Because the scan starts at whatever root it is given, a hit beneath a wrapper +/// property comes back already prefixed (e.g. `"data.issues"`) with no separate +/// stitching step needed. +/// +/// `None` when no array is found anywhere in the value — note a genuinely empty +/// list still serializes as `[]`, which *is* an array, so `None` means the shape +/// truly has no list at any depth. +#[must_use] +pub fn compute_primary_array_path_from_value( + value: &Value, + skip_root_keys: &[&str], +) -> Option { + let mut queue: std::collections::VecDeque<(String, &Value)> = std::collections::VecDeque::new(); + queue.push_back((String::new(), value)); + + while let Some((path, node)) = queue.pop_front() { + let Some(obj) = node.as_object() else { + continue; + }; + for (key, v) in obj { + if path.is_empty() && skip_root_keys.contains(&key.as_str()) { + continue; + } + if v.is_array() { + let prop_path = if path.is_empty() { + key.clone() + } else { + format!("{path}.{key}") + }; + return Some(prop_path); + } + } + for (key, v) in obj { + if path.is_empty() && skip_root_keys.contains(&key.as_str()) { + continue; + } + let prop_path = if path.is_empty() { + key.clone() + } else { + format!("{path}.{key}") + }; + queue.push_back((prop_path, v)); + } + } + None +} + +/// Extracts top-level field names from an output-schema value. +/// +/// Reads `.properties`' keys for a standard object schema +/// (`{"type": "object", "properties": {…}}`), and falls back to the schema's own +/// top-level keys only when no standard JSON-Schema keywords identify the +/// input as a schema. Empty when the schema is absent or a recognized schema +/// declares no properties — never fails. +#[must_use] +pub fn response_fields_from_schema(schema: Option<&Value>) -> Vec { + const SCHEMA_KEYWORDS: &[&str] = &[ + "type", + "required", + "additionalProperties", + "$schema", + "description", + "title", + "examples", + "$id", + "$anchor", + "$ref", + "$defs", + "definitions", + "properties", + "patternProperties", + "propertyNames", + "minProperties", + "maxProperties", + "dependencies", + "dependentRequired", + "dependentSchemas", + "unevaluatedProperties", + "contentEncoding", + "contentMediaType", + "contentSchema", + "default", + "deprecated", + "readOnly", + "writeOnly", + "const", + "enum", + "format", + "$comment", + "$vocabulary", + "$dynamicRef", + "$dynamicAnchor", + "allOf", + "anyOf", + "oneOf", + "not", + "if", + "then", + "else", + "minimum", + "maximum", + "exclusiveMinimum", + "exclusiveMaximum", + "multipleOf", + "minLength", + "maxLength", + "pattern", + "minItems", + "maxItems", + "uniqueItems", + "contains", + "minContains", + "maxContains", + "items", + "prefixItems", + "unevaluatedItems", + ]; + + let Some(obj) = schema.and_then(Value::as_object) else { + return Vec::new(); + }; + let mut fields: Vec = + if let Some(props) = obj.get("properties").and_then(Value::as_object) { + props.keys().cloned().collect() + } else { + obj.keys() + .filter(|k| !SCHEMA_KEYWORDS.contains(&k.as_str())) + .cloned() + .collect() + }; + fields.sort(); + fields +} + +/// Returns the names in `required` that are absent or `null` in `args`. +#[must_use] +pub fn missing_required_args(required: &[String], args: &Value) -> Vec { + required + .iter() + .filter(|name| match args.get(name.as_str()) { + None => true, + Some(v) => v.is_null(), + }) + .cloned() + .collect() +} + +/// Returns argument names in `args` that are NOT declared `properties` +/// of `schema` — the NAME-VALIDITY counterpart to [`missing_required_args`]'s +/// PRESENCE check. Catches the class of bug [`missing_required_args`] alone +/// cannot: a caller wires a real, well-typed value under an arg name the +/// tool's schema doesn't recognize at all (e.g. `text` when the tool actually +/// wants `markdown_text`) — the required arg still +/// LOOKS satisfied from [`missing_required_args`]' perspective (a value is +/// present under *some* key), so the mistake sails through authoring/save and +/// only fails at runtime. +/// +/// `None` means "cannot validate this schema — skip, never reject", so a +/// caller must never turn a `None` into a rejection: +/// - `schema` is `None` (the input schema is unknown for this slug), or +/// - `schema` is not a JSON object, or names no object `properties` map (an +/// unrecognized/legacy shape — nothing to check names against), or +/// - `schema` declares `additionalProperties: true` or a schema (the provider +/// explicitly allows arbitrary keys beyond the declared ones). +/// +/// Names matching `patternProperties` are accepted as declared names. +/// +/// `Some(vec![])` means the schema WAS usable and every arg name in `args` +/// is a real declared property. `args` must be a JSON object to check +/// against; any other shape (including `Value::Null` — no args wired at +/// all) yields `Some(vec![])`, mirroring [`missing_required_args`]'s treatment +/// of an absent/non-object `args`. +#[must_use] +pub fn unsupported_arg_names(schema: Option<&Value>, args: &Value) -> Option> { + let schema_obj = schema?.as_object()?; + if schema_obj + .get("additionalProperties") + .is_some_and(|value| value.as_bool() == Some(true) || value.is_object()) + { + return None; + } + let properties = schema_obj.get("properties")?.as_object()?; + let patterns = match schema_obj + .get("patternProperties") + .and_then(Value::as_object) + { + Some(patterns) => patterns + .keys() + .map(|pattern| regex::Regex::new(pattern)) + .collect::, _>>() + .ok()?, + None => Vec::new(), + }; + let Some(args_obj) = args.as_object() else { + return Some(Vec::new()); + }; + let mut unsupported: Vec = args_obj + .keys() + .filter(|key| { + if properties.contains_key(key.as_str()) { + return false; + } + let matches_pattern = patterns.iter().any(|compiled| compiled.is_match(key)); + !matches_pattern + }) + .cloned() + .collect(); + unsupported.sort(); + Some(unsupported) +} diff --git a/crates/tinyagents-harness/src/tool/schema_walk_test.rs b/crates/tinyagents-harness/src/tool/schema_walk_test.rs new file mode 100644 index 000000000..ddc2eb495 --- /dev/null +++ b/crates/tinyagents-harness/src/tool/schema_walk_test.rs @@ -0,0 +1,99 @@ +//! Tests for the JSON Schema shape walkers. + +use super::*; +use serde_json::json; + +#[test] +fn schema_array_path_prefers_the_shallowest_array() { + let schema = json!({"properties": { + "nested": {"properties": {"items": {"type": "array"}}}, + "top": {"type": "array"} + }}); + assert_eq!( + compute_primary_array_path(Some(&schema)).as_deref(), + Some("top") + ); +} + +#[test] +fn schema_array_path_recognizes_nullable_array_properties() { + let schema = json!({"properties": {"items": {"type": ["array", "null"]}}}); + assert_eq!( + compute_primary_array_path(Some(&schema)).as_deref(), + Some("items") + ); +} + +#[test] +fn value_array_path_skips_only_named_root_keys() { + let value = json!({"metadata": [], "data": {"metadata": []}}); + assert_eq!( + compute_primary_array_path_from_value(&value, &["metadata"]).as_deref(), + Some("data.metadata") + ); +} + +#[test] +fn response_fields_are_sorted_and_ignore_schema_keywords() { + let schema = json!({"type": "object", "z": {}, "a": {}}); + assert_eq!(response_fields_from_schema(Some(&schema)), ["a", "z"]); +} + +#[test] +fn response_fields_do_not_expose_json_schema_metadata() { + let schema = json!({"type": "object", "$id": "urn:example", "$defs": {"item": {}}}); + assert!(response_fields_from_schema(Some(&schema)).is_empty()); +} + +#[test] +fn required_arguments_treat_missing_and_null_as_absent() { + let required = vec!["missing".to_string(), "null".to_string(), "set".to_string()]; + assert_eq!( + missing_required_args(&required, &json!({"null": null, "set": 1})), + ["missing", "null"] + ); +} + +#[test] +fn unsupported_arguments_follow_schema_openness() { + let strict = json!({"properties": {"known": {}}}); + assert_eq!( + unsupported_arg_names(Some(&strict), &json!({"known": 1, "extra": 2})), + Some(vec!["extra".to_string()]) + ); + let open = json!({"properties": {"known": {}}, "additionalProperties": true}); + assert_eq!( + unsupported_arg_names(Some(&open), &json!({"extra": 2})), + None + ); +} + +#[test] +fn unsupported_arguments_follow_pattern_and_schema_additional_properties() { + let patterned = json!({ + "properties": {}, + "patternProperties": {"^x-": {}}, + "additionalProperties": false + }); + assert_eq!( + unsupported_arg_names(Some(&patterned), &json!({"x-note": "ok", "other": 1})), + Some(vec!["other".to_string()]) + ); + + let schema_additional = json!({"properties": {}, "additionalProperties": {"type": "string"}}); + assert_eq!( + unsupported_arg_names(Some(&schema_additional), &json!({"extra": "ok"})), + None + ); + + let unsupported_pattern = json!({ + "properties": {}, + "patternProperties": {"^(?!reserved$)[a-z]+$": {}}, + "additionalProperties": false + }); + assert_eq!( + unsupported_arg_names(Some(&unsupported_pattern), &json!({"custom": 1})), + None, + "patterns the regex crate cannot evaluate must disable name validation" + ); +} diff --git a/crates/tinyagents-harness/src/tool/shared/early_exit.rs b/crates/tinyagents-harness/src/tool/shared/early_exit.rs new file mode 100644 index 000000000..23b9473bd --- /dev/null +++ b/crates/tinyagents-harness/src/tool/shared/early_exit.rs @@ -0,0 +1,73 @@ +//! The early-exit hook: pause a run when a designated tool succeeds. + +use std::sync::{Arc, Mutex, PoisonError}; + +use crate::steering::{SteeringCommand, SteeringHandle}; + +/// A captured early-exit: a sub-agent invoked an early-exit tool (e.g. +/// `ask_user_clarification`), so the loop should pause and surface `question` +/// to the user. +#[derive(Debug, Clone)] +pub struct EarlyExit { + /// Name of the tool that requested the early exit. + pub tool: String, + /// The question (the tool's output) to surface to the user. + pub question: String, +} + +/// Shared early-exit hook handed to the adapters for the early-exit tool names. +/// On a successful call to one of those tools it records the [`EarlyExit`] and +/// sends a [`SteeringCommand::Pause`] so the harness loop short-circuits at the +/// next checkpoint (before the next model call) — so a sub-agent that asks the +/// user a question stops the loop instead of guessing. +#[derive(Clone)] +pub struct EarlyExitHook { + handle: SteeringHandle, + slot: Arc>>, +} + +impl std::fmt::Debug for EarlyExitHook { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("EarlyExitHook").finish_non_exhaustive() + } +} + +impl EarlyExitHook { + /// Build a hook that pauses `handle` and records into a fresh slot. + #[must_use] + pub fn new(handle: SteeringHandle) -> Self { + Self { + handle, + slot: Arc::new(Mutex::new(None)), + } + } + + /// The captured early-exit, if one fired during the run. + #[must_use] + pub fn take(&self) -> Option { + // Recover a poisoned slot rather than panic: a panic while some other + // tool held this lock must not swallow the early-exit. + self.slot + .lock() + .unwrap_or_else(PoisonError::into_inner) + .take() + } + + /// Record an early-exit and request a cooperative pause. Only the first + /// early-exit in a run is kept (halt on first). + pub(crate) fn trigger(&self, tool: &str, question: String) { + { + // `into_inner` keeps early-exit recording working even if the slot + // mutex was poisoned by an unrelated panic. + let mut slot = self.slot.lock().unwrap_or_else(PoisonError::into_inner); + if slot.is_none() { + *slot = Some(EarlyExit { + tool: tool.to_string(), + question, + }); + } + } + tracing::info!(tool, "[tinyagents] early-exit tool requesting pause"); + self.handle.send(SteeringCommand::Pause); + } +} diff --git a/crates/tinyagents-harness/src/tool/shared/mod.rs b/crates/tinyagents-harness/src/tool/shared/mod.rs new file mode 100644 index 000000000..1e437f9dd --- /dev/null +++ b/crates/tinyagents-harness/src/tool/shared/mod.rs @@ -0,0 +1,182 @@ +//! [`CanonicalSharedToolAdapter`]: a `tinytools::Tool` over a tool resolved +//! from shared, non-cloneable registries. +//! +//! The harness executes the canonical `tinytools::Tool` contract directly. A +//! host that keeps its tools in `Arc>>` sets shared across +//! sessions cannot hand the harness an owned tool, so this adapter resolves the +//! tool by name at call time, forwards spec, policy, exposure, family and +//! context, and preserves an optional [`EarlyExitHook`]. + +mod early_exit; + +use std::sync::Arc; + +use async_trait::async_trait; +use tinytools::{Tool, ToolCallOptions, ToolResult, ToolRunContext, ToolTimeout}; + +pub use early_exit::{EarlyExit, EarlyExitHook}; + +/// Canonical TinyTools adapter over shared tool sets. +/// +/// Resolves a non-cloneable tool from the shared registry on each call and +/// preserves the early-exit control hook. +pub struct CanonicalSharedToolAdapter { + sets: Vec>>>, + name: String, + description: String, + parameters_schema: serde_json::Value, + early_exit: Option, +} + +impl std::fmt::Debug for CanonicalSharedToolAdapter { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("CanonicalSharedToolAdapter") + .field("name", &self.name) + .field("early_exit", &self.early_exit.is_some()) + .finish_non_exhaustive() + } +} + +impl CanonicalSharedToolAdapter { + /// Builds an adapter for the tool called `name`, snapshotting its spec. + /// + /// Returns `None` when no set contains a tool with that name. + #[must_use] + pub fn for_name(sets: Vec>>>, name: &str) -> Option { + let spec = sets + .iter() + .flat_map(|set| set.iter()) + .find(|tool| tool.name() == name) + .map(|tool| tool.spec())?; + Some(Self { + sets, + name: spec.name, + description: spec.description, + parameters_schema: spec.parameters, + early_exit: None, + }) + } + + /// Attaches an [`EarlyExitHook`] fired after a successful call. + #[must_use] + pub fn with_early_exit(mut self, hook: EarlyExitHook) -> Self { + self.early_exit = Some(hook); + self + } + + fn resolved_tool(&self) -> Option<&dyn Tool> { + self.sets + .iter() + .flat_map(|set| set.iter()) + .find(|tool| tool.name() == self.name) + .map(|tool| tool.as_ref()) + } +} + +#[async_trait] +impl Tool for CanonicalSharedToolAdapter { + fn name(&self) -> &str { + &self.name + } + + fn description(&self) -> &str { + &self.description + } + + fn parameters_schema(&self) -> serde_json::Value { + self.parameters_schema.clone() + } + + fn policy(&self) -> tinytools::ToolPolicy { + self.resolved_tool().map(Tool::policy).unwrap_or_default() + } + + /// Forwarded so the harness indexes `Deferred` registrations for its + /// `tool_search` bridge instead of advertising them. Without this every + /// registered tool reported `Direct` and the bridge stayed inert. + /// + /// `Hidden` is **not** forwarded. The host is the exposure policy owner: + /// it already drops every `Hidden` registration from a wildcard belt, so a + /// `Hidden` tool that reaches harness registration was named by hand in a + /// belt or is a synthesised route the belt admitted. Forwarding `Hidden` + /// made the harness advertise only a subset of the visible tools while the + /// prompt described all of them, leaving calls the model was told about + /// unreachable. + fn exposure(&self) -> tinytools::ToolExposure { + match self.resolved_tool().map(Tool::exposure) { + Some(tinytools::ToolExposure::Deferred) => tinytools::ToolExposure::Deferred, + _ => tinytools::ToolExposure::Direct, + } + } + + fn family(&self) -> Option<&str> { + self.resolved_tool().and_then(Tool::family) + } + + fn injected_arguments(&self) -> Vec { + self.resolved_tool() + .map(Tool::injected_arguments) + .unwrap_or_default() + } + + fn supports_markdown(&self) -> bool { + self.resolved_tool().is_some_and(Tool::supports_markdown) + } + + fn is_concurrency_safe(&self, args: &serde_json::Value) -> bool { + self.resolved_tool() + .is_some_and(|tool| tool.is_concurrency_safe(args)) + } + + fn timeout_policy(&self, args: &serde_json::Value) -> ToolTimeout { + self.resolved_tool() + .map(|tool| tool.timeout_policy(args)) + .unwrap_or(ToolTimeout::Inherit) + } + + fn return_direct(&self) -> bool { + self.resolved_tool().is_some_and(Tool::return_direct) + } + + async fn execute(&self, args: serde_json::Value) -> anyhow::Result { + self.execute_with_context( + args, + ToolCallOptions { + prefer_markdown: true, + }, + None, + ) + .await + } + + async fn execute_with_context( + &self, + args: serde_json::Value, + options: ToolCallOptions, + context: Option<&dyn ToolRunContext>, + ) -> anyhow::Result { + let Some(tool) = self.resolved_tool() else { + tracing::warn!(tool = %self.name, "shared tool not found"); + return Ok(ToolResult::error(format!("unknown tool '{}'", self.name))); + }; + // A callable tool's operational failure is input to the agent loop, not + // a failure of the harness itself. Preserve it as an error result so + // the model can recover (or explain the failure) on its next round. + let result = match tool.execute_with_context(args, options, context).await { + Ok(result) => result, + Err(error) => { + tracing::warn!(tool = %self.name, %error, "shared tool execution failed"); + ToolResult::error(format!("{} failed: {error}", self.name)) + } + }; + if !result.is_error + && let Some(hook) = &self.early_exit + { + hook.trigger(&self.name, result.output_for_llm(true)); + } + Ok(result) + } +} + +#[cfg(test)] +mod test; diff --git a/crates/tinyagents-harness/src/tool/shared/test.rs b/crates/tinyagents-harness/src/tool/shared/test.rs new file mode 100644 index 000000000..29951b38f --- /dev/null +++ b/crates/tinyagents-harness/src/tool/shared/test.rs @@ -0,0 +1,336 @@ +//! Tests for the shared-registry adapter and its early-exit hook. + +use super::*; + +use crate::steering::SteeringHandle; + +use std::sync::atomic::{AtomicBool, Ordering}; + +use tinytools::{ToolContent, ToolPolicy}; + +struct RecordingTool { + saw_context: AtomicBool, +} + +#[async_trait] +impl Tool for RecordingTool { + fn name(&self) -> &str { + "recording" + } + + fn description(&self) -> &str { + "Preserves the canonical TinyTools contract." + } + + fn parameters_schema(&self) -> serde_json::Value { + serde_json::json!({"type": "object", "properties": {"fail": {"type": "boolean"}}}) + } + + fn policy(&self) -> ToolPolicy { + ToolPolicy::read_only().requiring_approval() + } + + async fn execute(&self, args: serde_json::Value) -> anyhow::Result { + self.execute_with_context(args, ToolCallOptions::default(), None) + .await + } + + async fn execute_with_context( + &self, + args: serde_json::Value, + options: ToolCallOptions, + context: Option<&dyn ToolRunContext>, + ) -> anyhow::Result { + self.saw_context.store( + context.is_some() && options.prefer_markdown, + Ordering::SeqCst, + ); + Ok(ToolResult { + content: vec![ + ToolContent::Text { + text: "plain content".to_string(), + }, + ToolContent::Json { + data: serde_json::json!({"preserved": true}), + }, + ], + is_error: args["fail"].as_bool().unwrap_or(false), + markdown_formatted: Some("markdown content".to_string()), + ..ToolResult::default() + }) + } +} + +struct TestToolRunContext; +impl ToolRunContext for TestToolRunContext {} + +#[tokio::test] +async fn canonical_adapter_preserves_spec_policy_context_and_result() { + let seen_context = Arc::new(AtomicBool::new(false)); + struct SharedRecordingTool(Arc); + #[async_trait] + impl Tool for SharedRecordingTool { + fn name(&self) -> &str { + "recording" + } + fn description(&self) -> &str { + "Preserves the canonical TinyTools contract." + } + fn parameters_schema(&self) -> serde_json::Value { + serde_json::json!({"type": "object"}) + } + fn policy(&self) -> ToolPolicy { + ToolPolicy::read_only().requiring_approval() + } + async fn execute(&self, args: serde_json::Value) -> anyhow::Result { + self.execute_with_context(args, ToolCallOptions::default(), None) + .await + } + async fn execute_with_context( + &self, + args: serde_json::Value, + options: ToolCallOptions, + context: Option<&dyn ToolRunContext>, + ) -> anyhow::Result { + self.0.store( + context.is_some() && options.prefer_markdown, + Ordering::SeqCst, + ); + Ok(ToolResult { + content: vec![ + ToolContent::Text { + text: "plain content".into(), + }, + ToolContent::Json { + data: serde_json::json!({"preserved": true}), + }, + ], + is_error: args["fail"].as_bool().unwrap_or(false), + markdown_formatted: Some("markdown content".into()), + ..ToolResult::default() + }) + } + } + let sets: Vec>>> = vec![Arc::new(vec![Box::new(SharedRecordingTool( + seen_context.clone(), + ))])]; + let adapter = CanonicalSharedToolAdapter::for_name(sets, "recording").expect("registered tool"); + + assert_eq!(adapter.name(), "recording"); + assert_eq!( + adapter.description(), + "Preserves the canonical TinyTools contract." + ); + assert_eq!( + adapter.parameters_schema(), + serde_json::json!({"type": "object"}) + ); + assert_eq!( + adapter.policy(), + ToolPolicy::read_only().requiring_approval() + ); + + let result = adapter + .execute_with_context( + serde_json::json!({"fail": true}), + ToolCallOptions::prefer_markdown(), + Some(&TestToolRunContext), + ) + .await + .expect("canonical result"); + assert!(seen_context.load(Ordering::SeqCst)); + assert!(result.is_error); + assert_eq!( + result.markdown_formatted.as_deref(), + Some("markdown content") + ); + assert_eq!(result.content.len(), 2); + assert!(matches!(result.content[0], ToolContent::Text { ref text } if text == "plain content")); + assert!( + matches!(result.content[1], ToolContent::Json { ref data } if data == &serde_json::json!({"preserved": true})) + ); +} + +#[tokio::test] +async fn canonical_adapter_fails_closed_when_the_registered_tool_is_gone() { + let adapter = CanonicalSharedToolAdapter { + sets: Vec::new(), + name: "missing".to_string(), + description: "missing".to_string(), + parameters_schema: serde_json::json!({"type": "object"}), + early_exit: None, + }; + + let result = adapter + .execute(serde_json::json!({})) + .await + .expect("reported failure"); + assert!(result.is_error); + assert_eq!(result.output(), "unknown tool 'missing'"); +} + +struct InjectedArgumentTool; + +#[async_trait] +impl Tool for InjectedArgumentTool { + fn name(&self) -> &str { + "injected" + } + + fn description(&self) -> &str { + "Declares a host-owned call identity." + } + + fn parameters_schema(&self) -> serde_json::Value { + serde_json::json!({"type": "object"}) + } + + fn injected_arguments(&self) -> Vec { + vec![tinytools::ToolInjectedArgument::tool_call_id("call_id")] + } + + async fn execute(&self, _args: serde_json::Value) -> anyhow::Result { + Ok(ToolResult::default()) + } +} + +#[test] +fn canonical_adapter_forwards_injected_argument_declarations() { + let sets: Vec>>> = vec![Arc::new(vec![Box::new(InjectedArgumentTool)])]; + let adapter = CanonicalSharedToolAdapter::for_name(sets, "injected").expect("registered tool"); + + assert_eq!( + adapter.injected_arguments(), + vec![tinytools::ToolInjectedArgument::tool_call_id("call_id")] + ); +} + +struct BehaviorTool; + +#[async_trait] +impl Tool for BehaviorTool { + fn name(&self) -> &str { + "behavior" + } + + fn description(&self) -> &str { + "Declares host-visible behavior." + } + + fn parameters_schema(&self) -> serde_json::Value { + serde_json::json!({"type": "object"}) + } + + fn supports_markdown(&self) -> bool { + true + } + + fn is_concurrency_safe(&self, args: &serde_json::Value) -> bool { + args["safe"].as_bool().unwrap_or(false) + } + + fn timeout_policy(&self, _args: &serde_json::Value) -> tinytools::ToolTimeout { + tinytools::ToolTimeout::Unbounded + } + + fn return_direct(&self) -> bool { + true + } + + async fn execute(&self, _args: serde_json::Value) -> anyhow::Result { + Ok(ToolResult::default()) + } +} + +#[test] +fn canonical_adapter_forwards_behavior_bearing_tool_methods() { + let sets: Vec>>> = vec![Arc::new(vec![Box::new(BehaviorTool)])]; + let adapter = CanonicalSharedToolAdapter::for_name(sets, "behavior").expect("registered tool"); + + assert!(adapter.supports_markdown()); + assert!(adapter.is_concurrency_safe(&serde_json::json!({"safe": true}))); + assert!(!adapter.is_concurrency_safe(&serde_json::json!({"safe": false}))); + assert_eq!( + adapter.timeout_policy(&serde_json::json!({})), + tinytools::ToolTimeout::Unbounded + ); + assert!(adapter.return_direct()); +} + +#[tokio::test] +async fn early_exit_only_fires_after_a_successful_canonical_result() { + let sets: Vec>>> = vec![Arc::new(vec![Box::new(RecordingTool { + saw_context: AtomicBool::new(false), + })])]; + let hook = EarlyExitHook::new(SteeringHandle::allow_all()); + let adapter = CanonicalSharedToolAdapter::for_name(sets, "recording") + .expect("registered tool") + .with_early_exit(hook.clone()); + + let failed = adapter + .execute(serde_json::json!({"fail": true})) + .await + .expect("reported failure"); + assert!(failed.is_error); + assert!( + hook.take().is_none(), + "reported tool errors must not pause the run" + ); + + let successful = adapter + .execute(serde_json::json!({"fail": false})) + .await + .expect("success"); + assert!(!successful.is_error); + let early_exit = hook + .take() + .expect("successful early-exit tool pauses the run"); + assert_eq!(early_exit.tool, "recording"); + assert_eq!(early_exit.question, "markdown content"); +} + +struct ExposedTool(&'static str, tinytools::ToolExposure); + +#[async_trait] +impl Tool for ExposedTool { + fn name(&self) -> &str { + self.0 + } + + fn description(&self) -> &str { + "exposure probe" + } + + fn parameters_schema(&self) -> serde_json::Value { + serde_json::json!({"type": "object"}) + } + + fn exposure(&self) -> tinytools::ToolExposure { + self.1 + } + + async fn execute(&self, _args: serde_json::Value) -> anyhow::Result { + Ok(ToolResult::default()) + } +} + +/// The host decides what a belt advertises; the harness only needs to know +/// which admitted registrations are searchable rather than advertised. A +/// `Hidden` tool that reached registration was named by the belt, so it is +/// advertised; `Deferred` stays deferred. +#[test] +fn adapter_advertises_admitted_hidden_tools_and_keeps_deferred_ones_deferred() { + let set: Arc>> = Arc::new(vec![ + Box::new(ExposedTool("direct", tinytools::ToolExposure::Direct)), + Box::new(ExposedTool("hidden", tinytools::ToolExposure::Hidden)), + Box::new(ExposedTool("deferred", tinytools::ToolExposure::Deferred)), + ]); + let exposure = |name: &str| { + CanonicalSharedToolAdapter::for_name(vec![set.clone()], name) + .expect("registered") + .exposure() + }; + assert_eq!(exposure("direct"), tinytools::ToolExposure::Direct); + assert_eq!(exposure("hidden"), tinytools::ToolExposure::Direct); + assert_eq!(exposure("deferred"), tinytools::ToolExposure::Deferred); +} diff --git a/crates/tinyagents-harness/src/tools/README.md b/crates/tinyagents-harness/src/tools/README.md index 1d66baff4..5bddcf8b9 100644 --- a/crates/tinyagents-harness/src/tools/README.md +++ b/crates/tinyagents-harness/src/tools/README.md @@ -27,8 +27,9 @@ other canonical tool. | File | Role | | --- | --- | | `mod.rs` | Feature gate, submodule wiring, and the `time` re-exports. | -| `time.rs` | `CurrentTimeTool`, `ResolveTimeTool`, and the expression parser (`resolve_expr`, `parse_relative_duration`, `ResolveZone`) backing them. | -| `time_test.rs` | Unit tests for both tools and the underlying expression parser. | +| `time.rs` | `CurrentTimeTool`, `ResolveTimeTool` and their JSON payloads. | +| `time_parse.rs` | The expression parser: durations (incl. compound `2h30m`), calendar phrases (`tomorrow at 9am`, `since Monday`), clock times, ISO forms; `resolve_expr[_at]`, `ResolveZone`. | +| `time_test.rs`, `time_parse_test.rs` | Unit tests for the tools and for the expression parser. | ## Operational notes diff --git a/crates/tinyagents-harness/src/tools/mod.rs b/crates/tinyagents-harness/src/tools/mod.rs index fc32a8fd6..a8a4845af 100644 --- a/crates/tinyagents-harness/src/tools/mod.rs +++ b/crates/tinyagents-harness/src/tools/mod.rs @@ -5,8 +5,11 @@ //! in extra dependencies by default. mod time; +mod time_parse; pub use time::{CurrentTimeTool, ResolveTimeTool, register_time_tools, time_tools}; +#[cfg(test)] +mod time_parse_test; #[cfg(test)] mod time_test; diff --git a/crates/tinyagents-harness/src/tools/time.rs b/crates/tinyagents-harness/src/tools/time.rs index 12f6ff202..a0e92d46a 100644 --- a/crates/tinyagents-harness/src/tools/time.rs +++ b/crates/tinyagents-harness/src/tools/time.rs @@ -5,18 +5,19 @@ //! They exist so a model can ground relative expressions ("in 10 minutes", //! "tomorrow") in an exact timestamp instead of hand-computing one, which //! models are unreliable at. Parsing (`resolve_expr`, `parse_relative_duration`) -//! and timezone handling (`ResolveZone`) are kept free of any `Tool` plumbing -//! so they are unit-testable on their own. +//! and timezone handling (`ResolveZone`) live in `time_parse.rs`, free of any +//! `Tool` plumbing so they are unit-testable on their own. use std::sync::Arc; use async_trait::async_trait; -use chrono::{DateTime, Duration, Local, NaiveDate, NaiveDateTime, SecondsFormat, Utc}; +use chrono::{DateTime, Local, SecondsFormat, Utc}; use chrono_tz::Tz; use serde_json::json; +use super::time_parse::{ResolveZone, resolve_expr}; use crate::tool::ToolRegistry; -use tinytools::{Tool, ToolPolicy, ToolResult}; +use tinytools::{Tool, ToolCallOptions, ToolPolicy, ToolResult}; /// Declared name of [`CurrentTimeTool`]. const CURRENT_TIME_NAME: &str = "current_time"; @@ -70,9 +71,60 @@ impl Tool for CurrentTimeTool { ToolPolicy::read_only() } + fn supports_markdown(&self) -> bool { + true + } + async fn execute(&self, arguments: serde_json::Value) -> anyhow::Result { - Ok(ToolResult::json(current_time_payload(&arguments))) + self.execute_with_options(arguments, ToolCallOptions::default()) + .await + } + + async fn execute_with_options( + &self, + arguments: serde_json::Value, + options: ToolCallOptions, + ) -> anyhow::Result { + tracing::debug!(args = %arguments, "[current_time] execute start"); + let payload = current_time_payload(&arguments); + let mut result = ToolResult::json(payload.clone()); + if options.prefer_markdown { + result = result.with_markdown(current_time_markdown(&payload)); + } + Ok(result) + } +} + +/// Renders the [`CurrentTimeTool`] payload as a compact markdown list. +fn current_time_markdown(payload: &serde_json::Value) -> String { + let text = |value: &serde_json::Value| value.as_str().unwrap_or("").to_string(); + let mut md = String::new(); + md.push_str(&format!("- **utc**: {}\n", text(&payload["utc"]))); + md.push_str(&format!( + "- **local**: {} ({})\n", + text(&payload["local"]), + text(&payload["local_timezone"]) + )); + md.push_str(&format!("- **weekday**: {}\n", text(&payload["weekday"]))); + md.push_str(&format!( + "- **unix_seconds**: {}\n", + payload["unix_seconds"].as_i64().unwrap_or(0) + )); + if let Some(requested) = payload.get("requested_timezone") { + md.push_str(&format!( + "- **{}**: {} ({})\n", + text(&requested["name"]), + text(&requested["time"]), + text(&requested["weekday"]) + )); } + if let Some(error) = payload + .get("requested_timezone_error") + .and_then(|value| value.as_str()) + { + md.push_str(&format!("- **timezone error**: {error}\n")); + } + md } /// Builds the JSON payload for [`CurrentTimeTool`]: always UTC + local time, @@ -142,8 +194,9 @@ impl Tool for ResolveTimeTool { Use this to produce date/time arguments for other tools instead of \ hand-computing Unix seconds. Accepted expressions include 'now', \ '24h ago', '7d', '2 weeks ago', 'in 10 minutes', '30m from now', \ - 'today', 'yesterday', 'tomorrow', RFC-3339 timestamps, bare dates, and \ - 'YYYY-MM-DD HH:MM:SS'." + 'today', 'yesterday', 'tomorrow', 'tomorrow at 9am', 'since Monday', \ + 'next Friday at 3:30 pm', '11pm tonight', RFC-3339 timestamps, bare dates, \ + and 'YYYY-MM-DD HH:MM:SS'." } fn parameters_schema(&self) -> serde_json::Value { @@ -172,11 +225,28 @@ impl Tool for ResolveTimeTool { ToolPolicy::read_only() } + fn supports_markdown(&self) -> bool { + true + } + async fn execute(&self, arguments: serde_json::Value) -> anyhow::Result { + self.execute_with_options(arguments, ToolCallOptions::default()) + .await + } + + async fn execute_with_options( + &self, + arguments: serde_json::Value, + options: ToolCallOptions, + ) -> anyhow::Result { + tracing::debug!(args = %arguments, "[resolve_time] execute start"); let expr = match arguments.get("expr").and_then(|value| value.as_str()) { Some(expr) => expr, None => { - return Ok(ToolResult::error("resolve_time: `expr` is required")); + return Ok(ToolResult::error( + "resolve_time: `expr` is required (e.g. \"24h ago\", \ + \"2026-06-09T19:12:00Z\", \"now\").", + )); } }; @@ -196,14 +266,43 @@ impl Tool for ResolveTimeTool { let dt = match resolve_expr(expr, zone) { Ok(dt) => dt, Err(error) => { + tracing::debug!(expr = expr, error = %error, "[resolve_time] parse failed"); return Ok(ToolResult::error(format!("resolve_time: {error}"))); } }; - Ok(ToolResult::json(resolve_time_payload(expr, &arguments, dt))) + let payload = resolve_time_payload(expr, &arguments, dt); + tracing::debug!( + "[resolve_time] resolved {expr:?} -> {} (unix_s={})", + payload["rfc3339"], + payload["unix_s"] + ); + let mut result = ToolResult::json(payload.clone()); + if options.prefer_markdown { + result = result.with_markdown(resolve_time_markdown(&payload)); + } + Ok(result) } } +/// Renders the [`ResolveTimeTool`] payload as a compact markdown list. +fn resolve_time_markdown(payload: &serde_json::Value) -> String { + let text = |key: &str| match &payload[key] { + serde_json::Value::String(value) => value.clone(), + other => other.to_string(), + }; + format!( + "- **interpreted**: {}\n- **value**: {}\n- **unix_s**: {}\n- **unix_ms**: {}\n\ + - **slack_ts**: {}\n- **rfc3339**: {}\n", + text("interpreted"), + text("value"), + text("unix_s"), + text("unix_ms"), + text("slack_ts"), + text("rfc3339"), + ) +} + /// Builds the JSON payload for [`ResolveTimeTool`]: the resolved instant /// rendered in every supported representation (`unix_s`/`unix_ms`/ /// `slack_ts`/`rfc3339`), with `args.format` (default `unix_s`) selecting @@ -241,162 +340,6 @@ fn resolve_time_payload( }) } -/// Parses a relative-time expression (`"24h ago"`, `"in 10 minutes"`, -/// `"next 2 weeks"`, `"-30m"`, ...) into a signed [`Duration`] to add to now. -/// Returns `None` when `raw` is not a recognised relative form (e.g. `"now"`, -/// an absolute date, or garbage), leaving [`resolve_expr`] to try the other -/// parse strategies. -pub(crate) fn parse_relative_duration(raw: &str) -> Option { - let mut text = raw.trim().to_ascii_lowercase(); - let mut future = false; - - if let Some(rest) = text.strip_suffix(" ago") { - text = rest.trim().to_string(); - } else if let Some(rest) = text.strip_suffix(" from now") { - future = true; - text = rest.trim().to_string(); - } - - for (prefix, is_future) in [ - ("in ", true), - ("next ", true), - ("last ", false), - ("past ", false), - ] { - if let Some(rest) = text.strip_prefix(prefix) { - future = is_future; - text = rest.trim().to_string(); - break; - } - } - - if let Some(rest) = text.strip_prefix('+') { - future = true; - text = rest.trim().to_string(); - } else if let Some(rest) = text.strip_prefix('-') { - text = rest.trim().to_string(); - } - - let text = text.split_whitespace().collect::>().join(" "); - let split_at = text.find(|ch: char| !ch.is_ascii_digit())?; - if split_at == 0 { - return None; - } - - let (num_str, unit_str) = text.split_at(split_at); - let n: i64 = num_str.trim().parse().ok()?; - let seconds_per = match unit_str.trim() { - "s" | "sec" | "secs" | "second" | "seconds" => 1, - "m" | "min" | "mins" | "minute" | "minutes" => 60, - "h" | "hr" | "hrs" | "hour" | "hours" => 3_600, - "d" | "day" | "days" => 86_400, - "w" | "wk" | "wks" | "week" | "weeks" => 604_800, - _ => return None, - }; - - let magnitude = Duration::seconds(n.saturating_mul(seconds_per)); - Some(if future { magnitude } else { -magnitude }) -} - -/// Resolves a free-form time expression to a UTC instant, trying each -/// recognised form in order: `"now"`, a relative duration -/// ([`parse_relative_duration`]), `"today"`/`"yesterday"`/`"tomorrow"` -/// (civil midnight in `zone`), RFC-3339, a handful of naive datetime formats, -/// then a bare `YYYY-MM-DD` date. Returns an error string (not -/// `TinyAgentsError`) so `ResolveTimeTool::execute` can surface it directly -/// as a tool-error result without wrapping. -pub(crate) fn resolve_expr( - expr: &str, - zone: ResolveZone, -) -> std::result::Result, String> { - let trimmed = expr.trim(); - if trimmed.is_empty() { - return Err("`expr` is required".to_string()); - } - let lower = trimmed.to_ascii_lowercase(); - - if lower == "now" { - return Ok(Utc::now()); - } - - if let Some(duration) = parse_relative_duration(trimmed) { - return Ok(Utc::now() + duration); - } - - if lower == "today" || lower == "yesterday" || lower == "tomorrow" { - let offset_days = match lower.as_str() { - "yesterday" => -1, - "tomorrow" => 1, - _ => 0, - }; - return zone.civil_midnight_to_utc(zone.now_civil_date() + Duration::days(offset_days)); - } - - if let Ok(dt) = DateTime::parse_from_rfc3339(trimmed) { - return Ok(dt.with_timezone(&Utc)); - } - - for format in ["%Y-%m-%d %H:%M:%S", "%Y-%m-%dT%H:%M:%S", "%Y-%m-%d %H:%M"] { - if let Ok(naive) = NaiveDateTime::parse_from_str(trimmed, format) { - return zone.naive_to_utc(naive); - } - } - - if let Ok(date) = NaiveDate::parse_from_str(trimmed, "%Y-%m-%d") { - return zone.civil_midnight_to_utc(date); - } - - Err(format!("could not parse time expression {trimmed:?}")) -} - -/// Timezone an offset-less expression (`"today"`, a naive datetime, a bare -/// date) is interpreted in, when [`ResolveTimeTool`]'s optional `timezone` -/// argument was not given or was invalid. -#[derive(Clone, Copy)] -pub(crate) enum ResolveZone { - /// The machine's local timezone. - Local, - /// An explicit IANA timezone. - Iana(Tz), -} - -impl ResolveZone { - /// Today's civil (wall-clock) date in this zone. - fn now_civil_date(&self) -> NaiveDate { - match self { - ResolveZone::Local => Local::now().date_naive(), - ResolveZone::Iana(tz) => Utc::now().with_timezone(tz).date_naive(), - } - } - - /// Converts a civil date's midnight in this zone to a UTC instant. - fn civil_midnight_to_utc(&self, date: NaiveDate) -> std::result::Result, String> { - let naive = date - .and_hms_opt(0, 0, 0) - .ok_or_else(|| "invalid civil midnight".to_string())?; - self.naive_to_utc(naive) - } - - /// Converts a naive (offset-less) datetime, interpreted in this zone, to - /// a UTC instant. Errors on an ambiguous or nonexistent local time (a DST - /// fold or gap), where a single unambiguous mapping does not exist. - fn naive_to_utc(&self, naive: NaiveDateTime) -> std::result::Result, String> { - use chrono::TimeZone; - match self { - ResolveZone::Local => Local - .from_local_datetime(&naive) - .single() - .map(|dt| dt.with_timezone(&Utc)) - .ok_or_else(|| format!("ambiguous or invalid local time {naive}")), - ResolveZone::Iana(tz) => tz - .from_local_datetime(&naive) - .single() - .map(|dt| dt.with_timezone(&Utc)) - .ok_or_else(|| format!("ambiguous or invalid time {naive} in {tz:?}")), - } - } -} - /// Returns the builtin time tool set. pub fn time_tools() -> Vec> { vec![ diff --git a/crates/tinyagents-harness/src/tools/time_parse.rs b/crates/tinyagents-harness/src/tools/time_parse.rs new file mode 100644 index 000000000..6f1632374 --- /dev/null +++ b/crates/tinyagents-harness/src/tools/time_parse.rs @@ -0,0 +1,359 @@ +//! Time-expression parsing for [`ResolveTimeTool`](super::ResolveTimeTool). +//! +//! Turns a relative or absolute phrase into an exact UTC instant, free of any +//! `Tool` plumbing so it is unit-testable on its own. Accepts durations +//! (`24h ago`, `in 2h30m`), conversational calendar phrases (`tomorrow at 9am`, +//! `since Monday`, `next Friday at 3:30 pm`, `11 PM tonight`), ISO dates and +//! timestamps. Bare durations look backward; `in` and `next` look forward. +//! Civil dates and clock times are interpreted in a [`ResolveZone`]. + +use chrono::{ + DateTime, Datelike, Duration, Local, NaiveDate, NaiveDateTime, NaiveTime, Utc, Weekday, +}; +use chrono_tz::Tz; + +/// Parse a relative-duration expression into a **signed** [`Duration`] offset +/// from now: negative = the past, positive = the future. +/// +/// Direction comes from `ago`, `last`, `past`, or `-` for the past and `in`, +/// `next`, `from now`, or `+` for the future. A bare duration looks backward. +/// +/// Getting the sign right matters: future phrasing like `"in 10 minutes"` must +/// resolve forward, not backward. Compound forms (`"2h30m"`, `"1 hour and 30 +/// minutes ago"`) are summed; overflow yields `None`. +pub(crate) fn parse_relative_duration(raw: &str) -> Option { + let mut s = raw.trim().to_ascii_lowercase(); + let mut future = false; + + // Suffix direction markers. + if let Some(rest) = s.strip_suffix(" ago") { + s = rest.trim().to_string(); + } else if let Some(rest) = s.strip_suffix(" from now") { + future = true; + s = rest.trim().to_string(); + } + // Prefix direction markers (first match wins). + for (prefix, is_future) in [ + ("in ", true), + ("next ", true), + ("last ", false), + ("past ", false), + ] { + if let Some(rest) = s.strip_prefix(prefix) { + future = is_future; + s = rest.trim().to_string(); + break; + } + } + if let Some(rest) = s.strip_prefix('+') { + future = true; + s = rest.trim().to_string(); + } else if let Some(rest) = s.strip_prefix('-') { + // Leading '-' is the past — which is already the default — just strip it. + s = rest.trim().to_string(); + } + + // Split both "2 hours 30 minutes" and "2h30m" into number/unit pairs. + let mut tokens = Vec::new(); + let mut current = String::new(); + let mut was_digit = None; + for c in s.chars() { + if c.is_ascii_digit() || c.is_ascii_alphabetic() { + let is_digit = c.is_ascii_digit(); + if was_digit.is_some_and(|previous| previous != is_digit) && !current.is_empty() { + tokens.push(std::mem::take(&mut current)); + } + current.push(c); + was_digit = Some(is_digit); + } else if c.is_whitespace() || c == ',' { + if !current.is_empty() { + tokens.push(std::mem::take(&mut current)); + } + was_digit = None; + } else { + return None; + } + } + if !current.is_empty() { + tokens.push(current); + } + let mut parts = tokens.into_iter().filter(|token| token != "and"); + let mut seconds = 0_i64; + let mut any = false; + while let Some(number) = parts.next() { + let n: i64 = number.parse().ok()?; + let unit = parts.next()?; + let secs_per = match unit.as_str() { + "s" | "sec" | "secs" | "second" | "seconds" => 1, + "m" | "min" | "mins" | "minute" | "minutes" => 60, + "h" | "hr" | "hrs" | "hour" | "hours" => 3_600, + "d" | "day" | "days" => 86_400, + "w" | "wk" | "wks" | "week" | "weeks" => 604_800, + _ => return None, + }; + seconds = seconds.checked_add(n.checked_mul(secs_per)?)?; + any = true; + } + if !any { + return None; + } + let magnitude = Duration::try_seconds(seconds)?; + Some(if future { magnitude } else { -magnitude }) +} + +/// Parses a lowercase weekday name or abbreviation. +fn parse_weekday(name: &str) -> Option { + match name { + "monday" | "mon" => Some(Weekday::Mon), + "tuesday" | "tue" | "tues" => Some(Weekday::Tue), + "wednesday" | "wed" => Some(Weekday::Wed), + "thursday" | "thu" | "thur" | "thurs" => Some(Weekday::Thu), + "friday" | "fri" => Some(Weekday::Fri), + "saturday" | "sat" => Some(Weekday::Sat), + "sunday" | "sun" => Some(Weekday::Sun), + _ => None, + } +} + +/// Parses a conversational clock time (`9am`, `3:30 pm`, `noon`, `midnight`, +/// `at 21:05`) into a [`NaiveTime`]. +fn parse_clock_time(raw: &str) -> Option { + let compact = raw + .trim() + .trim_start_matches("at ") + .replace([' ', '.'], "") + .to_ascii_uppercase(); + match compact.as_str() { + "NOON" => return NaiveTime::from_hms_opt(12, 0, 0), + "MIDNIGHT" => return NaiveTime::from_hms_opt(0, 0, 0), + _ => {} + } + let (clock, meridiem) = if let Some(clock) = compact.strip_suffix("AM") { + (clock, Some(false)) + } else if let Some(clock) = compact.strip_suffix("PM") { + (clock, Some(true)) + } else { + (compact.as_str(), None) + }; + let parts: Vec<&str> = clock.split(':').collect(); + if parts.is_empty() || parts.len() > 3 { + return None; + } + let mut hour: u32 = parts[0].parse().ok()?; + let minute: u32 = parts.get(1).map_or(Some(0), |s| s.parse().ok())?; + let second: u32 = parts.get(2).map_or(Some(0), |s| s.parse().ok())?; + if let Some(pm) = meridiem { + if !(1..=12).contains(&hour) { + return None; + } + hour = hour % 12 + if pm { 12 } else { 0 }; + } + NaiveTime::from_hms_opt(hour, minute, second) +} + +/// A calendar phrase may be a day on its own or a day plus a clock time. +/// Unqualified weekdays refer to the most recent occurrence, which makes +/// "since Monday" useful for history queries; "next" is always in the future. +fn resolve_calendar_phrase( + lower: &str, + zone: &ResolveZone, + now: DateTime, +) -> Option, String>> { + let words: Vec<&str> = lower.split_whitespace().collect(); + let first = *words.first()?; + let anchor_len = if matches!(first, "last" | "next" | "this" | "since") + && words + .get(1) + .is_some_and(|word| parse_weekday(word).is_some()) + { + 2 + } else if matches!(first, "today" | "tomorrow" | "yesterday" | "tonight") + || parse_weekday(first).is_some() + { + 1 + } else { + return None; + }; + let anchor = words[..anchor_len].join(" "); + let time_words = words[anchor_len..].join(" "); + let time = if time_words.is_empty() { + // "Tonight" is a period, not an exact instant. Require a clock time + // rather than silently inventing one for a reminder. + if anchor == "tonight" { + return None; + } + NaiveTime::from_hms_opt(0, 0, 0)? + } else { + parse_clock_time(&time_words)? + }; + let today = zone.civil_date(now); + let date = match anchor.as_str() { + "today" => today, + "tonight" if time == NaiveTime::from_hms_opt(0, 0, 0)? => today + Duration::days(1), + "tonight" => today, + "yesterday" => today - Duration::days(1), + "tomorrow" => today + Duration::days(1), + _ => { + let (direction, weekday_name) = anchor + .split_once(' ') + .map_or(("since", anchor.as_str()), |(a, b)| (a, b)); + let weekday = parse_weekday(weekday_name)?; + let current = today.weekday().num_days_from_monday() as i64; + let target = weekday.num_days_from_monday() as i64; + let days = match direction { + "next" => { + let ahead = (target - current).rem_euclid(7); + if ahead == 0 { 7 } else { ahead } + } + "this" => target - current, + "last" => { + let behind = (current - target).rem_euclid(7); + if behind == 0 { -7 } else { -behind } + } + _ => -(current - target).rem_euclid(7), + }; + today + Duration::days(days) + } + }; + Some(zone.naive_to_utc(date.and_time(time))) +} + +/// Resolves `expr` to a UTC instant relative to the real clock. See +/// [`resolve_expr_at`]. +pub(crate) fn resolve_expr(expr: &str, zone: ResolveZone) -> Result, String> { + resolve_expr_at(expr, zone, Utc::now()) +} + +/// Resolves `expr` to a UTC instant relative to an injected `now`, so the +/// conversational forms are deterministic under test. +/// +/// Tries, in order: `now`, relative durations, calendar phrases (`tomorrow at +/// 9am`, `since Monday`), RFC-3339, naive datetimes, bare dates, a date plus a +/// clock time, and time-first forms such as `9am next Friday`. +pub(crate) fn resolve_expr_at( + expr: &str, + zone: ResolveZone, + now: DateTime, +) -> Result, String> { + let trimmed = expr.trim(); + if trimmed.is_empty() { + return Err( + "`expr` is required (e.g. \"24h ago\", \"2026-06-09T19:12:00Z\", \"now\").".into(), + ); + } + let lower = trimmed.to_ascii_lowercase(); + + if lower == "now" { + return Ok(now); + } + + // Relative duration → signed offset from now (sign encodes past/future). + if let Some(dur) = parse_relative_duration(trimmed) { + return now + .checked_add_signed(dur) + .ok_or_else(|| "relative time is outside the supported date range".to_string()); + } + + if let Some(result) = resolve_calendar_phrase(&lower, &zone, now) { + return result; + } + + // RFC-3339 / ISO-8601 with explicit offset (e.g. ...Z, +05:30). + if let Ok(dt) = DateTime::parse_from_rfc3339(trimmed) { + return Ok(dt.with_timezone(&Utc)); + } + + // "YYYY-MM-DD HH:MM:SS" or "YYYY-MM-DDTHH:MM:SS" (no offset) → resolve in zone. + for fmt in [ + "%Y-%m-%d %H:%M:%S", + "%Y-%m-%dT%H:%M:%S", + "%Y-%m-%d %H:%M", + "%Y-%m-%dT%H:%M", + ] { + if let Ok(naive) = NaiveDateTime::parse_from_str(trimmed, fmt) { + return zone.naive_to_utc(naive); + } + } + + // Bare date "YYYY-MM-DD" → civil midnight in zone. + if let Ok(date) = NaiveDate::parse_from_str(trimmed, "%Y-%m-%d") { + return zone.civil_midnight_to_utc(date); + } + + // Date with a conversational clock time, e.g. "2026-06-09 at 9am". + if let Some((date_text, clock_text)) = trimmed.split_once(' ') + && let (Ok(date), Some(time)) = ( + NaiveDate::parse_from_str(date_text, "%Y-%m-%d"), + parse_clock_time(clock_text), + ) + { + return zone.naive_to_utc(date.and_time(time)); + } + + // Accept time-first forms too: "11pm tonight", "9am next Friday". + let words: Vec<&str> = lower.split_whitespace().collect(); + for anchor_len in [2, 1] { + if words.len() > anchor_len { + let split = words.len() - anchor_len; + let clock = words[..split].join(" "); + if parse_clock_time(&clock).is_some() { + let reordered = format!("{} {clock}", words[split..].join(" ")); + if let Some(result) = resolve_calendar_phrase(&reordered, &zone, now) { + return result; + } + } + } + } + + Err(format!( + "could not parse {trimmed:?}; try 'last 24 hours', 'tomorrow at 9am', \ + 'since Monday', or an ISO date/time." + )) +} + +/// Timezone used to interpret civil (offset-less) inputs: `"today"`, weekdays, +/// clock times, naive datetimes and bare dates. +pub(crate) enum ResolveZone { + /// The machine's local timezone (the default, matching `current_time`). + Local, + /// An explicit IANA timezone supplied by the caller. + Iana(Tz), +} + +impl ResolveZone { + /// The civil (wall-clock) date of `now` in this zone. + fn civil_date(&self, now: DateTime) -> NaiveDate { + match self { + ResolveZone::Local => now.with_timezone(&Local).date_naive(), + ResolveZone::Iana(tz) => now.with_timezone(tz).date_naive(), + } + } + + /// Converts a civil date's midnight in this zone to a UTC instant. + fn civil_midnight_to_utc(&self, date: NaiveDate) -> Result, String> { + let naive = date + .and_hms_opt(0, 0, 0) + .ok_or_else(|| "invalid civil midnight".to_string())?; + self.naive_to_utc(naive) + } + + /// Interprets a naive (offset-less) datetime in this zone and converts it to UTC. + /// Errors on an ambiguous or nonexistent local time (a DST fold or gap). + fn naive_to_utc(&self, naive: NaiveDateTime) -> Result, String> { + use chrono::TimeZone; + match self { + ResolveZone::Local => Local + .from_local_datetime(&naive) + .single() + .map(|dt| dt.with_timezone(&Utc)) + .ok_or_else(|| format!("ambiguous or invalid local time {naive} (DST boundary?)")), + ResolveZone::Iana(tz) => tz + .from_local_datetime(&naive) + .single() + .map(|dt| dt.with_timezone(&Utc)) + .ok_or_else(|| { + format!("ambiguous or invalid time {naive} in {tz:?} (DST boundary?)") + }), + } + } +} diff --git a/crates/tinyagents-harness/src/tools/time_parse_test.rs b/crates/tinyagents-harness/src/tools/time_parse_test.rs new file mode 100644 index 000000000..5f01d7561 --- /dev/null +++ b/crates/tinyagents-harness/src/tools/time_parse_test.rs @@ -0,0 +1,180 @@ +//! Tests for time-expression parsing. + +use chrono::{DateTime, SecondsFormat, Utc}; +use chrono_tz::Tz; + +use super::time_parse::{ResolveZone, parse_relative_duration, resolve_expr, resolve_expr_at}; + +#[test] +fn past_variants_resolve_to_a_negative_offset() { + // Past phrasing (and bare durations, which default to the past) yield a + // NEGATIVE offset so `now + dur` looks backward. + for s in [ + "24h ago", + "last 24 hours", + "past 24 hours", + "-24h", + "24 hours ago", + "24h", + ] { + let d = parse_relative_duration(s).unwrap_or_else(|| panic!("failed: {s}")); + assert_eq!(d.num_seconds(), -86_400, "{s}"); + } + assert_eq!( + parse_relative_duration("7d").unwrap().num_seconds(), + -604_800 + ); + assert_eq!( + parse_relative_duration("2 weeks").unwrap().num_seconds(), + -1_209_600 + ); + assert_eq!(parse_relative_duration("15m").unwrap().num_seconds(), -900); + assert_eq!( + parse_relative_duration("30 days").unwrap().num_seconds(), + -2_592_000 + ); +} + +#[test] +fn future_variants_resolve_to_a_positive_offset() { + // Regression for the CodeRabbit catch: future phrasing must look + // FORWARD (positive offset), not backward — scheduler_agent relies on + // "in 10 minutes" / "30m from now". + assert_eq!( + parse_relative_duration("in 10 minutes") + .unwrap() + .num_seconds(), + 600 + ); + assert_eq!( + parse_relative_duration("30m from now") + .unwrap() + .num_seconds(), + 1_800 + ); + assert_eq!(parse_relative_duration("+2h").unwrap().num_seconds(), 7_200); + assert_eq!( + parse_relative_duration("next 7d").unwrap().num_seconds(), + 604_800 + ); +} + +#[test] +fn combined_durations_are_accepted() { + assert_eq!( + parse_relative_duration("1 hour and 30 minutes ago") + .unwrap() + .num_seconds(), + -5_400 + ); + assert_eq!( + parse_relative_duration("in 2h30m").unwrap().num_seconds(), + 9_000 + ); + assert!(parse_relative_duration("in 2 hours and nonsense").is_none()); +} + +#[test] +fn conversational_dates_resolve_in_the_requested_timezone() { + let zone: Tz = "Asia/Kolkata".parse().unwrap(); + let now = DateTime::parse_from_rfc3339("2026-01-07T12:00:00Z") + .unwrap() + .with_timezone(&Utc); + for (expr, expected) in [ + ("tomorrow 9am", "2026-01-08T03:30:00Z"), + ("next Friday at 3:30 pm", "2026-01-09T10:00:00Z"), + ("since Monday", "2026-01-04T18:30:00Z"), + ("last Wednesday", "2025-12-30T18:30:00Z"), + ("2026-01-09 at 9am", "2026-01-09T03:30:00Z"), + ("2026-01-09T09:00", "2026-01-09T03:30:00Z"), + ("11 PM tonight", "2026-01-07T17:30:00Z"), + ("tonight at midnight", "2026-01-07T18:30:00Z"), + ("9am next Friday", "2026-01-09T03:30:00Z"), + ("tomorrow at noon", "2026-01-08T06:30:00Z"), + ] { + let actual = resolve_expr_at(expr, ResolveZone::Iana(zone), now).unwrap(); + assert_eq!( + actual.to_rfc3339_opts(SecondsFormat::Secs, true), + expected, + "{expr}" + ); + } + assert!(resolve_expr_at("tonight", ResolveZone::Iana(zone), now).is_err()); +} + +#[test] +fn rejects_non_durations() { + assert!(parse_relative_duration("now").is_none()); + assert!(parse_relative_duration("2026-06-09").is_none()); + assert!(parse_relative_duration("h").is_none()); + assert!(parse_relative_duration("24 lightyears").is_none()); +} + +#[test] +fn resolves_rfc3339_to_exact_utc() { + let dt = resolve_expr("2026-06-09T19:12:00Z", ResolveZone::Local).unwrap(); + // The exact epoch the real incident's agent miscomputed as 1752189120. + assert_eq!(dt.timestamp(), 1_781_032_320); +} + +#[test] +fn relative_is_close_to_now_minus_offset() { + let before = Utc::now().timestamp(); + let dt = resolve_expr("24h ago", ResolveZone::Local).unwrap(); + let after = Utc::now().timestamp(); + let expected_lo = before - 86_400 - 2; + let expected_hi = after - 86_400 + 2; + assert!( + dt.timestamp() >= expected_lo && dt.timestamp() <= expected_hi, + "got {}, expected ~[{expected_lo},{expected_hi}]", + dt.timestamp() + ); +} + +#[test] +fn future_relative_resolves_forward() { + // "in 10 minutes" must land ~600s in the FUTURE (the bug fix). + let before = Utc::now().timestamp(); + let dt = resolve_expr("in 10 minutes", ResolveZone::Local).unwrap(); + let after = Utc::now().timestamp(); + assert!( + dt.timestamp() >= before + 600 - 2 && dt.timestamp() <= after + 600 + 2, + "got {}, expected ~now+600", + dt.timestamp() + ); +} + +#[test] +fn tomorrow_is_after_today_after_yesterday() { + let tz: Tz = "Asia/Kolkata".parse().unwrap(); + let y = resolve_expr("yesterday", ResolveZone::Iana(tz)).unwrap(); + let t = resolve_expr("today", ResolveZone::Iana(tz)).unwrap(); + let m = resolve_expr("tomorrow", ResolveZone::Iana(tz)).unwrap(); + assert!(y < t && t < m, "ordering broken: {y} {t} {m}"); + // Consecutive civil days are exactly 24h apart. + assert_eq!((t - y).num_seconds(), 86_400); + assert_eq!((m - t).num_seconds(), 86_400); +} + +#[test] +fn now_resolves() { + let dt = resolve_expr("now", ResolveZone::Local).unwrap(); + assert!((dt.timestamp() - Utc::now().timestamp()).abs() <= 2); +} + +#[test] +fn bare_date_in_explicit_zone() { + // 2026-06-09 00:00 in Asia/Kolkata (UTC+5:30) == 2026-06-08T18:30:00Z. + let tz: Tz = "Asia/Kolkata".parse().unwrap(); + let dt = resolve_expr("2026-06-09", ResolveZone::Iana(tz)).unwrap(); + assert_eq!( + dt.to_rfc3339_opts(SecondsFormat::Secs, true), + "2026-06-08T18:30:00Z" + ); +} + +#[test] +fn unparseable_expr_errors() { + assert!(resolve_expr("sometime next quarter", ResolveZone::Local).is_err()); + assert!(resolve_expr("", ResolveZone::Local).is_err()); +} diff --git a/crates/tinyagents-harness/src/tools/time_test.rs b/crates/tinyagents-harness/src/tools/time_test.rs index 74c3513df..994ed302e 100644 --- a/crates/tinyagents-harness/src/tools/time_test.rs +++ b/crates/tinyagents-harness/src/tools/time_test.rs @@ -1,6 +1,6 @@ //! Tests for builtin time/date tools. -use super::time; +use super::time_parse; use super::*; use chrono::{SecondsFormat, Utc}; use chrono_tz::Tz; @@ -33,6 +33,10 @@ async fn current_time_returns_utc_local_and_unix_seconds() { assert!(payload["local"].is_string()); assert!(payload["local_timezone"].is_string()); assert!(payload["unix_seconds"].is_number()); + assert!(matches!( + result.content.as_slice(), + [tinytools::ToolContent::Json { .. }] + )); } #[tokio::test] @@ -70,11 +74,11 @@ fn relative_past_variants_are_negative_offsets() { "24 hours ago", "24h", ] { - let duration = time::parse_relative_duration(expr).unwrap(); + let duration = time_parse::parse_relative_duration(expr).unwrap(); assert_eq!(duration.num_seconds(), -86_400, "{expr}"); } assert_eq!( - time::parse_relative_duration("2 weeks") + time_parse::parse_relative_duration("2 weeks") .unwrap() .num_seconds(), -1_209_600 @@ -84,23 +88,25 @@ fn relative_past_variants_are_negative_offsets() { #[test] fn relative_future_variants_are_positive_offsets() { assert_eq!( - time::parse_relative_duration("in 10 minutes") + time_parse::parse_relative_duration("in 10 minutes") .unwrap() .num_seconds(), 600 ); assert_eq!( - time::parse_relative_duration("30m from now") + time_parse::parse_relative_duration("30m from now") .unwrap() .num_seconds(), 1_800 ); assert_eq!( - time::parse_relative_duration("+2h").unwrap().num_seconds(), + time_parse::parse_relative_duration("+2h") + .unwrap() + .num_seconds(), 7_200 ); assert_eq!( - time::parse_relative_duration("next 7d") + time_parse::parse_relative_duration("next 7d") .unwrap() .num_seconds(), 604_800 @@ -109,11 +115,12 @@ fn relative_future_variants_are_positive_offsets() { #[test] fn resolve_expr_handles_exact_rfc3339_and_explicit_zone_dates() { - let dt = time::resolve_expr("2026-06-09T19:12:00Z", time::ResolveZone::Local).unwrap(); + let dt = + time_parse::resolve_expr("2026-06-09T19:12:00Z", time_parse::ResolveZone::Local).unwrap(); assert_eq!(dt.timestamp(), 1_781_032_320); let tz: Tz = "Asia/Kolkata".parse().unwrap(); - let dt = time::resolve_expr("2026-06-09", time::ResolveZone::Iana(tz)).unwrap(); + let dt = time_parse::resolve_expr("2026-06-09", time_parse::ResolveZone::Iana(tz)).unwrap(); assert_eq!( dt.to_rfc3339_opts(SecondsFormat::Secs, true), "2026-06-08T18:30:00Z" @@ -123,8 +130,8 @@ fn resolve_expr_handles_exact_rfc3339_and_explicit_zone_dates() { #[test] fn relative_resolution_tracks_now_with_expected_sign() { let before = Utc::now().timestamp(); - let past = time::resolve_expr("24h ago", time::ResolveZone::Local).unwrap(); - let future = time::resolve_expr("in 10 minutes", time::ResolveZone::Local).unwrap(); + let past = time_parse::resolve_expr("24h ago", time_parse::ResolveZone::Local).unwrap(); + let future = time_parse::resolve_expr("in 10 minutes", time_parse::ResolveZone::Local).unwrap(); let after = Utc::now().timestamp(); assert!(past.timestamp() >= before - 86_400 - 2); @@ -150,6 +157,10 @@ async fn resolve_time_returns_all_formats_and_selected_value() { assert_eq!(payload["unix_ms"], 1_781_032_320_000_i64); assert_eq!(payload["slack_ts"], "1781032320.000000"); assert_eq!(payload["value"], "1781032320.000000"); + assert!(matches!( + result.content.as_slice(), + [tinytools::ToolContent::Json { .. }] + )); } #[tokio::test] @@ -167,3 +178,95 @@ async fn resolve_time_errors_for_missing_expr_and_bad_timezone() { assert!(bad_zone.is_error); assert!(bad_zone.output().contains("unknown IANA timezone")); } + +#[tokio::test] +async fn resolve_time_rejects_out_of_range_duration_without_panicking() { + let result = ResolveTimeTool::new() + .execute(json!({ "expr": "in 9223372036854776s" })) + .await + .unwrap(); + assert!(result.is_error); + assert!(result.output().contains("could not parse")); +} + +#[tokio::test] +async fn resolve_time_accepts_conversational_phrases() { + let result = ResolveTimeTool::new() + .execute(json!({ "expr": "tomorrow at 9am", "timezone": "Asia/Kolkata" })) + .await + .unwrap(); + assert!(!result.is_error, "{}", result.output()); + let payload: serde_json::Value = serde_json::from_str(&result.output()).unwrap(); + assert!(payload["unix_s"].is_i64()); +} + +#[test] +fn time_tools_are_read_only_and_support_markdown() { + use tinytools::PermissionLevel; + + for tool in time_tools() { + assert!(tool.supports_markdown(), "{}", tool.name()); + assert_eq!(tool.permission_level(), PermissionLevel::ReadOnly); + } +} + +#[tokio::test] +async fn current_time_markdown_only_when_preferred() { + use tinytools::ToolCallOptions; + + let tool = CurrentTimeTool::new(); + let args = json!({ "timezone": "Asia/Kolkata" }); + let plain = tool.execute(args.clone()).await.unwrap(); + assert!(plain.markdown_formatted.is_none()); + + let result = tool + .execute_with_options(args, ToolCallOptions::prefer_markdown()) + .await + .unwrap(); + let md = result.markdown_formatted.expect("markdown rendering"); + assert!(md.contains("- **utc**: "), "{md}"); + assert!(md.contains("- **Asia/Kolkata**: "), "{md}"); + + let bad = tool + .execute_with_options( + json!({ "timezone": "Not/AZone" }), + ToolCallOptions::prefer_markdown(), + ) + .await + .unwrap(); + assert!( + bad.markdown_formatted + .unwrap() + .contains("- **timezone error**: ") + ); +} + +#[tokio::test] +async fn resolve_time_markdown_lists_every_representation() { + use tinytools::ToolCallOptions; + + let tool = ResolveTimeTool::new(); + let args = json!({ "expr": "2026-06-09T19:12:00Z", "format": "slack_ts" }); + assert!( + tool.execute(args.clone()) + .await + .unwrap() + .markdown_formatted + .is_none() + ); + + let md = tool + .execute_with_options(args, ToolCallOptions::prefer_markdown()) + .await + .unwrap() + .markdown_formatted + .expect("markdown rendering"); + assert!( + md.contains("- **interpreted**: 2026-06-09T19:12:00Z\n"), + "{md}" + ); + assert!(md.contains("- **value**: 1781032320.000000\n"), "{md}"); + assert!(md.contains("- **unix_s**: 1781032320\n"), "{md}"); + assert!(md.contains("- **unix_ms**: 1781032320000\n"), "{md}"); + assert!(md.contains("- **rfc3339**: 2026-06-09T19:12:00Z\n"), "{md}"); +} diff --git a/docs/modules/graph/goals.md b/docs/modules/graph/goals.md index 657e3f9eb..763206597 100644 --- a/docs/modules/graph/goals.md +++ b/docs/modules/graph/goals.md @@ -90,3 +90,8 @@ Unit tests in `crates/tinyagents-graph/src/goals/test.rs` (types, store, tools, budget enforcement on `InMemoryStore`); an end-to-end self-driving loop in `tests/e2e_graph_goals.rs`; feature coverage for budget accounting and the mid-turn guard in `tests/feature_graph_goal_budget.rs`. + +Every control answers with `{ "goal": , "text": }` +(`text` is also the markdown form). A host that must react to a write registers +`GoalTool::with_update_hook` (a `GoalUpdateHook`), which runs after each +successful set / complete / pause / resume with the persisted goal. diff --git a/vendor/tinyinference b/vendor/tinyinference index c29d5118e..98d669455 160000 --- a/vendor/tinyinference +++ b/vendor/tinyinference @@ -1 +1 @@ -Subproject commit c29d5118eddfa12b83c266d7d5f07f6918dc3787 +Subproject commit 98d6694559e52b90ceaeda713a17c1b380a153a4 diff --git a/vendor/tinytools b/vendor/tinytools index 82c0d975e..f3fd59720 160000 --- a/vendor/tinytools +++ b/vendor/tinytools @@ -1 +1 @@ -Subproject commit 82c0d975e2667e1c525afe525006134431641785 +Subproject commit f3fd597206202cd8271ee59167a84b8b2523a277