diff --git a/crates/tinyagents-harness/src/agent_loop/README.md b/crates/tinyagents-harness/src/agent_loop/README.md index c98e64aea..dd35032a4 100644 --- a/crates/tinyagents-harness/src/agent_loop/README.md +++ b/crates/tinyagents-harness/src/agent_loop/README.md @@ -110,6 +110,30 @@ deterministic) and enabled per policy via `retry::RetryPolicy::with_backoff_sleep`. A real provider integration retries after a genuine, growing delay while unit tests stay sleep-free. +## Calls written on a turn with no callable tool + +A request can withdraw its tools — a final wrap-up call, a spent research +budget — or set `ToolChoice::None`, while its transcript is still full of tool +calls. A native model then has no tool channel and writes its next call as +plain text in its own markup (DeepSeek V4: `<｜DSML｜invoke name="shell">…`). +Declaring the tools with `tool_choice: "none"` does not prevent it. Taken as +the final answer, that markup ends the turn with a stray command where the +result should be. + +On such a turn `TextRecovery::withholding` is in force: the streaming +scrubber and, for unary replies, `withhold_text_calls` remove any complete +call the tool-call grammars recognise from the visible text, keep reasoning, +and count it in `DroppedBlocks::withheld`. Nothing is dispatched — the +I-2 rule (prose never becomes a call when none could be accepted) holds. If a +call was withheld and a model call remains, the loop drops that assistant row +and re-prompts once with `WITHHELD_TOOL_CALL_NUDGE`, which says plainly that +tools are unavailable, emitting `ControlApplied { control: +"withheld_tool_call" }` and `RetryScheduled`. Re-prompts are bounded by +`RunPolicy::dropped_tool_call_nudges` and run before the empty-reply retries, +since a bare re-send of the same transcript leaks the same way. With no call +left, the scrubbed reply stands. Language-tagged fenced examples and bare JSON +answers are left untouched. + ## Truncated-empty recovery Local reasoning models (for example `qwen3` via Ollama) intermittently spend diff --git a/crates/tinyagents-harness/src/agent_loop/dialect.rs b/crates/tinyagents-harness/src/agent_loop/dialect.rs index 3f3a6e7c6..5392b3e7f 100644 --- a/crates/tinyagents-harness/src/agent_loop/dialect.rs +++ b/crates/tinyagents-harness/src/agent_loop/dialect.rs @@ -344,6 +344,11 @@ pub(super) struct TextRecovery { /// Call blocks the model attempted that produced no call, on the /// attempt whose response the loop is about to read. pub(super) dropped: Arc, + /// No tool could be called this turn (none offered, or an effective + /// `ToolChoice::None`), yet call markup is still scrubbed from the + /// visible text and counted in [`DroppedBlocks::withheld`] instead of + /// becoming a call. See [`TextRecovery::withholding`]. + pub(super) withhold: bool, } /// Tool-call blocks a grammar recognised on one model attempt that did not @@ -357,6 +362,9 @@ pub(super) struct DroppedBlocks { /// Openers with no closer at end of text (`UnterminatedBlock`); the raw /// markup is released as visible text. unterminated: AtomicUsize, + /// Complete calls the model wrote on a turn that could not take one; + /// scrubbed from the visible text and never dispatched. + withheld: AtomicUsize, } impl DroppedBlocks { @@ -370,11 +378,32 @@ impl DroppedBlocks { self.unterminated.load(Ordering::Relaxed) } + /// Calls written on a turn that offered no callable tool. + pub(super) fn withheld(&self) -> usize { + self.withheld.load(Ordering::Relaxed) + } + /// Forgets the counts: the next response comes from a new attempt, or /// the attempt that produced them failed and its response is discarded. pub(super) fn reset(&self) { self.malformed.store(0, Ordering::Relaxed); self.unterminated.store(0, Ordering::Relaxed); + self.withheld.store(0, Ordering::Relaxed); + } + + /// Counts `calls` complete calls withheld on a turn that could not take + /// them. + fn record_withheld(&self, calls: usize, model_call_id: &CallId) { + if calls == 0 { + return; + } + self.withheld.fetch_add(calls, Ordering::Relaxed); + tracing::warn!( + target: "tinyagents::agent_loop", + call_id = %model_call_id, + withheld = calls, + "[agent_loop] tool call written on a turn with no callable tools; scrubbed, not run" + ); } /// Adds the dropped blocks among `diagnostics`, logging any it finds. @@ -403,10 +432,28 @@ impl DroppedBlocks { } impl TextRecovery { + /// Recovery for a turn on which no tool can be called: nothing is ever + /// recovered as a call, but call markup is still kept out of the answer. + /// + /// A request that withdraws its tools (a final wrap-up call, a spent + /// research budget, a summarizer) still carries a transcript full of tool + /// calls, and a native model with no tool channel open writes its next + /// call in its own markup as plain text: DeepSeek V4's + /// `<｜DSML｜invoke name="shell">…`. In replayed bench captures that + /// happened on most such requests, and declaring the tools with + /// `tool_choice: "none"` did not stop it. Read as a final answer the + /// markup ended the turn with a stray command in place of a result. + pub(super) fn withholding() -> Self { + Self { + withhold: true, + ..Self::default() + } + } + /// A scrubber for one streamed model call, or `None` when no tools were - /// offered and there is nothing to recover. + /// offered and no markup has to be withheld either. pub(super) fn scrubber(&self, model_call_id: &CallId) -> Option { - (!self.offered.is_empty()).then(|| { + (!self.offered.is_empty() || self.withhold).then(|| { // A fresh scrubber is one provider attempt; a retried attempt // must not inherit the previous one's count. self.dropped.reset(); @@ -416,10 +463,34 @@ impl TextRecovery { self.registry.clone(), Arc::clone(&self.dropped), ) + .withholding(self.withhold) }) } } +/// Scrubs tool-call markup out of a completed response on a turn that could +/// not take a call, counting what it removed in `dropped` and dispatching +/// nothing. Reasoning and other non-text blocks survive. +/// +/// The streamed path does the same incrementally through a withholding +/// [`DeltaScrubber`]; this covers a unary reply, and a streamed one whose +/// terminal text was not reconciled from the deltas. +pub(super) fn withhold_text_calls( + response: &mut ModelResponse, + model_call_id: &CallId, + dropped: &DroppedBlocks, +) { + let text = response.text(); + // Bare JSON stays: an answer may legitimately be a JSON object. + let outcome = tinytools_agent::parse_text(&text, &ParseOptions::new().without_bare_json()); + if outcome.calls.is_empty() { + return; + } + dropped.record_withheld(outcome.calls.len(), model_call_id); + response.message.content = + replace_text_blocks(std::mem::take(&mut response.message.content), outcome.text); +} + /// How one model call is made: streamed or unary, and what it needs to /// recover text-dialect calls from the answer. #[derive(Debug, Clone, Default)] @@ -533,6 +604,11 @@ pub(super) struct DeltaScrubber { model_call_id: CallId, calls: Vec, dropped: Arc, + /// Scrub complete calls without collecting them (a turn with no callable + /// tool); they are counted in [`DroppedBlocks::withheld`] instead. + withhold: bool, + /// Complete calls withheld on this attempt. + withheld: usize, } impl DeltaScrubber { @@ -553,9 +629,17 @@ impl DeltaScrubber { model_call_id, calls: Vec::new(), dropped, + withhold: false, + withheld: 0, } } + /// Scrub calls without dispatching them (see [`TextRecovery::withholding`]). + pub(super) fn withholding(mut self, withhold: bool) -> Self { + self.withhold = withhold; + self + } + /// Feeds one text delta; returns the text safe to forward. pub(super) fn feed(&mut self, text: &str) -> String { let step = self.inner.feed(text); @@ -572,6 +656,12 @@ impl DeltaScrubber { fn collect(&mut self, calls: Vec, diagnostics: &[ParseDiagnostic]) { self.dropped.record(diagnostics, &self.model_call_id); + if self.withhold { + self.withheld += calls.len(); + self.dropped + .record_withheld(calls.len(), &self.model_call_id); + return; + } for call in calls { let slot = self.calls.len() + 1; self.calls @@ -588,4 +678,9 @@ impl DeltaScrubber { pub(super) fn has_calls(&self) -> bool { !self.calls.is_empty() } + + /// Whether any complete call was withheld during the stream. + pub(super) fn has_withheld(&self) -> bool { + self.withheld > 0 + } } diff --git a/crates/tinyagents-harness/src/agent_loop/model_call.rs b/crates/tinyagents-harness/src/agent_loop/model_call.rs index e64dd22d8..3d93356b4 100644 --- a/crates/tinyagents-harness/src/agent_loop/model_call.rs +++ b/crates/tinyagents-harness/src/agent_loop/model_call.rs @@ -1178,9 +1178,12 @@ impl AgentHarness { // alongside the structured calls the scrubber recovered below, // and gets persisted into the transcript to be replayed back to // the model next turn. + // A withheld call (a turn with no callable tool) was scrubbed the + // same way, so its raw markup must not come back from the + // terminal text either. let scrubber_recovered_calls = text_scrubber .as_ref() - .is_some_and(super::dialect::DeltaScrubber::has_calls); + .is_some_and(|scrubber| scrubber.has_calls() || scrubber.has_withheld()); if let ModelStreamItem::Completed(response) = &mut item && (saw_streamed_content || scrubber_recovered_calls) { diff --git a/crates/tinyagents-harness/src/agent_loop/run_loop.rs b/crates/tinyagents-harness/src/agent_loop/run_loop.rs index 67610f5f9..1d514cdee 100644 --- a/crates/tinyagents-harness/src/agent_loop/run_loop.rs +++ b/crates/tinyagents-harness/src/agent_loop/run_loop.rs @@ -405,6 +405,9 @@ impl AgentHarness { // Consecutive "you said tool_calls but sent none" re-prompts // (see `RunPolicy::dropped_tool_call_nudges`). let mut dropped_tool_call_nudges_used: u32 = 0; + // Consecutive re-prompts after a call written on a turn with no + // callable tool (bounded by the same `dropped_tool_call_nudges`). + let mut withheld_call_nudges_used: u32 = 0; let mut boosted_max_tokens: Option = None; let mut truncation_base: Option = None; @@ -1005,9 +1008,13 @@ impl AgentHarness { offered: Arc::new(request.tools.clone()), registry: dialect.registry_for(&request.tools), dropped: Arc::default(), + withhold: false, } } else { - super::dialect::TextRecovery::default() + // Nothing can be recovered as a call, but a call the model + // writes anyway is kept out of the answer (see + // `TextRecovery::withholding`). + super::dialect::TextRecovery::withholding() }; // Applied before budget preflight below: for a text dialect this // rewrite folds the protocol block and full tool catalogue into @@ -1174,6 +1181,15 @@ impl AgentHarness { if forced_text_dialect || text_dialect_recovery_enabled { recover_text_dialect_calls(ctx, &mut response, &call_id, &recovery); } + // A turn that could not take a call: whatever call markup the + // model wrote is scrubbed from the answer and never run. Applied + // whatever `text_dialect_recovery` says — that policy decides + // whether prose can *become* a call, and here none can. A + // streamed reply was already scrubbed delta by delta; this + // catches a unary one. + if recovery.withhold { + super::dialect::withhold_text_calls(&mut response, &call_id, &recovery.dropped); + } // Account for the completed provider response before fallible // response middleware. A middleware rejection must not erase @@ -1429,6 +1445,7 @@ impl AgentHarness { // otherwise receive fewer than the policy's configured number // of consecutive re-prompts. dropped_tool_call_nudges_used = 0; + withheld_call_nudges_used = 0; empty_response_retries_used = 0; reset_truncated_empty_recovery( &mut truncated_empty_retries_used, @@ -1470,6 +1487,48 @@ impl AgentHarness { } if real_tool_calls.is_empty() { + // A call written on a turn that could not take one (tools + // withdrawn for a concluding answer, or `ToolChoice::None`). + // It was scrubbed and not run; what is left is either nothing + // or a lead-in to a step that never happened, so it is not the + // answer the request asked for. Drop that row and ask once + // more, telling the model plainly that tools are gone. + // Replaying the bench request that leaked (DeepSeek V4, tools + // withdrawn), the unchanged request leaked 6 times in 8; with + // the row dropped and this re-prompt added it leaked 0 times + // in 12. Runs before the empty-reply retries: a bare re-send + // of the same transcript leaks the same way. + let withheld_calls = recovery.dropped.withheld(); + if withheld_calls > 0 + && withheld_call_nudges_used < self.policy.dropped_tool_call_nudges + && ctx.limits.remaining_model_calls() > 0 + { + withheld_call_nudges_used += 1; + messages.pop(); + tracing::info!( + target: "tinyagents::agent_loop", + run_id = %ctx.run_id(), + call_id = %call_id, + withheld_calls, + attempt = withheld_call_nudges_used, + "[agent_loop] re-prompting after a tool call on a turn with no callable tools" + ); + ctx.emit(AgentEvent::ControlApplied { + control: "withheld_tool_call".to_string(), + detail: format!( + "{withheld_calls} tool call(s) written while no tool was callable \ + in model call `{call_id}`; scrubbed, not run, re-prompted" + ), + }); + messages.push(Message::user(WITHHELD_TOOL_CALL_NUDGE)); + let record = ctx.emit(AgentEvent::RetryScheduled { + call_id: call_id.clone(), + attempt: withheld_call_nudges_used as usize, + }); + status.set_last_event(record.id); + continue; + } + // Truncated-empty recovery (runs before structured extraction, // which would otherwise fail on the empty completion). A local // reasoning model can burn the whole token budget on its hidden @@ -1596,6 +1655,7 @@ impl AgentHarness { continue; } dropped_tool_call_nudges_used = 0; + withheld_call_nudges_used = 0; empty_response_retries_used = 0; // This turn resolved without scheduling a truncated-empty @@ -1710,6 +1770,7 @@ impl AgentHarness { // recovery state before the tools run so the next turn starts from // the caller's configured cap and a full retry budget. dropped_tool_call_nudges_used = 0; + withheld_call_nudges_used = 0; empty_response_retries_used = 0; reset_truncated_empty_recovery( &mut truncated_empty_retries_used, @@ -2437,6 +2498,13 @@ const DROPPED_TOOL_CALL_NUDGE: &str = "Your previous turn indicated a tool call included. If you meant to call a tool, issue the actual tool call now; otherwise answer \ directly."; +/// The re-prompt sent when the model wrote a tool call on a turn that offered +/// no callable tool. The call was scrubbed and not run; the wording names that +/// plainly, because a model told only to "answer" keeps trying to act. +const WITHHELD_TOOL_CALL_NUDGE: &str = "Your previous reply was a tool call, but tools are not \ + available for this reply, so it did not run. Do not write tool calls. Answer now in plain \ + text from the results already gathered, and state any remaining uncertainty."; + /// The re-prompt sent when a text-dialect tool-call block could not be /// decoded: no tool ran, and the model should know why rather than assume /// its call went through. @@ -2513,6 +2581,10 @@ fn reset_truncated_empty_recovery( #[path = "run_loop_recovery_tests.rs"] mod recovery_tests; +#[cfg(test)] +#[path = "run_loop_withheld_tests.rs"] +mod withheld_tests; + /// Attaches `default` to `request` when the request carries no reasoning /// config of its own. A request-level config always wins. pub(crate) fn apply_default_reasoning( diff --git a/crates/tinyagents-harness/src/agent_loop/run_loop_recovery_tests.rs b/crates/tinyagents-harness/src/agent_loop/run_loop_recovery_tests.rs index 1374e4646..1f66e9a4c 100644 --- a/crates/tinyagents-harness/src/agent_loop/run_loop_recovery_tests.rs +++ b/crates/tinyagents-harness/src/agent_loop/run_loop_recovery_tests.rs @@ -17,6 +17,7 @@ fn offered(names: &[&str]) -> TextRecovery { ), registry: None, dropped: Arc::default(), + withhold: false, } } diff --git a/crates/tinyagents-harness/src/agent_loop/run_loop_withheld_tests.rs b/crates/tinyagents-harness/src/agent_loop/run_loop_withheld_tests.rs new file mode 100644 index 000000000..49b93850f --- /dev/null +++ b/crates/tinyagents-harness/src/agent_loop/run_loop_withheld_tests.rs @@ -0,0 +1,176 @@ +//! A tool call written on a turn that could not take one: scrubbed from the +//! answer, never run, and re-prompted once (`TextRecovery::withholding`). +//! +//! The replies below are what DeepSeek V4 returned in bench captures when a +//! request withdrew its tools over a transcript full of tool calls. + +use std::sync::Arc; + +use super::*; +use crate::agent_loop::dialect::{TextRecovery, withhold_text_calls}; +use crate::context::RunConfig; +use crate::limits::RunLimits; +use crate::runtime::{AgentHarness, RunPolicy}; +use crate::testkit::{ScriptedModel, StreamingMock}; +use tinyinference_llm::message::{ContentBlock, MessageDelta}; +use tinyinference_llm::model::{ModelResponse, ModelStreamItem}; + +const DSML_CALL: &str = "<｜｜DSML｜｜ calls>\n<｜｜DSML｜｜ invoke name=\"shell\">\n\ +<｜｜DSML｜｜ parameter name=\"command\" string=\"true\">cd /app && grep -rn jsonpath pkg/\n\ +\n"; + +fn leaked_reply() -> String { + format!("I'll implement this from the spec. Let me first check the modules.\n\n{DSML_CALL}") +} + +fn has_markup(text: &str) -> bool { + text.contains("DSML") || text.contains("invoke name") +} + +#[test] +fn a_withheld_call_is_scrubbed_counted_and_never_becomes_a_call() { + let recovery = TextRecovery::withholding(); + let mut response = ModelResponse::assistant(leaked_reply()); + response.message.content.insert( + 0, + ContentBlock::Thinking { + text: "check the modules first".into(), + signature: None, + }, + ); + + withhold_text_calls(&mut response, &CallId::new("model-1"), &recovery.dropped); + + assert!( + response.message.tool_calls.is_empty(), + "nothing is dispatched" + ); + assert_eq!(recovery.dropped.withheld(), 1); + assert!(!has_markup(&response.text())); + assert!(response.text().contains("implement this from the spec")); + assert!( + matches!(response.message.content[0], ContentBlock::Thinking { .. }), + "reasoning survives the scrub" + ); +} + +#[test] +fn withholding_leaves_a_quoted_example_and_a_json_answer_alone() { + let recovery = TextRecovery::withholding(); + for text in [ + "The format is:\n```xml\nls\n```", + "{\"name\": \"shell\", \"arguments\": {\"command\": \"ls\"}}", + ] { + let mut response = ModelResponse::assistant(text); + withhold_text_calls(&mut response, &CallId::new("model-1"), &recovery.dropped); + assert_eq!(response.text(), text); + } + assert_eq!(recovery.dropped.withheld(), 0); +} + +fn harness_with( + model: Arc>, + max_calls: usize, +) -> AgentHarness<()> { + let mut harness: AgentHarness<()> = AgentHarness::new(); + harness.register_model("mock", model); + harness.with_policy(RunPolicy { + limits: RunLimits::default().with_max_model_calls(max_calls), + ..RunPolicy::default() + }); + harness +} + +#[tokio::test] +async fn a_call_on_a_turn_without_tools_is_withheld_and_reprompted() { + let model = Arc::new(ScriptedModel::replies(vec![ + leaked_reply(), + "The jsonpath module is not registered yet; here is the plan.".to_string(), + ])); + let harness = harness_with(model.clone(), 5); + + let run = harness + .invoke_default(&(), vec![Message::user("Answer now from what you have.")]) + .await + .expect("run succeeds"); + + assert_eq!( + run.text().as_deref(), + Some("The jsonpath module is not registered yet; here is the plan.") + ); + let requests = model.requests(); + assert_eq!(requests.len(), 2, "one re-prompt"); + let retry = &requests[1].messages; + assert_eq!( + retry.last().map(Message::text).as_deref(), + Some(WITHHELD_TOOL_CALL_NUDGE) + ); + assert!( + retry.iter().all(|m| !has_markup(&m.text())), + "the leaked row is not replayed to the model" + ); + assert!(run.messages.iter().all(|m| !has_markup(&m.text()))); +} + +#[tokio::test] +async fn with_no_call_left_the_scrubbed_reply_stands_without_markup() { + let model = Arc::new(ScriptedModel::replies(vec![leaked_reply()])); + let harness = harness_with(model.clone(), 1); + + let run = harness + .invoke_default(&(), vec![Message::user("Answer now.")]) + .await + .expect("run succeeds"); + + assert_eq!( + model.requests().len(), + 1, + "no call was left for a re-prompt" + ); + let text = run.text().unwrap_or_default(); + assert!(!has_markup(&text)); + assert!(text.contains("implement this from the spec")); +} + +#[tokio::test] +async fn a_streamed_call_on_a_turn_without_tools_never_reaches_the_answer() { + let chunks = [ + "I'll check first. ", + "<｜｜DSML｜｜ calls>\n<｜｜DSML｜｜ invoke name=\"shell\">\n", + "<｜｜DSML｜｜ parameter name=\"command\" string=\"true\">ls\n", + "\n", + ]; + let mut items = vec![ModelStreamItem::Started]; + items.extend( + chunks + .iter() + .map(|text| ModelStreamItem::MessageDelta(MessageDelta::text(*text))), + ); + items.push(ModelStreamItem::Completed(ModelResponse::assistant( + chunks.concat(), + ))); + // The mock replays the same leak on every call, so the run spends its + // re-prompts and then keeps the scrubbed reply. + let model = Arc::new(StreamingMock::new(items)); + let harness = harness_with(model.clone(), 10); + + let run = harness + .invoke_streaming( + &(), + (), + RunConfig::new("withheld-stream"), + vec![Message::user("Answer now.")], + ) + .await + .expect("streaming run succeeds"); + + let text = run.text().unwrap_or_default(); + assert!(!has_markup(&text), "got {text:?}"); + assert!(text.contains("I'll check first.")); + assert_eq!( + model.call_count(), + 1 + u64::from(RunPolicy::default().dropped_tool_call_nudges), + "re-prompted up to the nudge budget, then answered" + ); + assert!(run.messages.iter().all(|m| !has_markup(&m.text()))); +} diff --git a/crates/tinyagents-harness/src/summarization/README.md b/crates/tinyagents-harness/src/summarization/README.md index b4b4792a0..07a1d2a48 100644 --- a/crates/tinyagents-harness/src/summarization/README.md +++ b/crates/tinyagents-harness/src/summarization/README.md @@ -117,6 +117,16 @@ loop. `Message::text()`**, specifically so a tool-driven run's compacted history keeps its tool calls/results/reasoning instead of collapsing to empty strings. +- **`ModelSummarizer` fences the transcript and asks last.** Its user message + is `…` (after any ``), then the + instruction. A bare transcript that ends on a tool result reads as a live + agent loop, and a model continues it with its next tool call. The request + declares no tools, so that call arrives as plain-text markup (DeepSeek V4: + `<｜DSML｜invoke …>`). +- **A tool call is never accepted as a summary.** A reply the tool-call + grammars parse as a call is retried once, then returned as an error, so + `FaultTolerantCachingSummarizer` falls back to its deterministic trim + instead of installing one stray command as the whole compacted history. - This module never calls into the agent loop or decides *when* to invoke a `Summarizer` on its own — `should_summarize`/`plan` are pure decisions the caller (the agent loop, a middleware) acts on. diff --git a/crates/tinyagents-harness/src/summarization/model_summarizer.rs b/crates/tinyagents-harness/src/summarization/model_summarizer.rs index 0ab4e3a1e..6ae603a31 100644 --- a/crates/tinyagents-harness/src/summarization/model_summarizer.rs +++ b/crates/tinyagents-harness/src/summarization/model_summarizer.rs @@ -87,12 +87,7 @@ impl ModelSummarizer { .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, - }; + let request_text = summary_request_text(&transcript, previous_summary); tracing::info!( model = %self.model_id, @@ -103,17 +98,9 @@ impl ModelSummarizer { let request = ModelRequest::new(vec![ Message::system(SUMMARIZER_SYSTEM_PROMPT), - Message::user(transcript), + Message::user(request_text), ]); - 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 = self.summarize_once(request).await?; let summary = summary.trim(); if summary.is_empty() { @@ -148,6 +135,88 @@ impl ModelSummarizer { } } +impl ModelSummarizer { + /// One summary, with a single retry when the reply is a tool call rather + /// than a summary. + /// + /// The request declares no tools, so a model that answers with a call has + /// no structured channel for it and the provider returns its native call + /// markup as plain text (DeepSeek's `<｜DSML｜invoke …>`). Installed as the + /// summary, that markup would replace the whole compacted head with one + /// stray command. A retry usually lands a real summary; if it does not, + /// the error lets [`super::FaultTolerantCachingSummarizer`] fall back to + /// its deterministic trim instead of keeping the markup. + async fn summarize_once(&self, request: ModelRequest) -> Result { + let mut last_chars = 0; + for attempt in 1..=SUMMARY_MARKUP_ATTEMPTS { + let text = self + .model + .invoke(&(), request.clone()) + .await + .map_err(|e| { + tracing::warn!(error = %e, "[tinyagents::summarize] summarizer model call failed"); + TinyAgentsError::Model(format!("summarizer model call failed: {e}")) + })? + .text(); + if !contains_tool_call_markup(&text) { + return Ok(text); + } + last_chars = text.chars().count(); + tracing::warn!( + model = %self.model_id, + attempt, + reply_chars = last_chars, + "[tinyagents::summarize] summarizer replied with tool-call markup instead of a summary" + ); + } + Err(TinyAgentsError::Model(format!( + "summarizer replied with tool-call markup instead of a summary ({last_chars} chars) \ + after {SUMMARY_MARKUP_ATTEMPTS} attempts" + ))) + } +} + +/// Attempts at a summary before a reply that is tool-call markup becomes an +/// error. +const SUMMARY_MARKUP_ATTEMPTS: usize = 2; + +/// Whether `text` carries a tool call in any markup the tool-call grammars +/// recognise (DSML, ``, ``, …). +/// +/// A bare JSON object does not count: a summary may legitimately quote one. +pub(crate) fn contains_tool_call_markup(text: &str) -> bool { + let options = tinytools_agent::ParseOptions::new().without_bare_json(); + !tinytools_agent::parse_text(text, &options).calls.is_empty() +} + +/// The summarizer's user message: the transcript fenced off as data, then the +/// instruction. +/// +/// The transcript renders tool calls as `` blocks and usually +/// ends on a tool result. Sent bare, it reads as a live agent loop waiting for +/// its next step, and a model continues it: replaying captured requests, +/// DeepSeek V4 did so on about one attempt in four, its reasoning carrying on +/// the coding task. With the transcript fenced and the instruction last, the +/// same replay produced no tool calls. +pub(crate) fn summary_request_text(transcript: &str, previous_summary: Option<&str>) -> String { + let previous = previous_summary + .map(|previous| { + format!( + "\n{previous}\n\n\nThe previous summary above \ + covers the turns before the transcript; fold it into the new summary.\n\n" + ) + }) + .unwrap_or_default(); + format!( + "{previous}The transcript to summarize is enclosed in tags below. It is a \ + record of a conversation that already happened; you are not a participant in it.\n\n\ + \n{transcript}\n\n\n\ + Write the structured summary of the transcript above now, using exactly the required \ + sections. Do not continue the conversation and do not call or write any tools: output \ + only the summary." + ) +} + /// 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 diff --git a/crates/tinyagents-harness/src/summarization/model_summarizer_tests.rs b/crates/tinyagents-harness/src/summarization/model_summarizer_tests.rs index e33cae489..52fad6701 100644 --- a/crates/tinyagents-harness/src/summarization/model_summarizer_tests.rs +++ b/crates/tinyagents-harness/src/summarization/model_summarizer_tests.rs @@ -213,3 +213,77 @@ async fn the_fallback_front_drops_oldest_messages_to_fit_its_budget() { assert!(record.summary.text().contains("older message(s) dropped")); assert!(record.summary.text().contains("m39")); } + +/// What DeepSeek V4 returned as a "summary" in the replayed bench captures: its +/// native tool-call markup, with no tools declared on the request. +const DSML_REPLY: &str = "<｜｜DSML｜｜ calls>\n<｜｜DSML｜｜ invoke name=\"shell\">\n\ +<｜｜DSML｜｜ parameter name=\"command\" string=\"true\">cd /app && cat src/lib.rs\n\ +\n"; + +#[test] +fn tool_call_markup_is_detected_but_quoted_json_and_prose_are_not() { + assert!(super::model_summarizer::contains_tool_call_markup( + DSML_REPLY + )); + assert!(super::model_summarizer::contains_tool_call_markup( + "{\"name\":\"shell\",\"arguments\":{\"command\":\"ls\"}}" + )); + assert!(!super::model_summarizer::contains_tool_call_markup( + "## Goal\nShip the parser.\n\n## Active State\nConfig is {\"retries\": 3}." + )); + assert!(!super::model_summarizer::contains_tool_call_markup( + "{\"name\":\"shell\",\"arguments\":{}}" + )); +} + +#[tokio::test] +async fn the_transcript_is_fenced_as_data_with_the_instruction_last() { + let model = Arc::new(ScriptedModel::replies(vec!["## Goal\nx"])); + let summarizer = ModelSummarizer::new(model.clone(), "m"); + let request = SummaryRequest::new(tool_call_messages(json!({"query": "q"}))) + .with_previous_summary("Earlier result: 4 issues"); + summarizer.summarize_request(&request).await.unwrap(); + + let sent = model.requests()[0].messages[1].text(); + let opens = sent.find("").expect("transcript is fenced"); + let closes = sent.find("").expect("transcript fence closes"); + assert!(sent.find("").unwrap() < opens); + assert!(sent[opens..closes].contains("")); + // The last thing the model reads is the instruction, not a tool result. + assert!(sent.trim_end().ends_with("output only the summary.")); +} + +#[tokio::test] +async fn a_tool_call_reply_is_retried_once_and_a_real_summary_kept() { + let model = Arc::new(ScriptedModel::replies(vec![ + DSML_REPLY, + "## Goal\nShip it.", + ])); + let summarizer = ModelSummarizer::new(model.clone(), "m"); + let record = summarizer.summarize(&[Message::user("x")]).await.unwrap(); + + assert_eq!(model.requests().len(), 2); + assert!(record.summary.text().contains("Ship it.")); + assert!(!record.summary.text().contains("DSML")); +} + +#[tokio::test] +async fn a_summarizer_that_keeps_calling_tools_fails_so_the_fallback_trims() { + let model = Arc::new(ScriptedModel::replies(vec![DSML_REPLY, DSML_REPLY])); + let summarizer = ModelSummarizer::new(model.clone(), "m"); + let err = summarizer + .summarize(&[Message::user("x")]) + .await + .unwrap_err(); + assert!(err.to_string().contains("tool-call markup")); + assert_eq!(model.requests().len(), 2); + + // Wrapped as hosts do, the markup never becomes the summary. + let model = Arc::new(ScriptedModel::replies(vec![DSML_REPLY, DSML_REPLY])); + let policy = SummarizationPolicy::default().with_context_window(1_000); + let guarded = + FaultTolerantCachingSummarizer::new(Box::new(ModelSummarizer::new(model, "m")), &policy); + let record = guarded.summarize(&long_slice()).await.unwrap(); + assert!(record.summary.text().contains("deterministic trim")); + assert!(!record.summary.text().contains("DSML")); +}