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

allow responsechema as example #608

Merged
merged 5 commits into from
Aug 7, 2024
Merged

allow responsechema as example #608

merged 5 commits into from
Aug 7, 2024

Conversation

pelikhan
Copy link
Member

@pelikhan pelikhan commented Aug 7, 2024

  • The PromptParameterType type definition now supports object types. This enables complex parameter structures like nested objects and arrays. Your API has leveled up! 🎉

  • The responseSchema property in ModelOptions can now accept PromptParametersSchema or JSONSchemaObject. This allows users to define the response structure at a higher level. More freedom for you! 🦅

  • Added a new function promptParametersSchemaToJSONSchema to convert a PromptParametersSchema to a JSONSchema. This function is now used in various parts of the codebase, wherever a JSONSchema was needed from parameters. Stay DRY! 💦

  • In the functions applyRepairs and structurifyChatSession, the responseSchema is now converted to a JSONSchema before being passed to the validateJSONWithSchema function. 🔐

  • The callExpander function has been updated to use responseSchema in its JSONSchemaObject form. This way, it uses the high-level schema defined in templates. Consistency is key! 🔑

  • Updated the toStrictJSONSchema function to convert PromptParametersSchema to JSONSchema before applying transformations. This ensures the schema is always treated in its JSON form.🔄

  • There has been a modification in the file json_object_ex.genai.mjs, specifying a new responseSchema format that includes complex nested objects. 📁

Overall, these changes provide better utility to users, allowing them to use more complex and nested parameters and schemas. The codebase is also more DRY and consistent as higher level schemas are always transformed to JSONSchema before use. Amazing upgrades! 🥳

generated by pr-describe

script({
responseType: "json_object",
responseSchema: { name: "neo", age: 30 },
})
Copy link

Choose a reason for hiding this comment

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

Consider adding a description for the responseSchema example to clarify its purpose and usage.

generated by pr-docs-review-commit example_schema

Copy link

github-actions bot commented Aug 7, 2024

From the GIT_DIFF, it appears that the changes primarily deal with JSON schema handling and validation for chat responses and prompt templates in TypeScript. The changes include:

  • An additional import for promptParametersSchemaToJSONSchema in chat.ts, expander.ts, and runpromptcontext.ts.
  • Modifying their respective responseSchema handling to convert parameters into JSON Schemas using promptParametersSchemaToJSONSchema.
  • Introducing error tracing for invalid JSON validation in chat.ts.
  • Adding checking for object type in parameters.ts and converting them to JSONSchemaObjects.
  • Updating the promptParameterTypeToJSONSchema and promptParametersSchemaToJSONSchema functions to handle objects as a possible type and convert into JSONSchemaObjects.
  • Implementing promptParametersSchemaToJSONSchema in schema.ts for converting the schema to strict JSON schema.
  • Updating PromptParameters and PromptParametersSchema in prompt_template.d.ts to include objects as a possible type.

My concern is, while the changes seem to handle object types in the parameters, the code appears to implicitly assume that these objects will always be valid JSON schemas. This could potentially lead to errors if an object is passed that doesn't conform to a valid JSON schema. It would be good to include some form of validation or error handling for this scenario.

Other than that, LGTM 🚀

generated by pr-review

responseSchema: { name: "neo", age: 30 },
})
```

Copy link

Choose a reason for hiding this comment

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

The documentation has been updated to include an example of how to generate a schema using an object example in the responseSchema field. Ensure that this addition is consistent with the rest of the documentation and that the example provided is clear and accurate.

generated by pr-docs-review-commit documentation_update

@pelikhan pelikhan changed the title towrads more flexible schema definitions allow responsechema as example Aug 7, 2024
t.type !== "array" &&
t.default !== undefined
)
res[key] = t.default
Copy link

Choose a reason for hiding this comment

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

Default values for object and array types are not handled in the parsePromptParameters function. This could lead to unexpected behavior when these types are used in the parameters schema.

generated by pr-review-commit object_default

t.default !== undefined &&
t.default !== null
)
res.required.push(k)
Copy link

Choose a reason for hiding this comment

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

The condition for adding a field to the required array in the promptParametersSchemaToJSONSchema function excludes object and array types. This could lead to these types being unexpectedly optional in the resulting schema.

generated by pr-review-commit required_field

if (typeof s === "string") return s
else if (typeof s === "number") return s.toLocaleString()
else if (typeof s === "boolean") return s ? "true" : "false"
else if (typeof s === "object") throw new Error("object not supported")
Copy link

Choose a reason for hiding this comment

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

The normalizeString function throws an error when an object is passed. This could lead to unexpected crashes when objects are used in the parameters.

generated by pr-review-commit object_normalization

else if (t?.type === "boolean")
res[key] = /^\s*(y|yes|true|ok)\s*$/i.test(vars[key])
res[key] = /^\s*(y|yes|true|ok)\s*$/i.test(vars[key] + "")
else if (t?.type === "string") res[key] = vars[key]
Copy link

Choose a reason for hiding this comment

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

Parsing for object and array types is not handled in the parsePromptParameters function. This could lead to incorrect parsing when these types are used in the parameters schema.

generated by pr-review-commit object_parse

if (typeof s === "string") return s
else if (typeof s === "number") return s.toLocaleString()
else if (typeof s === "boolean") return s ? "true" : "false"
else if (typeof s === "object") throw new Error("object not supported")
Copy link

Choose a reason for hiding this comment

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

The normalizeString, normalizeFloat, and normalizeInt functions throw an error when an object is passed. This could lead to unexpected errors when these functions are used with object values.

generated by pr-review-commit object_normalize

else if (t?.type === "boolean")
res[key] = /^\s*(y|yes|true|ok)\s*$/i.test(vars[key])
res[key] = /^\s*(y|yes|true|ok)\s*$/i.test(vars[key] + "")
else if (t?.type === "string") res[key] = vars[key]
Copy link

Choose a reason for hiding this comment

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

Default values for object and array types are not handled properly. This could lead to unexpected behavior when these types are used in the parameters.

generated by pr-review-commit object_default

if (typeof s === "string") return s
else if (typeof s === "number") return s.toLocaleString()
else if (typeof s === "boolean") return s ? "true" : "false"
else if (typeof s === "object") throw new Error("object not supported")
else return undefined
Copy link

Choose a reason for hiding this comment

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

The normalizeString, normalizeFloat, and normalizeInt functions throw an error when an object is passed. This could lead to unexpected crashes if objects are used as parameters.

generated by pr-review-commit object_not_supported

promptParameterTypeToJSONSchema(v),
])
),
}
Copy link

Choose a reason for hiding this comment

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

The handling of objects in the promptParameterTypeToJSONSchema function could be improved. Currently, it assumes that all objects are of type object and it does not handle nested objects or arrays. This could lead to incorrect schema generation for complex objects.

generated by pr-review-commit object_handling

responseSchema: { characters: [{ name: "neo", age: 30 }] },
})
```

Copy link

Choose a reason for hiding this comment

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

The example provided for generating a schema is incorrect. The responseSchema should define the schema structure with type, properties, and required fields, not an instance of the object.

generated by pr-docs-review-commit schema_example_incorrect

t.type !== "array" &&
t.default !== undefined
)
res[key] = t.default
Copy link

Choose a reason for hiding this comment

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

The handling of default values in the parsePromptParameters function could be improved. Currently, it only sets default values for non-object and non-array types. This could lead to missing default values for nested properties in objects or arrays.

generated by pr-review-commit default_handling

@@ -212,28 +212,31 @@ export function groupBy<T>(
return r
}

export function normalizeString(s: string | number | boolean): string {
export function normalizeString(s: string | number | boolean | object): string {
Copy link

Choose a reason for hiding this comment

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

The normalizeString function in util.ts throws an error when the input is an object. This could lead to unexpected errors if the function is used with object inputs. Consider adding support for object inputs or clearly documenting this limitation.

generated by pr-review-commit normalize_string

@pelikhan pelikhan merged commit 3a896eb into main Aug 7, 2024
10 checks passed
@pelikhan pelikhan deleted the responseschemaparam branch August 7, 2024 16:31
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.

1 participant