Skip to content

Commit 696eda9

Browse files
haasonsaasclaude
andcommitted
feat: promote agent review to first-class UI with tool config and activity tracking
- Add dedicated Agent tab in Settings with tool-level toggles fetched from new GET /api/agent/tools endpoint; removing agent fields from Advanced tab - Persist agent loop activity (iterations, tool calls with timing) through ReviewResult → ReviewEvent → frontend EventPanel - Add AgentToolInfo catalog, build_review_tools() filtering by enabled list, AgentToolCallLog collection in agent loop - Show agent iteration indicator and collapsible tool call timeline in ReviewView Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent ce1f6d6 commit 696eda9

14 files changed

Lines changed: 385 additions & 44 deletions

File tree

src/config.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,10 @@ pub struct Config {
285285
/// Optional total token budget for agent loop.
286286
#[serde(default)]
287287
pub agent_max_total_tokens: Option<usize>,
288+
289+
/// Which agent tools are enabled. None = all tools enabled.
290+
#[serde(default)]
291+
pub agent_tools_enabled: Option<Vec<String>>,
288292
}
289293

290294
#[derive(Debug, Clone, Serialize, Deserialize, Default)]
@@ -426,6 +430,7 @@ impl Default for Config {
426430
agent_review: false,
427431
agent_max_iterations: default_agent_max_iterations(),
428432
agent_max_total_tokens: None,
433+
agent_tools_enabled: None,
429434
}
430435
}
431436
}

src/core/agent_loop.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,14 @@ pub enum AgentEvent {
4747
},
4848
}
4949

50+
/// Log entry for a single tool call during the agent loop.
51+
#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)]
52+
pub struct AgentToolCallLog {
53+
pub iteration: usize,
54+
pub tool_name: String,
55+
pub duration_ms: u64,
56+
}
57+
5058
/// Result of running the agent loop.
5159
#[derive(Debug, Clone)]
5260
#[allow(dead_code)]
@@ -59,6 +67,8 @@ pub struct AgentLoopResult {
5967
pub total_usage: Usage,
6068
/// Number of LLM round-trips performed.
6169
pub iterations: usize,
70+
/// Log of all tool calls made during the loop.
71+
pub tool_calls: Vec<AgentToolCallLog>,
6272
}
6373

6474
/// Run an iterative agent loop: LLM call → tool execution → repeat until done.
@@ -95,6 +105,7 @@ pub async fn run_agent_loop(
95105
total_tokens: 0,
96106
}),
97107
iterations: 1,
108+
tool_calls: Vec::new(),
98109
});
99110
}
100111

@@ -106,6 +117,7 @@ pub async fn run_agent_loop(
106117
};
107118
let mut accumulated_text = String::new();
108119
let mut model = String::new();
120+
let mut tool_call_log: Vec<AgentToolCallLog> = Vec::new();
109121

110122
for iteration in 0..config.max_iterations {
111123
let request = ChatRequest {
@@ -171,6 +183,7 @@ pub async fn run_agent_loop(
171183
model,
172184
total_usage,
173185
iterations: iteration + 1,
186+
tool_calls: tool_call_log,
174187
});
175188
}
176189

@@ -190,6 +203,7 @@ pub async fn run_agent_loop(
190203
model,
191204
total_usage,
192205
iterations: iteration + 1,
206+
tool_calls: tool_call_log,
193207
});
194208
}
195209

@@ -225,6 +239,11 @@ pub async fn run_agent_loop(
225239
};
226240

227241
let duration_ms = start.elapsed().as_millis() as u64;
242+
tool_call_log.push(AgentToolCallLog {
243+
iteration,
244+
tool_name: call_name.clone(),
245+
duration_ms,
246+
});
228247
if let Some(ref cb) = on_event {
229248
cb(AgentEvent::ToolCalled {
230249
iteration,
@@ -267,6 +286,7 @@ pub async fn run_agent_loop(
267286
model,
268287
total_usage,
269288
iterations: iteration + 1,
289+
tool_calls: tool_call_log,
270290
});
271291
}
272292
}
@@ -290,6 +310,7 @@ pub async fn run_agent_loop(
290310
model,
291311
total_usage,
292312
iterations: config.max_iterations,
313+
tool_calls: tool_call_log,
293314
})
294315
}
295316

src/core/agent_tools.rs

Lines changed: 105 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use anyhow::Result;
22
use async_trait::async_trait;
3+
use serde::{Deserialize, Serialize};
34
use serde_json::json;
45
use std::path::PathBuf;
56
use std::sync::Arc;
@@ -10,6 +11,51 @@ use crate::core::git_history::GitHistoryAnalyzer;
1011
use crate::core::symbol_graph::SymbolGraph;
1112
use crate::core::symbol_index::SymbolIndex;
1213

14+
/// Metadata about an available agent tool, for the API catalog.
15+
#[derive(Debug, Clone, Serialize, Deserialize)]
16+
pub struct AgentToolInfo {
17+
pub name: String,
18+
pub description: String,
19+
#[serde(skip_serializing_if = "Option::is_none")]
20+
pub requires: Option<String>,
21+
}
22+
23+
/// Return a static catalog of all agent tools with their descriptions.
24+
pub fn list_all_tool_info() -> Vec<AgentToolInfo> {
25+
vec![
26+
AgentToolInfo {
27+
name: "read_file".to_string(),
28+
description: "Read the contents of a file in the repository. Returns the file content with line numbers.".to_string(),
29+
requires: None,
30+
},
31+
AgentToolInfo {
32+
name: "search_codebase".to_string(),
33+
description: "Search the codebase for a regex pattern. Returns matching lines with file paths and line numbers.".to_string(),
34+
requires: None,
35+
},
36+
AgentToolInfo {
37+
name: "lookup_symbol".to_string(),
38+
description: "Look up a symbol (function, struct, class, etc.) in the codebase index. Returns definition locations and code snippets.".to_string(),
39+
requires: Some("symbol index".to_string()),
40+
},
41+
AgentToolInfo {
42+
name: "get_definitions".to_string(),
43+
description: "Get the definitions of specific symbols as they appear in a given file, using the symbol index for precise lookup.".to_string(),
44+
requires: Some("symbol index".to_string()),
45+
},
46+
AgentToolInfo {
47+
name: "get_related_symbols".to_string(),
48+
description: "Find symbols related to the given symbols (callers, callees, implementations, etc.) using the symbol graph.".to_string(),
49+
requires: Some("symbol graph".to_string()),
50+
},
51+
AgentToolInfo {
52+
name: "get_file_history".to_string(),
53+
description: "Get git history and churn metrics for a file: commit count, bug fix count, distinct authors, risk score.".to_string(),
54+
requires: Some("git history".to_string()),
55+
},
56+
]
57+
}
58+
1359
/// Maximum bytes returned by any single tool execution (8 KB).
1460
const MAX_TOOL_OUTPUT_BYTES: usize = 8 * 1024;
1561

@@ -30,22 +76,43 @@ pub trait ReviewTool: Send + Sync {
3076
}
3177

3278
/// Build the standard set of review tools from the given context.
33-
pub fn build_review_tools(ctx: Arc<ReviewToolContext>) -> Vec<Box<dyn ReviewTool>> {
34-
let mut tools: Vec<Box<dyn ReviewTool>> = vec![
35-
Box::new(ReadFileTool { ctx: ctx.clone() }),
36-
Box::new(SearchCodebaseTool { ctx: ctx.clone() }),
37-
];
79+
///
80+
/// If `enabled_filter` is `Some`, only tools whose names appear in the list are included.
81+
/// If `None`, all available tools are included.
82+
pub fn build_review_tools(
83+
ctx: Arc<ReviewToolContext>,
84+
enabled_filter: Option<&[String]>,
85+
) -> Vec<Box<dyn ReviewTool>> {
86+
let is_enabled = |name: &str| -> bool {
87+
match enabled_filter {
88+
None => true,
89+
Some(list) => list.iter().any(|s| s == name),
90+
}
91+
};
92+
93+
let mut tools: Vec<Box<dyn ReviewTool>> = Vec::new();
94+
95+
if is_enabled("read_file") {
96+
tools.push(Box::new(ReadFileTool { ctx: ctx.clone() }));
97+
}
98+
if is_enabled("search_codebase") {
99+
tools.push(Box::new(SearchCodebaseTool { ctx: ctx.clone() }));
100+
}
38101

39102
if ctx.symbol_index.is_some() {
40-
tools.push(Box::new(LookupSymbolTool { ctx: ctx.clone() }));
41-
tools.push(Box::new(GetDefinitionsTool { ctx: ctx.clone() }));
103+
if is_enabled("lookup_symbol") {
104+
tools.push(Box::new(LookupSymbolTool { ctx: ctx.clone() }));
105+
}
106+
if is_enabled("get_definitions") {
107+
tools.push(Box::new(GetDefinitionsTool { ctx: ctx.clone() }));
108+
}
42109
}
43110

44-
if ctx.symbol_graph.is_some() {
111+
if ctx.symbol_graph.is_some() && is_enabled("get_related_symbols") {
45112
tools.push(Box::new(GetRelatedSymbolsTool { ctx: ctx.clone() }));
46113
}
47114

48-
if ctx.git_history.is_some() {
115+
if ctx.git_history.is_some() && is_enabled("get_file_history") {
49116
tools.push(Box::new(GetFileHistoryTool { ctx: ctx.clone() }));
50117
}
51118

@@ -501,7 +568,7 @@ mod tests {
501568
symbol_graph: None,
502569
git_history: None,
503570
});
504-
let tools = build_review_tools(ctx);
571+
let tools = build_review_tools(ctx, None);
505572
// At minimum: read_file + search_codebase
506573
assert_eq!(tools.len(), 2);
507574
assert_eq!(tools[0].name(), "read_file");
@@ -517,7 +584,7 @@ mod tests {
517584
symbol_graph: Some(Arc::new(SymbolGraph::new())),
518585
git_history: Some(Arc::new(GitHistoryAnalyzer::new())),
519586
});
520-
let tools = build_review_tools(ctx);
587+
let tools = build_review_tools(ctx, None);
521588
assert_eq!(tools.len(), 6);
522589
let names: Vec<&str> = tools.iter().map(|t| t.name()).collect();
523590
assert!(names.contains(&"read_file"));
@@ -528,6 +595,32 @@ mod tests {
528595
assert!(names.contains(&"get_file_history"));
529596
}
530597

598+
#[test]
599+
fn test_build_review_tools_filtered() {
600+
let ctx = Arc::new(ReviewToolContext {
601+
repo_path: PathBuf::from("/tmp/test"),
602+
context_fetcher: Arc::new(ContextFetcher::new(PathBuf::from("/tmp/test"))),
603+
symbol_index: Some(Arc::new(SymbolIndex::default())),
604+
symbol_graph: Some(Arc::new(SymbolGraph::new())),
605+
git_history: Some(Arc::new(GitHistoryAnalyzer::new())),
606+
});
607+
let enabled = vec!["read_file".to_string(), "search_codebase".to_string()];
608+
let tools = build_review_tools(ctx, Some(&enabled));
609+
assert_eq!(tools.len(), 2);
610+
assert_eq!(tools[0].name(), "read_file");
611+
assert_eq!(tools[1].name(), "search_codebase");
612+
}
613+
614+
#[test]
615+
fn test_list_all_tool_info() {
616+
let info = list_all_tool_info();
617+
assert_eq!(info.len(), 6);
618+
assert_eq!(info[0].name, "read_file");
619+
assert!(info[0].requires.is_none());
620+
assert_eq!(info[2].name, "lookup_symbol");
621+
assert!(info[2].requires.is_some());
622+
}
623+
531624
#[test]
532625
fn test_tool_definitions_have_required_fields() {
533626
let ctx = Arc::new(ReviewToolContext {
@@ -537,7 +630,7 @@ mod tests {
537630
symbol_graph: Some(Arc::new(SymbolGraph::new())),
538631
git_history: Some(Arc::new(GitHistoryAnalyzer::new())),
539632
});
540-
let tools = build_review_tools(ctx);
633+
let tools = build_review_tools(ctx, None);
541634
for tool in &tools {
542635
let def = tool.definition();
543636
assert!(!def.name.is_empty());

src/review/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ pub use pipeline::{
1919
review_diff_content_with_repo, ProgressCallback, ProgressUpdate,
2020
};
2121
#[allow(unused_imports)]
22-
pub(crate) use pipeline::{FileMetric, ReviewResult};
22+
pub(crate) use pipeline::{AgentActivity, FileMetric, ReviewResult};
2323
pub use rule_helpers::{
2424
apply_rule_overrides, build_pr_summary_comment_body, inject_rule_context, load_review_rules,
2525
normalize_rule_id, summarize_rule_hits,

0 commit comments

Comments
 (0)