mirror of
https://github.com/openai/codex.git
synced 2026-11-09 08:40:33 +00:00
Preserve bounds in reserved tool schemas (#40719)
## Why Reserved tool parameter constraints must survive schema parsing so the model receives the declared limits. ## What changed - Preserve `minimum` and `maximum` as exact JSON numbers. - Preserve `maxLength` and infer a string schema when it is the only type hint. ## Testing - Cover bounded integer, number, and string schemas through both parsing paths. - Verify history and notes tools retain their constraints in app-server model requests. GitOrigin-RevId: 4cc89722b5b226d595b57232ec3b8e825863faf3
This commit is contained in:
1 parent
de70ec840f
commit
42624fd63b
3 files changed
+78
-3
No files matched your search
@@ -170,6 +170,31 @@ async fn app_server_uses_configured_notes_backend_for_context_window_hints(
|
|||||||
"app-server should expose {namespace}.{tool_name} to the model"
|
"app-server should expose {namespace}.{tool_name} to the model"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
// Reserved tools must retain their parameter constraints in the model request.
|
||||||
|
for (namespace, tool_name, property, expected) in [
|
||||||
|
(
|
||||||
|
"history",
|
||||||
|
"list_items",
|
||||||
|
"limit",
|
||||||
|
json!({"type": "integer", "minimum": 1, "maximum": 20}),
|
||||||
|
),
|
||||||
|
(
|
||||||
|
"notes",
|
||||||
|
"search_contents",
|
||||||
|
"query",
|
||||||
|
json!({"type": "string", "maxLength": 1000}),
|
||||||
|
),
|
||||||
|
] {
|
||||||
|
let tool = request
|
||||||
|
.tool_by_name(namespace, tool_name)
|
||||||
|
.expect("history/notes tool should be exposed");
|
||||||
|
let mut schema = tool["parameters"]["properties"][property].clone();
|
||||||
|
schema
|
||||||
|
.as_object_mut()
|
||||||
|
.expect("parameter schema")
|
||||||
|
.remove("description");
|
||||||
|
assert_eq!(schema, expected, "{namespace}.{tool_name}.{property}");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
assert!(request.tool_by_name("notes", "thread_hint").is_none());
|
assert!(request.tool_by_name("notes", "thread_hint").is_none());
|
||||||
|
|
||||||
|
|||||||
@@ -50,6 +50,13 @@ pub struct JsonSchema {
|
|||||||
pub encrypted: Option<bool>,
|
pub encrypted: Option<bool>,
|
||||||
#[serde(rename = "enum", skip_serializing_if = "Option::is_none")]
|
#[serde(rename = "enum", skip_serializing_if = "Option::is_none")]
|
||||||
pub enum_values: Option<Vec<JsonValue>>,
|
pub enum_values: Option<Vec<JsonValue>>,
|
||||||
|
// Preserve bounds in reserved tool declarations, including exact integer values.
|
||||||
|
#[serde(skip_serializing_if = "Option::is_none")]
|
||||||
|
pub minimum: Option<serde_json::Number>,
|
||||||
|
#[serde(skip_serializing_if = "Option::is_none")]
|
||||||
|
pub maximum: Option<serde_json::Number>,
|
||||||
|
#[serde(rename = "maxLength", skip_serializing_if = "Option::is_none")]
|
||||||
|
pub max_length: Option<u64>,
|
||||||
#[serde(skip_serializing_if = "Option::is_none")]
|
#[serde(skip_serializing_if = "Option::is_none")]
|
||||||
pub items: Option<Box<JsonSchema>>,
|
pub items: Option<Box<JsonSchema>>,
|
||||||
#[serde(skip_serializing_if = "Option::is_none")]
|
#[serde(skip_serializing_if = "Option::is_none")]
|
||||||
@@ -521,7 +528,10 @@ fn sanitize_json_schema(value: &mut JsonValue) {
|
|||||||
schema_types.push(JsonSchemaPrimitiveType::Object);
|
schema_types.push(JsonSchemaPrimitiveType::Object);
|
||||||
} else if map.contains_key("items") || map.contains_key("prefixItems") {
|
} else if map.contains_key("items") || map.contains_key("prefixItems") {
|
||||||
schema_types.push(JsonSchemaPrimitiveType::Array);
|
schema_types.push(JsonSchemaPrimitiveType::Array);
|
||||||
} else if map.contains_key("enum") || map.contains_key("format") {
|
} else if map.contains_key("enum")
|
||||||
|
|| map.contains_key("format")
|
||||||
|
|| map.contains_key("maxLength")
|
||||||
|
{
|
||||||
schema_types.push(JsonSchemaPrimitiveType::String);
|
schema_types.push(JsonSchemaPrimitiveType::String);
|
||||||
} else if map.contains_key("minimum")
|
} else if map.contains_key("minimum")
|
||||||
|| map.contains_key("maximum")
|
|| map.contains_key("maximum")
|
||||||
|
|||||||
@@ -38,6 +38,34 @@ fn json_schema_serializes_encrypted_marker() {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn parse_tool_input_schema_preserves_numeric_and_string_bounds() {
|
||||||
|
let input = serde_json::json!({
|
||||||
|
"type": "object",
|
||||||
|
"properties": {
|
||||||
|
"count": {
|
||||||
|
"type": "integer",
|
||||||
|
"minimum": 0,
|
||||||
|
"maximum": 9_007_199_254_740_993_u64
|
||||||
|
},
|
||||||
|
"score": {"type": "number", "minimum": -1.5, "maximum": 3.25},
|
||||||
|
"query": {"type": "string", "maxLength": 1000}
|
||||||
|
},
|
||||||
|
"additionalProperties": false
|
||||||
|
});
|
||||||
|
|
||||||
|
for parse in [
|
||||||
|
parse_tool_input_schema,
|
||||||
|
parse_tool_input_schema_without_compaction,
|
||||||
|
] {
|
||||||
|
let schema = parse(&input).expect("parse schema");
|
||||||
|
assert_eq!(
|
||||||
|
serde_json::to_value(schema).expect("serialize schema"),
|
||||||
|
input
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn parse_tool_input_schema_infers_object_shape_and_defaults_properties() {
|
fn parse_tool_input_schema_infers_object_shape_and_defaults_properties() {
|
||||||
// Example schema shape:
|
// Example schema shape:
|
||||||
@@ -224,7 +252,10 @@ fn parse_tool_input_schema_infers_number_from_numeric_keywords() {
|
|||||||
}))
|
}))
|
||||||
.expect("parse schema");
|
.expect("parse schema");
|
||||||
|
|
||||||
assert_eq!(schema, JsonSchema::number(/*description*/ None));
|
assert_eq!(
|
||||||
|
serde_json::to_value(schema).expect("serialize schema"),
|
||||||
|
serde_json::json!({"type": "number", "minimum": 1})
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
@@ -246,11 +277,12 @@ fn parse_tool_input_schema_infers_number_from_multiple_of() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn parse_tool_input_schema_infers_string_from_enum_const_and_format_keywords() {
|
fn parse_tool_input_schema_infers_string_from_keywords() {
|
||||||
// Example schema shapes:
|
// Example schema shapes:
|
||||||
// { "enum": ["fast", "safe"] }
|
// { "enum": ["fast", "safe"] }
|
||||||
// { "const": "file" }
|
// { "const": "file" }
|
||||||
// { "format": "date-time" }
|
// { "format": "date-time" }
|
||||||
|
// { "maxLength": 1000 }
|
||||||
//
|
//
|
||||||
// Expected normalization behavior:
|
// Expected normalization behavior:
|
||||||
// - `enum` and `const` normalize into explicit string-enum schemas.
|
// - `enum` and `const` normalize into explicit string-enum schemas.
|
||||||
@@ -267,6 +299,10 @@ fn parse_tool_input_schema_infers_string_from_enum_const_and_format_keywords() {
|
|||||||
"format": "date-time"
|
"format": "date-time"
|
||||||
}))
|
}))
|
||||||
.expect("parse format schema");
|
.expect("parse format schema");
|
||||||
|
let max_length_schema = parse_tool_input_schema(&serde_json::json!({
|
||||||
|
"maxLength": 1000
|
||||||
|
}))
|
||||||
|
.expect("parse maxLength schema");
|
||||||
|
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
enum_schema,
|
enum_schema,
|
||||||
@@ -280,6 +316,10 @@ fn parse_tool_input_schema_infers_string_from_enum_const_and_format_keywords() {
|
|||||||
JsonSchema::string_enum(vec![serde_json::json!("file")], /*description*/ None)
|
JsonSchema::string_enum(vec![serde_json::json!("file")], /*description*/ None)
|
||||||
);
|
);
|
||||||
assert_eq!(format_schema, JsonSchema::string(/*description*/ None));
|
assert_eq!(format_schema, JsonSchema::string(/*description*/ None));
|
||||||
|
assert_eq!(
|
||||||
|
serde_json::to_value(max_length_schema).expect("serialize schema"),
|
||||||
|
serde_json::json!({"type": "string", "maxLength": 1000})
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
Reference in new issue
Block a user