Skip to content
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

Add JsonSchemaRule #1165

Merged
merged 2 commits into from
Sep 11, 2024
Merged

Add JsonSchemaRule #1165

merged 2 commits into from
Sep 11, 2024

Conversation

collindutter
Copy link
Member

@collindutter collindutter commented Sep 10, 2024

Describe your changes

Added

  • JsonSchemaRule for instructing the LLM to output a JSON object that conforms to a schema.

Issue ticket number and link

NA


📚 Documentation preview 📚: https://griptape--1165.org.readthedocs.build//1165/

@collindutter
Copy link
Member Author

Question: should this be the mechanism by which we enable features like OpenAI Structured Outputs? If so, we'll need to make the prompt rendering happen at the Driver level rather than the Task level.

Copy link

codecov bot commented Sep 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

dylanholmes
dylanholmes previously approved these changes Sep 11, 2024
Copy link
Contributor

@dylanholmes dylanholmes left a comment

Choose a reason for hiding this comment

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

Nice!

Approving because I think there is some immediate value to this. Also, I like the choice of using a rule to enforce json schema, since that's the same as what you'd do now. It's intuitive imo.

Even if we need to change some things to make it work with say OpenAI Structure Outputs, I like this as a "Rule" from a API perspective.

@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added
- Parameter `meta: dict` on `BaseEvent`.
- `AzureOpenAiTextToSpeechDriver`.
- `JsonSchemaRule` for instructing the LLM to output a JSON object that conforms to a schema.
Copy link
Contributor

Choose a reason for hiding this comment

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

Responding with an inline comment, so it's easier to reply.

Question: should this be the mechanism by which we enable features like OpenAI Structured Outputs? If so, we'll need to make the prompt rendering happen at the Driver level rather than the Task level.

If we didn't use this mechanism for OpenAI Structured Outputs, then what would the mechanism for that look like? Would it require passing a json schema? (I would expect so) If it requires passing a json schema, then we'd have two choices, which doesn't seem like the right approach (imagine that we had taken that approach for tools 🤮 ).

Seems like making prompt rendering at the Driver level is the way forward because I don't really see duplicate ways of passing in a json schema as a viable option. This could technically wait until introducing native structured outputs, right?

(Also, if we do use this rule for native structure outputs, what mechanism will we use to opt-in/out of structure outputs when a driver supports it? Should we that the same approach as a use_native_tools flag?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Would it require passing a json schema?
Yes, it'd likely require passing a schema to the Prompt Driver 🤢.

Rendering at the Prompt Driver level allows us to lean on more provider-specific nuances so I think it's a worthwhile change. But as you point out it's not required for this PR -- this can still serve as a good intermediate solution.

dylanholmes
dylanholmes previously approved these changes Sep 11, 2024
Copy link
Contributor

@dylanholmes dylanholmes left a comment

Choose a reason for hiding this comment

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

Yay!

Optional suggestions on the sample

dylanholmes
dylanholmes previously approved these changes Sep 11, 2024
dylanholmes
dylanholmes previously approved these changes Sep 11, 2024
Copy link
Contributor

@dylanholmes dylanholmes left a comment

Choose a reason for hiding this comment

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

x2

@collindutter collindutter merged commit 2ae50b3 into dev Sep 11, 2024
15 checks passed
@collindutter collindutter deleted the feature/json-output branch September 11, 2024 21:15
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.

2 participants