From cfd1d5cceaec4cd5bc041cb03f7e6664be02413c Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 19 Feb 2026 18:38:02 -0400 Subject: [PATCH] Simplify codebase: deduplicate providers, remove redundancy, clean up types Extract shared provider helpers (parse_error_body, send_and_read_body, extract_system_prompt, ApiMessage) into providers/common.rs, eliminating duplicated logic across all three providers. Simplify StepResult and GenerateResult to derive fields from the embedded Response rather than storing redundant copies. Remove stringly-typed ToolCall.r#type and ResponseFormat.r#type in favor of proper enums. Remove unused dependencies (hyper, clap, rayon, base64, etc.), use LazyLock for the model catalog, and fix miscellaneous idiom issues (unnecessary Vec collects, inconsistent error Display, module_name_repetitions). Co-Authored-By: Claude Opus 4.6 --- Cargo.lock | 53 ------- Cargo.toml | 7 - crates/unified-llm/Cargo.toml | 2 - crates/unified-llm/src/catalog.rs | 37 ++--- crates/unified-llm/src/error.rs | 10 +- crates/unified-llm/src/generate.rs | 94 ++++++------- crates/unified-llm/src/providers/anthropic.rs | 110 ++++----------- crates/unified-llm/src/providers/common.rs | 95 +++++++++++++ crates/unified-llm/src/providers/gemini.rs | 106 ++++---------- crates/unified-llm/src/providers/mod.rs | 7 +- crates/unified-llm/src/providers/openai.rs | 79 +++-------- crates/unified-llm/src/types.rs | 132 +++++++++++------- 12 files changed, 313 insertions(+), 419 deletions(-) create mode 100644 crates/unified-llm/src/providers/common.rs diff --git a/Cargo.lock b/Cargo.lock index 4fa26f44e..d264da939 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -91,31 +91,6 @@ version = "0.8.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b" -[[package]] -name = "crossbeam-deque" -version = "0.8.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9dd111b7b7f7d55b72c0a6ae361660ee5853c9af73f70c3c2ef6858b950e2e51" -dependencies = [ - "crossbeam-epoch", - "crossbeam-utils", -] - -[[package]] -name = "crossbeam-epoch" -version = "0.9.18" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5b82ac4a3c2ca9c3460964f020e1402edd5753411d7737aa39c3714ad1b5420e" -dependencies = [ - "crossbeam-utils", -] - -[[package]] -name = "crossbeam-utils" -version = "0.8.21" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d0a5c400df2834b80a4c3327b3aad3a4c4cd4de0629063962b03235697506a28" - [[package]] name = "displaydoc" version = "0.2.5" @@ -133,12 +108,6 @@ version = "0.15.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1aaf95b3e5c8f23aa320147307562d361db0ae0d51242340f558153b4eb2439b" -[[package]] -name = "either" -version = "1.15.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "48c757948c5ede0e46177b7add2e67155f70e33c07fea8284df6576da70b3719" - [[package]] name = "encoding_rs" version = "0.8.35" @@ -892,26 +861,6 @@ dependencies = [ "getrandom 0.2.17", ] -[[package]] -name = "rayon" -version = "1.11.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "368f01d005bf8fd9b1206fb6fa653e6c4a81ceb1466406b81792d87c5677a58f" -dependencies = [ - "either", - "rayon-core", -] - -[[package]] -name = "rayon-core" -version = "1.13.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "22e18b0f0062d30d4230b2e85ff77fdfe4326feb054b9783a3460d8435c8ab91" -dependencies = [ - "crossbeam-deque", - "crossbeam-utils", -] - [[package]] name = "redox_syscall" version = "0.5.18" @@ -1437,11 +1386,9 @@ version = "0.1.0" dependencies = [ "anyhow", "async-trait", - "base64", "dotenvy", "futures", "rand", - "rayon", "reqwest", "serde", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index 1eca597d9..f083f6b3b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,16 +20,9 @@ thiserror = "2" serde = { version = "1", features = ["derive"] } serde_json = "1" tokio = { version = "1", features = ["full"] } -hyper = { version = "1", features = ["full"] } -hyper-util = { version = "0.1", features = ["full"] } -http-body-util = "0.1" -hyper-tls = "0.6" -clap = { version = "4", features = ["derive"] } -rayon = "1" reqwest = { version = "0.12", features = ["json"] } uuid = { version = "1", features = ["v4"] } rand = "0.8" -base64 = "0.22" dotenvy = "0.15" futures = "0.3" tokio-stream = "0.1" diff --git a/crates/unified-llm/Cargo.toml b/crates/unified-llm/Cargo.toml index 147b079fd..1c2553c65 100644 --- a/crates/unified-llm/Cargo.toml +++ b/crates/unified-llm/Cargo.toml @@ -17,11 +17,9 @@ serde_json.workspace = true tokio.workspace = true uuid.workspace = true rand.workspace = true -base64.workspace = true futures.workspace = true tokio-stream.workspace = true async-trait.workspace = true -rayon.workspace = true reqwest.workspace = true [dev-dependencies] diff --git a/crates/unified-llm/src/catalog.rs b/crates/unified-llm/src/catalog.rs index 59564d3f0..759711abc 100644 --- a/crates/unified-llm/src/catalog.rs +++ b/crates/unified-llm/src/catalog.rs @@ -1,8 +1,9 @@ use crate::types::ModelInfo; +use std::sync::LazyLock; /// Built-in model catalog (Section 2.9). /// The catalog is advisory, not restrictive -- unknown model strings pass through. -fn built_in_models() -> Vec { +static BUILT_IN_MODELS: LazyLock> = LazyLock::new(|| { vec![ // === Anthropic === ModelInfo { @@ -99,36 +100,36 @@ fn built_in_models() -> Vec { aliases: vec!["gemini-flash".into()], }, ] -} +}); /// Get model info by model ID (Section 2.9). -#[must_use] +#[must_use] pub fn get_model_info(model_id: &str) -> Option { - built_in_models() - .into_iter() - .find(|m| m.id == model_id || m.aliases.contains(&model_id.to_string())) + BUILT_IN_MODELS + .iter() + .find(|m| m.id == model_id || m.aliases.iter().any(|a| a == model_id)) + .cloned() } /// List all known models, optionally filtered by provider (Section 2.9). -#[must_use] +#[must_use] pub fn list_models(provider: Option<&str>) -> Vec { - let models = built_in_models(); - match provider { - Some(p) => models.into_iter().filter(|m| m.provider == p).collect(), - None => models, - } + provider.map_or_else( + || BUILT_IN_MODELS.clone(), + |p| BUILT_IN_MODELS.iter().filter(|m| m.provider == p).cloned().collect(), + ) } /// Get the latest/best model for a provider, optionally filtered by capability (Section 2.9). -#[must_use] +#[must_use] pub fn get_latest_model(provider: &str, capability: Option<&str>) -> Option { - let models = list_models(Some(provider)); + let mut models = BUILT_IN_MODELS.iter().filter(|m| m.provider == provider); match capability { - Some("reasoning") => models.into_iter().find(|m| m.supports_reasoning), - Some("vision") => models.into_iter().find(|m| m.supports_vision), - Some("tools") => models.into_iter().find(|m| m.supports_tools), - _ => models.into_iter().next(), + Some("reasoning") => models.find(|m| m.supports_reasoning).cloned(), + Some("vision") => models.find(|m| m.supports_vision).cloned(), + Some("tools") => models.find(|m| m.supports_tools).cloned(), + _ => models.next().cloned(), } } diff --git a/crates/unified-llm/src/error.rs b/crates/unified-llm/src/error.rs index 66396c7ad..05030ae1b 100644 --- a/crates/unified-llm/src/error.rs +++ b/crates/unified-llm/src/error.rs @@ -14,12 +14,12 @@ pub enum ProviderErrorKind { impl std::fmt::Display for ProviderErrorKind { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::Authentication => write!(f, "Authentication error"), - Self::AccessDenied => write!(f, "Access denied"), - Self::NotFound => write!(f, "Not found"), + Self::Authentication => write!(f, "Authentication error for"), + Self::AccessDenied => write!(f, "Access denied by"), + Self::NotFound => write!(f, "Not found on"), Self::InvalidRequest => write!(f, "Invalid request to"), Self::RateLimit => write!(f, "Rate limited by"), - Self::Server => write!(f, "Server error"), + Self::Server => write!(f, "Server error from"), Self::ContentFilter => write!(f, "Content filtered by"), Self::ContextLength => write!(f, "Context length exceeded for"), Self::QuotaExceeded => write!(f, "Quota exceeded for"), @@ -402,7 +402,7 @@ mod tests { }; assert_eq!( err.to_string(), - "Authentication error openai: invalid api key" + "Authentication error for openai: invalid api key" ); let err = SdkError::Configuration { diff --git a/crates/unified-llm/src/generate.rs b/crates/unified-llm/src/generate.rs index 2c08235bc..e755ce8e6 100644 --- a/crates/unified-llm/src/generate.rs +++ b/crates/unified-llm/src/generate.rs @@ -4,8 +4,8 @@ use crate::provider::StreamEventStream; use crate::retry::retry; use crate::tools::{execute_all_tools, Tool}; use crate::types::{ - FinishReason, GenerateResult, Message, Request, Response, ResponseFormat, RetryPolicy, - StepResult, StreamEvent, ToolCall, ToolChoice, ToolDefinition, Usage, + FinishReason, GenerateResult, Message, Request, Response, ResponseFormat, ResponseFormatType, + RetryPolicy, StepResult, StreamEvent, ToolCall, ToolChoice, ToolDefinition, Usage, }; use std::sync::Arc; use tokio::sync::OnceCell; @@ -47,13 +47,13 @@ fn build_initial_messages(params: &GenerateParams) -> Result, SdkEr fn build_request( params: &GenerateParams, messages: &[Message], - tool_definitions: Option<&Vec>, + tool_definitions: Option<&[ToolDefinition]>, ) -> Request { Request { model: params.model.clone(), messages: messages.to_vec(), provider: params.provider.clone(), - tools: tool_definitions.cloned(), + tools: tool_definitions.map(<[ToolDefinition]>::to_vec), tool_choice: params.tool_choice.clone(), response_format: params.response_format.clone(), temperature: params.temperature, @@ -67,25 +67,14 @@ fn build_request( } fn build_generate_result(steps: Vec, total_usage: Usage) -> GenerateResult { - let last_idx = steps.len() - 1; - let text = steps[last_idx].text.clone(); - let reasoning = steps[last_idx].reasoning.clone(); - let tool_calls = steps[last_idx].tool_calls.clone(); - let tool_results = steps[last_idx].tool_results.clone(); - let finish_reason = steps[last_idx].finish_reason.clone(); - let usage = steps[last_idx].usage.clone(); - let response = steps[last_idx].response.clone(); - + let last = steps.last().expect("steps should not be empty"); + let response = last.response.clone(); + let tool_results = last.tool_results.clone(); GenerateResult { - text, - reasoning, - tool_calls, + response, tool_results, - finish_reason, - usage, total_usage, steps, - response, output: None, } } @@ -124,7 +113,7 @@ pub async fn generate(params: GenerateParams) -> Result Result= max_tool_rounds || tool_results.is_empty() { - break; + let should_continue = !tool_calls.is_empty() + && response.finish_reason == FinishReason::ToolCalls + && round < max_tool_rounds + && !tool_results.is_empty(); + + if should_continue { + messages.push(response.message.clone()); + for result in &tool_results { + messages.push(Message::tool_result( + &result.tool_call_id, + result.content.to_string(), + result.is_error, + )); + } } - messages.push(response.message.clone()); - for result in &tool_results { - messages.push(Message::tool_result( - &result.tool_call_id, - result.content.to_string(), - result.is_error, - )); + steps.push(StepResult { + response, + tool_results, + }); + + if !should_continue { + break; } round += 1; @@ -361,7 +347,7 @@ pub async fn stream_generate(params: GenerateParams) -> Result Result { let params = GenerateParams { response_format: Some(ResponseFormat { - r#type: "json_schema".to_string(), + kind: ResponseFormatType::JsonSchema, json_schema: Some(schema), strict: true, }), @@ -387,7 +373,7 @@ pub async fn generate_object( let mut result = generate(params).await?; // Try to parse the text as JSON - match serde_json::from_str::(&result.text) { + match serde_json::from_str::(&result.text()) { Ok(parsed) => { result.output = Some(parsed); Ok(result) @@ -422,7 +408,6 @@ mod tests { } } - #[allow(clippy::unnecessary_literal_bound)] #[async_trait::async_trait] impl ProviderAdapter for MockProvider { fn name(&self) -> &str { @@ -505,9 +490,9 @@ mod tests { .await .unwrap(); - assert_eq!(result.text, "Hi there!"); - assert_eq!(result.finish_reason, FinishReason::Stop); - assert_eq!(result.usage.input_tokens, 10); + assert_eq!(result.text(), "Hi there!"); + assert_eq!(*result.finish_reason(), FinishReason::Stop); + assert_eq!(result.usage().input_tokens, 10); assert_eq!(result.steps.len(), 1); } @@ -522,7 +507,7 @@ mod tests { .await .unwrap(); - assert_eq!(result.text, "Greetings!"); + assert_eq!(result.text(), "Greetings!"); } #[tokio::test] @@ -539,7 +524,7 @@ mod tests { .await .unwrap(); - assert_eq!(result.text, "I'm doing well!"); + assert_eq!(result.text(), "I'm doing well!"); } #[tokio::test] @@ -565,7 +550,6 @@ mod tests { call_count: Arc, } - #[allow(clippy::unnecessary_literal_bound)] #[async_trait::async_trait] impl ProviderAdapter for ToolCallMockProvider { fn name(&self) -> &str { @@ -664,7 +648,7 @@ mod tests { .await .unwrap(); - assert_eq!(result.text, "The weather in SF is 72F"); + assert_eq!(result.text(), "The weather in SF is 72F"); assert_eq!(result.steps.len(), 2); assert_eq!(result.total_usage.input_tokens, 30); assert_eq!(result.total_usage.output_tokens, 15); diff --git a/crates/unified-llm/src/providers/anthropic.rs b/crates/unified-llm/src/providers/anthropic.rs index 4b348bccc..242c6d0aa 100644 --- a/crates/unified-llm/src/providers/anthropic.rs +++ b/crates/unified-llm/src/providers/anthropic.rs @@ -1,17 +1,17 @@ -use crate::error::{error_from_status_code, SdkError}; +use crate::error::SdkError; use crate::provider::{ProviderAdapter, StreamEventStream}; +use crate::providers::common::{extract_system_prompt, send_and_read_body, ApiMessage}; use crate::types::{ ContentPart, FinishReason, Message, Request, Response, Role, ToolCall, Usage, }; /// Provider adapter for the Anthropic Messages API. -#[allow(clippy::module_name_repetitions)] -pub struct AnthropicAdapter { +pub struct Adapter { api_key: String, client: reqwest::Client, } -impl AnthropicAdapter { +impl Adapter { #[must_use] pub fn new(api_key: impl Into) -> Self { Self { @@ -23,12 +23,6 @@ impl AnthropicAdapter { // --- Request types --- -#[derive(serde::Serialize)] -struct ApiMessage { - role: String, - content: String, -} - #[derive(serde::Serialize)] struct ApiRequest { model: String, @@ -73,7 +67,7 @@ fn map_finish_reason(stop_reason: Option<&str>) -> FinishReason { fn parse_content_block(block: &serde_json::Value) -> Option { match block.get("type")?.as_str()? { "text" => Some(ContentPart::text(block.get("text")?.as_str()?)), - "tool_use" => Some(ContentPart::tool_call(ToolCall::new( + "tool_use" => Some(ContentPart::ToolCall(ToolCall::new( block.get("id")?.as_str()?, block.get("name")?.as_str()?, block.get("input")?.clone(), @@ -82,58 +76,29 @@ fn parse_content_block(block: &serde_json::Value) -> Option { } } -fn parse_error_body(body: &str) -> (String, Option, Option) { - serde_json::from_str::(body).map_or_else( - |_| (body.to_string(), None, None), - |v| { - let message = v - .get("error") - .and_then(|e| e.get("message")) - .and_then(serde_json::Value::as_str) - .unwrap_or("Unknown error") - .to_string(); - let error_code = v - .get("error") - .and_then(|e| e.get("type")) - .and_then(serde_json::Value::as_str) - .map(String::from); - (message, error_code, Some(v)) - }, - ) -} - #[allow(clippy::unnecessary_literal_bound)] #[async_trait::async_trait] -impl ProviderAdapter for AnthropicAdapter { +impl ProviderAdapter for Adapter { fn name(&self) -> &str { "anthropic" } async fn complete(&self, request: &Request) -> Result { - let mut system_parts = Vec::new(); - let mut api_messages = Vec::new(); + let (system, other_messages) = extract_system_prompt(&request.messages); - for msg in &request.messages { - if msg.role == Role::System { - system_parts.push(msg.text()); - } else { - let role = if msg.role == Role::Assistant { - "assistant" - } else { - "user" + let api_messages: Vec = other_messages + .iter() + .map(|msg| { + let role = match msg.role { + Role::Assistant => "assistant", + Role::System | Role::User | Role::Tool | Role::Developer => "user", }; - api_messages.push(ApiMessage { + ApiMessage { role: role.to_string(), content: msg.text(), - }); - } - } - - let system = if system_parts.is_empty() { - None - } else { - Some(system_parts.join("\n")) - }; + } + }) + .collect(); let api_request = ApiRequest { model: request.model.clone(), @@ -145,37 +110,16 @@ impl ProviderAdapter for AnthropicAdapter { stop_sequences: request.stop_sequences.clone(), }; - let http_resp = self - .client - .post("https://api.anthropic.com/v1/messages") - .header("x-api-key", &self.api_key) - .header("anthropic-version", "2023-06-01") - .json(&api_request) - .send() - .await - .map_err(|e| SdkError::Network { - message: e.to_string(), - })?; - - let status = http_resp.status(); - let body = http_resp - .text() - .await - .map_err(|e| SdkError::Network { - message: e.to_string(), - })?; - - if !status.is_success() { - let (msg, code, raw) = parse_error_body(&body); - return Err(error_from_status_code( - status.as_u16(), - msg, - "anthropic".to_string(), - code, - raw, - None, - )); - } + let body = send_and_read_body( + self.client + .post("https://api.anthropic.com/v1/messages") + .header("x-api-key", &self.api_key) + .header("anthropic-version", "2023-06-01") + .json(&api_request), + "anthropic", + "type", + ) + .await?; let api_resp: ApiResponse = serde_json::from_str(&body).map_err(|e| SdkError::Network { diff --git a/crates/unified-llm/src/providers/common.rs b/crates/unified-llm/src/providers/common.rs new file mode 100644 index 000000000..3c9b70a96 --- /dev/null +++ b/crates/unified-llm/src/providers/common.rs @@ -0,0 +1,95 @@ +use crate::error::{error_from_status_code, SdkError}; +use crate::types::{Message, Role}; + +#[derive(serde::Serialize)] +pub struct ApiMessage { + pub role: String, + pub content: String, +} + +/// Parse an error response body, extracting the message and error code. +/// `error_code_field` is the JSON field name for the error code (e.g. "type" or "status"). +#[must_use] +pub fn parse_error_body( + body: &str, + error_code_field: &str, +) -> (String, Option, Option) { + serde_json::from_str::(body).map_or_else( + |_| (body.to_string(), None, None), + |v| { + let message = v + .get("error") + .and_then(|e| e.get("message")) + .and_then(serde_json::Value::as_str) + .unwrap_or("Unknown error") + .to_string(); + let error_code = v + .get("error") + .and_then(|e| e.get(error_code_field)) + .and_then(serde_json::Value::as_str) + .map(String::from); + (message, error_code, Some(v)) + }, + ) +} + +/// Send an HTTP request and read the response body, returning an error on non-success status. +/// +/// # Errors +/// +/// Returns `SdkError::Network` on connection failure or `SdkError::Provider` on non-success status. +pub async fn send_and_read_body( + request: reqwest::RequestBuilder, + provider: &str, + error_code_field: &str, +) -> Result { + let http_resp = request + .send() + .await + .map_err(|e| SdkError::Network { + message: e.to_string(), + })?; + + let status = http_resp.status(); + let body = http_resp + .text() + .await + .map_err(|e| SdkError::Network { + message: e.to_string(), + })?; + + if !status.is_success() { + let (msg, code, raw) = parse_error_body(&body, error_code_field); + return Err(error_from_status_code( + status.as_u16(), + msg, + provider.to_string(), + code, + raw, + None, + )); + } + + Ok(body) +} + +/// Extract system messages from a message list, returning the joined system prompt +/// and the remaining non-system messages. +#[must_use] +pub fn extract_system_prompt(messages: &[Message]) -> (Option, Vec<&Message>) { + let mut system_parts = Vec::new(); + let mut other = Vec::new(); + for msg in messages { + if msg.role == Role::System { + system_parts.push(msg.text()); + } else { + other.push(msg); + } + } + let system = if system_parts.is_empty() { + None + } else { + Some(system_parts.join("\n")) + }; + (system, other) +} diff --git a/crates/unified-llm/src/providers/gemini.rs b/crates/unified-llm/src/providers/gemini.rs index 2b12e1dc0..334ad68b8 100644 --- a/crates/unified-llm/src/providers/gemini.rs +++ b/crates/unified-llm/src/providers/gemini.rs @@ -1,18 +1,18 @@ -use crate::error::{error_from_status_code, SdkError}; +use crate::error::SdkError; use crate::provider::{ProviderAdapter, StreamEventStream}; use crate::error::{ProviderErrorDetail, ProviderErrorKind}; +use crate::providers::common::{extract_system_prompt, send_and_read_body}; use crate::types::{ ContentPart, FinishReason, Message, Request, Response, Role, ToolCall, Usage, }; /// Provider adapter for the Google Gemini `generateContent` API. -#[allow(clippy::module_name_repetitions)] -pub struct GeminiAdapter { +pub struct Adapter { api_key: String, client: reqwest::Client, } -impl GeminiAdapter { +impl Adapter { #[must_use] pub fn new(api_key: impl Into) -> Self { Self { @@ -112,7 +112,7 @@ fn parse_part(part: &serde_json::Value) -> Option { .get("args") .cloned() .unwrap_or_else(|| serde_json::Value::Object(serde_json::Map::new())); - return Some(ContentPart::tool_call(ToolCall::new( + return Some(ContentPart::ToolCall(ToolCall::new( uuid::Uuid::new_v4().to_string(), name, args, @@ -121,64 +121,35 @@ fn parse_part(part: &serde_json::Value) -> Option { None } -fn parse_error_body(body: &str) -> (String, Option, Option) { - serde_json::from_str::(body).map_or_else( - |_| (body.to_string(), None, None), - |v| { - let message = v - .get("error") - .and_then(|e| e.get("message")) - .and_then(serde_json::Value::as_str) - .unwrap_or("Unknown error") - .to_string(); - let error_code = v - .get("error") - .and_then(|e| e.get("status")) - .and_then(serde_json::Value::as_str) - .map(String::from); - (message, error_code, Some(v)) - }, - ) -} - -#[allow(clippy::too_many_lines, clippy::unnecessary_literal_bound)] +#[allow(clippy::unnecessary_literal_bound)] #[async_trait::async_trait] -impl ProviderAdapter for GeminiAdapter { +impl ProviderAdapter for Adapter { fn name(&self) -> &str { "gemini" } async fn complete(&self, request: &Request) -> Result { - let mut system_parts = Vec::new(); - let mut contents = Vec::new(); + let (system_text, other_messages) = extract_system_prompt(&request.messages); - for msg in &request.messages { - if msg.role == Role::System { - system_parts.push(Part { - text: msg.text(), - }); - } else { - let role = if msg.role == Role::Assistant { - "model" - } else { - "user" + let system_instruction = system_text.map(|text| SystemInstruction { + parts: vec![Part { text }], + }); + + let contents: Vec = other_messages + .iter() + .map(|msg| { + let role = match msg.role { + Role::Assistant => "model", + Role::System | Role::User | Role::Tool | Role::Developer => "user", }; - contents.push(Content { + Content { role: role.to_string(), parts: vec![Part { text: msg.text(), }], - }); - } - } - - let system_instruction = if system_parts.is_empty() { - None - } else { - Some(SystemInstruction { - parts: system_parts, + } }) - }; + .collect(); let generation_config = GenerationConfig { temperature: request.temperature, @@ -198,35 +169,12 @@ impl ProviderAdapter for GeminiAdapter { request.model, self.api_key ); - let http_resp = self - .client - .post(&url) - .json(&api_request) - .send() - .await - .map_err(|e| SdkError::Network { - message: e.to_string(), - })?; - - let status = http_resp.status(); - let body = http_resp - .text() - .await - .map_err(|e| SdkError::Network { - message: e.to_string(), - })?; - - if !status.is_success() { - let (msg, code, raw) = parse_error_body(&body); - return Err(error_from_status_code( - status.as_u16(), - msg, - "gemini".to_string(), - code, - raw, - None, - )); - } + let body = send_and_read_body( + self.client.post(&url).json(&api_request), + "gemini", + "status", + ) + .await?; let api_resp: ApiResponse = serde_json::from_str(&body).map_err(|e| SdkError::Network { diff --git a/crates/unified-llm/src/providers/mod.rs b/crates/unified-llm/src/providers/mod.rs index 4565304ca..fe19107de 100644 --- a/crates/unified-llm/src/providers/mod.rs +++ b/crates/unified-llm/src/providers/mod.rs @@ -1,7 +1,8 @@ pub mod anthropic; +pub mod common; pub mod gemini; pub mod openai; -pub use anthropic::AnthropicAdapter; -pub use gemini::GeminiAdapter; -pub use openai::OpenAiAdapter; +pub use anthropic::Adapter as AnthropicAdapter; +pub use gemini::Adapter as GeminiAdapter; +pub use openai::Adapter as OpenAiAdapter; diff --git a/crates/unified-llm/src/providers/openai.rs b/crates/unified-llm/src/providers/openai.rs index e52189e95..58360ccd1 100644 --- a/crates/unified-llm/src/providers/openai.rs +++ b/crates/unified-llm/src/providers/openai.rs @@ -1,18 +1,18 @@ -use crate::error::{error_from_status_code, SdkError}; +use crate::error::SdkError; use crate::provider::{ProviderAdapter, StreamEventStream}; use crate::error::{ProviderErrorDetail, ProviderErrorKind}; +use crate::providers::common::{send_and_read_body, ApiMessage}; use crate::types::{ ContentPart, FinishReason, Message, Request, Response, Role, ToolCall, Usage, }; /// Provider adapter for the `OpenAI` Chat Completions API. -#[allow(clippy::module_name_repetitions)] -pub struct OpenAiAdapter { +pub struct Adapter { api_key: String, client: reqwest::Client, } -impl OpenAiAdapter { +impl Adapter { #[must_use] pub fn new(api_key: impl Into) -> Self { Self { @@ -24,12 +24,6 @@ impl OpenAiAdapter { // --- Request types --- -#[derive(serde::Serialize)] -struct ApiMessage { - role: String, - content: String, -} - #[derive(serde::Serialize)] struct ApiRequest { model: String, @@ -96,29 +90,9 @@ fn map_finish_reason(reason: Option<&str>) -> FinishReason { } } -fn parse_error_body(body: &str) -> (String, Option, Option) { - serde_json::from_str::(body).map_or_else( - |_| (body.to_string(), None, None), - |v| { - let message = v - .get("error") - .and_then(|e| e.get("message")) - .and_then(serde_json::Value::as_str) - .unwrap_or("Unknown error") - .to_string(); - let error_code = v - .get("error") - .and_then(|e| e.get("type")) - .and_then(serde_json::Value::as_str) - .map(String::from); - (message, error_code, Some(v)) - }, - ) -} - -#[allow(clippy::too_many_lines, clippy::unnecessary_literal_bound)] +#[allow(clippy::unnecessary_literal_bound)] #[async_trait::async_trait] -impl ProviderAdapter for OpenAiAdapter { +impl ProviderAdapter for Adapter { fn name(&self) -> &str { "openai" } @@ -149,36 +123,15 @@ impl ProviderAdapter for OpenAiAdapter { stop: request.stop_sequences.clone(), }; - let http_resp = self - .client - .post("https://api.openai.com/v1/chat/completions") - .bearer_auth(&self.api_key) - .json(&api_request) - .send() - .await - .map_err(|e| SdkError::Network { - message: e.to_string(), - })?; - - let status = http_resp.status(); - let body = http_resp - .text() - .await - .map_err(|e| SdkError::Network { - message: e.to_string(), - })?; - - if !status.is_success() { - let (msg, code, raw) = parse_error_body(&body); - return Err(error_from_status_code( - status.as_u16(), - msg, - "openai".to_string(), - code, - raw, - None, - )); - } + let body = send_and_read_body( + self.client + .post("https://api.openai.com/v1/chat/completions") + .bearer_auth(&self.api_key) + .json(&api_request), + "openai", + "type", + ) + .await?; let api_resp: ApiResponse = serde_json::from_str(&body).map_err(|e| SdkError::Network { @@ -200,7 +153,7 @@ impl ProviderAdapter for OpenAiAdapter { for tc in tool_calls { let arguments = serde_json::from_str(&tc.function.arguments) .unwrap_or_else(|_| serde_json::json!({})); - content_parts.push(ContentPart::tool_call(ToolCall::new( + content_parts.push(ContentPart::ToolCall(ToolCall::new( &tc.id, &tc.function.name, arguments, diff --git a/crates/unified-llm/src/types.rs b/crates/unified-llm/src/types.rs index 06079f129..9c7a388e2 100644 --- a/crates/unified-llm/src/types.rs +++ b/crates/unified-llm/src/types.rs @@ -47,17 +47,11 @@ pub struct ThinkingData { // --- 5.4 ToolCall / ToolResult --- -fn default_tool_type() -> String { - "function".to_string() -} - #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct ToolCall { pub id: String, pub name: String, pub arguments: serde_json::Value, - #[serde(default = "default_tool_type")] - pub r#type: String, pub raw_arguments: Option, } @@ -71,7 +65,6 @@ impl ToolCall { id: id.into(), name: name.into(), arguments, - r#type: "function".to_string(), raw_arguments: None, } } @@ -104,25 +97,6 @@ impl ContentPart { Self::Text(text.into()) } - #[must_use] - pub const fn image(image: ImageData) -> Self { - Self::Image(image) - } - - #[must_use] - pub const fn tool_call(tool_call: ToolCall) -> Self { - Self::ToolCall(tool_call) - } - - #[must_use] - pub const fn tool_result(tool_result: ToolResult) -> Self { - Self::ToolResult(tool_result) - } - - #[must_use] - pub const fn thinking(thinking: ThinkingData) -> Self { - Self::Thinking(thinking) - } } // --- 3.1 Message --- @@ -190,8 +164,7 @@ impl Message { ContentPart::Text(text) => Some(text.as_str()), _ => None, }) - .collect::>() - .join("") + .collect() } } @@ -281,9 +254,18 @@ impl std::ops::Add for Usage { // --- 3.10 ResponseFormat --- +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ResponseFormatType { + Text, + JsonObject, + JsonSchema, +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct ResponseFormat { - pub r#type: String, + #[serde(rename = "type")] + pub kind: ResponseFormatType, pub json_schema: Option, #[serde(default)] pub strict: bool, @@ -390,7 +372,7 @@ impl Response { #[must_use] pub fn reasoning(&self) -> Option { - let texts: Vec<&str> = self + let reasoning: String = self .message .content .iter() @@ -400,10 +382,10 @@ impl Response { }) .collect(); - if texts.is_empty() { + if reasoning.is_empty() { None } else { - Some(texts.join("")) + Some(reasoning) } } } @@ -562,28 +544,76 @@ impl RetryPolicy { #[derive(Debug, Clone)] pub struct GenerateResult { - pub text: String, - pub reasoning: Option, - pub tool_calls: Vec, + pub response: Response, pub tool_results: Vec, - pub finish_reason: FinishReason, - pub usage: Usage, pub total_usage: Usage, pub steps: Vec, - pub response: Response, pub output: Option, } +impl GenerateResult { + #[must_use] + pub fn text(&self) -> String { + self.response.text() + } + + #[must_use] + pub fn reasoning(&self) -> Option { + self.response.reasoning() + } + + #[must_use] + pub fn tool_calls(&self) -> Vec { + self.response.tool_calls() + } + + #[must_use] + pub const fn finish_reason(&self) -> &FinishReason { + &self.response.finish_reason + } + + #[must_use] + pub const fn usage(&self) -> &Usage { + &self.response.usage + } +} + #[derive(Debug, Clone)] pub struct StepResult { - pub text: String, - pub reasoning: Option, - pub tool_calls: Vec, - pub tool_results: Vec, - pub finish_reason: FinishReason, - pub usage: Usage, pub response: Response, - pub warnings: Vec, + pub tool_results: Vec, +} + +impl StepResult { + #[must_use] + pub fn text(&self) -> String { + self.response.text() + } + + #[must_use] + pub fn reasoning(&self) -> Option { + self.response.reasoning() + } + + #[must_use] + pub fn tool_calls(&self) -> Vec { + self.response.tool_calls() + } + + #[must_use] + pub const fn finish_reason(&self) -> &FinishReason { + &self.response.finish_reason + } + + #[must_use] + pub const fn usage(&self) -> &Usage { + &self.response.usage + } + + #[must_use] + pub fn warnings(&self) -> &[Warning] { + &self.response.warnings + } } #[cfg(test)] @@ -925,7 +955,7 @@ mod tests { #[test] fn content_part_image_constructor() { - let part = ContentPart::image(ImageData { + let part = ContentPart::Image(ImageData { url: Some("https://example.com/img.png".into()), data: None, media_type: None, @@ -935,10 +965,11 @@ mod tests { } #[test] - fn tool_call_default_type() { - let tc: ToolCall = - serde_json::from_str(r#"{"id":"c1","name":"test","arguments":{}}"#).unwrap(); - assert_eq!(tc.r#type, "function"); + fn tool_call_serde_roundtrip() { + let tc = ToolCall::new("c1", "test", serde_json::json!({})); + let json = serde_json::to_string(&tc).unwrap(); + let deserialized: ToolCall = serde_json::from_str(&json).unwrap(); + assert_eq!(deserialized, tc); } #[test] @@ -946,7 +977,6 @@ mod tests { let tc = ToolCall::new("c1", "test", serde_json::json!({})); assert_eq!(tc.id, "c1"); assert_eq!(tc.name, "test"); - assert_eq!(tc.r#type, "function"); assert_eq!(tc.raw_arguments, None); } }