# BEFORE:
{:error, reason} ->
  {:error, reason}

# AFTER:
{:error, reason} ->
  Logger.error("[#{worker_state.worker_id}] Session checkout failed: #{inspect(reason)}")
  {:remove, {:checkout_failed, reason}, pool_state}

# BEFORE:
{:error, reason} ->
  {:error, reason}

# AFTER:
{:error, reason} ->
  Logger.error("[#{worker_state.worker_id}] Anonymous checkout failed: #{inspect(reason)}")
  {:remove, {:checkout_failed, reason}, pool_state}

# BEFORE:
catch
  :error, reason ->
    Logger.error("[#{worker_state.worker_id}] Failed to connect port to PID #{inspect(pid)} (alive? #{Process.alive?(pid)}): #{inspect(reason)}")
    {:remove, {:connect_failed, reason}, pool_state}
end

# AFTER:
catch
  :error, reason ->
    Logger.error("[#{worker_state.worker_id}] Failed to connect port: #{inspect(reason)}")
    {:remove, {:connect_failed, reason}, pool_state}
  
  :exit, reason ->
    Logger.error("[#{worker_state.worker_id}] Port connect exited: #{inspect(reason)}")
    {:remove, {:connect_exited, reason}, pool_state}
  
  kind, reason ->
    Logger.error("[#{worker_state.worker_id}] Unexpected error in connect: #{kind} - #{inspect(reason)}")
    {:remove, {:connect_error, {kind, reason}}, pool_state}
end

@doc """
Validates that a port is open and ready for connection.
Returns {:ok, port_info} or {:error, reason}
"""
defp validate_port(port) do
  cond do
    not is_port(port) ->
      {:error, :not_a_port}
      
    true ->
      case Port.info(port) do
        nil ->
          {:error, :port_closed}
          
        info ->
          # Check if port is connected to current process
          case Keyword.get(info, :connected) do
            pid when pid == self() ->
              {:ok, info}
            _ ->
              {:error, :port_not_owned}
          end
      end
  end
end

@doc """
Safely connects a port to a target process with validation.
"""
defp safe_port_connect(port, target_pid, worker_id) do
  with {:ok, _port_info} <- validate_port(port),
       true <- Process.alive?(target_pid) do
    try do
      Port.connect(port, target_pid)
      :ok
    catch
      :error, :badarg ->
        # Process died between alive? check and connect
        {:error, :process_died}
      :error, reason ->
        {:error, {:connect_failed, reason}}
    end
  else
    false ->
      {:error, :target_process_dead}
    {:error, reason} ->
      {:error, reason}
  end
end

# BEFORE:
if port_valid and pid_alive do
  Port.connect(worker_state.port, pid)
  # ... rest of the code
else
  reason = 
    cond do
      not port_valid -> :invalid_port
      not pid_alive -> :process_not_alive
      true -> :unknown_error
    end
  {:error, reason}
end

# AFTER:
case safe_port_connect(worker_state.port, pid, worker_state.worker_id) do
  :ok ->
    Logger.debug("[#{worker_state.worker_id}] Connected to session '#{session_id}' for PID #{inspect(pid)}")
    
    updated_state = %{worker_state | 
      current_session: session_id,
      stats: Map.update!(worker_state.stats, :checkouts, &(&1 + 1))
    }
    
    {:ok, :state, updated_state, pool_state}
    
  {:error, reason} ->
    Logger.error("[#{worker_state.worker_id}] Failed to connect port for session '#{session_id}': #{inspect(reason)}")
    {:remove, {:connect_failed, reason}, pool_state}
end

# BEFORE:
assert is_list(programs)

# AFTER:
assert is_map(result)
assert Map.has_key?(result, "programs")
programs = result["programs"]
assert is_list(programs)

# BEFORE:
assert length(programs) == 10

# AFTER:
assert result["total_count"] == 10
assert length(result["programs"]) == 10

@moduletag :layer_3

setup do
  test_mode = Application.get_env(:dspex, :test_mode, :mock_adapter)
  pooling_enabled = Application.get_env(:dspex, :pooling_enabled, false)
  
  unless test_mode == :full_integration and pooling_enabled do
    skip("This test requires TEST_MODE=full_integration with pooling enabled")
  end
  
  :ok
end

# BEFORE:
defp detect_running_service do
  pool_running = match?({:ok, _}, Registry.lookup(Registry.DSPex, SessionPool))
  bridge_running = match?({:ok, _}, Registry.lookup(Registry.DSPex, Bridge))
  
  case {pool_running, bridge_running} do
    {true, _} -> {:pool, SessionPool}
    {false, true} -> {:bridge, Bridge}
    _ -> {:error, "Python bridge not available"}
  end
end

# AFTER:
defp detect_running_service do
  # Use Process.whereis for more reliable detection
  pool_pid = Process.whereis(DSPex.PythonBridge.SessionPool)
  bridge_pid = Process.whereis(DSPex.PythonBridge.Bridge)
  
  cond do
    is_pid(pool_pid) and Process.alive?(pool_pid) ->
      {:pool, DSPex.PythonBridge.SessionPool}
      
    is_pid(bridge_pid) and Process.alive?(bridge_pid) ->
      {:bridge, DSPex.PythonBridge.Bridge}
      
    true ->
      # Try Registry as fallback
      case {Registry.lookup(Registry.DSPex, SessionPool), 
            Registry.lookup(Registry.DSPex, Bridge)} do
        {[{_, _}], _} -> {:pool, SessionPool}
        {_, [{_, _}]} -> {:bridge, Bridge}
        _ -> {:error, "Python bridge not available"}
      end
  end
end

# Helper to safely update worker stats
defp update_worker_stats(worker_state, checkin_type) do
  stats_update = case checkin_type do
    :ok -> 
      %{successful_checkins: &(&1 + 1), last_activity: System.monotonic_time(:millisecond)}
    {:error, _} -> 
      %{error_checkins: &(&1 + 1), last_activity: System.monotonic_time(:millisecond)}
    :close -> 
      %{last_activity: System.monotonic_time(:millisecond)}
    _ -> 
      %{last_activity: System.monotonic_time(:millisecond)}
  end
  
  updated_stats = Enum.reduce(stats_update, worker_state.stats, fn {key, updater}, stats ->
    case updater do
      fun when is_function(fun, 1) ->
        Map.update!(stats, key, fun)
      value ->
        Map.put(stats, key, value)
    end
  end)
  
  %{worker_state | stats: updated_stats}
end

# Line 117-118
# BEFORE:
stats = Map.update!(worker_state.stats, :successful_checkins, &(&1 + 1))
{:ok, %{worker_state | stats: stats, current_session: nil}, pool_state}

# AFTER:
updated_state = worker_state
  |> update_worker_stats(:ok)
  |> Map.put(:current_session, nil)
{:ok, updated_state, pool_state}

defmodule DSPex.Test.PoolTestHelpers do
  @moduledoc """
  Helper functions for pool-related tests.
  """
  
  @doc """
  Starts an isolated pool for testing with eager initialization.
  """
  def start_test_pool(opts \\ []) do
    pool_name = :"test_pool_#{System.unique_integer([:positive])}"
    genserver_name = :"genserver_#{pool_name}"
    
    config = Keyword.merge([
      name: genserver_name,
      pool_name: pool_name,
      lazy: false,  # Eager initialization for tests
      pool_size: 2,
      max_overflow: 0,
      init_timeout: 10_000  # Longer timeout for test stability
    ], opts)
    
    case start_supervised({DSPex.PythonBridge.SessionPoolV2, config}) do
      {:ok, pid} ->
        # Wait for pool to be ready
        wait_for_pool_ready(pool_name, 5_000)
        {:ok, %{pid: pid, pool_name: pool_name, genserver_name: genserver_name}}
        
      error ->
        error
    end
  end
  
  @doc """
  Waits for a pool to have at least one available worker.
  """
  def wait_for_pool_ready(pool_name, timeout) do
    deadline = System.monotonic_time(:millisecond) + timeout
    
    Stream.repeatedly(fn ->
      case get_pool_status(pool_name) do
        {:ok, %{available_workers: n}} when n > 0 ->
          :ready
        _ ->
          Process.sleep(100)
          :waiting
      end
    end)
    |> Stream.take_while(fn status ->
      status == :waiting and System.monotonic_time(:millisecond) < deadline
    end)
    |> Enum.to_list()
    
    :ok
  end
  
  defp get_pool_status(pool_name) do
    # Implementation depends on pool monitoring API
    {:ok, %{available_workers: 2}}  # Placeholder
  end
end