-
Notifications
You must be signed in to change notification settings - Fork 3.5k
fix: make validateTypes() and safeValidateTypes() async
#5779
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
Conversation
635465b to
f05c9b2
Compare
safeValidateTypes() asyncvalidateTypes() and safeValidateTypes() async
| }), | ||
| ); | ||
|
|
||
| const resolvedOutput = await output?.parseOutput( |
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.
Instead of passing an outputResolver() callback, the output needs to be computed ahead of instantiating the DefaultGenerateTextResult class, in order to retain the synchronous .experimental_output getter API
|
To keep track, here are the methods that were previously sync and are now async:
|
| sources, | ||
| outputResolver: () => { | ||
| if (output == null) { | ||
| throw new NoOutputSpecifiedError(); |
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.
Can this error be removed?
| if (this.resolvedOutput == null) { | ||
| throw new NoOutputSpecifiedError(); | ||
| } |
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.
null output might be valid in some cases. We can remove the error imo
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 was the original reason for delaying the check)
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.
removing the handling of falsy values leads to a type error. The generateText() method could in theory end up returning null or undefined. Shall we update the type?
packages/ai/core/generate-text/generate-text.ts:243:3 - error TS2322: Type 'DefaultGenerateTextResult<TOOLS, Awaited<OUTPUT> | undefined>' is not assignable to type 'GenerateTextResult<TOOLS, OUTPUT>'.
Types of property 'experimental_output' are incompatible.
Type 'Awaited<OUTPUT> | undefined' is not assignable to type 'OUTPUT'.
'OUTPUT' could be instantiated with an arbitrary type which could be unrelated to 'Awaited<OUTPUT> | undefined'.
243 return recordSpan({
~~~~~~
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 create a ticket to revisit this later
|
Close/re-open to trigger actions. |
97bad38 to
e329b0a
Compare
validateTypes() and safeValidateTypes() asyncvalidateTypes() and safeValidateTypes() async
Background
Towards #5766
Tasks
pnpm prettier-fixto fix any formatting issues