mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-13 02:14:39 +00:00
fix(schema): exempt multi-action tools from OpenAI strict mode
Strict mode forces ALL properties to be required, causing models to send ~13 empty params per tool call on multi-action tools like team_tasks (17 actions, 16 params). This wastes ~200-300 output tokens per call. - Add IsMultiActionSchema() to detect tools with action+enum pattern - Per-tool strict decision in CleanToolSchemas (exempt multi-action) - Extract normalizeWithProfile() for per-tool profile overrides - Add per-action param guide to team_tasks description - Fix schema/impl mismatches: file_id→path, add task_type property - Gate param guide on policy (lite edition only sees allowed actions)
This commit is contained in:
1 parent
43232c6189
commit
04dc34e32c
5 files changed
+273
-11
No files matched your search
@@ -7,19 +7,28 @@ func CleanToolSchemas(providerName string, tools []ToolDefinition) []ToolDefinit
|
||||
return tools
|
||||
}
|
||||
profile := profileForProvider(providerName)
|
||||
var strictPtr *bool
|
||||
if profile.StrictToolMode {
|
||||
t := true
|
||||
strictPtr = &t
|
||||
}
|
||||
cleaned := make([]ToolDefinition, len(tools))
|
||||
for i, t := range tools {
|
||||
// Exempt multi-action tools from strict mode — their many optional
|
||||
// params become required under strict, forcing models to send empty values
|
||||
// for every call (wasting ~200-300 output tokens per tool call).
|
||||
useStrict := profile.StrictToolMode && !IsMultiActionSchema(t.Function.Parameters)
|
||||
|
||||
var strictPtr *bool
|
||||
if useStrict {
|
||||
tr := true
|
||||
strictPtr = &tr
|
||||
}
|
||||
|
||||
toolProfile := profile
|
||||
toolProfile.StrictToolMode = useStrict
|
||||
|
||||
cleaned[i] = ToolDefinition{
|
||||
Type: t.Type,
|
||||
Function: ToolFunctionSchema{
|
||||
Name: t.Function.Name,
|
||||
Description: t.Function.Description,
|
||||
Parameters: NormalizeSchema(providerName, t.Function.Parameters),
|
||||
Parameters: normalizeWithProfile(toolProfile, t.Function.Parameters),
|
||||
Strict: strictPtr,
|
||||
},
|
||||
}
|
||||
|
||||
@@ -162,6 +162,196 @@ func TestCleanSchema_NestedArray(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Multi-action tool strict mode exemption
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
func TestIsMultiActionSchema(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
schema map[string]any
|
||||
want bool
|
||||
}{
|
||||
{
|
||||
name: "team_tasks-like with []string enum",
|
||||
schema: map[string]any{
|
||||
"type": "object",
|
||||
"properties": map[string]any{
|
||||
"action": map[string]any{
|
||||
"type": "string",
|
||||
"enum": []string{"list", "create", "search"},
|
||||
},
|
||||
"query": map[string]any{"type": "string"},
|
||||
},
|
||||
"required": []string{"action"},
|
||||
},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "cron-like with []any enum",
|
||||
schema: map[string]any{
|
||||
"type": "object",
|
||||
"properties": map[string]any{
|
||||
"action": map[string]any{
|
||||
"type": "string",
|
||||
"enum": []any{"status", "add", "remove"},
|
||||
},
|
||||
},
|
||||
},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "single action — not exempt",
|
||||
schema: map[string]any{
|
||||
"type": "object",
|
||||
"properties": map[string]any{
|
||||
"action": map[string]any{
|
||||
"type": "string",
|
||||
"enum": []string{"send"},
|
||||
},
|
||||
},
|
||||
},
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "no action property",
|
||||
schema: map[string]any{
|
||||
"type": "object",
|
||||
"properties": map[string]any{
|
||||
"query": map[string]any{"type": "string"},
|
||||
},
|
||||
},
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "action without enum",
|
||||
schema: map[string]any{
|
||||
"type": "object",
|
||||
"properties": map[string]any{
|
||||
"action": map[string]any{"type": "string"},
|
||||
},
|
||||
},
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "enum on non-action field",
|
||||
schema: map[string]any{
|
||||
"type": "object",
|
||||
"properties": map[string]any{
|
||||
"mode": map[string]any{
|
||||
"type": "string",
|
||||
"enum": []string{"markdown", "text"},
|
||||
},
|
||||
},
|
||||
},
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "nil schema",
|
||||
schema: nil,
|
||||
want: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := IsMultiActionSchema(tt.schema)
|
||||
if got != tt.want {
|
||||
t.Errorf("IsMultiActionSchema() = %v, want %v", got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestCleanToolSchemas_OpenAI_MultiActionExempt(t *testing.T) {
|
||||
multiAction := ToolDefinition{
|
||||
Type: "function",
|
||||
Function: ToolFunctionSchema{
|
||||
Name: "team_tasks",
|
||||
Description: "multi-action tool",
|
||||
Parameters: map[string]any{
|
||||
"type": "object",
|
||||
"properties": map[string]any{
|
||||
"action": map[string]any{
|
||||
"type": "string",
|
||||
"enum": []string{"list", "create", "search"},
|
||||
},
|
||||
"query": map[string]any{
|
||||
"type": "string",
|
||||
"description": "search query",
|
||||
},
|
||||
"subject": map[string]any{
|
||||
"type": "string",
|
||||
"description": "task subject",
|
||||
},
|
||||
},
|
||||
"required": []string{"action"},
|
||||
},
|
||||
},
|
||||
}
|
||||
simple := ToolDefinition{
|
||||
Type: "function",
|
||||
Function: ToolFunctionSchema{
|
||||
Name: "read_file",
|
||||
Description: "simple tool",
|
||||
Parameters: map[string]any{
|
||||
"type": "object",
|
||||
"properties": map[string]any{
|
||||
"path": map[string]any{
|
||||
"type": "string",
|
||||
"description": "file path",
|
||||
},
|
||||
},
|
||||
"required": []string{"path"},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
cleaned := CleanToolSchemas("openai", []ToolDefinition{multiAction, simple})
|
||||
|
||||
// Multi-action tool: strict should be nil (exempt).
|
||||
if cleaned[0].Function.Strict != nil {
|
||||
t.Errorf("multi-action tool: expected Strict=nil, got %v", *cleaned[0].Function.Strict)
|
||||
}
|
||||
|
||||
// Multi-action tool: required should stay ["action"] only.
|
||||
params0 := cleaned[0].Function.Parameters
|
||||
reqArr, ok := params0["required"].([]string)
|
||||
if !ok {
|
||||
// May be []any from original schema copy.
|
||||
if reqAny, ok2 := params0["required"].([]any); ok2 {
|
||||
if len(reqAny) != 1 {
|
||||
t.Errorf("multi-action: expected 1 required, got %d", len(reqAny))
|
||||
}
|
||||
}
|
||||
} else if len(reqArr) != 1 {
|
||||
t.Errorf("multi-action: expected 1 required, got %d: %v", len(reqArr), reqArr)
|
||||
}
|
||||
|
||||
// Multi-action tool: should NOT have additionalProperties:false.
|
||||
if _, hasAP := params0["additionalProperties"]; hasAP {
|
||||
t.Error("multi-action: expected no additionalProperties")
|
||||
}
|
||||
|
||||
// Simple tool: strict should be true.
|
||||
if cleaned[1].Function.Strict == nil || !*cleaned[1].Function.Strict {
|
||||
t.Error("simple tool: expected Strict=true")
|
||||
}
|
||||
|
||||
// Simple tool: all props required (strict mode applied).
|
||||
params1 := cleaned[1].Function.Parameters
|
||||
if reqAny, ok := params1["required"].([]any); ok {
|
||||
if len(reqAny) != 1 {
|
||||
t.Errorf("simple tool: expected 1 required (path), got %d", len(reqAny))
|
||||
}
|
||||
}
|
||||
|
||||
// Simple tool: additionalProperties:false should be set.
|
||||
if ap, ok := params1["additionalProperties"].(bool); !ok || ap {
|
||||
t.Error("simple tool: expected additionalProperties=false")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCleanSchema_DeepNesting(t *testing.T) {
|
||||
params := map[string]any{
|
||||
"type": "object",
|
||||
|
||||
@@ -11,10 +11,15 @@ const maxSchemaDepth = 64
|
||||
// This is the single entry point — all providers should call this (directly or
|
||||
// via CleanToolSchemas / CleanSchemaForProvider wrappers).
|
||||
func NormalizeSchema(providerName string, schema map[string]any) map[string]any {
|
||||
return normalizeWithProfile(profileForProvider(providerName), schema)
|
||||
}
|
||||
|
||||
// normalizeWithProfile applies normalization using a pre-resolved profile.
|
||||
// Used by CleanToolSchemas to pass per-tool profile overrides.
|
||||
func normalizeWithProfile(profile SchemaProfile, schema map[string]any) map[string]any {
|
||||
if schema == nil {
|
||||
return nil
|
||||
}
|
||||
profile := profileForProvider(providerName)
|
||||
result := copySchema(schema)
|
||||
|
||||
if profile.ResolveRefs {
|
||||
|
||||
@@ -72,6 +72,33 @@ func applyStrictMode(schema map[string]any, depth int) map[string]any {
|
||||
return schema
|
||||
}
|
||||
|
||||
// IsMultiActionSchema detects tool schemas using the multi-action pattern:
|
||||
// a top-level "action" property with an "enum" list of 2+ values.
|
||||
// These tools have many optional params shared across actions;
|
||||
// strict mode forces models to send all params for every call.
|
||||
func IsMultiActionSchema(schema map[string]any) bool {
|
||||
props, ok := schema["properties"].(map[string]any)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
actionProp, ok := props["action"].(map[string]any)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
enumVal, ok := actionProp["enum"]
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
// Only exempt if 2+ actions — single-action tools benefit from strict.
|
||||
switch e := enumVal.(type) {
|
||||
case []string:
|
||||
return len(e) >= 2
|
||||
case []any:
|
||||
return len(e) >= 2
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// makeNullable converts a property schema to accept null values.
|
||||
// - Simple type: "string" → ["string", "null"]
|
||||
// - Type array: ["string", "integer"] → ["string", "integer", "null"]
|
||||
|
||||
@@ -81,9 +81,13 @@ func (t *TeamTasksTool) Parameters() map[string]any {
|
||||
"type": "integer",
|
||||
"description": "Progress percentage 0-100 (for progress action)",
|
||||
},
|
||||
"file_id": map[string]any{
|
||||
"path": map[string]any{
|
||||
"type": "string",
|
||||
"description": "Workspace file ID (for attach)",
|
||||
"description": "Workspace file path (for attach)",
|
||||
},
|
||||
"task_type": map[string]any{
|
||||
"type": "string",
|
||||
"description": "Task type for create: 'general' (default), 'request', or 'note'",
|
||||
},
|
||||
"assignee": map[string]any{
|
||||
"type": "string",
|
||||
@@ -99,13 +103,40 @@ func (t *TeamTasksTool) Parameters() map[string]any {
|
||||
}
|
||||
|
||||
// buildActionDescription returns the action parameter description based on policy.
|
||||
// Includes per-action param guide so models know which params to send.
|
||||
func (t *TeamTasksTool) buildActionDescription() string {
|
||||
base := "Available actions: " + strings.Join(t.policy.AllowedActions(), ", ") + "."
|
||||
if t.policy.IsAllowed("ask_user") {
|
||||
base += " ask_user: set a periodic reminder sent to the USER when you need input. clear_ask_user: cancel a previously set reminder."
|
||||
base += " ask_user: set a periodic reminder. clear_ask_user: cancel reminder."
|
||||
}
|
||||
if t.policy.IsAllowed("retry") {
|
||||
base += " retry: re-dispatch a stale or failed task."
|
||||
base += " retry: re-dispatch a stale/failed task."
|
||||
}
|
||||
// Per-action param guide — only list actions allowed by policy.
|
||||
base += "\n\nParams per action (only send listed params):\n"
|
||||
guide := map[string]string{
|
||||
"list": "- list: status?, page?\n",
|
||||
"get": "- get: task_id\n",
|
||||
"create": "- create: subject, description, assignee, priority?, blocked_by?, require_approval?, task_type?\n",
|
||||
"claim": "- claim: task_id\n",
|
||||
"complete": "- complete: task_id?, result\n",
|
||||
"cancel": "- cancel: task_id, text\n",
|
||||
"search": "- search: query, page?\n",
|
||||
"review": "- review: task_id\n",
|
||||
"comment": "- comment: task_id?, text, type?\n",
|
||||
"progress": "- progress: task_id?, percent, text?\n",
|
||||
"attach": "- attach: task_id, path\n",
|
||||
"update": "- update: task_id, subject?, description?, priority?, blocked_by?\n",
|
||||
"approve": "- approve: task_id\n",
|
||||
"reject": "- reject: task_id, text\n",
|
||||
"ask_user": "- ask_user: task_id, text\n",
|
||||
"clear_ask_user": "- clear_ask_user: task_id\n",
|
||||
"retry": "- retry: task_id\n",
|
||||
}
|
||||
for _, action := range t.policy.AllowedActions() {
|
||||
if line, ok := guide[action]; ok {
|
||||
base += line
|
||||
}
|
||||
}
|
||||
return base
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user