Skip to content

fix(types): expose tool call id on Message.ToolCall#698

Open
winklemad wants to merge 1 commit into
ollama:mainfrom
winklemad:fix/toolcall-id
Open

fix(types): expose tool call id on Message.ToolCall#698
winklemad wants to merge 1 commit into
ollama:mainfrom
winklemad:fix/toolcall-id

Conversation

@winklemad

Copy link
Copy Markdown

What & why

The Ollama server returns an id on each tool call in the response, e.g. {"id": "call_p7o2gz50", "function": {...}}. But Message.ToolCall has no id field, so it's silently dropped — and it isn't in model_extra either — leaving no way to read the tool-call id from Python (#667). That id is needed to correlate a tool call with its result.

Change

Add an optional id: Optional[str] = None to Message.ToolCall. It's optional/defaulted so responses that omit it (older servers) keep parsing unchanged.

Testing

tests/test_type_serialization.py: the id round-trips from a raw response and serializes back via model_dump(), and defaults to None when absent. Full suite: 94 passed. ruff check + ruff format clean.

Fixes #667

The server returns an id on each tool call (e.g. "call_p7o2gz50"), but ToolCall had no id field, so it was dropped and wasn't in model_extra either. Add an optional id field so it round-trips.

Fixes ollama#667

@vaderollama vaderollama left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM — adds the missing id field to Message.ToolCall matching the Go API (ToolCall.Id in api/types.go). Backwards-compatible (optional, defaults to None), tests cover round-tripping and default. Resolves #667.

@ParthSareen — ready to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

id in ToolCall missing

2 participants