From d9634b27917abda8620c6ff3f349aa1196c8b33e Mon Sep 17 00:00:00 2001 From: Cstewart-HC <100775598+Cstewart-HC@users.noreply.github.com> Date: Fri, 10 Apr 2026 15:26:33 -0300 Subject: [PATCH 1/3] feat(chat): add summary budget discipline for compaction Enforce hard budget on LLM-generated compaction summaries via new compress_summary() function: - Max 1,200 characters total - Max 24 lines - Max 160 characters per line - Case-insensitive line deduplication - Priority-based line dropping: headers > bullets > plain text - Omission notice appended when lines are dropped Applied to both compact() method and standalone compact_session() function. Includes 8 unit tests covering all budget enforcement paths. --- crates/chat/src/lib.rs | 269 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 269 insertions(+) diff --git a/crates/chat/src/lib.rs b/crates/chat/src/lib.rs index 23e012826..574e8d874 100644 --- a/crates/chat/src/lib.rs +++ b/crates/chat/src/lib.rs @@ -831,6 +831,149 @@ fn capped_tool_result_payload(result: &Value, max_len: usize) -> Value { capped } +/// Maximum total characters for a compaction summary. +const SUMMARY_MAX_CHARS: usize = 1_200; +/// Maximum number of lines in a compaction summary (excluding omission notice). +const SUMMARY_MAX_LINES: usize = 24; +/// Maximum characters per line in a compaction summary. +const SUMMARY_MAX_LINE_CHARS: usize = 160; + +/// Compress a compaction summary to fit within budget constraints. +/// +/// Enforces: max 1,200 chars total, max 24 lines, max 160 chars per line. +/// Deduplicates lines (case-insensitive), preserves headers and bullets, +/// and appends an omission notice when lines are dropped. +#[must_use] +fn compress_summary(text: &str) -> String { + let text = text.trim(); + if text.is_empty() { + return String::new(); + } + + let lines: Vec<&str> = text.lines().collect(); + if lines.is_empty() { + return String::new(); + } + + // Step 1: deduplicate lines (case-insensitive, keep first occurrence). + let mut seen = HashSet::new(); + let mut deduped: Vec = Vec::with_capacity(lines.len()); + for line in lines { + let key = line.trim().to_ascii_lowercase(); + if key.is_empty() || seen.insert(key) { + deduped.push(if line.len() <= SUMMARY_MAX_LINE_CHARS { + line.to_string() + } else { + // Step 2: truncate individual lines exceeding 160 chars. + line[..line.floor_char_boundary(SUMMARY_MAX_LINE_CHARS)].to_string() + }); + } + } + drop(seen); + + // Step 3: check if already within budget. + let joined = deduped.join("\n"); + if deduped.len() <= SUMMARY_MAX_LINES && joined.len() <= SUMMARY_MAX_CHARS { + return joined; + } + + // Step 4: priority-based line dropping. + // Headers (starting with #) get highest priority, then bullets (- * •), then rest. + fn is_header(line: &str) -> bool { + line.trim_start().starts_with('#') + } + fn is_bullet(line: &str) -> bool { + let trimmed = line.trim_start(); + trimmed.starts_with("- ") || trimmed.starts_with("* ") || trimmed.starts_with("• ") + } + + let mut headers: Vec = Vec::new(); + let mut bullet_lines: Vec = Vec::new(); + let mut other_lines: Vec = Vec::new(); + + for line in deduped { + if is_header(&line) { + headers.push(line); + } else if is_bullet(&line) { + bullet_lines.push(line); + } else { + other_lines.push(line); + } + } + + // Build ordered candidate list: bullets first, then others. + // Headers are always kept. + let mut candidates: Vec = Vec::new(); + candidates.extend(bullet_lines); + candidates.extend(other_lines); + + let header_count = headers.len(); + + // Check if keeping all candidates fits. + if header_count + candidates.len() <= SUMMARY_MAX_LINES { + let total_len = headers.iter().chain(candidates.iter()).fold(0, |acc, l| { + acc + l.len() + 1 // +1 for newline + }); + if total_len <= SUMMARY_MAX_CHARS { + let mut result = headers; + result.extend(candidates); + return result.join("\n"); + } + } + + // Need to drop lines from the end of candidates. + // Account for omission notice in budget. + fn make_notice(n: usize) -> String { + format!("[... {n} lines omitted for brevity]") + } + + for drop_count in 1..=candidates.len() { + let keep_count = candidates.len() - drop_count; + let line_count = header_count + keep_count + 1; // +1 for omission notice + if line_count > SUMMARY_MAX_LINES { + continue; + } + + let notice = make_notice(drop_count); + let kept_candidates = &candidates[..keep_count]; + let total_len = headers + .iter() + .chain(kept_candidates.iter()) + .fold(0, |acc, l| acc + l.len() + 1) + + notice.len() + + 1; // +1 for newline before notice + + if total_len <= SUMMARY_MAX_CHARS { + let mut result = headers; + result.extend(kept_candidates.iter().cloned()); + result.push(notice); + return result.join("\n"); + } + } + + // Edge case: even dropping all candidates, headers alone are too long. + // Force-truncate headers from the end. + let all_dropped_count = candidates.len(); + let notice = make_notice(all_dropped_count); + let mut result: Vec = Vec::new(); + let mut char_budget = SUMMARY_MAX_CHARS.saturating_sub(notice.len() + 1); // +1 for newline + for line in &headers { + let needed = line.len() + + if result.is_empty() { + 0 + } else { + 1 + }; + if needed > char_budget || result.len() + 1 >= SUMMARY_MAX_LINES { + break; + } + char_budget -= needed; + result.push(line.clone()); + } + result.push(notice); + result.join("\n") +} + fn shell_reply_text_from_exec_result(result: &Value) -> String { let stdout = result .get("stdout") @@ -4541,6 +4684,9 @@ impl ChatService for LiveChatService { return Err("compact produced empty summary".into()); } + // Enforce summary budget discipline: max 1,200 chars, 24 lines, 160 chars/line. + summary = compress_summary(&summary); + // Replace history with a single user message containing the summary. // Using the user role (not assistant) avoids breaking providers like // llama.cpp that require every assistant message to follow a user message, @@ -7088,6 +7234,9 @@ async fn compact_session( return Err(error::Error::message("compact produced empty summary")); } + // Enforce summary budget discipline: max 1,200 chars, 24 lines, 160 chars/line. + summary = compress_summary(&summary); + // Use user role so strict providers (e.g. llama.cpp) don't reject the // history for having an assistant message without a preceding user message. // User role also keeps the summary in the conversation turn array for @@ -13076,4 +13225,124 @@ mod tests { "error should mention timeout: {err}" ); } + + // ── compress_summary tests ────────────────────────────────────────────── + + #[test] + fn compress_summary_under_budget_returns_unchanged() { + let input = "# Summary\n\n- Key point one\n- Key point two\nDone."; + let result = compress_summary(input); + assert_eq!(result, input); + } + + #[test] + fn compress_summary_over_char_limit() { + let mut lines = vec!["# Summary".to_string()]; + for i in 0..30 { + lines.push(format!( + "- This is line {i} with some padding text to make it longer than usual" + )); + } + let input = lines.join("\n"); + assert!(input.len() > 1_200, "input should exceed 1200 chars"); + + let result = compress_summary(&input); + assert!( + result.len() <= 1_200, + "result must be <= 1200 chars, got {}", + result.len() + ); + assert!( + result.contains("lines omitted"), + "should have omission notice" + ); + } + + #[test] + fn compress_summary_over_line_count() { + let mut lines = vec!["# Summary".to_string()]; + for i in 0..40 { + lines.push(format!("Line {i}")); + } + let input = lines.join("\n"); + + let result = compress_summary(&input); + let result_lines: Vec<&str> = result.lines().collect(); + assert!( + result_lines.len() <= 25, + "result should be <= 25 lines (24 + notice), got {}", + result_lines.len() + ); + assert!(result.contains("lines omitted")); + } + + #[test] + fn compress_summary_long_line_truncation() { + let long_line: String = "x".repeat(200); + let input = format!("Header\n{long_line}"); + let result = compress_summary(&input); + + let result_lines: Vec<&str> = result.lines().collect(); + assert_eq!(result_lines.len(), 2); + // The long line should be truncated to 160 chars. + assert!( + result_lines[1].len() <= 160, + "long line should be <= 160 chars, got {}", + result_lines[1].len() + ); + } + + #[test] + fn compress_summary_deduplication() { + let input = "Alpha\nalpha\nBeta\nBETA\nGamma"; + let result = compress_summary(input); + // Case-insensitive dedup: should keep first occurrence of each. + let result_lines: Vec<&str> = result.lines().collect(); + assert_eq!(result_lines, vec!["Alpha", "Beta", "Gamma"]); + } + + #[test] + fn compress_summary_header_preservation() { + let mut lines = vec!["# Section One".to_string()]; + for i in 0..30 { + lines.push(format!( + "Body line {i} with enough text to fill up space here" + )); + } + lines.push("## Section Two".to_string()); + for i in 0..10 { + lines.push(format!("- Bullet {i} important")); + } + let input = lines.join("\n"); + + let result = compress_summary(&input); + assert!( + result.contains("# Section One"), + "headers should be preserved" + ); + assert!( + result.contains("## Section Two"), + "second header should be preserved" + ); + assert!(result.contains("lines omitted")); + } + + #[test] + fn compress_summary_empty_input() { + assert_eq!(compress_summary(""), ""); + assert_eq!(compress_summary(" "), ""); + } + + #[test] + fn compress_summary_single_very_long_line() { + let long_line = "a".repeat(2_000); + let result = compress_summary(&long_line); + + // Should be truncated to 160 chars. + assert!( + result.len() <= 160, + "single long line should be truncated, got {} chars", + result.len() + ); + } } From 8fd89861d3cf90486b2011e63d194edd6e82c7ed Mon Sep 17 00:00:00 2001 From: Cstewart-HC <100775598+Cstewart-HC@users.noreply.github.com> Date: Fri, 10 Apr 2026 17:18:36 -0300 Subject: [PATCH 2/3] fix(chat): correct budget accounting bugs in compress_summary Fix three Greptile review findings from PR #652: 1. Off-by-one in fold-based character budget: fold adds l.len() + 1 per element but join("\n") produces N-1 separators, overcounting by 1 byte and dropping an extra line at the budget boundary. Added .saturating_sub(1) to both fold sites. 2. Omission notice undercounts when headers are also truncated: all_dropped_count only counted candidates, not headers skipped in the edge-case loop. Track header_drop_count separately and build the notice after the loop with the combined total. Changed break to continue so shorter headers later in the list still get a chance to fit. 3. Empty lines bypass deduplication: key.is_empty() short-circuited seen.insert(), allowing unlimited blank lines through. Removed the short-circuit so blank lines are deduplicated like any other line. --- crates/chat/src/lib.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/crates/chat/src/lib.rs b/crates/chat/src/lib.rs index 574e8d874..2fc364a91 100644 --- a/crates/chat/src/lib.rs +++ b/crates/chat/src/lib.rs @@ -860,7 +860,7 @@ fn compress_summary(text: &str) -> String { let mut deduped: Vec = Vec::with_capacity(lines.len()); for line in lines { let key = line.trim().to_ascii_lowercase(); - if key.is_empty() || seen.insert(key) { + if seen.insert(key) { deduped.push(if line.len() <= SUMMARY_MAX_LINE_CHARS { line.to_string() } else { @@ -913,7 +913,9 @@ fn compress_summary(text: &str) -> String { if header_count + candidates.len() <= SUMMARY_MAX_LINES { let total_len = headers.iter().chain(candidates.iter()).fold(0, |acc, l| { acc + l.len() + 1 // +1 for newline - }); + }) + // fold overcounts by 1 (N newlines vs N-1 for join); subtract to correct. + .saturating_sub(1); if total_len <= SUMMARY_MAX_CHARS { let mut result = headers; result.extend(candidates); @@ -940,6 +942,8 @@ fn compress_summary(text: &str) -> String { .iter() .chain(kept_candidates.iter()) .fold(0, |acc, l| acc + l.len() + 1) + // fold overcounts by 1 (N newlines vs N-1 for join); subtract to correct. + .saturating_sub(1) + notice.len() + 1; // +1 for newline before notice @@ -954,9 +958,10 @@ fn compress_summary(text: &str) -> String { // Edge case: even dropping all candidates, headers alone are too long. // Force-truncate headers from the end. let all_dropped_count = candidates.len(); - let notice = make_notice(all_dropped_count); let mut result: Vec = Vec::new(); - let mut char_budget = SUMMARY_MAX_CHARS.saturating_sub(notice.len() + 1); // +1 for newline + let mut header_drop_count = 0usize; + // Pre-compute budget assuming 0 header drops; notice rebuilt after loop with actual count. + let mut char_budget = SUMMARY_MAX_CHARS.saturating_sub(make_notice(all_dropped_count).len() + 1); for line in &headers { let needed = line.len() + if result.is_empty() { @@ -965,11 +970,13 @@ fn compress_summary(text: &str) -> String { 1 }; if needed > char_budget || result.len() + 1 >= SUMMARY_MAX_LINES { - break; + header_drop_count += 1; + continue; } char_budget -= needed; result.push(line.clone()); } + let notice = make_notice(all_dropped_count + header_drop_count); result.push(notice); result.join("\n") } From 14940fc87367e8d26d3f02117a5aa30cfa2a190e Mon Sep 17 00:00:00 2001 From: Fabien Penso Date: Sun, 12 Apr 2026 20:33:24 +0100 Subject: [PATCH 3/3] fix(chat): address greptile review feedback (greploop iteration 1) - Strip blank lines from summary dedup to avoid wasting line budget - Two-pass header truncation so omission notice length is exact - Add compress_summary_strips_blank_lines test Entire-Checkpoint: e1e7960139f9 --- crates/chat/src/lib.rs | 53 ++++++++++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 15 deletions(-) diff --git a/crates/chat/src/lib.rs b/crates/chat/src/lib.rs index 350f8ba24..276beda35 100644 --- a/crates/chat/src/lib.rs +++ b/crates/chat/src/lib.rs @@ -934,11 +934,16 @@ fn compress_summary(text: &str) -> String { return String::new(); } - // Step 1: deduplicate lines (case-insensitive, keep first occurrence). + // Step 1: deduplicate lines (case-insensitive, keep first occurrence) + // and strip blank lines so they don't consume the 24-line budget. let mut seen = HashSet::new(); let mut deduped: Vec = Vec::with_capacity(lines.len()); for line in lines { let key = line.trim().to_ascii_lowercase(); + // Drop blank lines — they waste budget without adding content. + if key.is_empty() { + continue; + } if seen.insert(key) { deduped.push(if line.len() <= SUMMARY_MAX_LINE_CHARS { line.to_string() @@ -1035,27 +1040,38 @@ fn compress_summary(text: &str) -> String { } // Edge case: even dropping all candidates, headers alone are too long. - // Force-truncate headers from the end. - let all_dropped_count = candidates.len(); - let mut result: Vec = Vec::new(); + // Force-truncate headers from the end. Run two passes so the notice + // length is exact: first pass counts dropped headers, second pass + // builds the result with the correct budget. + let base_dropped = candidates.len(); let mut header_drop_count = 0usize; - // Pre-compute budget assuming 0 header drops; notice rebuilt after loop with actual count. - let mut char_budget = SUMMARY_MAX_CHARS.saturating_sub(make_notice(all_dropped_count).len() + 1); - for line in &headers { - let needed = line.len() - + if result.is_empty() { - 0 + { + // First pass: determine how many headers must be dropped. + let mut budget = SUMMARY_MAX_CHARS.saturating_sub(make_notice(base_dropped).len() + 1); + let mut kept = 0usize; + for line in &headers { + let needed = line.len() + if kept == 0 { 0 } else { 1 }; + if needed > budget || kept + 1 >= SUMMARY_MAX_LINES { + header_drop_count += 1; } else { - 1 - }; + budget -= needed; + kept += 1; + } + } + } + + let notice = make_notice(base_dropped + header_drop_count); + // Second pass: rebuild with exact budget including final notice length. + let mut char_budget = SUMMARY_MAX_CHARS.saturating_sub(notice.len() + 1); + let mut result: Vec = Vec::new(); + for line in &headers { + let needed = line.len() + if result.is_empty() { 0 } else { 1 }; if needed > char_budget || result.len() + 1 >= SUMMARY_MAX_LINES { - header_drop_count += 1; continue; } char_budget -= needed; result.push(line.clone()); } - let notice = make_notice(all_dropped_count + header_drop_count); result.push(notice); result.join("\n") } @@ -15540,11 +15556,18 @@ mod tests { #[test] fn compress_summary_under_budget_returns_unchanged() { - let input = "# Summary\n\n- Key point one\n- Key point two\nDone."; + let input = "# Summary\n- Key point one\n- Key point two\nDone."; let result = compress_summary(input); assert_eq!(result, input); } + #[test] + fn compress_summary_strips_blank_lines() { + let input = "# Summary\n\n- Point one\n\n- Point two\n\nDone."; + let result = compress_summary(input); + assert_eq!(result, "# Summary\n- Point one\n- Point two\nDone."); + } + #[test] fn compress_summary_over_char_limit() { let mut lines = vec!["# Summary".to_string()];