-
-
Notifications
You must be signed in to change notification settings - Fork 148
Add Schema builder for Structured Outputs #90
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
base: main
Are you sure you want to change the base?
Conversation
Nice! I started some implementation in #65 as well, but it is very overlapping. My approach is that people can either use a Let me know if you want to join forces on this. |
Hey @kieranklaassen! I did see your PR, but noticed that it included the parser concept too, and in all honesty, it felt a bit overwhelming to dive into, so I wanted to suggest a simple approach for structured outputs only (initially). I'm game to join efforts — yours or mine — but let's maybe have @crmne's perspective on the direction before we spend too much time? |
Yes! For sure! I'm trying to build something I can actually use in production so kind of need that too but we can simplify. I love to keep things simple. Let's wait for @crmne to give his perspective and we can divide and conquer together |
@kieranklaassen feel free to take a look and see if I've missed anything and if it aligns with your expectations! |
@danielfriis I like your code, but on a high level I'm kind of against adding DSL to JSON Schema because IMO it's already it's own DSL. Then you're having to learn a DSL for a DSL. This SDK core value is it abstracts the LLM-specific code with a simple interface. Since JSON schema is already abstracted, we don't need to add another abstraction layer on that, ya know? That's just my opinion ... and that being said, your DSL here is quite simple and lovely - so good work! |
I hear you! To me, the JSON structure is not intuitive though. Not unlike how tools are also covered by the gem. But it's not a hill I will die on :) The gem works fine with just your new PR and I can run this next to the gem. |
I agree that the schema language itself isn't super intuitive. I think that's the value a DSL like yours brings.
Same :). I guess tools like Copilot now make me now lean towards not needing DSL since they autocomplete schema definitions anyways. (prob leaned the other way just last year!) |
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.
Thanks for your work @danielfriis!
Had a quick skim and I like the code.
However, isn't there any library that is already a great DSL for JSON Schemas? The core philosophy of RubyLLM is to not have to rely on any library for the core AI functionality because of how fast things change in that world, but for JSON Schemas I'm sure we can settle on a DSL made by someone else and actively maintained instead of maintaining our own
Thanks a lot @crmne ! There are gems covering this (also specifically for LLMs). Personally, I just don't like to rely on other libraries for core concepts — especially if the implementation is simple. My initial thought was to make it part of a Structured Output / JSON mode PR ( #131 ). To me, they go together as the current tool implementation has both a tool definition side, and a execution side. |
@crmne Take a look at EasyTalk for a JSON schema DSL that looks very clean and straightforward. It's being used by the Instructor gem for AI as well |
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.
Overall: Solid Foundation, Needs Polish
This is a great start! The core concept is sound and the DSL feels very Ruby-esque. However, there are several areas that need refinement before it's ready for extraction into a separate gem.
API Design Issues
1. Method Overloading Confusion
def string(name = nil, enum: nil, description: nil)
schema = { type: 'string', enum: enum, description: description }.compact
name ? add_property(name, schema) : schema
end
This is doing double duty:
string(:name)
→ adds property to current schemastring()
→ returns raw schema definition{ type: 'string' }
This is bad API design. The same method doing two different things based on whether you pass a name? That's confusing.
The real question is: why do you need to get the raw schema at all? Looking at your code, it seems like it's only used for the array method:
when :string, :number, :boolean, :null
send(type) # calls string() with no args
But this is over-engineering. You could just handle this internally:
def string(name, **options)
add_property(name, { type: 'string', **options }.compact)
end
def array(name, of:, **options)
items_schema = case of
when :string then { type: 'string' }
when :number then { type: 'number' }
when :boolean then { type: 'boolean' }
when Class then ref(of) # For schema classes
else raise ArgumentError, "Unknown array type: #{of}"
end
add_property(name, { type: 'array', items: items_schema, **options }.compact)
end
# Usage becomes clear:
array :tags, of: :string
array :contacts, of: ContactSchema
2. Method Missing is Over-Engineered
class SchemaCollector
def method_missing(method_name, ...)
if Schema.respond_to?(method_name)
@schemas << Schema.send(method_name, ...)
else
super
end
end
end
This allows:
any_of :status do
string enum: %w[active pending] # Now string() behaves differently!
null
end
This is confusing because now string
behaves differently inside the any_of
block vs outside it. Outside the block, string(:name)
adds a property. Inside the block, string(enum: [...])
returns a schema definition.
Much simpler:
# Just pass an array:
any_of :status, [
{ type: 'string', enum: %w[active pending] },
{ type: 'null' }
]
# Or helper methods:
any_of :status, [
string_type(enum: %w[active pending]),
null_type
]
3. Required vs Optional Fields
# Currently everything is required - no way to make optional fields
string :name # Always required
number :age # Always required
# We need something like:
string :name
string :nickname, required: false
# or
optional { string :nickname }
Implementation Concerns
1. Remove Unnecessary Validation Limits
# These shouldn't exist:
MAX_OBJECT_PROPERTIES = 100
MAX_NESTING_DEPTH = 5
def validate_schema
# ... validation logic
end
If I want to create a schema with 500 properties because I'm modeling some complex domain object, that's my business. The library shouldn't be the schema police. These limits smell like they came from some specific OpenAI API constraints that got baked into what should be a general-purpose tool.
2. Better Error Handling
# Instead of generic strings:
raise 'Exceeded maximum number of object properties'
# Use proper exception classes:
raise RubyLLM::StructuredOutput::InvalidSchemaError,
"Invalid schema definition: #{details}"
Ruby Style Issues
1. Naming Consistency
# Method should return what the name suggests:
def json_schema
# Returns Hash, but name suggests JSON string
end
# Better:
def to_json_schema # Returns Hash
def to_json # Returns JSON string
2. Make the Common Case Beautiful
# This is verbose for simple schemas:
class PersonSchema < RubyLLM::Schema
string :name
number :age
end
# Consider shorthand for simple cases:
PersonSchema = RubyLLM::Schema.define do
string :name
number :age
end
# Or even:
person_schema = schema do
string :name
number :age
end
Missing Features
1. Schema Composition
# Should be able to compose schemas:
class AddressSchema < RubyLLM::Schema
string :street
string :city
end
class PersonSchema < RubyLLM::Schema
string :name
schema :address, AddressSchema # Embed another schema
end
2. Schema Validation (for the schema itself)
# Should validate the schema definition:
def validate!
# Check for circular references
# Validate against JSON Schema spec
# Ensure field names are valid
end
Test Improvements
Your tests are good but need more edge cases:
# Test error conditions:
it 'raises when invalid field names'
it 'handles circular references gracefully'
# Test schema composition:
it 'can embed other schemas'
it 'validates schema definitions'
# Test optional fields:
it 'generates optional properties correctly'
Recommendation
This is definitely worth extracting into a separate gem! The core concept is solid. Focus on:
- Fix the method overloading -
string()
doing two different things is confusing - Remove the method_missing magic - makes the API unpredictable
- Remove unnecessary validation limits - let users create the schemas they need
- Add optional fields - most schemas need some optional properties
- Simplify the any_of API - just use arrays instead of magic blocks
- Better error handling - proper exception classes with helpful messages
- Polish the Ruby style - consistent naming and patterns
- More comprehensive tests - edge cases and error conditions
The foundation is excellent - with these refinements, this could be a really compelling addition to the Ruby ecosystem!
Would you be up for addressing these issues? Happy to help review iterations as you work through them.
Suggesting an approach similar to how
RubyLLM::Tool
is implemented, allowing for this:And then allowing to call
.json_schema
on the class as input for using structured outputs.