refactor: improve code quality and readability

- Reduce nested code in flow.ex, runtime.ex, primitives.ex, and tools.ex
- Convert explicit try blocks to implicit try patterns
- Remove trailing whitespace throughout codebase
- Extract helper functions to improve readability and reduce complexity
- Fix all Credo strict mode warnings

Author: madawei2699

.github/workflows/ci.yml

@@ -46,6 +46,9 @@ jobs:
       - name: Check Formatting
         run: mix format --check-formatted
 
+      - name: Code Quality Check
+        run: mix credo --strict
+
       - name: Compile (with warnings as errors)
         run: mix compile --warnings-as-errors
 

lib/agent_forge.ex

@@ -25,7 +25,7 @@ defmodule AgentForge do
       # result.data will be "Got: Hello"
   """
 
-  alias AgentForge.{Signal, Runtime}
+  alias AgentForge.{Runtime, Signal}
 
   @doc """
   Creates a new flow with the given handlers and options.

lib/agent_forge/config.ex

@@ -4,7 +4,7 @@ defmodule AgentForge.Config do
   Supports loading workflows from YAML or JSON.
   """
 
-  alias AgentForge.{Flow, Primitives, Tools, Signal, Runtime}
+  alias AgentForge.{Flow, Primitives, Runtime, Signal, Tools}
 
   @doc """
   Loads a workflow from a YAML or JSON string.
@@ -30,17 +30,11 @@ defmodule AgentForge.Config do
       true
   """
   def load_from_string(content) when is_binary(content) do
-    case parse_config(content) do
-      {:ok, config} ->
-        case build_flow(config) do
-          {:ok, handlers} ->
-            # Return an executable flow function
-            Runtime.configure(handlers)
-
-          {:error, reason} ->
-            fn _signal -> {:error, reason} end
-        end
-
+    with {:ok, config} <- parse_config(content),
+         {:ok, handlers} <- build_flow(config) do
+      # Return an executable flow function
+      Runtime.configure(handlers)
+    else
       {:error, reason} ->
         fn _signal -> {:error, reason} end
     end
@@ -69,14 +63,12 @@ defmodule AgentForge.Config do
   # Private functions
 
   defp parse_config(content) do
-    cond do
-      String.starts_with?(String.trim(content), "{") ->
-        # Looks like JSON
-        parse_json(content)
-
-      true ->
-        # Assume YAML
-        parse_yaml(content)
+    if String.starts_with?(String.trim(content), "{") do
+      # Looks like JSON
+      parse_json(content)
+    else
+      # Assume YAML
+      parse_yaml(content)
     end
   end
 
@@ -152,7 +144,7 @@ defmodule AgentForge.Config do
               {result, _} = Code.eval_string(custom, data: data)
               result
             rescue
-              e -> raise "Transform evaluation error: #{Exception.message(e)}"
+              e -> reraise "Transform evaluation error: #{Exception.message(e)}", __STACKTRACE__
             end
           end
 

lib/agent_forge/execution_stats.ex

@@ -126,8 +126,6 @@ defmodule AgentForge.ExecutionStats do
   defp get_state_size(_), do: 0
 
   defp format_counters(counters) do
-    counters
-    |> Enum.map(fn {key, count} -> "#{key}: #{count}" end)
-    |> Enum.join(", ")
+    Enum.map_join(counters, ", ", fn {key, count} -> "#{key}: #{count}" end)
   end
 end

lib/agent_forge/flow.ex

@@ -16,23 +16,21 @@ defmodule AgentForge.Flow do
   @type flow :: (Signal.t(), map() -> {term(), map()}) | [(Signal.t(), map() -> {term(), map()})]
 
   def process(handlers, signal, state) when is_list(handlers) do
-    try do
-      # Call process_with_limits with default option to not return statistics
-      # This ensures backward compatibility with existing code
-      case process_with_limits(handlers, signal, state, return_stats: false) do
-        {:ok, result, new_state} ->
-          {:ok, result, new_state}
-
-        {:error, reason, _state} ->
-          # Maintain original error format for backward compatibility
-          {:error, reason}
-
-        other ->
-          other
-      end
-    catch
-      _kind, error -> {:error, "Flow processing error: #{inspect(error)}"}
+    # Call process_with_limits with default option to not return statistics
+    # This ensures backward compatibility with existing code
+    case process_with_limits(handlers, signal, state, return_stats: false) do
+      {:ok, result, new_state} ->
+        {:ok, result, new_state}
+
+      {:error, reason, _state} ->
+        # Maintain original error format for backward compatibility
+        {:error, reason}
+
+      other ->
+        other
     end
+  rescue
+    error -> {:error, "Flow processing error: #{inspect(error)}"}
   end
 
   def get_last_execution_stats, do: Process.get(@last_execution_stats_key)
@@ -69,27 +67,15 @@ defmodule AgentForge.Flow do
   """
   def process_with_limits(handlers, signal, state, opts \\ []) when is_list(handlers) do
     # Extract options
-    timeout_ms = Keyword.get(opts, :timeout_ms, 30000)
+    timeout_ms = Keyword.get(opts, :timeout_ms, 30_000)
     collect_stats = Keyword.get(opts, :collect_stats, true)
     return_stats = Keyword.get(opts, :return_stats, false)
 
     # Initialize statistics if enabled
     stats = if collect_stats, do: ExecutionStats.new(), else: nil
 
     # Create a task to process the signal with timeout
-    # Use try-catch to wrap processing logic to ensure exceptions are properly caught
-    task =
-      Task.async(fn ->
-        try do
-          process_with_stats(handlers, signal, state, stats)
-        catch
-          # Explicitly catch exceptions and convert them to appropriate error results
-          _kind, error ->
-            error_message = "Flow processing error: #{inspect(error)}"
-            error_result = {:error, error_message, state, stats}
-            {error_result, stats}
-        end
-      end)
+    task = Task.async(fn -> execute_flow_safely(handlers, signal, state, stats) end)
 
     # Wait for the task to complete or timeout
     case Task.yield(task, timeout_ms) || Task.shutdown(task) do
@@ -120,33 +106,12 @@ defmodule AgentForge.Flow do
         updated_stats =
           ExecutionStats.record_step(current_stats, handler, current_signal, current_state)
 
-        # Process handler
-        case process_handler(handler, current_signal, current_state) do
-          {{:emit, new_signal}, new_state} ->
-            {:cont, {:ok, new_signal, new_state, updated_stats}}
-
-          {{:emit_many, signals}, new_state} when is_list(signals) ->
-            # When multiple signals are emitted, use the last one for continuation
-            {:cont, {:ok, List.last(signals), new_state, updated_stats}}
-
-          {:skip, new_state} ->
-            {:halt, {:ok, nil, new_state, updated_stats}}
-
-          {:halt, data} ->
-            {:halt, {:ok, data, current_state, updated_stats}}
-
-          {{:halt, data}, _state} ->
-            {:halt, {:ok, data, current_state, updated_stats}}
-
-          {{:error, reason}, new_state} ->
-            {:halt, {:error, reason, new_state, updated_stats}}
-
-          {other, _} ->
-            raise "Invalid handler result: #{inspect(other)}"
-
-          other ->
-            raise "Invalid handler result: #{inspect(other)}"
-        end
+        # Process handler and handle result
+        handle_process_result(
+          process_handler(handler, current_signal, current_state),
+          current_state,
+          updated_stats
+        )
       end)
 
     # Extract stats from result
@@ -307,4 +272,49 @@ defmodule AgentForge.Flow do
       {:skip, Map.put(state, key, signal.data)}
     end
   end
+
+  # Safely executes a flow with exception handling
+  defp execute_flow_safely(handlers, signal, state, stats) do
+    process_with_stats(handlers, signal, state, stats)
+  rescue
+    error ->
+      error_message = "Flow processing error: #{inspect(error)}"
+      error_result = {:error, error_message, state, stats}
+      {error_result, stats}
+  catch
+    _kind, error ->
+      error_message = "Flow processing error: #{inspect(error)}"
+      error_result = {:error, error_message, state, stats}
+      {error_result, stats}
+  end
+
+  # Helper function to handle results from process_handler
+  defp handle_process_result(result, current_state, updated_stats) do
+    case result do
+      {{:emit, new_signal}, new_state} ->
+        {:cont, {:ok, new_signal, new_state, updated_stats}}
+
+      {{:emit_many, signals}, new_state} when is_list(signals) ->
+        # When multiple signals are emitted, use the last one for continuation
+        {:cont, {:ok, List.last(signals), new_state, updated_stats}}
+
+      {:skip, new_state} ->
+        {:halt, {:ok, nil, new_state, updated_stats}}
+
+      {:halt, data} ->
+        {:halt, {:ok, data, current_state, updated_stats}}
+
+      {{:halt, data}, _state} ->
+        {:halt, {:ok, data, current_state, updated_stats}}
+
+      {{:error, reason}, new_state} ->
+        {:halt, {:error, reason, new_state, updated_stats}}
+
+      {other, _} ->
+        raise "Invalid handler result: #{inspect(other)}"
+
+      other ->
+        raise "Invalid handler result: #{inspect(other)}"
+    end
+  end
 end

lib/agent_forge/plugins/http.ex

@@ -43,9 +43,7 @@ defmodule AgentForge.Plugins.HTTP do
   # Tool implementations
 
   defp http_get(params) do
-    if not Code.ensure_loaded?(Finch) do
-      %{error: :finch_not_installed, message: "Finch dependency is required for HTTP operations"}
-    else
+    if Code.ensure_loaded?(Finch) do
       url = Map.fetch!(params, "url")
       headers = Map.get(params, "headers", %{})
 
@@ -62,13 +60,13 @@ defmodule AgentForge.Plugins.HTTP do
         {:error, reason} ->
           %{error: reason}
       end
+    else
+      %{error: :finch_not_installed, message: "Finch dependency is required for HTTP operations"}
     end
   end
 
   defp http_post(params) do
-    if not Code.ensure_loaded?(Finch) do
-      %{error: :finch_not_installed, message: "Finch dependency is required for HTTP operations"}
-    else
+    if Code.ensure_loaded?(Finch) do
       url = Map.fetch!(params, "url")
       headers = Map.get(params, "headers", %{})
       body = Map.get(params, "body", "")
@@ -86,6 +84,8 @@ defmodule AgentForge.Plugins.HTTP do
         {:error, reason} ->
           %{error: reason}
       end
+    else
+      %{error: :finch_not_installed, message: "Finch dependency is required for HTTP operations"}
     end
   end
 

lib/agent_forge/primitives.ex

@@ -29,17 +29,16 @@ defmodule AgentForge.Primitives do
   """
   def branch(condition, then_flow, else_flow) when is_function(condition, 2) do
     fn signal, state ->
-      if condition.(signal, state) do
-        case Flow.process(then_flow, signal, state) do
-          {:ok, result, new_state} -> {{:emit, result}, new_state}
-          {:error, reason} -> {{:emit, Signal.new(:error, reason)}, state}
-        end
-      else
-        case Flow.process(else_flow, signal, state) do
-          {:ok, result, new_state} -> {{:emit, result}, new_state}
-          {:error, reason} -> {{:emit, Signal.new(:error, reason)}, state}
-        end
-      end
+      flow_to_use = if condition.(signal, state), do: then_flow, else: else_flow
+      process_branch_flow(flow_to_use, signal, state)
+    end
+  end
+
+  # Helper function to process the selected branch flow
+  defp process_branch_flow(flow, signal, state) do
+    case Flow.process(flow, signal, state) do
+      {:ok, result, new_state} -> {{:emit, result}, new_state}
+      {:error, reason} -> {{:emit, Signal.new(:error, reason)}, state}
     end
   end
 
@@ -86,9 +85,7 @@ defmodule AgentForge.Primitives do
     fn signal, state ->
       items = signal.data
 
-      if not is_list(items) do
-        {{:emit, Signal.new(:error, "Loop data must be a list")}, state}
-      else
+      if is_list(items) do
         try do
           {results, final_state} =
             Enum.reduce(items, {[], state}, fn item, {results, current_state} ->
@@ -104,8 +101,10 @@ defmodule AgentForge.Primitives do
           {{:emit_many, Enum.reverse(results)}, final_state}
         catch
           {:halt, result, new_state} -> {{:halt, result}, new_state}
-          {:error, reason} -> {{:emit, Signal.new(:error, reason)}, state}
+          {:error, reason} -> {{:error, reason}, state}
         end
+      else
+        {{:emit, Signal.new(:error, "Loop data must be a list")}, state}
       end
     end
   end
@@ -207,24 +206,29 @@ defmodule AgentForge.Primitives do
 
       # Process each channel
       Enum.each(channels, fn channel_name ->
-        case AgentForge.Notification.Registry.get_channel(channel_name) do
-          {:ok, channel_module} ->
-            # Get channel-specific config
-            channel_config = Map.get(config, channel_name, %{})
-            # Send notification
-            channel_module.send(message, channel_config)
-
-          :error when channel_name == :console ->
-            # Built-in console support for backward compatibility
-            IO.puts("[Notification] #{message}")
-
-          :error ->
-            # Channel not found, log warning
-            IO.warn("Notification channel not found: #{inspect(channel_name)}")
-        end
+        send_notification(channel_name, message, config)
       end)
 
       {{:emit, Signal.new(:notification, message)}, state}
     end
   end
+
+  # Helper function to send a notification to a specific channel
+  defp send_notification(channel_name, message, config) do
+    case AgentForge.Notification.Registry.get_channel(channel_name) do
+      {:ok, channel_module} ->
+        # Get channel-specific config
+        channel_config = Map.get(config, channel_name, %{})
+        # Send notification
+        channel_module.send(message, channel_config)
+
+      :error when channel_name == :console ->
+        # Built-in console support for backward compatibility
+        IO.puts("[Notification] #{message}")
+
+      :error ->
+        # Channel not found, log warning
+        IO.warn("Notification channel not found: #{inspect(channel_name)}")
+    end
+  end
 end