-
Notifications
You must be signed in to change notification settings - Fork 199
(#178) Fix structured output JSON descriptions #191
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
(#178) Fix structured output JSON descriptions #191
Conversation
Properly handle description annotation during schema generation
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.
@EugeneTheDev please also add a test with a lot of different descriptors:
- class descriptor via annotation
- property descriptor via annotation
- property of Serializable type, also with field, each of them annotated
- one field is annotated and also overriden by the map
- one field is NOT annotated but provided through the map
- what else can be an edge case?
* @param schemaFormat Format of the generated schema, can be simple or detailed. | ||
* @param maxDepth Maximum recursion depth when generating schema to prevent infinite recursion for circular references. | ||
* @param descriptions Optional map of serial class names and property names to descriptions. | ||
* If a property/type is already described with [LLMDescription] annotation, value from the map will override this description. |
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.
Let's also please add good example in KDoc about how to form the descriptions map.
I mean, "className.propertyName" to "property description"
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.
Also, maybe let's name it descriptionOverrides
if it's an overriding map?
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.
Regarding testing edge cases, that's what I already modified in tests, to cover these cases you described. I didn't mark each field with annotation though because it looks redundant, and it's also nice to test that description is added only for these fields where it is specified, and is skipped for other fields.
Regarding examples and parameter renaming: sounds good, will do it
Co-authored-by: Damjan Polugic <[email protected]>
Based on #188 and fixes #178 by properly handling
@LLMDescription
during JSON schema generation. Also updated the tests to include the annotation too and simplified structured output example to use existing node