Skip to content

Commit 12e2304

Browse files
committed
fix: strip databricks- prefix for reasoning effort and responses API routing
extract_reasoning_effort and is_responses_model did not account for the databricks- prefix on model names. This caused: - databricks-gpt-5-4-high to hit endpoint databricks-gpt-5-4-high (404) instead of stripping the -high reasoning suffix - databricks-gpt-5.4 variants to miss the Responses API route Fix extract_reasoning_effort to strip provider prefixes (databricks-, goose-) before pattern matching, and fix Databricks is_responses_model and get_endpoint_path similarly. Signed-off-by: Bradley Axen <baxen@squareup.com>
1 parent ee17f24 commit 12e2304

2 files changed

Lines changed: 183 additions & 14 deletions

File tree

crates/goose/src/providers/databricks.rs

Lines changed: 131 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -270,19 +270,25 @@ impl DatabricksProvider {
270270

271271
fn is_responses_model(model_name: &str) -> bool {
272272
let normalized = model_name.to_ascii_lowercase();
273-
normalized.contains("codex")
274-
|| normalized.starts_with("gpt-5-pro")
275-
|| normalized.starts_with("gpt-5.2-pro")
276-
|| normalized.starts_with("gpt-5.4")
273+
let base = normalized
274+
.strip_prefix("databricks-")
275+
.unwrap_or(&normalized);
276+
base.contains("codex")
277+
|| base.starts_with("gpt-5-pro")
278+
|| base.starts_with("gpt-5.2-pro")
279+
|| base.starts_with("gpt-5.4")
277280
}
278281

279282
fn get_endpoint_path(&self, model_name: &str, is_embedding: bool) -> String {
280283
if is_embedding {
281284
"serving-endpoints/text-embedding-3-small/invocations".to_string()
282-
} else if Self::is_responses_model(model_name) {
283-
"serving-endpoints/responses".to_string()
284285
} else {
285-
format!("serving-endpoints/{}/invocations", model_name)
286+
let (clean_name, _) = super::utils::extract_reasoning_effort(model_name);
287+
if Self::is_responses_model(&clean_name) {
288+
"serving-endpoints/responses".to_string()
289+
} else {
290+
format!("serving-endpoints/{}/invocations", clean_name)
291+
}
286292
}
287293
}
288294

@@ -654,4 +660,122 @@ mod tests {
654660
assert!(!DatabricksProvider::is_responses_model("o3-mini"));
655661
assert!(!DatabricksProvider::is_responses_model("claude-sonnet-4"));
656662
}
663+
664+
// --- Bug-fix tests: databricks-prefixed model names ---
665+
666+
#[test]
667+
fn databricks_prefixed_gpt_5_4_is_responses_model() {
668+
assert!(
669+
DatabricksProvider::is_responses_model("databricks-gpt-5.4"),
670+
"databricks-gpt-5.4 should route to the Responses API"
671+
);
672+
}
673+
674+
#[test]
675+
fn databricks_prefixed_gpt_5_4_mini_is_responses_model() {
676+
assert!(DatabricksProvider::is_responses_model(
677+
"databricks-gpt-5.4-mini"
678+
));
679+
}
680+
681+
#[test]
682+
fn databricks_prefixed_gpt_5_pro_is_responses_model() {
683+
assert!(DatabricksProvider::is_responses_model(
684+
"databricks-gpt-5-pro"
685+
));
686+
}
687+
688+
#[test]
689+
fn databricks_prefixed_codex_is_responses_model() {
690+
assert!(DatabricksProvider::is_responses_model(
691+
"databricks-gpt-5-codex"
692+
));
693+
}
694+
695+
#[test]
696+
fn databricks_prefixed_non_responses_model_is_not_matched() {
697+
assert!(!DatabricksProvider::is_responses_model("databricks-gpt-4o"));
698+
assert!(!DatabricksProvider::is_responses_model(
699+
"databricks-claude-sonnet-4"
700+
));
701+
}
702+
703+
// --- Bug-fix tests: reasoning suffix must be stripped from endpoint path ---
704+
705+
#[test]
706+
fn endpoint_path_strips_reasoning_suffix_for_chat_model() {
707+
let provider = DatabricksProvider {
708+
api_client: super::super::api_client::ApiClient::new(
709+
"https://example.com".to_string(),
710+
super::super::api_client::AuthMethod::NoAuth,
711+
)
712+
.unwrap(),
713+
auth: DatabricksAuth::Token("fake".into()),
714+
model: ModelConfig::new_or_fail("databricks-gpt-5-4"),
715+
image_format: ImageFormat::OpenAi,
716+
retry_config: RetryConfig::default(),
717+
fast_retry_config: RetryConfig::new(0, 0, 1.0, 0),
718+
name: "databricks".into(),
719+
token_cache: std::sync::Arc::new(std::sync::Mutex::new(None)),
720+
instance_id: None,
721+
};
722+
723+
// "databricks-gpt-5-4-high" — the "-high" is a reasoning effort suffix,
724+
// not part of the Databricks endpoint name.
725+
let path = provider.get_endpoint_path("databricks-gpt-5-4-high", false);
726+
assert_eq!(
727+
path, "serving-endpoints/databricks-gpt-5-4/invocations",
728+
"reasoning suffix '-high' must be stripped from the endpoint name"
729+
);
730+
}
731+
732+
#[test]
733+
fn endpoint_path_routes_prefixed_responses_model_correctly() {
734+
let provider = DatabricksProvider {
735+
api_client: super::super::api_client::ApiClient::new(
736+
"https://example.com".to_string(),
737+
super::super::api_client::AuthMethod::NoAuth,
738+
)
739+
.unwrap(),
740+
auth: DatabricksAuth::Token("fake".into()),
741+
model: ModelConfig::new_or_fail("databricks-gpt-5.4"),
742+
image_format: ImageFormat::OpenAi,
743+
retry_config: RetryConfig::default(),
744+
fast_retry_config: RetryConfig::new(0, 0, 1.0, 0),
745+
name: "databricks".into(),
746+
token_cache: std::sync::Arc::new(std::sync::Mutex::new(None)),
747+
instance_id: None,
748+
};
749+
750+
let path = provider.get_endpoint_path("databricks-gpt-5.4-high", false);
751+
assert_eq!(
752+
path, "serving-endpoints/responses",
753+
"databricks-gpt-5.4 variants must route to the Responses API"
754+
);
755+
}
756+
757+
#[test]
758+
fn endpoint_path_unchanged_for_non_reasoning_model() {
759+
let provider = DatabricksProvider {
760+
api_client: super::super::api_client::ApiClient::new(
761+
"https://example.com".to_string(),
762+
super::super::api_client::AuthMethod::NoAuth,
763+
)
764+
.unwrap(),
765+
auth: DatabricksAuth::Token("fake".into()),
766+
model: ModelConfig::new_or_fail("databricks-claude-sonnet-4"),
767+
image_format: ImageFormat::OpenAi,
768+
retry_config: RetryConfig::default(),
769+
fast_retry_config: RetryConfig::new(0, 0, 1.0, 0),
770+
name: "databricks".into(),
771+
token_cache: std::sync::Arc::new(std::sync::Mutex::new(None)),
772+
instance_id: None,
773+
};
774+
775+
let path = provider.get_endpoint_path("databricks-claude-sonnet-4", false);
776+
assert_eq!(
777+
path,
778+
"serving-endpoints/databricks-claude-sonnet-4/invocations"
779+
);
780+
}
657781
}

crates/goose/src/providers/utils.rs

Lines changed: 52 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -194,22 +194,32 @@ pub async fn handle_response_google_compat(response: Response) -> Result<Value,
194194
}
195195

196196
pub fn extract_reasoning_effort(model_name: &str) -> (String, Option<String>) {
197-
let is_reasoning_model = model_name.starts_with("o1")
198-
|| model_name.starts_with("o2")
199-
|| model_name.starts_with("o3")
200-
|| model_name.starts_with("o4")
201-
|| model_name.starts_with("gpt-5");
197+
const PROVIDER_PREFIXES: &[&str] = &["goose-", "databricks-"];
198+
199+
let (prefix, base) = PROVIDER_PREFIXES
200+
.iter()
201+
.find_map(|p| model_name.strip_prefix(p).map(|rest| (*p, rest)))
202+
.unwrap_or(("", model_name));
203+
204+
let is_reasoning_model = base.starts_with("o1")
205+
|| base.starts_with("o2")
206+
|| base.starts_with("o3")
207+
|| base.starts_with("o4")
208+
|| base.starts_with("gpt-5");
202209

203210
if !is_reasoning_model {
204211
return (model_name.to_string(), None);
205212
}
206213

207-
let parts: Vec<&str> = model_name.split('-').collect();
214+
let parts: Vec<&str> = base.split('-').collect();
208215
let last_part = parts.last().unwrap();
209216
match *last_part {
210217
"none" | "low" | "medium" | "high" | "xhigh" => {
211218
let base_name = parts[..parts.len() - 1].join("-");
212-
(base_name, Some(last_part.to_string()))
219+
(
220+
format!("{}{}", prefix, base_name),
221+
Some(last_part.to_string()),
222+
)
213223
}
214224
_ => (model_name.to_string(), Some("medium".to_string())),
215225
}
@@ -905,4 +915,39 @@ mod tests {
905915
assert_eq!(name, "gpt-4o");
906916
assert_eq!(effort, None);
907917
}
918+
919+
#[test]
920+
fn test_extract_reasoning_effort_databricks_prefixed_high() {
921+
let (name, effort) = extract_reasoning_effort("databricks-gpt-5-4-high");
922+
assert_eq!(name, "databricks-gpt-5-4");
923+
assert_eq!(effort.as_deref(), Some("high"));
924+
}
925+
926+
#[test]
927+
fn test_extract_reasoning_effort_databricks_prefixed_default() {
928+
let (name, effort) = extract_reasoning_effort("databricks-gpt-5-4");
929+
assert_eq!(name, "databricks-gpt-5-4");
930+
assert_eq!(effort.as_deref(), Some("medium"));
931+
}
932+
933+
#[test]
934+
fn test_extract_reasoning_effort_databricks_prefixed_o3_low() {
935+
let (name, effort) = extract_reasoning_effort("databricks-o3-low");
936+
assert_eq!(name, "databricks-o3");
937+
assert_eq!(effort.as_deref(), Some("low"));
938+
}
939+
940+
#[test]
941+
fn test_extract_reasoning_effort_databricks_non_reasoning() {
942+
let (name, effort) = extract_reasoning_effort("databricks-claude-sonnet-4");
943+
assert_eq!(name, "databricks-claude-sonnet-4");
944+
assert_eq!(effort, None);
945+
}
946+
947+
#[test]
948+
fn test_extract_reasoning_effort_goose_prefixed_high() {
949+
let (name, effort) = extract_reasoning_effort("goose-gpt-5-high");
950+
assert_eq!(name, "goose-gpt-5");
951+
assert_eq!(effort.as_deref(), Some("high"));
952+
}
908953
}

0 commit comments

Comments
 (0)