[Refactor] Implement strategy pattern for generation fallbacks

[oleander]

Context

Fallback logic is currently scattered across commit.rs, openai.rs, and multi-step modules. This implements a clean strategy pattern to centralize fallback orchestration.

Priority

🟠 MEDIUM - Improves architecture significantly

Steps

1. Create src/generation/fallback.rs

//! Fallback strategy orchestration for commit message generation.
//!
//! Implements a strategy pattern that tries multiple generation
//! approaches in order until one succeeds:
//! 1. Multi-step with OpenAI API
//! 2. Local multi-step analysis
//! 3. Single-step API call
//!
//! # Example
//! ```rust
//! let message = generate_with_fallback(diff, config).await?;
//! ```

use anyhow::{anyhow, bail, Result};
use async_trait::async_trait;
use async_openai::config::OpenAIConfig;
use async_openai::Client;

use crate::config::AppConfig;
use super::multi_step;

/// Strategy for generating commit messages
#[async_trait]
pub trait GenerationStrategy: Send + Sync {
    /// Attempt to generate a commit message
    async fn generate(
        &self,
        diff: &str,
        config: &AppConfig,
    ) -> Result<String>;
    
    /// Name of this strategy (for logging)
    fn name(&self) -> &str;
    
    /// Whether this strategy requires an API key
    fn requires_api_key(&self) -> bool {
        false
    }
}

/// Multi-step generation using OpenAI API
pub struct MultiStepAPIStrategy {
    client: Client<OpenAIConfig>,
    model: String,
}

impl MultiStepAPIStrategy {
    pub fn new(config: &AppConfig) -> Result<Self> {
        let api_key = crate::api::auth::get_api_key(config)?;
        let openai_config = OpenAIConfig::new().with_api_key(api_key);
        let client = Client::with_config(openai_config);
        let model = config.model.clone().unwrap_or("gpt-4o-mini".to_string());
        
        Ok(Self { client, model })
    }
}

#[async_trait]
impl GenerationStrategy for MultiStepAPIStrategy {
    async fn generate(&self, diff: &str, config: &AppConfig) -> Result<String> {
        multi_step::generate_with_api(&self.client, &self.model, diff, config).await
    }
    
    fn name(&self) -> &str {
        "Multi-step API"
    }
    
    fn requires_api_key(&self) -> bool {
        true
    }
}

/// Local multi-step generation (no API)
pub struct LocalMultiStepStrategy;

#[async_trait]
impl GenerationStrategy for LocalMultiStepStrategy {
    async fn generate(&self, diff: &str, config: &AppConfig) -> Result<String> {
        multi_step::generate_local(diff, config)
    }
    
    fn name(&self) -> &str {
        "Local multi-step"
    }
}

/// Simple single-step API generation (original approach)
pub struct SingleStepAPIStrategy {
    client: Client<OpenAIConfig>,
    model: String,
}

impl SingleStepAPIStrategy {
    pub fn new(config: &AppConfig) -> Result<Self> {
        let api_key = crate::api::auth::get_api_key(config)?;
        let openai_config = OpenAIConfig::new().with_api_key(api_key);
        let client = Client::with_config(openai_config);
        let model = config.model.clone().unwrap_or("gpt-4o-mini".to_string());
        
        Ok(Self { client, model })
    }
}

#[async_trait]
impl GenerationStrategy for SingleStepAPIStrategy {
    async fn generate(&self, diff: &str, config: &AppConfig) -> Result<String> {
        // Use original single-step generation
        crate::api::openai::generate_single_step(&self.client, &self.model, diff, config).await
    }
    
    fn name(&self) -> &str {
        "Single-step API"
    }
    
    fn requires_api_key(&self) -> bool {
        true
    }
}

/// Main entry point: Try strategies in order until one succeeds
pub async fn generate_with_fallback(
    diff: &str,
    config: &AppConfig,
) -> Result<String> {
    // Build strategy list
    let mut strategies: Vec<Box<dyn GenerationStrategy>> = Vec::new();
    
    // Try API strategies first if we have a key
    if crate::api::auth::get_api_key(config).is_ok() {
        if let Ok(multi_step) = MultiStepAPIStrategy::new(config) {
            strategies.push(Box::new(multi_step));
        }
    }
    
    // Always include local fallback
    strategies.push(Box::new(LocalMultiStepStrategy));
    
    // Single-step as final API fallback
    if let Ok(single_step) = SingleStepAPIStrategy::new(config) {
        strategies.push(Box::new(single_step));
    }
    
    let mut errors = Vec::new();
    
    for strategy in strategies {
        log::info!("Attempting generation with: {}", strategy.name());
        
        match strategy.generate(diff, config).await {
            Ok(message) => {
                log::info!("Successfully generated with: {}", strategy.name());
                return Ok(message);
            }
            Err(e) => {
                let error_msg = e.to_string();
                log::warn!("{} failed: {}", strategy.name(), error_msg);
                
                // Don't retry on auth errors
                if error_msg.contains("invalid_api_key") 
                   || error_msg.contains("Invalid API key") {
                    return Err(e);
                }
                
                errors.push((strategy.name().to_string(), error_msg));
            }
        }
    }
    
    // All strategies failed
    let error_summary = errors
        .iter()
        .map(|(name, err)| format!("  - {}: {}", name, err))
        .collect::<Vec<_>>()
        .join("\n");
    
    bail!(
        "All generation strategies failed:\n{}",
        error_summary
    )
}

#[cfg(test)]
mod tests {
    use super::*;
    
    #[tokio::test]
    async fn test_fallback_with_no_api_key() {
        // Should fall back to local strategy
        let config = AppConfig::default();
        let result = generate_with_fallback("test diff", &config).await;
        // Should succeed with local strategy
        assert!(result.is_ok() || result.err().unwrap().to_string().contains("No files"));
    }
}

2. Update src/generation/mod.rs

pub mod multi_step;
pub mod fallback;

pub use fallback::generate_with_fallback;

3. Update src/commit.rs to use strategy pattern

Replace the generate function with:

pub async fn generate(
    diff: &str,
    config: &AppConfig,
) -> Result<String> {
    crate::generation::fallback::generate_with_fallback(diff, config).await
}

Remove all the scattered fallback logic (lines 80-176).

4. Add async-trait dependency to Cargo.toml

[dependencies]
async-trait = "0.1"

Verification Criteria

✅ Pass:

• Strategy pattern implemented in fallback.rs
• All strategies implement GenerationStrategy trait
• Fallback orchestration centralized in one place
• Old scattered fallback logic removed from commit.rs and openai.rs
• cargo build succeeds
• cargo test passes
• Fallback works correctly (test with/without API key)
• Error messages clearly indicate which strategies failed
• cargo clippy shows no warnings

Test scenarios

# Test 1: With valid API key
export OPENAI_API_KEY=your-real-key
cd test-repo
echo "test" > file.txt
git add file.txt
git commit --no-edit
# Should succeed with "Multi-step API" in logs

# Test 2: With invalid API key  
export OPENAI_API_KEY=invalid-key
echo "test2" > file2.txt
git add file2.txt
git commit --no-edit
# Should fail immediately with auth error (not try other strategies)

# Test 3: With no API key
unset OPENAI_API_KEY
echo "test3" > file3.txt
git add file3.txt
git commit --no-edit
# Should succeed with "Local multi-step" in logs

# Test 4: Check error messages
# Set up scenario where all strategies fail
# Error message should list all strategies that were tried

Estimated Time

4-5 hours

Dependencies

• Issue AI training #5 (API auth module) - uses auth::get_api_key
• Issue Use structopt instead of clap #8 (Consolidate multi-step) - uses generation::multi_step

Labels

• refactor
• architecture
• generation
• error-handling