Implement strategy pattern for generation fallbacks

Co-authored-by: oleander <220827+oleander@users.noreply.github.com>

Author: Copilot

Cargo.lock

@@ -44,6 +44,7 @@ tokio = { version = "1.45.1", features = ["full"] }
 futures = "0.3"
 parking_lot = "0.12.3"
 tracing = "0.1"
+async-trait = "0.1"
 
 # CLI and UI
 

Cargo.toml

@@ -1,12 +1,10 @@
 use anyhow::{anyhow, bail, Result};
 use maplit::hashmap;
 use mustache;
-use async_openai::Client;
 
-use crate::{config, debug_output, openai, profile};
+use crate::{config, openai, profile};
 use crate::model::Model;
 use crate::config::AppConfig;
-use crate::multi_step_integration::{generate_commit_message_local, generate_commit_message_multi_step};
 
 /// The instruction template included at compile time
 const INSTRUCTION_TEMPLATE: &str = include_str!("../resources/prompt.md");
@@ -73,125 +71,20 @@ pub fn create_commit_request(diff: String, max_tokens: usize, model: Model) -> R
 /// Returns an error if:
 /// - max_tokens is 0
 /// - OpenAI API call fails
-pub async fn generate(patch: String, remaining_tokens: usize, model: Model, settings: Option<&AppConfig>) -> Result<openai::Response> {
+pub async fn generate(patch: String, remaining_tokens: usize, _model: Model, settings: Option<&AppConfig>) -> Result<openai::Response> {
   profile!("Generate commit message");
 
   if remaining_tokens == 0 {
     bail!("Maximum token count must be greater than zero")
   }
 
-  // Try multi-step approach first
-  let max_length = settings
-    .and_then(|s| s.max_commit_length)
-    .or(config::APP_CONFIG.max_commit_length);
-
-  // Check if we have a valid API key configuration
-  let has_valid_api_key = if let Some(custom_settings) = settings {
-    custom_settings
-      .openai_api_key
-      .as_ref()
-      .map(|key| !key.is_empty() && key != "<PLACE HOLDER FOR YOUR API KEY>")
-      .unwrap_or(false)
-  } else {
-    // Check environment variable or config
-    config::APP_CONFIG
-      .openai_api_key
-      .as_ref()
-      .map(|key| !key.is_empty() && key != "<PLACE HOLDER FOR YOUR API KEY>")
-      .unwrap_or(false)
-      || std::env::var("OPENAI_API_KEY")
-        .map(|key| !key.is_empty())
-        .unwrap_or(false)
-  };
-
-  if !has_valid_api_key {
-    bail!("OpenAI API key not configured. Please set your API key using:\n  git-ai config set openai-api-key <your-key>\nor set the OPENAI_API_KEY environment variable.");
-  }
-
-  // Use custom settings if provided
-  if let Some(custom_settings) = settings {
-    if let Some(api_key) = &custom_settings.openai_api_key {
-      if !api_key.is_empty() && api_key != "<PLACE HOLDER FOR YOUR API KEY>" {
-        match openai::create_openai_config(custom_settings) {
-          Ok(config) => {
-            let client = Client::with_config(config);
-            let model_str = model.to_string();
-
-            match generate_commit_message_multi_step(&client, &model_str, &patch, max_length).await {
-              Ok(message) => return Ok(openai::Response { response: message }),
-              Err(e) => {
-                // Check if it's an API key error
-                if e.to_string().contains("invalid_api_key") || e.to_string().contains("Incorrect API key") {
-                  bail!("Invalid OpenAI API key. Please check your API key configuration.");
-                }
-                log::warn!("Multi-step generation with custom settings failed: {e}");
-                if let Some(session) = debug_output::debug_session() {
-                  session.set_multi_step_error(e.to_string());
-                }
-              }
-            }
-          }
-          Err(e) => {
-            // If config creation fails due to API key, propagate the error
-            return Err(e);
-          }
-        }
-      }
-    }
-  } else {
-    // Try with default settings
-    if let Ok(api_key) = std::env::var("OPENAI_API_KEY") {
-      if !api_key.is_empty() {
-        let client = Client::new();
-        let model_str = model.to_string();
-
-        match generate_commit_message_multi_step(&client, &model_str, &patch, max_length).await {
-          Ok(message) => return Ok(openai::Response { response: message }),
-          Err(e) => {
-            // Check if it's an API key error
-            if e.to_string().contains("invalid_api_key") || e.to_string().contains("Incorrect API key") {
-              bail!("Invalid OpenAI API key. Please check your API key configuration.");
-            }
-            log::warn!("Multi-step generation failed: {e}");
-            if let Some(session) = debug_output::debug_session() {
-              session.set_multi_step_error(e.to_string());
-            }
-          }
-        }
-      }
-    }
-  }
-
-  // Try local multi-step generation
-  match generate_commit_message_local(&patch, max_length) {
-    Ok(message) => return Ok(openai::Response { response: message }),
-    Err(e) => {
-      log::warn!("Local multi-step generation failed: {e}");
-    }
-  }
-
-  // Mark that we're using single-step fallback
-  if let Some(session) = debug_output::debug_session() {
-    session.set_single_step_success(true);
-  }
-
-  // Fallback to original single-step approach
-  let request = create_commit_request(patch, remaining_tokens, model)?;
-
-  // Use custom settings if provided, otherwise use global config
-  match settings {
-    Some(custom_settings) => {
-      // Create a client with custom settings
-      match openai::create_openai_config(custom_settings) {
-        Ok(config) => openai::call_with_config(request, config).await,
-        Err(e) => Err(e)
-      }
-    }
-    None => {
-      // Use the default global config
-      openai::call(request).await
-    }
-  }
+  // Use the provided settings, or fall back to global config
+  let config = settings.unwrap_or(&config::APP_CONFIG);
+  
+  // Use the new strategy pattern for generation
+  let message = crate::generation::fallback::generate_with_fallback(&patch, config).await?;
+  
+  Ok(openai::Response { response: message })
 }
 
 pub fn token_used(model: &Model) -> Result<usize> {
@@ -216,7 +109,7 @@ mod tests {
   use super::*;
 
   #[tokio::test]
-  async fn test_missing_api_key_error() {
+  async fn test_missing_api_key_fallback() {
     // Create settings with no API key
     let settings = AppConfig {
       openai_api_key:    None,
@@ -230,7 +123,7 @@ mod tests {
     let original_key = std::env::var("OPENAI_API_KEY").ok();
     std::env::remove_var("OPENAI_API_KEY");
 
-    // Test that generate returns an error for missing API key
+    // Test that generate falls back to local generation when no API key is available
     let result = generate(
       "diff --git a/test.txt b/test.txt\n+Hello World".to_string(),
       1024,
@@ -244,17 +137,14 @@ mod tests {
       std::env::set_var("OPENAI_API_KEY", key);
     }
 
-    assert!(result.is_err());
-    let error_message = result.unwrap_err().to_string();
-    assert!(
-      error_message.contains("OpenAI API key not configured"),
-      "Expected error message about missing API key, got: {}",
-      error_message
-    );
+    // Should succeed with local fallback
+    assert!(result.is_ok(), "Expected fallback to local generation to succeed, got error: {:?}", result.err());
+    let response = result.unwrap();
+    assert!(!response.response.is_empty(), "Expected non-empty commit message");
   }
 
   #[tokio::test]
-  async fn test_invalid_api_key_error() {
+  async fn test_invalid_api_key_fallback() {
     // Create settings with invalid API key
     let settings = AppConfig {
       openai_api_key:    Some("<PLACE HOLDER FOR YOUR API KEY>".to_string()),
@@ -264,7 +154,7 @@ mod tests {
       timeout:           Some(30)
     };
 
-    // Test that generate returns an error for invalid API key
+    // Test that generate falls back to local generation when API key is invalid
     let result = generate(
       "diff --git a/test.txt b/test.txt\n+Hello World".to_string(),
       1024,
@@ -273,12 +163,9 @@ mod tests {
     )
     .await;
 
-    assert!(result.is_err());
-    let error_message = result.unwrap_err().to_string();
-    assert!(
-      error_message.contains("OpenAI API key not configured"),
-      "Expected error message about invalid API key, got: {}",
-      error_message
-    );
+    // Should succeed with local fallback
+    assert!(result.is_ok(), "Expected fallback to local generation to succeed, got error: {:?}", result.err());
+    let response = result.unwrap();
+    assert!(!response.response.is_empty(), "Expected non-empty commit message");
   }
 }