refactor(g3-core): extract tool output formatting to streaming.rs
Centralize tool output formatting logic that was duplicated/scattered in stream_completion_with_tools(). This eliminates code-path aliasing where tool type checks were done in multiple places. Changes: - Add ToolOutputFormat enum (SelfHandled, Compact, Regular) - Add format_tool_result_summary() for centralized formatting decisions - Add is_compact_tool() and is_self_handled_tool() helper functions - Move parse_diff_stats() from lib.rs to streaming.rs - Simplify tool execution display logic in lib.rs using new helpers Net effect: -86 lines in lib.rs, +112 lines in streaming.rs The streaming.rs additions are reusable, well-named functions. All 585+ workspace tests pass. Agent: fowler
This commit is contained in:
@@ -1,5 +1,5 @@
|
|||||||
# Project Memory
|
# Project Memory
|
||||||
> Updated: 2026-01-20T09:01:08Z | Size: 16.7k chars
|
> Updated: 2026-01-20T09:35:59Z | Size: 17.5k chars
|
||||||
|
|
||||||
### Remember Tool Wiring
|
### Remember Tool Wiring
|
||||||
- `crates/g3-core/src/tools/memory.rs` [0..5000] - `execute_remember()`, `get_memory_path()`, `merge_memory()`
|
- `crates/g3-core/src/tools/memory.rs` [0..5000] - `execute_remember()`, `get_memory_path()`, `merge_memory()`
|
||||||
@@ -303,3 +303,14 @@ Handles `/` commands in interactive mode (extracted from interactive.rs).
|
|||||||
- `crates/g3-cli/src/commands.rs`
|
- `crates/g3-cli/src/commands.rs`
|
||||||
- `handle_command()` [17..320] - dispatches `/help`, `/compact`, `/thinnify`, `/skinnify`, `/fragments`, `/rehydrate`, `/run`, `/dump`, `/clear`, `/readme`, `/stats`, `/resume`
|
- `handle_command()` [17..320] - dispatches `/help`, `/compact`, `/thinnify`, `/skinnify`, `/fragments`, `/rehydrate`, `/run`, `/dump`, `/clear`, `/readme`, `/stats`, `/resume`
|
||||||
- Returns `Result<bool>` - true if command handled and loop should continue
|
- Returns `Result<bool>` - true if command handled and loop should continue
|
||||||
|
|
||||||
|
### Streaming State Management
|
||||||
|
State structs for the main streaming loop in `stream_completion_with_tools()`.
|
||||||
|
|
||||||
|
- `crates/g3-core/src/streaming.rs`
|
||||||
|
- `StreamingState` [17..42] - cross-iteration state: `full_response`, `first_token_time`, `stream_start`, `iteration_count`, `response_started`, `any_tool_executed`, `assistant_message_added`, `turn_accumulated_usage`
|
||||||
|
- `IterationState` [65..90] - per-iteration state: `parser`, `current_response`, `tool_executed`, `chunks_received`, `raw_chunks`, `accumulated_usage`, `stream_stop_reason`
|
||||||
|
- `MAX_ITERATIONS` [15] - constant (400) for loop safety
|
||||||
|
|
||||||
|
- `crates/g3-core/src/lib.rs`
|
||||||
|
- `stream_completion_with_tools()` [1879..2712] - 834-line main streaming loop, uses `state: StreamingState` and `iter: IterationState`
|
||||||
@@ -2120,8 +2120,8 @@ Skip if nothing new. Be brief."#;
|
|||||||
|
|
||||||
self.ui_writer.finish_streaming_markdown();
|
self.ui_writer.finish_streaming_markdown();
|
||||||
|
|
||||||
let is_todo_tool =
|
let is_todo_tool = streaming::is_self_handled_tool(&tool_call.tool);
|
||||||
tool_call.tool == "todo_read" || tool_call.tool == "todo_write";
|
let is_compact_tool = streaming::is_compact_tool(&tool_call.tool);
|
||||||
|
|
||||||
// Tool call header (TODO tools print their own)
|
// Tool call header (TODO tools print their own)
|
||||||
if !is_todo_tool {
|
if !is_todo_tool {
|
||||||
@@ -2139,19 +2139,6 @@ Skip if nothing new. Be brief."#;
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check if this is a compact tool (file operations)
|
|
||||||
let is_compact_tool = matches!(
|
|
||||||
tool_call.tool.as_str(),
|
|
||||||
"read_file"
|
|
||||||
| "write_file"
|
|
||||||
| "str_replace"
|
|
||||||
| "remember"
|
|
||||||
| "screenshot"
|
|
||||||
| "coverage"
|
|
||||||
| "rehydrate"
|
|
||||||
| "code_search"
|
|
||||||
);
|
|
||||||
|
|
||||||
// Only print output header for non-compact tools
|
// Only print output header for non-compact tools
|
||||||
if !is_compact_tool && !is_todo_tool {
|
if !is_compact_tool && !is_todo_tool {
|
||||||
self.ui_writer.print_tool_output_header();
|
self.ui_writer.print_tool_output_header();
|
||||||
@@ -2201,50 +2188,15 @@ Skip if nothing new. Be brief."#;
|
|||||||
const MAX_LINE_WIDTH: usize = 80;
|
const MAX_LINE_WIDTH: usize = 80;
|
||||||
let output_len = output_lines.len();
|
let output_len = output_lines.len();
|
||||||
|
|
||||||
// Determine output format based on tool type
|
// Use centralized tool output formatting
|
||||||
if is_todo_tool {
|
match streaming::format_tool_result_summary(
|
||||||
// TODO tools handle their own output
|
&tool_call.tool,
|
||||||
None
|
|
||||||
} else if is_compact_tool {
|
|
||||||
// Compact tools: show one-line summary
|
|
||||||
if !tool_success {
|
|
||||||
Some(streaming::truncate_for_display(&tool_result, 60))
|
|
||||||
} else {
|
|
||||||
match tool_call.tool.as_str() {
|
|
||||||
"read_file" => {
|
|
||||||
Some(streaming::format_read_file_summary(
|
|
||||||
output_len,
|
|
||||||
tool_result.len(),
|
|
||||||
))
|
|
||||||
}
|
|
||||||
"write_file" => Some(
|
|
||||||
streaming::format_write_file_result(&tool_result),
|
|
||||||
),
|
|
||||||
"str_replace" => {
|
|
||||||
let (ins, del) = parse_diff_stats(&tool_result);
|
|
||||||
Some(streaming::format_str_replace_summary(
|
|
||||||
ins, del,
|
|
||||||
))
|
|
||||||
}
|
|
||||||
"remember" => Some(streaming::format_remember_summary(
|
|
||||||
&tool_result,
|
&tool_result,
|
||||||
)),
|
tool_success,
|
||||||
"screenshot" => Some(
|
) {
|
||||||
streaming::format_screenshot_summary(&tool_result),
|
streaming::ToolOutputFormat::SelfHandled => None,
|
||||||
),
|
streaming::ToolOutputFormat::Compact(summary) => Some(summary),
|
||||||
"coverage" => Some(streaming::format_coverage_summary(
|
streaming::ToolOutputFormat::Regular => {
|
||||||
&tool_result,
|
|
||||||
)),
|
|
||||||
"rehydrate" => Some(
|
|
||||||
streaming::format_rehydrate_summary(&tool_result),
|
|
||||||
),
|
|
||||||
"code_search" => Some(
|
|
||||||
streaming::format_code_search_summary(&tool_result),
|
|
||||||
),
|
|
||||||
_ => Some("✅ completed".to_string()),
|
|
||||||
}
|
|
||||||
}
|
|
||||||
} else {
|
|
||||||
// Regular tools: show truncated output lines
|
// Regular tools: show truncated output lines
|
||||||
let max_lines_to_show =
|
let max_lines_to_show =
|
||||||
if wants_full { output_len } else { MAX_LINES };
|
if wants_full { output_len } else { MAX_LINES };
|
||||||
@@ -2265,6 +2217,7 @@ Skip if nothing new. Be brief."#;
|
|||||||
}
|
}
|
||||||
None
|
None
|
||||||
}
|
}
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
// Add the tool call and result to the context window using RAW unfiltered content
|
// Add the tool call and result to the context window using RAW unfiltered content
|
||||||
@@ -2771,33 +2724,6 @@ Skip if nothing new. Be brief."#;
|
|||||||
pub use utils::apply_unified_diff_to_string;
|
pub use utils::apply_unified_diff_to_string;
|
||||||
use utils::truncate_to_word_boundary;
|
use utils::truncate_to_word_boundary;
|
||||||
|
|
||||||
/// Parse insertions and deletions from a str_replace result.
|
|
||||||
/// Result format: "✅ +N insertions | -M deletions"
|
|
||||||
fn parse_diff_stats(result: &str) -> (i32, i32) {
|
|
||||||
let mut insertions = 0i32;
|
|
||||||
let mut deletions = 0i32;
|
|
||||||
|
|
||||||
// Look for "+N insertions" pattern
|
|
||||||
if let Some(pos) = result.find("+") {
|
|
||||||
let after_plus = &result[pos + 1..];
|
|
||||||
insertions = after_plus
|
|
||||||
.split_whitespace()
|
|
||||||
.next()
|
|
||||||
.and_then(|s| s.parse().ok())
|
|
||||||
.unwrap_or(0);
|
|
||||||
}
|
|
||||||
// Look for "-M deletions" pattern
|
|
||||||
if let Some(pos) = result.find("-") {
|
|
||||||
let after_minus = &result[pos + 1..];
|
|
||||||
deletions = after_minus
|
|
||||||
.split_whitespace()
|
|
||||||
.next()
|
|
||||||
.and_then(|s| s.parse().ok())
|
|
||||||
.unwrap_or(0);
|
|
||||||
}
|
|
||||||
(insertions, deletions)
|
|
||||||
}
|
|
||||||
|
|
||||||
// Implement Drop to clean up safaridriver process
|
// Implement Drop to clean up safaridriver process
|
||||||
impl<W: UiWriter> Drop for Agent<W> {
|
impl<W: UiWriter> Drop for Agent<W> {
|
||||||
fn drop(&mut self) {
|
fn drop(&mut self) {
|
||||||
|
|||||||
@@ -53,6 +53,118 @@ impl StreamingState {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Result of formatting a tool's output for display
|
||||||
|
pub enum ToolOutputFormat {
|
||||||
|
/// Tool handles its own output (e.g., TODO tools)
|
||||||
|
SelfHandled,
|
||||||
|
/// Compact one-line summary for file operations
|
||||||
|
Compact(String),
|
||||||
|
/// Regular multi-line output (already displayed via ui_writer)
|
||||||
|
Regular,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Format a tool result for compact display.
|
||||||
|
/// Returns the appropriate format based on tool type and success.
|
||||||
|
pub fn format_tool_result_summary(
|
||||||
|
tool_name: &str,
|
||||||
|
tool_result: &str,
|
||||||
|
tool_success: bool,
|
||||||
|
) -> ToolOutputFormat {
|
||||||
|
let is_todo_tool = tool_name == "todo_read" || tool_name == "todo_write";
|
||||||
|
let is_compact_tool = matches!(
|
||||||
|
tool_name,
|
||||||
|
"read_file"
|
||||||
|
| "write_file"
|
||||||
|
| "str_replace"
|
||||||
|
| "remember"
|
||||||
|
| "screenshot"
|
||||||
|
| "coverage"
|
||||||
|
| "rehydrate"
|
||||||
|
| "code_search"
|
||||||
|
);
|
||||||
|
|
||||||
|
if is_todo_tool {
|
||||||
|
ToolOutputFormat::SelfHandled
|
||||||
|
} else if is_compact_tool {
|
||||||
|
if !tool_success {
|
||||||
|
ToolOutputFormat::Compact(truncate_for_display(tool_result, 60))
|
||||||
|
} else {
|
||||||
|
let summary = format_compact_tool_summary(tool_name, tool_result);
|
||||||
|
ToolOutputFormat::Compact(summary)
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
ToolOutputFormat::Regular
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Check if a tool is a "compact" tool that shows one-line summaries
|
||||||
|
pub fn is_compact_tool(tool_name: &str) -> bool {
|
||||||
|
matches!(
|
||||||
|
tool_name,
|
||||||
|
"read_file"
|
||||||
|
| "write_file"
|
||||||
|
| "str_replace"
|
||||||
|
| "remember"
|
||||||
|
| "screenshot"
|
||||||
|
| "coverage"
|
||||||
|
| "rehydrate"
|
||||||
|
| "code_search"
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Check if a tool handles its own output display
|
||||||
|
pub fn is_self_handled_tool(tool_name: &str) -> bool {
|
||||||
|
tool_name == "todo_read" || tool_name == "todo_write"
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Format a compact summary for a successful compact tool
|
||||||
|
fn format_compact_tool_summary(tool_name: &str, tool_result: &str) -> String {
|
||||||
|
let output_lines: Vec<&str> = tool_result.lines().collect();
|
||||||
|
let output_len = output_lines.len();
|
||||||
|
|
||||||
|
match tool_name {
|
||||||
|
"read_file" => format_read_file_summary(output_len, tool_result.len()),
|
||||||
|
"write_file" => format_write_file_result(tool_result),
|
||||||
|
"str_replace" => {
|
||||||
|
let (ins, del) = parse_diff_stats(tool_result);
|
||||||
|
format_str_replace_summary(ins, del)
|
||||||
|
}
|
||||||
|
"remember" => format_remember_summary(tool_result),
|
||||||
|
"screenshot" => format_screenshot_summary(tool_result),
|
||||||
|
"coverage" => format_coverage_summary(tool_result),
|
||||||
|
"rehydrate" => format_rehydrate_summary(tool_result),
|
||||||
|
"code_search" => format_code_search_summary(tool_result),
|
||||||
|
_ => "✅ completed".to_string(),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Parse diff stats from str_replace result
|
||||||
|
fn parse_diff_stats(result: &str) -> (i32, i32) {
|
||||||
|
// Format: "✅ +N insertions | -M deletions"
|
||||||
|
let mut insertions = 0i32;
|
||||||
|
let mut deletions = 0i32;
|
||||||
|
|
||||||
|
// Look for "+N insertions" pattern
|
||||||
|
if let Some(pos) = result.find("+") {
|
||||||
|
let after_plus = &result[pos + 1..];
|
||||||
|
insertions = after_plus
|
||||||
|
.split_whitespace()
|
||||||
|
.next()
|
||||||
|
.and_then(|s| s.parse().ok())
|
||||||
|
.unwrap_or(0);
|
||||||
|
}
|
||||||
|
// Look for "-M deletions" pattern
|
||||||
|
if let Some(pos) = result.find("-") {
|
||||||
|
let after_minus = &result[pos + 1..];
|
||||||
|
deletions = after_minus
|
||||||
|
.split_whitespace()
|
||||||
|
.next()
|
||||||
|
.and_then(|s| s.parse().ok())
|
||||||
|
.unwrap_or(0);
|
||||||
|
}
|
||||||
|
(insertions, deletions)
|
||||||
|
}
|
||||||
|
|
||||||
impl Default for StreamingState {
|
impl Default for StreamingState {
|
||||||
fn default() -> Self {
|
fn default() -> Self {
|
||||||
Self::new()
|
Self::new()
|
||||||
|
|||||||
Reference in New Issue
Block a user