Harden adapters with retries and normalize config

haasonsaas · haasonsaas · commit 3e66c536b607 · 2025-12-24T23:46:36.000-08:00
diff --git a/src/adapters/anthropic.rs b/src/adapters/anthropic.rs
@@ -1,7 +1,9 @@
 use anyhow::{Context, Result};
 use async_trait::async_trait;
-use reqwest::Client;
+use reqwest::{Client, StatusCode};
 use serde::{Deserialize, Serialize};
+use std::time::Duration;
+use tokio::time::sleep;
 use crate::adapters::llm::{LLMAdapter, LLMRequest, LLMResponse, ModelConfig, Usage};
 
 pub struct AnthropicAdapter {
@@ -66,6 +68,42 @@ impl AnthropicAdapter {
             base_url,
         })
     }
+
+    async fn send_with_retry<F>(&self, mut make_request: F) -> Result<reqwest::Response>
+    where
+        F: FnMut() -> reqwest::RequestBuilder,
+    {
+        const MAX_RETRIES: usize = 2;
+        const BASE_DELAY_MS: u64 = 250;
+
+        for attempt in 0..=MAX_RETRIES {
+            match make_request().send().await {
+                Ok(response) => {
+                    if response.status().is_success() {
+                        return Ok(response);
+                    }
+
+                    let status = response.status();
+                    let body = response.text().await.unwrap_or_default();
+                    if is_retryable_status(status) && attempt < MAX_RETRIES {
+                        sleep(Duration::from_millis(BASE_DELAY_MS * (attempt as u64 + 1))).await;
+                        continue;
+                    }
+
+                    anyhow::bail!("Anthropic API error ({}): {}", status, body);
+                }
+                Err(err) => {
+                    if attempt < MAX_RETRIES {
+                        sleep(Duration::from_millis(BASE_DELAY_MS * (attempt as u64 + 1))).await;
+                        continue;
+                    }
+                    return Err(err.into());
+                }
+            }
+        }
+
+        anyhow::bail!("Anthropic request failed after retries");
+    }
 }
 
 #[async_trait]
@@ -86,21 +124,18 @@ impl LLMAdapter for AnthropicAdapter {
             system: request.system_prompt,
         };
         
-        let response = self.client
-            .post(format!("{}/messages", self.base_url))
-            .header("x-api-key", &self.api_key)
-            .header("anthropic-version", "2023-06-01")
-            .header("anthropic-beta", "messages-2023-12-15")
-            .header("Content-Type", "application/json")
-            .json(&anthropic_request)
-            .send()
-            .await
-            .context("Failed to send request to Anthropic")?;
-        
-        if !response.status().is_success() {
-            let error_text = response.text().await?;
-            anyhow::bail!("Anthropic API error: {}", error_text);
-        }
+        let url = format!("{}/messages", self.base_url);
+        let response = self.send_with_retry(|| {
+            self.client
+                .post(&url)
+                .header("x-api-key", &self.api_key)
+                .header("anthropic-version", "2023-06-01")
+                .header("anthropic-beta", "messages-2023-12-15")
+                .header("Content-Type", "application/json")
+                .json(&anthropic_request)
+        })
+        .await
+        .context("Failed to send request to Anthropic")?;
         
         let anthropic_response: AnthropicResponse = response.json().await
             .context("Failed to parse Anthropic response")?;
@@ -131,4 +166,8 @@ impl LLMAdapter for AnthropicAdapter {
     fn _model_name(&self) -> &str {
         &self.config.model_name
     }
-}
+}
+
+fn is_retryable_status(status: StatusCode) -> bool {
+    status == StatusCode::TOO_MANY_REQUESTS || status.is_server_error()
+}
diff --git a/src/adapters/ollama.rs b/src/adapters/ollama.rs
@@ -1,7 +1,9 @@
 use anyhow::{Context, Result};
 use async_trait::async_trait;
-use reqwest::Client;
+use reqwest::{Client, StatusCode};
 use serde::{Deserialize, Serialize};
+use std::time::Duration;
+use tokio::time::sleep;
 use crate::adapters::llm::{LLMAdapter, LLMRequest, LLMResponse, ModelConfig, Usage};
 
 pub struct OllamaAdapter {
@@ -46,6 +48,42 @@ impl OllamaAdapter {
             base_url,
         })
     }
+
+    async fn send_with_retry<F>(&self, mut make_request: F) -> Result<reqwest::Response>
+    where
+        F: FnMut() -> reqwest::RequestBuilder,
+    {
+        const MAX_RETRIES: usize = 2;
+        const BASE_DELAY_MS: u64 = 250;
+
+        for attempt in 0..=MAX_RETRIES {
+            match make_request().send().await {
+                Ok(response) => {
+                    if response.status().is_success() {
+                        return Ok(response);
+                    }
+
+                    let status = response.status();
+                    let body = response.text().await.unwrap_or_default();
+                    if is_retryable_status(status) && attempt < MAX_RETRIES {
+                        sleep(Duration::from_millis(BASE_DELAY_MS * (attempt as u64 + 1))).await;
+                        continue;
+                    }
+
+                    anyhow::bail!("Ollama API error ({}): {}", status, body);
+                }
+                Err(err) => {
+                    if attempt < MAX_RETRIES {
+                        sleep(Duration::from_millis(BASE_DELAY_MS * (attempt as u64 + 1))).await;
+                        continue;
+                    }
+                    return Err(err.into());
+                }
+            }
+        }
+
+        anyhow::bail!("Ollama request failed after retries");
+    }
 }
 
 #[async_trait]
@@ -64,17 +102,14 @@ impl LLMAdapter for OllamaAdapter {
             stream: false,
         };
         
-        let response = self.client
-            .post(format!("{}/api/generate", self.base_url))
-            .json(&ollama_request)
-            .send()
-            .await
-            .context("Failed to send request to Ollama")?;
-        
-        if !response.status().is_success() {
-            let error_text = response.text().await?;
-            anyhow::bail!("Ollama API error: {}", error_text);
-        }
+        let url = format!("{}/api/generate", self.base_url);
+        let response = self.send_with_retry(|| {
+            self.client
+                .post(&url)
+                .json(&ollama_request)
+        })
+        .await
+        .context("Failed to send request to Ollama")?;
         
         let ollama_response: OllamaResponse = response.json().await
             .context("Failed to parse Ollama response")?;
@@ -97,4 +132,8 @@ impl LLMAdapter for OllamaAdapter {
     fn _model_name(&self) -> &str {
         &self.config.model_name
     }
-}
+}
+
+fn is_retryable_status(status: StatusCode) -> bool {
+    status == StatusCode::TOO_MANY_REQUESTS || status.is_server_error()
+}
diff --git a/src/adapters/openai.rs b/src/adapters/openai.rs
@@ -1,7 +1,9 @@
 use anyhow::{Context, Result};
 use async_trait::async_trait;
-use reqwest::Client;
+use reqwest::{Client, StatusCode};
 use serde::{Deserialize, Serialize};
+use std::time::Duration;
+use tokio::time::sleep;
 use crate::adapters::llm::{LLMAdapter, LLMRequest, LLMResponse, ModelConfig, Usage};
 
 pub struct OpenAIAdapter {
@@ -64,6 +66,42 @@ impl OpenAIAdapter {
             base_url,
         })
     }
+
+    async fn send_with_retry<F>(&self, mut make_request: F) -> Result<reqwest::Response>
+    where
+        F: FnMut() -> reqwest::RequestBuilder,
+    {
+        const MAX_RETRIES: usize = 2;
+        const BASE_DELAY_MS: u64 = 250;
+
+        for attempt in 0..=MAX_RETRIES {
+            match make_request().send().await {
+                Ok(response) => {
+                    if response.status().is_success() {
+                        return Ok(response);
+                    }
+
+                    let status = response.status();
+                    let body = response.text().await.unwrap_or_default();
+                    if is_retryable_status(status) && attempt < MAX_RETRIES {
+                        sleep(Duration::from_millis(BASE_DELAY_MS * (attempt as u64 + 1))).await;
+                        continue;
+                    }
+
+                    anyhow::bail!("OpenAI API error ({}): {}", status, body);
+                }
+                Err(err) => {
+                    if attempt < MAX_RETRIES {
+                        sleep(Duration::from_millis(BASE_DELAY_MS * (attempt as u64 + 1))).await;
+                        continue;
+                    }
+                    return Err(err.into());
+                }
+            }
+        }
+
+        anyhow::bail!("OpenAI request failed after retries");
+    }
 }
 
 #[async_trait]
@@ -87,19 +125,16 @@ impl LLMAdapter for OpenAIAdapter {
             max_tokens: request.max_tokens.unwrap_or(self.config.max_tokens),
         };
         
-        let response = self.client
-            .post(format!("{}/chat/completions", self.base_url))
-            .header("Authorization", format!("Bearer {}", self.api_key))
-            .header("Content-Type", "application/json")
-            .json(&openai_request)
-            .send()
-            .await
-            .context("Failed to send request to OpenAI")?;
-        
-        if !response.status().is_success() {
-            let error_text = response.text().await?;
-            anyhow::bail!("OpenAI API error: {}", error_text);
-        }
+        let url = format!("{}/chat/completions", self.base_url);
+        let response = self.send_with_retry(|| {
+            self.client
+                .post(&url)
+                .header("Authorization", format!("Bearer {}", self.api_key))
+                .header("Content-Type", "application/json")
+                .json(&openai_request)
+        })
+        .await
+        .context("Failed to send request to OpenAI")?;
         
         let openai_response: OpenAIResponse = response.json().await
             .context("Failed to parse OpenAI response")?;
@@ -123,4 +158,8 @@ impl LLMAdapter for OpenAIAdapter {
     fn _model_name(&self) -> &str {
         &self.config.model_name
     }
-}
+}
+
+fn is_retryable_status(status: StatusCode) -> bool {
+    status == StatusCode::TOO_MANY_REQUESTS || status.is_server_error()
+}
diff --git a/src/config.rs b/src/config.rs
@@ -125,6 +125,20 @@ impl Config {
             self.system_prompt = Some(prompt);
         }
     }
+
+    pub fn normalize(&mut self) {
+        if self.model.trim().is_empty() {
+            self.model = default_model();
+        }
+
+        if !self.temperature.is_finite() || self.temperature < 0.0 || self.temperature > 2.0 {
+            self.temperature = default_temperature();
+        }
+
+        if self.max_tokens == 0 {
+            self.max_tokens = default_max_tokens();
+        }
+    }
     
     pub fn get_path_config(&self, file_path: &PathBuf) -> Option<&PathConfig> {
         let file_path_str = file_path.to_string_lossy();
@@ -181,6 +195,25 @@ impl Config {
     }
 }
 
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn normalize_clamps_values() {
+        let mut config = Config::default();
+        config.model = "   ".to_string();
+        config.temperature = 5.0;
+        config.max_tokens = 0;
+
+        config.normalize();
+
+        assert_eq!(config.model, default_model());
+        assert_eq!(config.temperature, default_temperature());
+        assert_eq!(config.max_tokens, default_max_tokens());
+    }
+}
+
 fn default_model() -> String {
     "gpt-4o".to_string()
 }
@@ -195,4 +228,4 @@ fn default_max_tokens() -> usize {
 
 fn default_true() -> bool {
     true
-}
+}
diff --git a/src/main.rs b/src/main.rs
@@ -146,6 +146,7 @@ async fn main() -> Result<()> {
     if let Some(tokens) = cli.max_tokens {
         config.max_tokens = tokens;
     }
+    config.normalize();
     
     match cli.command {
         Commands::Review { diff, patch, output } => {

Original file line number	Diff line number	Diff line change
`@@ -146,6 +146,7 @@ async fn main() -> Result<()> {`
`146`	`146`	`if let Some(tokens) = cli.max_tokens {`
`147`	`147`	`config.max_tokens = tokens;`
`148`	`148`	`}`
	`149`	`+ config.normalize();`
`149`	`150`
`150`	`151`	`match cli.command {`
`151`	`152`	`Commands::Review { diff, patch, output } => {`