fix(agent): address review comments for iteration limit fix

Changes:

- Extract tool budget prompt into reusable helper function build_tool_budget_prompt()

- Use dynamic tier ranges based on max_iterations (12%/20%/32%/48%/60%/80%)

- Apply budget awareness to both run() and process_message() code paths

- Update all test assertions from 10 to 25

- Add comprehensive doc comment with rollback guidance in schema.rs

Fixes review comments on PR #27

Author: louisdevzz

src/agent/loop_.rs

@@ -2741,6 +2741,23 @@ pub(crate) fn build_tool_instructions(tools_registry: &[Box<dyn Tool>]) -> Strin
     instructions
 }
 
+/// Build the tool budget awareness section for the system prompt.
+/// This helps the agent plan efficiently based on the maximum allowed iterations.
+pub(crate) fn build_tool_budget_prompt(max_iterations: usize) -> String {
+    // Calculate dynamic tier ranges based on max_iterations
+    let simple_min = std::cmp::max(1, (max_iterations as f32 * 0.12).round() as usize);
+    let simple_max = std::cmp::max(2, (max_iterations as f32 * 0.2).round() as usize);
+    let medium_min = std::cmp::max(3, (max_iterations as f32 * 0.32).round() as usize);
+    let medium_max = std::cmp::max(5, (max_iterations as f32 * 0.48).round() as usize);
+    let complex_min = std::cmp::max(8, (max_iterations as f32 * 0.6).round() as usize);
+    let complex_max = std::cmp::max(12, (max_iterations as f32 * 0.8).round() as usize);
+
+    format!(
+        "\n## Tool Use Budget\n\nYou have a maximum of {} tool-use iterations to complete this task. Plan efficiently:\n\n- **Complex tasks** (build projects, multiple files): ~{}-{} iterations\n- **Medium tasks** (modify existing code): ~{}-{} iterations\n- **Simple tasks** (read files, explain): ~{}-{} iterations\n\nIf approaching the limit, prioritize:\n1. Complete the critical path first (create → build → verify)\n2. Skip verification steps if already confident\n3. Provide partial results with explanation rather than failing\n\n",
+        max_iterations, complex_min, complex_max, medium_min, medium_max, simple_min, simple_max
+    )
+}
+
 // ── CLI Entrypoint ───────────────────────────────────────────────────────
 // Wires up all subsystems (observer, runtime, security, memory, tools,
 // provider, hardware RAG, peripherals) and enters either single-shot or
@@ -3027,10 +3044,7 @@ pub async fn run(
     } else {
         config.agent.max_tool_iterations
     };
-    system_prompt.push_str(&format!(
-        "\n## Tool Use Budget\n\nYou have a maximum of {} tool-use iterations to complete this task. Plan efficiently:\n\n- **Complex tasks** (build projects, multiple files): ~15-20 iterations\n- **Medium tasks** (modify existing code): ~8-12 iterations\n- **Simple tasks** (read files, explain): ~3-5 iterations\n\nIf approaching the limit, prioritize:\n1. Complete the critical path first (create → build → verify)\n2. Skip verification steps if already confident\n3. Provide partial results with explanation rather than failing\n\n",
-        max_iterations
-    ));
+    system_prompt.push_str(&build_tool_budget_prompt(max_iterations));
 
     // ── Approval manager (supervised mode) ───────────────────────
     let approval_manager = if interactive {
@@ -3446,6 +3460,14 @@ pub async fn process_message(config: Config, message: &str) -> Result<String> {
         system_prompt.push_str(&build_tool_instructions(&tools_registry));
     }
 
+    // Add iteration budget awareness to system prompt
+    let max_iterations = if config.agent.max_tool_iterations == 0 {
+        DEFAULT_MAX_TOOL_ITERATIONS
+    } else {
+        config.agent.max_tool_iterations
+    };
+    system_prompt.push_str(&build_tool_budget_prompt(max_iterations));
+
     let mem_context = build_context(mem.as_ref(), message, config.memory.min_relevance_score).await;
     let rag_limit = if config.agent.compact_context { 2 } else { 5 };
     let hw_context = hardware_rag

src/channels/mod.rs

@@ -4110,7 +4110,7 @@ BTC is currently around $65,000 based on latest tool output."#
             model: Arc::new("test-model".to_string()),
             temperature: 0.0,
             auto_save_memory: false,
-            max_tool_iterations: 10,
+            max_tool_iterations: 25,
             min_relevance_score: 0.0,
             conversation_histories: Arc::new(Mutex::new(HashMap::new())),
             provider_cache: Arc::new(Mutex::new(HashMap::new())),
@@ -4169,7 +4169,7 @@ BTC is currently around $65,000 based on latest tool output."#
             model: Arc::new("test-model".to_string()),
             temperature: 0.0,
             auto_save_memory: false,
-            max_tool_iterations: 10,
+            max_tool_iterations: 25,
             min_relevance_score: 0.0,
             conversation_histories: Arc::new(Mutex::new(HashMap::new())),
             provider_cache: Arc::new(Mutex::new(HashMap::new())),
@@ -4242,7 +4242,7 @@ BTC is currently around $65,000 based on latest tool output."#
             model: Arc::new("test-model".to_string()),
             temperature: 0.0,
             auto_save_memory: false,
-            max_tool_iterations: 10,
+            max_tool_iterations: 25,
             min_relevance_score: 0.0,
             conversation_histories: Arc::new(Mutex::new(HashMap::new())),
             provider_cache: Arc::new(Mutex::new(HashMap::new())),
@@ -4301,7 +4301,7 @@ BTC is currently around $65,000 based on latest tool output."#
             model: Arc::new("test-model".to_string()),
             temperature: 0.0,
             auto_save_memory: false,
-            max_tool_iterations: 10,
+            max_tool_iterations: 25,
             min_relevance_score: 0.0,
             conversation_histories: Arc::new(Mutex::new(HashMap::new())),
             provider_cache: Arc::new(Mutex::new(HashMap::new())),
@@ -4917,7 +4917,7 @@ BTC is currently around $65,000 based on latest tool output."#
             model: Arc::new("test-model".to_string()),
             temperature: 0.0,
             auto_save_memory: false,
-            max_tool_iterations: 10,
+            max_tool_iterations: 25,
             min_relevance_score: 0.0,
             conversation_histories: Arc::new(Mutex::new(HashMap::new())),
             provider_cache: Arc::new(Mutex::new(HashMap::new())),
@@ -4997,7 +4997,7 @@ BTC is currently around $65,000 based on latest tool output."#
             model: Arc::new("test-model".to_string()),
             temperature: 0.0,
             auto_save_memory: false,
-            max_tool_iterations: 10,
+            max_tool_iterations: 25,
             min_relevance_score: 0.0,
             conversation_histories: Arc::new(Mutex::new(HashMap::new())),
             provider_cache: Arc::new(Mutex::new(HashMap::new())),
@@ -5089,7 +5089,7 @@ BTC is currently around $65,000 based on latest tool output."#
             model: Arc::new("test-model".to_string()),
             temperature: 0.0,
             auto_save_memory: false,
-            max_tool_iterations: 10,
+            max_tool_iterations: 25,
             min_relevance_score: 0.0,
             conversation_histories: Arc::new(Mutex::new(HashMap::new())),
             provider_cache: Arc::new(Mutex::new(HashMap::new())),
@@ -5163,7 +5163,7 @@ BTC is currently around $65,000 based on latest tool output."#
             model: Arc::new("test-model".to_string()),
             temperature: 0.0,
             auto_save_memory: false,
-            max_tool_iterations: 10,
+            max_tool_iterations: 25,
             min_relevance_score: 0.0,
             conversation_histories: Arc::new(Mutex::new(HashMap::new())),
             provider_cache: Arc::new(Mutex::new(HashMap::new())),
@@ -5222,7 +5222,7 @@ BTC is currently around $65,000 based on latest tool output."#
             model: Arc::new("test-model".to_string()),
             temperature: 0.0,
             auto_save_memory: false,
-            max_tool_iterations: 10,
+            max_tool_iterations: 25,
             min_relevance_score: 0.0,
             conversation_histories: Arc::new(Mutex::new(HashMap::new())),
             provider_cache: Arc::new(Mutex::new(HashMap::new())),

src/config/schema.rs

@@ -544,7 +544,19 @@ pub struct AgentConfig {
     pub compact_context: bool,
     /// Maximum tool-call loop turns per user message. Default: `25`.
     /// Setting to `0` falls back to the safe default of `25`.
-    /// Increased from 10 to support complex multi-step tasks (see issue #26).
+    ///
+    /// ## Compatibility Note
+    /// This change only alters the default value. Existing configurations with
+    /// explicit `max_tool_iterations` values are respected and will continue to work.
+    ///
+    /// ## Rollback / Migration
+    /// To restore the previous behavior (limit of 10 iterations), explicitly set:
+    /// ```toml
+    /// [agent]
+    /// max_tool_iterations = 10
+    /// ```
+    ///
+    /// See issue #26 for context on why the default was increased.
     #[serde(default = "default_agent_max_tool_iterations")]
     pub max_tool_iterations: usize,
     /// Maximum conversation history messages retained per session. Default: `50`.
@@ -5029,7 +5041,7 @@ reasoning_enabled = false
     async fn agent_config_defaults() {
         let cfg = AgentConfig::default();
         assert!(!cfg.compact_context);
-        assert_eq!(cfg.max_tool_iterations, 10);
+        assert_eq!(cfg.max_tool_iterations, 25);
         assert_eq!(cfg.max_history_messages, 50);
         assert!(!cfg.parallel_tools);
         assert_eq!(cfg.tool_dispatcher, "auto");

src/sandbox/mod.rs

@@ -15,7 +15,7 @@ use std::fmt;
 
 /// Package manager types supported by the sandbox, ordered by priority.
 /// Priority: pnpm > yarn > npm
-/// 
+///
 /// Note: Default is Npm as the safe fallback, but detection prioritizes pnpm/yarn.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
 pub enum PackageManager {

src/tools/github_ops.rs

@@ -110,10 +110,21 @@ fn validate_labels(labels: &[String]) -> Result<(), String> {
 fn sanitize_labels(labels: &[String]) -> Vec<String> {
     // Known valid scope labels that don't contain spaces
     const VALID_SCOPE_LABELS: &[&str] = &[
-        "provider", "channel", "tool", "gateway", "memory", "runtime", 
-        "config", "ci", "performance", "ui", "api", "database", "deps",
+        "provider",
+        "channel",
+        "tool",
+        "gateway",
+        "memory",
+        "runtime",
+        "config",
+        "ci",
+        "performance",
+        "ui",
+        "api",
+        "database",
+        "deps",
     ];
-    
+
     labels
         .iter()
         .filter(|label| {

src/tools/mod.rs

@@ -110,7 +110,8 @@ pub use pushover::PushoverTool;
 pub use sandbox::{
     SandboxCreateTool, SandboxGetPackageManagerTool, SandboxGetPreviewUrlTool,
     SandboxGetPublicUrlTool, SandboxKillTool, SandboxListFilesTool, SandboxReadFileTool,
-    SandboxRestoreSnapshotTool, SandboxRunCommandTool, SandboxSaveSnapshotTool, SandboxWriteFileTool,
+    SandboxRestoreSnapshotTool, SandboxRunCommandTool, SandboxSaveSnapshotTool,
+    SandboxWriteFileTool,
 };
 pub use schedule::ScheduleTool;
 #[allow(unused_imports)]

src/tools/sandbox/create.rs

@@ -88,7 +88,10 @@ impl Tool for SandboxCreateTool {
                 let tip = if pm == crate::sandbox::PackageManager::Npm {
                     "💡 Tip: Using npm as package manager. Consider installing pnpm for faster installs.".to_string()
                 } else {
-                    format!("💡 Tip: Use '{install}' for faster installs instead of 'npm install'", install = pm.install_cmd())
+                    format!(
+                        "💡 Tip: Use '{install}' for faster installs instead of 'npm install'",
+                        install = pm.install_cmd()
+                    )
                 };
                 Ok(ToolResult {
                     success: true,

src/tools/sandbox/package_manager.rs

@@ -56,7 +56,7 @@ impl Tool for SandboxGetPackageManagerTool {
 
         // Actively detect package manager instead of using cached state
         let pm = self.client.detect_package_manager().await;
-        
+
         let output = format!(
             "Detected package manager: {pm}\n\
              Install command: {install}\n\

tests/agent_loop_robustness.rs

@@ -315,7 +315,7 @@ async fn agent_handles_mixed_tool_success_and_failure() {
 // TG4.3: Iteration limit enforcement (#777)
 // ═════════════════════════════════════════════════════════════════════════════
 
-/// Agent should not exceed max_tool_iterations (default=10) even with
+/// Agent should not exceed max_tool_iterations (default=25) even with
 /// a provider that keeps returning tool calls
 #[tokio::test]
 async fn agent_respects_max_tool_iterations() {

tests/config_persistence.rs

@@ -48,8 +48,8 @@ fn config_default_temperature_positive() {
 fn agent_config_default_max_tool_iterations() {
     let agent = AgentConfig::default();
     assert_eq!(
-        agent.max_tool_iterations, 10,
-        "default max_tool_iterations should be 10"
+        agent.max_tool_iterations, 25,
+        "default max_tool_iterations should be 25"
     );
 }