Add basic image generation support; introduce new ToolBuiltIn class #214
Conversation
cpsievert
force-pushed
the
feat/tool-builtin
branch
from
30d0125 to
6afe843
Compare
cpsievert
force-pushed
the
feat/tool-builtin
branch
from
6afe843 to
c4b4e1f
Compare
cpsievert
force-pushed
the
feat/tool-builtin
branch
from
c4b4e1f to
552284b
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for image generation and introduces a new ToolBuiltIn class to handle provider-specific built-in tools. The key changes enable the framework to differentiate between custom tools (regular Tool instances) and provider-native tools (like image generation) that pass raw provider-specific JSON directly through to the API.
Key changes:
- Introduced
ToolBuiltInclass for provider-specific tools that pass raw definitions to APIs - Updated all provider implementations to accept
Tool | ToolBuiltInin tool-related signatures - Added image generation support for OpenAI provider with proper MIME type detection
- Enhanced Google provider to handle inline image data responses
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| chatlas/_tools.py | Adds new ToolBuiltIn class definition and exports it |
| chatlas/_provider_snowflake.py | Updates type signatures to accept ToolBuiltIn and raises NotImplementedError for built-in tools |
| chatlas/_provider_openai_generic.py | Updates token counting signatures to support ToolBuiltIn |
| chatlas/_provider_openai_completions.py | Handles both Tool and ToolBuiltIn in schema generation |
| chatlas/_provider_openai.py | Implements image generation result handling and supports ToolBuiltIn in tool parameters |
| chatlas/_provider_google.py | Adds inline image data handling and filters out ToolBuiltIn from tool conversion |
| chatlas/_provider_anthropic.py | Updates tool schema generation to handle ToolBuiltIn instances |
| chatlas/_provider.py | Updates abstract provider interface signatures for ToolBuiltIn support |
| chatlas/_mcp_manager.py | Updates tool storage type to support ToolBuiltIn |
| chatlas/_content.py | Adds from_tool support for ToolBuiltIn and Jupyter display methods for images |
| chatlas/_chat.py | Updates tool registration, invocation logic, and type checking for ToolBuiltIn |
| chatlas/init.py | Exports ToolBuiltIn class |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if mime_type and data: | ||
| contents.append( | ||
| ContentImageInline( | ||
| data=data.decode("utf-8"), |
There was a problem hiding this comment.
The data variable is already a string (base64-encoded) from the API response, but it's being treated as bytes and decoded. This will cause an AttributeError since strings don't have a .decode() method. Remove the .decode('utf-8') call and pass data directly.
| data=data.decode("utf-8"), | |
| data=data, |
| tool = Tool.from_func(func, name=name, model=model, annotations=annotations) | ||
| else: | ||
| if isinstance(func, ToolBuiltIn): | ||
| tool = func | ||
| else: | ||
| tool = Tool.from_func( | ||
| func, name=name, model=model, annotations=annotations |
There was a problem hiding this comment.
The logic for creating a Tool from a function is duplicated on lines 1985 and 1990-1992. Consider restructuring to eliminate this duplication by handling the ToolBuiltIn case first, then having a single Tool.from_func call for all other cases.
| def _repr_png_(self): | ||
| """Display PNG images directly in Jupyter notebooks.""" | ||
| if self.image_content_type == "image/png" and self.data: | ||
| import base64 | ||
|
|
||
| return base64.b64decode(self.data) | ||
| return None | ||
|
|
||
| def _repr_jpeg_(self): | ||
| """Display JPEG images directly in Jupyter notebooks.""" | ||
| if self.image_content_type == "image/jpeg" and self.data: | ||
| import base64 | ||
|
|
||
| return base64.b64decode(self.data) | ||
| return None | ||
|
|
There was a problem hiding this comment.
Need to think through non-notebook contexts as well -- maybe for another PR though
Pairs with tidyverse/ellmer#795