Skip to content

fix: preserve nested tool properties #725

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.

Sign up for GitHub

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

Open
original4422 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: preserve nested tool properties #725

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions ollama/_types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,7 @@ class Property(SubscriptableBaseModel):
items: Optional[Any] = None
description: Optional[str] = None
enum: Optional[Sequence[Any]] = None
properties: Optional[Mapping[str, 'Tool.Function.Parameters.Property']] = None

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve nested schema keywords along with properties

When a nested object schema includes required—for example, address: {type: "object", required: ["city"], properties: ...}—validating each child as Property silently discards that keyword because this model only declares type, items, description, enum, and properties, while Pydantic ignores extra fields by default. The resulting chat request therefore weakens the supplied tool contract and may let the model emit incomplete arguments; other nested keywords such as additionalProperties, anyOf, constraints, and $ref are lost for the same reason. Preserve arbitrary JSON Schema fields or model the complete recursive schema rather than recursively narrowing every child to this partial type.

Useful? React with 👍 / 👎.

@original4422 original4422 Sep 5, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Confirmed the broader schema-keyword limitation, but it is not newly introduced by this field. I compared base fa85099 and head bebd348 using the same payload through Client.chat and captured the serialized request: both omit address.required and additionalProperties; base also drops the entire nested properties, while this change preserves the supported child fields. Existing array-item schemas and top-level required/$defs remain unchanged. The 26 serialization/utility tests still pass.

#724 specifically requests the recursive properties field. Arbitrary keyword preservation (or a complete JSON Schema model) would change the existing model contract beyond that focused fix, so I have clarified this limitation in the PR description and kept the implementation scoped. Nested constraints, anyOf, and $ref support remain a separate limitation.


properties: Optional[Mapping[str, Property]] = None

Expand Down
44 changes: 43 additions & 1 deletion tests/test_type_serialization.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,52 @@
import json
import tempfile
from base64 import b64encode
from pathlib import Path

import pytest

from ollama._types import CreateRequest, Image
from ollama._types import ChatRequest, CreateRequest, Image, Tool


def test_tool_nested_properties_serialization():
parameters = {
'type': 'object',
'properties': {
'address': {
'type': 'object',
'description': 'Delivery address',
'properties': {
'location': {
'type': 'object',
'properties': {'city': {'type': 'string', 'enum': ['London', 'Paris']}},
},
'lines': {'type': 'array', 'items': {'type': 'string'}},
},
},
},
}
tool = Tool(function=Tool.Function(name='deliver', parameters=parameters))
assert tool.function.parameters.model_dump(exclude_none=True) == parameters
assert isinstance(tool.function.parameters.properties['address'].properties['location'].properties['city'], Tool.Function.Parameters.Property)

request = ChatRequest(model='test-model', tools=[tool])
assert request.model_dump(exclude_none=True)['tools'][0]['function']['parameters'] == parameters
assert json.loads(request.model_dump_json(exclude_none=True))['tools'][0]['function']['parameters'] == parameters


def test_tool_flat_properties_serialization():
parameters = {
'type': 'object',
'$defs': {'Label': {'type': 'string'}},
'required': ['name'],
'properties': {
'name': {'type': ['string', 'null'], 'description': 'Name', 'enum': ['a', 'b', None]},
'labels': {'type': 'array', 'items': {'$ref': '#/$defs/Label'}},
'empty': {'type': 'object', 'properties': {}},
},
}
request = ChatRequest(model='test-model', tools=[{'function': {'name': 'label', 'parameters': parameters}}])
assert request.model_dump(exclude_none=True)['tools'][0]['function']['parameters'] == parameters


def test_image_serialization_bytes():
Expand Down
4 changes: 2 additions & 2 deletions tests/test_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,8 @@ def only_description_with_args(x: int, y: int):
'defs': None,
'items': None,
'properties': {
'x': {'type': 'integer', 'description': '', 'enum': None, 'items': None},
'y': {'type': 'integer', 'description': '', 'enum': None, 'items': None},
'x': {'type': 'integer', 'description': '', 'enum': None, 'items': None, 'properties': None},
'y': {'type': 'integer', 'description': '', 'enum': None, 'items': None, 'properties': None},
},
'required': ['x', 'y'],
}
Expand Down