fix(api): harden strict tool schemas (#1005)
This commit is contained in:
@@ -967,6 +967,7 @@ mod tests {
|
||||
use crate::client::chat::{
|
||||
build_chat_messages, build_chat_messages_for_request, count_reasoning_replay_chars,
|
||||
parse_chat_message, parse_sse_chunk, sanitize_thinking_mode_messages, tool_to_chat,
|
||||
tool_to_chat_for_base_url,
|
||||
};
|
||||
use crate::models::{
|
||||
ContentBlock, ContentBlockStart, Delta, Message, MessageRequest, StreamEvent, Tool,
|
||||
@@ -1365,6 +1366,59 @@ mod tests {
|
||||
assert!(encoded.get("strict").is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deepseek_non_beta_base_url_strips_strict_tool_flag() {
|
||||
let tool = Tool {
|
||||
tool_type: Some("function".to_string()),
|
||||
name: "emit_json".to_string(),
|
||||
description: "Emit JSON".to_string(),
|
||||
input_schema: json!({"type": "object", "properties": {}}),
|
||||
allowed_callers: None,
|
||||
defer_loading: None,
|
||||
input_examples: None,
|
||||
strict: Some(true),
|
||||
cache_control: None,
|
||||
};
|
||||
|
||||
let encoded = tool_to_chat_for_base_url(&tool, "https://api.deepseek.com/v1");
|
||||
|
||||
assert!(
|
||||
encoded
|
||||
.get("function")
|
||||
.and_then(|function| function.get("strict"))
|
||||
.is_none()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deepseek_beta_and_custom_base_urls_keep_strict_tool_flag() {
|
||||
let tool = Tool {
|
||||
tool_type: Some("function".to_string()),
|
||||
name: "emit_json".to_string(),
|
||||
description: "Emit JSON".to_string(),
|
||||
input_schema: json!({"type": "object", "properties": {}}),
|
||||
allowed_callers: None,
|
||||
defer_loading: None,
|
||||
input_examples: None,
|
||||
strict: Some(true),
|
||||
cache_control: None,
|
||||
};
|
||||
|
||||
for base_url in [
|
||||
"https://api.deepseek.com/beta",
|
||||
"https://example.com/openai/v1",
|
||||
] {
|
||||
let encoded = tool_to_chat_for_base_url(&tool, base_url);
|
||||
assert_eq!(
|
||||
encoded
|
||||
.get("function")
|
||||
.and_then(|function| function.get("strict"))
|
||||
.and_then(Value::as_bool),
|
||||
Some(true)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn chat_messages_drop_thinking_only_assistant_for_non_reasoning_model() {
|
||||
let message = Message {
|
||||
|
||||
@@ -88,7 +88,12 @@ impl DeepSeekClient {
|
||||
body["top_p"] = json!(top_p);
|
||||
}
|
||||
if let Some(tools) = request.tools.as_ref() {
|
||||
body["tools"] = json!(tools.iter().map(tool_to_chat).collect::<Vec<_>>());
|
||||
body["tools"] = json!(
|
||||
tools
|
||||
.iter()
|
||||
.map(|tool| tool_to_chat_for_base_url(tool, &self.base_url))
|
||||
.collect::<Vec<_>>()
|
||||
);
|
||||
}
|
||||
if let Some(choice) = request.tool_choice.as_ref()
|
||||
&& let Some(mapped) = map_tool_choice_for_chat(choice)
|
||||
@@ -157,7 +162,12 @@ impl DeepSeekClient {
|
||||
body["top_p"] = json!(top_p);
|
||||
}
|
||||
if let Some(tools) = request.tools.as_ref() {
|
||||
body["tools"] = json!(tools.iter().map(tool_to_chat).collect::<Vec<_>>());
|
||||
body["tools"] = json!(
|
||||
tools
|
||||
.iter()
|
||||
.map(|tool| tool_to_chat_for_base_url(tool, &self.base_url))
|
||||
.collect::<Vec<_>>()
|
||||
);
|
||||
}
|
||||
if let Some(choice) = request.tool_choice.as_ref()
|
||||
&& let Some(mapped) = map_tool_choice_for_chat(choice)
|
||||
@@ -712,6 +722,28 @@ pub(super) fn tool_to_chat(tool: &Tool) -> Value {
|
||||
value
|
||||
}
|
||||
|
||||
pub(super) fn tool_to_chat_for_base_url(tool: &Tool, base_url: &str) -> Value {
|
||||
let mut value = tool_to_chat(tool);
|
||||
if !deepseek_base_url_supports_strict_tools(base_url)
|
||||
&& let Some(function) = value.get_mut("function")
|
||||
&& let Some(obj) = function.as_object_mut()
|
||||
{
|
||||
obj.remove("strict");
|
||||
}
|
||||
value
|
||||
}
|
||||
|
||||
fn deepseek_base_url_supports_strict_tools(base_url: &str) -> bool {
|
||||
let trimmed = base_url.trim_end_matches('/').to_ascii_lowercase();
|
||||
let is_deepseek = trimmed == "https://api.deepseek.com"
|
||||
|| trimmed == "https://api.deepseek.com/v1"
|
||||
|| trimmed == "https://api.deepseek.com/beta"
|
||||
|| trimmed == "https://api.deepseeki.com"
|
||||
|| trimmed == "https://api.deepseeki.com/v1"
|
||||
|| trimmed == "https://api.deepseeki.com/beta";
|
||||
!is_deepseek || trimmed.ends_with("/beta")
|
||||
}
|
||||
|
||||
fn map_tool_choice_for_chat(choice: &Value) -> Option<Value> {
|
||||
if let Some(choice_str) = choice.as_str() {
|
||||
return Some(json!(choice_str));
|
||||
|
||||
@@ -741,8 +741,9 @@ pub struct Config {
|
||||
pub mcp_config_path: Option<String>,
|
||||
pub notes_path: Option<String>,
|
||||
pub memory_path: Option<String>,
|
||||
/// When true, set `tool_choice: "required"` in all API requests so the
|
||||
/// model MUST call a tool on every step (V4 strict tool-following mode).
|
||||
/// When true, set `tool_choice: "required"` and opt compatible function
|
||||
/// schemas into DeepSeek beta strict mode. Schemas with root alternatives
|
||||
/// stay non-strict to avoid changing optional/one-of tool semantics.
|
||||
pub strict_tool_mode: Option<bool>,
|
||||
/// Additional system-prompt sources concatenated in declared order
|
||||
/// (#454). Paths are expanded via `expand_path` so `~` and env
|
||||
|
||||
@@ -148,8 +148,8 @@ pub struct EngineConfig {
|
||||
/// caller resolves this from `Settings` once at engine
|
||||
/// construction; the engine never touches disk for it.
|
||||
pub locale_tag: String,
|
||||
/// When true, force `tool_choice: "required"` so the model always calls
|
||||
/// a tool on every turn step (V4 strict tool-following mode).
|
||||
/// When true, force `tool_choice: "required"` and opt compatible function
|
||||
/// schemas into DeepSeek beta strict mode.
|
||||
pub strict_tool_mode: bool,
|
||||
/// Workshop / large-tool-output routing (#548). `None` disables routing.
|
||||
pub workshop: Option<crate::tools::large_output_router::WorkshopConfig>,
|
||||
|
||||
@@ -214,7 +214,7 @@ impl Engine {
|
||||
|
||||
// Build the request
|
||||
let force_update_plan_this_step = force_update_plan_first && turn.tool_calls.is_empty();
|
||||
let active_tools = if tool_catalog.is_empty() {
|
||||
let mut active_tools = if tool_catalog.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(active_tools_for_step(
|
||||
@@ -223,6 +223,11 @@ impl Engine {
|
||||
force_update_plan_this_step,
|
||||
))
|
||||
};
|
||||
if self.config.strict_tool_mode
|
||||
&& let Some(tools) = active_tools.as_mut()
|
||||
{
|
||||
crate::tools::schema_sanitize::prepare_tools_for_strict_mode(tools);
|
||||
}
|
||||
|
||||
// Resolve `auto` reasoning_effort to a concrete tier (#663).
|
||||
let effective_reasoning_effort = resolve_auto_effort(
|
||||
|
||||
@@ -12,6 +12,8 @@
|
||||
|
||||
use serde_json::{Map, Value};
|
||||
|
||||
use crate::models::Tool;
|
||||
|
||||
/// Sanitize a JSON Schema in-place for DeepSeek strict-tool compatibility.
|
||||
///
|
||||
/// Applies a sequence of normalisations chosen to be semantics-preserving:
|
||||
@@ -37,6 +39,66 @@ pub fn sanitize(schema: &mut Value) {
|
||||
}
|
||||
}
|
||||
|
||||
/// Prepare a complete active tool set for DeepSeek strict function-calling.
|
||||
///
|
||||
/// Returns `false` and leaves the tools in non-strict mode when any root schema
|
||||
/// uses conditional alternatives (`anyOf`, `oneOf`, or `allOf`). DeepSeek's
|
||||
/// strict object rules make every property required, so forcing strict mode on
|
||||
/// root-alternative tools such as `apply_patch` or `finance` would either 400 or
|
||||
/// change their semantics. In that case callers should keep the normal
|
||||
/// best-effort schema and may still use `tool_choice = "required"`.
|
||||
pub fn prepare_tools_for_strict_mode(tools: &mut [Tool]) -> bool {
|
||||
if tools
|
||||
.iter()
|
||||
.any(|tool| !strict_schema_supported(&tool.input_schema))
|
||||
{
|
||||
for tool in tools {
|
||||
tool.strict = None;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
for tool in tools {
|
||||
sanitize_for_strict(&mut tool.input_schema);
|
||||
tool.strict = Some(true);
|
||||
}
|
||||
true
|
||||
}
|
||||
|
||||
/// Sanitize a schema for DeepSeek strict function-calling.
|
||||
///
|
||||
/// This extends the general sanitizer with the official strict-mode object
|
||||
/// rules: every object must set `additionalProperties: false`, and every
|
||||
/// property must be listed in `required`.
|
||||
pub fn sanitize_for_strict(schema: &mut Value) {
|
||||
sanitize(schema);
|
||||
enforce_strict_subset(schema);
|
||||
}
|
||||
|
||||
fn strict_schema_supported(schema: &Value) -> bool {
|
||||
let mut normalized = schema.clone();
|
||||
sanitize(&mut normalized);
|
||||
!has_strict_incompatible_composition(&normalized, true)
|
||||
}
|
||||
|
||||
fn has_strict_incompatible_composition(schema: &Value, is_root: bool) -> bool {
|
||||
if let Some(obj) = schema.as_object() {
|
||||
if obj.contains_key("oneOf") || obj.contains_key("allOf") {
|
||||
return true;
|
||||
}
|
||||
if is_root && obj.contains_key("anyOf") {
|
||||
return true;
|
||||
}
|
||||
return obj
|
||||
.values()
|
||||
.any(|value| has_strict_incompatible_composition(value, false));
|
||||
}
|
||||
schema.as_array().is_some_and(|arr| {
|
||||
arr.iter()
|
||||
.any(|value| has_strict_incompatible_composition(value, false))
|
||||
})
|
||||
}
|
||||
|
||||
/// Collapse `{"anyOf":[X, {"type":"null"}]}` → `X ∪ {"nullable": true}`.
|
||||
///
|
||||
/// Same treatment for `oneOf`. Only collapses when exactly one non-null
|
||||
@@ -135,6 +197,59 @@ fn collapse_single_element_unions(schema: &mut Value) {
|
||||
}
|
||||
}
|
||||
|
||||
fn enforce_strict_subset(schema: &mut Value) {
|
||||
if let Some(obj) = schema.as_object_mut() {
|
||||
strip_unsupported_strict_keywords(obj);
|
||||
if is_object_schema(obj) {
|
||||
let mut property_names: Vec<Value> = ensure_properties_object(obj)
|
||||
.keys()
|
||||
.cloned()
|
||||
.map(Value::String)
|
||||
.collect();
|
||||
property_names.sort_by(|a, b| a.as_str().cmp(&b.as_str()));
|
||||
obj.insert("required".into(), Value::Array(property_names));
|
||||
obj.insert("additionalProperties".into(), Value::Bool(false));
|
||||
}
|
||||
|
||||
for value in obj.values_mut() {
|
||||
enforce_strict_subset(value);
|
||||
}
|
||||
} else if let Some(arr) = schema.as_array_mut() {
|
||||
for value in arr {
|
||||
enforce_strict_subset(value);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn strip_unsupported_strict_keywords(obj: &mut Map<String, Value>) {
|
||||
obj.remove("patternProperties");
|
||||
match obj.get("type").and_then(Value::as_str) {
|
||||
Some("string") => {
|
||||
obj.remove("minLength");
|
||||
obj.remove("maxLength");
|
||||
}
|
||||
Some("array") => {
|
||||
obj.remove("minItems");
|
||||
obj.remove("maxItems");
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
fn is_object_schema(obj: &Map<String, Value>) -> bool {
|
||||
obj.get("type").and_then(Value::as_str) == Some("object") || obj.contains_key("properties")
|
||||
}
|
||||
|
||||
fn ensure_properties_object(obj: &mut Map<String, Value>) -> &mut Map<String, Value> {
|
||||
let needs_replacement = !matches!(obj.get("properties"), Some(Value::Object(_)));
|
||||
if needs_replacement {
|
||||
obj.insert("properties".into(), Value::Object(Map::new()));
|
||||
}
|
||||
obj.get_mut("properties")
|
||||
.and_then(Value::as_object_mut)
|
||||
.expect("properties was just ensured as object")
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -322,4 +437,170 @@ mod tests {
|
||||
sanitize(&mut schema);
|
||||
assert_eq!(schema, after_first, "sanitize must be idempotent");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strict_sanitize_requires_all_object_properties_and_closes_extra_keys() {
|
||||
let mut schema = json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"name": {"type": "string"},
|
||||
"count": {"type": "integer"}
|
||||
},
|
||||
"required": ["name"],
|
||||
"additionalProperties": {"type": "string"}
|
||||
});
|
||||
|
||||
sanitize_for_strict(&mut schema);
|
||||
|
||||
assert_eq!(schema["additionalProperties"], false);
|
||||
assert_eq!(schema["required"], json!(["count", "name"]));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strict_sanitize_applies_object_rules_recursively() {
|
||||
let mut schema = json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"outer": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"inner": {"type": "string"}
|
||||
},
|
||||
"required": []
|
||||
}
|
||||
},
|
||||
"required": []
|
||||
});
|
||||
|
||||
sanitize_for_strict(&mut schema);
|
||||
|
||||
assert_eq!(schema["required"], json!(["outer"]));
|
||||
assert_eq!(schema["additionalProperties"], false);
|
||||
assert_eq!(schema["properties"]["outer"]["required"], json!(["inner"]));
|
||||
assert_eq!(schema["properties"]["outer"]["additionalProperties"], false);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strict_sanitize_removes_unsupported_string_and_array_bounds() {
|
||||
let mut schema = json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"name": {
|
||||
"type": "string",
|
||||
"minLength": 1,
|
||||
"maxLength": 64,
|
||||
"pattern": "^[a-z]+$"
|
||||
},
|
||||
"items": {
|
||||
"type": "array",
|
||||
"minItems": 1,
|
||||
"maxItems": 5,
|
||||
"items": {"type": "string"}
|
||||
},
|
||||
"score": {
|
||||
"type": "integer",
|
||||
"minimum": 1,
|
||||
"maximum": 5
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
sanitize_for_strict(&mut schema);
|
||||
|
||||
let name = &schema["properties"]["name"];
|
||||
assert!(name.get("minLength").is_none());
|
||||
assert!(name.get("maxLength").is_none());
|
||||
assert_eq!(name["pattern"], "^[a-z]+$");
|
||||
|
||||
let items = &schema["properties"]["items"];
|
||||
assert!(items.get("minItems").is_none());
|
||||
assert!(items.get("maxItems").is_none());
|
||||
|
||||
let score = &schema["properties"]["score"];
|
||||
assert_eq!(score["minimum"], 1);
|
||||
assert_eq!(score["maximum"], 5);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strict_mode_rejects_root_composition_for_whole_tool_set() {
|
||||
let mut tools = vec![Tool {
|
||||
tool_type: None,
|
||||
name: "either".to_string(),
|
||||
description: "Either input shape".to_string(),
|
||||
input_schema: json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"a": {"type": "string"},
|
||||
"b": {"type": "string"}
|
||||
},
|
||||
"anyOf": [
|
||||
{"required": ["a"]},
|
||||
{"required": ["b"]}
|
||||
]
|
||||
}),
|
||||
allowed_callers: None,
|
||||
defer_loading: None,
|
||||
input_examples: None,
|
||||
strict: Some(true),
|
||||
cache_control: None,
|
||||
}];
|
||||
|
||||
assert!(!prepare_tools_for_strict_mode(&mut tools));
|
||||
assert_eq!(tools[0].strict, None);
|
||||
assert!(tools[0].input_schema.get("anyOf").is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strict_mode_rejects_nested_unsupported_composition() {
|
||||
let mut tools = vec![Tool {
|
||||
tool_type: None,
|
||||
name: "nested".to_string(),
|
||||
description: "Nested oneOf".to_string(),
|
||||
input_schema: json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"value": {
|
||||
"oneOf": [
|
||||
{"type": "string"},
|
||||
{"type": "integer"}
|
||||
]
|
||||
}
|
||||
}
|
||||
}),
|
||||
allowed_callers: None,
|
||||
defer_loading: None,
|
||||
input_examples: None,
|
||||
strict: None,
|
||||
cache_control: None,
|
||||
}];
|
||||
|
||||
assert!(!prepare_tools_for_strict_mode(&mut tools));
|
||||
assert_eq!(tools[0].strict, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strict_mode_marks_compatible_tools_strict() {
|
||||
let mut tools = vec![Tool {
|
||||
tool_type: None,
|
||||
name: "lookup".to_string(),
|
||||
description: "Lookup".to_string(),
|
||||
input_schema: json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"query": {"type": "string"}
|
||||
},
|
||||
"required": []
|
||||
}),
|
||||
allowed_callers: None,
|
||||
defer_loading: None,
|
||||
input_examples: None,
|
||||
strict: None,
|
||||
cache_control: None,
|
||||
}];
|
||||
|
||||
assert!(prepare_tools_for_strict_mode(&mut tools));
|
||||
assert_eq!(tools[0].strict, Some(true));
|
||||
assert_eq!(tools[0].input_schema["required"], json!(["query"]));
|
||||
assert_eq!(tools[0].input_schema["additionalProperties"], false);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4297,6 +4297,7 @@ mod tests {
|
||||
#[test]
|
||||
fn recoverable_clear_stashes_nonempty_draft() {
|
||||
let mut app = App::new(test_options(false), &Config::default());
|
||||
app.input_history.clear();
|
||||
app.input = "recover this".to_string();
|
||||
app.cursor_position = app.input.chars().count();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user