-
Notifications
You must be signed in to change notification settings - Fork 6k
Override session name generator for ollama provider #3710
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,8 +4,10 @@ use super::utils::{get_model, handle_response_openai_compat}; | |
| use crate::message::Message; | ||
| use crate::model::ModelConfig; | ||
| use crate::providers::formats::openai::{create_request, get_usage, response_to_message}; | ||
| use crate::utils::safe_truncate; | ||
| use anyhow::Result; | ||
| use async_trait::async_trait; | ||
| use regex::Regex; | ||
| use reqwest::Client; | ||
| use rmcp::model::Tool; | ||
| use serde_json::Value; | ||
|
|
@@ -154,4 +156,77 @@ impl Provider for OllamaProvider { | |
| super::utils::emit_debug_trace(&self.model, &payload, &response, &usage); | ||
| Ok((message, ProviderUsage::new(model, usage))) | ||
| } | ||
|
|
||
| /// Generate a session name based on the conversation history | ||
| /// This override filters out reasoning tokens that some Ollama models produce | ||
| async fn generate_session_name(&self, messages: &[Message]) -> Result<String, ProviderError> { | ||
| let message = Message::user().with_text(&self.create_session_name_prompt(messages)); | ||
| let result = self | ||
| .complete( | ||
| "You are a title generator. Output only the requested title with no additional text, reasoning, or explanations.", | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't we tell it to keep it to 4 words here too?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we tell it in the prompt already. I can also add it here. |
||
| &[message], | ||
| &[], | ||
| ) | ||
| .await?; | ||
|
|
||
| let mut description = result.0.as_concat_text(); | ||
| description = Self::filter_reasoning_tokens(&description); | ||
|
|
||
| let sanitized_description = if description.chars().count() > 100 { | ||
| safe_truncate(&description, 100) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. safe_truncate already does check for length, so need to do that here. good to use chars().count) though
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ahh looks like we do that in the original code too. Let me clean both up |
||
| } else { | ||
| description | ||
| }; | ||
|
|
||
| // If the description is too long, the model failed to give us a short description, provide a fallback instead | ||
| if sanitized_description.split_whitespace().count() > 6 { | ||
|
angelahning marked this conversation as resolved.
Outdated
|
||
| Ok("Ollama Chat Session".to_string()) | ||
| } else { | ||
| Ok(sanitized_description) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl OllamaProvider { | ||
| /// Filter out reasoning tokens and thinking patterns from model responses | ||
| fn filter_reasoning_tokens(text: &str) -> String { | ||
| let mut filtered = text.to_string(); | ||
|
|
||
| // Remove common reasoning patterns | ||
| let reasoning_patterns = [ | ||
| r"<think>.*?</think>", | ||
| r"<thinking>.*?</thinking>", | ||
| r"Let me think.*?\n", | ||
| r"I need to.*?\n", | ||
| r"First, I.*?\n", | ||
| r"Okay, .*?\n", | ||
| r"So, .*?\n", | ||
| r"Well, .*?\n", | ||
| r"Hmm, .*?\n", | ||
| r"Actually, .*?\n", | ||
| r"Based on.*?I think", | ||
| r"Looking at.*?I would say", | ||
| ]; | ||
|
|
||
| for pattern in reasoning_patterns { | ||
| if let Ok(re) = Regex::new(pattern) { | ||
| filtered = re.replace_all(&filtered, "").to_string(); | ||
| } | ||
| } | ||
| // Remove any remaining thinking markers | ||
| filtered = filtered | ||
| .replace("<think>", "") | ||
| .replace("</think>", "") | ||
| .replace("<thinking>", "") | ||
| .replace("</thinking>", ""); | ||
| // Clean up extra whitespace | ||
| filtered = filtered | ||
| .lines() | ||
| .map(|line| line.trim()) | ||
| .filter(|line| !line.is_empty()) | ||
| .collect::<Vec<_>>() | ||
| .join(" "); | ||
|
|
||
| filtered | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we pass this in, instead of messages? the outer callers are only going to call us with the first three messages anyway (which I guess also means we should probably not cut at 3 here at all)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure.