From fa0d53a969be37d6d83af123165b8f7884e87fc2 Mon Sep 17 00:00:00 2001 From: reacechalmers Date: Thu, 13 Aug 2026 23:04:06 -0600 Subject: [PATCH] fix(runtime): bound tool-result payloads to the actual context window Two related gaps let a single agentic turn exceed a small context window, with no recovery path once it happened. 1. read_file's character ceiling was a hardcoded 64 KB (~16k tokens), sized for a 200k window. Two reads in one turn reach ~48k tokens, which a 32k backend rejects outright. Make it configurable via CLAW_MAX_READ_CHARS, defaulting to the existing 64 KB so behavior is unchanged unless set. Reads remain paged, not refused: callers still get truncated/nextOffset. 2. Compaction could not help, because it works at message granularity and an agentic turn is five messages (user / assistant+tool_use / tool_result / assistant+tool_use / tool_result). With preserve_recent_messages: 4 the only droppable message is the short user prompt, so the two oversized tool results were preserved verbatim by design. Trim preserved tool-result payloads over 16 KB to a head/tail excerpt with an explicit "[... N characters elided ...]" marker, so the model knows its view is partial and can re-read. Small payloads pass through untouched. Also apply trimming on the early-return path: a session can exceed the window while holding fewer messages than preserve_recent_messages, where should_compact is false and message-granular compaction has no move to make. compact_now no longer gates on removed_message_count alone, which would have computed the smaller session and then discarded it. Also fixes a pre-existing flaky test: repl_executes_python_code resolves the python runtime through PATH, which the PowerShell tests blank while they run. It now takes the same env_lock, so it no longer intermittently fails with "python runtime not found" on a machine that has python. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_013bVvRYn7CQKR6hT2j1L6Cs (cherry picked from commit b0d4d41e8132f8dc75717b1f74554d649639de10) --- rust/crates/runtime/src/compact.rs | 275 +++++++++++++++++++++++- rust/crates/runtime/src/conversation.rs | 9 +- rust/crates/runtime/src/file_ops.rs | 99 ++++++++- rust/crates/tools/src/lib.rs | 6 + 4 files changed, 378 insertions(+), 11 deletions(-) diff --git a/rust/crates/runtime/src/compact.rs b/rust/crates/runtime/src/compact.rs index 797d3a6b..d0273f44 100644 --- a/rust/crates/runtime/src/compact.rs +++ b/rust/crates/runtime/src/compact.rs @@ -95,10 +95,21 @@ pub fn get_compact_continuation_message( #[must_use] pub fn compact_session(session: &Session, config: CompactionConfig) -> CompactionResult { if !should_compact(session, config) { + // "Nothing to drop" is not the same as "nothing to do". A session can exceed the window + // while holding fewer messages than `preserve_recent_messages` — a single read of a large + // file does it — and message-granular compaction has no move to make there. Trim the + // payloads anyway, so the one lever that still works is not gated behind the one that + // does not. + let mut trimmed_session = session.clone(); + trimmed_session.messages = session + .messages + .iter() + .map(truncate_oversized_tool_results) + .collect(); return CompactionResult { summary: String::new(), formatted_summary: String::new(), - compacted_session: session.clone(), + compacted_session: trimmed_session, removed_message_count: 0, }; } @@ -165,7 +176,14 @@ pub fn compact_session(session: &Session, config: CompactionConfig) -> Compactio k }; let removed = &session.messages[compacted_prefix_len..keep_from]; - let preserved = session.messages[keep_from..].to_vec(); + // Dropping whole messages is not enough on its own. An agentic turn is five messages + // (user / assistant+tool_use / tool_result / assistant+tool_use / tool_result), so the + // preserved tail IS the pair of tool results that overflowed the window, and the only + // message compaction may drop is the short user prompt. Trim the payloads that must stay. + let preserved = session.messages[keep_from..] + .iter() + .map(truncate_oversized_tool_results) + .collect::>(); let summary = merge_compact_summaries(existing_summary.as_deref(), &summarize_messages(removed)); let formatted_summary = format_compact_summary(&summary); @@ -190,6 +208,78 @@ pub fn compact_session(session: &Session, config: CompactionConfig) -> Compactio } } +/// Largest tool-result payload compaction will preserve verbatim, in characters. +/// +/// ~16 KB is ~4 k tokens: big enough that ordinary tool output is never touched, small enough +/// that several preserved results still fit a small window. +const MAX_PRESERVED_TOOL_RESULT_CHARS: usize = 16 * 1024; + +/// Characters kept from the tail of a trimmed payload. The head carries the structure a model +/// needs to recognise what it read; the tail is where a file's most recent edits usually are. +const PRESERVED_TOOL_RESULT_TAIL_CHARS: usize = 2 * 1024; + +/// Returns `message` with any oversized `ToolResult` payload replaced by a head/tail excerpt. +/// +/// Clones only when something is actually trimmed, so the common path stays cheap. +fn truncate_oversized_tool_results(message: &ConversationMessage) -> ConversationMessage { + let needs_trim = message.blocks.iter().any(|block| match block { + ContentBlock::ToolResult { output, .. } => { + output.chars().count() > MAX_PRESERVED_TOOL_RESULT_CHARS + } + _ => false, + }); + if !needs_trim { + return message.clone(); + } + + let blocks = message + .blocks + .iter() + .map(|block| match block { + ContentBlock::ToolResult { + tool_use_id, + tool_name, + output, + is_error, + } => ContentBlock::ToolResult { + tool_use_id: tool_use_id.clone(), + tool_name: tool_name.clone(), + output: excerpt_tool_result(output), + is_error: *is_error, + }, + other => other.clone(), + }) + .collect(); + + ConversationMessage { + role: message.role, + blocks, + usage: message.usage, + } +} + +/// Builds a head + marker + tail excerpt of an oversized tool-result payload. +/// +/// Operates on chars, not bytes, so a multi-byte boundary can never be split. +fn excerpt_tool_result(output: &str) -> String { + let chars: Vec = output.chars().collect(); + let total = chars.len(); + if total <= MAX_PRESERVED_TOOL_RESULT_CHARS { + return output.to_string(); + } + + let tail_len = PRESERVED_TOOL_RESULT_TAIL_CHARS.min(total); + let head_len = MAX_PRESERVED_TOOL_RESULT_CHARS.saturating_sub(tail_len); + let elided = total.saturating_sub(head_len + tail_len); + if elided == 0 { + return output.to_string(); + } + + let head: String = chars[..head_len].iter().collect(); + let tail: String = chars[total - tail_len..].iter().collect(); + format!("{head}\n\n[... {elided} characters elided by compaction; re-read this file if you need the omitted portion ...]\n\n{tail}") +} + fn compacted_summary_prefix_len(session: &Session) -> usize { usize::from( session @@ -573,7 +663,7 @@ fn extract_summary_timeline(summary: &str) -> Vec { #[cfg(test)] mod tests { use super::{ - collect_key_files, compact_session, format_compact_summary, + collect_key_files, compact_session, estimate_session_tokens, format_compact_summary, get_compact_continuation_message, infer_pending_work, should_compact, CompactionConfig, }; use crate::session::{ContentBlock, ConversationMessage, MessageRole, Session}; @@ -763,6 +853,185 @@ mod tests { /// user(ToolResult) pair at the boundary. An orphaned tool-result message /// without the preceding assistant `tool_calls` causes a 400 on the /// OpenAI-compat path (gaebal-gajae repro 2026-04-09). + /// The failure this exists for: an agentic turn is 5 messages (user / assistant+tool_use / + /// tool_result / assistant+tool_use / tool_result), so `preserve_recent_messages: 4` protects + /// exactly the two giant tool results that overflow the window. Message-granular compaction + /// can only drop the tiny user prompt, so the payloads themselves must be trimmed. + #[test] + fn compaction_truncates_oversized_tool_results_it_must_preserve() { + let huge = "z".repeat(80_000); + let mut session = Session::new(); + session + .push_message(ConversationMessage::user_text("read two files")) + .expect("user message"); + session + .push_message(ConversationMessage::assistant(vec![ + ContentBlock::ToolUse { + id: "call-1".to_string(), + name: "read_file".to_string(), + input: "{\"path\":\"a.rs\"}".to_string(), + }, + ])) + .expect("assistant tool use"); + session + .push_message(ConversationMessage::tool_result( + "call-1", + "read_file", + huge.clone(), + false, + )) + .expect("tool result"); + session + .push_message(ConversationMessage::assistant(vec![ + ContentBlock::ToolUse { + id: "call-2".to_string(), + name: "read_file".to_string(), + input: "{\"path\":\"b.rs\"}".to_string(), + }, + ])) + .expect("second assistant tool use"); + session + .push_message(ConversationMessage::tool_result( + "call-2", + "read_file", + huge.clone(), + false, + )) + .expect("second tool result"); + + let before = estimate_session_tokens(&session); + let result = compact_session( + &session, + CompactionConfig { + preserve_recent_messages: 4, + max_estimated_tokens: 0, + }, + ); + let after = estimate_session_tokens(&result.compacted_session); + + assert!( + after < before / 2, + "compaction must shrink an oversized preserved tool result: {before} -> {after}" + ); + + let preserved_output = result + .compacted_session + .messages + .iter() + .flat_map(|message| &message.blocks) + .find_map(|block| match block { + ContentBlock::ToolResult { output, .. } => Some(output.clone()), + _ => None, + }) + .expect("the tool result must still be present, not deleted"); + + assert!( + preserved_output.len() < huge.len(), + "the payload must actually be trimmed" + ); + assert!( + preserved_output.contains("elided"), + "the model must be told the view is partial, got: {}", + &preserved_output[..preserved_output.len().min(200)] + ); + assert!( + preserved_output.starts_with('z'), + "the head of the output must be kept" + ); + assert!( + preserved_output.ends_with('z'), + "the tail of the output must be kept" + ); + } + + /// A session can exceed the window while holding FEWER messages than + /// `preserve_recent_messages` — one read of a large file does it. `should_compact` returns + /// false there (nothing is droppable), so trimming must not be gated behind it. + #[test] + fn oversized_tool_results_are_trimmed_even_when_no_message_can_be_dropped() { + let huge = "q".repeat(80_000); + let mut session = Session::new(); + session + .push_message(ConversationMessage::user_text("read one enormous file")) + .expect("user message"); + session + .push_message(ConversationMessage::assistant(vec![ + ContentBlock::ToolUse { + id: "call-1".to_string(), + name: "read_file".to_string(), + input: "{\"path\":\"huge.rs\"}".to_string(), + }, + ])) + .expect("assistant tool use"); + session + .push_message(ConversationMessage::tool_result( + "call-1", + "read_file", + huge, + false, + )) + .expect("tool result"); + + // 3 messages, preserve 4: there is nothing to remove, yet the session is far too big. + let before = estimate_session_tokens(&session); + let result = compact_session( + &session, + CompactionConfig { + preserve_recent_messages: 4, + max_estimated_tokens: 0, + }, + ); + let after = estimate_session_tokens(&result.compacted_session); + + assert!( + after < before / 2, + "an untrimmable session must still have its payloads trimmed: {before} -> {after}" + ); + } + + /// Trimming must not touch payloads that already fit — otherwise every ordinary tool result + /// would grow an elision marker it does not need. + #[test] + fn compaction_leaves_small_tool_results_untouched() { + let small = "ok".repeat(50); + let mut session = Session::new(); + for index in 0..6 { + session + .push_message(ConversationMessage::user_text(format!("turn {index}"))) + .expect("user message"); + session + .push_message(ConversationMessage::tool_result( + format!("call-{index}"), + "read_file", + small.clone(), + false, + )) + .expect("tool result"); + } + + let result = compact_session( + &session, + CompactionConfig { + preserve_recent_messages: 4, + max_estimated_tokens: 0, + }, + ); + + for block in result + .compacted_session + .messages + .iter() + .flat_map(|message| &message.blocks) + { + if let ContentBlock::ToolResult { output, .. } = block { + assert_eq!( + output, &small, + "a small tool result must pass through as-is" + ); + } + } + } + #[test] fn compaction_does_not_split_tool_use_tool_result_pair() { use crate::session::{ContentBlock, Session}; diff --git a/rust/crates/runtime/src/conversation.rs b/rust/crates/runtime/src/conversation.rs index 7f29a378..570190bb 100644 --- a/rust/crates/runtime/src/conversation.rs +++ b/rust/crates/runtime/src/conversation.rs @@ -625,7 +625,7 @@ where self.compact_now() } - /// Compacts the session unconditionally, returning `None` when there was nothing to remove. + /// Compacts the session unconditionally, returning `None` when nothing actually changed. fn compact_now(&mut self) -> Option { let result = compact_session( &self.session, @@ -635,7 +635,12 @@ where }, ); - if result.removed_message_count == 0 { + // `removed_message_count` alone is the wrong test for "did anything happen": compaction + // also trims oversized tool-result payloads in messages it must PRESERVE, and on a + // session too short to have anything droppable that trimming is the entire result. + // Gating on the count would compute the smaller session and then throw it away. + let shrank = result.compacted_session.messages != self.session.messages; + if result.removed_message_count == 0 && !shrank { return None; } diff --git a/rust/crates/runtime/src/file_ops.rs b/rust/crates/runtime/src/file_ops.rs index f22c9b87..d13b2668 100644 --- a/rust/crates/runtime/src/file_ops.rs +++ b/rust/crates/runtime/src/file_ops.rs @@ -31,7 +31,27 @@ const DEFAULT_READ_LINES: usize = 2000; /// /// `DEFAULT_READ_LINES` alone does not bound the payload: minified bundles, JSON blobs and /// generated sources routinely put megabytes on a handful of lines. ~64 KB is ~16 k tokens. -const MAX_READ_CHARS: usize = 64 * 1024; +/// +/// That default assumes a large (200 k) context window. Against a small self-hosted backend it +/// is far too generous: two reads at this size put a request over 47 k tokens, which a 32 k +/// window rejects outright — and compaction cannot recover it, because the preserved recent +/// messages ARE the oversized tool results. Override with [`MAX_READ_CHARS_ENV_VAR`]. +const DEFAULT_MAX_READ_CHARS: usize = 64 * 1024; + +/// Environment override for [`DEFAULT_MAX_READ_CHARS`], in characters. +const MAX_READ_CHARS_ENV_VAR: &str = "CLAW_MAX_READ_CHARS"; + +/// Effective character ceiling for one `read_file` page. +/// +/// Read per call rather than cached: a long-lived process should pick up a changed budget, and +/// this runs once per read, not per line. +fn max_read_chars() -> usize { + std::env::var(MAX_READ_CHARS_ENV_VAR) + .ok() + .and_then(|raw| raw.trim().parse::().ok()) + .filter(|chars| *chars > 0) + .unwrap_or(DEFAULT_MAX_READ_CHARS) +} /// Maximum file size that can be written (10 MB). const MAX_WRITE_SIZE: usize = 10 * 1024 * 1024; @@ -269,9 +289,10 @@ pub fn read_file( // otherwise an oversized single line would return nothing and the caller could not advance. let mut end_index = start_index; let mut chars = 0usize; + let char_ceiling = max_read_chars(); for line in &lines[start_index..requested_end] { let next = chars + line.chars().count() + 1; - if next > MAX_READ_CHARS && end_index > start_index { + if next > char_ceiling && end_index > start_index { break; } chars = next; @@ -864,12 +885,19 @@ mod tests { component_contains_glob, derive_glob_walk_root, edit_file, expand_braces, glob_search, use super::{ component_contains_glob, derive_glob_walk_root, edit_file, expand_braces, glob_search, - grep_search, is_symlink_escape, normalize_for_comparison, read_file, + grep_search, is_symlink_escape, max_read_chars, normalize_for_comparison, read_file, read_file_in_workspace, write_file, write_file_in_workspace, GrepSearchInput, - DEFAULT_READ_LINES, MAX_READ_CHARS, - }; + DEFAULT_MAX_READ_CHARS, DEFAULT_READ_LINES, MAX_READ_CHARS, MAX_READ_CHARS_ENV_VAR, MAX_WRITE_SIZE, }; + }; + + /// Serializes the tests that mutate `CLAW_MAX_READ_CHARS`; env vars are process-global. + fn env_lock() -> std::sync::MutexGuard<'static, ()> { + static LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + LOCK.lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + } fn temp_path(name: &str) -> std::path::PathBuf { let unique = SystemTime::now() @@ -930,8 +958,10 @@ mod tests { fn char_ceiling_bounds_files_whose_lines_are_enormous() { // A line cap alone does not bound the payload: minified bundles put megabytes on a few // lines, so the byte ceiling has to apply too — and must still yield forward progress. + let _guard = env_lock(); + std::env::remove_var(MAX_READ_CHARS_ENV_VAR); let path = temp_path("long-lines.txt"); - let huge = "x".repeat(MAX_READ_CHARS); + let huge = "x".repeat(DEFAULT_MAX_READ_CHARS); let body = format!("{huge}\n{huge}\n{huge}"); write_file(path.to_string_lossy().as_ref(), &body).expect("write should succeed"); @@ -946,6 +976,63 @@ mod tests { assert!(greedy.file.truncated); } + /// The 64 KB default is a 200 k-context number. A small self-hosted backend must be able to + /// shrink it, or two reads overflow a 32 k window in a single turn. + #[test] + fn read_char_ceiling_is_configurable_via_the_environment() { + let _guard = env_lock(); + let path = temp_path("configurable-ceiling.txt"); + // 40 lines of 100 chars = ~4 KB: far under the default ceiling, over a 1 KB one. + let body = (0..40) + .map(|_| "y".repeat(100)) + .collect::>() + .join("\n"); + write_file(path.to_string_lossy().as_ref(), &body).expect("write should succeed"); + + std::env::remove_var(MAX_READ_CHARS_ENV_VAR); + let whole = read_file(path.to_string_lossy().as_ref(), None, None).expect("default read"); + assert_eq!( + whole.file.num_lines, 40, + "default ceiling returns the file whole" + ); + assert!(!whole.file.truncated); + + std::env::set_var(MAX_READ_CHARS_ENV_VAR, "1024"); + let clipped = read_file(path.to_string_lossy().as_ref(), None, None).expect("clipped read"); + std::env::remove_var(MAX_READ_CHARS_ENV_VAR); + assert!( + clipped.file.num_lines < 40, + "a smaller ceiling must actually clip, got {} lines", + clipped.file.num_lines + ); + assert!( + clipped.file.truncated, + "a clipped page must report truncated" + ); + assert_eq!( + clipped.file.next_offset, + Some(clipped.file.num_lines), + "the caller must be told where to resume" + ); + } + + #[test] + fn read_char_ceiling_ignores_junk_and_zero_values() { + let _guard = env_lock(); + for value in ["", "0", "-5", "not-a-number"] { + std::env::set_var(MAX_READ_CHARS_ENV_VAR, value); + assert_eq!( + max_read_chars(), + DEFAULT_MAX_READ_CHARS, + "{value:?} must fall back to the default rather than disabling reads" + ); + } + std::env::set_var(MAX_READ_CHARS_ENV_VAR, "24000"); + assert_eq!(max_read_chars(), 24_000); + std::env::remove_var(MAX_READ_CHARS_ENV_VAR); + assert_eq!(max_read_chars(), DEFAULT_MAX_READ_CHARS); + } + #[test] fn small_files_are_returned_whole_and_not_marked_truncated() { let path = temp_path("small-read.txt"); diff --git a/rust/crates/tools/src/lib.rs b/rust/crates/tools/src/lib.rs index dd90949e..e39efd21 100644 --- a/rust/crates/tools/src/lib.rs +++ b/rust/crates/tools/src/lib.rs @@ -10405,6 +10405,12 @@ mod tests { #[test] fn repl_executes_python_code() { + // Resolving the python runtime reads PATH, and PATH is process-global: the PowerShell + // tests below blank it while they run. Without this guard that races and this test + // intermittently fails with "python runtime not found" on a machine that has python. + let _guard = env_lock() + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); let result = execute_tool( "REPL", &json!({"language": "python", "code": "print(1 + 1)", "timeout_ms": 500}),