fix(chat): process openai request based on model name, drop penalty if is reasoning models (#4049)

Author: zwpaper

crates/http-api-bindings/src/chat/mod.rs

@@ -33,13 +33,10 @@ pub async fn create(model: &HttpModelConfig) -> Arc<dyn ChatCompletionStream> {
             let mut builder = ExtendedOpenAIConfig::builder();
             builder
                 .base(config)
+                .kind(model.kind.clone())
                 .supported_models(model.supported_models.clone())
                 .model_name(model.model_name.as_deref().expect("Model name is required"));
 
-            if model.kind == "mistral/chat" {
-                builder.fields_to_remove(ExtendedOpenAIConfig::mistral_fields_to_remove());
-            }
-
             Box::new(
                 async_openai_alt::Client::with_config(
                     builder.build().expect("Failed to build config"),

crates/tabby-inference/src/chat.rs

@@ -22,38 +22,25 @@ pub trait ChatCompletionStream: Sync + Send {
     ) -> Result<ChatCompletionResponseStream, OpenAIError>;
 }
 
-#[derive(Clone)]
-pub enum OpenAIRequestFieldEnum {
-    PresencePenalty,
-    User,
-}
-
 #[derive(Builder, Clone)]
 pub struct ExtendedOpenAIConfig {
+    #[builder(default)]
+    kind: String,
+
     base: OpenAIConfig,
 
     #[builder(setter(into))]
     model_name: String,
 
     #[builder(setter(into))]
     supported_models: Option<Vec<String>>,
-
-    #[builder(default)]
-    fields_to_remove: Vec<OpenAIRequestFieldEnum>,
 }
 
 impl ExtendedOpenAIConfig {
     pub fn builder() -> ExtendedOpenAIConfigBuilder {
         ExtendedOpenAIConfigBuilder::default()
     }
 
-    pub fn mistral_fields_to_remove() -> Vec<OpenAIRequestFieldEnum> {
-        vec![
-            OpenAIRequestFieldEnum::PresencePenalty,
-            OpenAIRequestFieldEnum::User,
-        ]
-    }
-
     fn process_request(
         &self,
         mut request: CreateChatCompletionRequest,
@@ -70,21 +57,33 @@ impl ExtendedOpenAIConfig {
             }
         }
 
-        for field in &self.fields_to_remove {
-            match field {
-                OpenAIRequestFieldEnum::PresencePenalty => {
-                    request.presence_penalty = None;
-                }
-                OpenAIRequestFieldEnum::User => {
-                    request.user = None;
-                }
+        match self.kind.as_str() {
+            "mistral/chat" => {
+                request.presence_penalty = None;
+                request.user = None;
+            }
+            "openai/chat" => {
+                request = process_request_openai(request);
             }
+            _ => {}
         }
 
         request
     }
 }
 
+fn process_request_openai(request: CreateChatCompletionRequest) -> CreateChatCompletionRequest {
+    let mut request = request;
+
+    // Check for specific O-series model prefixes
+    if request.model.starts_with("o1") || request.model.starts_with("o3-mini") {
+        request.presence_penalty = None;
+        request.frequency_penalty = None;
+    }
+
+    request
+}
+
 impl async_openai_alt::config::Config for ExtendedOpenAIConfig {
     fn headers(&self) -> reqwest::header::HeaderMap {
         self.base.headers()
@@ -132,13 +131,15 @@ impl ChatCompletionStream for async_openai_alt::Client<async_openai_alt::config:
         &self,
         request: CreateChatCompletionRequest,
     ) -> Result<CreateChatCompletionResponse, OpenAIError> {
+        let request = process_request_openai(request);
         self.chat().create(request).await
     }
 
     async fn chat_stream(
         &self,
         request: CreateChatCompletionRequest,
     ) -> Result<ChatCompletionResponseStream, OpenAIError> {
+        let request = process_request_openai(request);
         self.chat().create_stream(request).await
     }
 }