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

[Go] fix go generate #243

Merged
merged 1 commit into from
May 24, 2024
Merged

[Go] fix go generate #243

merged 1 commit into from
May 24, 2024

Conversation

jba
Copy link
Contributor

@jba jba commented May 24, 2024

Some new types were added, and a new aspect of JSON Schema
that the code generator didn't know about.

The new aspect is that a "$ref" can refer to a property
inside a schema. The code generator still doesn't understand
these, but the config file handles them by ignoring them or
replacing them with another type.

Some new types were added, and a new aspect of JSON Schema
that the code generator didn't know about.

The new aspect is that a "$ref" can refer to a property
inside a schema. The code generator still doesn't understand
these, but the config file handles them by ignoring them or
replacing them with another type.
@@ -39,10 +53,16 @@ const (
FinishReasonUnknown FinishReason = "unknown"
)

type DataPart struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be dataPart? The other Part types (textPart, mediaPart) are not exported.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See below.

@@ -144,16 +148,27 @@ ToolResponsePartToolResponse pkg ai
Part pkg ai
TextPart pkg ai
TextPart name textPart
TextPart.data omit
Copy link
Contributor

Choose a reason for hiding this comment

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

Any idea what these new Part fields are used for? Should we be using them in document.go somehow?

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've asked Keith to look into this. I just got things working so I could continue my API refactoring.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the js side just split the part that we call "blob" into two, "media" and "data". I will make that same change in our code.

@jba jba merged commit 5360a11 into main May 24, 2024
5 checks passed
@jba jba deleted the jba-fix-gen branch May 24, 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.

3 participants