improve workflow and simplify tool call

rozgo · rozgo · commit 4db8f5e89c71 · 2025-05-26T23:14:57.000-06:00
diff --git a/src/developer/descriptions/workflow.md b/src/developer/descriptions/workflow.md
@@ -1,40 +1,71 @@
-Manages multi-step problem-solving processes with support for sequential progression, branching paths, and step revisions.
+# Workflow Tool: Guiding Complex Problem-Solving
 
-Use this tool to:
-- Break down complex problems into sequential steps
-- Create alternative solution paths through branching
-- Revise previous steps as your understanding evolves
-- Maintain context across multi-step reasoning processes
+Manages multi-step problem-solving processes with support for sequential progression, branching paths, and step revisions. This tool is designed to help you (the LLM) structure your reasoning, explore alternatives, and adapt your approach as your understanding of a problem evolves.
 
-Returns JSON response with workflow status.
+Returns a JSON response detailing the current workflow status, including the last step taken, active branches, and overall progress.
+
+## When to Use This Workflow Tool
+
+This tool is most effective when you need to:
+
+* **Deconstruct Complex Problems**: Break down large or multifaceted tasks into a manageable sequence of discrete steps.
+* **Plan and Design Iteratively**: Develop plans or designs where initial assumptions might need revision as more information is processed.
+* **Explore Multiple Solution Paths**: Investigate different approaches or hypotheses by creating distinct branches in your reasoning process.
+* **Perform In-depth Analysis with Course Correction**: Conduct detailed analysis that may require backtracking or revising earlier conclusions.
+* **Handle Problems with Unclear Scope**: Tackle tasks where the full extent of steps or requirements isn't known at the outset.
+* **Maintain Long-Term Context**: Keep track of progress and decisions across a series of interactions for a multi-step task.
+* **Systematically Generate and Verify Hypotheses**: Formulate a potential solution or explanation, then use subsequent steps to test and validate it.
+* **Document Your Reasoning Process**: Clearly lay out each step of your thought process for transparency and review.
+
+## Best Practices for Using the Workflow Tool
+
+To leverage this tool most effectively, please follow these guidelines:
+
+1.  **Start with an Initial Estimate, Adapt as Needed**: Begin with a reasonable estimate for `total_steps`. However, feel free to adjust this value (up or down) in subsequent steps as your understanding of the task's complexity changes. The tool will accommodate these adjustments.
+2.  **Embrace Revisions**: Don't hesitate to use `is_step_revision` and `revises_step` if you realize an earlier step needs correction or refinement. Clearly describe what is being revised and why.
+3.  **Utilize Branching for Alternatives**: If multiple approaches seem plausible, use `branch_from_step` and `branch_id` to explore them in parallel without losing the main line of thought. Give your branches descriptive `branch_id`s.
+4.  **Be Explicit About Uncertainty**: If you are unsure about a step or a direction, you can note this in the `step_description`.
+5.  **Clearly Define Each Step**: Ensure each `step_description` is detailed and clearly states what that specific step accomplishes, any conclusions drawn, or any questions that arise.
+6.  **Manage `next_step_needed` and `needs_more_steps` Deliberately**:
+    * Set `next_step_needed` to `false` only when the *current specific sequence or sub-task* is complete.
+    * Use `needs_more_steps` (optional) to indicate if the *overall problem or workflow* requires further steps, even if the current local sequence (`next_step_needed: false`) is ending. For example, you might finish a branch (`next_step_needed: false`) but still need to return to the main workflow or start a new one (`needs_more_steps: true`). Set `needs_more_steps: false` when you are confident the entire problem is resolved.
+7.  **Hypothesis Management**:
+    * When appropriate, use a step to explicitly state a hypothesis in the `step_description`.
+    * In subsequent steps, describe how you are testing or verifying this hypothesis.
+8.  **Sequential Integrity**: Ensure `step_number` increments logically. While you can revise past steps, new steps should generally follow in sequence.
+9.  **Final Answer**: When the entire workflow is complete (`next_step_needed: false` and `needs_more_steps: false` (or omitted and implied false)), the final step's description should ideally contain or clearly lead to the final answer or solution.
 
 ## Parameters
 
 ### Required
-- `step_description`: Detailed description of what this step accomplishes
-- `step_number`: Current position in the workflow sequence (e.g., 1 for first step)
-- `total_steps`: Estimated total number of steps in the complete workflow
-- `next_step_needed`: Set to true if another step will follow this one, false if this is the final step
+-   `step_description` (string): Detailed description of what this step accomplishes, including any reasoning, conclusions, or questions.
+-   `step_number` (integer): Current position in the workflow sequence (e.g., 1 for first step). Must be >= 1.
+-   `total_steps` (integer): Your current best estimate of the total number of steps needed for the *entire* workflow. Can be adjusted in subsequent steps. Must be >= `step_number`.
+-   `next_step_needed` (boolean): Set to `true` if another step is expected to *immediately follow this one* in the current sequence or branch. Set to `false` if this is the last step of the current sequence/branch.
 
 ### Optional
-- `is_step_revision`: Set to true if this step revises a previous step
-- `revises_step`: If revising a previous step, specify which step number is being revised
-- `branch_from_step`: If creating a branch, specify which step number this branch starts from
-- `branch_id`: A unique identifier for this branch (required when creating a branch)
-- `needs_more_steps`: Indicates whether additional steps are required to complete the workflow
-
-## Features
+-   `is_step_revision` (boolean): Set to `true` if this step revises or corrects a previous step.
+-   `revises_step` (integer): If `is_step_revision` is true, specify the `step_number` of the step being revised. Must be a valid, existing step number.
+-   `branch_from_step` (integer): If creating a new branch, specify the `step_number` from which this new branch originates. Must be a valid, existing step number.
+-   `branch_id` (string): A unique and descriptive identifier for the branch being created or continued (e.g., "explore-alternative-api", "deep-dive-cause-analysis"). Required if `branch_from_step` is provided for a new branch, or if continuing an existing branch.
+-   `needs_more_steps` (boolean): Set to `true` if you anticipate more steps are needed to fully resolve the *overall problem*, even if `next_step_needed` is `false` for the current step (e.g., you've completed a branch but need to return to the main flow or analyze another aspect). If omitted, the system may infer based on `next_step_needed` and `total_steps`. Set explicitly to `false` when the entire problem is solved.
 
-- **Sequential Progression**: Track steps in order with automatic numbering
-- **Branching**: Create alternative solution paths from any step
-- **Step Revision**: Update and improve previous steps as understanding evolves
+## Features Summary
 
-- **Context Preservation**: Maintain workflow state across multiple interactions
-- **Validation**: Automatic validation of step relationships and branching logic
+-   **Sequential Progression**: Steps are tracked in order.
+-   **Dynamic `total_steps`**: The `total_steps` can be adjusted by you as the workflow progresses.
+-   **Branching**: Create and switch between alternative solution paths.
+-   **Step Revision**: Mark steps that update or correct prior steps.
+-   **Context Preservation**: Workflow state (history, branches) is maintained across calls.
+-   **Input Validation**: Ensures logical consistency of parameters.
 
 ## Output
 
-Returns a JSON response containing:
-- Current step information and status
-- Branch information and active branch status
-- Step count and completion status 
+Returns a JSON object detailing the current workflow status:
+-   `step_number` (integer): The number of the step just processed.
+-   `total_steps` (integer): The current total number of steps, possibly updated by the last call.
+-   `next_step_needed` (boolean): The `next_step_needed` value from the last step.
+-   `last_step_description` (string): The description of the step just processed.
+-   `current_branch` (string | null): The ID of the active branch, if any.
+-   `branches` (array of strings): A list of all unique `branch_id`s created so far.
+-   `step_history_length` (integer): The total number of steps recorded in the main history (includes steps from all branches). 
diff --git a/src/developer/mod.rs b/src/developer/mod.rs
@@ -91,7 +91,7 @@ impl Developer {
             shell: Shell::new().with_ignore_patterns(ignore_patterns),
             screen_capture: ScreenCapture::new(),
             image_processor: ImageProcessor::new(),
-            workflow: Workflow::default(),
+            workflow: Workflow::new(true, None, true),
         }
     }
 
@@ -256,32 +256,44 @@ impl Developer {
         #[schemars(description = "Detailed description of what this step accomplishes")]
         step_description: String,
         #[tool(param)]
-        #[schemars(description = "Current position in the workflow sequence (e.g., 1 for first step)")]
+        #[schemars(
+            description = "Current position in the workflow sequence (e.g., 1 for first step)"
+        )]
         step_number: i32,
         #[tool(param)]
         #[schemars(description = "Estimated total number of steps in the complete workflow")]
         total_steps: i32,
         #[tool(param)]
-        #[schemars(description = "Set to true if another step will follow this one, false if this is the final step")]
+        #[schemars(
+            description = "Set to true if another step will follow this one, false if this is the final step"
+        )]
         next_step_needed: bool,
         #[tool(param)]
         #[schemars(description = "Set to true if this step revises a previous step")]
         is_step_revision: Option<bool>,
         #[tool(param)]
-        #[schemars(description = "If revising a previous step, specify which step number is being revised")]
+        #[schemars(
+            description = "If revising a previous step, specify which step number is being revised"
+        )]
         revises_step: Option<i32>,
         #[tool(param)]
-        #[schemars(description = "If creating a branch, specify which step number this branch starts from")]
+        #[schemars(
+            description = "If creating a branch, specify which step number this branch starts from"
+        )]
         branch_from_step: Option<i32>,
         #[tool(param)]
-        #[schemars(description = "A unique identifier for this branch (required when creating a branch)")]
+        #[schemars(
+            description = "A unique identifier for this branch (required when creating a branch)"
+        )]
         branch_id: Option<String>,
         #[tool(param)]
-        #[schemars(description = "Indicates whether additional steps are required to complete the workflow")]
+        #[schemars(
+            description = "Indicates whether additional steps are required to complete the workflow"
+        )]
         needs_more_steps: Option<bool>,
     ) -> Result<CallToolResult, McpError> {
         use workflow::WorkflowStep;
-        
+
         let step = WorkflowStep {
             step_description,
             step_number,
@@ -296,8 +308,6 @@ impl Developer {
 
         self.workflow.execute_step(step).await
     }
-
-
 }
 
 #[tool(tool_box)]
diff --git a/src/developer/workflow.rs b/src/developer/workflow.rs
@@ -40,6 +40,7 @@ pub struct Workflow {
     state: Arc<Mutex<WorkflowState>>,
     allow_branches: bool,
     max_steps: Option<i32>,
+    log_steps: bool,
 }
 
 impl Default for Workflow {
@@ -48,62 +49,101 @@ impl Default for Workflow {
             state: Arc::new(Mutex::new(WorkflowState::default())),
             allow_branches: true,
             max_steps: None,
+            log_steps: true,
         }
     }
 }
 
 impl Workflow {
-    pub fn new(allow_branches: bool, max_steps: Option<i32>) -> Self {
+    pub fn new(allow_branches: bool, max_steps: Option<i32>, log_steps: bool) -> Self {
         Self {
             state: Arc::new(Mutex::new(WorkflowState::default())),
             allow_branches,
             max_steps,
+            log_steps,
         }
     }
 
     pub async fn execute_step(&self, args: WorkflowStep) -> Result<CallToolResult, McpError> {
+        // Optional: Log the received arguments at the beginning
+        if self.log_steps {
+            tracing::debug!(workflow_step_args = ?args, "Workflow step arguments received");
+        }
+
         if let Some(max) = self.max_steps {
             if args.step_number > max {
-                return Ok(Self::error(format!(
+                let error_msg = format!(
                     "Step number {} exceeds configured maximum of {}",
                     args.step_number, max
-                )));
+                );
+                if self.log_steps {
+                    tracing::warn!(error_msg, "Workflow step validation error");
+                }
+                return Ok(Self::error(error_msg));
             }
         }
 
         let mut state = self.state.lock().await;
 
         let mut step_data = args.clone();
         if step_data.step_number > step_data.total_steps {
+            // Log this adjustment if desired
+            if self.log_steps {
+                tracing::info!(
+                    old_total_steps = step_data.total_steps,
+                    new_total_steps = step_data.step_number,
+                    step_number = step_data.step_number,
+                    "Adjusting total_steps to match current step_number as it was greater."
+                );
+            }
             step_data.total_steps = step_data.step_number;
         }
 
         if step_data.revises_step.is_some() && step_data.is_step_revision.is_none() {
-            return Ok(Self::error(
-                "When specifying revises_step, is_step_revision must be set to true",
-            ));
+            let error_msg = "When specifying revises_step, is_step_revision must be set to true";
+            if self.log_steps {
+                tracing::warn!(error_msg, invalid_revision_args = ?step_data, "Workflow step validation error");
+            }
+            return Ok(Self::error(error_msg));
         }
 
         if step_data.branch_id.is_some() && step_data.branch_from_step.is_none() {
-            return Ok(Self::error(
-                "When creating a branch (branch_id), you must specify branch_from_step",
-            ));
+            let error_msg = "When creating a branch (branch_id), you must specify branch_from_step";
+            if self.log_steps {
+                tracing::warn!(error_msg, invalid_branch_args = ?step_data, "Workflow step validation error");
+            }
+            return Ok(Self::error(error_msg));
         }
 
         if let (Some(branch_id), Some(branch_from_step)) =
             (&step_data.branch_id, &step_data.branch_from_step)
         {
             if !self.allow_branches {
-                return Ok(Self::error(
-                    "Branching is disabled in current configuration",
-                ));
+                let error_msg = "Branching is disabled in current configuration";
+                if self.log_steps {
+                    tracing::warn!(error_msg, "Workflow branching validation error");
+                }
+                return Ok(Self::error(error_msg));
             }
 
             if *branch_from_step <= 0 || *branch_from_step > state.step_history.len() as i32 {
-                return Ok(Self::error(format!(
+                let error_msg = format!(
                     "branch_from_step {} does not exist in step history",
                     branch_from_step
-                )));
+                );
+                if self.log_steps {
+                    tracing::warn!(error_msg, "Workflow branching validation error");
+                }
+                return Ok(Self::error(error_msg));
+            }
+
+            if self.log_steps {
+                tracing::info!(
+                    branch_id,
+                    branch_from_step,
+                    step_number = step_data.step_number,
+                    "Processing branch step."
+                );
             }
 
             state.current_branch = Some(branch_id.clone());
@@ -113,16 +153,44 @@ impl Workflow {
                 .or_default()
                 .push(step_data.clone());
         } else if state.current_branch.is_some() && step_data.branch_id.is_none() {
+            if self.log_steps {
+                tracing::info!(
+                    previous_branch = ?state.current_branch,
+                    step_number = step_data.step_number,
+                    "Moving from branch back to main history (or default)."
+                );
+            }
             state.current_branch = None;
         }
 
         state.step_history.push(step_data.clone());
 
-        let response = self.build_workflow_status(&state, &step_data).await;
+        // Log before returning success
+        if self.log_steps {
+            tracing::info!(
+                step_number = step_data.step_number,
+                total_steps = step_data.total_steps,
+                description = step_data.step_description.as_str(),
+                is_revision = step_data.is_step_revision,
+                revises_step = step_data.revises_step,
+                branch_id = step_data.branch_id.as_deref(),
+                next_step_needed = step_data.next_step_needed,
+                needs_more_steps = step_data.needs_more_steps,
+                "Workflow step processed successfully."
+            );
+        }
 
-        match serde_json::to_string_pretty(&response) {
+        let response_status = self.build_workflow_status(&state, &step_data).await;
+
+        match serde_json::to_string_pretty(&response_status) {
             Ok(json_response) => Ok(Self::success(json_response)),
-            Err(e) => Ok(Self::error(format!("Failed to serialize response: {}", e))),
+            Err(e) => {
+                // Also log serialization errors
+                if self.log_steps {
+                    tracing::error!(error = %e, "Failed to serialize workflow status response");
+                }
+                Ok(Self::error(format!("Failed to serialize response: {}", e)))
+            }
         }
     }
 
@@ -235,18 +303,20 @@ mod tests {
 
     #[test]
     fn test_workflow_creation() {
-        let tool = Workflow::new(true, Some(10));
+        let tool = Workflow::new(true, Some(10), true);
         assert_eq!(tool.allow_branches, true);
         assert_eq!(tool.max_steps, Some(10));
+        assert_eq!(tool.log_steps, true);
 
         let default_tool = Workflow::default();
         assert_eq!(default_tool.allow_branches, true);
         assert_eq!(default_tool.max_steps, None);
+        assert_eq!(default_tool.log_steps, true);
     }
 
     #[tokio::test]
     async fn test_error_conditions() {
-        let tool = Workflow::new(false, Some(2)); // No branching, max 2 steps
+        let tool = Workflow::new(false, Some(2), false); // No branching, max 2 steps, no logging for test
 
         // Test max steps exceeded
         let step = WorkflowStep {
