Skip to content
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

fix: validate function call json schemas for openai #1156

Merged
merged 9 commits into from
Feb 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
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
130 changes: 129 additions & 1 deletion crates/goose/src/providers/formats/openai.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,55 @@ pub fn get_usage(data: &Value) -> Result<Usage, ProviderError> {
Ok(Usage::new(input_tokens, output_tokens, total_tokens))
}

/// Validates and fixes tool schemas to ensure they have proper parameter structure.
/// If parameters exist, ensures they have properties and required fields, or removes parameters entirely.
pub fn validate_tool_schemas(tools: &mut [Value]) {
for tool in tools.iter_mut() {
if let Some(function) = tool.get_mut("function") {
if let Some(parameters) = function.get_mut("parameters") {
wendytang marked this conversation as resolved.
Show resolved Hide resolved
if parameters.is_object() {
ensure_valid_json_schema(parameters);
}
}
}
}
}

/// Ensures that the given JSON value follows the expected JSON Schema structure.
fn ensure_valid_json_schema(schema: &mut Value) {
if let Some(params_obj) = schema.as_object_mut() {
// Check if this is meant to be an object type schema
let is_object_type = params_obj
.get("type")
.and_then(|t| t.as_str())
.map_or(true, |t| t == "object"); // Default to true if no type is specified

// Only apply full schema validation to object types
if is_object_type {
// Ensure required fields exist with default values
params_obj.entry("properties").or_insert_with(|| json!({}));
params_obj.entry("required").or_insert_with(|| json!([]));
params_obj.entry("type").or_insert_with(|| json!("object"));

// Recursively validate properties if it exists
if let Some(properties) = params_obj.get_mut("properties") {
if let Some(properties_obj) = properties.as_object_mut() {
for (_key, prop) in properties_obj.iter_mut() {
if prop.is_object()
&& prop
.get("type")
.and_then(|t| t.as_str())
.map_or(false, |t| t == "object")
{
ensure_valid_json_schema(prop);
Copy link
Collaborator Author

@wendytang wendytang Feb 10, 2025

Choose a reason for hiding this comment

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

we could put a recursive counter here, but I don't think extensions are going to have massively nested json schemas

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah I would hope they wouldn't!

}
}
}
}
}
}
}

pub fn create_request(
model_config: &ModelConfig,
system: &str,
Expand All @@ -275,12 +324,15 @@ pub fn create_request(
});

let messages_spec = format_messages(messages, image_format);
let tools_spec = if !tools.is_empty() {
let mut tools_spec = if !tools.is_empty() {
format_tools(tools)?
} else {
vec![]
};

// Validate tool schemas
validate_tool_schemas(&mut tools_spec);

let mut messages_array = vec![system_message];
messages_array.extend(messages_spec);

Expand Down Expand Up @@ -326,6 +378,82 @@ mod tests {
use mcp_core::content::Content;
use serde_json::json;

#[test]
fn test_validate_tool_schemas() {
// Test case 1: Empty parameters object
// Input JSON with an incomplete parameters object
let mut actual = vec![json!({
"type": "function",
"function": {
"name": "test_func",
"description": "test description",
"parameters": {
"type": "object"
}
}
})];

// Run the function to validate and update schemas
validate_tool_schemas(&mut actual);

// Expected JSON after validation
let expected = vec![json!({
"type": "function",
"function": {
"name": "test_func",
"description": "test description",
"parameters": {
"type": "object",
"properties": {},
"required": []
}
}
})];

// Compare entire JSON structures instead of individual fields
assert_eq!(actual, expected);

// Test case 2: Missing type field
let mut tools = vec![json!({
"type": "function",
"function": {
"name": "test_func",
"description": "test description",
"parameters": {
"properties": {}
}
}
})];

validate_tool_schemas(&mut tools);

let params = tools[0]["function"]["parameters"].as_object().unwrap();
assert_eq!(params["type"], "object");

// Test case 3: Complete valid schema should remain unchanged
let original_schema = json!({
"type": "function",
"function": {
"name": "test_func",
"description": "test description",
"parameters": {
"type": "object",
"properties": {
"location": {
"type": "string",
"description": "City and country"
}
},
"required": ["location"]
}
}
});

let mut tools = vec![original_schema.clone()];
validate_tool_schemas(&mut tools);
assert_eq!(tools[0], original_schema);
}

const OPENAI_TOOL_USE_RESPONSE: &str = r#"{
"choices": [{
"role": "assistant",
Expand Down
4 changes: 0 additions & 4 deletions documentation/docs/tutorials/jetbrains-mcp.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@ The JetBrains extension is designed to work within your IDE. Goose can accomplis

This tutorial covers how to enable and use the JetBrains MCP Server as a built-in Goose extension to integrate with any JetBrains IDE.

:::warning Known Limitation
The JetBrains extension [does not work](https://github.com/block/goose/issues/933) with OpenAI models (e.g. gpt-4o).
:::

## Configuration

1. Add the [MCP Server plugin](https://plugins.jetbrains.com/plugin/26071-mcp-server) to your IDE.
Expand Down
Loading