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-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/tools/time.rs b/crates/tinyagents-harness/src/tools/time.rs index cd14c61e0..6ecbaeb0a 100644 --- a/crates/tinyagents-harness/src/tools/time.rs +++ b/crates/tinyagents-harness/src/tools/time.rs @@ -17,7 +17,7 @@ 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"; @@ -71,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::success(serde_json::to_string_pretty(&payload)?); + 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, @@ -174,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\").", + )); } }; @@ -198,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::success(serde_json::to_string_pretty(&payload)?); + 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 diff --git a/crates/tinyagents-harness/src/tools/time_test.rs b/crates/tinyagents-harness/src/tools/time_test.rs index 136265ca8..7fd5954cd 100644 --- a/crates/tinyagents-harness/src/tools/time_test.rs +++ b/crates/tinyagents-harness/src/tools/time_test.rs @@ -191,3 +191,74 @@ async fn resolve_time_accepts_conversational_phrases() { 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 613df41fb..98d669455 160000 --- a/vendor/tinyinference +++ b/vendor/tinyinference @@ -1 +1 @@ -Subproject commit 613df41fbad1f901f6752deea0fe53f7abba46f6 +Subproject commit 98d6694559e52b90ceaeda713a17c1b380a153a4 diff --git a/vendor/tinytools b/vendor/tinytools index 657362585..f3fd59720 160000 --- a/vendor/tinytools +++ b/vendor/tinytools @@ -1 +1 @@ -Subproject commit 6573625852b6420addb2eb2125b0de83d02b7902 +Subproject commit f3fd597206202cd8271ee59167a84b8b2523a277