mirror of
https://github.com/docker/docker-agent.git
synced 2026-11-04 10:02:15 +00:00
Merge pull request #4760 from jaeminbird/fix/openai-typeless-enum
fix(tools): type typeless enum/const schemas from their values
This commit is contained in:
5 files changed
+234
-7
No files matched your search
@@ -276,10 +276,17 @@ func makeAllRequired(schema shared.FunctionParameters) shared.FunctionParameters
|
||||
}
|
||||
|
||||
_, isRef := propMap["$ref"]
|
||||
_, hasEnum := propMap["enum"]
|
||||
_, hasConst := propMap["const"]
|
||||
t, isScalarType := propMap["type"].(string)
|
||||
switch {
|
||||
case isRef || t == "array" || t == "object":
|
||||
properties[propName] = nullableAnyOf(propMap)
|
||||
case hasEnum || hasConst:
|
||||
// A "null" type alongside enum/const still rejects null (both
|
||||
// apply), and the type may be a multi-type array. Wrapping
|
||||
// keeps the values intact and adds null as an alternative.
|
||||
properties[propName] = nullableAnyOf(propMap)
|
||||
case isScalarType:
|
||||
propMap["type"] = []string{t, "null"}
|
||||
}
|
||||
@@ -352,7 +359,8 @@ func isCompositionNode(node map[string]any) bool {
|
||||
|
||||
// ensureTypeFields ensures every schema node that is a map has a "type" key.
|
||||
// OpenAI Responses API requires all schema nodes to have an explicit type.
|
||||
// Nodes with "properties" default to "object"; other nodes default to "object" as well.
|
||||
// enum/const nodes get the type of their values (pydantic emits a typeless
|
||||
// enum for a mixed Literal); other nodes default to "object".
|
||||
// $ref/anyOf/oneOf/allOf nodes are left as leaves — see isCompositionNode.
|
||||
func ensureTypeFields(schema shared.FunctionParameters) shared.FunctionParameters {
|
||||
if schema == nil {
|
||||
@@ -364,7 +372,7 @@ func ensureTypeFields(schema shared.FunctionParameters) shared.FunctionParameter
|
||||
return
|
||||
}
|
||||
if _, hasType := node["type"]; !hasType {
|
||||
node["type"] = "object"
|
||||
node["type"] = tools.DefaultType(node)
|
||||
}
|
||||
})
|
||||
|
||||
|
||||
@@ -2,8 +2,10 @@ package openai
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"maps"
|
||||
"testing"
|
||||
|
||||
"github.com/google/jsonschema-go/jsonschema"
|
||||
"github.com/openai/openai-go/v3/shared"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
@@ -1008,6 +1010,40 @@ func TestEnsureTypeFields_RefLeaf(t *testing.T) {
|
||||
assert.Equal(t, "#/$defs/thing", p["$ref"])
|
||||
}
|
||||
|
||||
// TestEnsureTypeFields_EnumConstTakeValueType ensures a typeless enum/const
|
||||
// node gets the type of its values rather than "object", which would reject
|
||||
// every value. pydantic emits such a node for Literal["auto", 0, 1].
|
||||
func TestEnsureTypeFields_EnumConstTakeValueType(t *testing.T) {
|
||||
t.Parallel()
|
||||
tests := []struct {
|
||||
name string
|
||||
node map[string]any
|
||||
want any
|
||||
}{
|
||||
{"string enum", map[string]any{"enum": []any{"a", "b"}}, "string"},
|
||||
{"integer enum", map[string]any{"enum": []any{float64(1), float64(2)}}, "integer"},
|
||||
{"number enum", map[string]any{"enum": []any{float64(1), 1.5}}, "number"},
|
||||
{"boolean enum", map[string]any{"enum": []any{true}}, "boolean"},
|
||||
{"mixed enum", map[string]any{"enum": []any{"auto", float64(0), nil}}, []any{"string", "integer", "null"}},
|
||||
{"string const", map[string]any{"const": "x"}, "string"},
|
||||
{"no values", map[string]any{"description": "d"}, "object"},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
schema := shared.FunctionParameters{
|
||||
"type": "object",
|
||||
"properties": map[string]any{"p": tt.node},
|
||||
}
|
||||
|
||||
updated := ensureTypeFields(schema)
|
||||
|
||||
p := updated["properties"].(map[string]any)["p"].(map[string]any)
|
||||
assert.Equal(t, tt.want, p["type"])
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestEnsureTypeFields_CompositionNodesLeftAsLeaves ensures ensureTypeFields
|
||||
// never injects a "type" onto an anyOf/oneOf/allOf node. A "type" sibling
|
||||
// combines with these keywords via AND, so injecting one can make the
|
||||
@@ -1331,3 +1367,66 @@ func assertNoNullableContainerTypePairs(t *testing.T, schema map[string]any) {
|
||||
assert.False(t, isNullablePair, "found nullable %s type pair: %v", container, node)
|
||||
})
|
||||
}
|
||||
|
||||
// TestConvertParametersToSchema_EnumConstNullability checks typeless and
|
||||
// nullable enum/const properties through the full conversion by validating
|
||||
// instances against the result: optional ones must still accept null once
|
||||
// made required, and a required enum that lists null must keep accepting it.
|
||||
func TestConvertParametersToSchema_EnumConstNullability(t *testing.T) {
|
||||
t.Parallel()
|
||||
params := map[string]any{
|
||||
"type": "object",
|
||||
"properties": map[string]any{
|
||||
"mode": map[string]any{"enum": []any{"auto", 0, 1}},
|
||||
"only": map[string]any{"const": "x"},
|
||||
"level": map[string]any{"type": "string", "enum": []any{"low", "high"}},
|
||||
"pick": map[string]any{"enum": []any{"auto", 0, nil}},
|
||||
},
|
||||
"required": []any{"pick"},
|
||||
}
|
||||
|
||||
out, strict, err := ConvertParametersToSchema(params)
|
||||
require.NoError(t, err)
|
||||
assert.True(t, strict)
|
||||
|
||||
data, err := json.Marshal(out)
|
||||
require.NoError(t, err)
|
||||
var parsed jsonschema.Schema
|
||||
require.NoError(t, json.Unmarshal(data, &parsed))
|
||||
resolved, err := parsed.Resolve(nil)
|
||||
require.NoError(t, err)
|
||||
|
||||
valid := map[string]any{"mode": "auto", "only": "x", "level": "low", "pick": "auto"}
|
||||
tests := []struct {
|
||||
name string
|
||||
field string
|
||||
value any
|
||||
ok bool
|
||||
}{
|
||||
{"mixed enum string", "mode", "auto", true},
|
||||
{"mixed enum integer", "mode", 0, true},
|
||||
{"mixed enum omitted as null", "mode", nil, true},
|
||||
{"mixed enum other", "mode", "bad", false},
|
||||
{"const value", "only", "x", true},
|
||||
{"const omitted as null", "only", nil, true},
|
||||
{"const other", "only", "y", false},
|
||||
{"typed enum omitted as null", "level", nil, true},
|
||||
{"typed enum other", "level", "mid", false},
|
||||
{"required enum allows null", "pick", nil, true},
|
||||
{"required enum integer", "pick", 0, true},
|
||||
{"required enum other", "pick", "bad", false},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
instance := maps.Clone(valid)
|
||||
instance[tt.field] = tt.value
|
||||
err := resolved.Validate(instance)
|
||||
if tt.ok {
|
||||
assert.NoError(t, err)
|
||||
} else {
|
||||
assert.Error(t, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -305,9 +305,16 @@ func TestSchemaForOpenai(t *testing.T) {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"direction": {
|
||||
"description": "Order",
|
||||
"enum": ["ASC", "DESC"],
|
||||
"type": ["string", "null"]
|
||||
"anyOf": [
|
||||
{
|
||||
"description": "Order",
|
||||
"enum": ["ASC", "DESC"],
|
||||
"type": "string"
|
||||
},
|
||||
{
|
||||
"type": "null"
|
||||
}
|
||||
]
|
||||
},
|
||||
"labels": {
|
||||
"anyOf": [
|
||||
|
||||
+76
-2
@@ -3,6 +3,7 @@ package tools
|
||||
import (
|
||||
"encoding/json"
|
||||
"reflect"
|
||||
"slices"
|
||||
|
||||
"github.com/google/jsonschema-go/jsonschema"
|
||||
)
|
||||
@@ -130,6 +131,11 @@ func requiredSet(schema map[string]any) map[string]bool {
|
||||
}
|
||||
|
||||
func removeNullFromType(prop map[string]any) {
|
||||
// Required means the key must be present, not that its value can't be
|
||||
// null: keep "null" when an enum/const lists it as an allowed value.
|
||||
if AllowsNullValue(prop) {
|
||||
return
|
||||
}
|
||||
typeVal, exists := prop["type"]
|
||||
if !exists {
|
||||
return
|
||||
@@ -158,7 +164,8 @@ func removeNullFromType(prop map[string]any) {
|
||||
}
|
||||
|
||||
// ensurePropertyTypes recursively walks a JSON Schema map and ensures
|
||||
// every property has a "type" set, defaulting to "object" if missing.
|
||||
// every property has a "type" set, defaulting to "object" (or the type of
|
||||
// its enum/const values, see DefaultType) if missing.
|
||||
// It descends into nested "properties" and array "items".
|
||||
// $ref nodes are skipped: a $ref's type is defined by its target, and
|
||||
// injecting one adds a sibling keyword that providers like OpenAI reject
|
||||
@@ -207,7 +214,7 @@ func ensurePropertyTypes(schema map[string]any) {
|
||||
}
|
||||
|
||||
if prop["type"] == nil {
|
||||
prop["type"] = "object"
|
||||
prop["type"] = DefaultType(prop)
|
||||
}
|
||||
|
||||
// Recurse into nested object properties.
|
||||
@@ -250,3 +257,70 @@ func ConvertSchema(params, v any) error {
|
||||
|
||||
return json.Unmarshal(buf, v)
|
||||
}
|
||||
|
||||
// DefaultType returns the "type" to give a schema node that has none: the
|
||||
// JSON type(s) of its enum/const values, or "object" when it has none.
|
||||
// pydantic emits a typeless enum for a mixed Literal such as
|
||||
// Literal["auto", 0]; a "type": "object" sibling would reject every value.
|
||||
func DefaultType(node map[string]any) any {
|
||||
var values []any
|
||||
if enum, ok := node["enum"].([]any); ok {
|
||||
values = slices.Clone(enum)
|
||||
}
|
||||
if c, ok := node["const"]; ok {
|
||||
values = append(values, c)
|
||||
}
|
||||
|
||||
var types []string
|
||||
for _, v := range values {
|
||||
var t string
|
||||
switch v := v.(type) {
|
||||
case nil:
|
||||
t = "null"
|
||||
case bool:
|
||||
t = "boolean"
|
||||
case string:
|
||||
t = "string"
|
||||
case float64:
|
||||
t = "number"
|
||||
if v == float64(int64(v)) {
|
||||
t = "integer"
|
||||
}
|
||||
case int, int32, int64:
|
||||
t = "integer"
|
||||
case []any:
|
||||
t = "array"
|
||||
default:
|
||||
t = "object"
|
||||
}
|
||||
if !slices.Contains(types, t) {
|
||||
types = append(types, t)
|
||||
}
|
||||
}
|
||||
// "number" already admits integers.
|
||||
if slices.Contains(types, "number") {
|
||||
types = slices.DeleteFunc(types, func(t string) bool { return t == "integer" })
|
||||
}
|
||||
|
||||
switch len(types) {
|
||||
case 0:
|
||||
return "object"
|
||||
case 1:
|
||||
return types[0]
|
||||
default:
|
||||
out := make([]any, len(types))
|
||||
for i, t := range types {
|
||||
out[i] = t
|
||||
}
|
||||
return out
|
||||
}
|
||||
}
|
||||
|
||||
// AllowsNullValue reports whether node's enum or const explicitly lists null.
|
||||
func AllowsNullValue(node map[string]any) bool {
|
||||
if enum, ok := node["enum"].([]any); ok && slices.Contains(enum, nil) {
|
||||
return true
|
||||
}
|
||||
c, ok := node["const"]
|
||||
return ok && c == nil
|
||||
}
|
||||
@@ -73,6 +73,45 @@ func TestSchemaToMap_PropertyWithoutType(t *testing.T) {
|
||||
}, m)
|
||||
}
|
||||
|
||||
// TestSchemaToMap_EnumPropertyWithoutType covers pydantic's schema for
|
||||
// Literal["auto", 0, 1] and Literal["x"]: the injected type must admit the
|
||||
// enum/const values, not reject them all as "object" would.
|
||||
func TestSchemaToMap_EnumPropertyWithoutType(t *testing.T) {
|
||||
t.Parallel()
|
||||
m, err := SchemaToMap(map[string]any{
|
||||
"type": "object",
|
||||
"properties": map[string]any{
|
||||
"mode": map[string]any{"enum": []any{"auto", 0, 1}},
|
||||
"only": map[string]any{"const": "x"},
|
||||
},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
props := m["properties"].(map[string]any)
|
||||
assert.Equal(t, []any{"string", "integer"}, props["mode"].(map[string]any)["type"])
|
||||
assert.Equal(t, "string", props["only"].(map[string]any)["type"])
|
||||
}
|
||||
|
||||
// TestSchemaToMap_RequiredEnumKeepsNull ensures a required property whose
|
||||
// enum lists null keeps "null" in its inferred type: required means present,
|
||||
// and null is one of the allowed values.
|
||||
func TestSchemaToMap_RequiredEnumKeepsNull(t *testing.T) {
|
||||
t.Parallel()
|
||||
m, err := SchemaToMap(map[string]any{
|
||||
"type": "object",
|
||||
"properties": map[string]any{
|
||||
"mode": map[string]any{"enum": []any{"auto", 0, nil}},
|
||||
"none": map[string]any{"const": nil},
|
||||
},
|
||||
"required": []any{"mode", "none"},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
props := m["properties"].(map[string]any)
|
||||
assert.Equal(t, []any{"string", "integer", "null"}, props["mode"].(map[string]any)["type"])
|
||||
assert.Equal(t, "null", props["none"].(map[string]any)["type"])
|
||||
}
|
||||
|
||||
func TestSchemaToMap_NestedPropertyWithoutType(t *testing.T) {
|
||||
t.Parallel()
|
||||
m, err := SchemaToMap(map[string]any{
|
||||
|
||||
Reference in new issue
Block a user