mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-09 03:20:56 +00:00
Simplify coding-agent-loop: remove redundant constructors, use let-else, add doc comments
- Remove History::new() (redundant with #[derive(Default)]), replace all callers with History::default() - Replace match with let-else in extract_signatures_from_assistant for clearer happy path - Add doc comments to Turn::System and Turn::Steering explaining their LLM role mapping - Verified: Arc import in openai.rs is used in production code, mutex .expect() messages are consistent Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
c15b532bcf
commit
23cb9d8e58
4 changed files with 30 additions and 31 deletions
|
|
@ -7,10 +7,6 @@ pub struct History {
|
|||
}
|
||||
|
||||
impl History {
|
||||
pub fn new() -> Self {
|
||||
Self { turns: Vec::new() }
|
||||
}
|
||||
|
||||
pub fn push(&mut self, turn: Turn) {
|
||||
self.turns.push(turn);
|
||||
}
|
||||
|
|
@ -87,14 +83,14 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn empty_history_produces_empty_messages() {
|
||||
let history = History::new();
|
||||
let history = History::default();
|
||||
assert!(history.convert_to_messages().is_empty());
|
||||
assert_eq!(history.turns().len(), 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn user_turn_maps_to_user_message() {
|
||||
let mut history = History::new();
|
||||
let mut history = History::default();
|
||||
history.push(Turn::User {
|
||||
content: "Hello".into(),
|
||||
timestamp: SystemTime::now(),
|
||||
|
|
@ -107,7 +103,7 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn assistant_turn_maps_to_assistant_message() {
|
||||
let mut history = History::new();
|
||||
let mut history = History::default();
|
||||
history.push(Turn::Assistant {
|
||||
content: "Hi there".into(),
|
||||
tool_calls: vec![],
|
||||
|
|
@ -124,7 +120,7 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn assistant_turn_with_tool_calls() {
|
||||
let mut history = History::new();
|
||||
let mut history = History::default();
|
||||
let tc = ToolCall::new("call_1", "read_file", serde_json::json!({"path": "foo.rs"}));
|
||||
history.push(Turn::Assistant {
|
||||
content: "Let me read that".into(),
|
||||
|
|
@ -146,7 +142,7 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn assistant_turn_with_reasoning() {
|
||||
let mut history = History::new();
|
||||
let mut history = History::default();
|
||||
history.push(Turn::Assistant {
|
||||
content: "The answer is 42".into(),
|
||||
tool_calls: vec![],
|
||||
|
|
@ -166,7 +162,7 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn tool_results_turn_maps_to_tool_message() {
|
||||
let mut history = History::new();
|
||||
let mut history = History::default();
|
||||
let result = ToolResult {
|
||||
tool_call_id: "call_1".into(),
|
||||
content: serde_json::json!("file contents here"),
|
||||
|
|
@ -186,7 +182,7 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn system_turn_maps_to_system_message() {
|
||||
let mut history = History::new();
|
||||
let mut history = History::default();
|
||||
history.push(Turn::System {
|
||||
content: "You are a coding assistant".into(),
|
||||
timestamp: SystemTime::now(),
|
||||
|
|
@ -199,7 +195,7 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn steering_turn_maps_to_user_message() {
|
||||
let mut history = History::new();
|
||||
let mut history = History::default();
|
||||
history.push(Turn::Steering {
|
||||
content: "Focus on the main task".into(),
|
||||
timestamp: SystemTime::now(),
|
||||
|
|
@ -212,7 +208,7 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn turns_len_matches_push_count() {
|
||||
let mut history = History::new();
|
||||
let mut history = History::default();
|
||||
assert_eq!(history.turns().len(), 0);
|
||||
history.push(Turn::User {
|
||||
content: "First".into(),
|
||||
|
|
@ -232,7 +228,7 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn round_trip_preserves_content() {
|
||||
let mut history = History::new();
|
||||
let mut history = History::default();
|
||||
history.push(Turn::User {
|
||||
content: "Hello".into(),
|
||||
timestamp: SystemTime::now(),
|
||||
|
|
|
|||
|
|
@ -12,13 +12,13 @@ fn tool_call_signature(name: &str, arguments: &serde_json::Value) -> u64 {
|
|||
}
|
||||
|
||||
fn extract_signatures_from_assistant(turn: &Turn) -> Vec<u64> {
|
||||
match turn {
|
||||
Turn::Assistant { tool_calls, .. } => tool_calls
|
||||
.iter()
|
||||
.map(|tc| tool_call_signature(&tc.name, &tc.arguments))
|
||||
.collect(),
|
||||
_ => vec![],
|
||||
}
|
||||
let Turn::Assistant { tool_calls, .. } = turn else {
|
||||
return vec![];
|
||||
};
|
||||
tool_calls
|
||||
.iter()
|
||||
.map(|tc| tool_call_signature(&tc.name, &tc.arguments))
|
||||
.collect()
|
||||
}
|
||||
|
||||
pub fn detect_loop(history: &History, window_size: usize) -> bool {
|
||||
|
|
@ -109,20 +109,20 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn too_few_turns_returns_false() {
|
||||
let history = History::new();
|
||||
let history = History::default();
|
||||
assert!(!detect_loop(&history, 10));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn single_turn_returns_false() {
|
||||
let mut history = History::new();
|
||||
let mut history = History::default();
|
||||
history.push(assistant_with_tool("shell", serde_json::json!({"cmd": "ls"})));
|
||||
assert!(!detect_loop(&history, 10));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pattern_1_repeating_detected() {
|
||||
let mut history = History::new();
|
||||
let mut history = History::default();
|
||||
// Same tool call repeated 3 times
|
||||
history.push(assistant_with_tool("shell", serde_json::json!({"cmd": "ls"})));
|
||||
history.push(assistant_with_tool("shell", serde_json::json!({"cmd": "ls"})));
|
||||
|
|
@ -132,7 +132,7 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn pattern_2_repeating_detected() {
|
||||
let mut history = History::new();
|
||||
let mut history = History::default();
|
||||
// A-B-A-B pattern
|
||||
history.push(assistant_with_tool("shell", serde_json::json!({"cmd": "ls"})));
|
||||
history.push(assistant_with_tool("read_file", serde_json::json!({"path": "foo.rs"})));
|
||||
|
|
@ -143,7 +143,7 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn pattern_3_repeating_detected() {
|
||||
let mut history = History::new();
|
||||
let mut history = History::default();
|
||||
// A-B-C-A-B-C pattern
|
||||
history.push(assistant_with_tool("shell", serde_json::json!({"cmd": "ls"})));
|
||||
history.push(assistant_with_tool("read_file", serde_json::json!({"path": "a.rs"})));
|
||||
|
|
@ -156,7 +156,7 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn non_repeating_returns_false() {
|
||||
let mut history = History::new();
|
||||
let mut history = History::default();
|
||||
history.push(assistant_with_tool("shell", serde_json::json!({"cmd": "ls"})));
|
||||
history.push(assistant_with_tool("read_file", serde_json::json!({"path": "a.rs"})));
|
||||
history.push(assistant_with_tool("grep", serde_json::json!({"pattern": "fn"})));
|
||||
|
|
@ -166,7 +166,7 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn same_name_different_args_are_different() {
|
||||
let mut history = History::new();
|
||||
let mut history = History::default();
|
||||
history.push(assistant_with_tool("shell", serde_json::json!({"cmd": "ls"})));
|
||||
history.push(assistant_with_tool("shell", serde_json::json!({"cmd": "pwd"})));
|
||||
history.push(assistant_with_tool("shell", serde_json::json!({"cmd": "cat"})));
|
||||
|
|
@ -196,7 +196,7 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn user_turns_are_ignored() {
|
||||
let mut history = History::new();
|
||||
let mut history = History::default();
|
||||
history.push(Turn::User {
|
||||
content: "hello".into(),
|
||||
timestamp: SystemTime::now(),
|
||||
|
|
@ -214,7 +214,7 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn window_size_limits_lookback() {
|
||||
let mut history = History::new();
|
||||
let mut history = History::default();
|
||||
// Add non-repeating turns first
|
||||
history.push(assistant_with_tool("shell", serde_json::json!({"cmd": "unique1"})));
|
||||
history.push(assistant_with_tool("shell", serde_json::json!({"cmd": "unique2"})));
|
||||
|
|
|
|||
|
|
@ -45,7 +45,7 @@ impl Session {
|
|||
Self {
|
||||
id: uuid::Uuid::new_v4().to_string(),
|
||||
config,
|
||||
history: History::new(),
|
||||
history: History::default(),
|
||||
event_emitter: EventEmitter::new(),
|
||||
state: SessionState::Idle,
|
||||
llm_client,
|
||||
|
|
|
|||
|
|
@ -19,10 +19,13 @@ pub enum Turn {
|
|||
results: Vec<ToolResult>,
|
||||
timestamp: SystemTime,
|
||||
},
|
||||
/// Injected content sent as a system-role message to the LLM (maps to `Role::System`).
|
||||
System {
|
||||
content: String,
|
||||
timestamp: SystemTime,
|
||||
},
|
||||
/// Injected steering content sent as a user-role message to the LLM (maps to `Role::User`).
|
||||
/// Used to guide the assistant's behavior mid-conversation without appearing as actual user input.
|
||||
Steering {
|
||||
content: String,
|
||||
timestamp: SystemTime,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue