-
Notifications
You must be signed in to change notification settings - Fork 306
add openai chatgpt reasoning effort mapping from config #410
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
1f0ba38
e39f942
8a9c6d1
d255715
2271017
987774f
47b9eec
7b9e3ac
6376fe8
cbaa0ef
dd1b053
1f5bfb5
cd85a42
c2514c2
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -545,18 +545,33 @@ impl CompletionModel for SpacebotModel { | |||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| impl SpacebotModel { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn configured_thinking_effort(&self) -> &str { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| let Some(routing) = self.routing.as_ref() else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return "auto"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| if let Some(process_type) = self.process_type.as_deref() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return match process_type { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| "channel" => routing.channel_thinking_effort.as_str(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| "branch" => routing.branch_thinking_effort.as_str(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| "worker" => routing.worker_thinking_effort.as_str(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| "compactor" => routing.compactor_thinking_effort.as_str(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| "cortex" => routing.cortex_thinking_effort.as_str(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| _ => routing.thinking_effort_for_model(&self.model_name), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| routing.thinking_effort_for_model(&self.model_name) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+558
to
+570
Contributor
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. Looks like
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| async fn call_anthropic( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| &self, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| request: CompletionRequest, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| provider_config: &ProviderConfig, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> Result<completion::CompletionResponse<RawResponse>, CompletionError> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| let api_key = provider_config.api_key.as_str(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| let effort = self | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| .routing | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| .as_ref() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| .map(|r| r.thinking_effort_for_model(&self.model_name)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| .unwrap_or("auto"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| let effort = self.configured_thinking_effort(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| let anthropic_request = crate::llm::anthropic::build_anthropic_request( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.llm_manager.http_client(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| api_key, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -766,6 +781,12 @@ impl SpacebotModel { | |||||||||||||||||||||||||||||||||||||||||||||||||||
| body["stream"] = serde_json::json!(true); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| if self.provider == "openai-chatgpt" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| && let Some(effort) = map_openai_reasoning_effort(self.configured_thinking_effort()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| body["reasoning"] = serde_json::json!({ "effort": effort }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| if !request.tools.is_empty() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| let tools: Vec<serde_json::Value> = request | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| .tools | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -2859,6 +2880,16 @@ fn provider_display_name(provider_id: &str) -> String { | |||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn map_openai_reasoning_effort(config_effort: &str) -> Option<&'static str> { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| match config_effort.trim().to_ascii_lowercase().as_str() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| "auto" => None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| "max" | "high" => Some("high"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| "medium" => Some("medium"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| "low" | "minimal" => Some("low"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| _ => None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+2938
to
+2945
Contributor
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. Keep the OpenAI mapper model-aware. This helper now collapses Proposed fix-fn map_openai_reasoning_effort(config_effort: &str) -> Option<&'static str> {
- match config_effort.trim().to_ascii_lowercase().as_str() {
+fn map_openai_reasoning_effort(model_name: &str, config_effort: &str) -> Option<&'static str> {
+ let normalized_model_name = model_name.trim().to_ascii_lowercase();
+ let is_gpt_5_4_pro = normalized_model_name.starts_with("gpt-5.4-pro");
+ let is_gpt_5_4_family = normalized_model_name.starts_with("gpt-5.4");
+
+ match config_effort.trim().to_ascii_lowercase().as_str() {
"auto" => None,
- "max" | "high" => Some("high"),
+ "max" if is_gpt_5_4_family => Some("xhigh"),
+ "high" | "max" => Some("high"),
"medium" => Some("medium"),
- "low" | "minimal" => Some("low"),
+ "low" | "minimal" if is_gpt_5_4_pro => Some("medium"),
+ "low" | "minimal" => Some("low"),
_ => None,
}
}Update the call site accordingly: - if self.provider == "openai-chatgpt"
- && let Some(effort) = map_openai_reasoning_effort(self.configured_thinking_effort())
+ if self.provider == "openai-chatgpt"
+ && let Some(effort) =
+ map_openai_reasoning_effort(&self.model_name, self.configured_thinking_effort())
{
body["reasoning"] = serde_json::json!({ "effort": effort });
}Based on learnings: In 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn remap_model_name_for_api(provider: &str, model_name: &str) -> String { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if provider == "zai-coding-plan" { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Coding Plan endpoint expects plain model ids (e.g. "glm-5"). | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -2916,6 +2947,18 @@ mod tests { | |||||||||||||||||||||||||||||||||||||||||||||||||||
| panic!("expected ToolCall"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[test] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn map_openai_reasoning_effort_maps_values() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert_eq!(map_openai_reasoning_effort("auto"), None); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert_eq!(map_openai_reasoning_effort("max"), Some("high")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert_eq!(map_openai_reasoning_effort("high"), Some("high")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert_eq!(map_openai_reasoning_effort("medium"), Some("medium")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert_eq!(map_openai_reasoning_effort("low"), Some("low")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert_eq!(map_openai_reasoning_effort("minimal"), Some("low")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert_eq!(map_openai_reasoning_effort("invalid"), None); | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[test] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn coding_plan_model_name_uses_plain_glm_id() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert_eq!( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
Align the documented mapping with the runtime contract.
This section now documents
max -> highfor allopenai-chatgpt/*models and omitsminimalentirely. That will mislead users: the runtime already treatsminimalas an alias, and GPT-5.4-family models need model-specific normalization rather than the genericmax -> highrule. Please either document those exceptions here or tighten the parser to match the docs exactly.Based on learnings: In
src/llm/model.rs, GPT-5.4-family models intentionally supportxhigh, andgpt-5.4-prorequires model-specific normalization rather than the generic low/medium/high mapping.🤖 Prompt for AI Agents