Skip to content

Structured outputs #463

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

Open
wants to merge 5 commits into
base: next
Choose a base branch
from
Open

Structured outputs #463

wants to merge 5 commits into from

Conversation

damo
Copy link
Contributor

@damo damo commented May 5, 2025

Includes optional local JSON schema validation and draft SDK documentation.

@damo damo requested a review from a team as a code owner May 5, 2025 15:44
README.md Outdated
Comment on lines 346 to 347
be derived automatically from the structure of an arbitrary Java class. The response will then
automatically convert the generated JSON content to an instance of that Java class.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
be derived automatically from the structure of an arbitrary Java class. The response will then
automatically convert the generated JSON content to an instance of that Java class.
be derived automatically from the structure of an arbitrary Java class. The response's JSON will then
be automatically converted to an instance of that Java class.

Cause the response is not what does the converting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The switch from active to passive voice doesn't scan as well. Read "the response" as "the process of responding" rather than as HttpResponse and see if you feel more comfortable about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll change it anyway, as the previous sentence is passive, so they can all be the same.

README.md Outdated
```java
class Person {
public String name;
public int yearOfBirth;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe birthYear?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to make them a bit more English-y so that the AI model could interpret them with less fuss, but, sure, I''ll change that for you. I'll need to change yearPublished to publicationYear too, for consistency.

README.md Outdated
StructuredChatCompletionCreateParams<BookList> params = ChatCompletionCreateParams.builder()
.addUserMessage("List six famous nineteenth century novels.")
.model(ChatModel.GPT_4_1)
.responseFormat(BookList.class, false) // Disable local validation.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't like this second false parameter because it's hard to tell what it means without reading the documentation... In general, boolean positional params are a bit sad

Maybe instead we should have responseFormat(Class<T>) and responseFormatUnchecked(Class<T>) or responseFormatUnvalidated(Class<T>)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough. How about one of these?

  • responseFormatWithoutValidation
  • responseFormatNoValidation
  • responseFormatSkipValidation
  • responseFormatLenient
  • responseFormatNonStrict
  • responseFormatAllowInvalid
  • responseFormatPlain

@@ -27,6 +27,8 @@ dependencies {
implementation("com.fasterxml.jackson.module:jackson-module-kotlin:2.18.2")
implementation("org.apache.httpcomponents.core5:httpcore5:5.2.4")
implementation("org.apache.httpcomponents.client5:httpclient5:5.3.1")
implementation("com.github.victools:jsonschema-generator:4.38.0")
implementation("com.github.victools:jsonschema-module-jackson:4.38.0")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Curious what version of jackson this depends on? Just wanna make sure it's compatible with Jackson 2.13.4

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAICT, it depends on Jackson 2.17.2 (see here).

.addModule(JavaTimeModule())
.build()

internal fun <T> fromClass(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
internal fun <T> fromClass(
@JvmSynthetic
internal fun <T> fromClass(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added that to all three functions in StructuredOutputs.kt. We can remove it again if we decide to make any of them public.

Comment on lines 14 to 15
val responseFormat: Class<T>,
val chatCompletion: ChatCompletion,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
val responseFormat: Class<T>,
val chatCompletion: ChatCompletion,
@get:JvmName("responseFormat") val responseFormat: Class<T>,
@get:JvmName("rawChatCompletion") val rawChatCompletion: ChatCompletion,

Otherwise I think Kotlin will generate getResponseFormat() and getChatCompletion() methods, instead of non-get prefixed

Comment on lines 71 to 72
internal val responseFormat: Class<T>,
internal val choice: ChatCompletion.Choice,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
internal val responseFormat: Class<T>,
internal val choice: ChatCompletion.Choice,
@get:JvmName("responseFormat") val responseFormat: Class<T>,
@get:JvmName("rawChoice") val rawChoice: ChatCompletion.Choice,


class StructuredChatCompletionCreateParams<T : Any>
internal constructor(
val responseFormat: Class<T>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
val responseFormat: Class<T>,
@get:JvmName("responseFormat") val responseFormat: Class<T>,

Comment on lines 12 to 13
val responseFormat: Class<T>,
val chatCompletionMessage: ChatCompletionMessage,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
val responseFormat: Class<T>,
val chatCompletionMessage: ChatCompletionMessage,
@get:JvmName("responseFormat") val responseFormat: Class<T>,
@get:JvmName("rawChatCompletionMessage") val rawChatCompletionMessage: ChatCompletionMessage,

Comment on lines 204 to 206
if (isValidationComplete) {
throw IllegalStateException("Validation already complete.")
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (isValidationComplete) {
throw IllegalStateException("Validation already complete.")
}
check (!isValidationComplete) { "Validation already complete." }

@damo damo force-pushed the damo/structured-outputs branch from 8c0d60a to 67381ff Compare May 7, 2025 10:32
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