Skip to content

Commit 9890ceb

Browse files
authored
Avoid double truncation (#6631)
1. Avoid double truncation by giving 10% above the tool default constant 2. Add tests that fails when const = 1
1 parent 7b027e7 commit 9890ceb

File tree

7 files changed

+264
-35
lines changed

7 files changed

+264
-35
lines changed

codex-rs/core/src/context_manager/history.rs

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use crate::codex::TurnContext;
22
use crate::context_manager::normalize;
3+
use crate::context_manager::truncate;
34
use crate::context_manager::truncate::format_output_for_model_body;
45
use crate::context_manager::truncate::globally_truncate_function_output_items;
56
use codex_protocol::models::FunctionCallOutputPayload;
@@ -9,6 +10,12 @@ use codex_protocol::protocol::TokenUsageInfo;
910
use codex_utils_tokenizer::Tokenizer;
1011
use std::ops::Deref;
1112

13+
const CONTEXT_WINDOW_HARD_LIMIT_FACTOR: f64 = 1.1;
14+
const CONTEXT_WINDOW_HARD_LIMIT_BYTES: usize =
15+
(truncate::MODEL_FORMAT_MAX_BYTES as f64 * CONTEXT_WINDOW_HARD_LIMIT_FACTOR) as usize;
16+
const CONTEXT_WINDOW_HARD_LIMIT_LINES: usize =
17+
(truncate::MODEL_FORMAT_MAX_LINES as f64 * CONTEXT_WINDOW_HARD_LIMIT_FACTOR) as usize;
18+
1219
/// Transcript of conversation history
1320
#[derive(Debug, Clone, Default)]
1421
pub(crate) struct ContextManager {
@@ -146,7 +153,11 @@ impl ContextManager {
146153
fn process_item(item: &ResponseItem) -> ResponseItem {
147154
match item {
148155
ResponseItem::FunctionCallOutput { call_id, output } => {
149-
let truncated = format_output_for_model_body(output.content.as_str());
156+
let truncated = format_output_for_model_body(
157+
output.content.as_str(),
158+
CONTEXT_WINDOW_HARD_LIMIT_BYTES,
159+
CONTEXT_WINDOW_HARD_LIMIT_LINES,
160+
);
150161
let truncated_items = output
151162
.content_items
152163
.as_ref()
@@ -161,7 +172,11 @@ impl ContextManager {
161172
}
162173
}
163174
ResponseItem::CustomToolCallOutput { call_id, output } => {
164-
let truncated = format_output_for_model_body(output);
175+
let truncated = format_output_for_model_body(
176+
output,
177+
CONTEXT_WINDOW_HARD_LIMIT_BYTES,
178+
CONTEXT_WINDOW_HARD_LIMIT_LINES,
179+
);
165180
ResponseItem::CustomToolCallOutput {
166181
call_id: call_id.clone(),
167182
output: truncated,

codex-rs/core/src/context_manager/history_tests.rs

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use super::*;
2+
use crate::context_manager::MODEL_FORMAT_MAX_LINES;
23
use crate::context_manager::truncate;
34
use codex_git::GhostCommit;
45
use codex_protocol::models::ContentItem;
@@ -308,8 +309,10 @@ fn assert_truncated_message_matches(message: &str, line: &str, total_lines: usiz
308309
}
309310

310311
fn truncated_message_pattern(line: &str, total_lines: usize) -> String {
311-
let head_take = truncate::MODEL_FORMAT_HEAD_LINES.min(total_lines);
312-
let tail_take = truncate::MODEL_FORMAT_TAIL_LINES.min(total_lines.saturating_sub(head_take));
312+
let head_lines = MODEL_FORMAT_MAX_LINES / 2;
313+
let tail_lines = MODEL_FORMAT_MAX_LINES - head_lines;
314+
let head_take = head_lines.min(total_lines);
315+
let tail_take = tail_lines.min(total_lines.saturating_sub(head_take));
313316
let omitted = total_lines.saturating_sub(head_take + tail_take);
314317
let escaped_line = regex_lite::escape(line);
315318
if omitted == 0 {
@@ -328,7 +331,11 @@ fn format_exec_output_truncates_large_error() {
328331
let line = "very long execution error line that should trigger truncation\n";
329332
let large_error = line.repeat(2_500); // way beyond both byte and line limits
330333

331-
let truncated = truncate::format_output_for_model_body(&large_error);
334+
let truncated = truncate::format_output_for_model_body(
335+
&large_error,
336+
truncate::MODEL_FORMAT_MAX_BYTES,
337+
truncate::MODEL_FORMAT_MAX_LINES,
338+
);
332339

333340
let total_lines = large_error.lines().count();
334341
assert_truncated_message_matches(&truncated, line, total_lines);
@@ -338,7 +345,11 @@ fn format_exec_output_truncates_large_error() {
338345
#[test]
339346
fn format_exec_output_marks_byte_truncation_without_omitted_lines() {
340347
let long_line = "a".repeat(truncate::MODEL_FORMAT_MAX_BYTES + 50);
341-
let truncated = truncate::format_output_for_model_body(&long_line);
348+
let truncated = truncate::format_output_for_model_body(
349+
&long_line,
350+
truncate::MODEL_FORMAT_MAX_BYTES,
351+
truncate::MODEL_FORMAT_MAX_LINES,
352+
);
342353

343354
assert_ne!(truncated, long_line);
344355
let marker_line = format!(
@@ -359,7 +370,14 @@ fn format_exec_output_marks_byte_truncation_without_omitted_lines() {
359370
fn format_exec_output_returns_original_when_within_limits() {
360371
let content = "example output\n".repeat(10);
361372

362-
assert_eq!(truncate::format_output_for_model_body(&content), content);
373+
assert_eq!(
374+
truncate::format_output_for_model_body(
375+
&content,
376+
truncate::MODEL_FORMAT_MAX_BYTES,
377+
truncate::MODEL_FORMAT_MAX_LINES
378+
),
379+
content
380+
);
363381
}
364382

365383
#[test]
@@ -369,7 +387,11 @@ fn format_exec_output_reports_omitted_lines_and_keeps_head_and_tail() {
369387
.map(|idx| format!("line-{idx}\n"))
370388
.collect();
371389

372-
let truncated = truncate::format_output_for_model_body(&content);
390+
let truncated = truncate::format_output_for_model_body(
391+
&content,
392+
truncate::MODEL_FORMAT_MAX_BYTES,
393+
truncate::MODEL_FORMAT_MAX_LINES,
394+
);
373395
let omitted = total_lines - truncate::MODEL_FORMAT_MAX_LINES;
374396
let expected_marker = format!("[... omitted {omitted} of {total_lines} lines ...]");
375397

@@ -397,7 +419,11 @@ fn format_exec_output_prefers_line_marker_when_both_limits_exceeded() {
397419
.map(|idx| format!("line-{idx}-{long_line}\n"))
398420
.collect();
399421

400-
let truncated = truncate::format_output_for_model_body(&content);
422+
let truncated = truncate::format_output_for_model_body(
423+
&content,
424+
truncate::MODEL_FORMAT_MAX_BYTES,
425+
truncate::MODEL_FORMAT_MAX_LINES,
426+
);
401427

402428
assert!(
403429
truncated.contains("[... omitted 42 of 298 lines ...]"),

codex-rs/core/src/context_manager/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,6 @@ mod normalize;
33
mod truncate;
44

55
pub(crate) use history::ContextManager;
6+
pub(crate) use truncate::MODEL_FORMAT_MAX_BYTES;
7+
pub(crate) use truncate::MODEL_FORMAT_MAX_LINES;
68
pub(crate) use truncate::format_output_for_model_body;

codex-rs/core/src/context_manager/truncate.rs

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,11 @@ use codex_protocol::models::FunctionCallOutputContentItem;
22
use codex_utils_string::take_bytes_at_char_boundary;
33
use codex_utils_string::take_last_bytes_at_char_boundary;
44

5+
use crate::util::error_or_panic;
6+
57
// Model-formatting limits: clients get full streams; only content sent to the model is truncated.
6-
pub(crate) const MODEL_FORMAT_MAX_BYTES: usize = 10 * 1024; // 10 KiB
7-
pub(crate) const MODEL_FORMAT_MAX_LINES: usize = 256; // lines
8-
pub(crate) const MODEL_FORMAT_HEAD_LINES: usize = MODEL_FORMAT_MAX_LINES / 2;
9-
pub(crate) const MODEL_FORMAT_TAIL_LINES: usize = MODEL_FORMAT_MAX_LINES - MODEL_FORMAT_HEAD_LINES; // 128
10-
pub(crate) const MODEL_FORMAT_HEAD_BYTES: usize = MODEL_FORMAT_MAX_BYTES / 2;
8+
pub const MODEL_FORMAT_MAX_BYTES: usize = 10 * 1024; // 10 KiB
9+
pub const MODEL_FORMAT_MAX_LINES: usize = 256; // lines
1110

1211
pub(crate) fn globally_truncate_function_output_items(
1312
items: &[FunctionCallOutputContentItem],
@@ -56,21 +55,34 @@ pub(crate) fn globally_truncate_function_output_items(
5655
out
5756
}
5857

59-
pub(crate) fn format_output_for_model_body(content: &str) -> String {
58+
pub(crate) fn format_output_for_model_body(
59+
content: &str,
60+
limit_bytes: usize,
61+
limit_lines: usize,
62+
) -> String {
6063
// Head+tail truncation for the model: show the beginning and end with an elision.
6164
// Clients still receive full streams; only this formatted summary is capped.
6265
let total_lines = content.lines().count();
63-
if content.len() <= MODEL_FORMAT_MAX_BYTES && total_lines <= MODEL_FORMAT_MAX_LINES {
66+
if content.len() <= limit_bytes && total_lines <= limit_lines {
6467
return content.to_string();
6568
}
66-
let output = truncate_formatted_exec_output(content, total_lines);
69+
let output = truncate_formatted_exec_output(content, total_lines, limit_bytes, limit_lines);
6770
format!("Total output lines: {total_lines}\n\n{output}")
6871
}
6972

70-
fn truncate_formatted_exec_output(content: &str, total_lines: usize) -> String {
73+
fn truncate_formatted_exec_output(
74+
content: &str,
75+
total_lines: usize,
76+
limit_bytes: usize,
77+
limit_lines: usize,
78+
) -> String {
79+
debug_panic_on_double_truncation(content);
80+
let head_lines: usize = limit_lines / 2;
81+
let tail_lines: usize = limit_lines - head_lines; // 128
82+
let head_bytes: usize = limit_bytes / 2;
7183
let segments: Vec<&str> = content.split_inclusive('\n').collect();
72-
let head_take = MODEL_FORMAT_HEAD_LINES.min(segments.len());
73-
let tail_take = MODEL_FORMAT_TAIL_LINES.min(segments.len().saturating_sub(head_take));
84+
let head_take = head_lines.min(segments.len());
85+
let tail_take = tail_lines.min(segments.len().saturating_sub(head_take));
7486
let omitted = segments.len().saturating_sub(head_take + tail_take);
7587

7688
let head_slice_end: usize = segments
@@ -91,32 +103,32 @@ fn truncate_formatted_exec_output(content: &str, total_lines: usize) -> String {
91103
};
92104
let head_slice = &content[..head_slice_end];
93105
let tail_slice = &content[tail_slice_start..];
94-
let truncated_by_bytes = content.len() > MODEL_FORMAT_MAX_BYTES;
106+
let truncated_by_bytes = content.len() > limit_bytes;
95107
// this is a bit wrong. We are counting metadata lines and not just shell output lines.
96108
let marker = if omitted > 0 {
97109
Some(format!(
98110
"\n[... omitted {omitted} of {total_lines} lines ...]\n\n"
99111
))
100112
} else if truncated_by_bytes {
101113
Some(format!(
102-
"\n[... output truncated to fit {MODEL_FORMAT_MAX_BYTES} bytes ...]\n\n"
114+
"\n[... output truncated to fit {limit_bytes} bytes ...]\n\n"
103115
))
104116
} else {
105117
None
106118
};
107119

108120
let marker_len = marker.as_ref().map_or(0, String::len);
109-
let base_head_budget = MODEL_FORMAT_HEAD_BYTES.min(MODEL_FORMAT_MAX_BYTES);
110-
let head_budget = base_head_budget.min(MODEL_FORMAT_MAX_BYTES.saturating_sub(marker_len));
121+
let base_head_budget = head_bytes.min(limit_bytes);
122+
let head_budget = base_head_budget.min(limit_bytes.saturating_sub(marker_len));
111123
let head_part = take_bytes_at_char_boundary(head_slice, head_budget);
112-
let mut result = String::with_capacity(MODEL_FORMAT_MAX_BYTES.min(content.len()));
124+
let mut result = String::with_capacity(limit_bytes.min(content.len()));
113125

114126
result.push_str(head_part);
115127
if let Some(marker_text) = marker.as_ref() {
116128
result.push_str(marker_text);
117129
}
118130

119-
let remaining = MODEL_FORMAT_MAX_BYTES.saturating_sub(result.len());
131+
let remaining = limit_bytes.saturating_sub(result.len());
120132
if remaining == 0 {
121133
return result;
122134
}
@@ -126,3 +138,11 @@ fn truncate_formatted_exec_output(content: &str, total_lines: usize) -> String {
126138

127139
result
128140
}
141+
142+
fn debug_panic_on_double_truncation(content: &str) {
143+
if content.contains("Total output lines:") && content.contains("omitted") {
144+
error_or_panic(format!(
145+
"FunctionCallOutput content was already truncated before ContextManager::record_items; this would cause double truncation {content}"
146+
));
147+
}
148+
}

codex-rs/core/src/tools/mod.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ pub mod runtimes;
99
pub mod sandboxing;
1010
pub mod spec;
1111

12+
use crate::context_manager::MODEL_FORMAT_MAX_BYTES;
13+
use crate::context_manager::MODEL_FORMAT_MAX_LINES;
1214
use crate::context_manager::format_output_for_model_body;
1315
use crate::exec::ExecToolCallOutput;
1416
pub use router::ToolRouter;
@@ -75,5 +77,5 @@ pub fn format_exec_output_str(exec_output: &ExecToolCallOutput) -> String {
7577
};
7678

7779
// Truncate for model consumption before serialization.
78-
format_output_for_model_body(&body)
80+
format_output_for_model_body(&body, MODEL_FORMAT_MAX_BYTES, MODEL_FORMAT_MAX_LINES)
7981
}

0 commit comments

Comments
 (0)