diff --git a/crates/goose/src/agents/agent.rs b/crates/goose/src/agents/agent.rs index 9f556520e655..a6d69c75bc61 100644 --- a/crates/goose/src/agents/agent.rs +++ b/crates/goose/src/agents/agent.rs @@ -1813,9 +1813,10 @@ impl Agent { ) .await { - Ok((compacted_conversation, summarization_usage)) => { + Ok(compaction) => { + let compacted_conversation = compaction.conversation; session_manager.replace_conversation(&session_config.id, &compacted_conversation).await?; - self.update_session_metrics(&session_config.id, session_config.schedule_id.clone(), &summarization_usage, true).await?; + self.update_session_metrics(&session_config.id, session_config.schedule_id.clone(), &compaction.usage, Some(compaction.retained_context_tokens)).await?; yield AgentEvent::HistoryReplaced(compacted_conversation.clone()); @@ -2089,7 +2090,7 @@ impl Agent { compaction_attempts = 0; if let Some(ref usage) = usage { - let enriched = self.update_session_metrics(&session_config.id, session_config.schedule_id.clone(), usage, false).await?; + let enriched = self.update_session_metrics(&session_config.id, session_config.schedule_id.clone(), usage, None).await?; yield AgentEvent::Usage(enriched.clone()); pending_turn_usage = Some(enriched); } @@ -2571,10 +2572,10 @@ impl Agent { ) .await { - Ok((compacted_conversation, usage)) => { - session_manager.replace_conversation(&session_config.id, &compacted_conversation).await?; - self.update_session_metrics(&session_config.id, session_config.schedule_id.clone(), &usage, true).await?; - conversation = compacted_conversation; + Ok(compaction) => { + session_manager.replace_conversation(&session_config.id, &compaction.conversation).await?; + self.update_session_metrics(&session_config.id, session_config.schedule_id.clone(), &compaction.usage, Some(compaction.retained_context_tokens)).await?; + conversation = compaction.conversation; did_recovery_compact_this_iteration = true; yield AgentEvent::HistoryReplaced(conversation.clone()); break; diff --git a/crates/goose/src/agents/execute_commands.rs b/crates/goose/src/agents/execute_commands.rs index 84e9152b0382..ba23712c2ed7 100644 --- a/crates/goose/src/agents/execute_commands.rs +++ b/crates/goose/src/agents/execute_commands.rs @@ -157,7 +157,7 @@ impl Agent { .ok_or_else(|| anyhow!("Session has no conversation"))?; let model_config = self.model_config_for_session(session_id).await?; - let (compacted_conversation, usage) = compact_messages( + let compaction = compact_messages( self.provider().await?.as_ref(), &model_config, session_id, @@ -167,11 +167,16 @@ impl Agent { .await?; manager - .replace_conversation(session_id, &compacted_conversation) + .replace_conversation(session_id, &compaction.conversation) .await?; - self.update_session_metrics(session_id, session.schedule_id, &usage, true) - .await?; + self.update_session_metrics( + session_id, + session.schedule_id, + &compaction.usage, + Some(compaction.retained_context_tokens), + ) + .await?; Ok(Some(user_only_assistant_text("Compaction complete"))) } diff --git a/crates/goose/src/agents/reply_parts.rs b/crates/goose/src/agents/reply_parts.rs index 8238e1f1e693..80a1a0767597 100644 --- a/crates/goose/src/agents/reply_parts.rs +++ b/crates/goose/src/agents/reply_parts.rs @@ -565,12 +565,15 @@ impl Agent { (frontend_requests, other_requests, filtered_message) } + /// `post_compaction_context_tokens` is `Some` when this usage came from a + /// compaction call: the value (the retained summary size, not the billable + /// output) becomes the session's new context baseline. pub(crate) async fn update_session_metrics( &self, session_id: &str, schedule_id: Option, usage: &ProviderUsage, - is_compaction_usage: bool, + post_compaction_context_tokens: Option, ) -> Result { let manager = self.config.session_manager.clone(); let session = manager.get_session(session_id, false).await?; @@ -581,14 +584,12 @@ impl Agent { let mut enriched = usage.clone(); enriched.cost = chunk_cost; enriched.cost_source = cost_source; - let ledger = MessageUsage::from_provider_usage(&enriched, is_compaction_usage); + let ledger = + MessageUsage::from_provider_usage(&enriched, post_compaction_context_tokens.is_some()); - let current_usage = if is_compaction_usage { - // After compaction: summary output becomes new input context - let new_input = usage.usage.output_tokens; - Usage::new(new_input, None, new_input) - } else { - usage.usage + let current_usage = match post_compaction_context_tokens { + Some(retained) => Usage::new(Some(retained), None, Some(retained)), + None => usage.usage, }; manager diff --git a/crates/goose/src/context_mgmt/mod.rs b/crates/goose/src/context_mgmt/mod.rs index 857cd3d3670d..1f0d074e3bc8 100644 --- a/crates/goose/src/context_mgmt/mod.rs +++ b/crates/goose/src/context_mgmt/mod.rs @@ -1,3 +1,6 @@ +pub mod structured; + +use crate::context_mgmt::structured::StructuredSummary; use crate::conversation::message::{ActionRequiredData, MessageMetadata}; use crate::conversation::message::{Message, MessageContent}; use crate::conversation::{merge_consecutive_messages, Conversation}; @@ -48,6 +51,17 @@ struct SummarizeContext { messages: String, } +pub struct CompactionResult { + pub conversation: Conversation, + /// Billable usage of the summarization call, counting the raw model + /// output even when it is rewritten to the rendered structured summary. + pub usage: ProviderUsage, + /// Estimated tokens of the agent-visible context retained after + /// compaction. Smaller than the billable output when the raw response was + /// rewritten to the rendered structured summary. + pub retained_context_tokens: i32, +} + /// Compact messages by summarizing them /// /// This function performs the actual compaction by summarizing messages and updating @@ -59,18 +73,13 @@ struct SummarizeContext { /// * `session_id` - The session to use for summarization /// * `conversation` - The current conversation history /// * `manual_compact` - If true, this is a manual compaction (don't preserve user message) -/// -/// # Returns -/// * A tuple containing: -/// - `Conversation`: The compacted messages -/// - `ProviderUsage`: Provider usage from summarization pub async fn compact_messages( provider: &dyn Provider, model_config: &ModelConfig, session_id: &str, conversation: &Conversation, manual_compact: bool, -) -> Result<(Conversation, ProviderUsage)> { +) -> Result { info!("Performing message compaction"); let messages = conversation.messages(); @@ -163,10 +172,41 @@ pub async fn compact_messages( final_messages.push(user_msg); } - Ok(( - Conversation::new_unvalidated(final_messages), - summarization_usage, - )) + let conversation = Conversation::new_unvalidated(final_messages); + let retained_context_tokens = count_retained_context_tokens(&conversation) + .await + .or(summarization_usage.usage.output_tokens) + .unwrap_or(0); + + Ok(CompactionResult { + conversation, + usage: summarization_usage, + retained_context_tokens, + }) +} + +/// Estimate the tokens of the agent-visible conversation retained after +/// compaction, counted the same way as the fallback estimation in +/// `check_if_compaction_needed`. +async fn count_retained_context_tokens(conversation: &Conversation) -> Option { + match create_token_counter().await { + Ok(counter) => { + let total: usize = conversation + .messages() + .iter() + .filter(|m| m.is_agent_visible()) + .map(|msg| counter.count_chat_tokens("", std::slice::from_ref(msg), &[])) + .sum(); + Some(total as i32) + } + Err(e) => { + warn!( + "Failed to count retained context tokens, using billable output tokens: {}", + e + ); + None + } + } } /// Check if messages exceed the auto-compaction threshold @@ -318,6 +358,9 @@ async fn do_compact( Ok((mut response, mut provider_usage)) => { response.role = Role::User; + // Usage must reflect the raw model output (billable tokens), + // so estimate before the response is rewritten to the smaller + // rendered summary. crate::providers::usage_estimator::ensure_usage_tokens( &mut provider_usage, &system_prompt, @@ -328,6 +371,8 @@ async fn do_compact( .await .map_err(|e| anyhow::anyhow!("Failed to ensure usage tokens: {}", e))?; + apply_structured_summary(&mut response); + return Ok((response, provider_usage)); } Err(e) => { @@ -350,6 +395,27 @@ async fn do_compact( )) } +/// When the model didn't follow the structured output format (schema-ignoring +/// models, user-customized prompts), the raw response text is kept unchanged +/// as the summary. +fn apply_structured_summary(response: &mut Message) { + let Some(summary) = StructuredSummary::parse(&response.as_concat_text()) else { + return; + }; + match summary.render() { + Ok(rendered) if !rendered.trim().is_empty() => { + response.content = vec![MessageContent::text(rendered)]; + } + Ok(_) => warn!( + "Structured compaction summary rendered empty (broken template override?), keeping raw output" + ), + Err(e) => warn!( + "Failed to render structured compaction summary, keeping raw output: {}", + e + ), + } +} + pub fn format_message_for_compacting(msg: &Message) -> String { let content_parts: Vec = msg .content @@ -718,7 +784,7 @@ mod tests { let conversation = Conversation::new_unvalidated(basic_conversation); let model_config = provider.config.clone(); - let (compacted_conversation, _usage) = compact_messages( + let compaction = compact_messages( &provider, &model_config, "test-session-id", @@ -728,12 +794,92 @@ mod tests { .await .unwrap(); - let agent_conversation = compacted_conversation.agent_visible_messages(); + let agent_conversation = compaction.conversation.agent_visible_messages(); let _ = Conversation::new(agent_conversation) .expect("compaction should produce a valid conversation"); } + #[tokio::test] + async fn test_structured_summary_is_rendered() { + let structured_response = r#"User asked to fix a bug; I patched parser.rs. +```json +{ + "user_intent": ["Fix the parser bug"], + "files": [{"path": "src/parser.rs", "summary": "Fixed off-by-one"}], + "pending_tasks": ["Add a regression test"], + "current_work": "Writing the regression test" +} +```"#; + let provider = + MockProvider::new(Message::assistant().with_text(structured_response), 100_000); + let conversation = Conversation::new_unvalidated(vec![ + Message::user().with_text("fix the parser bug"), + Message::assistant().with_text("Looking into it"), + ]); + + let model_config = provider.config.clone(); + let compaction = compact_messages( + &provider, + &model_config, + "test-session-id", + &conversation, + true, + ) + .await + .unwrap(); + + let summary_text = compaction.conversation.agent_visible_messages()[0].as_concat_text(); + assert!(summary_text.contains("# Conversation Summary")); + assert!(summary_text.contains("## User Intent")); + assert!(summary_text.contains("- Fix the parser bug")); + assert!(summary_text.contains("### src/parser.rs")); + assert!( + !summary_text.contains("```json"), + "raw JSON should be replaced" + ); + assert!( + !summary_text.contains(""), + "analysis scratchpad should be dropped" + ); + assert!(compaction.retained_context_tokens > 0); + assert!( + compaction.usage.usage.output_tokens.is_some(), + "billable output tokens must survive the rewrite" + ); + } + + #[tokio::test] + async fn retained_context_counts_preserved_user_message() { + async fn retained(final_user_text: &str) -> i32 { + let provider = + MockProvider::new(Message::assistant().with_text(""), 100_000); + let conversation = Conversation::new_unvalidated(vec![ + Message::user().with_text("start"), + Message::assistant().with_text("ok"), + Message::user().with_text(final_user_text), + ]); + let model_config = provider.config.clone(); + compact_messages( + &provider, + &model_config, + "test-session-id", + &conversation, + false, + ) + .await + .unwrap() + .retained_context_tokens + } + + let short = retained("continue").await; + let long = retained(&"long preserved user message ".repeat(200)).await; + assert!( + long > short, + "the preserved user message must be part of the retained context ({short} vs {long})" + ); + } + #[tokio::test] async fn preserved_user_message_keeps_audience_projection_after_compaction() { use rmcp::model::{RawTextContent, Role}; @@ -762,7 +908,7 @@ mod tests { ]); let provider = MockProvider::new(Message::assistant().with_text("summary"), 1000); - let (compacted, _) = compact_messages( + let compacted = compact_messages( &provider, &provider.config, "test-session-id", @@ -770,7 +916,8 @@ mod tests { false, ) .await - .unwrap(); + .unwrap() + .conversation; let preserved_copies = compacted .messages() diff --git a/crates/goose/src/context_mgmt/structured.rs b/crates/goose/src/context_mgmt/structured.rs new file mode 100644 index 000000000000..f5e0dbca9340 --- /dev/null +++ b/crates/goose/src/context_mgmt/structured.rs @@ -0,0 +1,477 @@ +use crate::prompt_template::render_template; +use goose_providers::json::safely_parse_json; +use serde::{Deserialize, Serialize}; + +/// Structured output of the compaction LLM call. +/// +/// Every list is ordered most-important-first so consumers (the render +/// template, experiments that truncate sections) can cut from the tail. +/// Fields deserialize leniently - omitted fields default to empty, and an +/// object or number where a string was asked for is stringified rather than +/// failing - because models routinely enrich the schema (e.g. `{"error": .., +/// "fix": ..}` entries) and one such field must not discard a good summary. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct StructuredSummary { + #[serde(default, deserialize_with = "lenient_string_list")] + pub user_intent: Vec, + #[serde(default, deserialize_with = "lenient_string_list")] + pub technical_concepts: Vec, + #[serde(default, deserialize_with = "lenient_file_list")] + pub files: Vec, + #[serde(default, deserialize_with = "lenient_string_list")] + pub errors_and_fixes: Vec, + #[serde(default, deserialize_with = "lenient_string_list")] + pub problem_solving: Vec, + #[serde(default, deserialize_with = "lenient_string_list")] + pub user_messages: Vec, + #[serde(default, deserialize_with = "lenient_string_list")] + pub pending_tasks: Vec, + #[serde(default, deserialize_with = "lenient_string_opt")] + pub current_work: Option, + #[serde(default, deserialize_with = "lenient_string_opt")] + pub next_step: Option, + /// Unknown top-level fields, kept so a user-customized compaction prompt + /// that adds fields can still reach them from a customized render + /// template. Not counted when deciding whether a summary is empty. + #[serde(flatten)] + pub extra: serde_json::Map, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct FileActivity { + #[serde(default, deserialize_with = "lenient_string")] + pub path: String, + #[serde(default, deserialize_with = "lenient_string")] + pub summary: String, + #[serde(default, deserialize_with = "lenient_string_opt")] + pub key_code: Option, +} + +fn stringify_lenient(value: &serde_json::Value) -> String { + match value { + serde_json::Value::String(s) => s.clone(), + serde_json::Value::Null => String::new(), + serde_json::Value::Object(map) => map + .iter() + .map(|(k, v)| format!("{k}: {}", stringify_lenient(v))) + .collect::>() + .join("; "), + serde_json::Value::Array(items) => items + .iter() + .map(stringify_lenient) + .collect::>() + .join("; "), + other => other.to_string(), + } +} + +fn lenient_string_list<'de, D>(deserializer: D) -> Result, D::Error> +where + D: serde::Deserializer<'de>, +{ + let value = serde_json::Value::deserialize(deserializer)?; + Ok(match value { + serde_json::Value::Array(items) => items.iter().map(stringify_lenient).collect(), + serde_json::Value::Null => Vec::new(), + other => vec![stringify_lenient(&other)], + }) +} + +fn lenient_string<'de, D>(deserializer: D) -> Result +where + D: serde::Deserializer<'de>, +{ + let value = serde_json::Value::deserialize(deserializer)?; + Ok(stringify_lenient(&value)) +} + +fn lenient_string_opt<'de, D>(deserializer: D) -> Result, D::Error> +where + D: serde::Deserializer<'de>, +{ + let value = serde_json::Value::deserialize(deserializer)?; + Ok(match value { + serde_json::Value::Null => None, + other => Some(stringify_lenient(&other)), + }) +} + +/// `files` entries should be objects, but a model that over-applies the +/// "plain strings" rule may emit them as strings; render those as path-only +/// activities rather than discarding the whole summary. +fn lenient_file_list<'de, D>(deserializer: D) -> Result, D::Error> +where + D: serde::Deserializer<'de>, +{ + let value = serde_json::Value::deserialize(deserializer)?; + let items = match value { + serde_json::Value::Array(items) => items, + serde_json::Value::Null => return Ok(Vec::new()), + other => vec![other], + }; + Ok(items + .into_iter() + .filter_map(|item| match item { + serde_json::Value::Object(_) => serde_json::from_value(item).ok(), + other => { + let path = stringify_lenient(&other); + (!path.trim().is_empty()).then_some(FileActivity { + path, + summary: String::new(), + key_code: None, + }) + } + }) + .collect()) +} + +impl StructuredSummary { + /// Returns `None` when no usable JSON document is found so the caller can + /// keep the raw response text - the lossless fallback. + pub fn parse(response_text: &str) -> Option { + json_candidates(response_text).into_iter().find_map(|c| { + let value = safely_parse_json(c).ok()?; + let mut summary: Self = serde_json::from_value(value).ok()?; + summary.normalize(); + (!summary.is_empty()).then_some(summary) + }) + } + + pub fn render(&self) -> Result { + render_template("compaction_summary.md", self) + } + + /// Drops blank entries so a response of blank strings counts as empty + /// (raw-text fallback) rather than rendering a summary of nothing. + fn normalize(&mut self) { + fn blank(s: &str) -> bool { + s.trim().is_empty() + } + for list in [ + &mut self.user_intent, + &mut self.technical_concepts, + &mut self.errors_and_fixes, + &mut self.problem_solving, + &mut self.user_messages, + &mut self.pending_tasks, + ] { + list.retain(|s| !blank(s)); + } + for file in &mut self.files { + if file.key_code.as_deref().is_some_and(blank) { + file.key_code = None; + } + } + self.files + .retain(|f| !blank(&f.path) || !blank(&f.summary) || f.key_code.is_some()); + if self.current_work.as_deref().is_some_and(blank) { + self.current_work = None; + } + if self.next_step.as_deref().is_some_and(blank) { + self.next_step = None; + } + } + + fn is_empty(&self) -> bool { + self.user_intent.is_empty() + && self.technical_concepts.is_empty() + && self.files.is_empty() + && self.errors_and_fixes.is_empty() + && self.problem_solving.is_empty() + && self.user_messages.is_empty() + && self.pending_tasks.is_empty() + && self.current_work.is_none() + && self.next_step.is_none() + } +} + +/// Candidate JSON documents in the model's response, tried in order until one +/// parses: after each `` terminator (last first) the +/// post-terminator ```json fences (last first) then a leading object, and +/// finally a leading object of the whole text. +/// +/// Terminators before the last are retried because the summary JSON may +/// itself quote `` (e.g. a session editing compaction prompts), +/// hiding the real terminator from a plain rfind. Such a candidate is +/// accepted only if it contains every later terminator occurrence - proof +/// they were quoted inside it - so a fenced example inside the scratchpad, +/// which precedes the real terminator, can never leak through. +/// +/// Every candidate must sit directly at its marker and brace-balance to a +/// close. Anything looser erodes the lossless fallback: JSON merely quoted in +/// a prose response, or a fenced example inside the discarded scratchpad, +/// would silently replace the raw text. Extraction is brace-balanced rather +/// than fence-delimited because string values may legally contain ```, and an +/// unterminated object (output cut off mid-JSON) is not repaired - repair +/// would drop the late, continuation-critical sections that the raw-text +/// fallback preserves. +#[allow(clippy::string_slice)] // All markers are ASCII; indices are byte offsets of ASCII matches. +fn json_candidates(text: &str) -> Vec<&str> { + const TERMINATOR: &str = ""; + + let mut cuts: Vec = text + .match_indices(TERMINATOR) + .map(|(idx, _)| idx + TERMINATOR.len()) + .collect(); + if cuts.is_empty() { + cuts.push(0); + } + + let mut candidates: Vec<&str> = Vec::new(); + for &cut in cuts.iter().rev() { + let tail = &text[cut..]; + let later_terminators = tail.matches(TERMINATOR).count(); + candidates.extend( + fenced_json_blocks(tail) + .chain(leading_object(tail)) + .filter(|candidate| candidate.matches(TERMINATOR).count() == later_terminators), + ); + } + candidates.extend(leading_object(text)); + candidates.dedup(); + candidates +} + +/// Every fence is tried, last first, because a string value may itself quote +/// a fenced JSON snippet, and such an embedded fence must not shadow the real +/// one. +#[allow(clippy::string_slice)] // The marker is ASCII; indices are byte offsets of ASCII matches. +fn fenced_json_blocks(text: &str) -> impl Iterator { + text.match_indices("```json") + .collect::>() + .into_iter() + .rev() + .filter_map(|(idx, marker)| leading_object(&text[idx + marker.len()..])) +} + +#[allow(clippy::string_slice)] // Indices come from char_indices(); slicing is safe. +fn leading_object(text: &str) -> Option<&str> { + let text = text.trim_start(); + if !text.starts_with('{') { + return None; + } + + let mut depth = 0usize; + let mut in_string = false; + let mut escaped = false; + + for (idx, ch) in text.char_indices() { + if in_string { + if escaped { + escaped = false; + } else if ch == '\\' { + escaped = true; + } else if ch == '"' { + in_string = false; + } + continue; + } + match ch { + '"' => in_string = true, + '{' => depth += 1, + '}' => { + depth -= 1; + if depth == 0 { + return Some(&text[..=idx]); + } + } + _ => {} + } + } + + None +} + +#[cfg(test)] +mod tests { + use super::*; + + const FULL_RESPONSE: &str = r#" +The user asked to fix a bug in parser.rs. I traced it to an off-by-one +in {brace handling} and patched it. + + +```json +{ + "user_intent": ["Fix the parser bug", "Add a regression test"], + "technical_concepts": ["off-by-one", "tokenizer"], + "files": [ + {"path": "src/parser.rs", "summary": "Fixed off-by-one in scan loop", "key_code": "fn scan(&mut self) { .. }"} + ], + "errors_and_fixes": ["Panic on empty input, fixed with early return"], + "problem_solving": ["Root-caused via failing unit test"], + "user_messages": ["fix the parser bug", "add a test"], + "pending_tasks": ["Add a regression test"], + "current_work": "Writing the regression test in tests/parser.rs", + "next_step": "Finish the regression test" +} +```"#; + + #[test] + fn parses_fenced_json_after_analysis() { + let summary = StructuredSummary::parse(FULL_RESPONSE).expect("should parse"); + assert_eq!( + summary.user_intent, + vec!["Fix the parser bug", "Add a regression test"] + ); + assert_eq!(summary.files.len(), 1); + assert_eq!(summary.files[0].path, "src/parser.rs"); + assert_eq!( + summary.current_work.as_deref(), + Some("Writing the regression test in tests/parser.rs") + ); + } + + #[test] + fn unusable_responses_fall_back_to_raw_text() { + for text in [ + // freeform prose, no JSON document + "Here is a summary of the conversation. The user asked about compaction.", + // no visible content + "{}", + r#"{"notes": "unknown fields alone are not a summary"}"#, + r#"{"current_work": ""}"#, + r#"{"files": [{}], "user_intent": [" "]}"#, + // output cut off mid-JSON: never repaired + "```json\n{\"user_intent\": [\"Fix the bug\"], \"pending_tasks\": [\"Write tests\", \"Update docs", + // JSON quoted inside prose is not anchored at a marker + r#"The session focused on the parser migration. The tracker entry {"current_work": "migrate parser"} is unchanged, and tests still need porting."#, + "reviewing\nA prose recap: the config was set to {\"user_intent\": [\"quoted example\"]} per the docs, then the run passed.", + // a fenced example inside the scratchpad is not the summary + "\nThe target shape is:\n```json\n{\"user_intent\": [\"example only\"]}\n```\nNow let me review the conversation.\n\nSorry, I ran out of room and could not produce the summary document.", + // a quoted terminator inside the scratchpad must not expose its fenced example + "\nThe prompt ends with and shows the shape:\n```json\n{\"user_intent\": [\"example only\"]}\n```\nNow let me review the conversation.\n\nSorry, I ran out of room and could not produce the summary document.", + ] { + assert!( + StructuredSummary::parse(text).is_none(), + "should fall back to raw text for: {text}" + ); + } + } + + #[test] + fn embedded_fences_in_string_values_do_not_break_extraction() { + let text = "```json\n{\"user_intent\": [\"Document the build\"], \"files\": [{\"path\": \"README.md\", \"summary\": \"Added build docs\", \"key_code\": \"```bash\\ncargo build\\n```\"}], \"pending_tasks\": [\"Publish the docs\"]}\n```"; + let summary = StructuredSummary::parse(text).expect("should parse"); + assert_eq!( + summary.files[0].key_code.as_deref(), + Some("```bash\ncargo build\n```") + ); + assert_eq!(summary.pending_tasks, vec!["Publish the docs"]); + + let quoted_json_fence = "```json\n{\"user_intent\": [\"Document the config\"], \"files\": [{\"path\": \"docs/config.md\", \"summary\": \"Added config examples\", \"key_code\": \"```json\\n{\\\"retries\\\": 3}\\n```\"}]}\n```"; + let summary = + StructuredSummary::parse(quoted_json_fence).expect("should parse via the outer fence"); + assert_eq!( + summary.files[0].key_code.as_deref(), + Some("```json\n{\"retries\": 3}\n```") + ); + } + + #[test] + fn quoted_terminator_inside_summary_json_does_not_hide_it() { + let text = "\nThe session edited the compaction prompt itself.\n\n```json\n{\"user_intent\": [\"Rework the scratchpad prompt\"], \"files\": [{\"path\": \"prompts/compact.md\", \"summary\": \"Tightened the ... instructions\"}]}\n```"; + let summary = StructuredSummary::parse(text).expect("should parse"); + assert_eq!(summary.user_intent, vec!["Rework the scratchpad prompt"]); + assert_eq!( + summary.files[0].summary, + "Tightened the ... instructions" + ); + } + + #[test] + fn retries_next_candidate_when_fenced_extraction_fails() { + let text = "the model was told to emit ```json with {braces\n{\"user_intent\": [\"Real goal\"]}"; + let summary = StructuredSummary::parse(text).expect("should parse"); + assert_eq!(summary.user_intent, vec!["Real goal"]); + } + + #[test] + fn lenient_shapes_are_stringified_not_rejected() { + let text = r#"{ + "user_intent": "fix the flaky test", + "errors_and_fixes": [ + {"error": "cursor drifted after replay batch 34", "fix": "bounded mpsc channel"}, + "plain string entry", + null + ], + "pending_tasks": [42], + "current_work": {"task": "regression test", "status": "in progress"} + }"#; + let summary = StructuredSummary::parse(text).expect("should parse leniently"); + assert_eq!(summary.user_intent, vec!["fix the flaky test"]); + assert_eq!( + summary.errors_and_fixes, + vec![ + "error: cursor drifted after replay batch 34; fix: bounded mpsc channel", + "plain string entry", + ] + ); + assert_eq!(summary.pending_tasks, vec!["42"]); + assert_eq!( + summary.current_work.as_deref(), + Some("task: regression test; status: in progress") + ); + } + + #[test] + fn file_entries_parse_leniently() { + let text = r#"{"files": [ + "src/parser.rs", + {"path": "tests/parser.rs", "summary": "Added regression test"}, + {"path": "src/scan.rs", "summary": 42, "key_code": ["fn a() {}", "fn b() {}"]}, + "" + ]}"#; + let summary = StructuredSummary::parse(text).expect("should parse"); + assert_eq!(summary.files.len(), 3); + assert_eq!(summary.files[0].path, "src/parser.rs"); + assert_eq!(summary.files[0].summary, ""); + assert_eq!(summary.files[1].summary, "Added regression test"); + assert_eq!(summary.files[2].summary, "42"); + assert_eq!( + summary.files[2].key_code.as_deref(), + Some("fn a() {}; fn b() {}") + ); + } + + #[test] + fn drops_blank_entries_but_keeps_content() { + let text = r#"{"user_intent": ["", "Fix the bug"], "files": [{"path": "a.rs", "summary": "Patched", "key_code": " "}], "next_step": " "}"#; + let summary = StructuredSummary::parse(text).expect("should parse"); + assert_eq!(summary.user_intent, vec!["Fix the bug"]); + assert_eq!(summary.files[0].key_code, None); + assert_eq!(summary.next_step, None); + } + + #[test] + fn renders_markdown_sections() { + let summary = StructuredSummary::parse(FULL_RESPONSE).unwrap(); + let rendered = summary.render().expect("should render"); + assert!(rendered.contains("## User Intent")); + assert!(rendered.contains("- Fix the parser bug")); + assert!(rendered.contains("### src/parser.rs")); + assert!(rendered.contains("fn scan(&mut self) { .. }")); + assert!(rendered.contains("## Next Step")); + } + + #[test] + fn render_fences_exceed_backtick_runs_in_key_code() { + let summary = StructuredSummary { + files: vec![FileActivity { + path: "docs/build.md".to_string(), + summary: "Documented the build".to_string(), + key_code: Some( + "```bash\ncargo build\n```\n````\nnested fence docs\n````".to_string(), + ), + }], + errors_and_fixes: vec!["None".to_string()], + ..Default::default() + }; + let rendered = summary.render().expect("should render"); + // key_code's longest backtick run is four, so the fence must be five + assert_eq!(rendered.matches("\n`````\n").count(), 2); + let errors_heading = rendered.find("## Errors + Fixes").unwrap(); + let closing_fence = rendered.rfind("\n`````\n").unwrap(); + assert!(errors_heading > closing_fence); + } +} diff --git a/crates/goose/src/prompt_template.rs b/crates/goose/src/prompt_template.rs index 4754cb34960b..4d662b611f8b 100644 --- a/crates/goose/src/prompt_template.rs +++ b/crates/goose/src/prompt_template.rs @@ -15,6 +15,10 @@ static TEMPLATE_REGISTRY: &[(&str, &str)] = &[ "compaction.md", "Prompt for summarizing conversation history when context limits are reached", ), + ( + "compaction_summary.md", + "Renders the structured compaction output into the post-compaction context", + ), ( "subagent_system.md", "System prompt for subagents spawned to handle specific tasks", @@ -68,6 +72,23 @@ fn is_registered(name: &str) -> bool { TEMPLATE_REGISTRY.iter().any(|(n, _)| *n == name) } +/// Wrap code in a markdown fence longer than any backtick run it contains, +/// so embedded fences cannot close the block early. +fn code_fence(code: String) -> String { + let longest_run = code + .chars() + .fold((0usize, 0usize), |(max, run), c| { + if c == '`' { + (max.max(run + 1), run + 1) + } else { + (max, 0) + } + }) + .0; + let fence = "`".repeat((longest_run + 1).max(3)); + format!("{fence}\n{}\n{fence}", code.trim_end_matches('\n')) +} + pub fn render_string( template_str: &str, context: &T, @@ -75,6 +96,7 @@ pub fn render_string( let mut env = Environment::new(); env.set_trim_blocks(true); env.set_lstrip_blocks(true); + env.add_filter("code_fence", code_fence); env.add_template("template", template_str)?; let tmpl = env.get_template("template")?; let ctx = MJValue::from_serialize(context); diff --git a/crates/goose/src/prompts/compaction.md b/crates/goose/src/prompts/compaction.md index b0170517bebb..10e8692a4d41 100644 --- a/crates/goose/src/prompts/compaction.md +++ b/crates/goose/src/prompts/compaction.md @@ -1,34 +1,45 @@ ## Task Context - An llm context limit was reached when a user was in a working session with an agent (you) -- Generate a version of the below messages with only the most verbose parts removed +- Distill the conversation below into a structured summary with only the most verbose parts removed - Include user requests, your responses, all technical content, and as much of the original context as possible - This will be used to let the user continue the working session -- Use framing and tone knowing the content will be read an agent (you) on a next exchange to allow for continuation of the session +- The summary will be read by an agent (you) on a next exchange to allow for continuation of the session **Conversation History:** {{ messages }} -Wrap reasoning in `` tags: -- Review conversation chronologically -- For each part, log: - - User goals and requests - - Your method and solution - - Key decisions and designs - - File names, code, signatures, errors, fixes -- Highlight user feedback and revisions -- Confirm completeness and accuracy -- This summary will only be read by you so it is ok to make it much longer than a normal summary you would show to a human -- Do not exclude any information that might be important to continuing a session working with you +Wrap reasoning in `` tags: +- Review conversation chronologically: user goals, your methods, key decisions, files, errors, fixes +- Keep this brief - the analysis is discarded, so it is a checklist of what to include, not the place for detail + +After the closing `` tag, output exactly one ```json code block and nothing else, matching this schema: -### Include the Following Sections: -1. **User Intent** – All goals and requests -2. **Technical Concepts** – All discussed tools, methods -3. **Files + Code** – Viewed/edited files, full code, change justifications -4. **Errors + Fixes** – Bugs, resolutions, user-driven changes -5. **Problem Solving** – Issues solved or in progress -6. **User Messages** – All user messages including tool calls, but truncate long tool call arguments or results -7. **Pending Tasks** – All unresolved user requests -8. **Current Work** – Active work at summary request time: filenames, code, alignment to latest instruction -9. **Next Step** – *Include only if* directly continues user instruction +```json +{ + "user_intent": ["every user goal and request, most important first"], + "technical_concepts": ["all discussed tools, methods, and concepts"], + "files": [ + { + "path": "path of a file that was viewed or edited", + "summary": "what was done to it and why", + "key_code": "important code, signatures, or diffs from this file (omit if none)" + } + ], + "errors_and_fixes": ["bugs hit, their resolutions, and user-driven changes"], + "problem_solving": ["issues solved or in progress, and key decisions: what was chosen, what was rejected, and why"], + "user_messages": ["all user messages, truncating long tool call arguments or results"], + "pending_tasks": ["all unresolved user requests, most important first"], + "current_work": "active work at summary request time: filenames, code, alignment to latest instruction", + "next_step": "include only if it directly continues a user instruction, otherwise omit" +} +``` -> No new ideas unless user confirmed +Rules for the JSON: +- The `` block is a discarded scratchpad: only the JSON survives, so it must be self-contained and repeat every detail from the analysis that matters for continuing +- Order every list from most to least important +- Every list entry must be a plain string, not a nested object - except `files`, whose entries are objects shaped as shown above +- Quote error messages, panic text, and failing test output verbatim in `errors_and_fixes` - exact strings including numbers, identifiers, and paths, not paraphrases +- This summary will only be read by you, so it is ok to make it much longer than a normal summary you would show to a human: spend your entire length budget on the JSON fields, and quote liberally - full output blocks, complete code snippets, exact user wording +- Do not exclude any information that might be important to continuing a session working with you +- Omit a field rather than inventing content for it +- No new ideas unless user confirmed diff --git a/crates/goose/src/prompts/compaction_summary.md b/crates/goose/src/prompts/compaction_summary.md new file mode 100644 index 000000000000..6098bacf91d7 --- /dev/null +++ b/crates/goose/src/prompts/compaction_summary.md @@ -0,0 +1,75 @@ +{# + This template is user-overridable: place a modified copy at + ~/.config/goose/prompts/compaction_summary.md to experiment with what the + post-compaction context contains (e.g. `user_intent[:3]` to keep only the + three most important goals) without rebuilding goose. + + key_code is wrapped via the code_fence filter so embedded fences cannot + break out of the block. +#} +# Conversation Summary + +{% if user_intent %} +## User Intent +{% for item in user_intent %} +- {{ item }} +{% endfor %} + +{% endif %} +{% if technical_concepts %} +## Technical Concepts +{% for item in technical_concepts %} +- {{ item }} +{% endfor %} + +{% endif %} +{% if files %} +## Files + Code +{% for file in files %} +{% if file.path %} +### {{ file.path }} +{% endif %} +{{ file.summary }} +{% if file.key_code %} +{{ file.key_code | code_fence }} +{% endif %} + +{% endfor %} +{% endif %} +{% if errors_and_fixes %} +## Errors + Fixes +{% for item in errors_and_fixes %} +- {{ item }} +{% endfor %} + +{% endif %} +{% if problem_solving %} +## Problem Solving +{% for item in problem_solving %} +- {{ item }} +{% endfor %} + +{% endif %} +{% if user_messages %} +## User Messages +{% for item in user_messages %} +- {{ item }} +{% endfor %} + +{% endif %} +{% if pending_tasks %} +## Pending Tasks +{% for item in pending_tasks %} +- {{ item }} +{% endfor %} + +{% endif %} +{% if current_work %} +## Current Work +{{ current_work }} + +{% endif %} +{% if next_step %} +## Next Step +{{ next_step }} +{% endif %} diff --git a/crates/goose/tests/compaction.rs b/crates/goose/tests/compaction.rs index 7630c0e4dbe9..5f92de7dccfc 100644 --- a/crates/goose/tests/compaction.rs +++ b/crates/goose/tests/compaction.rs @@ -380,20 +380,16 @@ async fn test_manual_compaction_updates_token_counts_and_conversation() -> Resul // - Single "summarize" message: 100 tokens // - Total input observed: ~6100 tokens // - // After compaction: - // - current input_tokens = summary output (200) - the new compact context - // - current output_tokens = None (compaction doesn't produce new output) - // - current total_tokens = 200 - // - accumulated_total = initial (1000) + compaction cost - let expected_summary_output = 200; // compact summary - - // Verify the key invariants after manual compaction: - // After compaction, the current context is ONLY the summary (200 tokens) - // This is the new agent-visible input context - assert_eq!( - updated_session.usage.input_tokens, - Some(expected_summary_output), - "Input tokens should be exactly the summary output (200 tokens)" + // After compaction the baseline is the estimated retained conversation + // (summary + continuation), not the provider-reported output count + let input_after = updated_session + .usage + .input_tokens + .expect("Input tokens should be set after compaction"); + assert!( + input_after > 0 && input_after < 200, + "Input tokens should be the estimated retained context (smaller than the mock's claimed 200 output tokens). Got: {}", + input_after ); assert_eq!( updated_session.usage.output_tokens, None, @@ -401,19 +397,20 @@ async fn test_manual_compaction_updates_token_counts_and_conversation() -> Resul ); assert_eq!( updated_session.usage.total_tokens, - Some(expected_summary_output), - "Total should equal input (200 tokens) after compaction" + Some(input_after), + "Total should equal input after compaction" ); // Accumulated tokens increased by the compaction cost // Initial: 1000 - // Compaction input: ~6400 (system 6000 + 4 messages ~400) + // Compaction input: ~6700 (system 6000 + compaction prompt + 4 messages; + // the mock derives input tokens from the rendered prompt length, so the + // band must absorb compaction.md wording changes) // Compaction output: 200 - // Expected accumulated: 1000 + 6400 + 200 = 7600 let accumulated = updated_session.accumulated_usage.total_tokens.unwrap(); assert!( - (7300..=7900).contains(&accumulated), - "Accumulated should be ~7600 (1000 initial + 6400 input + 200 output). Got: {}", + (7300..=8600).contains(&accumulated), + "Accumulated should be ~7900 (1000 initial + ~6700 input + 200 output). Got: {}", accumulated ); @@ -686,22 +683,20 @@ async fn test_context_limit_recovery_compaction() -> Result<()> { // 1. Initial attempt: >20000 tokens -> Context limit exceeded // 2. Compaction triggered: // - Input: system prompt + messages (including long_tool_call with 15k tokens) - // - Output: 200 tokens (summary) - // - New context size: 200 tokens + // - Output: 200 tokens (summary, as claimed by the mock) + // - New context size: estimated tokens of the retained conversation // 3. Retry with compacted context: - // - Input: system prompt + summary (200) + new message + // - Input: system prompt + summary + new message // - Output: 100 tokens (response) // Verify that current input context is dramatically reduced after compaction let tokens_after = input_tokens_after_compaction.expect("Should have captured tokens after compaction"); - // After compaction, the input context should be ONLY the summary: 200 tokens // Before: system (6000) + long_tool_call messages (~15,400) = 21,400 (exceeded limit!) - // After: only summary (200 tokens) - assert_eq!( - tokens_after, 200, - "Input tokens after compaction should be exactly 200 (summary only). Got: {}", + assert!( + tokens_after > 0 && tokens_after < 200, + "Input tokens after compaction should be the estimated retained context (under the mock's claimed 200). Got: {}", tokens_after );