-
-
Notifications
You must be signed in to change notification settings - Fork 234
Support nested parameters for tools #112
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.
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
Conversation
Does this qualify? What am I missing? I guess docs. Do we need to add something to with_tool? Seems like opting in to complex parameters by just putting the schema on the params is pretty straightforward. Do you like #11 (comment) better which adds "schema" as a parameter to Param? |
I guess I'm missing a bunch of the json schema by just supporting items and properties, like for example, enums. |
@tpaulshippy Thanks for putting this together! Yeah, you're right, after thinking of it and using my monkey patch in an app, I'm realizing that the Parameters class is kind of pointless except now for reverse-compatibility. The hash is a JSON schema - it's already what it needs to be. In the future when more complex JSON schema structures are supported it'd be better not to have to update the library (ie I'm waiting eagerly for I'll post a comment inline via a review of what my idea is. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's my idea for a reverse-compatible Parameter
class that supports future schema.
The LLMs change so often it'd be great not to have to update this library everytime a new schema feature is supported by the LLM. Also kind of simplifies the code.
type: param.type, | ||
description: param.description | ||
}.compact | ||
clean_parameter(param) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should just let the parameter class return the schema. This way it is more forward compatible when LLMs add schema support (like minItems, etc)
Here's an implementation of RubyLLM::Parameter that is reverse compatible and can simply get .schema
on the parameter:
# In `lib/ruby_llm/providers/*/tools.rb`:
##
# @param parameters [Hash{String => Parameters}]
# @return [Hash]
def clean_parameters(parameters)
parameters.transform_values(&:schema)
end
# In `lib/ruby_llm/tool.rb`:
# ...
##
# @!attribute name [r]
# @return [Symbol]
# @!attribute required [r]
# @return [Boolean]
# @!attribute schema [r]
# @return [Hash{String => Object}]
class Parameter
attr_reader :name, :required, :schema
##
# If providing schema directly MAKE SURE TO USE STRING KEYS.
# Also note that under_scored keys are NOT automatically transformed to camelCase.
# @param name [Symbol]
# @param schema [Hash{String => Object}, NilClass]
# @option type [String] (default: `'string'`)
# @options required [Boolean] (default: `true`)
# @options desc [String]
def initialize(name, schema = nil, required: true, type: 'string', desc: nil)
@name = name
@schema = schema.to_h
@schema['type'] ||= type
@schema['description'] ||= desc if desc.present?
@required = required
end
##
# @return [String]
def type
@schema['type']
end
##
# @return [String, NilClass]
def description
@schema['description']
end
alias required? required
end
# ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love it! Did you want to submit a separate PR or should I try to get it into this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uhh I'm realizing maybe I should just do a separate PR since I should test the code thoroughly.
I realized my solution is now using string and symbol keys so just want to ensure we're not breaking anything 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to borrow from this one as much as you need.
Closing in favor of #124 |
Purpose
Support calling tools that take arrays or nested objects.
Approach
Simply define the items and properties as part of the Parameter class. Output hashes.
Related to #76 (although I didn't actually test enums). I realize this probably doesn't align perfectly with #65 or #90 but I kinda need this now and this seems to be the simplest approach.
Testing
Tested with Bedrock, Gemini, and OpenAI - I do not currently have ready access to Anthropic or Deepseek.