improve error handling

[t0mdavid-m]

Summary by CodeRabbit

• Bug Fixes
  • Improved workflow execution tracking with explicit success/failure status reporting throughout the execution pipeline
  • Enhanced error messages with clearer command failure indicators
  • Simplified workflow completion detection logic for more reliable user feedback

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The pull request updates the command execution chain to return boolean success status instead of void. CommandExecutor methods (run_command, run_topp, run_multiple_commands) now return bool with threading-based parallel execution. WorkflowManager.execution returns bool, logging "WORKFLOW FINISHED" only on success. StreamlitUI simplifies completion detection to report success or error based on return status.

Changes

Cohort / File(s)	Summary
CommandExecutor methods src/workflow/CommandExecutor.py	run_command(), run_topp(), and run_multiple_commands() return type changed from None to bool. Added threading with shared results list and lock for parallel command execution. Error logging standardized to include exit code and command identifier. run_multiple_commands() returns True only if all commands succeed.
WorkflowManager execution src/workflow/WorkflowManager.py	execution() method return type changed from None to bool. Returns True on successful execution. workflow_process() updated to branch on execution result and logs "WORKFLOW FINISHED" only when execution succeeds.
StreamlitUI completion handling src/workflow/StreamlitUI.py	Modified execution_section logic to report success when "WORKFLOW FINISHED" is detected in logs; reports error otherwise. Replaced multi-branch warning logic with unified error state when completion is not detected.

Poem

🐰 Commands now speak in boolean tongue,
Success and failure when their work is done!
Through threading's dance, they run in flight,
Then whisper up the chain—all's well or not quite.
hop hop ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'improve error handling' is vague and generic, using a non-descriptive term that doesn't convey the specific technical changes made to the codebase.	Consider a more specific title such as 'Add boolean return types to command execution methods' or 'Return execution status from command and workflow methods' to better describe the primary change.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/workflow/WorkflowManager.py`:
- Around line 104-106: The subclass Workflow.execution() currently declares ->
None but should conform to the base WorkflowManager.execution() signature and
return a bool; change Workflow.execution() to return True on success and False
on failure (update any early exits/exception handling in Workflow.execution to
return False or re-raise appropriately), ensuring callers like the check using
"success = self.execution()" and "if success:" work correctly; reference the
Workflow.execution method and the WorkflowManager.execution expectation when
making the change.

🧹 Nitpick comments (1)

src/workflow/CommandExecutor.py (1)

175-175: Avoid mutable default argument.

Using dict = {} as a default argument can lead to unexpected behavior if the dict is ever mutated, since the same object is shared across calls.

Suggested fix

-    def run_topp(self, tool: str, input_output: dict, custom_params: dict = {}) -> bool:
+    def run_topp(self, tool: str, input_output: dict, custom_params: dict = None) -> bool:

Then at the start of the method body:

if custom_params is None:
    custom_params = {}

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find all execution() method implementations
ast-grep --pattern $'def execution(self)$_:
    $$$'

Repository: OpenMS/streamlit-template

Length of output: 51

---

🏁 Script executed:

#!/bin/bash
# Search for execution method definitions using ripgrep
rg "def execution" -A 10

Repository: OpenMS/streamlit-template

Length of output: 2772

---

🏁 Script executed:

#!/bin/bash
# Check the class definitions and inheritance
rg "class (WorkflowManager|Workflow)" -B 1 -A 5 src/

Repository: OpenMS/streamlit-template

Length of output: 1067

---

Fix return type in Workflow.execution().

The subclass Workflow.execution() is declared with return type -> None, but the base class WorkflowManager.execution() expects -> bool. This causes the check at lines 104-106 (if success:) to fail when a Workflow instance is used, since Python implicitly returns None. Update Workflow.execution() to return True on success and False on failure to match the base class contract.

🤖 Prompt for AI Agents

In `@src/workflow/WorkflowManager.py` around lines 104 - 106, The subclass
Workflow.execution() currently declares -> None but should conform to the base
WorkflowManager.execution() signature and return a bool; change
Workflow.execution() to return True on success and False on failure (update any
early exits/exception handling in Workflow.execution to return False or re-raise
appropriately), ensuring callers like the check using "success =
self.execution()" and "if success:" work correctly; reference the
Workflow.execution method and the WorkflowManager.execution expectation when
making the change.