-
Notifications
You must be signed in to change notification settings - Fork 80
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
[easy][ts] 2/n Set run output type to be Output[] instead of Union #618
Conversation
83d63eb
to
7ef27c8
Compare
@@ -398,7 +398,7 @@ export class AIConfigRuntime implements AIConfig { | |||
promptName: string, | |||
params: JSONObject = {}, | |||
options?: InferenceOptions | |||
) { | |||
): Promise<Output[]> { |
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.
line 430 below guarantees the output of run() is an array of outputs
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 mostly lgtm. One thing I'd suggest as followup is cleaning up callsites where we check for array or singular output, since now that can all be simplified
…617) [eng week] Set run output type to be list[Output] instead of Output This diff is the same as #343, rebased onto main. Sapling decided to create a new pr. Makes it more scalable. The Prompt schema already refers to `outputs` as `List[Output]` anyways --- Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/617). * #618 * __->__ #617
This diff is the same as #343, rebased onto main. Sapling decided to create a new pr. Makes it more scalable. The Prompt schema already refers to `outputs` as `List[Output]` anyways
## What Updating the types in the ts sdk to return `Output[]` instead of `Output | Output[]` ## Why Run method should always return a array of outputs, even if its just one output. The Prompt schema already refers to `outputs` as `outputs?: Output[];` anyways ## Testplan Ran tests without errors. No type errors on compilation either <img width="997" alt="Screenshot 2023-12-26 at 1 16 46 PM" src="https://github.com/lastmile-ai/aiconfig/assets/141073967/a5e3f16f-39e4-4325-95e1-6ac837daeab0">
7ef27c8
to
c91d8bf
Compare
|
created #645 |
[easy][ts] 2/n Set run output type to be Output[] instead of Union
What
Updating the types in the ts sdk to return
Output[]
instead ofOutput | Output[]
Why
Run method should always return a array of outputs, even if its just one output.
The Prompt schema already refers to
outputs
asoutputs?: Output[];
anywaysTestplan
Ran tests without errors. No type errors on compilation either
Stack created with Sapling. Best reviewed with ReviewStack.