diff --git a/crates/language_models/src/provider/ollama.rs b/crates/language_models/src/provider/ollama.rs index 30234687633215ec6a1da6f9d63ea136d08254b8..551fcd55358c11bdf64bf2f27b32fa9a7f702252 100644 --- a/crates/language_models/src/provider/ollama.rs +++ b/crates/language_models/src/provider/ollama.rs @@ -400,7 +400,14 @@ impl OllamaLanguageModel { stream: true, options: Some(ChatOptions { num_ctx: Some(self.model.max_tokens), - stop: Some(request.stop), + // Only send stop tokens if explicitly provided. When empty/None, + // Ollama will use the model's default stop tokens from its Modelfile. + // Sending an empty array would override and disable the defaults. + stop: if request.stop.is_empty() { + None + } else { + Some(request.stop) + }, temperature: request.temperature.or(Some(1.0)), ..Default::default() }), diff --git a/crates/ollama/src/ollama.rs b/crates/ollama/src/ollama.rs index dd439bc5d690c308828ca7be491efdf751a9a09c..bae8212d34891a79107c42cb445088a55fbf3f4f 100644 --- a/crates/ollama/src/ollama.rs +++ b/crates/ollama/src/ollama.rs @@ -123,10 +123,15 @@ pub struct ChatRequest { // https://github.com/ollama/ollama/blob/main/docs/modelfile.md#valid-parameters-and-values #[derive(Serialize, Default, Debug)] pub struct ChatOptions { + #[serde(skip_serializing_if = "Option::is_none")] pub num_ctx: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub num_predict: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub stop: Option>, + #[serde(skip_serializing_if = "Option::is_none")] pub temperature: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub top_p: Option, } @@ -588,4 +593,96 @@ mod tests { assert_eq!(message_images.len(), 1); assert_eq!(message_images[0].as_str().unwrap(), base64_image); } + + #[test] + fn test_chat_options_serialization() { + // When stop is None, it should not appear in JSON at all + // This allows Ollama to use the model's default stop tokens + let options_no_stop = ChatOptions { + num_ctx: Some(4096), + stop: None, + temperature: Some(0.7), + ..Default::default() + }; + let serialized = serde_json::to_string(&options_no_stop).unwrap(); + assert!( + !serialized.contains("stop"), + "stop should not be in JSON when None" + ); + assert!(serialized.contains("num_ctx")); + assert!(serialized.contains("temperature")); + + // When stop has values, they should be serialized + let options_with_stop = ChatOptions { + stop: Some(vec!["<|eot_id|>".to_string()]), + ..Default::default() + }; + let serialized = serde_json::to_string(&options_with_stop).unwrap(); + assert!(serialized.contains("stop")); + assert!(serialized.contains("<|eot_id|>")); + + // All None options should result in empty object + let options_all_none = ChatOptions::default(); + let serialized = serde_json::to_string(&options_all_none).unwrap(); + assert_eq!(serialized, "{}"); + } + + #[test] + fn test_chat_request_with_stop_tokens() { + let request = ChatRequest { + model: "rnj-1:8b".to_string(), + messages: vec![ChatMessage::User { + content: "Hello".to_string(), + images: None, + }], + stream: true, + keep_alive: KeepAlive::default(), + options: Some(ChatOptions { + stop: Some(vec!["<|eot_id|>".to_string(), "<|end|>".to_string()]), + ..Default::default() + }), + think: None, + tools: vec![], + }; + + let serialized = serde_json::to_string(&request).unwrap(); + let parsed: serde_json::Value = serde_json::from_str(&serialized).unwrap(); + + let stop = parsed["options"]["stop"].as_array().unwrap(); + assert_eq!(stop.len(), 2); + assert_eq!(stop[0].as_str().unwrap(), "<|eot_id|>"); + assert_eq!(stop[1].as_str().unwrap(), "<|end|>"); + } + + #[test] + fn test_chat_request_without_stop_tokens_omits_field() { + // This tests the fix for issue #47798 + // When no stop tokens are provided, the field should be omitted + // so Ollama uses the model's default stop tokens from Modelfile + let request = ChatRequest { + model: "rnj-1:8b".to_string(), + messages: vec![ChatMessage::User { + content: "Hello".to_string(), + images: None, + }], + stream: true, + keep_alive: KeepAlive::default(), + options: Some(ChatOptions { + num_ctx: Some(4096), + stop: None, // No stop tokens - should be omitted from JSON + ..Default::default() + }), + think: None, + tools: vec![], + }; + + let serialized = serde_json::to_string(&request).unwrap(); + + // The key check: "stop" should not appear in the serialized JSON + assert!( + !serialized.contains("\"stop\""), + "stop field should be omitted when None, got: {}", + serialized + ); + } }