Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 17 additions & 10 deletions crates/goose/src/providers/databricks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,11 @@ impl DatabricksProvider {
normalized.contains("codex")
}

fn supports_stream_options(model_name: &str) -> bool {
let normalized = model_name.to_ascii_lowercase();
!normalized.contains("gemini")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Detect Gemini endpoints without relying on endpoint name

supports_stream_options() keys off model_config.model_name, but in this provider that value is the Databricks serving endpoint name: get_endpoint_path() interpolates it directly into /serving-endpoints/{name}/invocations, and fetch_supported_models() returns endpoint.name. In deployments where a Gemini-backed endpoint is aliased to something like prod-chat or assistant-v1, this predicate stays true, Goose still sends stream_options, and the request reproduces the same 400 INVALID_ARGUMENT this patch is meant to eliminate.

Useful? React with πŸ‘Β / πŸ‘Ž.

}

fn get_endpoint_path(&self, model_name: &str, is_embedding: bool) -> String {
if is_embedding {
"serving-endpoints/text-embedding-3-small/invocations".to_string()
Expand Down Expand Up @@ -389,16 +394,18 @@ impl Provider for DatabricksProvider {
.unwrap()
.insert("stream".to_string(), Value::Bool(true));

if let Some(opts) = payload
.get_mut("stream_options")
.and_then(|v| v.as_object_mut())
{
opts.entry("include_usage").or_insert(json!(true));
} else {
payload
.as_object_mut()
.unwrap()
.insert("stream_options".to_string(), json!({"include_usage": true}));
if Self::supports_stream_options(&model_config.model_name) {
if let Some(opts) = payload
.get_mut("stream_options")
.and_then(|v| v.as_object_mut())
{
opts.entry("include_usage").or_insert(json!(true));
} else {
payload
.as_object_mut()
.unwrap()
.insert("stream_options".to_string(), json!({"include_usage": true}));
}
}

let mut log = RequestLog::start(model_config, &payload)?;
Expand Down
51 changes: 47 additions & 4 deletions crates/goose/src/providers/formats/openai.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,25 @@ struct DeltaToolCall {
extra: Option<serde_json::Map<String, Value>>,
}

#[derive(Serialize, Deserialize, Debug)]
#[serde(untagged)]
enum DeltaContent {
String(String),
Array(Vec<ContentPart>),
}

#[derive(Serialize, Deserialize, Debug)]
struct ContentPart {
r#type: String,
text: String,
#[serde(rename = "thoughtSignature")]
thought_signature: Option<String>,
}

#[derive(Serialize, Deserialize, Debug)]
struct Delta {
content: Option<String>,
#[serde(default)]
content: Option<DeltaContent>,
role: Option<String>,
tool_calls: Option<Vec<DeltaToolCall>>,
reasoning_details: Option<Vec<Value>>,
Expand All @@ -74,6 +90,32 @@ struct StreamingChunk {
model: Option<String>,
}

fn extract_content_and_signature(
delta_content: Option<&DeltaContent>,
) -> (Option<String>, Option<String>) {
match delta_content {
Some(DeltaContent::String(s)) => (Some(s.clone()), None),
Some(DeltaContent::Array(parts)) => {
let text_parts: Vec<_> = parts.iter().filter(|p| p.r#type == "text").collect();

let text = text_parts
.iter()
.map(|p| p.text.as_str())
.collect::<String>();
Comment on lines +99 to +104

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep thought parts out of visible assistant text

Gemini's OpenAI-compatible chat completions support include_thoughts, and those thought summaries arrive as content parts rather than reasoning_content. This code concatenates every type == "text" part into MessageContent::text, but ContentPart never records whether a part was a thought, so any thought: true segment will be shown and persisted as normal assistant output instead of hidden MessageContent::Thinking. In thought-enabled sessions that leaks the model's internal reasoning to users.

Useful? React with πŸ‘Β / πŸ‘Ž.


let signature = text_parts
.iter()
.find_map(|p| p.thought_signature.as_ref())
Comment on lines +106 to +108

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Use the last thoughtSignature in a streamed content array

Gemini 3 places the continuation signature on the last part of a non-function response, but extract_content_and_signature() keeps the first thoughtSignature it sees. If a single delta.content array contains multiple text parts, last_signature can be set to a stale value, and the next tool call/follow-up turn will be replayed with the wrong signature. That breaks the continuation contract this patch is trying to restore and can reproduce Gemini's missing/invalid thought_signature 400s on later turns.

Useful? React with πŸ‘Β / πŸ‘Ž.

.cloned();

let text = if text.is_empty() { None } else { Some(text) };

(text, signature)
}
None => (None, None),
}
}

pub fn format_messages(messages: &[Message], image_format: &ImageFormat) -> Vec<Value> {
let mut messages_spec = Vec::new();
for message in messages {
Expand Down Expand Up @@ -735,9 +777,11 @@ where
}
}

if let Some(text) = &chunk.choices[0].delta.content {
let (text_content, _thought_signature) = extract_content_and_signature(chunk.choices[0].delta.content.as_ref());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep Gemini thought signatures when parsing content arrays

The new array-content path parses thoughtSignature and then drops it: extract_content_and_signature() flattens all parts into one string, and response_to_streaming_message() ignores the returned signature entirely. That makes the first streamed turn succeed, but the assistant Message no longer contains the signature-bearing part, so the next request cannot replay Gemini's saved reasoning state. The native Google formatter in this repo preserves these signatures for follow-up turns/tool loops, so this OpenAI-compatible path will still lose context after the initial response.

Useful? React with πŸ‘Β / πŸ‘Ž.


if let Some(text) = text_content {
if !text.is_empty() {
content.push(MessageContent::text(text));
content.push(MessageContent::text(&text));
Comment on lines +791 to +799

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reuse a stable id when array-content chunks omit id

Databricks/Gemini chunks on this new DeltaContent::Array path can have "id": null (the example in this commit does), and ui/desktop/src/hooks/useChatStream.ts:178-212 only coalesces streamed deltas when consecutive messages share an id. Because these array-content chunks now flow through as separate assistant messages with no fallback id, the desktop stream renders one timestamped bubble per delta instead of extending a single reply, which matches the regression called out in the commit message.

Useful? React with πŸ‘Β / πŸ‘Ž.

}
Comment on lines +797 to 800

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve thought signatures on plain-text assistant turns

This branch updates last_signature and then downgrades the signed Gemini part to MessageContent::Text, which means the signature is discarded unless a tool call happens later in the same response. In format_messages() assistant text is serialized back as bare text, so a successful first reply from Gemini 3 cannot be appended "including thought signatures" on the next request. For text-only chats that turns this fix into a one-turn success followed by lost reasoning context or another thought_signature validation error on the next turn.

Useful? React with πŸ‘Β / πŸ‘Ž.

}

Expand All @@ -748,7 +792,6 @@ where
content,
);

// Add ID if present
if let Some(id) = chunk.id {
msg = msg.with_id(id);
}
Expand Down
Loading