-
Notifications
You must be signed in to change notification settings - Fork 390
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
Zod validation support, switch to Fetch API, and refactoring #147
Conversation
"axios": "^1.4.0", | ||
"typescript": "^5.1.3" | ||
"typescript": "^5.3.3", | ||
"zod": "^3.22.4" |
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.
Did the optional peer dependency trick not work out? (Happy to help get that working!)
👍 |
examples/music/package.json
Outdated
"typechat": "^0.0.10" | ||
"typechat": "^1.0.0" |
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.
This seems off, right?
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.
Yeah, we probably want a 0.1.0 change at most
examples/restaurant/package.json
Outdated
@@ -12,11 +12,11 @@ | |||
"license": "MIT", | |||
"dependencies": { | |||
"dotenv": "^16.3.1", | |||
"typechat": "^0.0.10" | |||
"typechat": "^1.0.0" |
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.
Seems like all of these got rev'd - I'm not sure why.
package.json
Outdated
@@ -1,7 +1,7 @@ | |||
{ | |||
"name": "typechat", | |||
"author": "Microsoft", | |||
"version": "0.0.10", | |||
"version": "1.0.0", |
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.
Woah, I don't think we want to bump to 1.0.0 - I see now why the other packages got updated!
translator.validator.stripNulls = true; | ||
const validator = createTypeScriptJsonValidator<CalendarActions>(schema, "CalendarActions"); | ||
const translator = createJsonTranslator(model, validator); | ||
//translator.stripNulls = true; |
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.
//translator.stripNulls = true; |
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.
Should we drop this?
function validate(jsonObject: object) { | ||
const result = targetType.safeParse(jsonObject); | ||
if (!result.success) { | ||
return error(result.error.issues.map(({ path, message }) => `${path.map(key => `[${JSON.stringify(key)}]`).join("")}: ${message}`).join("\"")); |
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.
In Python I just join the paths with a .
- which is not exactly correct of course, but it might be more readable in the usual case.
case z.ZodFirstPartyTypeKind.ZodNumber: | ||
return append("number"); |
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.
One of the things I'm wondering about is how you perform deduplication. For example, if a user wrote:
z.union([z.number().positive(), z.number().negative()])
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.
Currently it doesn't. Maybe that's fine for now.
*/ | ||
export interface TypeChatJsonValidator<T extends object> { | ||
/** | ||
* Return a string containing TypeScript source code for the validation schema. |
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.
One of the things I did in Python was I completely decoupled the translator and the validator. So here what you could do is create a ZodTranslator
and a ZodValidator
. The ZodTranslator
would know everything it needs for generating TypeScript and crafting the appropriate prompt for a model. The ZodValidator
would know everything it needs to validating input JSON.
While it does mean that you need a different kind of validator, I think that separation of concerns would make a lot more sense here. It would also decouple TypeChat validators from specifically talking about TypeScript as a schema language.
On the flip side, you could take the stance that the Python implementation implicitly couples the default translator to talking about TypeScript as a schema language - which itself might be worth discussing.
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.
What was your take on this idea @ahejlsberg?
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.
I prefer to have the validator have all knowledge of schema (including how to construct the schema string). This works much better in a dynamic schema scenario.
package.json
Outdated
"types": "./dist/index.d.ts", | ||
"default": "./dist/index.js" | ||
}, | ||
"./ts": { | ||
"types": "./dist/ts/index.d.ts", | ||
"default": "./dist/ts/index.js" | ||
}, | ||
"./zod": { | ||
"types": "./dist/zod/index.d.ts", | ||
"default": "./dist/zod/index.js" |
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.
This looks right to me, but figure we can check in with @andrewbranch and @jakebailey.
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.
Looks good, but can be simplified to
".": "./dist/index.js",
"./ts": "./dist/ts/index.js",
"./zod: "./dist/zod/index.js"
TS knows how to find sibling types.
Also, note that anyone accidentally using --moduleResolution node10
(still a default for some settings) will not be able to access these subpaths. TS will tell them to import from "typechat/dist/zod"
even though that won’t work in anything modern at runtime. It’s up to you whether you want to use that as an opportunity to get them to update their settings or provide a mitigation like one of these: https://github.com/andrewbranch/example-subpath-exports-ts-compat
If we're good with the current state of things, can you just pull from |
*/ | ||
export interface TypeChatJsonValidator<T extends object> { | ||
/** | ||
* Return a string containing TypeScript source code for the validation schema. |
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.
I prefer to have the validator have all knowledge of schema (including how to construct the schema string). This works much better in a dynamic schema scenario.
# Conflicts: # package-lock.json
This PR introduces the following changes:
axios
package.Zod support
The new Zod support is useful in applications that already incorporate JSON validation and/or dynamically change schema based on interaction history, databases, or other information sources. Also, Zod is significantly lighter weight than incorporating the TypeScript compiler for validation purposes.
Even though validation of JSON responses can now be performed by the Zod validator, schemas are still communicated to the LLM as TypeScript type definitions. The new
typechat/zod
validator automatically converts Zod schema to TypeScript source code suitable for inclusion in LLM prompts (and the conversion is available in a helper function for other purposes). In our experience, using TypeScript to describe schema to LLMs produces better outcomes and consumes fewer tokens than using JSON Schema.Two new examples,
examples/coffeeShop-zod
andexamples/sentiment-zod
, demonstrate the new Zod support. These are Zod equivalents of the corresponding examples that use the TypeScript compiler for validation.An example of a simple Zod schema for determining the sentiment of a some user input.:
In addition to the
createZodJsonValidator
function, the newtypechat/zod
module exports a convenientgetZodSchemaAsTypeScript
function that can be used to convert Zod schemas into TypeScript source code.Switch to Fetch API
The implementation has switched to using the JavaScript Fetch API instead of depending on the
axios
package. This helps reduce bundle sizes with no effect on client applications.Refactoring
The implementation has been refactored as follows:
typechat/ts
andtypechat/zod
modules (instead of the maintypechat
module).createJsonTranslator
function now takes avalidator
parameter instead of actually creating the validator.TypeChatJsonValidator
interface has been changed to have methods instead of properties for accessing the schema text and target type name, and thevalidate
method has been changed to take anobject
instead of astring
parameter.These are breaking changes, but quite easy to adopt.