refactor(g3-core): extract streaming helpers and simplify cache control logic
Readability improvements to g3-core/src/lib.rs: - Extract format_tool_arg_value() to streaming.rs for tool argument display - Extract format_read_file_summary() to streaming.rs for file read summaries - Add format_tool_output_summary() helper for consistent output formatting - Add get_provider_cache_control() helper to eliminate duplicated cache lookup - Simplify cache control logic in execute_single_task and stream_completion_with_tools - Add unit tests for all new streaming helpers Results: - lib.rs: 2979 → 2945 lines (34 lines saved) - streaming.rs: 305 → 379 lines (74 lines added as reusable, tested helpers) - All 155+ tests pass Agent: carmack
This commit is contained in:
@@ -360,6 +360,21 @@ impl<W: UiWriter> Agent<W> {
|
|||||||
.count()
|
.count()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Get the cache control config for the current provider (if Anthropic with cache enabled).
|
||||||
|
fn get_provider_cache_control(&self) -> Option<CacheControl> {
|
||||||
|
let provider = self.providers.get(None).ok()?;
|
||||||
|
let provider_name = provider.name();
|
||||||
|
let (provider_type, config_name) = provider_config::parse_provider_ref(provider_name);
|
||||||
|
|
||||||
|
match provider_type {
|
||||||
|
"anthropic" => self.config.providers.anthropic
|
||||||
|
.get(config_name)
|
||||||
|
.and_then(|c| c.cache_config.as_ref())
|
||||||
|
.and_then(|config| Self::parse_cache_control(config)),
|
||||||
|
_ => None,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Resolve the max_tokens to use for a given provider, applying fallbacks.
|
/// Resolve the max_tokens to use for a given provider, applying fallbacks.
|
||||||
fn resolve_max_tokens(&self, provider_name: &str) -> u32 {
|
fn resolve_max_tokens(&self, provider_name: &str) -> u32 {
|
||||||
provider_config::resolve_max_tokens(&self.config, provider_name)
|
provider_config::resolve_max_tokens(&self.config, provider_name)
|
||||||
@@ -699,30 +714,19 @@ impl<W: UiWriter> Agent<W> {
|
|||||||
|
|
||||||
// Add user message to context window
|
// Add user message to context window
|
||||||
let mut user_message = {
|
let mut user_message = {
|
||||||
// Check if we should use cache control (every 10 tool calls)
|
|
||||||
// But only if we haven't already added 4 cache_control annotations
|
|
||||||
let provider = self.providers.get(None)?;
|
let provider = self.providers.get(None)?;
|
||||||
let provider_name = provider.name();
|
let content = format!("Task: {}", description);
|
||||||
let (provider_type, config_name) = provider_config::parse_provider_ref(provider_name);
|
|
||||||
if let Some(cache_config) = match provider_type {
|
// Apply cache control if provider supports it
|
||||||
"anthropic" => {
|
if let Some(cache_config) = self.get_provider_cache_control() {
|
||||||
self.config
|
|
||||||
.providers
|
|
||||||
.anthropic
|
|
||||||
.get(config_name)
|
|
||||||
.and_then(|c| c.cache_config.as_ref())
|
|
||||||
.and_then(|config| Self::parse_cache_control(config))
|
|
||||||
}
|
|
||||||
_ => None,
|
|
||||||
} {
|
|
||||||
Message::with_cache_control_validated(
|
Message::with_cache_control_validated(
|
||||||
MessageRole::User,
|
MessageRole::User,
|
||||||
format!("Task: {}", description),
|
content,
|
||||||
cache_config,
|
cache_config,
|
||||||
provider,
|
provider,
|
||||||
)
|
)
|
||||||
} else {
|
} else {
|
||||||
Message::new(MessageRole::User, format!("Task: {}", description))
|
Message::new(MessageRole::User, content)
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -2142,26 +2146,11 @@ impl<W: UiWriter> Agent<W> {
|
|||||||
self.ui_writer.print_tool_header(&tool_call.tool, Some(&tool_call.args));
|
self.ui_writer.print_tool_header(&tool_call.tool, Some(&tool_call.args));
|
||||||
if let Some(args_obj) = tool_call.args.as_object() {
|
if let Some(args_obj) = tool_call.args.as_object() {
|
||||||
for (key, value) in args_obj {
|
for (key, value) in args_obj {
|
||||||
let value_str = match value {
|
let value_str = streaming::format_tool_arg_value(
|
||||||
serde_json::Value::String(s) => {
|
&tool_call.tool,
|
||||||
if tool_call.tool == "shell" && key == "command" {
|
key,
|
||||||
if let Some(first_line) = s.lines().next() {
|
value,
|
||||||
if s.lines().count() > 1 {
|
);
|
||||||
format!("{}...", first_line)
|
|
||||||
} else {
|
|
||||||
first_line.to_string()
|
|
||||||
}
|
|
||||||
} else {
|
|
||||||
s.clone()
|
|
||||||
}
|
|
||||||
} else if s.chars().count() > 100 {
|
|
||||||
streaming::truncate_for_display(s, 100)
|
|
||||||
} else {
|
|
||||||
s.clone()
|
|
||||||
}
|
|
||||||
}
|
|
||||||
_ => value.to_string(),
|
|
||||||
};
|
|
||||||
self.ui_writer.print_tool_arg(key, &value_str);
|
self.ui_writer.print_tool_arg(key, &value_str);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -2213,14 +2202,7 @@ impl<W: UiWriter> Agent<W> {
|
|||||||
let is_read_file = tool_call.tool == "read_file";
|
let is_read_file = tool_call.tool == "read_file";
|
||||||
|
|
||||||
if is_read_file && tool_success {
|
if is_read_file && tool_success {
|
||||||
// Calculate summary: lines and chars
|
let summary = streaming::format_read_file_summary(output_len, tool_result.len());
|
||||||
let char_count = tool_result.len();
|
|
||||||
let char_display = if char_count >= 1000 {
|
|
||||||
format!("{:.1}k", char_count as f64 / 1000.0)
|
|
||||||
} else {
|
|
||||||
format!("{}", char_count)
|
|
||||||
};
|
|
||||||
let summary = format!("✅ read {} lines | {} chars", output_len, char_display);
|
|
||||||
self.ui_writer.update_tool_output_line(&summary);
|
self.ui_writer.update_tool_output_line(&summary);
|
||||||
} else if is_todo_tool {
|
} else if is_todo_tool {
|
||||||
// Skip - todo tools print their own content
|
// Skip - todo tools print their own content
|
||||||
@@ -2264,43 +2246,27 @@ impl<W: UiWriter> Agent<W> {
|
|||||||
)
|
)
|
||||||
};
|
};
|
||||||
let mut result_message = {
|
let mut result_message = {
|
||||||
// Check if we should use cache control (every 10 tool calls)
|
let content = format!("Tool result: {}", tool_result);
|
||||||
// But only if we haven't already added 4 cache_control annotations
|
|
||||||
if self.tool_call_count > 0
|
// Apply cache control every 10 tool calls (max 4 annotations)
|
||||||
|
let should_cache = self.tool_call_count > 0
|
||||||
&& self.tool_call_count % 10 == 0
|
&& self.tool_call_count % 10 == 0
|
||||||
&& self.count_cache_controls_in_history() < 4
|
&& self.count_cache_controls_in_history() < 4;
|
||||||
{
|
|
||||||
|
if should_cache {
|
||||||
let provider = self.providers.get(None)?;
|
let provider = self.providers.get(None)?;
|
||||||
let provider_name = provider.name();
|
if let Some(cache_config) = self.get_provider_cache_control() {
|
||||||
let (provider_type, config_name) = provider_config::parse_provider_ref(provider_name);
|
|
||||||
if let Some(cache_config) = match provider_type {
|
|
||||||
"anthropic" => {
|
|
||||||
self.config
|
|
||||||
.providers
|
|
||||||
.anthropic
|
|
||||||
.get(config_name)
|
|
||||||
.and_then(|c| c.cache_config.as_ref())
|
|
||||||
.and_then(|config| Self::parse_cache_control(config))
|
|
||||||
}
|
|
||||||
_ => None,
|
|
||||||
} {
|
|
||||||
Message::with_cache_control_validated(
|
Message::with_cache_control_validated(
|
||||||
MessageRole::User,
|
MessageRole::User,
|
||||||
format!("Tool result: {}", tool_result),
|
content,
|
||||||
cache_config,
|
cache_config,
|
||||||
provider,
|
provider,
|
||||||
)
|
)
|
||||||
} else {
|
} else {
|
||||||
Message::new(
|
Message::new(MessageRole::User, content)
|
||||||
MessageRole::User,
|
|
||||||
format!("Tool result: {}", tool_result),
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
Message::new(
|
Message::new(MessageRole::User, content)
|
||||||
MessageRole::User,
|
|
||||||
format!("Tool result: {}", tool_result),
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|||||||
@@ -248,6 +248,50 @@ pub fn are_tool_calls_duplicate(tc1: &ToolCall, tc2: &ToolCall) -> bool {
|
|||||||
tc1.tool == tc2.tool && tc1.args == tc2.args
|
tc1.tool == tc2.tool && tc1.args == tc2.args
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Format a tool argument value for display (truncated for readability).
|
||||||
|
/// Special handling for shell commands (show first line only if multiline).
|
||||||
|
pub fn format_tool_arg_value(tool_name: &str, key: &str, value: &serde_json::Value) -> String {
|
||||||
|
match value {
|
||||||
|
serde_json::Value::String(s) => {
|
||||||
|
if tool_name == "shell" && key == "command" {
|
||||||
|
// For shell commands, show first line only if multiline
|
||||||
|
match s.lines().next() {
|
||||||
|
Some(first_line) if s.lines().count() > 1 => format!("{}...", first_line),
|
||||||
|
Some(first_line) => first_line.to_string(),
|
||||||
|
None => s.clone(),
|
||||||
|
}
|
||||||
|
} else if s.chars().count() > 100 {
|
||||||
|
truncate_for_display(s, 100)
|
||||||
|
} else {
|
||||||
|
s.clone()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
_ => value.to_string(),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Format tool output lines for display, respecting machine vs human mode.
|
||||||
|
pub fn format_tool_output_summary(output: &str, max_lines: usize, max_width: usize, wants_full: bool) -> Vec<String> {
|
||||||
|
let lines: Vec<&str> = output.lines().collect();
|
||||||
|
let limit = if wants_full { lines.len() } else { max_lines };
|
||||||
|
|
||||||
|
lines
|
||||||
|
.iter()
|
||||||
|
.take(limit)
|
||||||
|
.map(|line| truncate_line(line, max_width, !wants_full))
|
||||||
|
.collect()
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Format a read_file result summary (e.g., "✅ read 42 lines | 1.2k chars").
|
||||||
|
pub fn format_read_file_summary(line_count: usize, char_count: usize) -> String {
|
||||||
|
let char_display = if char_count >= 1000 {
|
||||||
|
format!("{:.1}k", char_count as f64 / 1000.0)
|
||||||
|
} else {
|
||||||
|
format!("{}", char_count)
|
||||||
|
};
|
||||||
|
format!("🔍 {} lines read ({} chars)", line_count, char_display)
|
||||||
|
}
|
||||||
|
|
||||||
/// Determine if a response is essentially empty (whitespace or timing only)
|
/// Determine if a response is essentially empty (whitespace or timing only)
|
||||||
pub fn is_empty_response(response: &str) -> bool {
|
pub fn is_empty_response(response: &str) -> bool {
|
||||||
response.trim().is_empty()
|
response.trim().is_empty()
|
||||||
@@ -302,4 +346,36 @@ mod tests {
|
|||||||
assert!(is_connection_error("connection reset"));
|
assert!(is_connection_error("connection reset"));
|
||||||
assert!(!is_connection_error("invalid JSON"));
|
assert!(!is_connection_error("invalid JSON"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_format_tool_arg_value_shell_command() {
|
||||||
|
let val = serde_json::json!("echo hello\necho world");
|
||||||
|
assert_eq!(format_tool_arg_value("shell", "command", &val), "echo hello...");
|
||||||
|
|
||||||
|
let single_line = serde_json::json!("ls -la");
|
||||||
|
assert_eq!(format_tool_arg_value("shell", "command", &single_line), "ls -la");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_format_tool_arg_value_truncation() {
|
||||||
|
let long_str = "a".repeat(150);
|
||||||
|
let val = serde_json::json!(long_str);
|
||||||
|
let result = format_tool_arg_value("read_file", "path", &val);
|
||||||
|
assert!(result.len() < 110); // 100 chars + "..."
|
||||||
|
assert!(result.ends_with("..."));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_format_read_file_summary() {
|
||||||
|
assert_eq!(format_read_file_summary(42, 500), "🔍 42 lines read (500 chars)");
|
||||||
|
assert_eq!(format_read_file_summary(100, 1500), "🔍 100 lines read (1.5k chars)");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_format_tool_output_summary() {
|
||||||
|
let output = "line1\nline2\nline3\nline4\nline5\nline6";
|
||||||
|
let lines = format_tool_output_summary(output, 3, 80, false);
|
||||||
|
assert_eq!(lines.len(), 3);
|
||||||
|
assert_eq!(lines[0], "line1");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user