refactor: improve code modularity and fix linting issues

madawei2699 · madawei2699 · commit 3b0ce40705ed · 2025-03-30T21:35:25.000+08:00
- Refactor LLMAgent.Flows.conversation to reduce complexity by extracting signal handlers into separate helper functions
- Fix function nesting depth in sequence_flows by introducing process_flow_result helper
- Add proper Logger require statements to all handler functions
- Apply code formatting and fix trailing whitespace issues
- Fix documentation and naming conventions
diff --git a/lib/llm_agent/flows.ex b/lib/llm_agent/flows.ex
@@ -57,136 +57,157 @@ defmodule LLMAgent.Flows do
       require Logger
       Logger.debug("Conversation flow - Processing signal: #{inspect(signal.type)}")
 
-      # Determine appropriate handler based on signal type
-      result =
-        case signal.type do
-          :user_message ->
-            # User message flow path
-            Logger.debug("Conversation flow - Processing user message")
+      # Handle the signal with the appropriate handler based on type
+      result = handle_signal(signal, state)
 
-            case Handlers.message_handler(signal, state) do
-              {{:emit, response_signal}, new_state} ->
-                {:emit, response_signal, new_state}
+      Logger.debug("Conversation flow - Result: #{inspect(result)}")
+      result
+    end
 
-              {:skip, new_state} ->
-                {:skip, new_state}
-
-              other ->
-                Logger.warning("Unexpected message handler result: #{inspect(other)}")
-                {:skip, state}
-            end
+    {flow, initial_state}
+  end
 
-          :thinking ->
-            # Thinking flow path
-            Logger.debug("Conversation flow - Processing thinking signal")
+  # Helper functions for signal handling in conversation flow
+
+  # Dispatches to the appropriate handler based on signal type
+  defp handle_signal(signal, state) do
+    require Logger
+
+    case signal.type do
+      :user_message -> handle_user_message(signal, state)
+      :thinking -> handle_thinking(signal, state)
+      :tool_call -> handle_tool_call(signal, state)
+      :tool_result -> handle_tool_result(signal, state)
+      :task -> handle_task(signal, state)
+      :response -> handle_response(signal, state)
+      :error -> handle_error(signal, state)
+      _ -> handle_unknown_signal_type(signal, state)
+    end
+  end
 
-            case Handlers.thinking_handler(signal, state) do
-              {{:emit, new_signal}, new_state} ->
-                {:emit, new_signal, new_state}
+  defp handle_user_message(signal, state) do
+    require Logger
+    Logger.debug("Conversation flow - Processing user message")
 
-              {:skip, new_state} ->
-                {:skip, new_state}
+    case Handlers.message_handler(signal, state) do
+      {{:emit, response_signal}, new_state} ->
+        {:emit, response_signal, new_state}
 
-              other ->
-                Logger.warning("Unexpected thinking handler result: #{inspect(other)}")
-                {:skip, state}
-            end
+      {:skip, new_state} ->
+        {:skip, new_state}
 
-          :tool_call ->
-            # Tool call flow path
-            Logger.debug("Conversation flow - Processing tool call signal")
+      other ->
+        Logger.warning("Unexpected message handler result: #{inspect(other)}")
+        {:skip, state}
+    end
+  end
 
-            case Handlers.tool_handler(signal, state) do
-              {{:emit, new_signal}, new_state} ->
-                {:emit, new_signal, new_state}
+  defp handle_thinking(signal, state) do
+    require Logger
+    Logger.debug("Conversation flow - Processing thinking signal")
 
-              {:skip, new_state} ->
-                {:skip, new_state}
+    case Handlers.thinking_handler(signal, state) do
+      {{:emit, new_signal}, new_state} ->
+        {:emit, new_signal, new_state}
 
-              other ->
-                Logger.warning("Unexpected tool handler result: #{inspect(other)}")
-                {:skip, state}
-            end
+      {:skip, new_state} ->
+        {:skip, new_state}
 
-          :tool_result ->
-            # Tool result flow path
-            Logger.debug("Conversation flow - Processing tool result signal")
+      other ->
+        Logger.warning("Unexpected thinking handler result: #{inspect(other)}")
+        {:skip, state}
+    end
+  end
 
-            case Handlers.tool_result_handler(signal, state) do
-              {{:emit, new_signal}, new_state} ->
-                {:emit, new_signal, new_state}
+  defp handle_tool_call(signal, state) do
+    require Logger
+    Logger.debug("Conversation flow - Processing tool call signal")
 
-              {:skip, new_state} ->
-                {:skip, new_state}
+    case Handlers.tool_handler(signal, state) do
+      {{:emit, new_signal}, new_state} ->
+        {:emit, new_signal, new_state}
 
-              other ->
-                Logger.warning("Unexpected tool result handler result: #{inspect(other)}")
-                {:skip, state}
-            end
+      {:skip, new_state} ->
+        {:skip, new_state}
 
-          :task ->
-            # Task flow path
-            Logger.debug("Conversation flow - Processing task signal")
+      other ->
+        Logger.warning("Unexpected tool handler result: #{inspect(other)}")
+        {:skip, state}
+    end
+  end
 
-            case Handlers.task_handler(signal, state) do
-              {{:emit, new_signal}, new_state} ->
-                {:emit, new_signal, new_state}
+  defp handle_tool_result(signal, state) do
+    require Logger
+    Logger.debug("Conversation flow - Processing tool result signal")
 
-              {:skip, new_state} ->
-                {:skip, new_state}
+    case Handlers.tool_result_handler(signal, state) do
+      {{:emit, new_signal}, new_state} ->
+        {:emit, new_signal, new_state}
 
-              other ->
-                Logger.warning("Unexpected task handler result: #{inspect(other)}")
-                {:skip, state}
-            end
+      {:skip, new_state} ->
+        {:skip, new_state}
 
-          :response ->
-            # Response flow path
-            Logger.debug("Conversation flow - Processing response signal")
+      other ->
+        Logger.warning("Unexpected tool result handler result: #{inspect(other)}")
+        {:skip, state}
+    end
+  end
 
-            case Handlers.response_handler(signal, state) do
-              {{:emit, new_signal}, new_state} ->
-                {:emit, new_signal, new_state}
+  defp handle_task(signal, state) do
+    require Logger
+    Logger.debug("Conversation flow - Processing task signal")
 
-              {:skip, new_state} ->
-                {:skip, new_state}
+    case Handlers.task_handler(signal, state) do
+      {:skip, new_state} ->
+        {:skip, new_state}
 
-              other ->
-                Logger.warning("Unexpected response handler result: #{inspect(other)}")
-                {:skip, state}
-            end
+      other ->
+        Logger.warning("Unexpected task handler result: #{inspect(other)}")
+        {:skip, state}
+    end
+  end
 
-          :error ->
-            # Error flow path
-            Logger.debug("Conversation flow - Processing error signal")
+  defp handle_response(signal, state) do
+    require Logger
+    Logger.debug("Conversation flow - Processing response signal")
 
-            case Handlers.error_handler(signal, state) do
-              {{:emit, new_signal}, new_state} ->
-                {:emit, new_signal, new_state}
+    case Handlers.response_handler(signal, state) do
+      {:skip, new_state} ->
+        {:skip, new_state}
 
-              {:skip, new_state} ->
-                {:skip, new_state}
+      {{:halt, result}, new_state} ->
+        {:halt, result, new_state}
 
-              other ->
-                Logger.warning("Unexpected error handler result: #{inspect(other)}")
-                {:skip, state}
-            end
+      other ->
+        Logger.warning("Unexpected response handler result: #{inspect(other)}")
+        {:skip, state}
+    end
+  end
 
-          _ ->
-            # Fallback for unknown signal types
-            Logger.warning("Conversation flow - Unknown signal type: #{inspect(signal.type)}")
+  defp handle_error(signal, state) do
+    require Logger
+    Logger.debug("Conversation flow - Processing error signal")
 
-            fallback_signal =
-              Signals.response("Received unhandled signal type: #{inspect(signal.type)}")
+    case Handlers.error_handler(signal, state) do
+      {{:emit, new_signal}, new_state} ->
+        {:emit, new_signal, new_state}
 
-            {:emit, fallback_signal, state}
-        end
+      {:skip, new_state} ->
+        {:skip, new_state}
 
-      Logger.debug("Conversation flow - Result: #{inspect(result)}")
-      result
+      other ->
+        Logger.warning("Unexpected error handler result: #{inspect(other)}")
+        {:skip, state}
     end
+  end
 
-    {flow, initial_state}
+  defp handle_unknown_signal_type(signal, state) do
+    require Logger
+    Logger.warning("Conversation flow - Unknown signal type: #{inspect(signal.type)}")
+
+    fallback_signal = Signals.response("Received unhandled signal type: #{inspect(signal.type)}")
+
+    {:emit, fallback_signal, state}
   end
 
   @doc """
@@ -428,26 +449,30 @@ defmodule LLMAgent.Flows do
   end
 
   # Create a flow that combines flows together in sequence
+  # This function is currently unused but kept for future use
   defp sequence_flows(flows) do
     fn signal, state ->
-      Enum.reduce_while(flows, {signal, state}, fn flow, {current_signal, current_state} ->
-        case flow.(current_signal, current_state) do
-          {:emit, new_signal, new_state} ->
-            {:cont, {new_signal, new_state}}
+      Enum.reduce_while(flows, {signal, state}, &process_flow_result/2)
+    end
+  end
 
-          {:skip, new_state} ->
-            {:cont, {current_signal, new_state}}
+  # Helper function to process flow results and determine continuation
+  defp process_flow_result(flow, {current_signal, current_state}) do
+    case flow.(current_signal, current_state) do
+      {:emit, new_signal, new_state} ->
+        {:cont, {new_signal, new_state}}
 
-          {:halt, result, new_state} ->
-            {:halt, {:halt, result, new_state}}
+      {:skip, new_state} ->
+        {:cont, {current_signal, new_state}}
 
-          {:error, reason, new_state} ->
-            {:halt, {:error, reason, new_state}}
+      {:halt, result, new_state} ->
+        {:halt, {:halt, result, new_state}}
 
-          other ->
-            {:halt, other}
-        end
-      end)
+      {:error, reason, new_state} ->
+        {:halt, {:error, reason, new_state}}
+
+      other ->
+        {:halt, other}
     end
   end
 end
diff --git a/lib/llm_agent/handlers.ex b/lib/llm_agent/handlers.ex
@@ -421,50 +421,53 @@ defmodule LLMAgent.Handlers do
 
   defp extract_content(llm_result) do
     # First handle responses wrapped in tuples, which is the format returned by MockElixirQAProvider
-    result =
-      case llm_result do
-        {:ok, data} ->
-          Logger.debug("Extract_content - Unwrapping {:ok, data} format")
-          data
-
-        other ->
-          other
-      end
+    result = unwrap_result(llm_result)
 
-    # Then extract content from different possible locations
-    case result do
-      %{"content" => content} when is_binary(content) ->
-        content
+    # Then extract content from different possible formats
+    extract_from_format(result)
+  end
 
-      %{"message" => %{"content" => content}} when is_binary(content) ->
-        content
+  # Unwraps result from potential tuple format
+  defp unwrap_result({:ok, data}) do
+    Logger.debug("Extract_content - Unwrapping {:ok, data} format")
+    data
+  end
 
-      %{"choices" => [%{"message" => %{"content" => content}} | _]} when is_binary(content) ->
-        content
+  defp unwrap_result(other), do: other
 
-      %{"choices" => [%{"text" => content} | _]} when is_binary(content) ->
-        content
+  # Extract content from various response formats
+  defp extract_from_format(%{"content" => content}) when is_binary(content), do: content
 
-      # Handle deeply nested data structure
-      %{"choices" => [first | _]} ->
-        content = get_in(first, ["message", "content"])
-        if is_binary(content), do: content, else: ""
+  defp extract_from_format(%{"message" => %{"content" => content}}) when is_binary(content),
+    do: content
 
-      # Handle format containing type and content
-      %{"type" => "response", "content" => content} when is_binary(content) ->
-        content
+  defp extract_from_format(%{"choices" => [%{"message" => %{"content" => content}} | _]})
+       when is_binary(content),
+       do: content
 
-      %{"type" => "thinking", "content" => thought} when is_binary(thought) ->
-        thought
+  defp extract_from_format(%{"choices" => [%{"text" => content} | _]}) when is_binary(content),
+    do: content
 
-      # Fallback to empty string
-      _ ->
-        Logger.warning(
-          "Extract_content - Could not extract content from response: #{inspect(result)}"
-        )
+  # Handle deeply nested data structure
+  defp extract_from_format(%{"choices" => [first | _]}) do
+    content = get_in(first, ["message", "content"])
+    if is_binary(content), do: content, else: ""
+  end
 
-        ""
-    end
+  # Handle format containing type and content
+  defp extract_from_format(%{"type" => "response", "content" => content}) when is_binary(content),
+    do: content
+
+  defp extract_from_format(%{"type" => "thinking", "content" => thought}) when is_binary(thought),
+    do: thought
+
+  # Fallback to empty string
+  defp extract_from_format(result) do
+    Logger.warning(
+      "Extract_content - Could not extract content from response: #{inspect(result)}"
+    )
+
+    ""
   end
 
   defp extract_tool_calls(llm_result) do
