Skip to content

Commit e914e97

Browse files
authored
refactor: drop max_walker_depth from the configuration (#2080)
1 parent b9b71e9 commit e914e97

17 files changed

+279
-84
lines changed

crates/forge_app/src/app.rs

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use crate::tool_resolver::ToolResolver;
2222
use crate::user_prompt::UserPromptGenerator;
2323
use crate::{
2424
AgentProviderResolver, ConversationService, EnvironmentService, FileDiscoveryService,
25-
ProviderService, Services, Walker, WorkflowService,
25+
ProviderService, Services, WorkflowService,
2626
};
2727

2828
/// ForgeApp handles the core chat functionality by orchestrating various
@@ -62,22 +62,9 @@ impl<S: Services> ForgeApp<S> {
6262

6363
// Discover files using the discovery service
6464
let workflow = self.services.read_merged(None).await.unwrap_or_default();
65-
let max_depth = workflow.max_walker_depth;
6665
let environment = services.get_environment();
6766

68-
let mut walker = Walker::conservative().cwd(environment.cwd.clone());
69-
70-
if let Some(depth) = max_depth {
71-
walker = walker.max_depth(depth);
72-
};
73-
74-
let files = services
75-
.collect_files(walker)
76-
.await?
77-
.into_iter()
78-
.filter(|f| !f.is_dir)
79-
.map(|f| f.path)
80-
.collect::<Vec<_>>();
67+
let files = services.list_current_directory().await?;
8168

8269
// Register templates using workflow path or environment fallback
8370
let template_path = workflow

crates/forge_app/src/command_generator.rs

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ use forge_domain::{extract_tag_content, *};
55

66
use crate::{
77
AppConfigService, EnvironmentService, FileDiscoveryService, ProviderService, TemplateEngine,
8-
Walker,
98
};
109

1110
/// CommandGenerator handles shell command generation from natural language
@@ -27,18 +26,7 @@ where
2726
// Get system information for context
2827
let env = self.services.get_environment();
2928

30-
let walker = Walker::conservative()
31-
.cwd(env.cwd.clone())
32-
.max_depth(1usize);
33-
let mut files = self
34-
.services
35-
.collect_files(walker)
36-
.await?
37-
.into_iter()
38-
.filter(|f| !f.is_dir)
39-
.map(|f| f.path)
40-
.collect::<Vec<_>>();
41-
files.sort();
29+
let files = self.services.list_current_directory().await?;
4230

4331
let rendered_system_prompt = TemplateEngine::default().render(
4432
"forge-command-generator-prompt.md",
@@ -93,6 +81,7 @@ mod tests {
9381
use url::Url;
9482

9583
use super::*;
84+
use crate::Walker;
9685

9786
struct MockServices {
9887
files: Vec<(String, bool)>,
@@ -135,6 +124,23 @@ mod tests {
135124
.map(|(path, is_dir)| File { path: path.clone(), is_dir: *is_dir })
136125
.collect())
137126
}
127+
128+
async fn list_current_directory(&self) -> Result<Vec<File>> {
129+
let mut files: Vec<File> = self
130+
.files
131+
.iter()
132+
.map(|(path, is_dir)| File { path: path.clone(), is_dir: *is_dir })
133+
.collect();
134+
135+
// Sort: directories first (alphabetically), then files (alphabetically)
136+
files.sort_by(|a, b| match (a.is_dir, b.is_dir) {
137+
(true, false) => std::cmp::Ordering::Less,
138+
(false, true) => std::cmp::Ordering::Greater,
139+
_ => a.path.cmp(&b.path),
140+
});
141+
142+
Ok(files)
143+
}
138144
}
139145

140146
#[async_trait::async_trait]

crates/forge_app/src/orch_spec/orch_setup.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ use chrono::{DateTime, Local};
55
use derive_setters::Setters;
66
use forge_domain::{
77
Agent, AgentId, Attachment, ChatCompletionMessage, ChatResponse, ContextMessage, Conversation,
8-
Environment, Event, HttpConfig, ModelId, ProviderId, RetryConfig, Role, Template, ToolCallFull,
9-
ToolDefinition, ToolResult, Workflow,
8+
Environment, Event, File, HttpConfig, ModelId, ProviderId, RetryConfig, Role, Template,
9+
ToolCallFull, ToolDefinition, ToolResult, Workflow,
1010
};
1111
use url::Url;
1212

@@ -25,7 +25,7 @@ pub struct TestContext {
2525
pub mock_assistant_responses: Vec<ChatCompletionMessage>,
2626
pub workflow: Workflow,
2727
pub templates: HashMap<String, String>,
28-
pub files: Vec<String>,
28+
pub files: Vec<File>,
2929
pub env: Environment,
3030
pub current_time: DateTime<Local>,
3131
pub title: Option<String>,

crates/forge_app/src/orch_spec/orch_system_spec.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ async fn test_system_prompt_tool_supported() {
2626
.custom_rules("Do it nicely"),
2727
)
2828
.files(vec![
29-
"/users/john/foo.txt".to_string(),
30-
"/users/jason/bar.txt".to_string(),
29+
forge_domain::File { path: "/users/john/foo.txt".to_string(), is_dir: false },
30+
forge_domain::File { path: "/users/jason/bar.txt".to_string(), is_dir: false },
3131
])
3232
.mock_assistant_responses(vec![
3333
ChatCompletionMessage::assistant(Content::full("Sure"))

crates/forge_app/src/orch_spec/snapshots/forge_app__orch_spec__orch_system_spec__system_prompt_tool_supported.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ You are Forge
1010
<default_shell>bash</default_shell>
1111
<home_directory>/Users/tushar</home_directory>
1212
<file_list>
13-
- /users/jason/bar.txt
1413
- /users/john/foo.txt
14+
- /users/jason/bar.txt
1515
</file_list>
1616
</system_information>
1717

crates/forge_app/src/services.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,10 @@ pub trait WorkflowService {
308308
#[async_trait::async_trait]
309309
pub trait FileDiscoveryService: Send + Sync {
310310
async fn collect_files(&self, config: Walker) -> anyhow::Result<Vec<File>>;
311+
312+
/// Lists all entries (files and directories) in the current directory
313+
/// Returns a sorted vector of File entries with directories first
314+
async fn list_current_directory(&self) -> anyhow::Result<Vec<File>>;
311315
}
312316

313317
#[async_trait::async_trait]
@@ -713,6 +717,10 @@ impl<I: Services> FileDiscoveryService for I {
713717
async fn collect_files(&self, config: Walker) -> anyhow::Result<Vec<File>> {
714718
self.file_discovery_service().collect_files(config).await
715719
}
720+
721+
async fn list_current_directory(&self) -> anyhow::Result<Vec<File>> {
722+
self.file_discovery_service().list_current_directory().await
723+
}
716724
}
717725

718726
#[async_trait::async_trait]

crates/forge_app/src/system_prompt.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use std::sync::Arc;
22

33
use derive_setters::Setters;
44
use forge_domain::{
5-
Agent, Conversation, Environment, Model, SystemContext, Template, ToolDefinition,
5+
Agent, Conversation, Environment, File, Model, SystemContext, Template, ToolDefinition,
66
ToolUsagePrompt,
77
};
88
use tracing::debug;
@@ -15,7 +15,7 @@ pub struct SystemPrompt<S> {
1515
environment: Environment,
1616
agent: Agent,
1717
tool_definitions: Vec<ToolDefinition>,
18-
files: Vec<String>,
18+
files: Vec<File>,
1919
models: Vec<Model>,
2020
custom_instructions: Vec<String>,
2121
}
@@ -41,8 +41,7 @@ impl<S: SkillFetchService> SystemPrompt<S> {
4141
let agent = &self.agent;
4242
let context = if let Some(system_prompt) = &agent.system_prompt {
4343
let env = self.environment.clone();
44-
let mut files = self.files.clone();
45-
files.sort();
44+
let files = self.files.clone();
4645

4746
let tool_supported = self.is_tool_supported()?;
4847
let supports_parallel_tool_calls = self.is_parallel_tool_call_supported();

crates/forge_domain/src/agent.rs

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,6 @@ pub struct Agent {
4545
/// Maximum number of turns the agent can take
4646
pub max_turns: Option<u64>,
4747

48-
/// Maximum depth to which the file walker should traverse for this agent
49-
pub max_walker_depth: Option<usize>,
50-
5148
/// Configuration for automatic context compaction
5249
pub compact: Option<Compact>,
5350

@@ -90,7 +87,6 @@ impl Agent {
9087
user_prompt: Default::default(),
9188
tools: Default::default(),
9289
max_turns: Default::default(),
93-
max_walker_depth: Default::default(),
9490
compact: Default::default(),
9591
custom_rules: Default::default(),
9692
temperature: Default::default(),
@@ -138,10 +134,6 @@ impl Agent {
138134
}
139135
}
140136

141-
if let Some(max_walker_depth) = workflow.max_walker_depth {
142-
agent.max_walker_depth = Some(max_walker_depth);
143-
}
144-
145137
if let Some(temperature) = workflow.temperature {
146138
agent.temperature = Some(temperature);
147139
}
@@ -231,7 +223,6 @@ impl Agent {
231223
reasoning: def.reasoning,
232224
compact: def.compact,
233225
max_turns: def.max_turns,
234-
max_walker_depth: def.max_walker_depth,
235226
custom_rules: def.custom_rules,
236227
max_tool_failure_per_turn: def.max_tool_failure_per_turn,
237228
max_requests_per_turn: def.max_requests_per_turn,

crates/forge_domain/src/agent_definition.rs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,6 @@ pub struct AgentDefinition {
104104
#[merge(strategy = crate::merge::option)]
105105
pub max_turns: Option<u64>,
106106

107-
/// Maximum depth to which the file walker should traverse for this agent
108-
/// If not provided, the maximum possible depth will be used
109-
#[serde(default, skip_serializing_if = "Option::is_none")]
110-
#[merge(strategy = crate::merge::option)]
111-
pub max_walker_depth: Option<usize>,
112-
113107
/// Configuration for automatic context compaction
114108
#[serde(default, skip_serializing_if = "Option::is_none")]
115109
#[merge(strategy = crate::merge::option)]
@@ -249,7 +243,6 @@ impl AgentDefinition {
249243
user_prompt: Default::default(),
250244
tools: Default::default(),
251245
max_turns: Default::default(),
252-
max_walker_depth: Default::default(),
253246
compact: Default::default(),
254247
custom_rules: Default::default(),
255248
temperature: Default::default(),

crates/forge_domain/src/file.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
use serde::Serialize;
1+
use serde::{Deserialize, Serialize};
22

3-
#[derive(Serialize)]
3+
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
44
pub struct File {
55
pub path: String,
66
pub is_dir: bool,

0 commit comments

Comments
 (0)